fix: preserve CalDAV inverse relations when parent has no RELATED-TO (#2389)
- Fixes `removeStaleRelations` in CalDAV storage provider to only remove relations of kinds explicitly declared in the incoming VTODO's `RELATED-TO` properties - When a VTODO has no `RELATED-TO` at all (e.g., a parent task from Tasks.org), no relations are removed — they were auto-created as inverses by child tasks - When a VTODO declares specific relation kinds (e.g., `RELATED-TO;RELTYPE=PARENT`), only relations of that kind are checked for staleness; other kinds (like auto-created `subtask` inverses) are preserved Fixes #2383 --------- Co-authored-by: kolaente <k@knt.li>
This commit is contained in:
Tink
GitHub
parent
28cc9e0571
commit
ada2ebab9e
|
|
@ -523,6 +523,25 @@ func removeStaleRelations(s *xorm.Session, a web.Auth, task *models.Task, newRel
|
|||
|
||||
for relationKind, relatedTasks := range existingTask.RelatedTasks {
|
||||
|
||||
// Only process CalDAV-compatible relation kinds (parenttask, subtask).
|
||||
// Other kinds (related, blocking, etc.) are never set via CalDAV and
|
||||
// should not be removed here.
|
||||
if relationKind != models.RelationKindParenttask && relationKind != models.RelationKindSubtask {
|
||||
continue
|
||||
}
|
||||
|
||||
// For subtask relations: only consider removal if the VTODO explicitly
|
||||
// declares RELATED-TO;RELTYPE=CHILD (i.e., subtask kind is a key in
|
||||
// newRelations). Subtask relations are often auto-created as inverses
|
||||
// when child tasks declare RELATED-TO;RELTYPE=PARENT pointing to this
|
||||
// task. Removing them just because this task's VTODO doesn't mention
|
||||
// RELATED-TO;RELTYPE=CHILD would break those child-declared links.
|
||||
if relationKind == models.RelationKindSubtask {
|
||||
if _, hasSubtaskKind := newRelations[models.RelationKindSubtask]; !hasSubtaskKind {
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
for _, relatedTask := range relatedTasks {
|
||||
relationInNewList := slices.ContainsFunc(newRelations[relationKind], func(newRelation *models.Task) bool { return newRelation.UID == relatedTask.UID })
|
||||
|
||||
|
|
|
|||
|
|
@ -22,6 +22,8 @@ import (
|
|||
"strings"
|
||||
"testing"
|
||||
|
||||
"code.vikunja.io/api/pkg/db"
|
||||
"code.vikunja.io/api/pkg/models"
|
||||
"code.vikunja.io/api/pkg/routes/caldav"
|
||||
|
||||
ics "github.com/arran4/golang-ical"
|
||||
|
|
@ -212,6 +214,257 @@ END:VTODO`
|
|||
assert.Contains(t, rec.Body.String(), "RELATED-TO;RELTYPE=PARENT:uid_child_import")
|
||||
})
|
||||
|
||||
t.Run("Import Task & Subtask (Reverse - Parent without RELATED-TO)", func(t *testing.T) {
|
||||
e, _ := setupTestEnv()
|
||||
|
||||
// Step 1: Subtask arrives FIRST, referencing a parent that doesn't exist yet.
|
||||
// This is the standard Tasks.org behavior: only the child has RELATED-TO.
|
||||
const vtodoSubtaskStub = `BEGIN:VTODO
|
||||
UID:uid_child_no_reltype
|
||||
DTSTAMP:20230301T073337Z
|
||||
SUMMARY:Subtask without parent RELTYPE
|
||||
CREATED:20230301T073337Z
|
||||
LAST-MODIFIED:20230301T073337Z
|
||||
RELATED-TO;RELTYPE=PARENT:uid_parent_no_reltype
|
||||
END:VTODO`
|
||||
|
||||
const subtaskVTODO = vtodoHeader + vtodoSubtaskStub + vtodoFooter
|
||||
rec, err := newCaldavTestRequestWithUser(t, e, http.MethodPut, caldav.TaskHandler, &testuser15, subtaskVTODO, nil, map[string]string{"project": "36", "task": "uid_child_no_reltype"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 201, rec.Result().StatusCode)
|
||||
|
||||
// Step 2: Parent arrives with NO RELATED-TO at all.
|
||||
// This is how Tasks.org sends parent tasks — no RELATED-TO;RELTYPE=CHILD.
|
||||
const vtodoParentStub = `BEGIN:VTODO
|
||||
UID:uid_parent_no_reltype
|
||||
DTSTAMP:20230301T073337Z
|
||||
SUMMARY:Parent without RELTYPE
|
||||
CREATED:20230301T073337Z
|
||||
LAST-MODIFIED:20230301T073337Z
|
||||
END:VTODO`
|
||||
|
||||
const parentVTODO = vtodoHeader + vtodoParentStub + vtodoFooter
|
||||
rec, err = newCaldavTestRequestWithUser(t, e, http.MethodPut, caldav.TaskHandler, &testuser15, parentVTODO, nil, map[string]string{"project": "36", "task": "uid_parent_no_reltype"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 201, rec.Result().StatusCode)
|
||||
|
||||
// Step 3: Verify relations at the DB level.
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
childTasks, err := models.GetTasksByUIDs(s, []string{"uid_child_no_reltype"}, &testuser15)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, childTasks, 1)
|
||||
childTask := childTasks[0]
|
||||
|
||||
parentTasks, err := models.GetTasksByUIDs(s, []string{"uid_parent_no_reltype"}, &testuser15)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, parentTasks, 1)
|
||||
parentTask := parentTasks[0]
|
||||
|
||||
// Parent should have correct title (DUMMY should have been replaced)
|
||||
assert.Equal(t, "Parent without RELTYPE", parentTask.Title)
|
||||
|
||||
// No DUMMY-UID tasks should remain
|
||||
db.AssertMissing(t, "tasks", map[string]interface{}{
|
||||
"title": "DUMMY-UID-uid_parent_no_reltype",
|
||||
})
|
||||
|
||||
// Subtask should still have parenttask relation to parent
|
||||
db.AssertExists(t, "task_relations", map[string]interface{}{
|
||||
"task_id": childTask.ID,
|
||||
"other_task_id": parentTask.ID,
|
||||
"relation_kind": models.RelationKindParenttask,
|
||||
}, false)
|
||||
|
||||
// Parent should have the inverse subtask relation
|
||||
db.AssertExists(t, "task_relations", map[string]interface{}{
|
||||
"task_id": parentTask.ID,
|
||||
"other_task_id": childTask.ID,
|
||||
"relation_kind": models.RelationKindSubtask,
|
||||
}, false)
|
||||
})
|
||||
|
||||
t.Run("Parent re-sync without RELATED-TO preserves child relations", func(t *testing.T) {
|
||||
e, _ := setupTestEnv()
|
||||
|
||||
// Step 1: Parent created first (no RELATED-TO).
|
||||
const vtodoParentStub = `BEGIN:VTODO
|
||||
UID:uid_parent_resync
|
||||
DTSTAMP:20230301T073337Z
|
||||
SUMMARY:Parent for resync test
|
||||
CREATED:20230301T073337Z
|
||||
LAST-MODIFIED:20230301T073337Z
|
||||
END:VTODO`
|
||||
|
||||
const parentVTODO = vtodoHeader + vtodoParentStub + vtodoFooter
|
||||
rec, err := newCaldavTestRequestWithUser(t, e, http.MethodPut, caldav.TaskHandler, &testuser15, parentVTODO, nil, map[string]string{"project": "36", "task": "uid_parent_resync"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 201, rec.Result().StatusCode)
|
||||
|
||||
// Step 2: Subtask arrives with RELATED-TO;RELTYPE=PARENT.
|
||||
const vtodoSubtaskStub = `BEGIN:VTODO
|
||||
UID:uid_child_resync
|
||||
DTSTAMP:20230301T073337Z
|
||||
SUMMARY:Child for resync test
|
||||
CREATED:20230301T073337Z
|
||||
LAST-MODIFIED:20230301T073337Z
|
||||
RELATED-TO;RELTYPE=PARENT:uid_parent_resync
|
||||
END:VTODO`
|
||||
|
||||
const subtaskVTODO = vtodoHeader + vtodoSubtaskStub + vtodoFooter
|
||||
rec, err = newCaldavTestRequestWithUser(t, e, http.MethodPut, caldav.TaskHandler, &testuser15, subtaskVTODO, nil, map[string]string{"project": "36", "task": "uid_child_resync"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 201, rec.Result().StatusCode)
|
||||
|
||||
// Step 3: Parent is re-synced (updated) — still no RELATED-TO.
|
||||
// This simulates DAVx5 re-syncing the parent after a change (e.g., title update).
|
||||
const vtodoParentUpdatedStub = `BEGIN:VTODO
|
||||
UID:uid_parent_resync
|
||||
DTSTAMP:20230302T073337Z
|
||||
SUMMARY:Parent for resync test (updated)
|
||||
CREATED:20230301T073337Z
|
||||
LAST-MODIFIED:20230302T073337Z
|
||||
END:VTODO`
|
||||
|
||||
const parentUpdatedVTODO = vtodoHeader + vtodoParentUpdatedStub + vtodoFooter
|
||||
rec, err = newCaldavTestRequestWithUser(t, e, http.MethodPut, caldav.TaskHandler, &testuser15, parentUpdatedVTODO, nil, map[string]string{"project": "36", "task": "uid_parent_resync"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 201, rec.Result().StatusCode)
|
||||
|
||||
// Step 4: Verify relations still intact after parent re-sync.
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
parentTasks, err := models.GetTasksByUIDs(s, []string{"uid_parent_resync"}, &testuser15)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, parentTasks, 1)
|
||||
parentTask := parentTasks[0]
|
||||
|
||||
childTasks, err := models.GetTasksByUIDs(s, []string{"uid_child_resync"}, &testuser15)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, childTasks, 1)
|
||||
childTask := childTasks[0]
|
||||
|
||||
// Parent should have updated title
|
||||
assert.Equal(t, "Parent for resync test (updated)", parentTask.Title)
|
||||
|
||||
// Child should still have parenttask relation to parent
|
||||
db.AssertExists(t, "task_relations", map[string]interface{}{
|
||||
"task_id": childTask.ID,
|
||||
"other_task_id": parentTask.ID,
|
||||
"relation_kind": models.RelationKindParenttask,
|
||||
}, false)
|
||||
|
||||
// Parent should still have inverse subtask relation
|
||||
db.AssertExists(t, "task_relations", map[string]interface{}{
|
||||
"task_id": parentTask.ID,
|
||||
"other_task_id": childTask.ID,
|
||||
"relation_kind": models.RelationKindSubtask,
|
||||
}, false)
|
||||
})
|
||||
|
||||
t.Run("Multiple subtasks with same parent (one-sided RELATED-TO)", func(t *testing.T) {
|
||||
e, _ := setupTestEnv()
|
||||
|
||||
// Step 1: First subtask arrives, parent doesn't exist yet.
|
||||
const vtodoSubtask1Stub = `BEGIN:VTODO
|
||||
UID:uid_multi_child_1
|
||||
DTSTAMP:20230301T073337Z
|
||||
SUMMARY:Multi child 1
|
||||
CREATED:20230301T073337Z
|
||||
LAST-MODIFIED:20230301T073337Z
|
||||
RELATED-TO;RELTYPE=PARENT:uid_multi_parent
|
||||
END:VTODO`
|
||||
|
||||
const subtask1VTODO = vtodoHeader + vtodoSubtask1Stub + vtodoFooter
|
||||
rec, err := newCaldavTestRequestWithUser(t, e, http.MethodPut, caldav.TaskHandler, &testuser15, subtask1VTODO, nil, map[string]string{"project": "36", "task": "uid_multi_child_1"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 201, rec.Result().StatusCode)
|
||||
|
||||
// Step 2: Second subtask arrives, parent should exist as DUMMY now.
|
||||
const vtodoSubtask2Stub = `BEGIN:VTODO
|
||||
UID:uid_multi_child_2
|
||||
DTSTAMP:20230301T073337Z
|
||||
SUMMARY:Multi child 2
|
||||
CREATED:20230301T073337Z
|
||||
LAST-MODIFIED:20230301T073337Z
|
||||
RELATED-TO;RELTYPE=PARENT:uid_multi_parent
|
||||
END:VTODO`
|
||||
|
||||
const subtask2VTODO = vtodoHeader + vtodoSubtask2Stub + vtodoFooter
|
||||
rec, err = newCaldavTestRequestWithUser(t, e, http.MethodPut, caldav.TaskHandler, &testuser15, subtask2VTODO, nil, map[string]string{"project": "36", "task": "uid_multi_child_2"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 201, rec.Result().StatusCode)
|
||||
|
||||
// Step 3: Parent arrives with NO RELATED-TO.
|
||||
const vtodoParentStub = `BEGIN:VTODO
|
||||
UID:uid_multi_parent
|
||||
DTSTAMP:20230301T073337Z
|
||||
SUMMARY:Multi parent
|
||||
CREATED:20230301T073337Z
|
||||
LAST-MODIFIED:20230301T073337Z
|
||||
END:VTODO`
|
||||
|
||||
const parentVTODO = vtodoHeader + vtodoParentStub + vtodoFooter
|
||||
rec, err = newCaldavTestRequestWithUser(t, e, http.MethodPut, caldav.TaskHandler, &testuser15, parentVTODO, nil, map[string]string{"project": "36", "task": "uid_multi_parent"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, 201, rec.Result().StatusCode)
|
||||
|
||||
// Step 4: Verify all relations intact and no DUMMY tasks.
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
parentTasks, err := models.GetTasksByUIDs(s, []string{"uid_multi_parent"}, &testuser15)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, parentTasks, 1)
|
||||
parentTask := parentTasks[0]
|
||||
|
||||
child1Tasks, err := models.GetTasksByUIDs(s, []string{"uid_multi_child_1"}, &testuser15)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, child1Tasks, 1)
|
||||
child1Task := child1Tasks[0]
|
||||
|
||||
child2Tasks, err := models.GetTasksByUIDs(s, []string{"uid_multi_child_2"}, &testuser15)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, child2Tasks, 1)
|
||||
child2Task := child2Tasks[0]
|
||||
|
||||
// Parent should have correct title
|
||||
assert.Equal(t, "Multi parent", parentTask.Title)
|
||||
|
||||
// No DUMMY-UID tasks should remain
|
||||
db.AssertMissing(t, "tasks", map[string]interface{}{
|
||||
"title": "DUMMY-UID-uid_multi_parent",
|
||||
})
|
||||
|
||||
// Child 1 should have parenttask relation to parent
|
||||
db.AssertExists(t, "task_relations", map[string]interface{}{
|
||||
"task_id": child1Task.ID,
|
||||
"other_task_id": parentTask.ID,
|
||||
"relation_kind": models.RelationKindParenttask,
|
||||
}, false)
|
||||
|
||||
// Child 2 should have parenttask relation to parent
|
||||
db.AssertExists(t, "task_relations", map[string]interface{}{
|
||||
"task_id": child2Task.ID,
|
||||
"other_task_id": parentTask.ID,
|
||||
"relation_kind": models.RelationKindParenttask,
|
||||
}, false)
|
||||
|
||||
// Parent should have inverse subtask relations to both children
|
||||
db.AssertExists(t, "task_relations", map[string]interface{}{
|
||||
"task_id": parentTask.ID,
|
||||
"other_task_id": child1Task.ID,
|
||||
"relation_kind": models.RelationKindSubtask,
|
||||
}, false)
|
||||
db.AssertExists(t, "task_relations", map[string]interface{}{
|
||||
"task_id": parentTask.ID,
|
||||
"other_task_id": child2Task.ID,
|
||||
"relation_kind": models.RelationKindSubtask,
|
||||
}, false)
|
||||
})
|
||||
|
||||
t.Run("Delete Subtask", func(t *testing.T) {
|
||||
e, _ := setupTestEnv()
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue