fix(kanban): don't error moving a task into the done bucket when it already sits in another view's done bucket
TaskBucket.upsert decided between UPDATE and INSERT based on the number of rows the UPDATE affected. On MySQL/MariaDB an UPDATE that sets a column to the value it already holds reports 0 affected rows, so upsert mistook an existing row for a missing one, ran an INSERT, and hit the unique(task_id, project_view_id) index with ErrTaskAlreadyExistsInBucket. This surfaced when marking a task done: the done state is synced into the done bucket of every kanban view in the project, and if the task already lived in one of those done buckets the no-op update tripped the bug. Decide based on row existence instead of the affected-row count, which also makes the earlier repeating-task workaround unnecessary. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JK7JwuvjhcsNf5d5fUopeY
This commit is contained in:
parent
5236e0c306
commit
ba566ee9bc
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue