From c6e79926f00e36ea993bc7a8fa9317bb79159d79 Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 2 Apr 2026 19:02:15 +0200 Subject: [PATCH] fix: add position conflict resolution for batch-inserted positions Add resolvePositionConflictsAfterInsert() which checks newly inserted task positions for duplicate position values within the same view and resolves them using existing conflict resolution logic. --- pkg/models/task_position.go | 37 ++++++++++ pkg/models/task_position_test.go | 115 ++++++++++++++++++++++++++++++- 2 files changed, 151 insertions(+), 1 deletion(-) diff --git a/pkg/models/task_position.go b/pkg/models/task_position.go index 64678d1a4..86f53373f 100644 --- a/pkg/models/task_position.go +++ b/pkg/models/task_position.go @@ -596,3 +596,40 @@ func resolveTaskPositionConflicts(s *xorm.Session, projectViewID int64, conflict return nil } + +// resolvePositionConflictsAfterInsert checks a batch of newly inserted task positions +// for conflicts (duplicate position values within the same view) and resolves them. +// This is called after bulk-inserting positions during task creation. +func resolvePositionConflictsAfterInsert(s *xorm.Session, positions []*TaskPosition) error { + // Track which (viewID, position) pairs we've already checked to avoid + // resolving the same conflict group twice. + type viewPos struct { + viewID int64 + position float64 + } + checked := make(map[viewPos]bool) + + for _, pos := range positions { + key := viewPos{viewID: pos.ProjectViewID, position: pos.Position} + if checked[key] { + continue + } + checked[key] = true + + conflicts, err := findPositionConflicts(s, pos.ProjectViewID, pos.Position) + if err != nil { + return err + } + + if len(conflicts) <= 1 { + continue + } + + err = resolveTaskPositionConflicts(s, pos.ProjectViewID, conflicts) + if err != nil { + return err + } + } + + return nil +} diff --git a/pkg/models/task_position_test.go b/pkg/models/task_position_test.go index 67e2221e8..48911c117 100644 --- a/pkg/models/task_position_test.go +++ b/pkg/models/task_position_test.go @@ -374,7 +374,7 @@ func TestRepairTaskPositions(t *testing.T) { assert.NotEqual(t, pos1.Position, pos2.Position) }) - t.Run("dry run reports without changes", func(t *testing.T) { + t.Run("dry run reports without changes - view 92", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() defer s.Close() @@ -433,3 +433,116 @@ func TestRepairTaskPositions(t *testing.T) { assert.Empty(t, result.Errors) }) } + +func TestCreateTaskPositionConflictResolution(t *testing.T) { + t.Run("resolves conflicts after position insert", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Insert two positions with the same value for the same view + pos1 := &TaskPosition{TaskID: 600, ProjectViewID: 1, Position: 12345} + pos2 := &TaskPosition{TaskID: 601, ProjectViewID: 1, Position: 12345} + + _, err := s.Insert(pos1) + require.NoError(t, err) + _, err = s.Insert(pos2) + require.NoError(t, err) + + // Call the new function that should detect and resolve conflicts + err = resolvePositionConflictsAfterInsert(s, []*TaskPosition{pos1, pos2}) + require.NoError(t, err) + + // Verify positions are now unique + var updated1, updated2 TaskPosition + _, err = s.Where("task_id = ? AND project_view_id = ?", 600, 1).Get(&updated1) + require.NoError(t, err) + _, err = s.Where("task_id = ? AND project_view_id = ?", 601, 1).Get(&updated2) + require.NoError(t, err) + + assert.NotEqual(t, updated1.Position, updated2.Position) + }) + + t.Run("no-op when no conflicts", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + pos1 := &TaskPosition{TaskID: 700, ProjectViewID: 1, Position: 11111} + pos2 := &TaskPosition{TaskID: 701, ProjectViewID: 1, Position: 22222} + + _, err := s.Insert(pos1) + require.NoError(t, err) + _, err = s.Insert(pos2) + require.NoError(t, err) + + err = resolvePositionConflictsAfterInsert(s, []*TaskPosition{pos1, pos2}) + require.NoError(t, err) + + // Positions should remain unchanged + var updated1, updated2 TaskPosition + _, err = s.Where("task_id = ? AND project_view_id = ?", 700, 1).Get(&updated1) + require.NoError(t, err) + _, err = s.Where("task_id = ? AND project_view_id = ?", 701, 1).Get(&updated2) + require.NoError(t, err) + + assert.Equal(t, 11111.0, updated1.Position) + assert.Equal(t, 22222.0, updated2.Position) + }) +} + +func TestSetTaskInBucketInViewsResolvesConflicts(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Pre-insert a position that will conflict with the one calculateNewPositionForTask produces. + views, err := getViewsForProject(s, 1) + require.NoError(t, err) + require.NotEmpty(t, views) + + // Pick the first view + view := views[0] + + // Get the current lowest position to predict the new task's position + lowestPosition := &TaskPosition{} + exists, err := s.Where("project_view_id = ?", view.ID). + OrderBy("position asc"). + Get(lowestPosition) + require.NoError(t, err) + + if exists && lowestPosition.Position >= MinPositionSpacing { + predictedPosition := lowestPosition.Position / 2 + + // Insert a conflicting position at the predicted value + _, err = s.Insert(&TaskPosition{ + TaskID: 999, + ProjectViewID: view.ID, + Position: predictedPosition, + }) + require.NoError(t, err) + + // Now create positions as task creation would + newPos := &TaskPosition{ + TaskID: 998, + ProjectViewID: view.ID, + Position: predictedPosition, + } + _, err = s.Insert(newPos) + require.NoError(t, err) + + // Resolve conflicts + err = resolvePositionConflictsAfterInsert(s, []*TaskPosition{newPos}) + require.NoError(t, err) + + // Verify they have different positions + var p1, p2 TaskPosition + _, err = s.Where("task_id = ? AND project_view_id = ?", 999, view.ID).Get(&p1) + require.NoError(t, err) + _, err = s.Where("task_id = ? AND project_view_id = ?", 998, view.ID).Get(&p2) + require.NoError(t, err) + + assert.NotEqual(t, p1.Position, p2.Position, + "Positions should be unique after conflict resolution") + } +}