Refactor checking if bucket belongs to the same list when moving a task
This commit is contained in:
parent
29a37c705a
commit
fc92fd3325
|
@ -707,33 +707,19 @@ func addMoreInfoToTasks(s *xorm.Session, taskMap map[int64]*Task) (err error) {
|
|||
return
|
||||
}
|
||||
|
||||
func checkBucketAndTaskBelongToSameList(s *xorm.Session, fullTask *Task, bucketID int64) (err error) {
|
||||
if bucketID != 0 {
|
||||
b, err := getBucketByID(s, bucketID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if fullTask.ListID != b.ListID {
|
||||
return ErrBucketDoesNotBelongToList{
|
||||
ListID: fullTask.ListID,
|
||||
BucketID: fullTask.BucketID,
|
||||
}
|
||||
func checkBucketAndTaskBelongToSameList(s *xorm.Session, fullTask *Task, bucket *Bucket) (err error) {
|
||||
if fullTask.ListID != bucket.ListID {
|
||||
return ErrBucketDoesNotBelongToList{
|
||||
ListID: fullTask.ListID,
|
||||
BucketID: fullTask.BucketID,
|
||||
}
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// Checks if adding a new task would exceed the bucket limit
|
||||
func checkBucketLimit(s *xorm.Session, t *Task, bucket *Bucket) (err error) {
|
||||
|
||||
// We need the bucket to check if it has more tasks than the limit allows
|
||||
if bucket == nil {
|
||||
bucket, err = getBucketByID(s, t.BucketID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
// Check the limit
|
||||
if bucket.Limit > 0 {
|
||||
taskCount, err := s.
|
||||
|
@ -799,12 +785,6 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool) (err
|
|||
t.UID = utils.MakeRandomString(40)
|
||||
}
|
||||
|
||||
// If there is a bucket set, make sure they belong to the same list as the task
|
||||
err = checkBucketAndTaskBelongToSameList(s, t, t.BucketID)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
// Get the default bucket and move the task there
|
||||
var bucket *Bucket
|
||||
if t.BucketID == 0 {
|
||||
|
@ -815,6 +795,19 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool) (err
|
|||
t.BucketID = bucket.ID
|
||||
}
|
||||
|
||||
if bucket == nil {
|
||||
bucket, err = getBucketByID(s, t.BucketID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
// If there is a bucket set, make sure they belong to the same list as the task
|
||||
err = checkBucketAndTaskBelongToSameList(s, t, bucket)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
// Bucket Limit
|
||||
if err := checkBucketLimit(s, t, bucket); err != nil {
|
||||
return err
|
||||
|
@ -886,6 +879,10 @@ func (t *Task) Update(s *xorm.Session, a web.Auth) (err error) {
|
|||
return
|
||||
}
|
||||
|
||||
if t.ListID == 0 {
|
||||
t.ListID = ot.ListID
|
||||
}
|
||||
|
||||
// Get the reminders
|
||||
reminders, err := getRemindersForTasks(s, []int64{t.ID})
|
||||
if err != nil {
|
||||
|
@ -910,12 +907,6 @@ func (t *Task) Update(s *xorm.Session, a web.Auth) (err error) {
|
|||
return err
|
||||
}
|
||||
|
||||
// If there is a bucket set, make sure they belong to the same list as the task
|
||||
err = checkBucketAndTaskBelongToSameList(s, &ot, t.BucketID)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
// All columns to update in a separate variable to be able to add to them
|
||||
colsToUpdate := []string{
|
||||
"title",
|
||||
|
@ -946,6 +937,19 @@ func (t *Task) Update(s *xorm.Session, a web.Auth) (err error) {
|
|||
t.BucketID = bucket.ID
|
||||
}
|
||||
|
||||
if bucket == nil {
|
||||
bucket, err = getBucketByID(s, t.BucketID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
// If there is a bucket set, make sure they belong to the same list as the task
|
||||
err = checkBucketAndTaskBelongToSameList(s, &ot, bucket)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
// If the task is being moved between lists, make sure to move the bucket + index as well
|
||||
if t.ListID != 0 && ot.ListID != t.ListID {
|
||||
latestTask := &Task{}
|
||||
|
|
|
@ -202,6 +202,22 @@ func TestTask_Update(t *testing.T) {
|
|||
err := task.Update(s, u)
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
t.Run("bucket on other list", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
task := &Task{
|
||||
ID: 1,
|
||||
Title: "test10000",
|
||||
Description: "Lorem Ipsum Dolor",
|
||||
ListID: 1,
|
||||
BucketID: 4, // Bucket 4 belongs to list 2
|
||||
}
|
||||
err := task.Update(s, u)
|
||||
assert.Error(t, err)
|
||||
assert.True(t, IsErrBucketDoesNotBelongToList(err))
|
||||
})
|
||||
}
|
||||
|
||||
func TestTask_Delete(t *testing.T) {
|
||||
|
|
Loading…
Reference in New Issue