mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
* fix(tasks): self-heal orphan tasks to Inbox on navigation (#8780) Tapping a global-search result did nothing for a task with a falsy projectId (an empty-string projectId survives hydration because it passes typia validation), no tags, and not due today. Such a task's id is in no work context's `taskIds` ordering array, so it renders in no list. The previous fix routed these to the Today list, but the task is not shown there unless it is due/overdue today, so navigation still landed on the wrong view with nothing focused. Re-home the orphan into the Inbox via the existing moveToOtherProject op (which assigns projectId and adds the id to the Inbox list), then navigate there so the task actually renders and can be focused. The move reducer no-ops the source removal for an empty/dangling source and reads canonical subtask data from state, so replay is deterministic and concurrent heals converge (LWW projectId=INBOX + unique() taskIds). Also surface the existing error snack when a same-context task can never be focused, instead of silently leaving the user on the wrong view. * fix(tasks): suppress 'moved to project' snack on orphan self-heal (#8780) The #8780 self-heal dispatches moveToOtherProject to re-home an orphan task (falsy projectId, no tag) into the Inbox on navigation. That fired goToProjectSnack$ — a SUCCESS 'Moved task to project Inbox' snack — even though the user only tapped a search result to view the task and is being navigated to it anyway. Guard the snack on the moved task having a real source project, so the silent data-repair path stays silent while genuine user-initiated moves still announce. Tests: - task-ui.effects.spec: cover goToProjectSnack$ (orphan heal = no snack; real move = snack), previously untested. - layout.service.spec: cover the new onFailure retry-exhaustion branch. - navigate-to-task.service.spec: model the real orphan (empty-string projectId, no due date) with an accurate root-cause comment. * refactor(tasks): address multi-review findings for #8780 orphan heal Applied fixes from a three-way review (correctness, sync-correctness, tests): - Command/query separation: `_getLocation` renamed to `_resolveNavTarget` and made pure (returns {location, orphanToHeal}); `navigate()` now owns the synced moveToOtherProject dispatch, so it's an explicit navigation step, not a hidden side effect of computing a URL. Prevents a future display-only caller from silently syncing a move. - Scope the snack suppression to Inbox filing (`!task.projectId && targetProjectId === INBOX_PROJECT.id`) instead of any falsy-source move, so quick-add default-project assignment to a REAL project (short-syntax.effects) keeps its 'Moved to project' wayfinding snack. - Tests: add reducer coverage for the empty-string-projectId heal into Inbox (the exact repair the fix relies on, previously verified nowhere); add navigate cases for same-context heal and for an orphaned subtask whose parent can't load (routed, not moved); add effect case proving a non-Inbox falsy-source move still announces. Specs: navigate 7/7, task-ui.effects 14/14, project-shared.reducer 19/19, layout 13/13. checkFile clean on all touched files. * docs(tasks): note route-change focus delegates to AppComponent (#8780)
This commit is contained in:
parent
6fefd741c5
commit
56ddeafd90
7 changed files with 371 additions and 26 deletions
|
|
@ -1,4 +1,4 @@
|
|||
import { TestBed } from '@angular/core/testing';
|
||||
import { fakeAsync, TestBed, tick } from '@angular/core/testing';
|
||||
import { Store } from '@ngrx/store';
|
||||
import { LayoutService } from './layout.service';
|
||||
import { hideAddTaskBar, showAddTaskBar } from './store/layout.actions';
|
||||
|
|
@ -340,5 +340,20 @@ describe('LayoutService', () => {
|
|||
done();
|
||||
}, 400);
|
||||
});
|
||||
|
||||
it('should invoke onFailure (and not onSuccess) once the retries are exhausted', fakeAsync(() => {
|
||||
const onSuccess = jasmine.createSpy('onSuccess');
|
||||
const onFailure = jasmine.createSpy('onFailure');
|
||||
|
||||
// No element with id `t-never-rendered` exists, so the task never becomes
|
||||
// focusable. Use a small retry budget so the loop exhausts quickly.
|
||||
service.focusTaskInViewWhenReady('never-rendered', onSuccess, onFailure, 2);
|
||||
|
||||
// 2 retries * 250ms delay; tick generously to drain every scheduled retry.
|
||||
tick(1000);
|
||||
|
||||
expect(onSuccess).not.toHaveBeenCalled();
|
||||
expect(onFailure).toHaveBeenCalledTimes(1);
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -168,6 +168,7 @@ export class LayoutService {
|
|||
focusTaskInViewWhenReady(
|
||||
taskId: string,
|
||||
onSuccess?: (el: HTMLElement) => void,
|
||||
onFailure?: () => void,
|
||||
retriesLeft: number = LayoutService._TASK_FOCUS_MAX_RETRIES,
|
||||
): void {
|
||||
if (this._pendingTaskRevealTimeout) {
|
||||
|
|
@ -182,12 +183,13 @@ export class LayoutService {
|
|||
}
|
||||
|
||||
if (retriesLeft <= 0) {
|
||||
onFailure?.();
|
||||
return;
|
||||
}
|
||||
|
||||
this._pendingTaskRevealTimeout = window.setTimeout(() => {
|
||||
this._pendingTaskRevealTimeout = undefined;
|
||||
this.focusTaskInViewWhenReady(taskId, onSuccess, retriesLeft - 1);
|
||||
this.focusTaskInViewWhenReady(taskId, onSuccess, onFailure, retriesLeft - 1);
|
||||
}, LayoutService._TASK_FOCUS_RETRY_DELAY);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { TestBed } from '@angular/core/testing';
|
||||
import { Router } from '@angular/router';
|
||||
import { Store } from '@ngrx/store';
|
||||
import { NavigateToTaskService } from './navigate-to-task.service';
|
||||
import { TaskService } from '../../features/tasks/task.service';
|
||||
import { ProjectService } from '../../features/project/project.service';
|
||||
|
|
@ -8,6 +9,8 @@ import { DateService } from '../../core/date/date.service';
|
|||
import { LayoutService } from '../layout/layout.service';
|
||||
import { Task } from '../../features/tasks/task.model';
|
||||
import { TODAY_TAG } from '../../features/tag/tag.const';
|
||||
import { INBOX_PROJECT } from '../../features/project/project.const';
|
||||
import { TaskSharedActions } from '../../root-store/meta/task-shared.actions';
|
||||
import { of } from 'rxjs';
|
||||
|
||||
const TODAY_STR = '2026-07-06';
|
||||
|
|
@ -31,6 +34,7 @@ describe('NavigateToTaskService', () => {
|
|||
let taskService: jasmine.SpyObj<TaskService>;
|
||||
let router: jasmine.SpyObj<Router> & { url: string };
|
||||
let layoutService: jasmine.SpyObj<LayoutService>;
|
||||
let store: jasmine.SpyObj<Store>;
|
||||
|
||||
beforeEach(() => {
|
||||
taskService = jasmine.createSpyObj('TaskService', [
|
||||
|
|
@ -45,6 +49,7 @@ describe('NavigateToTaskService', () => {
|
|||
dateService.todayStr.and.returnValue(TODAY_STR);
|
||||
dateService.isToday.and.returnValue(false);
|
||||
layoutService = jasmine.createSpyObj('LayoutService', ['focusTaskInViewWhenReady']);
|
||||
store = jasmine.createSpyObj('Store', ['dispatch']);
|
||||
|
||||
const routerSpy = jasmine.createSpyObj('Router', ['navigate']);
|
||||
routerSpy.navigate.and.resolveTo(true);
|
||||
|
|
@ -54,6 +59,7 @@ describe('NavigateToTaskService', () => {
|
|||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
NavigateToTaskService,
|
||||
{ provide: Store, useValue: store },
|
||||
{ provide: TaskService, useValue: taskService },
|
||||
{ provide: ProjectService, useValue: projectService },
|
||||
{ provide: SnackService, useValue: snackService },
|
||||
|
|
@ -65,16 +71,26 @@ describe('NavigateToTaskService', () => {
|
|||
service = TestBed.inject(NavigateToTaskService);
|
||||
});
|
||||
|
||||
it('navigates to the Today list for an overdue task with no project and no tags (#8780)', async () => {
|
||||
// Overdue = dueDay in the past → shown only in the Today "Overdue" section.
|
||||
it('self-heals an orphan task (no project, no tags, not due today) into the Inbox and navigates there (#8780)', async () => {
|
||||
// Empty-string projectId is the real-world case: it survives hydration
|
||||
// (passes typia validation, unlike `undefined`). With no tags and no due
|
||||
// date, the task's id is in no project's or tag's `taskIds` array and it is
|
||||
// not overdue/due-today, so it renders in no reachable list. It must be
|
||||
// re-homed into the Inbox to become focusable.
|
||||
taskService.getByIdFromEverywhere.and.resolveTo(
|
||||
createTask({ id: 't1', dueDay: '2020-01-01', projectId: undefined, tagIds: [] }),
|
||||
createTask({ id: 't1', projectId: '', tagIds: [] }),
|
||||
);
|
||||
|
||||
await service.navigate('t1');
|
||||
|
||||
expect(store.dispatch).toHaveBeenCalledWith(
|
||||
jasmine.objectContaining({
|
||||
type: TaskSharedActions.moveToOtherProject.type,
|
||||
targetProjectId: INBOX_PROJECT.id,
|
||||
}),
|
||||
);
|
||||
expect(router.navigate).toHaveBeenCalledWith(
|
||||
[`/tag/${TODAY_TAG.id}/tasks`],
|
||||
[`/project/${INBOX_PROJECT.id}/tasks`],
|
||||
jasmine.objectContaining({
|
||||
queryParams: jasmine.objectContaining({ focusItem: 't1' }),
|
||||
}),
|
||||
|
|
@ -83,7 +99,7 @@ describe('NavigateToTaskService', () => {
|
|||
expect(layoutService.focusTaskInViewWhenReady).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('navigates to the project list for a project task', async () => {
|
||||
it('navigates to the project list for a project task without re-homing it', async () => {
|
||||
taskService.getByIdFromEverywhere.and.resolveTo(
|
||||
createTask({ id: 't2', projectId: 'p1', tagIds: [] }),
|
||||
);
|
||||
|
|
@ -94,9 +110,10 @@ describe('NavigateToTaskService', () => {
|
|||
['/project/p1/tasks'],
|
||||
jasmine.anything(),
|
||||
);
|
||||
expect(store.dispatch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('navigates to the tag list for a tagged task with no project', async () => {
|
||||
it('navigates to the tag list for a tagged task with no project without re-homing it', async () => {
|
||||
taskService.getByIdFromEverywhere.and.resolveTo(
|
||||
createTask({ id: 't3', projectId: undefined, tagIds: ['tag-a'] }),
|
||||
);
|
||||
|
|
@ -107,5 +124,86 @@ describe('NavigateToTaskService', () => {
|
|||
['/tag/tag-a/tasks'],
|
||||
jasmine.anything(),
|
||||
);
|
||||
expect(store.dispatch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('navigates to the Today list for a task due today (no re-home needed)', async () => {
|
||||
taskService.getByIdFromEverywhere.and.resolveTo(
|
||||
createTask({ id: 't4', projectId: undefined, tagIds: [], dueDay: TODAY_STR }),
|
||||
);
|
||||
|
||||
await service.navigate('t4');
|
||||
|
||||
expect(router.navigate).toHaveBeenCalledWith(
|
||||
[`/tag/${TODAY_TAG.id}/tasks`],
|
||||
jasmine.anything(),
|
||||
);
|
||||
expect(store.dispatch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('surfaces an error snack instead of silently failing when a same-context task cannot be focused', async () => {
|
||||
const snackService = TestBed.inject(SnackService) as jasmine.SpyObj<SnackService>;
|
||||
// Already on the task's context so navigate() takes the same-context branch.
|
||||
router.url = `/project/p1/tasks`;
|
||||
taskService.getByIdFromEverywhere.and.resolveTo(
|
||||
createTask({ id: 't5', projectId: 'p1', tagIds: [] }),
|
||||
);
|
||||
// Simulate focus never succeeding → onFailure invoked.
|
||||
layoutService.focusTaskInViewWhenReady.and.callFake(
|
||||
(_taskId, _onSuccess, onFailure) => onFailure?.(),
|
||||
);
|
||||
|
||||
await service.navigate('t5');
|
||||
|
||||
expect(router.navigate).not.toHaveBeenCalled();
|
||||
expect(snackService.open).toHaveBeenCalledWith(
|
||||
jasmine.objectContaining({ type: 'ERROR' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('heals an orphan and focuses it in place when already on the Inbox (same-context)', async () => {
|
||||
// Already on the Inbox, so navigate() takes the same-context branch — but the
|
||||
// heal must still fire first so the task is added to the Inbox list.
|
||||
router.url = `/project/${INBOX_PROJECT.id}/tasks`;
|
||||
taskService.getByIdFromEverywhere.and.resolveTo(
|
||||
createTask({ id: 't6', projectId: '', tagIds: [] }),
|
||||
);
|
||||
|
||||
await service.navigate('t6');
|
||||
|
||||
expect(store.dispatch).toHaveBeenCalledWith(
|
||||
jasmine.objectContaining({
|
||||
type: TaskSharedActions.moveToOtherProject.type,
|
||||
targetProjectId: INBOX_PROJECT.id,
|
||||
}),
|
||||
);
|
||||
expect(router.navigate).not.toHaveBeenCalled();
|
||||
expect(layoutService.focusTaskInViewWhenReady).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT re-home an orphaned subtask whose parent cannot be loaded (moveToOtherProject is top-level only)', async () => {
|
||||
// The subtask itself resolves; its parent lookup returns undefined, so
|
||||
// `taskToCheck` stays the subtask. It routes to the Inbox but must NOT be
|
||||
// dispatched as a top-level move (which would corrupt the parent/child link).
|
||||
taskService.getByIdFromEverywhere.and.callFake((id: string) =>
|
||||
Promise.resolve(
|
||||
id === 'sub-1'
|
||||
? createTask({
|
||||
id: 'sub-1',
|
||||
parentId: 'missing-parent',
|
||||
projectId: '',
|
||||
tagIds: [],
|
||||
})
|
||||
: (undefined as unknown as Task),
|
||||
),
|
||||
);
|
||||
|
||||
await service.navigate('sub-1');
|
||||
|
||||
expect(store.dispatch).not.toHaveBeenCalled();
|
||||
expect(router.navigate).toHaveBeenCalledWith(
|
||||
[`/project/${INBOX_PROJECT.id}/tasks`],
|
||||
jasmine.anything(),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,11 +1,14 @@
|
|||
import { inject, Injectable } from '@angular/core';
|
||||
import { Store } from '@ngrx/store';
|
||||
import { SearchQueryParams } from '../../pages/search-page/search-page.model';
|
||||
import { first } from 'rxjs/operators';
|
||||
import { devError } from '../../util/dev-error';
|
||||
import { TaskService } from '../../features/tasks/task.service';
|
||||
import { ProjectService } from '../../features/project/project.service';
|
||||
import { Router } from '@angular/router';
|
||||
import { Task } from '../../features/tasks/task.model';
|
||||
import { Task, TaskWithSubTasks } from '../../features/tasks/task.model';
|
||||
import { TaskSharedActions } from '../../root-store/meta/task-shared.actions';
|
||||
import { INBOX_PROJECT } from '../../features/project/project.const';
|
||||
import { getDbDateStr } from '../../util/get-db-date-str';
|
||||
import { DateService } from '../../core/date/date.service';
|
||||
import { TODAY_TAG } from '../../features/tag/tag.const';
|
||||
|
|
@ -19,6 +22,7 @@ import { recordSearchNavDebug } from '../../util/search-nav-debug';
|
|||
providedIn: 'root',
|
||||
})
|
||||
export class NavigateToTaskService {
|
||||
private _store = inject(Store);
|
||||
private _taskService = inject(TaskService);
|
||||
private _projectService = inject(ProjectService);
|
||||
private _router = inject(Router);
|
||||
|
|
@ -32,12 +36,22 @@ export class NavigateToTaskService {
|
|||
if (!task) {
|
||||
throw new Error(`Task with id ${taskId} not found`);
|
||||
}
|
||||
const location = await this._getLocation(task, isArchiveTask);
|
||||
const { location, orphanToHeal } = await this._resolveNavTarget(
|
||||
task,
|
||||
isArchiveTask,
|
||||
);
|
||||
if (!location) {
|
||||
// Never fall through with an empty location: `''.startsWith` would make
|
||||
// the same-context check below always true and swallow the navigation.
|
||||
throw new Error(`Could not resolve a location for task ${taskId}`);
|
||||
}
|
||||
// Perform the orphan self-heal here (not inside the resolver) so the
|
||||
// synced state mutation is an explicit navigation step, not a hidden side
|
||||
// effect of computing a URL. Must run before the same-context check below
|
||||
// so the task is added to the Inbox list in either branch. (#8780)
|
||||
if (orphanToHeal) {
|
||||
this._healOrphanTaskToInbox(orphanToHeal);
|
||||
}
|
||||
recordSearchNavDebug('navigateToTask:start', {
|
||||
taskId,
|
||||
isArchiveTask,
|
||||
|
|
@ -58,6 +72,11 @@ export class NavigateToTaskService {
|
|||
return;
|
||||
}
|
||||
|
||||
// Route-change path: focus is handed off to the destination view via the
|
||||
// `focusItem` query param (AppComponent), which owns its own reveal/retry.
|
||||
// The explicit onFailure error snack is only wired to the same-context
|
||||
// branch above; here a heal always adds the task to the Inbox main list, so
|
||||
// it renders and focuses normally.
|
||||
const queryParams: SearchQueryParams = { focusItem: taskId };
|
||||
if (isArchiveTask) {
|
||||
queryParams.dateStr = await this._getArchivedDate(task);
|
||||
|
|
@ -77,14 +96,20 @@ export class NavigateToTaskService {
|
|||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
Log.err(err);
|
||||
this._snackService.open({
|
||||
type: 'ERROR',
|
||||
msg: T.GLOBAL_SNACK.NAVIGATE_TO_TASK_ERR,
|
||||
});
|
||||
this._showNavErrorSnack();
|
||||
}
|
||||
}
|
||||
|
||||
private async _getLocation(task: Task, isArchiveTask: boolean): Promise<string> {
|
||||
/**
|
||||
* Pure resolver: computes the navigation location and, for an orphan task,
|
||||
* returns the top-level task that must be re-homed into the Inbox — WITHOUT
|
||||
* mutating state. The caller (`navigate`) performs the heal, keeping this a
|
||||
* side-effect-free "where does this task live?" query. (#8780)
|
||||
*/
|
||||
private async _resolveNavTarget(
|
||||
task: Task,
|
||||
isArchiveTask: boolean,
|
||||
): Promise<{ location: string; orphanToHeal: Task | null }> {
|
||||
const tasksOrWorklog = isArchiveTask ? 'history' : 'tasks';
|
||||
|
||||
let taskToCheck = task;
|
||||
|
|
@ -99,25 +124,66 @@ export class NavigateToTaskService {
|
|||
}
|
||||
|
||||
if (!isArchiveTask && this._isDueToday(taskToCheck)) {
|
||||
return `/tag/${TODAY_TAG.id}/${tasksOrWorklog}`;
|
||||
return { location: `/tag/${TODAY_TAG.id}/${tasksOrWorklog}`, orphanToHeal: null };
|
||||
}
|
||||
|
||||
if (taskToCheck.projectId) {
|
||||
return `/project/${taskToCheck.projectId}/${tasksOrWorklog}`;
|
||||
return {
|
||||
location: `/project/${taskToCheck.projectId}/${tasksOrWorklog}`,
|
||||
orphanToHeal: null,
|
||||
};
|
||||
} else if (taskToCheck.tagIds?.length > 0 && taskToCheck.tagIds[0]) {
|
||||
return `/tag/${taskToCheck.tagIds[0]}/${tasksOrWorklog}`;
|
||||
return {
|
||||
location: `/tag/${taskToCheck.tagIds[0]}/${tasksOrWorklog}`,
|
||||
orphanToHeal: null,
|
||||
};
|
||||
} else if (!isArchiveTask) {
|
||||
// A non-archived task with neither project nor tags only ever lives in the
|
||||
// Today list — either due today (handled above) or overdue. Route there so
|
||||
// navigation reveals it instead of resolving to '' and silently no-op'ing
|
||||
// (an empty location makes `url.startsWith(location)` always true). (#8780)
|
||||
return `/tag/${TODAY_TAG.id}/${tasksOrWorklog}`;
|
||||
// No project, no tag, and not due today: the task's id is in no work
|
||||
// context's `taskIds` ordering array, so it renders in no list view
|
||||
// (routing to Today only reveals tasks due or overdue *today*). It must be
|
||||
// self-healed into the Inbox — assigning its projectId and adding it to the
|
||||
// Inbox list — so navigation can actually reveal and focus it. moveToOther-
|
||||
// Project operates on a top-level task, so an orphaned subtask whose parent
|
||||
// could not be loaded (still has `parentId`) is routed but not healed. (#8780)
|
||||
return {
|
||||
location: `/project/${INBOX_PROJECT.id}/${tasksOrWorklog}`,
|
||||
orphanToHeal: taskToCheck.parentId ? null : taskToCheck,
|
||||
};
|
||||
} else {
|
||||
devError("Couldn't find task location");
|
||||
return '';
|
||||
return { location: '', orphanToHeal: null };
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Assign a project-less, tag-less task to the Inbox so it lives in a real list
|
||||
* and can be revealed. The move reducer only strips the task from its source
|
||||
* project when that project exists, so an empty or dangling projectId is
|
||||
* handled gracefully, and it reads canonical subtask data from the store, so
|
||||
* passing an empty `subTasks` here is safe. (#8780)
|
||||
*/
|
||||
private _healOrphanTaskToInbox(task: Task): void {
|
||||
// Defense-in-depth: `moveToOtherProject` operates on a top-level task, so
|
||||
// never move a subtask as if it were a parent (the resolver already returns
|
||||
// `null` for subtasks, so this only guards against future misuse).
|
||||
if (task.parentId) {
|
||||
return;
|
||||
}
|
||||
this._store.dispatch(
|
||||
TaskSharedActions.moveToOtherProject({
|
||||
task: { ...task, subTasks: [] } as TaskWithSubTasks,
|
||||
targetProjectId: INBOX_PROJECT.id,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
private _showNavErrorSnack(): void {
|
||||
this._snackService.open({
|
||||
type: 'ERROR',
|
||||
msg: T.GLOBAL_SNACK.NAVIGATE_TO_TASK_ERR,
|
||||
});
|
||||
}
|
||||
|
||||
private _isDueToday(task: Task): boolean {
|
||||
if (task.dueWithTime) {
|
||||
return this._dateService.isToday(task.dueWithTime);
|
||||
|
|
@ -126,7 +192,12 @@ export class NavigateToTaskService {
|
|||
}
|
||||
|
||||
private _focusTaskElement(taskId: string): void {
|
||||
this._layoutService.focusTaskInViewWhenReady(taskId);
|
||||
// Never swallow silently: if the task never becomes focusable in the current
|
||||
// context, surface the error instead of leaving the user on the wrong view.
|
||||
this._layoutService.focusTaskInViewWhenReady(taskId, undefined, () => {
|
||||
recordSearchNavDebug('navigateToTask:focusFailed', { taskId });
|
||||
this._showNavErrorSnack();
|
||||
});
|
||||
}
|
||||
|
||||
private async _isInBacklog(task: Task): Promise<boolean> {
|
||||
|
|
|
|||
|
|
@ -376,6 +376,113 @@ describe('TaskUiEffects', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('goToProjectSnack$', () => {
|
||||
const createMockTaskWithSubTasks = (
|
||||
overrides: Partial<TaskWithSubTasks> = {},
|
||||
): TaskWithSubTasks => ({
|
||||
...createMockTask(),
|
||||
subTasks: [],
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const setup = (): void => {
|
||||
actions$ = new Subject<Action>();
|
||||
snackServiceMock = jasmine.createSpyObj('SnackService', ['open']);
|
||||
taskServiceMock = jasmine.createSpyObj('TaskService', ['setSelectedId']);
|
||||
navigateToTaskServiceMock = jasmine.createSpyObj('NavigateToTaskService', [
|
||||
'navigate',
|
||||
]);
|
||||
layoutServiceMock = jasmine.createSpyObj('LayoutService', ['hideAddTaskBar']);
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
TaskUiEffects,
|
||||
{ provide: LOCAL_ACTIONS, useValue: actions$ },
|
||||
provideMockStore({
|
||||
initialState: {},
|
||||
selectors: [
|
||||
{
|
||||
selector: selectProjectById,
|
||||
value: { id: 'INBOX_PROJECT', title: 'Inbox' },
|
||||
},
|
||||
],
|
||||
}),
|
||||
{ provide: SnackService, useValue: snackServiceMock },
|
||||
{ provide: TaskService, useValue: taskServiceMock },
|
||||
{ provide: NavigateToTaskService, useValue: navigateToTaskServiceMock },
|
||||
{ provide: LayoutService, useValue: layoutServiceMock },
|
||||
{
|
||||
provide: WorkContextService,
|
||||
// Active context is a project the moved task is not in, so only the
|
||||
// orphan-source guard decides whether the snack fires.
|
||||
useValue: {
|
||||
activeWorkContextId: 'some-other-project',
|
||||
mainListTaskIds$: of([]),
|
||||
},
|
||||
},
|
||||
{
|
||||
provide: NotifyService,
|
||||
useValue: jasmine.createSpyObj('NotifyService', ['notify']),
|
||||
},
|
||||
{
|
||||
provide: BannerService,
|
||||
useValue: jasmine.createSpyObj('BannerService', ['open', 'dismiss']),
|
||||
},
|
||||
{
|
||||
provide: GlobalConfigService,
|
||||
useValue: { sound$: of({ doneSound: null }) },
|
||||
},
|
||||
{ provide: Router, useValue: jasmine.createSpyObj('Router', ['navigate']) },
|
||||
],
|
||||
});
|
||||
|
||||
effects = TestBed.inject(TaskUiEffects);
|
||||
};
|
||||
|
||||
beforeEach(setup);
|
||||
|
||||
it('should NOT show the "moved to project" snack when self-healing an orphan (no source project) into the Inbox (#8780)', () => {
|
||||
const sub = effects.goToProjectSnack$.subscribe();
|
||||
actions$.next(
|
||||
TaskSharedActions.moveToOtherProject({
|
||||
task: createMockTaskWithSubTasks({ id: 'orphan-1', projectId: '' }),
|
||||
targetProjectId: 'INBOX_PROJECT',
|
||||
}),
|
||||
);
|
||||
|
||||
expect(snackServiceMock.open).not.toHaveBeenCalled();
|
||||
sub.unsubscribe();
|
||||
});
|
||||
|
||||
it('should show the "moved to project" snack for a real move of a task that had a source project into the Inbox', () => {
|
||||
const sub = effects.goToProjectSnack$.subscribe();
|
||||
actions$.next(
|
||||
TaskSharedActions.moveToOtherProject({
|
||||
task: createMockTaskWithSubTasks({ id: 'real-1', projectId: 'project-a' }),
|
||||
targetProjectId: 'INBOX_PROJECT',
|
||||
}),
|
||||
);
|
||||
|
||||
expect(snackServiceMock.open).toHaveBeenCalled();
|
||||
const snackParams = snackServiceMock.open.calls.mostRecent().args[0] as SnackParams;
|
||||
expect(snackParams.msg).toBe(T.F.TASK.S.MOVED_TO_PROJECT);
|
||||
sub.unsubscribe();
|
||||
});
|
||||
|
||||
it('should still show the snack for a project-less task assigned to a NON-Inbox project (quick-add default-project), since the suppression is scoped to Inbox filing', () => {
|
||||
const sub = effects.goToProjectSnack$.subscribe();
|
||||
actions$.next(
|
||||
TaskSharedActions.moveToOtherProject({
|
||||
task: createMockTaskWithSubTasks({ id: 'quick-1', projectId: '' }),
|
||||
targetProjectId: 'default-project',
|
||||
}),
|
||||
);
|
||||
|
||||
expect(snackServiceMock.open).toHaveBeenCalled();
|
||||
sub.unsubscribe();
|
||||
});
|
||||
});
|
||||
|
||||
describe('deadlineTodayBanner$', () => {
|
||||
let store: MockStore;
|
||||
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ import { Task } from '../task.model';
|
|||
import { EMPTY } from 'rxjs';
|
||||
import { selectProjectById } from '../../project/store/project.selectors';
|
||||
import { Project } from '../../project/project.model';
|
||||
import { INBOX_PROJECT } from '../../project/project.const';
|
||||
import { Router } from '@angular/router';
|
||||
import { NavigateToTaskService } from '../../../core-ui/navigate-to-task/navigate-to-task.service';
|
||||
import { LayoutService } from '../../../core-ui/layout/layout.service';
|
||||
|
|
@ -221,7 +222,14 @@ export class TaskUiEffects {
|
|||
this._actions$.pipe(
|
||||
ofType(TaskSharedActions.moveToOtherProject),
|
||||
filter(
|
||||
({ targetProjectId }) =>
|
||||
({ task, targetProjectId }) =>
|
||||
// Don't announce *filing a project-less task into the Inbox*: that is
|
||||
// never a user-initiated move. It is either the #8780 orphan self-heal
|
||||
// (a silent, navigation-triggered repair — the user is navigated there
|
||||
// anyway) or the quick-add default-project assignment when the default
|
||||
// IS the Inbox (short-syntax.effects). Genuine moves of a real task
|
||||
// into the Inbox still have a source project, so they still announce.
|
||||
!(!task.projectId && targetProjectId === INBOX_PROJECT.id) &&
|
||||
targetProjectId !== this._workContextService.activeWorkContextId,
|
||||
),
|
||||
withLatestFrom(this._workContextService.mainListTaskIds$),
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import {
|
|||
expectTagUpdates,
|
||||
} from './test-utils';
|
||||
import { TASK_REPEAT_CFG_FEATURE_NAME } from '../../../features/task-repeat-cfg/store/task-repeat-cfg.selectors';
|
||||
import { INBOX_PROJECT } from '../../../features/project/project.const';
|
||||
import {
|
||||
DEFAULT_TASK_REPEAT_CFG,
|
||||
TaskRepeatCfg,
|
||||
|
|
@ -103,6 +104,49 @@ describe('projectSharedMetaReducer', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should heal an orphan task (empty-string source projectId) into the Inbox without throwing (#8780)', () => {
|
||||
// This is the exact repair the #8780 navigation self-heal relies on: an
|
||||
// orphan with a falsy `projectId` whose source "project" does not exist.
|
||||
const testState = createBaseState();
|
||||
testState[PROJECT_FEATURE_NAME].entities[INBOX_PROJECT.id] = createMockProject({
|
||||
id: INBOX_PROJECT.id,
|
||||
title: 'Inbox',
|
||||
taskIds: [],
|
||||
backlogTaskIds: [],
|
||||
});
|
||||
(testState[PROJECT_FEATURE_NAME].ids as string[]) = [
|
||||
...(testState[PROJECT_FEATURE_NAME].ids as string[]),
|
||||
INBOX_PROJECT.id,
|
||||
];
|
||||
testState[TASK_FEATURE_NAME].entities.orphan1 = createMockTask({
|
||||
id: 'orphan1',
|
||||
projectId: '',
|
||||
tagIds: [],
|
||||
subTaskIds: [],
|
||||
});
|
||||
(testState[TASK_FEATURE_NAME].ids as string[]) = ['orphan1'];
|
||||
|
||||
const payload: TaskWithSubTasks = {
|
||||
...createMockTask({ id: 'orphan1', projectId: '', subTaskIds: [] }),
|
||||
subTasks: [],
|
||||
};
|
||||
const action = createMoveToOtherProjectAction(payload, INBOX_PROJECT.id);
|
||||
|
||||
expect(() => metaReducer(testState, action)).not.toThrow();
|
||||
const updatedState = mockReducer.calls.mostRecent().args[0];
|
||||
|
||||
// (a) the orphan is now owned by the Inbox
|
||||
expect(updatedState[TASK_FEATURE_NAME].entities.orphan1.projectId).toBe(
|
||||
INBOX_PROJECT.id,
|
||||
);
|
||||
// (b) and added to the Inbox's ordering array so it renders
|
||||
expect(
|
||||
updatedState[PROJECT_FEATURE_NAME].entities[INBOX_PROJECT.id].taskIds,
|
||||
).toContain('orphan1');
|
||||
// (c) the (non-existent) empty source project caused no source-strip / crash
|
||||
expect(updatedState[PROJECT_FEATURE_NAME].entities.project1.taskIds).toEqual([]);
|
||||
});
|
||||
|
||||
it('should remove task from actual source project even if payload is stale', () => {
|
||||
const testState = createStateWithExistingTasks(['task1'], [], []);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue