diff --git a/pkg/models/kanban_task_bucket.go b/pkg/models/kanban_task_bucket.go index ae6757b30..7650efe64 100644 --- a/pkg/models/kanban_task_bucket.go +++ b/pkg/models/kanban_task_bucket.go @@ -58,25 +58,33 @@ func (b *TaskBucket) CanUpdate(s *xorm.Session, a web.Auth) (bool, error) { } func (b *TaskBucket) upsert(s *xorm.Session) (err error) { - count, err := s.Where("task_id = ? AND project_view_id = ?", b.TaskID, b.ProjectViewID). - Cols("bucket_id"). - Update(b) + // Decide update vs. insert based on whether the row exists, not on the + // number of rows the UPDATE affects: MySQL/MariaDB report 0 affected rows + // when an UPDATE sets a column to the value it already holds, which would + // make us fall through to an INSERT and hit the unique index. + exists, err := s.Where("task_id = ? AND project_view_id = ?", b.TaskID, b.ProjectViewID). + Exist(&TaskBucket{}) if err != nil { return } - if count == 0 { - _, err = s.Insert(b) - if err != nil { - // Check if this is a unique constraint violation for the task_buckets table - if db.IsUniqueConstraintError(err, "UQE_task_buckets_task_project_view") { - return ErrTaskAlreadyExistsInBucket{ - TaskID: b.TaskID, - ProjectViewID: b.ProjectViewID, - } + if exists { + _, err = s.Where("task_id = ? AND project_view_id = ?", b.TaskID, b.ProjectViewID). + Cols("bucket_id"). + Update(b) + return + } + + _, err = s.Insert(b) + if err != nil { + // Check if this is a unique constraint violation for the task_buckets table + if db.IsUniqueConstraintError(err, "UQE_task_buckets_task_project_view") { + return ErrTaskAlreadyExistsInBucket{ + TaskID: b.TaskID, + ProjectViewID: b.ProjectViewID, } - return } + return } return @@ -151,10 +159,8 @@ func updateTaskBucket(s *xorm.Session, a web.Auth, b *TaskBucket) (err error) { if err != nil { return err } - // If the task is already in the default bucket, skip the - // upsert — MySQL's UPDATE returns 0 affected rows when - // the value is unchanged, which would make upsert fall - // through to INSERT and hit the unique constraint. + // The task is already in the default bucket, so there is + // nothing to move and no count to bump. if b.BucketID == oldTaskBucket.BucketID { updateBucket = false } diff --git a/pkg/models/kanban_task_bucket_test.go b/pkg/models/kanban_task_bucket_test.go index 7bde2ade4..bf5c42ac3 100644 --- a/pkg/models/kanban_task_bucket_test.go +++ b/pkg/models/kanban_task_bucket_test.go @@ -226,6 +226,63 @@ func TestTaskBucket_Update(t *testing.T) { }) }) + t.Run("done task already in another view's done bucket", func(t *testing.T) { + // Regression test: marking a task done syncs it into the done bucket + // of every kanban view in the project. When the task already sits in + // such a view's done bucket the sync is a no-op update, but on + // MySQL/MariaDB an UPDATE that doesn't change the value reports 0 + // affected rows. The upsert then mistook that for "row missing" and + // inserted, hitting the unique index with ErrTaskAlreadyExistsInBucket. + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // A second manual kanban view on project 1. Creating it auto-generates + // the To-Do/Doing/Done buckets and sets its done bucket. + secondView := &ProjectView{ + Title: "Second Kanban", + ProjectID: 1, + ViewKind: ProjectViewKindKanban, + BucketConfigurationMode: BucketConfigurationModeManual, + } + err := secondView.Create(s, u) + require.NoError(t, err) + require.NotZero(t, secondView.DoneBucketID) + + // Pre-place task 1 in the second view's done bucket without going + // through the done-sync, so the task itself is still open and view 4 + // still has it in its default bucket. + _, err = s.Where("task_id = ? AND project_view_id = ?", 1, secondView.ID). + Cols("bucket_id"). + Update(&TaskBucket{BucketID: secondView.DoneBucketID}) + require.NoError(t, err) + + // Moving task 1 into view 4's done bucket marks it done and triggers + // the cross-view sync into the second view's done bucket, where it + // already lives. This must succeed rather than error. + tb := &TaskBucket{ + TaskID: 1, + BucketID: 3, // done bucket on view 4 + ProjectViewID: 4, + ProjectID: 1, + } + err = tb.Update(s, u) + require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + assert.True(t, tb.Task.Done) + + db.AssertExists(t, "task_buckets", map[string]interface{}{ + "task_id": 1, + "bucket_id": 3, + }, false) + db.AssertExists(t, "task_buckets", map[string]interface{}{ + "task_id": 1, + "project_view_id": secondView.ID, + "bucket_id": secondView.DoneBucketID, + }, false) + }) + t.Run("saved filter: first task into empty limited bucket is allowed", func(t *testing.T) { // Regression test for #2672: on a saved-filter kanban view the bucket // limit was checked against the total number of tasks matching the