From 20143435579c4b3c3a1cf18337f2227848db963d Mon Sep 17 00:00:00 2001 From: kolaente Date: Wed, 8 Apr 2026 10:28:25 +0200 Subject: [PATCH] fix: catch ErrNeedsFullRecalculation in task creation position conflict resolution resolvePositionConflictsAfterInsert now falls back to a full position recalculation when resolveTaskPositionConflicts returns ErrNeedsFullRecalculation, instead of bubbling the error up as HTTP 500. This mirrors the existing fallback logic in the CLI repair command. Ref: #2550 --- pkg/models/task_position.go | 17 ++++++++++++ pkg/models/task_position_test.go | 46 ++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/pkg/models/task_position.go b/pkg/models/task_position.go index 42fa02be5..325033207 100644 --- a/pkg/models/task_position.go +++ b/pkg/models/task_position.go @@ -600,12 +600,20 @@ func resolveTaskPositionConflicts(s *xorm.Session, projectViewID int64, conflict // 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. +// If resolveTaskPositionConflicts returns ErrNeedsFullRecalculation for a view, +// it falls back to a full recalculation of all positions in that view. 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. checked := make(map[int64]map[float64]bool) + // Track views that have already been fully recalculated so we skip + // further conflict checks for them. + recalculated := make(map[int64]bool) for _, pos := range positions { + if recalculated[pos.ProjectViewID] { + continue + } if checked[pos.ProjectViewID] != nil && checked[pos.ProjectViewID][pos.Position] { continue } @@ -624,6 +632,15 @@ func resolvePositionConflictsAfterInsert(s *xorm.Session, positions []*TaskPosit } err = resolveTaskPositionConflicts(s, pos.ProjectViewID, conflicts) + if IsErrNeedsFullRecalculation(err) { + view := &ProjectView{ID: pos.ProjectViewID} + err = recalculateTaskPositionsForRepair(s, view) + if err != nil { + return err + } + recalculated[pos.ProjectViewID] = true + continue + } if err != nil { return err } diff --git a/pkg/models/task_position_test.go b/pkg/models/task_position_test.go index 84967dcfc..cc2fed01a 100644 --- a/pkg/models/task_position_test.go +++ b/pkg/models/task_position_test.go @@ -547,3 +547,49 @@ func TestSetTaskInBucketInViewsResolvesConflicts(t *testing.T) { assert.NotEqual(t, p1.Position, p2.Position, "Positions should be unique after conflict resolution") } + +func TestResolvePositionConflictsAfterInsertFallsBackToRecalculation(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + viewID := int64(1) + + // Clear existing positions for this view + _, err := s.Where("project_view_id = ?", viewID).Delete(&TaskPosition{}) + require.NoError(t, err) + + // Set up extremely tight spacing that forces ErrNeedsFullRecalculation: + // Two existing positions with a gap smaller than MinPositionSpacing * (conflicts+1) + basePos := 100.0 + tinyGap := MinPositionSpacing * 0.1 // Much smaller than needed + _, err = s.Insert(&TaskPosition{TaskID: 800, ProjectViewID: viewID, Position: basePos}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 801, ProjectViewID: viewID, Position: basePos + tinyGap}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 802, ProjectViewID: viewID, Position: basePos + tinyGap}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 803, ProjectViewID: viewID, Position: basePos + 2*tinyGap}) + require.NoError(t, err) + + // The conflicting positions that would trigger ErrNeedsFullRecalculation + conflictPositions := []*TaskPosition{ + {TaskID: 801, ProjectViewID: viewID, Position: basePos + tinyGap}, + {TaskID: 802, ProjectViewID: viewID, Position: basePos + tinyGap}, + } + + // This should NOT return an error -- it should fall back to full recalculation + err = resolvePositionConflictsAfterInsert(s, conflictPositions) + require.NoError(t, err) + + // Verify all positions are now unique + var positions []*TaskPosition + err = s.Where("project_view_id = ?", viewID).OrderBy("position ASC").Find(&positions) + require.NoError(t, err) + + seen := make(map[float64]bool) + for _, p := range positions { + assert.False(t, seen[p.Position], "duplicate position found: %f for task %d", p.Position, p.TaskID) + seen[p.Position] = true + } +}