fix: sort subtasks by their position in the view being read

Nested subtasks were returned in the order their relations were created, so they ignored the view's own order - reordering a subtask by dragging it, or sorting the view, left the nesting untouched. Positions only exist per view, and not every caller reads tasks through one: the task detail page, CalDAV and exports pass no view, and a subtask in another project has no row for this view either. Those keep relation id order, which stays stable, and sort after the ones that do have a position.
2026-08-31 12:45:51 -05:00 · 2026-07-26 18:25:35 +02:00
parent 589088ffac
commit 7918351e12
3 changed files with 128 additions and 29 deletions
@@ -48,7 +48,7 @@ func TestAddRelatedTasksToTasks_InheritedProjectAccess(t *testing.T) {
 	}
 	taskIDs := []int64{49}

-	err := addRelatedTasksToTasks(s, taskIDs, taskMap, u)
+	err := addRelatedTasksToTasks(s, taskIDs, taskMap, u, nil)
 	require.NoError(t, err)

 	foundTask50 := false
@@ -81,7 +81,7 @@ func TestAddRelatedTasksToTasks_NoAccessToHierarchy(t *testing.T) {
 	}
 	taskIDs := []int64{49}

-	err := addRelatedTasksToTasks(s, taskIDs, taskMap, u)
+	err := addRelatedTasksToTasks(s, taskIDs, taskMap, u, nil)
 	require.NoError(t, err)

 	foundTask50 := false
@@ -117,7 +117,7 @@ func TestAddRelatedTasksToTasks_FiltersInaccessibleProjects(t *testing.T) {
 	}
 	taskIDs := []int64{1}

-	err := addRelatedTasksToTasks(s, taskIDs, taskMap, u)
+	err := addRelatedTasksToTasks(s, taskIDs, taskMap, u, nil)
 	require.NoError(t, err)

 	// Task 29 is in project 1 (same project, user 1 has access) — should be present
