diff --git a/pkg/models/kanban_task_bucket.go b/pkg/models/kanban_task_bucket.go index 80b23f1b9..bd89da1d9 100644 --- a/pkg/models/kanban_task_bucket.go +++ b/pkg/models/kanban_task_bucket.go @@ -149,23 +149,26 @@ func (b *TaskBucket) Update(s *xorm.Session, a web.Auth) (err error) { var updateBucket = true - // mark task done if moved into the done bucket + // mark task done if moved into or out of the done bucket + // Only change the done state if the task's done value actually changes var doneChanged bool - if view.DoneBucketID == b.BucketID { - doneChanged = true - task.Done = true - if task.isRepeating() { - oldTask := task - oldTask.Done = false - updateDone(oldTask, task) - updateBucket = false - b.BucketID = oldTaskBucket.BucketID + if view.DoneBucketID != 0 { + if view.DoneBucketID == b.BucketID && !task.Done { + doneChanged = true + task.Done = true + if task.isRepeating() { + oldTask := task + oldTask.Done = false + updateDone(oldTask, task) + updateBucket = false + b.BucketID = oldTaskBucket.BucketID + } } - } - if oldTaskBucket.BucketID == view.DoneBucketID { - doneChanged = true - task.Done = false + if oldTaskBucket.BucketID == view.DoneBucketID && task.Done && b.BucketID != view.DoneBucketID { + doneChanged = true + task.Done = false + } } if doneChanged { diff --git a/pkg/models/kanban_task_bucket_test.go b/pkg/models/kanban_task_bucket_test.go index c11cead57..3679b2726 100644 --- a/pkg/models/kanban_task_bucket_test.go +++ b/pkg/models/kanban_task_bucket_test.go @@ -18,6 +18,7 @@ package models import ( "testing" + "time" "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/user" @@ -156,4 +157,65 @@ func TestTaskBucket_Update(t *testing.T) { "bucket_id": 1, }, false) }) + + t.Run("keep done timestamp when moving task between projects", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + u := &user.User{ID: 1} + + doneAt := time.Now().Round(time.Second) + + // Set a done timestamp on the task + { + s := db.NewSession() + _, err := s.ID(2).Cols("done_at").Update(&Task{DoneAt: doneAt}) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + s.Close() + } + + // Move the task to another project without a done bucket using Task.Update + { + s := db.NewSession() + task := &Task{ID: 2, Done: true, ProjectID: 9} + err := task.Update(s, u) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + s.Close() + } + + // Verify the task still has the same done timestamp + { + s := db.NewSession() + var task Task + _, err := s.ID(2).Get(&task) + require.NoError(t, err) + assert.True(t, task.Done) + assert.WithinDuration(t, doneAt, task.DoneAt, time.Second) + s.Close() + } + + // Move the task back to the original project with a done bucket using Task.Update + { + s := db.NewSession() + task := &Task{ID: 2, Done: true, ProjectID: 1} + err := task.Update(s, u) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + s.Close() + } + + // Verify the done timestamp is still preserved + { + s := db.NewSession() + var task Task + _, err := s.ID(2).Get(&task) + require.NoError(t, err) + assert.True(t, task.Done) + assert.WithinDuration(t, doneAt, task.DoneAt, time.Second) + s.Close() + } + }) }