fix(tasks): do not return subtasks multiple times when they are related
to multiple tasks Resolves https://community.vikunja.io/t/tasks-with-parent-tasks-appear-duplicated-in-list-views/3676
This commit is contained in:
kolaente
parent
4d36771362
commit
9b579d282c
|
|
@ -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)
|
||||
})
|
||||
})
|
||||
|
|
@ -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)
|
||||
})
|
||||
},
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue