diff --git a/pkg/modules/migration/create_from_structure.go b/pkg/modules/migration/create_from_structure.go index abd76f4b4..432c3d447 100644 --- a/pkg/modules/migration/create_from_structure.go +++ b/pkg/modules/migration/create_from_structure.go @@ -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) diff --git a/pkg/modules/migration/create_from_structure_test.go b/pkg/modules/migration/create_from_structure_test.go index f0f3b90c7..d2984a205 100644 --- a/pkg/modules/migration/create_from_structure_test.go +++ b/pkg/modules/migration/create_from_structure_test.go @@ -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)