Add names for link shares (#829)

Co-authored-by: kolaente <k@knt.li>
Reviewed-on: vikunja/api#829
Co-authored-by: konrad <konrad@kola-entertainments.de>
Co-committed-by: konrad <konrad@kola-entertainments.de>
This commit is contained in:
konrad 2021-04-07 12:44:39 +00:00
parent b7d832891a
commit 84291679cc
24 changed files with 398 additions and 56 deletions

View File

@ -209,3 +209,9 @@
created_by_id: 1
created: 2020-04-18 21:13:52
updated: 2020-04-18 21:13:52
- id: 35
title: testbucket35
list_id: 23
created_by_id: -2
created: 2020-04-18 21:13:52
updated: 2020-04-18 21:13:52

View File

@ -9,3 +9,8 @@
file_id: 9999
created_by_id: 1
created: 2018-12-01 15:13:12
- id: 3
task_id: 1
file_id: 1
created_by_id: -2
created: 2018-12-01 15:13:12

View File

@ -94,3 +94,9 @@
task_id: 36
created: 2020-02-19 18:07:06
updated: 2020-02-19 18:07:06
- id: 17
comment: comment 17
author_id: -2
task_id: 35
created: 2020-02-19 18:07:06
updated: 2020-02-19 18:07:06

View File

@ -338,5 +338,11 @@
bucket_id: 20
created: 2018-12-01 01:12:04
updated: 2018-12-01 01:12:04
- id: 37
title: 'task #37'
done: false
created_by_id: -2
list_id: 2
index: 2
created: 2018-12-01 01:12:04
updated: 2018-12-01 01:12:04

View File

@ -19,6 +19,8 @@ package integrations
import (
"testing"
"code.vikunja.io/api/pkg/db"
"code.vikunja.io/api/pkg/models"
"code.vikunja.io/web/handler"
"github.com/labstack/echo/v4"
@ -33,6 +35,20 @@ func TestBucket(t *testing.T) {
},
t: t,
}
testHandlerLinkShareWrite := webHandlerTest{
linkShare: &models.LinkSharing{
ID: 2,
Hash: "test2",
ListID: 2,
Right: models.RightWrite,
SharingType: models.SharingTypeWithoutPassword,
SharedByID: 1,
},
strFunc: func() handler.CObject {
return &models.Bucket{}
},
t: t,
}
t.Run("ReadAll", func(t *testing.T) {
t.Run("Normal", func(t *testing.T) {
rec, err := testHandler.testReadAllWithUser(nil, map[string]string{"list": "1"})
@ -297,5 +313,15 @@ func TestBucket(t *testing.T) {
assert.Contains(t, rec.Body.String(), `"title":"Lorem Ipsum"`)
})
})
t.Run("Link Share", func(t *testing.T) {
rec, err := testHandlerLinkShareWrite.testCreateWithLinkShare(nil, map[string]string{"list": "2"}, `{"title":"Lorem Ipsum"}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"Lorem Ipsum"`)
db.AssertExists(t, "buckets", map[string]interface{}{
"list_id": 2,
"created_by_id": -2,
"title": "Lorem Ipsum",
}, false)
})
})
}

View File

@ -553,7 +553,7 @@ func TestLinkSharing(t *testing.T) {
rec, err := testHandlerTaskWriteCollection.testReadAllWithLinkShare(nil, nil)
assert.NoError(t, err)
assert.NotContains(t, rec.Body.String(), `task #2`)
assert.NotContains(t, rec.Body.String(), `task #3`)
assert.NotContains(t, rec.Body.String(), `task #3"`)
assert.NotContains(t, rec.Body.String(), `task #4`)
assert.NotContains(t, rec.Body.String(), `task #5`)
assert.NotContains(t, rec.Body.String(), `task #6`)

View File

@ -19,6 +19,8 @@ package integrations
import (
"testing"
"code.vikunja.io/api/pkg/db"
"code.vikunja.io/api/pkg/models"
"code.vikunja.io/web/handler"
"github.com/labstack/echo/v4"
@ -33,6 +35,20 @@ func TestTaskComments(t *testing.T) {
},
t: t,
}
testHandlerLinkShareWrite := webHandlerTest{
linkShare: &models.LinkSharing{
ID: 2,
Hash: "test2",
ListID: 2,
Right: models.RightWrite,
SharingType: models.SharingTypeWithoutPassword,
SharedByID: 1,
},
strFunc: func() handler.CObject {
return &models.TaskComment{}
},
t: t,
}
// Only run specific nested tests:
// ^TestTaskComments$/^Update$/^Update_task_items$/^Removing_Assignees_null$
t.Run("Update", func(t *testing.T) {
@ -281,5 +297,15 @@ func TestTaskComments(t *testing.T) {
assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`)
})
})
t.Run("Link Share", func(t *testing.T) {
rec, err := testHandlerLinkShareWrite.testCreateWithLinkShare(nil, map[string]string{"task": "13"}, `{"comment":"Lorem Ipsum"}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"comment":"Lorem Ipsum"`)
db.AssertExists(t, "task_comments", map[string]interface{}{
"task_id": 13,
"comment": "Lorem Ipsum",
"author_id": -2,
}, false)
})
})
}

View File

@ -19,6 +19,8 @@ package integrations
import (
"testing"
"code.vikunja.io/api/pkg/db"
"code.vikunja.io/api/pkg/models"
"code.vikunja.io/web/handler"
"github.com/labstack/echo/v4"
@ -33,6 +35,20 @@ func TestTask(t *testing.T) {
},
t: t,
}
testHandlerLinkShareWrite := webHandlerTest{
linkShare: &models.LinkSharing{
ID: 2,
Hash: "test2",
ListID: 2,
Right: models.RightWrite,
SharingType: models.SharingTypeWithoutPassword,
SharedByID: 1,
},
strFunc: func() handler.CObject {
return &models.Task{}
},
t: t,
}
// Only run specific nested tests:
// ^TestTask$/^Update$/^Update_task_items$/^Removing_Assignees_null$
t.Run("Update", func(t *testing.T) {
@ -489,5 +505,15 @@ func TestTask(t *testing.T) {
assertHandlerErrorCode(t, err, models.ErrCodeBucketDoesNotExist)
})
})
t.Run("Link Share", func(t *testing.T) {
rec, err := testHandlerLinkShareWrite.testCreateWithLinkShare(nil, map[string]string{"list": "2"}, `{"title":"Lorem Ipsum"}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"Lorem Ipsum"`)
db.AssertExists(t, "tasks", map[string]interface{}{
"list_id": 2,
"title": "Lorem Ipsum",
"created_by_id": -2,
}, false)
})
})
}

View File

@ -0,0 +1,43 @@
// Vikunja is a to-do list application to facilitate your life.
// Copyright 2018-2021 Vikunja and contributors. All rights reserved.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public Licensee as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public Licensee for more details.
//
// You should have received a copy of the GNU Affero General Public Licensee
// along with this program. If not, see <https://www.gnu.org/licenses/>.
package migration
import (
"src.techknowlogick.com/xormigrate"
"xorm.io/xorm"
)
type linkShares20210403220653 struct {
Name string `xorm:"text null" json:"name"`
}
func (linkShares20210403220653) TableName() string {
return "link_shares"
}
func init() {
migrations = append(migrations, &xormigrate.Migration{
ID: "20210403220653",
Description: "Add the name column to link shares",
Migrate: func(tx *xorm.Engine) error {
return tx.Sync2(linkShares20210403220653{})
},
Rollback: func(tx *xorm.Engine) error {
return nil
},
})
}

View File

@ -135,12 +135,9 @@ func (b *Bucket) ReadAll(s *xorm.Session, auth web.Auth, search string, page int
}
// Get all users
users := make(map[int64]*user.User)
if len(userIDs) > 0 {
err = s.In("id", userIDs).Find(&users)
if err != nil {
return
}
users, err := getUsersOrLinkSharesFromIDs(s, userIDs)
if err != nil {
return
}
for _, bb := range buckets {
@ -234,7 +231,11 @@ func (b *Bucket) ReadAll(s *xorm.Session, auth web.Auth, search string, page int
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{id}/buckets [put]
func (b *Bucket) Create(s *xorm.Session, a web.Auth) (err error) {
b.CreatedByID = a.GetID()
b.CreatedBy, err = getUserOrLinkShareUser(s, a)
if err != nil {
return
}
b.CreatedByID = b.CreatedBy.ID
_, err = s.Insert(b)
return

View File

@ -89,6 +89,20 @@ func TestBucket_ReadAll(t *testing.T) {
assert.Len(t, buckets, 3)
assert.Equal(t, int64(2), buckets[0].Tasks[0].ID)
})
t.Run("link share", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
testuser := &user.User{ID: 1}
b := &Bucket{ListID: 23}
result, _, _, err := b.ReadAll(s, testuser, "", 0, 0)
assert.NoError(t, err)
buckets, _ := result.([]*Bucket)
assert.Len(t, buckets, 1)
assert.NotNil(t, buckets[0].CreatedBy)
assert.Equal(t, int64(-2), buckets[0].CreatedByID)
})
}
func TestBucket_Delete(t *testing.T) {

View File

@ -42,6 +42,8 @@ type LinkSharing struct {
ID int64 `xorm:"bigint autoincr not null unique pk" json:"id" param:"share"`
// The public id to get this shared list
Hash string `xorm:"varchar(40) not null unique" json:"hash" param:"hash"`
// The name of this link share. All actions someone takes while being authenticated with that link will appear with that name.
Name string `xorm:"text null" json:"name"`
// The ID of the shared list
ListID int64 `xorm:"bigint not null" json:"-" param:"list"`
// The right this list is shared with. 0 = Read only, 1 = Read & Write, 2 = Admin. See the docs for more details.
@ -84,6 +86,25 @@ func GetLinkShareFromClaims(claims jwt.MapClaims) (share *LinkSharing, err error
return
}
func (share *LinkSharing) getUserID() int64 {
return share.ID * -1
}
func (share *LinkSharing) toUser() *user.User {
suffix := "Link Share"
if share.Name != "" {
suffix = " (" + suffix + ")"
}
return &user.User{
ID: share.getUserID(),
Name: share.Name + suffix,
Username: share.Name,
Created: share.Created,
Updated: share.Updated,
}
}
// Create creates a new link share for a given list
// @Summary Share a list via link
// @Description Share a list via link. The user needs to have write-access to the list to be able do this.
@ -246,3 +267,23 @@ func GetListByShareHash(s *xorm.Session, hash string) (list *List, err error) {
list, err = GetListSimpleByID(s, share.ListID)
return
}
// GetLinkShareByID returns a link share by its id.
func GetLinkShareByID(s *xorm.Session, id int64) (share *LinkSharing, err error) {
share = &LinkSharing{}
has, err := s.Where("id = ?", id).Get(share)
if err != nil {
return
}
if !has {
return share, ErrListShareDoesNotExist{ID: id}
}
return
}
// GetLinkSharesByIDs returns all link shares from a slice of ids
func GetLinkSharesByIDs(s *xorm.Session, ids []int64) (shares map[int64]*LinkSharing, err error) {
shares = make(map[int64]*LinkSharing)
err = s.In("id", ids).Find(&shares)
return
}

View File

@ -64,7 +64,17 @@ func (ta *TaskAttachment) NewAttachment(s *xorm.Session, f io.ReadCloser, realna
// Add an entry to the db
ta.FileID = file.ID
ta.CreatedByID = a.GetID()
ta.CreatedBy, err = getUserOrLinkShareUser(s, a)
if err != nil {
// remove the uploaded file if adding it to the db fails
if err2 := file.Delete(); err2 != nil {
return err2
}
return err
}
ta.CreatedByID = ta.CreatedBy.ID
_, err = s.Insert(ta)
if err != nil {
// remove the uploaded file if adding it to the db fails
@ -74,8 +84,6 @@ func (ta *TaskAttachment) NewAttachment(s *xorm.Session, f io.ReadCloser, realna
return err
}
ta.CreatedBy, _ = user.GetFromAuth(a) // Ignoring cases where the auth is not a user
return nil
}
@ -145,19 +153,19 @@ func (ta *TaskAttachment) ReadAll(s *xorm.Session, a web.Auth, search string, pa
return nil, 0, 0, err
}
us := make(map[int64]*user.User)
err = s.In("id", userIDs).Find(&us)
users, err := getUsersOrLinkSharesFromIDs(s, userIDs)
if err != nil {
return nil, 0, 0, err
}
for _, r := range attachments {
r.CreatedBy = users[r.CreatedByID]
// If the actual file does not exist, don't try to load it as that would fail with nil panic
if _, exists := fs[r.FileID]; !exists {
continue
}
r.File = fs[r.FileID]
r.CreatedBy = us[r.CreatedByID]
}
numberOfTotalItems, err = s.
@ -231,12 +239,9 @@ func getTaskAttachmentsByTaskIDs(s *xorm.Session, taskIDs []int64) (attachments
return
}
users := make(map[int64]*user.User)
if len(userIDs) > 0 {
err = s.In("id", userIDs).Find(&users)
if err != nil {
return
}
users, err := getUsersOrLinkSharesFromIDs(s, userIDs)
if err != nil {
return nil, err
}
// Obfuscate all user emails

View File

@ -146,8 +146,13 @@ func TestTaskAttachment_ReadAll(t *testing.T) {
as, _, _, err := ta.ReadAll(s, &user.User{ID: 1}, "", 0, 50)
attachments, _ := as.([]*TaskAttachment)
assert.NoError(t, err)
assert.Len(t, attachments, 2)
assert.Len(t, attachments, 3)
assert.Equal(t, "test", attachments[0].File.Name)
for _, a := range attachments {
assert.NotNil(t, a.CreatedBy)
}
assert.Equal(t, int64(-2), attachments[2].CreatedByID)
assert.Equal(t, int64(-2), attachments[2].CreatedBy.ID)
}
func TestTaskAttachment_Delete(t *testing.T) {

View File

@ -59,6 +59,12 @@ func TestTaskCollection_ReadAll(t *testing.T) {
Created: testCreatedTime,
Updated: testUpdatedTime,
}
linkShareUser2 := &user.User{
ID: -2,
Name: "Link Share",
Created: testCreatedTime,
Updated: testUpdatedTime,
}
loc := config.GetTimeZone()
@ -124,6 +130,21 @@ func TestTaskCollection_ReadAll(t *testing.T) {
CreatedBy: user1,
Created: testCreatedTime,
},
{
ID: 3,
TaskID: 1,
FileID: 1,
CreatedByID: -2,
CreatedBy: linkShareUser2,
Created: testCreatedTime,
File: &files.File{
ID: 1,
Name: "test",
Size: 100,
Created: time.Unix(1570998791, 0).In(loc),
CreatedByID: 1,
},
},
},
Created: time.Unix(1543626724, 0).In(loc),
Updated: time.Unix(1543626724, 0).In(loc),

View File

@ -67,24 +67,22 @@ func (tc *TaskComment) Create(s *xorm.Session, a web.Auth) (err error) {
return err
}
tc.AuthorID = a.GetID()
tc.Author, err = getUserOrLinkShareUser(s, a)
if err != nil {
return err
}
tc.AuthorID = tc.Author.ID
_, err = s.Insert(tc)
if err != nil {
return
}
doer, _ := user.GetFromAuth(a)
err = events.Dispatch(&TaskCommentCreatedEvent{
return events.Dispatch(&TaskCommentCreatedEvent{
Task: &task,
Comment: tc,
Doer: doer,
Doer: tc.Author,
})
if err != nil {
return err
}
tc.Author, err = user.GetUserByID(s, a.GetID())
return
}
// Delete removes a task comment
@ -215,14 +213,12 @@ func (tc *TaskComment) ReadAll(s *xorm.Session, auth web.Auth, search string, pa
return
}
// Get all authors
authors := make(map[int64]*user.User)
err = s.
Select("users.*").
Table("task_comments").
Where("task_id = ? AND comment like ?", tc.TaskID, "%"+search+"%").
Join("INNER", "users", "users.id = task_comments.author_id").
Find(&authors)
var authorIDs []int64
for _, comment := range comments {
authorIDs = append(authorIDs, comment.AuthorID)
}
authors, err := getUsersOrLinkSharesFromIDs(s, authorIDs)
if err != nil {
return
}

View File

@ -184,4 +184,18 @@ func TestTaskComment_ReadAll(t *testing.T) {
assert.Error(t, err)
assert.True(t, IsErrGenericForbidden(err))
})
t.Run("comment from link share", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
tc := &TaskComment{TaskID: 35}
u := &user.User{ID: 1}
result, _, _, err := tc.ReadAll(s, u, "", 0, -1)
comments := result.([]*TaskComment)
assert.NoError(t, err)
assert.Len(t, comments, 2)
assert.Equal(t, int64(-2), comments[1].AuthorID)
assert.NotNil(t, comments[1].Author)
})
}

View File

@ -144,13 +144,17 @@ func (rel *TaskRelation) Create(s *xorm.Session, a web.Auth) error {
}
}
rel.CreatedByID = a.GetID()
rel.CreatedBy, err = getUserOrLinkShareUser(s, a)
if err != nil {
return err
}
rel.CreatedByID = rel.CreatedBy.ID
// Build up the other relation (see the comment above for explanation)
otherRelation := &TaskRelation{
TaskID: rel.OtherTaskID,
OtherTaskID: rel.TaskID,
CreatedByID: a.GetID(),
CreatedByID: rel.CreatedByID,
}
switch rel.RelationKind {

View File

@ -670,7 +670,7 @@ func addMoreInfoToTasks(s *xorm.Session, taskMap map[int64]*Task) (err error) {
return
}
users, err := user.GetUsersByIDs(s, userIDs)
users, err := getUsersOrLinkSharesFromIDs(s, userIDs)
if err != nil {
return
}
@ -817,17 +817,11 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool) (err
return err
}
if _, is := a.(*LinkSharing); is {
// A negative user id indicates user share links
t.CreatedByID = a.GetID() * -1
} else {
u, err := user.GetUserByID(s, a.GetID())
if err != nil {
return err
}
t.CreatedByID = u.ID
t.CreatedBy = u
createdBy, err := getUserOrLinkShareUser(s, a)
if err != nil {
return err
}
t.CreatedByID = createdBy.ID
// Generate a uuid if we don't already have one
if t.UID == "" {
@ -856,6 +850,8 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool) (err
return err
}
t.CreatedBy = createdBy
// Update the assignees
if updateAssignees {
if err := t.updateTaskAssignees(s, t.Assignees, a); err != nil {
@ -870,10 +866,9 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool) (err
t.setIdentifier(l)
doer, _ := user.GetFromAuth(a)
err = events.Dispatch(&TaskCreatedEvent{
Task: t,
Doer: doer,
Doer: createdBy,
})
if err != nil {
return err

View File

@ -579,4 +579,17 @@ func TestTask_ReadOne(t *testing.T) {
assert.NoError(t, err)
assert.NotNil(t, task.Subscription)
})
t.Run("created by link share", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
task := &Task{ID: 37}
err := task.ReadOne(s, u)
assert.NoError(t, err)
assert.Equal(t, "task #37", task.Title)
assert.Equal(t, int64(-2), task.CreatedByID)
assert.NotNil(t, task.CreatedBy)
assert.Equal(t, int64(-2), task.CreatedBy.ID)
})
}

78
pkg/models/users.go Normal file
View File

@ -0,0 +1,78 @@
// Vikunja is a to-do list application to facilitate your life.
// Copyright 2018-2021 Vikunja and contributors. All rights reserved.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public Licensee as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public Licensee for more details.
//
// You should have received a copy of the GNU Affero General Public Licensee
// along with this program. If not, see <https://www.gnu.org/licenses/>.
package models
import (
"code.vikunja.io/api/pkg/user"
"code.vikunja.io/web"
"xorm.io/xorm"
)
// Returns either a user or a link share disguised as a user.
func getUserOrLinkShareUser(s *xorm.Session, a web.Auth) (uu *user.User, err error) {
if u, is := a.(*user.User); is {
uu, err = user.GetUserByID(s, u.ID)
return
}
if ls, is := a.(*LinkSharing); is {
l, err := GetLinkShareByID(s, ls.ID)
if err != nil {
return nil, err
}
return l.toUser(), nil
}
return
}
// Returns all users or pseudo link shares from a slice of ids. ids < 0 are considered to be a link share in that case.
func getUsersOrLinkSharesFromIDs(s *xorm.Session, ids []int64) (users map[int64]*user.User, err error) {
users = make(map[int64]*user.User)
var userIDs []int64
var linkShareIDs []int64
for _, id := range ids {
if id < 0 {
linkShareIDs = append(linkShareIDs, id*-1)
continue
}
userIDs = append(userIDs, id)
}
if len(userIDs) > 0 {
users, err = user.GetUsersByIDs(s, userIDs)
if err != nil {
return
}
}
if len(linkShareIDs) == 0 {
return
}
shares, err := GetLinkSharesByIDs(s, linkShareIDs)
if err != nil {
return nil, err
}
for _, share := range shares {
users[share.ID*-1] = share.toUser()
}
return
}

View File

@ -7489,6 +7489,10 @@ var doc = `{
"description": "The ID of the shared thing",
"type": "integer"
},
"name": {
"description": "The name of this link share. All actions someone takes while being authenticated with that link will appear with that name.",
"type": "string"
},
"right": {
"description": "The right this list is shared with. 0 = Read only, 1 = Read \u0026 Write, 2 = Admin. See the docs for more details.",
"type": "integer",

View File

@ -7472,6 +7472,10 @@
"description": "The ID of the shared thing",
"type": "integer"
},
"name": {
"description": "The name of this link share. All actions someone takes while being authenticated with that link will appear with that name.",
"type": "string"
},
"right": {
"description": "The right this list is shared with. 0 = Read only, 1 = Read \u0026 Write, 2 = Admin. See the docs for more details.",
"type": "integer",

View File

@ -310,6 +310,9 @@ definitions:
id:
description: The ID of the shared thing
type: integer
name:
description: The name of this link share. All actions someone takes while being authenticated with that link will appear with that name.
type: string
right:
default: 0
description: The right this list is shared with. 0 = Read only, 1 = Read & Write, 2 = Admin. See the docs for more details.