diff --git a/src/app/features/tasks/util/resolve-display-tag-ids.util.spec.ts b/src/app/features/tasks/util/resolve-display-tag-ids.util.spec.ts new file mode 100644 index 0000000000..0209c83ca0 --- /dev/null +++ b/src/app/features/tasks/util/resolve-display-tag-ids.util.spec.ts @@ -0,0 +1,29 @@ +import { resolveDisplayTagIds } from './resolve-display-tag-ids.util'; +import { Task } from '../task.model'; + +const t = (tagIds: string[] | undefined): Pick => ({ + tagIds: tagIds as string[], +}); + +describe('resolveDisplayTagIds', () => { + it('returns own tagIds when the task has any', () => { + expect(resolveDisplayTagIds(t(['own']), t(['parent']))).toEqual(['own']); + }); + + it('falls back to parent tagIds when own is empty', () => { + expect(resolveDisplayTagIds(t([]), t(['parent']))).toEqual(['parent']); + }); + + it('falls back to parent tagIds when own is undefined', () => { + expect(resolveDisplayTagIds(t(undefined), t(['parent']))).toEqual(['parent']); + }); + + it('returns empty array when neither task nor parent has tags', () => { + expect(resolveDisplayTagIds(t([]), t(undefined))).toEqual([]); + expect(resolveDisplayTagIds(t(undefined), undefined)).toEqual([]); + }); + + it('returns own tagIds when no parent is supplied', () => { + expect(resolveDisplayTagIds(t(['own']))).toEqual(['own']); + }); +}); diff --git a/src/app/features/tasks/util/resolve-display-tag-ids.util.ts b/src/app/features/tasks/util/resolve-display-tag-ids.util.ts new file mode 100644 index 0000000000..5d3db6e375 --- /dev/null +++ b/src/app/features/tasks/util/resolve-display-tag-ids.util.ts @@ -0,0 +1,17 @@ +import { Task } from '../task.model'; + +/** + * Resolves the tagIds that represent a task for *display* purposes. Sub-tasks + * may carry their own tags (#7756). When they do, those win; otherwise we fall + * back to the parent's tags so sub-tasks without explicit tags still inherit + * the parent's context (matches the historical UX). Top-level tasks always + * use their own tagIds. + * + * Do not use this in sync / op-log paths — those must read `task.tagIds` + * verbatim, since the parent fallback is a presentation concern, not a state + * one. + */ +export const resolveDisplayTagIds = ( + task: Pick, + parent?: Pick, +): string[] => (task.tagIds?.length ? task.tagIds : parent?.tagIds) ?? []; diff --git a/src/app/features/work-context/work-context-markdown.service.ts b/src/app/features/work-context/work-context-markdown.service.ts index d6716d685f..85f628d34a 100644 --- a/src/app/features/work-context/work-context-markdown.service.ts +++ b/src/app/features/work-context/work-context-markdown.service.ts @@ -75,8 +75,19 @@ export class WorkContextMarkdownService { .pipe(first()) .toPromise()) || []; + // Drop sub-tasks whose parent is also in the loaded set; otherwise the + // sub-task is rendered twice — once nested under the parent, once at the + // top level. Matches the tag-view selector dedup (tag.reducer.ts:121-127). + // Build the set from loaded tasks (not raw `ids`) so a stale parent id in + // tag.taskIds doesn't suppress its real sub-task entry. + const loadedIdSet = new Set( + tasks.filter((task): task is TaskWithSubTasks => !!task).map((task) => task.id), + ); return { - tasks: tasks.filter((task): task is TaskWithSubTasks => !!task), + tasks: tasks.filter( + (task): task is TaskWithSubTasks => + !!task && (!task.parentId || !loadedIdSet.has(task.parentId)), + ), contextTitle, }; } diff --git a/src/app/features/worklog/util/get-complete-state-for-work-context.util.spec.ts b/src/app/features/worklog/util/get-complete-state-for-work-context.util.spec.ts index 659ddc4430..6c868cb0d0 100644 --- a/src/app/features/worklog/util/get-complete-state-for-work-context.util.spec.ts +++ b/src/app/features/worklog/util/get-complete-state-for-work-context.util.spec.ts @@ -56,6 +56,33 @@ describe('getCompleteStateForWorkContext', () => { expect(r.completeStateForWorkContext.ids).toEqual(['PT', 'SUB_B', 'SUB_C']); }); + it('should include a sub-task tagged independently of its parent (#7756)', () => { + // Sub-task is tagged TAG_ID, parent is not. Pre-#7756 the sub-task would + // be filtered out because membership was inherited from the parent only. + const ts = fakeTaskStateFromArray([ + { + ...DEFAULT_TASK, + projectId: 'P1', + title: 'PT', + id: 'PT', + tagIds: ['OTHER_TAG'], + subTaskIds: ['SUB_TAGGED'], + }, + { + ...DEFAULT_TASK, + projectId: 'P1', + title: 'SUB_TAGGED', + id: 'SUB_TAGGED', + parentId: 'PT', + tagIds: [TAG_ID], + }, + ]); + const archiveS = fakeTaskStateFromArray([]); + + const r = getCompleteStateForWorkContext(TAG_CTX, ts, archiveS); + expect(r.completeStateForWorkContext.ids).toContain('SUB_TAGGED'); + }); + it('should include sub tasks for tags for the archive', () => { const ts = fakeTaskStateFromArray([ { diff --git a/src/app/features/worklog/util/get-complete-state-for-work-context.util.ts b/src/app/features/worklog/util/get-complete-state-for-work-context.util.ts index 0a166af4d3..0c7b5cd799 100644 --- a/src/app/features/worklog/util/get-complete-state-for-work-context.util.ts +++ b/src/app/features/worklog/util/get-complete-state-for-work-context.util.ts @@ -2,6 +2,7 @@ import { WorkContext, WorkContextType } from '../../work-context/work-context.mo import { Dictionary, EntityState } from '@ngrx/entity'; import { Task } from '../../tasks/task.model'; import { TODAY_TAG } from '../../tag/tag.const'; +import { resolveDisplayTagIds } from '../../tasks/util/resolve-display-tag-ids.util'; export const getCompleteStateForWorkContext = ( workContext: WorkContext, @@ -69,13 +70,8 @@ const _filterIdsForTag = (state: EntityState, workContextId: string): stri (state.ids as string[]).filter((id) => { const t = state.entities[id]; if (!t) return false; - if (t.parentId) { - const parent = state.entities[t.parentId]; - return parent - ? parent.tagIds.includes(workContextId) - : t.tagIds.includes(workContextId); - } - return t.tagIds.includes(workContextId); + const parent = t.parentId ? state.entities[t.parentId] : undefined; + return resolveDisplayTagIds(t, parent).includes(workContextId); }); const _limitStateToIds = ( diff --git a/src/app/features/worklog/worklog-export/worklog-export.util.spec.ts b/src/app/features/worklog/worklog-export/worklog-export.util.spec.ts index 616b987f63..38397b83ff 100644 --- a/src/app/features/worklog/worklog-export/worklog-export.util.spec.ts +++ b/src/app/features/worklog/worklog-export/worklog-export.util.spec.ts @@ -264,6 +264,31 @@ describe('createRows', () => { expect(rows[2].tags).toEqual([tagId2]); }); + it('should use sub-task own tags when present, not parent tags (#7756)', () => { + // Mirrors #7756 follow-up: sub-tasks may now own tags distinct from the + // parent. Worklog rows for the sub-task should report what the sub-task + // actually carries, not silently swap in the parent's tags. + const pId = 'PT_OWN', + sId = 'ST_OWN', + parentTagId = 'TagParent', + subOwnTagId = 'TagOwn'; + const parent = createTask({ id: pId }); + const sub = createSubTask({ id: sId }, parent); + const parentTag = createTag(parentTagId, parent); + const subOwnTag = createTag(subOwnTagId, sub); + const ownData = createWorklogData({ + tasks: [parent, sub], + tags: [parentTag, subOwnTag], + }); + + const rows = createRows(ownData, WorklogGrouping.WORKLOG); + // Parent row gets parent's tag, sub-task row gets its own tag. + const parentRow = rows.find((r) => r.titlesWithSub[0] === pId); + const subRow = rows.find((r) => r.titlesWithSub[0] === sId); + expect(parentRow?.tags).toEqual([parentTagId]); + expect(subRow?.tags).toEqual([subOwnTagId]); + }); + it('should have today tags', () => { const todayTaskId = 'T1', todayTagId = 'Tag1'; diff --git a/src/app/features/worklog/worklog-export/worklog-export.util.ts b/src/app/features/worklog/worklog-export/worklog-export.util.ts index 41f7b5620f..1435dbc00e 100644 --- a/src/app/features/worklog/worklog-export/worklog-export.util.ts +++ b/src/app/features/worklog/worklog-export/worklog-export.util.ts @@ -7,6 +7,7 @@ import { unique } from '../../../util/unique'; import { ProjectCopy } from '../../project/project.model'; import { TagCopy } from '../../tag/tag.model'; import { WorklogTask } from '../../tasks/task.model'; +import { resolveDisplayTagIds } from '../../tasks/util/resolve-display-tag-ids.util'; import { WorklogExportSettingsCopy, WorklogGrouping } from '../worklog.model'; import { Log } from '../../../core/log'; import { @@ -204,11 +205,10 @@ const getTaskFields = (task: WorklogTask, data: WorklogExportData): TaskFields = ] : []; - // by design subtasks don't have tags, so we must set its parent's tags - let tags = parentTask ? parentTask.tagIds : task.tagIds; - tags = tags.map( - (tagId) => (data.tags.find((tag) => tag.id === tagId) as TagCopy).title, - ); + const tags = resolveDisplayTagIds( + task, + typeof parentTask === 'object' ? parentTask : undefined, + ).map((tagId) => (data.tags.find((tag) => tag.id === tagId) as TagCopy).title); const tasks = [task]; return { tasks, titlesWithSub, titles, notes, projects, tags }; diff --git a/src/app/pages/search-page/search-page.component.spec.ts b/src/app/pages/search-page/search-page.component.spec.ts index 9ca1141b35..976628a705 100644 --- a/src/app/pages/search-page/search-page.component.spec.ts +++ b/src/app/pages/search-page/search-page.component.spec.ts @@ -193,6 +193,29 @@ describe('SearchPageComponent', () => { expect(latestResults[0].parentTitle).toBe('Parent'); })); + it("should prefer subtask's own tagId over parent's when both have tags (#7756)", fakeAsync(() => { + const parent = createTask({ + id: 'parent-2', + title: 'Parent2', + tagIds: ['tag-parent'], + }); + const child = createTask({ + id: 'child-2', + title: 'Tagged Child', + parentId: 'parent-2', + tagIds: ['tag-own'], + }); + tags$.next([ + createTag({ id: 'tag-parent', title: 'Parent Tag', icon: 'star' }), + createTag({ id: 'tag-own', title: 'Own Tag', icon: 'label' }), + ]); + allTasks$.next([parent, child]); + initAndFlush(); + typeAndFlush('Tagged'); + expect(latestResults.length).toBe(1); + expect(latestResults[0].tagId).toBe('tag-own'); + })); + it('should use project context for tasks with projectId', fakeAsync(() => { projectList$.next([createProject({ id: 'proj-1', title: 'My Project' })]); allTasks$.next([ @@ -294,6 +317,10 @@ describe('SearchPageComponent', () => { })); it('should not crash when parent.tagIds is undefined', fakeAsync(() => { + // Robustness: parent.tagIds undefined must not crash the chip resolution. + // With #7756 the child's own tags win when present, so the child gets its + // own tag here. The no-crash guarantee comes from the optional chaining on + // the fallback path (parent?.tagIds?.[0]). const parent = createTask({ id: 'parent-1', title: 'Parent', @@ -309,6 +336,25 @@ describe('SearchPageComponent', () => { initAndFlush(); typeAndFlush('Child Bad'); expect(latestResults.length).toBe(1); + expect(latestResults[0].tagId).toBe('tag-1'); + })); + + it('should not crash when both task.tagIds and parent.tagIds are undefined', fakeAsync(() => { + const parent = createTask({ + id: 'parent-1', + title: 'Parent', + tagIds: undefined as any, + }); + const child = createTask({ + id: 'child-1', + title: 'Child No Tags', + parentId: 'parent-1', + tagIds: undefined as any, + }); + allTasks$.next([parent, child]); + initAndFlush(); + typeAndFlush('Child No'); + expect(latestResults.length).toBe(1); expect(latestResults[0].tagId).toBeUndefined(); })); diff --git a/src/app/pages/search-page/search-page.component.ts b/src/app/pages/search-page/search-page.component.ts index 5519e9f014..a8851a99f7 100644 --- a/src/app/pages/search-page/search-page.component.ts +++ b/src/app/pages/search-page/search-page.component.ts @@ -17,6 +17,7 @@ import { Tag } from '../../features/tag/tag.model'; import { ProjectService } from '../../features/project/project.service'; import { TagService } from '../../features/tag/tag.service'; import { Task } from '../../features/tasks/task.model'; +import { resolveDisplayTagIds } from '../../features/tasks/util/resolve-display-tag-ids.util'; import { SearchItem } from './search-page.model'; import { NavigateToTaskService } from '../../core-ui/navigate-to-task/navigate-to-task.service'; import { AsyncPipe } from '@angular/common'; @@ -101,11 +102,8 @@ export class SearchPageComponent implements OnInit { const tagMap = new Map(tags.map((t) => [t.id, t])); return tasks.map((task) => { - // By design subtasks cannot have tags. - // If a subtask does not belong to a project, it will neither have a project nor a tag. - // Therefore, we need to use the parent's tag. const parent = task.parentId ? taskMap.get(task.parentId) : undefined; - const tagId = parent ? parent.tagIds?.[0] : task.tagIds?.[0]; + const tagId = resolveDisplayTagIds(task, parent)[0]; const taskNotes = task.notes || ''; return { diff --git a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.spec.ts b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.spec.ts index e949ff1f69..e5c517467b 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.spec.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.spec.ts @@ -1548,203 +1548,7 @@ describe('taskSharedCrudMetaReducer', () => { }); }); - describe('parent/sub-task tag conflict resolution', () => { - describe('addTask action', () => { - it('should remove parent from tag when adding sub-task to same tag', () => { - // Setup: parent task already in tag - const testState = createStateWithExistingTasks( - ['parent-task'], - [], - ['parent-task'], - ); - - // Create sub-task that will be added to the same tag - const subTask = createMockTask({ - id: 'sub-task', - parentId: 'parent-task', - tagIds: ['tag1'], - }); - - const action = TaskSharedActions.addTask({ - task: subTask, - workContextId: 'project1', - workContextType: WorkContextType.PROJECT, - isAddToBottom: false, - isAddToBacklog: false, - }); - - metaReducer(testState, action); - - // Parent should be removed from tag, and tag removed from parent - expectStateUpdate( - { - ...expectTaskUpdate('parent-task', { - tagIds: [], // Tag should be removed from parent - }), - ...expectTagUpdate('tag1', { - taskIds: ['sub-task'], // Only sub-task should remain - }), - }, - action, - mockReducer, - testState, - ); - }); - - it('should remove sub-tasks from tag when adding parent to same tag', () => { - // Setup: sub-tasks already in tag - const testState = createStateWithExistingTasks( - ['sub1', 'sub2', 'other-task'], - [], - ['sub1', 'sub2', 'other-task'], - ); - - // Update sub-tasks to have parentId - testState[TASK_FEATURE_NAME].entities.sub1 = createMockTask({ - id: 'sub1', - parentId: 'parent-task', - tagIds: ['tag1'], - }); - testState[TASK_FEATURE_NAME].entities.sub2 = createMockTask({ - id: 'sub2', - parentId: 'parent-task', - tagIds: ['tag1'], - }); - - // Create parent task with sub-tasks - const parentTask = createMockTask({ - id: 'parent-task', - subTaskIds: ['sub1', 'sub2'], - tagIds: ['tag1'], - }); - - const action = TaskSharedActions.addTask({ - task: parentTask, - workContextId: 'project1', - workContextType: WorkContextType.PROJECT, - isAddToBottom: false, - isAddToBacklog: false, - }); - - metaReducer(testState, action); - - // Sub-tasks should be removed from tag, and tag removed from sub-tasks - expectStateUpdate( - { - ...expectTaskUpdate('sub1', { - tagIds: [], // Tag should be removed - }), - ...expectTaskUpdate('sub2', { - tagIds: [], // Tag should be removed - }), - ...expectTagUpdate('tag1', { - taskIds: ['parent-task', 'other-task'], // Only parent and other-task - }), - }, - action, - mockReducer, - testState, - ); - }); - }); - - describe('updateTask action', () => { - it('should remove parent from tag when adding tag to sub-task', () => { - // Setup: parent task in tag, sub-task without tag - const testState = createStateWithExistingTasks( - ['parent-task', 'sub-task'], - [], - ['parent-task'], - ); - - testState[TASK_FEATURE_NAME].entities['sub-task'] = createMockTask({ - id: 'sub-task', - parentId: 'parent-task', - tagIds: [], - }); - - const action = TaskSharedActions.updateTask({ - task: { - id: 'sub-task', - changes: { tagIds: ['tag1'] }, - }, - }); - - metaReducer(testState, action); - - expectStateUpdate( - { - ...expectTaskUpdate('parent-task', { - tagIds: [], // Tag removed from parent - }), - ...expectTaskUpdate('sub-task', { - tagIds: ['tag1'], // Tag added to sub-task - }), - ...expectTagUpdate('tag1', { - taskIds: ['sub-task'], // Only sub-task in tag - }), - }, - action, - mockReducer, - testState, - ); - }); - - it('should remove sub-tasks from tag when adding tag to parent', () => { - // Setup: sub-tasks in tag, parent without tag - const testState = createStateWithExistingTasks( - ['parent-task', 'sub1', 'sub2'], - [], - ['sub1', 'sub2'], - ); - - testState[TASK_FEATURE_NAME].entities['parent-task'] = createMockTask({ - id: 'parent-task', - subTaskIds: ['sub1', 'sub2'], - tagIds: [], - }); - testState[TASK_FEATURE_NAME].entities.sub1 = createMockTask({ - id: 'sub1', - parentId: 'parent-task', - tagIds: ['tag1'], - }); - testState[TASK_FEATURE_NAME].entities.sub2 = createMockTask({ - id: 'sub2', - parentId: 'parent-task', - tagIds: ['tag1'], - }); - - const action = TaskSharedActions.updateTask({ - task: { - id: 'parent-task', - changes: { tagIds: ['tag1'] }, - }, - }); - - metaReducer(testState, action); - - expectStateUpdate( - { - ...expectTaskUpdate('parent-task', { - tagIds: ['tag1'], // Tag added to parent - }), - ...expectTaskUpdate('sub1', { - tagIds: [], // Tag removed from sub-task - }), - ...expectTaskUpdate('sub2', { - tagIds: [], // Tag removed from sub-task - }), - ...expectTagUpdate('tag1', { - taskIds: ['parent-task'], // Only parent in tag - }), - }, - action, - mockReducer, - testState, - ); - }); - }); - + describe('parent/sub-task tag interaction', () => { describe('convertToMainTask action', () => { it('should keep parent tags when converting sub-task to main task with same tag', () => { // Setup: parent and sub-task exist, parent is in tag @@ -1792,6 +1596,148 @@ describe('taskSharedCrudMetaReducer', () => { ); }); }); + + // Regression test for issue #7756 — parent and sub-task may share a tag. + describe('issue #7756: parent and sub-task should share tags independently', () => { + it('parent retains tag when sub-task gains the same tag', () => { + // Step 1: parent in tag1 with sub-task that has no tags + const initialState = createStateWithExistingTasks( + ['parent-task', 'sub-task'], + [], + ['parent-task'], + ); + initialState[TASK_FEATURE_NAME].entities['parent-task'] = createMockTask({ + id: 'parent-task', + subTaskIds: ['sub-task'], + tagIds: ['tag1'], + }); + initialState[TASK_FEATURE_NAME].entities['sub-task'] = createMockTask({ + id: 'sub-task', + parentId: 'parent-task', + tagIds: [], + }); + + // Step 2: add tag1 to sub-task (simulates Automations plugin or manual add) + const addTagToSub = TaskSharedActions.updateTask({ + task: { id: 'sub-task', changes: { tagIds: ['tag1'] } }, + }); + + metaReducer(initialState, addTagToSub); + + expectStateUpdate( + { + ...expectTaskUpdate('parent-task', { + tagIds: ['tag1'], // parent should KEEP its tag + }), + ...expectTaskUpdate('sub-task', { + tagIds: ['tag1'], + }), + ...expectTagUpdate('tag1', { + taskIds: jasmine.arrayContaining([ + 'parent-task', + 'sub-task', + ]) as unknown as string[], + }), + }, + addTagToSub, + mockReducer, + initialState, + ); + }); + + it('sub-task retains tag when parent gains the same tag', () => { + // Step 3: sub-task in tag1, parent without tag1 + const initialState = createStateWithExistingTasks( + ['parent-task', 'sub-task'], + [], + ['sub-task'], + ); + initialState[TASK_FEATURE_NAME].entities['parent-task'] = createMockTask({ + id: 'parent-task', + subTaskIds: ['sub-task'], + tagIds: [], + }); + initialState[TASK_FEATURE_NAME].entities['sub-task'] = createMockTask({ + id: 'sub-task', + parentId: 'parent-task', + tagIds: ['tag1'], + }); + + // Step 4: add tag1 to parent (user "adds the tag back") + const addTagToParent = TaskSharedActions.updateTask({ + task: { id: 'parent-task', changes: { tagIds: ['tag1'] } }, + }); + + metaReducer(initialState, addTagToParent); + + expectStateUpdate( + { + ...expectTaskUpdate('parent-task', { + tagIds: ['tag1'], + }), + ...expectTaskUpdate('sub-task', { + tagIds: ['tag1'], // sub-task should KEEP its tag + }), + ...expectTagUpdate('tag1', { + taskIds: jasmine.arrayContaining([ + 'parent-task', + 'sub-task', + ]) as unknown as string[], + }), + }, + addTagToParent, + mockReducer, + initialState, + ); + }); + + it('addTask: parent retains tag when sub-task is created with the same tag', () => { + // Mirrors the Automations-plugin flow: parent already in tag, sub-task + // gets created (via addTask) and the automation immediately tags it. + const initialState = createStateWithExistingTasks( + ['parent-task'], + [], + ['parent-task'], + ); + initialState[TASK_FEATURE_NAME].entities['parent-task'] = createMockTask({ + id: 'parent-task', + subTaskIds: [], + tagIds: ['tag1'], + }); + + const subTask = createMockTask({ + id: 'sub-task', + parentId: 'parent-task', + tagIds: ['tag1'], + }); + const action = TaskSharedActions.addTask({ + task: subTask, + workContextId: 'project1', + workContextType: WorkContextType.PROJECT, + isAddToBottom: false, + isAddToBacklog: false, + }); + + metaReducer(initialState, action); + + expectStateUpdate( + { + ...expectTaskUpdate('parent-task', { + tagIds: ['tag1'], // parent should KEEP its tag + }), + ...expectTagUpdate('tag1', { + taskIds: jasmine.arrayContaining([ + 'parent-task', + 'sub-task', + ]) as unknown as string[], + }), + }, + action, + mockReducer, + initialState, + ); + }); + }); }); describe('restoreDeletedTask action', () => { diff --git a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.ts b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.ts index b40a7970b6..c8fcc7c6fe 100644 --- a/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.ts +++ b/src/app/root-store/meta/task-shared-meta-reducers/task-shared-crud.reducer.ts @@ -43,77 +43,6 @@ import { } from './task-shared-helpers'; import { plannerFeatureKey } from '../../../features/planner/store/planner.reducer'; -// ============================================================================= -// HELPER FUNCTIONS -// ============================================================================= - -/** - * Removes parent or sub-tasks from a tag's task list to maintain the constraint - * that parent and sub-tasks should never be in the same tag list. - * Also removes the tag from the conflicting tasks. - * - * @param state - The current state - * @param taskId - The task being added - * @param tagId - The tag to check - * @returns Updated state with conflicts resolved - */ -const removeConflictingTasksFromTag = ( - state: RootState, - taskId: string, - tagId: string, -): RootState => { - const task = state[TASK_FEATURE_NAME].entities[taskId] as Task; - if (!task) return state; - - const tag = getTag(state, tagId); - // PERF: Use Set for O(1) lookups instead of O(n) Array.includes() - const tagTaskIdSet = new Set(tag.taskIds); - const conflictingTaskIds: string[] = []; - - // If this is a sub-task, check if parent is in the tag - if (task.parentId && tagTaskIdSet.has(task.parentId)) { - conflictingTaskIds.push(task.parentId); - } - - // If this is a parent task, check if any sub-tasks are in the tag - if (task.subTaskIds && task.subTaskIds.length > 0) { - const subTasksInTag = task.subTaskIds.filter((subId) => tagTaskIdSet.has(subId)); - conflictingTaskIds.push(...subTasksInTag); - } - - if (conflictingTaskIds.length === 0) { - return state; - } - - // Update the conflicting tasks to remove the tag - const taskUpdates: Update[] = conflictingTaskIds.map((conflictingTaskId) => { - const conflictingTask = state[TASK_FEATURE_NAME].entities[conflictingTaskId] as Task; - return { - id: conflictingTaskId, - changes: { - tagIds: conflictingTask.tagIds.filter((id) => id !== tagId), - }, - }; - }); - - // Update the task state - const updatedState = { - ...state, - [TASK_FEATURE_NAME]: taskAdapter.updateMany(taskUpdates, state[TASK_FEATURE_NAME]), - }; - - // PERF: Use Set for O(1) lookups when filtering - const conflictingSet = new Set(conflictingTaskIds); - const tagUpdate: Update = { - id: tagId, - changes: { - taskIds: tag.taskIds.filter((id) => !conflictingSet.has(id)), - }, - }; - - return updateTags(updatedState, [tagUpdate]); -}; - // ============================================================================= // ACTION HANDLERS // ============================================================================= @@ -170,12 +99,7 @@ const handleAddTask = ( ...(shouldAddToToday ? [TODAY_TAG.id] : []), // Add TODAY_TAG if task is for today ].filter((tagId) => state[TAG_FEATURE_NAME].entities[tagId]); - // First, handle conflicts for all tags - for (const tagId of tagIdsToUpdate) { - updatedState = removeConflictingTasksFromTag(updatedState, task.id, tagId); - } - - // Then add the task to all its tags + // Add the task to all its tags const tagUpdates = tagIdsToUpdate.map( (tagId): Update => ({ id: tagId, @@ -762,18 +686,11 @@ const handleTagUpdates = ( .filter((newId) => newId !== TODAY_TAG.id) .filter((tagId) => state[TAG_FEATURE_NAME].entities[tagId]); // Only existing tags - let updatedState = state; - - // First, handle conflicts for all tags we're adding to - for (const tagId of tagsToAddTo) { - updatedState = removeConflictingTasksFromTag(updatedState, taskId, tagId); - } - const removeUpdates = tagsToRemoveFrom.map( (tagId): Update => ({ id: tagId, changes: { - taskIds: getTag(updatedState, tagId).taskIds.filter((id) => id !== taskId), + taskIds: getTag(state, tagId).taskIds.filter((id) => id !== taskId), }, }), ); @@ -782,12 +699,12 @@ const handleTagUpdates = ( (tagId): Update => ({ id: tagId, changes: { - taskIds: unique([taskId, ...getTag(updatedState, tagId).taskIds]), + taskIds: unique([taskId, ...getTag(state, tagId).taskIds]), }, }), ); - return updateTags(updatedState, [...removeUpdates, ...addUpdates]); + return updateTags(state, [...removeUpdates, ...addUpdates]); }; // =============================================================================