@@ -24,6 +24,7 @@ import (

 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"xorm.io/xorm"
 )

 func TestTaskRelation_Create(t *testing.T) {
@@ -422,32 +423,84 @@ func TestTaskRelation_CanCreate(t *testing.T) {
 }

 func TestTaskRelation_SubtaskOrder(t *testing.T) {
-	// Clients render subtasks in the order the api returns them, so the order must
-	// follow relation creation and not whatever the database hands back.
-	// SQLite returns insertion order even without ORDER BY, so this only catches regressions on MySQL/Postgres.
-	db.LoadAndAssertFixtures(t)
-	s := db.NewSession()
-	defer s.Close()
-
 	u := &user.User{ID: 1}
 	subtaskIDs := []int64{12, 11, 10, 9, 8}
-	for _, subtaskID := range subtaskIDs {
-		rel := TaskRelation{
-			TaskID:       2,
-			OtherTaskID:  subtaskID,
-			RelationKind: RelationKindSubtask,
+
+	// Relations are created in an order that does not match the subtask ids, so an
+	// accidental sort by task id can't pass these.
+	seed := func(t *testing.T, s *xorm.Session) {
+		for _, subtaskID := range subtaskIDs {
+			rel := TaskRelation{
+				TaskID:       2,
+				OtherTaskID:  subtaskID,
+				RelationKind: RelationKindSubtask,
+			}
+			require.NoError(t, rel.Create(s, u))
 		}
-		require.NoError(t, rel.Create(s, u))
 	}

-	taskMap := map[int64]*Task{
-		2: {ID: 2, RelatedTasks: map[RelationKind][]*Task{}},
+	readSubtaskIDs := func(t *testing.T, s *xorm.Session, view *ProjectView) (ids []int64) {
+		taskMap := map[int64]*Task{
+			2: {ID: 2, RelatedTasks: map[RelationKind][]*Task{}},
+		}
+		require.NoError(t, addRelatedTasksToTasks(s, []int64{2}, taskMap, u, view))
+		for _, subtask := range taskMap[2].RelatedTasks[RelationKindSubtask] {
+			ids = append(ids, subtask.ID)
+		}
+		return ids
 	}
-	require.NoError(t, addRelatedTasksToTasks(s, []int64{2}, taskMap, u))

-	var got []int64
-	for _, subtask := range taskMap[2].RelatedTasks[RelationKindSubtask] {
-		got = append(got, subtask.ID)
-	}
-	assert.Equal(t, subtaskIDs, got)
+	t.Run("sorted by their position in the view", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+		seed(t, s)
+
+		// Deliberately the reverse of the relation creation order
+		for i, subtaskID := range subtaskIDs {
+			_, err := s.Insert(&TaskPosition{
+				TaskID:        subtaskID,
+				ProjectViewID: 1,
+				Position:      float64(len(subtaskIDs)-i) * 100,
+			})
+			require.NoError(t, err)
+		}
+
+		view, err := GetProjectViewByID(s, 1)
+		require.NoError(t, err)
+
+		reversed := make([]int64, 0, len(subtaskIDs))
+		for i := len(subtaskIDs) - 1; i >= 0; i-- {
+			reversed = append(reversed, subtaskIDs[i])
+		}
+		assert.Equal(t, reversed, readSubtaskIDs(t, s, view))
+	})
+
+	t.Run("falls back to relation order without a view", func(t *testing.T) {
+		// The task detail page, CalDAV and exports read tasks without a view, so there
+		// are no positions to sort by. Relation id keeps that stable.
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+		seed(t, s)
+
+		assert.Equal(t, subtaskIDs, readSubtaskIDs(t, s, nil))
+	})
+
+	t.Run("sorts subtasks without a position in the view last", func(t *testing.T) {
+		// Subtasks living in another project have no position row for this view.
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+		seed(t, s)
+
+		_, err := s.Insert(&TaskPosition{TaskID: 9, ProjectViewID: 1, Position: 100})
+		require.NoError(t, err)
+
+		view, err := GetProjectViewByID(s, 1)
+		require.NoError(t, err)
+
+		// 9 is positioned so it comes first, the rest keep their relation order
+		assert.Equal(t, []int64{9, 12, 11, 10, 8}, readSubtaskIDs(t, s, view))
+	})
 }
@@ -572,10 +572,10 @@ func getTaskReminderMap(s *xorm.Session, taskIDs []int64) (taskReminders map[int
 	return
 }

-func addRelatedTasksToTasks(s *xorm.Session, taskIDs []int64, taskMap map[int64]*Task, a web.Auth) (err error) {
+func addRelatedTasksToTasks(s *xorm.Session, taskIDs []int64, taskMap map[int64]*Task, a web.Auth, view *ProjectView) (err error) {
 	relatedTasks := []*TaskRelation{}
-	// Ordered by id so clients rendering subtasks get them in the order the relations
-	// were created in, instead of whatever order the database happens to return.
+	// Ordered by id so the relations have a stable order to fall back to when they
+	// cannot be sorted by position - see sortRelatedTasksByPosition.
 	err = s.In("task_id", taskIDs).OrderBy("id ASC").Find(&relatedTasks)
 	if err != nil {
 		return
@@ -627,7 +627,53 @@ func addRelatedTasksToTasks(s *xorm.Session, taskIDs []int64, taskMap map[int64]
 		taskMap[rt.TaskID].RelatedTasks[rt.RelationKind] = append(taskMap[rt.TaskID].RelatedTasks[rt.RelationKind], otherTask)
 	}

-	return
+	return sortRelatedTasksByPosition(s, taskMap, relatedTaskIDs, view)
+}
+
+// sortRelatedTasksByPosition orders every task's related tasks by the position they
+// have in the view being read, so clients rendering subtasks under their parent show
+// them in the same order as the view itself.
+//
+// Not every related task has a position to sort by: there is no view at all on the
+// task detail page, in CalDAV and in exports, and a related task living in another
+// project has no row for this view. Those keep the relation order they were fetched
+// in (by relation id) and sort after the ones that do have a position.
+func sortRelatedTasksByPosition(s *xorm.Session, taskMap map[int64]*Task, relatedTaskIDs []int64, view *ProjectView) (err error) {
+	if view == nil {
+		return nil
+	}
+
+	positions := []*TaskPosition{}
+	err = s.In("task_id", relatedTaskIDs).
+		And("project_view_id = ?", view.ID).
+		Find(&positions)
+	if err != nil {
+		return err
+	}
+	if len(positions) == 0 {
+		return nil
+	}
+
+	positionByTask := make(map[int64]float64, len(positions))
+	for _, p := range positions {
+		positionByTask[p.TaskID] = p.Position
+	}
+
+	for _, task := range taskMap {
+		for _, related := range task.RelatedTasks {
+			// Stable so tasks without a position keep their relation id order
+			sort.SliceStable(related, func(i, j int) bool {
+				a, hasA := positionByTask[related[i].ID]
+				b, hasB := positionByTask[related[j].ID]
+				if hasA != hasB {
+					return hasA
+				}
+				return hasA && a < b
+			})
+		}
+	}
+
+	return nil
 }

 func addBucketsToTasks(s *xorm.Session, a web.Auth, taskIDs []int64, taskMap map[int64]*Task) (err error) {
@@ -817,7 +863,7 @@ func addMoreInfoToTasks(s *xorm.Session, taskMap map[int64]*Task, a web.Auth, vi
 	}

 	// Get all related tasks
-	err = addRelatedTasksToTasks(s, taskIDs, taskMap, a)
+	err = addRelatedTasksToTasks(s, taskIDs, taskMap, a, view)
 	return
 }