diff --git a/frontend/src/helpers/calculateItemPosition.ts b/frontend/src/helpers/calculateItemPosition.ts index de0309c0d..d4ea2f76d 100644 --- a/frontend/src/helpers/calculateItemPosition.ts +++ b/frontend/src/helpers/calculateItemPosition.ts @@ -1,21 +1,31 @@ +// Minimum spacing between positions. Must survive JSON round-trip. +// Matches backend MinPositionSpacing constant. +const MIN_POSITION_SPACING = 0.01 + export const calculateItemPosition = ( positionBefore: number | null = null, positionAfter: number | null = null, ): number => { + // Both neighbors have the same position (conflict) + if (positionBefore !== null && positionAfter !== null && positionBefore === positionAfter) { + // Nudge slightly above to maintain ordering intent + return positionAfter + MIN_POSITION_SPACING + } + if (positionBefore === null) { if (positionAfter === null) { return 0 } - - // If there is no task after it, we just add 2^16 to the last position to have enough room in the future + + // If there is no task before it, place it at half the position of the task after return positionAfter / 2 } - + // If there is no task after it, we just add 2^16 to the last position to have enough room in the future if (positionAfter === null) { return positionBefore + Math.pow(2, 16) } - + // If we have both a task before and after it, we actually calculate the position return positionBefore + (positionAfter - positionBefore) / 2 } diff --git a/frontend/src/helpers/calculateTaskPosition.test.ts b/frontend/src/helpers/calculateTaskPosition.test.ts index ef6bab437..b494fa0bd 100644 --- a/frontend/src/helpers/calculateTaskPosition.test.ts +++ b/frontend/src/helpers/calculateTaskPosition.test.ts @@ -1,20 +1,44 @@ -import {it, expect} from 'vitest' +import {describe, it, expect} from 'vitest' import {calculateItemPosition} from './calculateItemPosition' -it('should calculate the task position', () => { - const result = calculateItemPosition(10, 100) - expect(result).toBe(55) -}) -it('should return 0 if no position was provided', () => { - const result = calculateItemPosition(null, null) - expect(result).toBe(0) -}) -it('should calculate the task position for the first task', () => { - const result = calculateItemPosition(null, 100) - expect(result).toBe(50) -}) -it('should calculate the task position for the last task', () => { - const result = calculateItemPosition(10, null) - expect(result).toBe(65546) +describe('calculateItemPosition', () => { + it('should calculate the task position', () => { + const result = calculateItemPosition(10, 100) + expect(result).toBe(55) + }) + + it('should return 0 if no position was provided', () => { + const result = calculateItemPosition(null, null) + expect(result).toBe(0) + }) + + it('should calculate the task position for the first task', () => { + const result = calculateItemPosition(null, 100) + expect(result).toBe(50) + }) + + it('should calculate the task position for the last task', () => { + const result = calculateItemPosition(10, null) + expect(result).toBe(65546) + }) + + it('should handle equal positions (conflict) by nudging above', () => { + const result = calculateItemPosition(100, 100) + expect(result).toBeGreaterThan(100) + expect(result).toBeLessThan(101) + }) + + it('should handle equal positions at zero', () => { + const result = calculateItemPosition(0, 0) + expect(result).toBeGreaterThan(0) + }) + + it('should preserve precision after JSON round-trip', () => { + const position = calculateItemPosition(100, 100) + const serialized = JSON.stringify(position) + const deserialized = JSON.parse(serialized) + expect(deserialized).toBe(position) + expect(deserialized).toBeGreaterThan(100) + }) }) diff --git a/pkg/cmd/repair_task_positions.go b/pkg/cmd/repair_task_positions.go new file mode 100644 index 000000000..456f780e8 --- /dev/null +++ b/pkg/cmd/repair_task_positions.go @@ -0,0 +1,102 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/initialize" + "code.vikunja.io/api/pkg/log" + "code.vikunja.io/api/pkg/models" + + "github.com/spf13/cobra" +) + +func init() { + repairTaskPositionsCmd.Flags().Bool("dry-run", false, "Preview repairs without making changes") + rootCmd.AddCommand(repairTaskPositionsCmd) +} + +var repairTaskPositionsCmd = &cobra.Command{ + Use: "repair-task-positions", + Short: "Detect and repair duplicate task positions across all views", + Long: `Scans all project views for tasks with duplicate position values and repairs them. + +Duplicate positions can occur due to race conditions or historical bugs, causing +tasks to appear in the wrong order or jump around when the page is refreshed. + +This command will: +1. Scan all project views for duplicate positions +2. Attempt localized repair by redistributing conflicting tasks +3. Fall back to full view recalculation if localized repair fails + +Use --dry-run to preview what would be fixed without making changes.`, + PreRun: func(_ *cobra.Command, _ []string) { + initialize.FullInitWithoutAsync() + }, + Run: func(cmd *cobra.Command, _ []string) { + dryRun, _ := cmd.Flags().GetBool("dry-run") + + s := db.NewSession() + defer s.Close() + + if dryRun { + log.Infof("Running in dry-run mode - no changes will be made") + } else { + if err := s.Begin(); err != nil { + log.Errorf("Failed to start transaction: %s", err) + return + } + defer func() { + // Rollback is a no-op if commit already succeeded + _ = s.Rollback() + }() + } + + result, err := models.RepairTaskPositions(s, dryRun) + if err != nil { + log.Errorf("Failed to repair task positions: %s", err) + return + } + + if !dryRun { + if err := s.Commit(); err != nil { + log.Errorf("Failed to commit changes: %s", err) + return + } + } + + // Print summary + log.Infof("Repair complete:") + log.Infof(" Views scanned: %d", result.ViewsScanned) + log.Infof(" Views repaired: %d", result.ViewsRepaired) + log.Infof(" Tasks affected: %d", result.TasksAffected) + if result.FullRecalcViews > 0 { + log.Infof(" Views requiring full recalculation: %d", result.FullRecalcViews) + } + + if len(result.Errors) > 0 { + log.Errorf("Errors encountered (%d):", len(result.Errors)) + for _, e := range result.Errors { + log.Errorf(" - %s", e) + } + } + + if result.ViewsRepaired == 0 && len(result.Errors) == 0 { + log.Infof("No position conflicts found - all views are healthy!") + } + }, +} diff --git a/pkg/models/error.go b/pkg/models/error.go index b7353c473..5ab79e85c 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -1192,6 +1192,34 @@ func (err ErrInvalidTaskColumn) HTTPError() web.HTTPError { } } +// ErrNeedsFullRecalculation represents an error where localized position repair cannot proceed +// and the entire view must be recalculated. +type ErrNeedsFullRecalculation struct { + ProjectViewID int64 +} + +// IsErrNeedsFullRecalculation checks if an error is ErrNeedsFullRecalculation. +func IsErrNeedsFullRecalculation(err error) bool { + _, ok := err.(*ErrNeedsFullRecalculation) + return ok +} + +func (err *ErrNeedsFullRecalculation) Error() string { + return fmt.Sprintf("Insufficient spacing for localized repair [ProjectViewID: %d]", err.ProjectViewID) +} + +// ErrCodeNeedsFullRecalculation holds the unique world-error code of this error +const ErrCodeNeedsFullRecalculation = 4028 + +// HTTPError holds the http error description +func (err *ErrNeedsFullRecalculation) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusInternalServerError, + Code: ErrCodeNeedsFullRecalculation, + Message: "Position repair requires full view recalculation.", + } +} + // ============ // Team errors // ============ diff --git a/pkg/models/task_position.go b/pkg/models/task_position.go index c6e291eb3..7881afe03 100644 --- a/pkg/models/task_position.go +++ b/pkg/models/task_position.go @@ -17,7 +17,9 @@ package models import ( + "fmt" "math" + "sort" "xorm.io/xorm" @@ -26,6 +28,9 @@ import ( "code.vikunja.io/api/pkg/web" ) +// MinPositionSpacing is the smallest gap we allow between positions. +const MinPositionSpacing = 0.01 + type TaskPosition struct { // The ID of the task this position is for TaskID int64 `xorm:"bigint not null index" json:"task_id" param:"task"` @@ -54,20 +59,20 @@ func (tp *TaskPosition) CanUpdate(s *xorm.Session, a web.Auth) (bool, error) { return t.CanUpdate(s, a) } +func (tp *TaskPosition) refresh(s *xorm.Session) (err error) { + updatedPosition := &TaskPosition{} + _, err = s.Where("task_id = ? AND project_view_id = ?", tp.TaskID, tp.ProjectViewID).Get(updatedPosition) + if err != nil { + return err + } + + tp.Position = updatedPosition.Position + return nil +} + // updateTaskPosition is the internal function that performs the task position update logic // without dispatching events. This is used by moveTaskToDoneBuckets to avoid duplicate events. func updateTaskPosition(s *xorm.Session, a web.Auth, tp *TaskPosition) (err error) { - // Update all positions if the newly saved position is < 0.1 - var shouldRecalculate bool - var view *ProjectView - if tp.Position < 0.1 { - shouldRecalculate = true - view, err = GetProjectViewByID(s, tp.ProjectViewID) - if err != nil { - return err - } - } - exists, err := s. Where("task_id = ? AND project_view_id = ?", tp.TaskID, tp.ProjectViewID). Exist(&TaskPosition{}) @@ -80,22 +85,42 @@ func updateTaskPosition(s *xorm.Session, a web.Auth, tp *TaskPosition) (err erro if err != nil { return } - if shouldRecalculate { - return RecalculateTaskPositions(s, view, a) + } else { + _, err = s. + Where("task_id = ? AND project_view_id = ?", tp.TaskID, tp.ProjectViewID). + Cols("project_view_id", "position"). + Update(tp) + if err != nil { + return } - return nil } - _, err = s. - Where("task_id = ? AND project_view_id = ?", tp.TaskID, tp.ProjectViewID). - Cols("project_view_id", "position"). - Update(tp) + if tp.Position < MinPositionSpacing { + view, err := GetProjectViewByID(s, tp.ProjectViewID) + if err != nil { + return err + } + err = RecalculateTaskPositions(s, view, a) + if err != nil { + return err + } + + return tp.refresh(s) + } + + // Check for and resolve position conflicts + conflicts, err := findPositionConflicts(s, tp.ProjectViewID, tp.Position) if err != nil { - return + return err } - if shouldRecalculate { - return RecalculateTaskPositions(s, view, a) + if len(conflicts) > 1 { + err = resolveTaskPositionConflicts(s, tp.ProjectViewID, conflicts) + if err != nil { + return err + } + + return tp.refresh(s) } return nil @@ -231,6 +256,57 @@ func getPositionsForView(s *xorm.Session, view *ProjectView) (positions []*TaskP return } +// recalculateTaskPositionsForRepair recalculates positions for all tasks in a view +// without requiring auth. Used by CLI repair when localized repair fails. +// Unlike RecalculateTaskPositions, this only operates on tasks that already have +// positions in the view (doesn't discover new tasks). +func recalculateTaskPositionsForRepair(s *xorm.Session, view *ProjectView) error { + log.Debugf("Recalculating task positions for view %d (repair mode)", view.ID) + + // Get all existing positions for this view, ordered by current position then task ID + var existingPositions []*TaskPosition + err := s.Where("project_view_id = ?", view.ID). + OrderBy("position ASC, task_id ASC"). + Find(&existingPositions) + if err != nil { + return err + } + + if len(existingPositions) == 0 { + return nil + } + + // Delete all existing positions + _, err = s.Where("project_view_id = ?", view.ID).Delete(&TaskPosition{}) + if err != nil { + return err + } + + // Reassign evenly spaced positions + maxPosition := math.Pow(2, 32) + newPositions := make([]*TaskPosition, 0, len(existingPositions)) + + for i, pos := range existingPositions { + currentPosition := maxPosition / float64(len(existingPositions)) * float64(i+1) + newPositions = append(newPositions, &TaskPosition{ + TaskID: pos.TaskID, + ProjectViewID: view.ID, + Position: currentPosition, + }) + } + + count, err := s.Insert(newPositions) + if err != nil { + return err + } + + log.Debugf("Repair: inserted %d new positions for view %d", count, view.ID) + + return events.Dispatch(&TaskPositionsRecalculatedEvent{ + NewTaskPositions: newPositions, + }) +} + func calculateNewPositionForTask(s *xorm.Session, a web.Auth, t *Task, view *ProjectView) (*TaskPosition, error) { if t.Position == 0 { lowestPosition := &TaskPosition{} @@ -291,8 +367,7 @@ func createPositionsForTasksInView(s *xorm.Session, tasks []*Task, view *Project } var basePosition float64 - if !has || lowestPosition.Position == 0 { - // No existing positions or all are zero - trigger full recalculation + if !has || lowestPosition.Position < MinPositionSpacing { return RecalculateTaskPositions(s, view, a) } @@ -312,3 +387,207 @@ func createPositionsForTasksInView(s *xorm.Session, tasks []*Task, view *Project _, err = s.Insert(&newPositions) return err } + +// findPositionConflicts returns all task positions that share the same position value +// within a given project view. Returns an empty slice if no conflicts exist. +func findPositionConflicts(s *xorm.Session, projectViewID int64, position float64) (conflicts []*TaskPosition, err error) { + conflicts = []*TaskPosition{} + err = s. + Where("project_view_id = ? AND position = ?", projectViewID, position). + Find(&conflicts) + if err != nil { + return nil, err + } + return conflicts, nil +} + +// RepairResult contains the summary of a repair operation. +type RepairResult struct { + ViewsScanned int + ViewsRepaired int + TasksAffected int + FullRecalcViews int // Views that needed full recalculation + Errors []string // Views that couldn't be repaired +} + +// RepairTaskPositions scans all project views for duplicate task positions +// and repairs them using localized conflict resolution or full recalculation. +// If dryRun is true, it reports what would be fixed without making changes. +func RepairTaskPositions(s *xorm.Session, dryRun bool) (*RepairResult, error) { + result := &RepairResult{} + + // Get all task positions in a single query + var allPositions []*TaskPosition + err := s.OrderBy("project_view_id ASC, position ASC").Find(&allPositions) + if err != nil { + return nil, err + } + + if len(allPositions) == 0 { + return result, nil + } + + // Group positions by view ID + positionsByView := make(map[int64][]*TaskPosition) + for _, pos := range allPositions { + positionsByView[pos.ProjectViewID] = append(positionsByView[pos.ProjectViewID], pos) + } + + viewIDs := []int64{} + for viewID := range positionsByView { + viewIDs = append(viewIDs, viewID) + } + + viewsByID := make(map[int64]*ProjectView) + err = s.In("id", viewIDs).Find(&viewsByID) + if err != nil { + return nil, err + } + + // Process each view + for viewID, positions := range positionsByView { + result.ViewsScanned++ + + // Find duplicate positions within this view's positions + duplicates := findDuplicatesInPositions(positions) + if len(duplicates) == 0 { + continue + } + + if dryRun { + // Count affected tasks without making changes + for _, dup := range duplicates { + result.TasksAffected += len(dup) + } + result.ViewsRepaired++ + log.Infof("[dry-run] Would repair %d position conflicts in view %d", len(duplicates), viewID) + continue + } + + view, has := viewsByID[viewID] + if !has { + continue + } + + viewRepaired := false + for _, conflicts := range duplicates { + result.TasksAffected += len(conflicts) + + err = resolveTaskPositionConflicts(s, viewID, conflicts) + if IsErrNeedsFullRecalculation(err) { + // Fall back to full recalculation for this view + err = recalculateTaskPositionsForRepair(s, view) + if err != nil { + result.Errors = append(result.Errors, fmt.Sprintf("view %d: recalculation failed: %v", viewID, err)) + continue + } + result.FullRecalcViews++ + viewRepaired = true + // After full recalculation, no need to process more duplicates in this view + break + } else if err != nil { + result.Errors = append(result.Errors, fmt.Sprintf("view %d: %v", viewID, err)) + continue + } + viewRepaired = true + } + + if viewRepaired { + result.ViewsRepaired++ + } + } + + return result, nil +} + +// findDuplicatesInPositions finds groups of positions that share the same position value. +func findDuplicatesInPositions(positions []*TaskPosition) [][]*TaskPosition { + // Group by position + positionGroups := make(map[float64][]*TaskPosition) + for _, tp := range positions { + positionGroups[tp.Position] = append(positionGroups[tp.Position], tp) + } + + // Filter to only duplicates (groups with more than 1 task) + var duplicates [][]*TaskPosition + for _, group := range positionGroups { + if len(group) > 1 { + duplicates = append(duplicates, group) + } + } + + return duplicates +} + +// resolveTaskPositionConflicts redistributes conflicting task positions within the +// available gap between their neighbors. Returns ErrNeedsFullRecalculation if there +// is insufficient spacing to assign unique positions. +func resolveTaskPositionConflicts(s *xorm.Session, projectViewID int64, conflicts []*TaskPosition) error { + if len(conflicts) <= 1 { + return nil // No conflict to resolve + } + + conflictPosition := conflicts[0].Position + + // Find the nearest distinct neighbor positions + var leftNeighbor, rightNeighbor *TaskPosition + var lowerBound, upperBound float64 + + // Get the position immediately before the conflict position + leftNeighbor = &TaskPosition{} + hasLeft, err := s. + Where("project_view_id = ? AND position < ?", projectViewID, conflictPosition). + OrderBy("position DESC"). + Get(leftNeighbor) + if err != nil { + return err + } + if hasLeft { + lowerBound = leftNeighbor.Position + } + + // Get the position immediately after the conflict position + rightNeighbor = &TaskPosition{} + hasRight, err := s. + Where("project_view_id = ? AND position > ?", projectViewID, conflictPosition). + OrderBy("position ASC"). + Get(rightNeighbor) + if err != nil { + return err + } + if hasRight { + upperBound = rightNeighbor.Position + } else { + upperBound = math.Pow(2, 32) + } + + // Calculate spacing needed + availableGap := upperBound - lowerBound + spacing := availableGap / float64(len(conflicts)+1) + + // Check if we have enough spacing + if spacing < MinPositionSpacing { + return &ErrNeedsFullRecalculation{ProjectViewID: projectViewID} + } + + // Sort conflicts by task ID for deterministic ordering + sort.Slice(conflicts, func(i, j int) bool { + return conflicts[i].TaskID < conflicts[j].TaskID + }) + + // Assign new positions + for i, tp := range conflicts { + newPosition := lowerBound + spacing*float64(i+1) + _, err = s. + Where("task_id = ? AND project_view_id = ?", tp.TaskID, projectViewID). + Cols("position"). + Update(&TaskPosition{Position: newPosition}) + if err != nil { + return err + } + } + + log.Debugf("Repaired position conflict in view %d: %d tasks respaced from position %.6f", projectViewID, len(conflicts), conflictPosition) + + return nil +} diff --git a/pkg/models/task_position_test.go b/pkg/models/task_position_test.go new file mode 100644 index 000000000..67e2221e8 --- /dev/null +++ b/pkg/models/task_position_test.go @@ -0,0 +1,435 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package models + +import ( + "testing" + + "code.vikunja.io/api/pkg/db" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFindPositionConflicts(t *testing.T) { + t.Run("no conflicts", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Project view 1 has tasks at positions 2 and 4 - no conflicts + conflicts, err := findPositionConflicts(s, 1, 2) + require.NoError(t, err) + assert.Len(t, conflicts, 1) // Only one task at position 2 + }) + + t.Run("finds conflicts", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Insert two tasks with the same position + _, err := s.Insert(&TaskPosition{TaskID: 100, ProjectViewID: 1, Position: 999}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 101, ProjectViewID: 1, Position: 999}) + require.NoError(t, err) + + conflicts, err := findPositionConflicts(s, 1, 999) + require.NoError(t, err) + assert.Len(t, conflicts, 2) + }) + + t.Run("no conflicts at nonexistent position", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + conflicts, err := findPositionConflicts(s, 1, 12345) + require.NoError(t, err) + assert.Empty(t, conflicts) + }) +} + +func TestResolveTaskPositionConflicts(t *testing.T) { + t.Run("no conflict to resolve", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Single task - no conflict + conflicts := []*TaskPosition{ + {TaskID: 1, ProjectViewID: 1, Position: 100}, + } + err := resolveTaskPositionConflicts(s, 1, conflicts) + require.NoError(t, err) + }) + + t.Run("resolves conflicts with neighbors", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Set up: Create positions at 100, 200 (conflict), 200 (conflict), 300 + _, err := s.Insert(&TaskPosition{TaskID: 100, ProjectViewID: 1, Position: 100}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 101, ProjectViewID: 1, Position: 200}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 102, ProjectViewID: 1, Position: 200}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 103, ProjectViewID: 1, Position: 300}) + require.NoError(t, err) + + conflicts := []*TaskPosition{ + {TaskID: 101, ProjectViewID: 1, Position: 200}, + {TaskID: 102, ProjectViewID: 1, Position: 200}, + } + + err = resolveTaskPositionConflicts(s, 1, conflicts) + require.NoError(t, err) + + // Check that the positions are now different + var pos1, pos2 TaskPosition + _, err = s.Where("task_id = ? AND project_view_id = ?", 101, 1).Get(&pos1) + require.NoError(t, err) + _, err = s.Where("task_id = ? AND project_view_id = ?", 102, 1).Get(&pos2) + require.NoError(t, err) + + assert.NotEqual(t, pos1.Position, pos2.Position) + // Both should be between 100 and 300 + assert.Greater(t, pos1.Position, 100.0) + assert.Less(t, pos1.Position, 300.0) + assert.Greater(t, pos2.Position, 100.0) + assert.Less(t, pos2.Position, 300.0) + }) + + t.Run("resolves conflicts at start (no left neighbor)", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Clear existing positions for this view to control test data + _, err := s.Where("project_view_id = ?", 99).Delete(&TaskPosition{}) + require.NoError(t, err) + + // Set up: positions at 50 (conflict), 50 (conflict), 100 + _, err = s.Insert(&TaskPosition{TaskID: 200, ProjectViewID: 99, Position: 50}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 201, ProjectViewID: 99, Position: 50}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 202, ProjectViewID: 99, Position: 100}) + require.NoError(t, err) + + conflicts := []*TaskPosition{ + {TaskID: 200, ProjectViewID: 99, Position: 50}, + {TaskID: 201, ProjectViewID: 99, Position: 50}, + } + + err = resolveTaskPositionConflicts(s, 99, conflicts) + require.NoError(t, err) + + // Check positions are unique and between 0 and 100 + var pos1, pos2 TaskPosition + _, err = s.Where("task_id = ? AND project_view_id = ?", 200, 99).Get(&pos1) + require.NoError(t, err) + _, err = s.Where("task_id = ? AND project_view_id = ?", 201, 99).Get(&pos2) + require.NoError(t, err) + + assert.NotEqual(t, pos1.Position, pos2.Position) + assert.GreaterOrEqual(t, pos1.Position, 0.0) + assert.Less(t, pos1.Position, 100.0) + assert.GreaterOrEqual(t, pos2.Position, 0.0) + assert.Less(t, pos2.Position, 100.0) + }) + + t.Run("resolves conflicts at end (no right neighbor)", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Clear existing positions for this view + _, err := s.Where("project_view_id = ?", 98).Delete(&TaskPosition{}) + require.NoError(t, err) + + // Set up: positions at 100, 200 (conflict), 200 (conflict) - no right neighbor + _, err = s.Insert(&TaskPosition{TaskID: 300, ProjectViewID: 98, Position: 100}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 301, ProjectViewID: 98, Position: 200}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 302, ProjectViewID: 98, Position: 200}) + require.NoError(t, err) + + conflicts := []*TaskPosition{ + {TaskID: 301, ProjectViewID: 98, Position: 200}, + {TaskID: 302, ProjectViewID: 98, Position: 200}, + } + + err = resolveTaskPositionConflicts(s, 98, conflicts) + require.NoError(t, err) + + // Check positions are unique and > 100 + var pos1, pos2 TaskPosition + _, err = s.Where("task_id = ? AND project_view_id = ?", 301, 98).Get(&pos1) + require.NoError(t, err) + _, err = s.Where("task_id = ? AND project_view_id = ?", 302, 98).Get(&pos2) + require.NoError(t, err) + + assert.NotEqual(t, pos1.Position, pos2.Position) + assert.Greater(t, pos1.Position, 100.0) + assert.Greater(t, pos2.Position, 100.0) + }) + + t.Run("returns error when spacing exhausted", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Clear existing positions for this view + _, err := s.Where("project_view_id = ?", 97).Delete(&TaskPosition{}) + require.NoError(t, err) + + // Set up: extremely tight spacing that can't accommodate multiple tasks + // Gap of 2e-9 with 2 conflicts means spacing of ~6.67e-10 < MinPositionSpacing (1e-9) + basePos := 100.0 + tinyGap := 1e-9 + _, err = s.Insert(&TaskPosition{TaskID: 400, ProjectViewID: 97, Position: basePos}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 401, ProjectViewID: 97, Position: basePos + tinyGap}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 402, ProjectViewID: 97, Position: basePos + tinyGap}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 403, ProjectViewID: 97, Position: basePos + 2*tinyGap}) + require.NoError(t, err) + + conflicts := []*TaskPosition{ + {TaskID: 401, ProjectViewID: 97, Position: basePos + tinyGap}, + {TaskID: 402, ProjectViewID: 97, Position: basePos + tinyGap}, + } + + err = resolveTaskPositionConflicts(s, 97, conflicts) + assert.True(t, IsErrNeedsFullRecalculation(err)) + }) + + t.Run("handles multiple conflicts deterministically", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Clear existing positions for this view + _, err := s.Where("project_view_id = ?", 96).Delete(&TaskPosition{}) + require.NoError(t, err) + + // Set up: 4 tasks at the same position + _, err = s.Insert(&TaskPosition{TaskID: 504, ProjectViewID: 96, Position: 0}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 501, ProjectViewID: 96, Position: 500}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 503, ProjectViewID: 96, Position: 500}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 502, ProjectViewID: 96, Position: 500}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 500, ProjectViewID: 96, Position: 500}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 505, ProjectViewID: 96, Position: 1000}) + require.NoError(t, err) + + conflicts := []*TaskPosition{ + {TaskID: 501, ProjectViewID: 96, Position: 500}, + {TaskID: 503, ProjectViewID: 96, Position: 500}, + {TaskID: 502, ProjectViewID: 96, Position: 500}, + {TaskID: 500, ProjectViewID: 96, Position: 500}, + } + + err = resolveTaskPositionConflicts(s, 96, conflicts) + require.NoError(t, err) + + // Fetch all positions and verify they are unique and ordered by task ID + var positions []*TaskPosition + err = s.Where("project_view_id = ? AND task_id IN (500, 501, 502, 503)", 96). + OrderBy("task_id ASC"). + Find(&positions) + require.NoError(t, err) + require.Len(t, positions, 4) + + // Positions should be strictly increasing (sorted by task_id) + for i := 1; i < len(positions); i++ { + assert.Greater(t, positions[i].Position, positions[i-1].Position, + "Position for task %d should be greater than task %d", + positions[i].TaskID, positions[i-1].TaskID) + } + + // All should be between 0 and 1000 + for _, p := range positions { + assert.Greater(t, p.Position, 0.0) + assert.Less(t, p.Position, 1000.0) + } + }) +} + +func TestUpdateTaskPositionWithConflictResolution(t *testing.T) { + t.Run("resolves conflict on update", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Clear existing positions for this view + _, err := s.Where("project_view_id = ?", 95).Delete(&TaskPosition{}) + require.NoError(t, err) + + // Set up: two tasks with different positions + _, err = s.Insert(&TaskPosition{TaskID: 600, ProjectViewID: 95, Position: 100}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 601, ProjectViewID: 95, Position: 200}) + require.NoError(t, err) + + // Update task 600 to have the same position as task 601 + tp := &TaskPosition{ + TaskID: 600, + ProjectViewID: 95, + Position: 200, + } + + err = updateTaskPosition(s, nil, tp) + require.NoError(t, err) + + // Verify both tasks now have unique positions + var pos1, pos2 TaskPosition + _, err = s.Where("task_id = ? AND project_view_id = ?", 600, 95).Get(&pos1) + require.NoError(t, err) + _, err = s.Where("task_id = ? AND project_view_id = ?", 601, 95).Get(&pos2) + require.NoError(t, err) + + assert.NotEqual(t, pos1.Position, pos2.Position) + }) +} + +func TestRepairTaskPositions(t *testing.T) { + t.Run("no duplicates to repair", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Clear all positions and set up clean data with no duplicates + _, err := s.Where("project_view_id = ?", 94).Delete(&TaskPosition{}) + require.NoError(t, err) + + _, err = s.Insert(&TaskPosition{TaskID: 700, ProjectViewID: 94, Position: 100}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 701, ProjectViewID: 94, Position: 200}) + require.NoError(t, err) + + result, err := RepairTaskPositions(s, false) + require.NoError(t, err) + + // View 94 should be scanned but not repaired (no duplicates) + assert.GreaterOrEqual(t, result.ViewsScanned, 1) + assert.Empty(t, result.Errors) + }) + + t.Run("repairs duplicates in view", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Clear and set up duplicates + _, err := s.Where("project_view_id = ?", 93).Delete(&TaskPosition{}) + require.NoError(t, err) + + _, err = s.Insert(&TaskPosition{TaskID: 800, ProjectViewID: 93, Position: 100}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 801, ProjectViewID: 93, Position: 200}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 802, ProjectViewID: 93, Position: 200}) // Duplicate! + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 803, ProjectViewID: 93, Position: 300}) + require.NoError(t, err) + + result, err := RepairTaskPositions(s, false) + require.NoError(t, err) + + assert.GreaterOrEqual(t, result.ViewsRepaired, 1) + assert.GreaterOrEqual(t, result.TasksAffected, 2) + assert.Empty(t, result.Errors) + + // Verify positions are now unique + var pos1, pos2 TaskPosition + _, err = s.Where("task_id = ? AND project_view_id = ?", 801, 93).Get(&pos1) + require.NoError(t, err) + _, err = s.Where("task_id = ? AND project_view_id = ?", 802, 93).Get(&pos2) + require.NoError(t, err) + + assert.NotEqual(t, pos1.Position, pos2.Position) + }) + + t.Run("dry run reports without changes", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Clear and set up duplicates + _, err := s.Where("project_view_id = ?", 92).Delete(&TaskPosition{}) + require.NoError(t, err) + + _, err = s.Insert(&TaskPosition{TaskID: 900, ProjectViewID: 92, Position: 500}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 901, ProjectViewID: 92, Position: 500}) // Duplicate! + require.NoError(t, err) + + result, err := RepairTaskPositions(s, true) // dry run + require.NoError(t, err) + + assert.GreaterOrEqual(t, result.ViewsRepaired, 1) + assert.GreaterOrEqual(t, result.TasksAffected, 2) + + // Verify positions are still duplicates (dry run shouldn't change them) + var pos1, pos2 TaskPosition + _, err = s.Where("task_id = ? AND project_view_id = ?", 900, 92).Get(&pos1) + require.NoError(t, err) + _, err = s.Where("task_id = ? AND project_view_id = ?", 901, 92).Get(&pos2) + require.NoError(t, err) + + assert.InDelta(t, pos1.Position, pos2.Position, 0) // Still duplicates + }) + + t.Run("handles multiple views", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + // Set up duplicates in two different views + _, err := s.Where("project_view_id IN (90, 91)").Delete(&TaskPosition{}) + require.NoError(t, err) + + // View 90: duplicates + _, err = s.Insert(&TaskPosition{TaskID: 1000, ProjectViewID: 90, Position: 100}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 1001, ProjectViewID: 90, Position: 100}) + require.NoError(t, err) + + // View 91: duplicates + _, err = s.Insert(&TaskPosition{TaskID: 1002, ProjectViewID: 91, Position: 200}) + require.NoError(t, err) + _, err = s.Insert(&TaskPosition{TaskID: 1003, ProjectViewID: 91, Position: 200}) + require.NoError(t, err) + + result, err := RepairTaskPositions(s, false) + require.NoError(t, err) + + assert.GreaterOrEqual(t, result.ViewsRepaired, 2) + assert.GreaterOrEqual(t, result.TasksAffected, 4) + assert.Empty(t, result.Errors) + }) +}