mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-18 00:46:45 +00:00
* fix(tasks): allow parent and sub-tasks to share tags (#7756)
Drop the parent/sub-task tag exclusivity constraint in the CRUD
meta-reducer. The constraint forcibly stripped a tag from the parent
whenever a sub-task gained the same tag (and vice versa), which surfaced
as data loss in flows that tag both -- notably the Automations plugin
adding tags on taskCreated.
Tag membership is already derived from task.tagIds (tag.taskIds is only
an ordering hint), and the tag-view selector at tag.reducer.ts:121-127
already filters sub-tasks whose parent has the same tag. The constraint
was a hygiene-driven judgment call from c19b3dbb0e with no documented
rationale and no consumer that relied on the invariant.
Also fix the markdown export in work-context-markdown.service: with
parent+sub-task coexistence now possible, drop sub-tasks whose parent
is also in the requested ids so they aren't rendered twice (once nested
under the parent, once as a top-level entry).
* fix(tasks): show sub-task own tags in worklog and search (#7762)
Follow-up to #7756. Two pre-existing code paths assumed "sub-tasks
don't have tags" and substituted the parent's tags unconditionally:
- Worklog export (worklog-export.util.ts:207) — sub-task rows reported
the parent's tags even when the sub-task carried its own.
- Search results (search-page.component.ts:108) — the context chip on
a sub-task always showed the parent's first tag.
Both now prefer the sub-task's own tags when present and fall back to
the parent's only when the sub-task has none, so the no-tags-on-sub-task
case keeps the historical UX. Updated the misleading "by design" comments
in both files. Added regression tests for the new policy in both specs.
* refactor(tasks): extract resolveDisplayTagIds; fix worklog tag filter
Multi-review of the #7756/#7762 work surfaced two issues:
1. Worklog's _filterIdsForTag in get-complete-state-for-work-context.util.ts
still used "parent wins" for sub-task tag membership, so a sub-task
tagged independently of its parent was excluded from worklog views
before reaching the display-side fix in #7762. Now mirrors the same
"own tags win, parent as fallback" policy. Regression spec added.
2. With three call sites converging on the same policy (worklog filter,
worklog export, search results), extract resolveDisplayTagIds as a
tiny named helper with its own spec. The helper carries a JSDoc
warning that it is for display only — sync/op-log paths must read
task.tagIds verbatim. The previous inline expressions diverged in
shape; collapsing them removes the risk of future drift.
Also tighten the markdown-service dedup to filter against the loaded
task set rather than the requested ids, so a stale parent id in
tag.taskIds (ordering hint) doesn't suppress a real sub-task entry.
This commit is contained in:
parent
334b14aa26
commit
4e3d79edd5
11 changed files with 313 additions and 301 deletions
|
|
@ -0,0 +1,29 @@
|
|||
import { resolveDisplayTagIds } from './resolve-display-tag-ids.util';
|
||||
import { Task } from '../task.model';
|
||||
|
||||
const t = (tagIds: string[] | undefined): Pick<Task, 'tagIds'> => ({
|
||||
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']);
|
||||
});
|
||||
});
|
||||
17
src/app/features/tasks/util/resolve-display-tag-ids.util.ts
Normal file
17
src/app/features/tasks/util/resolve-display-tag-ids.util.ts
Normal file
|
|
@ -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<Task, 'tagIds'>,
|
||||
parent?: Pick<Task, 'tagIds'>,
|
||||
): string[] => (task.tagIds?.length ? task.tagIds : parent?.tagIds) ?? [];
|
||||
|
|
@ -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,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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([
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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<Task>, 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 = (
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
|
|
|
|||
|
|
@ -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 };
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}));
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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<Task>[] = 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<Tag> = {
|
||||
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<Tag> => ({
|
||||
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<Tag> => ({
|
||||
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<Tag> => ({
|
||||
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]);
|
||||
};
|
||||
|
||||
// =============================================================================
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue