vikunja
/
vikunja
6
43
Fork 71
Code Issues 150 Pull Requests 16 Packages Releases 32 Activity

Add session handling to list teams and related entities

This commit is contained in:
kolaente 2020-12-22 18:25:36 +01:00
parent 5758506e7c
commit 14cb1123e7
Signed by: konrad
GPG Key ID: F40E70337AB24C9B
6 changed files with 92 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/models/list.go
View File

@ -571,7 +571,7 @@ func updateListByTaskID(s *xorm.Session, taskID int64) (err error) {
// @Failure 500 {object} models.Message "Internal error"
// @Router /namespaces/{namespaceID}/lists [put]
func (l *List) Create(s *xorm.Session, a web.Auth) (err error) {
err = l.CheckIsArchived()
err = l.CheckIsArchived(s)
if err != nil {
return err
}
@ -585,7 +585,7 @@ func (l *List) Create(s *xorm.Session, a web.Auth) (err error) {
l.Owner = doer
l.ID = 0 // Otherwise only the first time a new list would be created
err = CreateOrUpdateList(l)
err = CreateOrUpdateList(s, l)
if err != nil {
return
}

41
pkg/models/list_duplicate.go
View File

@ -21,6 +21,7 @@ import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/api/pkg/utils"
"code.vikunja.io/web"
"xorm.io/xorm"
)
// ListDuplicate holds everything needed to duplicate a list
@ -38,7 +39,7 @@ type ListDuplicate struct {
}
// CanCreate checks if a user has the right to duplicate a list
func (ld *ListDuplicate) CanCreate(a web.Auth) (canCreate bool, err error) {
func (ld *ListDuplicate) CanCreate(a web.Auth, s *xorm.Session) (canCreate bool, err error) {
// List Exists + user has read access to list
ld.List = &List{ID: ld.ListID}
canRead, _, err := ld.List.CanRead(s, a)
@ -66,7 +67,7 @@ func (ld *ListDuplicate) CanCreate(a web.Auth) (canCreate bool, err error) {
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{listID}/duplicate [put]
//nolint:gocyclo
func (ld *ListDuplicate) Create(a web.Auth) (err error) {
func (ld *ListDuplicate) Create(s *xorm.Session, a web.Auth) (err error) {
log.Debugf("Duplicating list %d", ld.ListID)
@ -74,7 +75,7 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
ld.List.Identifier = "" // Reset the identifier to trigger regenerating a new one
// Set the owner to the current user
ld.List.OwnerID = a.GetID()
if err := CreateOrUpdateList(ld.List); err != nil {
if err := CreateOrUpdateList(s, ld.List); err != nil {
// If there is no available unique list identifier, just reset it.
if IsErrListIdentifierIsNotUnique(err) {
ld.List.Identifier = ""
@ -90,7 +91,7 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
// Used to map the newly created tasks to their new buckets
bucketMap := make(map[int64]int64)
buckets := []*Bucket{}
err = x.Where("list_id = ?", ld.ListID).Find(&buckets)
err = s.Where("list_id = ?", ld.ListID).Find(&buckets)
if err != nil {
return
}
@ -123,10 +124,8 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
t.ListID = ld.List.ID
t.BucketID = bucketMap[t.BucketID]
t.UID = ""
s := x.NewSession()
err := createTask(s, t, a, false)
if err != nil {
_ = s.Rollback()
return err
}
taskMap[oldID] = t.ID
@ -180,7 +179,7 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
// Copy label tasks (not the labels)
labelTasks := []*LabelTask{}
err = x.In("task_id", oldTaskIDs).Find(&labelTasks)
err = s.In("task_id", oldTaskIDs).Find(&labelTasks)
if err != nil {
return
}
@ -188,7 +187,7 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
for _, lt := range labelTasks {
lt.ID = 0
lt.TaskID = taskMap[lt.TaskID]
if _, err := x.Insert(lt); err != nil {
if _, err := s.Insert(lt); err != nil {
return err
}
}
@ -198,7 +197,7 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
// Assignees
// Only copy those assignees who have access to the task
assignees := []*TaskAssginee{}
err = x.In("task_id", oldTaskIDs).Find(&assignees)
err = s.In("task_id", oldTaskIDs).Find(&assignees)
if err != nil {
return
}
@ -219,14 +218,14 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
// Comments
comments := []*TaskComment{}
err = x.In("task_id", oldTaskIDs).Find(&comments)
err = s.In("task_id", oldTaskIDs).Find(&comments)
if err != nil {
return
}
for _, c := range comments {
c.ID = 0
c.TaskID = taskMap[c.TaskID]
if _, err := x.Insert(c); err != nil {
if _, err := s.Insert(c); err != nil {
return err
}
}
@ -237,7 +236,7 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
// Low-Effort: Only copy those relations which are between tasks in the same list
// because we can do that without a lot of hassle
relations := []*TaskRelation{}
err = x.In("task_id", oldTaskIDs).Find(&relations)
err = s.In("task_id", oldTaskIDs).Find(&relations)
if err != nil {
return
}
@ -249,7 +248,7 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
r.ID = 0
r.OtherTaskID = otherTaskID
r.TaskID = taskMap[r.TaskID]
if _, err := x.Insert(r); err != nil {
if _, err := s.Insert(r); err != nil {
return err
}
}
@ -276,7 +275,7 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
}
// Get unsplash info if applicable
up, err := GetUnsplashPhotoByFileID(ld.List.BackgroundFileID)
up, err := GetUnsplashPhotoByFileID(s, ld.List.BackgroundFileID)
if err != nil && files.IsErrFileIsNotUnsplashFile(err) {
return err
}
@ -288,7 +287,7 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
}
}
if err := SetListBackground(ld.List.ID, file); err != nil {
if err := SetListBackground(s, ld.List.ID, file); err != nil {
return err
}
@ -298,14 +297,14 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
// Rights / Shares
// To keep it simple(r) we will only copy rights which are directly used with the list, no namespace changes.
users := []*ListUser{}
err = x.Where("list_id = ?", ld.ListID).Find(&users)
err = s.Where("list_id = ?", ld.ListID).Find(&users)
if err != nil {
return
}
for _, u := range users {
u.ID = 0
u.ListID = ld.List.ID
if _, err := x.Insert(u); err != nil {
if _, err := s.Insert(u); err != nil {
return err
}
}
@ -313,21 +312,21 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
log.Debugf("Duplicated user shares from list %d into %d", ld.ListID, ld.List.ID)
teams := []*TeamList{}
err = x.Where("list_id = ?", ld.ListID).Find(&teams)
err = s.Where("list_id = ?", ld.ListID).Find(&teams)
if err != nil {
return
}
for _, t := range teams {
t.ID = 0
t.ListID = ld.List.ID
if _, err := x.Insert(t); err != nil {
if _, err := s.Insert(t); err != nil {
return err
}
}
// Generate new link shares if any are available
linkShares := []*LinkSharing{}
err = x.Where("list_id = ?", ld.ListID).Find(&linkShares)
err = s.Where("list_id = ?", ld.ListID).Find(&linkShares)
if err != nil {
return
}
@ -335,7 +334,7 @@ func (ld *ListDuplicate) Create(a web.Auth) (err error) {
share.ID = 0
share.ListID = ld.List.ID
share.Hash = utils.MakeRandomString(40)
if _, err := x.Insert(share); err != nil {
if _, err := s.Insert(share); err != nil {
return err
}
}

6
pkg/models/list_duplicate_test.go
View File

@ -29,6 +29,7 @@ func TestListDuplicate(t *testing.T) {
db.LoadAndAssertFixtures(t)
files.InitTestFileFixtures(t)
s := x.NewSession()
u := &user.User{
ID: 1,
@ -38,11 +39,12 @@ func TestListDuplicate(t *testing.T) {
ListID: 1,
NamespaceID: 1,
}
can, err := l.CanCreate(u)
can, err := l.CanCreate(u, s)
assert.NoError(t, err)
assert.True(t, can)
err = l.Create(u)
err = l.Create(s, u)
assert.NoError(t, err)
// To make this test 100% useful, it would need to assert a lot more stuff, but it is good enough for now.
// Also, we're lacking utility functions to do all needed assertions.
_ = s.Close()
}

24
pkg/models/list_team.go
View File

@ -18,6 +18,7 @@ package models
import (
"time"
"xorm.io/xorm"
"code.vikunja.io/web"
)
@ -68,7 +69,7 @@ type TeamWithRight struct {
// @Failure 403 {object} web.HTTPError "The user does not have access to the list"
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{id}/teams [put]
func (tl *TeamList) Create(a web.Auth) (err error) {
func (tl *TeamList) Create(s *xorm.Session, a web.Auth) (err error) {
// Check if the rights are valid
if err = tl.Right.isValid(); err != nil {
@ -88,7 +89,7 @@ func (tl *TeamList) Create(a web.Auth) (err error) {
}
// Check if the team is already on the list
exists, err := x.Where("team_id = ?", tl.TeamID).
exists, err := s.Where("team_id = ?", tl.TeamID).
And("list_id = ?", tl.ListID).
Get(&TeamList{})
if err != nil {
@ -99,7 +100,7 @@ func (tl *TeamList) Create(a web.Auth) (err error) {
}
// Insert the new team
_, err = x.Insert(tl)
_, err = s.Insert(tl)
if err != nil {
return err
}
@ -121,7 +122,7 @@ func (tl *TeamList) Create(a web.Auth) (err error) {
// @Failure 404 {object} web.HTTPError "Team or list does not exist."
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{listID}/teams/{teamID} [delete]
func (tl *TeamList) Delete() (err error) {
func (tl *TeamList) Delete(s *xorm.Session) (err error) {
// Check if the team exists
_, err = GetTeamByID(tl.TeamID)
@ -130,7 +131,8 @@ func (tl *TeamList) Delete() (err error) {
}
// Check if the team has access to the list
has, err := x.Where("team_id = ? AND list_id = ?", tl.TeamID, tl.ListID).
has, err := s.
Where("team_id = ? AND list_id = ?", tl.TeamID, tl.ListID).
Get(&TeamList{})
if err != nil {
return
@ -140,7 +142,7 @@ func (tl *TeamList) Delete() (err error) {
}
// Delete the relation
_, err = x.Where("team_id = ?", tl.TeamID).
_, err = s.Where("team_id = ?", tl.TeamID).
And("list_id = ?", tl.ListID).
Delete(TeamList{})
if err != nil {
@ -166,7 +168,7 @@ func (tl *TeamList) Delete() (err error) {
// @Failure 403 {object} web.HTTPError "No right to see the list."
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{id}/teams [get]
func (tl *TeamList) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, totalItems int64, err error) {
func (tl *TeamList) ReadAll(s *xorm.Session, a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, totalItems int64, err error) {
// Check if the user can read the namespace
l := &List{ID: tl.ListID}
canRead, _, err := l.CanRead(s, a)
@ -181,7 +183,7 @@ func (tl *TeamList) ReadAll(a web.Auth, search string, page int, perPage int) (r
// Get the teams
all := []*TeamWithRight{}
query := x.
query := s.
Table("teams").
Join("INNER", "team_list", "team_id = teams.id").
Where("team_list.list_id = ?", tl.ListID).
@ -204,7 +206,7 @@ func (tl *TeamList) ReadAll(a web.Auth, search string, page int, perPage int) (r
return
}
totalItems, err = x.
totalItems, err = s.
Table("teams").
Join("INNER", "team_list", "team_id = teams.id").
Where("team_list.list_id = ?", tl.ListID).
@ -232,14 +234,14 @@ func (tl *TeamList) ReadAll(a web.Auth, search string, page int, perPage int) (r
// @Failure 404 {object} web.HTTPError "Team or list does not exist."
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{listID}/teams/{teamID} [post]
func (tl *TeamList) Update() (err error) {
func (tl *TeamList) Update(s *xorm.Session) (err error) {
// Check if the right is valid
if err := tl.Right.isValid(); err != nil {
return err
}
_, err = x.
_, err = s.
Where("list_id = ? AND team_id = ?", tl.ListID, tl.TeamID).
Cols("right").
Update(tl)

15
pkg/models/list_team_rights.go
View File

@ -18,24 +18,25 @@ package models
import (
"code.vikunja.io/web"
"xorm.io/xorm"
)
// CanCreate checks if the user can create a team <-> list relation
func (tl *TeamList) CanCreate(a web.Auth) (bool, error) {
return tl.canDoTeamList(a)
func (tl *TeamList) CanCreate(s *xorm.Session, a web.Auth) (bool, error) {
return tl.canDoTeamList(s, a)
}
// CanDelete checks if the user can delete a team <-> list relation
func (tl *TeamList) CanDelete(a web.Auth) (bool, error) {
return tl.canDoTeamList(a)
func (tl *TeamList) CanDelete(s *xorm.Session, a web.Auth) (bool, error) {
return tl.canDoTeamList(s, a)
}
// CanUpdate checks if the user can update a team <-> list relation
func (tl *TeamList) CanUpdate(a web.Auth) (bool, error) {
return tl.canDoTeamList(a)
func (tl *TeamList) CanUpdate(s *xorm.Session, a web.Auth) (bool, error) {
return tl.canDoTeamList(s, a)
}
func (tl *TeamList) canDoTeamList(a web.Auth) (bool, error) {
func (tl *TeamList) canDoTeamList(s *xorm.Session, a web.Auth) (bool, error) {
// Link shares aren't allowed to do anything
if _, is := a.(*LinkSharing); is {
return false, nil

61
pkg/models/list_team_test.go
View File

@ -37,20 +37,24 @@ func TestTeamList_ReadAll(t *testing.T) {
ListID: 3,
}
db.LoadAndAssertFixtures(t)
teams, _, _, err := tl.ReadAll(u, "", 1, 50)
s := x.NewSession()
teams, _, _, err := tl.ReadAll(s, u, "", 1, 50)
assert.NoError(t, err)
assert.Equal(t, reflect.TypeOf(teams).Kind(), reflect.Slice)
s := reflect.ValueOf(teams)
assert.Equal(t, s.Len(), 1)
ts := reflect.ValueOf(teams)
assert.Equal(t, ts.Len(), 1)
_ = s.Close()
})
t.Run("nonexistant list", func(t *testing.T) {
tl := TeamList{
ListID: 99999,
}
db.LoadAndAssertFixtures(t)
_, _, _, err := tl.ReadAll(u, "", 1, 50)
s := x.NewSession()
_, _, _, err := tl.ReadAll(s, u, "", 1, 50)
assert.Error(t, err)
assert.True(t, IsErrListDoesNotExist(err))
_ = s.Close()
})
t.Run("namespace owner", func(t *testing.T) {
tl := TeamList{
@ -59,8 +63,10 @@ func TestTeamList_ReadAll(t *testing.T) {
Right: RightAdmin,
}
db.LoadAndAssertFixtures(t)
_, _, _, err := tl.ReadAll(u, "", 1, 50)
s := x.NewSession()
_, _, _, err := tl.ReadAll(s, u, "", 1, 50)
assert.NoError(t, err)
_ = s.Close()
})
t.Run("no access", func(t *testing.T) {
tl := TeamList{
@ -69,9 +75,11 @@ func TestTeamList_ReadAll(t *testing.T) {
Right: RightAdmin,
}
db.LoadAndAssertFixtures(t)
_, _, _, err := tl.ReadAll(u, "", 1, 50)
s := x.NewSession()
_, _, _, err := tl.ReadAll(s, u, "", 1, 50)
assert.Error(t, err)
assert.True(t, IsErrNeedToHaveListReadAccess(err))
_ = s.Close()
})
}
@ -79,14 +87,17 @@ func TestTeamList_Create(t *testing.T) {
u := &user.User{ID: 1}
t.Run("normal", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := x.NewSession()
tl := TeamList{
TeamID: 1,
ListID: 1,
Right: RightAdmin,
}
allowed, _ := tl.CanCreate(u)
allowed, _ := tl.CanCreate(s, u)
assert.True(t, allowed)
err := tl.Create(u)
err := tl.Create(s, u)
assert.NoError(t, err)
err = s.Commit()
assert.NoError(t, err)
db.AssertExists(t, "team_list", map[string]interface{}{
"team_id": 1,
@ -96,56 +107,67 @@ func TestTeamList_Create(t *testing.T) {
})
t.Run("team already has access", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := x.NewSession()
tl := TeamList{
TeamID: 1,
ListID: 3,
Right: RightAdmin,
}
err := tl.Create(u)
err := tl.Create(s, u)
assert.Error(t, err)
assert.True(t, IsErrTeamAlreadyHasAccess(err))
_ = s.Close()
})
t.Run("wrong rights", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := x.NewSession()
tl := TeamList{
TeamID: 1,
ListID: 1,
Right: RightUnknown,
}
err := tl.Create(u)
err := tl.Create(s, u)
assert.Error(t, err)
assert.True(t, IsErrInvalidRight(err))
_ = s.Close()
})
t.Run("nonexistant team", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := x.NewSession()
tl := TeamList{
TeamID: 9999,
ListID: 1,
}
err := tl.Create(u)
err := tl.Create(s, u)
assert.Error(t, err)
assert.True(t, IsErrTeamDoesNotExist(err))
_ = s.Close()
})
t.Run("nonexistant list", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := x.NewSession()
tl := TeamList{
TeamID: 1,
ListID: 9999,
}
err := tl.Create(u)
err := tl.Create(s, u)
assert.Error(t, err)
assert.True(t, IsErrListDoesNotExist(err))
_ = s.Close()
})
}
func TestTeamList_Delete(t *testing.T) {
t.Run("normal", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := x.NewSession()
tl := TeamList{
TeamID: 1,
ListID: 3,
}
err := tl.Delete()
err := tl.Delete(s)
assert.NoError(t, err)
err = s.Commit()
assert.NoError(t, err)
db.AssertMissing(t, "team_list", map[string]interface{}{
"team_id": 1,
@ -154,23 +176,27 @@ func TestTeamList_Delete(t *testing.T) {
})
t.Run("nonexistant team", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := x.NewSession()
tl := TeamList{
TeamID: 9999,
ListID: 1,
}
err := tl.Delete()
err := tl.Delete(s)
assert.Error(t, err)
assert.True(t, IsErrTeamDoesNotExist(err))
_ = s.Close()
})
t.Run("nonexistant list", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := x.NewSession()
tl := TeamList{
TeamID: 1,
ListID: 9999,
}
err := tl.Delete()
err := tl.Delete(s)
assert.Error(t, err)
assert.True(t, IsErrTeamDoesNotHaveAccessToList(err))
_ = s.Close()
})
}
@ -229,6 +255,7 @@ func TestTeamList_Update(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := x.NewSession()
tl := &TeamList{
ID: tt.fields.ID,
@ -240,13 +267,15 @@ func TestTeamList_Update(t *testing.T) {
CRUDable: tt.fields.CRUDable,
Rights: tt.fields.Rights,
}
err := tl.Update()
err := tl.Update(s)
if (err != nil) != tt.wantErr {
t.Errorf("TeamList.Update() error = %v, wantErr %v", err, tt.wantErr)
}
if (err != nil) && tt.wantErr && !tt.errType(err) {
t.Errorf("TeamList.Update() Wrong error type! Error = %v, want = %v", err, runtime.FuncForPC(reflect.ValueOf(tt.errType).Pointer()).Name())
}
err = s.Commit()
assert.NoError(t, err)
if !tt.wantErr {
db.AssertExists(t, "team_list", map[string]interface{}{
"list_id": tt.fields.ListID,