fix(task): preserve done timestamp when moving tasks between projects (#1470)

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 {

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()
+		}
+	})
 }