diff --git a/frontend/cypress/e2e/task/subtask-duplicates.spec.ts b/frontend/cypress/e2e/task/subtask-duplicates.spec.ts new file mode 100644 index 000000000..293a15759 --- /dev/null +++ b/frontend/cypress/e2e/task/subtask-duplicates.spec.ts @@ -0,0 +1,70 @@ +import {createFakeUserAndLogin} from '../../support/authenticateUser' +import {ProjectFactory} from '../../factories/project' +import {TaskFactory} from '../../factories/task' +import {ProjectViewFactory} from '../../factories/project_view' + +function createViews(projectId: number, projectViewId: number) { + return ProjectViewFactory.create(1, { + id: projectViewId, + project_id: projectId, + view_kind: 0, + }, false)[0] +} + +describe('Subtask duplicate handling', () => { + createFakeUserAndLogin() + + let projectA + let projectB + let parentA + let parentB + let subtask + + beforeEach(() => { + ProjectFactory.truncate() + ProjectViewFactory.truncate() + TaskFactory.truncate() + + projectA = ProjectFactory.create(1, {id: 1, title: 'Project A'})[0] + createViews(projectA.id, 1) + projectB = ProjectFactory.create(1, {id: 2, title: 'Project B'}, false)[0] + createViews(projectB.id, 2) + + parentA = TaskFactory.create(1, {id: 10, title: 'Parent A', project_id: projectA.id}, false)[0] + parentB = TaskFactory.create(1, {id: 11, title: 'Parent B', project_id: projectB.id}, false)[0] + subtask = TaskFactory.create(1, {id: 12, title: 'Shared subtask', project_id: projectA.id}, false)[0] + + cy.request({ + method: 'PUT', + url: `${Cypress.env('API_URL')}/tasks/${parentA.id}/relations`, + headers: { + 'Authorization': `Bearer ${window.localStorage.getItem('token')}`, + }, + body: { + other_task_id: subtask.id, + relation_kind: 'subtask', + }, + }) + cy.request({ + method: 'PUT', + url: `${Cypress.env('API_URL')}/tasks/${parentB.id}/relations`, + headers: { + 'Authorization': `Bearer ${window.localStorage.getItem('token')}`, + }, + body: { + other_task_id: subtask.id, + relation_kind: 'subtask', + }, + }) + }) + + it('shows subtask only once in each project list', () => { + cy.visit(`/projects/${projectA.id}/1`) + cy.get('.subtask-nested .task-link').contains(subtask.title).should('exist') + cy.get('.tasks .task-link').contains(subtask.title).should('have.length', 1) + + cy.visit(`/projects/${projectB.id}/1`) + cy.get('.subtask-nested .task-link').contains(subtask.title).should('exist') + cy.get('.tasks .task-link').contains(subtask.title).should('have.length', 1) + }) +}) diff --git a/frontend/src/components/project/views/ProjectList.vue b/frontend/src/components/project/views/ProjectList.vue index 80ce68e4d..e36e05fe0 100644 --- a/frontend/src/components/project/views/ProjectList.vue +++ b/frontend/src/components/project/views/ProjectList.vue @@ -160,22 +160,8 @@ watch( if (props.projectId < 0) { return } - const tasksById = {} - tasks.value.forEach(t => tasksById[t.id] = true) - tasks.value = tasks.value.filter(t => { - if (typeof t.relatedTasks?.parenttask === 'undefined') { - return true - } - - // If the task is a subtask, make sure the parent task is available in the current view as well - for (const pt of t.relatedTasks.parenttask) { - if (typeof tasksById[pt.id] === 'undefined') { - return true - } - } - - return false + return !((t.relatedTasks?.parenttask?.length ?? 0) > 0) }) }, ) diff --git a/pkg/models/task_collection_subtasks_move_test.go b/pkg/models/task_collection_subtasks_move_test.go index 68a919439..ca84c84a5 100644 --- a/pkg/models/task_collection_subtasks_move_test.go +++ b/pkg/models/task_collection_subtasks_move_test.go @@ -96,3 +96,73 @@ func TestTaskSearchWithExpandSubtasks(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, tasks) } + +func TestTaskCollection_SubtaskWithMultipleParentsNoDuplicates(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + u := &user.User{ID: 15} + + // Use existing tasks from fixtures: + // - Task 41: Parent task in project 36 (already exists) + // - Task 42: Another parent task in project 36 (already exists) + // - Task 43: Subtask in project 36 (already a subtask of task 41) + + // Add a second parent relationship: task 43 -> task 42 + // This will make task 43 have multiple parents (task 41 and task 42) + relation := &TaskRelation{ + TaskID: 43, // subtask + OtherTaskID: 42, // second parent + RelationKind: RelationKindParenttask, + CreatedByID: 15, + } + _, err := s.Insert(relation) + require.NoError(t, err) + + // Create inverse relation: task 42 -> task 43 + inverseRelation := &TaskRelation{ + TaskID: 42, // second parent + OtherTaskID: 43, // subtask + RelationKind: RelationKindSubtask, + CreatedByID: 15, + } + _, err = s.Insert(inverseRelation) + require.NoError(t, err) + + // Test Project 36 - should include tasks 41, 42, and 43, but task 43 should only appear once + c := &TaskCollection{ + ProjectID: 36, + Expand: []TaskCollectionExpandable{TaskCollectionExpandSubtasks}, + } + + res, _, _, err := c.ReadAll(s, u, "", 0, 50) + require.NoError(t, err) + tasks, ok := res.([]*Task) + require.True(t, ok) + + // Count how many times task 43 (the subtask) appears + subtaskCount := 0 + for _, task := range tasks { + if task.ID == 43 { + subtaskCount++ + } + } + + // The subtask should appear exactly once (as a subtask, not as a standalone task) + assert.Equal(t, 1, subtaskCount, "Subtask should appear exactly once in Project 36") + + // Verify that both parent tasks are present + foundParent1 := false + foundParent2 := false + for _, task := range tasks { + if task.ID == 41 { + foundParent1 = true + } + if task.ID == 42 { + foundParent2 = true + } + } + assert.True(t, foundParent1, "Parent task 41 should be present") + assert.True(t, foundParent2, "Parent task 42 should be present") +} diff --git a/pkg/models/task_search.go b/pkg/models/task_search.go index 029149cb5..3c53194c7 100644 --- a/pkg/models/task_search.go +++ b/pkg/models/task_search.go @@ -374,24 +374,23 @@ func (d *dbTaskSearcher) Search(opts *taskSearchOptions) (tasks []*Task, totalCo } // fetch subtasks when expanding - if expandSubtasks { + if expandSubtasks && len(tasks) > 0 { subtasks := []*Task{} - taskIDs := []int64{} + taskIDs := []any{} for _, task := range tasks { taskIDs = append(taskIDs, task.ID) } - inQuery := builder.Dialect(db.GetDialect()). - Select("*"). - From("task_relations"). - Where(builder.In("task_id", taskIDs)) - inSQL, inArgs, err := inQuery.ToSQL() - if err != nil { - return nil, totalCount, err - } + var inPlaceholders = strings.Repeat("?,", len(taskIDs)) + inPlaceholders = inPlaceholders[:len(inPlaceholders)-1] - inSQL = strings.TrimPrefix(inSQL, "SELECT * FROM task_relations WHERE") + var notIn = strings.Repeat("?,", len(taskIDs)) + notIn = notIn[:len(notIn)-1] + + allArgs := make([]any, 0, len(taskIDs)*2) + allArgs = append(allArgs, taskIDs...) + allArgs = append(allArgs, taskIDs...) err = d.s.SQL(`SELECT * FROM tasks WHERE id IN (WITH RECURSIVE sub_tasks AS ( SELECT task_id, @@ -400,7 +399,7 @@ func (d *dbTaskSearcher) Search(opts *taskSearchOptions) (tasks []*Task, totalCo created_by_id, created FROM task_relations - WHERE `+inSQL+` + WHERE task_id IN (`+inPlaceholders+`) AND relation_kind = '`+string(RelationKindSubtask)+`' UNION ALL @@ -415,7 +414,7 @@ func (d *dbTaskSearcher) Search(opts *taskSearchOptions) (tasks []*Task, totalCo sub_tasks st ON tr.task_id = st.other_task_id WHERE tr.relation_kind = '`+string(RelationKindSubtask)+`') SELECT other_task_id - FROM sub_tasks)`, inArgs...).Find(&subtasks) + FROM sub_tasks) AND id NOT IN (`+notIn+`)`, allArgs...).Find(&subtasks) if err != nil { return nil, totalCount, err }