fix(import): create imported tasks in one batch with preserved indexes

Top-level and related tasks are collected first and created in one batch so preset indexes survive, related references resolve to the created struct instead of a mutable id lookup, untitled relation stubs are skipped, and positions and buckets remap through one old-id map.
2026-11-10 15:16:20 -06:00 · 2026-09-11 20:47:11 +02:00
parent c0545eb559
commit b263affe93
2 changed files with 326 additions and 56 deletions
@@ -448,34 +448,75 @@ func createProjectWithEverything(s *xorm.Session, project *models.ProjectWithTas

 	seedMissingTaskPositions(tasks)

-	tasksByOldID := make(map[int64]*models.TaskWithComments, len(tasks))
-	newTaskIDs := []int64{}
-	// Create all tasks
-	for i, t := range tasks {
-		oldid := t.ID
-		t.ProjectID = project.ID
-		originalBucketID := t.BucketID
-		t.BucketID = 0
-		t.Assignees = remapAssignees(t.Assignees, user)
-		err = t.Create(s, user)
-		if err != nil {
-			if models.IsErrTaskCannotBeEmpty(err) {
-				continue
+	type initialTask struct {
+		task     *models.Task
+		comments []*models.TaskComment
+		bucketID int64
+	}
+	initialTasks := make([]initialTask, 0, len(tasks))
+	// Duplicate old ids resolve to the first queued struct.
+	canonicalByOldID := make(map[int64]*models.Task, len(tasks))
+	queue := func(task *models.Task, comments []*models.TaskComment) {
+		initialTasks = append(initialTasks, initialTask{
+			task:     task,
+			comments: comments,
+			bucketID: task.BucketID,
+		})
+		if task.ID != 0 {
+			if _, exists := canonicalByOldID[task.ID]; !exists {
+				canonicalByOldID[task.ID] = task
 			}
-			return err
 		}
+	}
+	for _, t := range tasks {
+		if t == nil || t.Title == "" {
+			continue
+		}
+		queue(&t.Task, t.Comments)
+	}
+	for i := 0; i < len(initialTasks); i++ {
+		for kind, relatedTasks := range initialTasks[i].task.RelatedTasks {
+			kept := make([]*models.Task, 0, len(relatedTasks))
+			for _, related := range relatedTasks {
+				if related == nil {
+					continue
+				}
+				// Id-only references (TickTick parents) must resolve before the title check.
+				if canonical, exists := canonicalByOldID[related.ID]; exists {
+					kept = append(kept, canonical)
+					continue
+				}
+				if related.Title == "" {
+					continue
+				}
+				queue(related, nil)
+				kept = append(kept, related)
+			}
+			initialTasks[i].task.RelatedTasks[kind] = kept
+		}
+	}

-		t.BucketID = originalBucketID
+	tasksToCreate := make([]*models.Task, 0, len(initialTasks))
+	for _, state := range initialTasks {
+		state.task.ProjectID = project.ID
+		state.task.BucketID = 0
+		state.task.Assignees = remapAssignees(state.task.Assignees, user)
+		tasksToCreate = append(tasksToCreate, state.task)
+	}
+	err = models.CreateTasksForImport(s, project.ID, tasksToCreate, user)
+	if err != nil {
+		return err
+	}

-		err = setBucketOrDefault(&tasks[i].Task)
+	for _, state := range initialTasks {
+		t := state.task
+		t.BucketID = state.bucketID
+
+		err = setBucketOrDefault(t)
 		if err != nil {
 			return
 		}

-		newTaskIDs = append(newTaskIDs, t.ID)
-
-		tasksByOldID[oldid] = t
-
 		log.Debugf("[creating structure] Created task %d", t.ID)
 		if len(t.RelatedTasks) > 0 {
 			log.Debugf("[creating structure] Creating %d related task kinds", len(t.RelatedTasks))
@@ -489,39 +530,11 @@ func createProjectWithEverything(s *xorm.Session, project *models.ProjectWithTas
 			}

 			for _, rt := range tasks {
-				// First create the related tasks if they do not exist
-				if _, exists := tasksByOldID[rt.ID]; !exists || rt.ID == 0 {
-					oldid := rt.ID
-					rt.ProjectID = t.ProjectID
-					originalBucketID := rt.BucketID
-					rt.BucketID = 0
-					rt.Assignees = remapAssignees(rt.Assignees, user)
-
-					err = rt.Create(s, user)
-					if err != nil {
-						log.Debugf("[creating structure] Error while creating related task %d: %s", rt.ID, err.Error())
-						return
-					}
-
-					rt.BucketID = originalBucketID
-
-					err = setBucketOrDefault(rt)
-					if err != nil {
-						return
-					}
-					tasksByOldID[oldid] = &models.TaskWithComments{Task: *rt}
-					log.Debugf("[creating structure] Created related task %d", rt.ID)
-				}
-
-				// Then create the relation
 				taskRel := &models.TaskRelation{
 					TaskID:       t.ID,
 					OtherTaskID:  rt.ID,
 					RelationKind: kind,
 				}
-				if ttt, exists := tasksByOldID[rt.ID]; exists {
-					taskRel.OtherTaskID = ttt.ID
-				}

 				// Add this check to prevent self-relations
 				if taskRel.TaskID == taskRel.OtherTaskID {
@@ -615,7 +628,7 @@ func createProjectWithEverything(s *xorm.Session, project *models.ProjectWithTas
 		}

 		// Comments
-		for _, comment := range t.Comments {
+		for _, comment := range state.comments {
 			comment.TaskID = t.ID
 			comment.ID = 0
 			err = comment.CreateWithTimestamps(s, user)
@@ -670,21 +683,23 @@ func createProjectWithEverything(s *xorm.Session, project *models.ProjectWithTas

 	if len(viewsByOldIDs) > 0 {
 		newPositions := []*models.TaskPosition{}
+		positionTaskIDs := make([]int64, 0, len(project.Positions))
 		for _, pos := range project.Positions {
-			_, hasTask := tasksByOldID[pos.TaskID]
+			_, hasTask := canonicalByOldID[pos.TaskID]
 			_, hasView := viewsByOldIDs[pos.ProjectViewID]
 			if !hasTask || !hasView {
 				continue
 			}
 			newPositions = append(newPositions, &models.TaskPosition{
-				TaskID:        tasksByOldID[pos.TaskID].ID,
+				TaskID:        canonicalByOldID[pos.TaskID].ID,
 				ProjectViewID: viewsByOldIDs[pos.ProjectViewID].ID,
 				Position:      pos.Position,
 			})
+			positionTaskIDs = append(positionTaskIDs, canonicalByOldID[pos.TaskID].ID)
 		}

 		if len(newPositions) > 0 {
-			_, err = s.In("task_id", newTaskIDs).Delete(&models.TaskPosition{})
+			_, err = s.In("task_id", positionTaskIDs).Delete(&models.TaskPosition{})
 			if err != nil {
 				return
 			}
@@ -695,21 +710,23 @@ func createProjectWithEverything(s *xorm.Session, project *models.ProjectWithTas
 		}

 		newTaskBuckets := make([]*models.TaskBucket, 0, len(project.TaskBuckets))
+		bucketTaskIDs := make([]int64, 0, len(project.TaskBuckets))
 		for _, tb := range project.TaskBuckets {
-			_, hasTask := tasksByOldID[tb.TaskID]
+			_, hasTask := canonicalByOldID[tb.TaskID]
 			_, hasBucket := bucketsByOldID[tb.BucketID]
 			if !hasTask || !hasBucket {
 				continue
 			}
 			newTaskBuckets = append(newTaskBuckets, &models.TaskBucket{
-				TaskID:        tasksByOldID[tb.TaskID].ID,
+				TaskID:        canonicalByOldID[tb.TaskID].ID,
 				BucketID:      bucketsByOldID[tb.BucketID].ID,
 				ProjectViewID: bucketsByOldID[tb.BucketID].ProjectViewID,
 			})
+			bucketTaskIDs = append(bucketTaskIDs, canonicalByOldID[tb.TaskID].ID)
 		}

 		if len(newTaskBuckets) > 0 {
-			_, err = s.In("task_id", newTaskIDs).Delete(&models.TaskBucket{})
+			_, err = s.In("task_id", bucketTaskIDs).Delete(&models.TaskBucket{})
 			if err != nil {
 				return
 			}
@@ -727,7 +744,7 @@ func createProjectWithEverything(s *xorm.Session, project *models.ProjectWithTas
 }

 // Oversized provider files retain the preloaded path's skip behavior.
-func createAttachmentFromProvider(s *xorm.Session, t *models.TaskWithComments, a *models.TaskAttachment, user *user.User, provider FileProvider, createdFiles *[]int64) (err error) {
+func createAttachmentFromProvider(s *xorm.Session, t *models.Task, a *models.TaskAttachment, user *user.User, provider FileProvider, createdFiles *[]int64) (err error) {
 	oldID := a.ID

 	content, size, err := provider.OpenAttachment(a)
@@ -421,6 +421,259 @@ func TestInsertFromStructure(t *testing.T) {
 			assert.Equal(t, int64(4), count, "task %q must keep position %v in all views", title, position)
 		}
 	})
+	t.Run("preserves generated view rows for related-only tasks", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+
+		child := &models.Task{ID: 20, Title: "Related child"}
+		structure := []*models.ProjectWithTasksAndBuckets{{
+			Project: models.Project{
+				Title: "Import project",
+				Views: []*models.ProjectView{{
+					ID:                      100,
+					Title:                   "Kanban",
+					ViewKind:                models.ProjectViewKindKanban,
+					BucketConfigurationMode: models.BucketConfigurationModeManual,
+					DefaultBucketID:         200,
+				}},
+			},
+			Tasks: []*models.TaskWithComments{{Task: models.Task{
+				ID:       10,
+				Title:    "Parent",
+				BucketID: 200,
+				RelatedTasks: map[models.RelationKind][]*models.Task{
+					models.RelationKindSubtask: {child},
+				},
+			}}},
+			Buckets: []*models.Bucket{{
+				ID:            200,
+				Title:         "Backlog",
+				ProjectViewID: 100,
+			}},
+			Positions: []*models.TaskPosition{{
+				TaskID:        10,
+				ProjectViewID: 100,
+				Position:      123,
+			}},
+			TaskBuckets: []*models.TaskBucket{{
+				TaskID:        10,
+				BucketID:      200,
+				ProjectViewID: 100,
+			}},
+		}}
+
+		require.NoError(t, InsertFromStructure(structure, u))
+
+		s := db.NewSession()
+		defer s.Close()
+
+		viewID := structure[0].Views[0].ID
+		for title, taskID := range map[string]int64{
+			"Parent":        structure[0].Tasks[0].ID,
+			"Related child": child.ID,
+		} {
+			positionCount, err := s.Where("task_id = ? AND project_view_id = ?", taskID, viewID).Count(&models.TaskPosition{})
+			require.NoError(t, err)
+			assert.Equal(t, int64(1), positionCount, "task %q needs a position", title)
+
+			bucketCount, err := s.Where("task_id = ? AND project_view_id = ?", taskID, viewID).Count(&models.TaskBucket{})
+			require.NoError(t, err)
+			assert.Equal(t, int64(1), bucketCount, "task %q needs a bucket", title)
+		}
+
+		parentPosition := &models.TaskPosition{}
+		exists, err := s.Where("task_id = ? AND project_view_id = ?", structure[0].Tasks[0].ID, viewID).Get(parentPosition)
+		require.NoError(t, err)
+		require.True(t, exists)
+		assert.InDelta(t, 123, parentPosition.Position, 0)
+
+		parentBucketCount, err := s.Where(
+			"task_id = ? AND project_view_id = ? AND bucket_id = ?",
+			structure[0].Tasks[0].ID,
+			viewID,
+			structure[0].Buckets[0].ID,
+		).Count(&models.TaskBucket{})
+		require.NoError(t, err)
+		assert.Equal(t, int64(1), parentBucketCount)
+	})
+	t.Run("preserves out-of-order indexes before assigning automatic indexes", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+
+		structure := []*models.ProjectWithTasksAndBuckets{{
+			Project: models.Project{Title: "Import task indexes"},
+			Tasks: []*models.TaskWithComments{
+				{Task: models.Task{Title: "high index", Index: 10}},
+				{Task: models.Task{Title: "low index", Index: 5}},
+				{Task: models.Task{Title: "automatic index"}},
+			},
+		}}
+		require.NoError(t, InsertFromStructure(structure, u))
+
+		assert.Equal(t, int64(10), structure[0].Tasks[0].Index)
+		assert.Equal(t, int64(5), structure[0].Tasks[1].Index)
+		assert.Equal(t, int64(1), structure[0].Tasks[2].Index)
+
+		s := db.NewSession()
+		defer s.Close()
+		counter := &models.ProjectTaskCounter{}
+		has, err := s.ID(structure[0].ID).Get(counter)
+		require.NoError(t, err)
+		require.True(t, has)
+		assert.Equal(t, int64(10), counter.LastIndex)
+	})
+	t.Run("preserves related-only task indexes in the initial batch", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+
+		child := &models.Task{Title: "related child", Index: 5}
+		structure := []*models.ProjectWithTasksAndBuckets{{
+			Project: models.Project{Title: "Related task indexes"},
+			Tasks: []*models.TaskWithComments{{
+				Task: models.Task{
+					Title: "parent",
+					Index: 10,
+					RelatedTasks: models.RelatedTaskMap{
+						models.RelationKindSubtask: {child},
+					},
+				},
+			}},
+		}}
+		require.NoError(t, InsertFromStructure(structure, u))
+
+		assert.Equal(t, int64(10), structure[0].Tasks[0].Index)
+		assert.Equal(t, int64(5), child.Index)
+
+		s := db.NewSession()
+		defer s.Close()
+		counter := &models.ProjectTaskCounter{}
+		has, err := s.ID(structure[0].ID).Get(counter)
+		require.NoError(t, err)
+		require.True(t, has)
+		assert.Equal(t, int64(10), counter.LastIndex)
+	})
+	t.Run("relates the right task when an old id collides with a new one", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+
+		// Task ids are handed out consecutively, so a probe import reveals which ids the next one gets.
+		probe := []*models.ProjectWithTasksAndBuckets{{
+			Project: models.Project{Title: "Id probe"},
+			Tasks:   []*models.TaskWithComments{{Task: models.Task{Title: "probe"}}},
+		}}
+		require.NoError(t, InsertFromStructure(probe, u))
+
+		// The import below creates parent, decoy and child in that order.
+		childNewID := probe[0].Tasks[0].ID + 3
+
+		child := &models.Task{ID: 11, Title: "child"}
+		structure := []*models.ProjectWithTasksAndBuckets{{
+			Project: models.Project{Title: "Colliding task ids"},
+			Tasks: []*models.TaskWithComments{
+				{Task: models.Task{
+					ID:    10,
+					Title: "parent",
+					RelatedTasks: models.RelatedTaskMap{
+						models.RelationKindSubtask: {child},
+					},
+				}},
+				{Task: models.Task{ID: childNewID, Title: "decoy"}},
+			},
+		}}
+		require.NoError(t, InsertFromStructure(structure, u))
+
+		s := db.NewSession()
+		defer s.Close()
+
+		require.Equal(t, childNewID, child.ID, "the child needs to collide with the decoy's old id")
+
+		relation := &models.TaskRelation{}
+		exists, err := s.
+			Where("task_id = ? AND relation_kind = ?", structure[0].Tasks[0].ID, models.RelationKindSubtask).
+			Get(relation)
+		require.NoError(t, err)
+		require.True(t, exists)
+
+		other := &models.Task{}
+		exists, err = s.ID(relation.OtherTaskID).Get(other)
+		require.NoError(t, err)
+		require.True(t, exists)
+		assert.Equal(t, "child", other.Title)
+	})
+	t.Run("creates both top-level tasks when their old ids collide", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+
+		structure := []*models.ProjectWithTasksAndBuckets{{
+			Project: models.Project{Title: "Duplicate old ids"},
+			Tasks: []*models.TaskWithComments{
+				{Task: models.Task{ID: 42, Title: "first duplicate"}},
+				{Task: models.Task{ID: 42, Title: "second duplicate"}},
+			},
+		}}
+		require.NoError(t, InsertFromStructure(structure, u))
+
+		s := db.NewSession()
+		defer s.Close()
+
+		for _, title := range []string{"first duplicate", "second duplicate"} {
+			count, err := s.Where("project_id = ? AND title = ?", structure[0].ID, title).Count(&models.Task{})
+			require.NoError(t, err)
+			assert.Equal(t, int64(1), count, "task %q must exist", title)
+		}
+	})
+	t.Run("skips related tasks without a title", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+
+		structure := []*models.ProjectWithTasksAndBuckets{{
+			Project: models.Project{Title: "Untitled relation"},
+			Tasks: []*models.TaskWithComments{{Task: models.Task{
+				Title: "parent",
+				RelatedTasks: models.RelatedTaskMap{
+					models.RelationKindSubtask: {{ID: 11}},
+				},
+			}}},
+		}}
+		require.NoError(t, InsertFromStructure(structure, u))
+
+		s := db.NewSession()
+		defer s.Close()
+
+		taskCount, err := s.Where("project_id = ?", structure[0].ID).Count(&models.Task{})
+		require.NoError(t, err)
+		assert.Equal(t, int64(1), taskCount)
+
+		relationCount, err := s.Where("task_id = ?", structure[0].Tasks[0].ID).Count(&models.TaskRelation{})
+		require.NoError(t, err)
+		assert.Equal(t, int64(0), relationCount)
+	})
+	t.Run("resolves id-only related tasks to already queued tasks", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+
+		structure := []*models.ProjectWithTasksAndBuckets{{
+			Project: models.Project{Title: "Id only relation"},
+			Tasks: []*models.TaskWithComments{
+				{Task: models.Task{ID: 100, Title: "parent"}},
+				{Task: models.Task{ID: 101, Title: "child", RelatedTasks: models.RelatedTaskMap{
+					models.RelationKindParenttask: {{ID: 100}},
+				}}},
+			},
+		}}
+		require.NoError(t, InsertFromStructure(structure, u))
+
+		s := db.NewSession()
+		defer s.Close()
+
+		ids := make(map[string]int64, 2)
+		for _, title := range []string{"parent", "child"} {
+			task := &models.Task{}
+			exists, err := s.Where("project_id = ? AND title = ?", structure[0].ID, title).Get(task)
+			require.NoError(t, err)
+			require.True(t, exists, title)
+			ids[title] = task.ID
+		}
+
+		relationCount, err := s.
+			Where("task_id = ? AND other_task_id = ? AND relation_kind = ?", ids["child"], ids["parent"], models.RelationKindParenttask).
+			Count(&models.TaskRelation{})
+		require.NoError(t, err)
+		assert.Equal(t, int64(1), relationCount)
+	})
 	t.Run("assignees from a foreign instance", func(t *testing.T) {
 		db.LoadAndAssertFixtures(t)