diff --git a/e2e/pages/task.page.ts b/e2e/pages/task.page.ts index aabe7b690c..917a9a4c17 100644 --- a/e2e/pages/task.page.ts +++ b/e2e/pages/task.page.ts @@ -53,10 +53,14 @@ export class TaskPage extends BasePage { */ async markTaskAsDone(task: Locator): Promise { await task.waitFor({ state: 'visible' }); - // `data-task-id` is only bound on the host. Wrapper components such - // as (Boards) render the same done-toggle but expose no id, - // so the done-confirmation strategy is chosen based on its presence. - const taskId = await task.getAttribute('data-task-id'); + // The done-confirmation strategy differs for real rows vs. wrapper + // components ( etc.) that render the same done-toggle. Key it + // off the element actually being a , not off `data-task-id` presence: + // also carries `data-task-id` in the Planner overdue list + // (#8851), but querying `document.querySelectorAll('task')` for it finds + // nothing, so the strategy would hang for a wrapper. + const isTaskHost = (await task.evaluate((el) => el.tagName.toLowerCase())) === 'task'; + const taskId = isTaskHost ? await task.getAttribute('data-task-id') : null; await task.hover(); // Give hover effects time to settle diff --git a/src/app/features/planner/planner-day-overdue/planner-day-overdue.component.html b/src/app/features/planner/planner-day-overdue/planner-day-overdue.component.html index 10b54390b3..e767ecf67e 100644 --- a/src/app/features/planner/planner-day-overdue/planner-day-overdue.component.html +++ b/src/app/features/planner/planner-day-overdue/planner-day-overdue.component.html @@ -27,6 +27,7 @@ [cdkDragLockAxis]="isXs() ? 'y' : ''" [task]="task" [day]="" + [focusable]="true" > } diff --git a/src/app/features/planner/planner-task/planner-task.component.ts b/src/app/features/planner/planner-task/planner-task.component.ts index 685191763d..9806ac5a37 100644 --- a/src/app/features/planner/planner-task/planner-task.component.ts +++ b/src/app/features/planner/planner-task/planner-task.component.ts @@ -50,6 +50,13 @@ import { TranslatePipe } from '@ngx-translate/core'; ], /* eslint-disable @typescript-eslint/naming-convention */ host: { + // data-task-id + tabindex only where the id-based schedule-today shortcut + // needs them (the Planner overdue list, #8851). Elsewhere data-task-id + // must stay absent: the e2e page object (e2e/pages/task.page.ts) picks its + // done-confirmation strategy based on its presence, and tabindex would add + // a Tab stop to every planner-day/scheduled card board-wide. + '[attr.data-task-id]': 'focusable() ? task().id : null', + '[attr.tabindex]': 'focusable() ? "0" : null', '[class.isDone]': 'task().isDone', '[class.isDragReady]': 'isDragReady()', '[class.isCurrent]': 'isCurrent()', @@ -72,6 +79,9 @@ export class PlannerTaskComponent implements OnInit, OnDestroy, AfterViewInit { // TODO remove readonly day = input(); readonly tagsToHide = input(); + // Opt-in DOM focusability (only the Planner overdue list needs it, for the + // schedule-today shortcut). Off everywhere else to avoid stray Tab stops. + readonly focusable = input(false); readonly T = T; readonly isTouchActive = isTouchActive; diff --git a/src/app/features/tasks/store/task.selectors.ts b/src/app/features/tasks/store/task.selectors.ts index 6db7b64c56..4db7a89f2a 100644 --- a/src/app/features/tasks/store/task.selectors.ts +++ b/src/app/features/tasks/store/task.selectors.ts @@ -22,6 +22,10 @@ import { import { dateStrToUtcDate } from '../../../util/date-str-to-utc-date'; import { isTodayWithOffset } from '../../../util/is-today.util'; import { getTimeConflictTaskIds } from '../util/get-time-conflict-task-ids'; +import { + getLogicalTodayStartMs, + isTaskOverdueByThreshold, +} from '../util/is-task-overdue'; export const isCalendarIssueTask = (task: Task | undefined): task is Task => !!task && @@ -413,24 +417,18 @@ export const selectAllTasksWithSubTasks = createSelector( ); // selectOverdueTasks (rebased): decision reads only dueDay/dueWithTime from the -// snapshot; public re-maps ids to live Task refs. +// snapshot; public re-maps ids to live Task refs. Shares the overdue comparison +// with the isTaskOverdue util (Shift+T path) via isTaskOverdueByThreshold so the +// two definitions cannot drift; the threshold is computed once per recompute. export const selectOverdueTaskIds = createSelector( selectTaskSchedulingSnapshot, selectTodayStr, selectStartOfNextDayDiffMs, (snapshot, todayStr, startOfNextDayDiffMs): string[] => { - const today = dateStrToUtcDate(todayStr); - today.setHours(0, 0, 0, 0); - // The logical start of "today" is shifted by the offset - const todayStartMs = today.getTime() + startOfNextDayDiffMs; + const todayStartMs = getLogicalTodayStartMs(todayStr, startOfNextDayDiffMs); const ids: string[] = []; for (const snap of snapshot) { - // Note: String comparison works correctly here because dueDay is in YYYY-MM-DD format - // which is lexicographically sortable. This avoids timezone conversion issues. - if ( - (snap.dueDay && isDBDateStr(snap.dueDay) && snap.dueDay < todayStr) || - (snap.dueWithTime && snap.dueWithTime < todayStartMs) - ) { + if (isTaskOverdueByThreshold(snap, todayStr, todayStartMs)) { ids.push(snap.id); } } diff --git a/src/app/features/tasks/task-shortcut.service.spec.ts b/src/app/features/tasks/task-shortcut.service.spec.ts index db39a8c00d..e94c5c0255 100644 --- a/src/app/features/tasks/task-shortcut.service.spec.ts +++ b/src/app/features/tasks/task-shortcut.service.spec.ts @@ -74,6 +74,7 @@ describe('TaskShortcutService', () => { currentTaskId: signal(null), setCurrentId: jasmine.createSpy('setCurrentId'), toggleStartTask: jasmine.createSpy('toggleStartTask'), + scheduleForTodayById: jasmine.createSpy('scheduleForTodayById'), } as any; mockConfigService = { @@ -100,6 +101,41 @@ describe('TaskShortcutService', () => { service = TestBed.inject(TaskShortcutService); }); + // The shortcut handler now treats the DOM as authoritative for task focus + // (#8851): a task shortcut only fires when document.activeElement is inside + // the matching focusedTaskId. Helper stubs both so "a task is focused" + // tests reflect real focus. activeElement is stubbed directly rather than via + // el.focus() — headless Chrome only updates activeElement when the test iframe + // has window focus, which is not guaranteed inside a large suite. + let focusedTaskEl: HTMLElement | null = null; + let activeElementStubbed = false; + + const stubActiveElement = (el: Element | null): void => { + Object.defineProperty(document, 'activeElement', { + configurable: true, + get: () => el, + }); + activeElementStubbed = true; + }; + + const setFocusedTask = (id: string): HTMLElement => { + focusedTaskEl = document.createElement('task'); + focusedTaskEl.setAttribute('data-task-id', id); + document.body.appendChild(focusedTaskEl); + stubActiveElement(focusedTaskEl); + mockTaskFocusService.focusedTaskId.set(id); + return focusedTaskEl; + }; + + afterEach(() => { + focusedTaskEl?.remove(); + focusedTaskEl = null; + if (activeElementStubbed) { + delete (document as unknown as { activeElement?: unknown }).activeElement; + activeElementStubbed = false; + } + }); + describe('handleTaskShortcuts - togglePlay (Y key)', () => { describe('when focused task exists', () => { it('should delegate to focused task component togglePlayPause method', () => { @@ -109,7 +145,7 @@ describe('TaskShortcutService', () => { togglePlayPause: jasmine.createSpy('togglePlayPause'), taskContextMenu: () => undefined, // No context menu open }; - mockTaskFocusService.focusedTaskId.set('focused-task-1'); + setFocusedTask('focused-task-1'); mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); const event = createKeyboardEvent('Y'); @@ -255,7 +291,7 @@ describe('TaskShortcutService', () => { togglePlayPause: jasmine.createSpy('togglePlayPause'), taskContextMenu: () => undefined, // No context menu open }; - mockTaskFocusService.focusedTaskId.set('focused-task-1'); + setFocusedTask('focused-task-1'); mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); mockTaskService.selectedTaskId.set('selected-task-2'); // Different task selected @@ -280,7 +316,7 @@ describe('TaskShortcutService', () => { openNotesPanel: jasmine.createSpy('openNotesPanel'), taskContextMenu: () => undefined, }; - mockTaskFocusService.focusedTaskId.set('focused-task-1'); + setFocusedTask('focused-task-1'); mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); const event = createKeyboardEvent('N'); @@ -299,7 +335,7 @@ describe('TaskShortcutService', () => { openDeadlineDialog: jasmine.createSpy('openDeadlineDialog'), taskContextMenu: () => undefined, }; - mockTaskFocusService.focusedTaskId.set('focused-task-1'); + setFocusedTask('focused-task-1'); mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); const event = createKeyboardEvent('S', 'KeyS', { shiftKey: true }); @@ -318,7 +354,7 @@ describe('TaskShortcutService', () => { openContextMenu: jasmine.createSpy('openContextMenu'), taskContextMenu: () => undefined, }; - mockTaskFocusService.focusedTaskId.set('focused-task-1'); + setFocusedTask('focused-task-1'); mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); const event = createKeyboardEvent('ContextMenu', 'ContextMenu'); @@ -337,7 +373,7 @@ describe('TaskShortcutService', () => { openContextMenu: jasmine.createSpy('openContextMenu'), taskContextMenu: () => undefined, }; - mockTaskFocusService.focusedTaskId.set('focused-task-1'); + setFocusedTask('focused-task-1'); mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); const event = createKeyboardEvent('Unidentified', 'ContextMenu'); @@ -356,7 +392,7 @@ describe('TaskShortcutService', () => { openContextMenu: jasmine.createSpy('openContextMenu'), taskContextMenu: () => undefined, }; - mockTaskFocusService.focusedTaskId.set('focused-task-1'); + setFocusedTask('focused-task-1'); mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); const event = createKeyboardEvent('ContextMenu', 'ContextMenu', { @@ -403,7 +439,7 @@ describe('TaskShortcutService', () => { configurable: true, value: { writeText }, }); - mockTaskFocusService.focusedTaskId.set('focused-task-1'); + setFocusedTask('focused-task-1'); mockTaskFocusService.lastFocusedTaskComponent.set({ task: () => ({ id: 'focused-task-1', title: 'Task title to copy' }), taskContextMenu: () => undefined, @@ -496,22 +532,10 @@ describe('TaskShortcutService', () => { * test iframe has window focus, which is not guaranteed inside a * large suite (other tests can steal/drop focus). */ + // Reuses the outer stubActiveElement helper (and its afterEach cleanup). let taskEl: HTMLElement; - let activeElementStubbed = false; - - const stubActiveElement = (el: Element | null): void => { - Object.defineProperty(document, 'activeElement', { - configurable: true, - get: () => el, - }); - activeElementStubbed = true; - }; afterEach(() => { - if (activeElementStubbed) { - delete (document as unknown as { activeElement?: unknown }).activeElement; - activeElementStubbed = false; - } taskEl?.remove(); }); @@ -604,4 +628,141 @@ describe('TaskShortcutService', () => { expect(mockTaskComponent.toggleDoneKeyboard).toHaveBeenCalled(); }); }); + + describe('stale-focus guard (#8851)', () => { + let taskEl: HTMLElement; + + afterEach(() => { + taskEl?.remove(); + }); + + it('drops a task shortcut when focus has left all elements', () => { + // focusedTaskId still points at a task, but the DOM contradicts it: + // the active element is , i.e. focus left every (e.g. after + // navigating to a view with no live ). The shortcut must not fire. + stubActiveElement(document.body); + const mockTaskComponent = { + task: () => ({ id: 'stale-task' }), + toggleDoneKeyboard: jasmine.createSpy('toggleDoneKeyboard'), + taskContextMenu: () => undefined, + }; + mockTaskFocusService.focusedTaskId.set('stale-task'); + mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); + + const result = service.handleTaskShortcuts(createKeyboardEvent('D')); + + expect(result).toBe(false); + expect(mockTaskComponent.toggleDoneKeyboard).not.toHaveBeenCalled(); + }); + + it('uses the containing focus over a mismatched focusedTaskId', () => { + // Active element is inside a different than focusedTaskId claims — + // the DOM wins, so delegation targets the DOM task, not the stale id. + taskEl = document.createElement('task'); + taskEl.setAttribute('data-task-id', 'dom-task'); + document.body.appendChild(taskEl); + stubActiveElement(taskEl); + + const mockTaskComponent = { + task: () => ({ id: 'dom-task' }), + toggleDoneKeyboard: jasmine.createSpy('toggleDoneKeyboard'), + taskContextMenu: () => undefined, + }; + mockTaskFocusService.focusedTaskId.set('stale-task'); + mockTaskFocusService.lastFocusedTaskComponent.set(mockTaskComponent); + + const result = service.handleTaskShortcuts(createKeyboardEvent('D')); + + expect(result).toBe(true); + expect(mockTaskComponent.toggleDoneKeyboard).toHaveBeenCalled(); + }); + }); + + describe('schedule-today shortcut (#8851)', () => { + let hostEl: HTMLElement; + + afterEach(() => { + hostEl?.remove(); + }); + + it('delegates to the focused component (preserves overdue/backlog branching)', () => { + const taskComponent = { + task: () => ({ id: 'focused-task-1' }), + moveToTodayWithFocus: jasmine.createSpy('moveToTodayWithFocus'), + taskContextMenu: () => undefined, + }; + setFocusedTask('focused-task-1'); + mockTaskFocusService.lastFocusedTaskComponent.set(taskComponent); + + const event = createKeyboardEvent('F'); + spyOn(event, 'preventDefault'); + + const result = service.handleTaskShortcuts(event); + + expect(result).toBe(true); + expect(taskComponent.moveToTodayWithFocus).toHaveBeenCalled(); + expect(mockTaskService.scheduleForTodayById).not.toHaveBeenCalled(); + }); + + it('schedules by id from a focused without a live ', () => { + // The Planner overdue list renders , which is not a + // TaskComponent and never registers focus. It carries data-task-id and is + // focusable, so Shift+T resolves the id from the DOM and dispatches + // planTasksForToday by id instead of delegating to a stale . + hostEl = document.createElement('planner-task'); + hostEl.setAttribute('data-task-id', 'overdue-planner-task'); + document.body.appendChild(hostEl); + stubActiveElement(hostEl); + mockTaskFocusService.focusedTaskId.set(null); + + const event = createKeyboardEvent('F'); + spyOn(event, 'preventDefault'); + + const result = service.handleTaskShortcuts(event); + + expect(result).toBe(true); + expect(event.preventDefault).toHaveBeenCalled(); + expect(mockTaskService.scheduleForTodayById).toHaveBeenCalledWith( + 'overdue-planner-task', + ); + }); + + it('does nothing when no task can be resolved from focus', () => { + stubActiveElement(document.body); + mockTaskFocusService.focusedTaskId.set(null); + + const result = service.handleTaskShortcuts(createKeyboardEvent('F')); + + expect(result).toBe(false); + expect(mockTaskService.scheduleForTodayById).not.toHaveBeenCalled(); + }); + + it('schedules the focused planner-task, not a stale focusedTaskId (literal #8851 repro)', () => { + // The exact reported shape: focusedTaskId still points at a from a + // previously-visited view, while a in the overdue list + // actually holds DOM focus. Shift+T must act on the planner task's id and + // never touch the stale component (which produced the stray sync write). + hostEl = document.createElement('planner-task'); + hostEl.setAttribute('data-task-id', 'overdue-planner-task'); + document.body.appendChild(hostEl); + stubActiveElement(hostEl); + + const staleComponent = { + task: () => ({ id: 'stale-task-elsewhere' }), + moveToTodayWithFocus: jasmine.createSpy('moveToTodayWithFocus'), + taskContextMenu: () => undefined, + }; + mockTaskFocusService.focusedTaskId.set('stale-task-elsewhere'); + mockTaskFocusService.lastFocusedTaskComponent.set(staleComponent); + + const result = service.handleTaskShortcuts(createKeyboardEvent('F')); + + expect(result).toBe(true); + expect(mockTaskService.scheduleForTodayById).toHaveBeenCalledWith( + 'overdue-planner-task', + ); + expect(mockTaskService.scheduleForTodayById).toHaveBeenCalledTimes(1); + expect(staleComponent.moveToTodayWithFocus).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/app/features/tasks/task-shortcut.service.ts b/src/app/features/tasks/task-shortcut.service.ts index 1e305314eb..1476a58737 100644 --- a/src/app/features/tasks/task-shortcut.service.ts +++ b/src/app/features/tasks/task-shortcut.service.ts @@ -63,17 +63,48 @@ export class TaskShortcutService { const keys = cfg.keyboard; let focusedTaskId: TaskId | null = this._taskFocusService.focusedTaskId(); - // Focus-tracking recovery: a `focusout` can clear focusedTaskId without a - // following `focusin` rebinding it (e.g. when focus stays on the task host - // after an inline-edit blur, the host's `.focus()` becomes a no-op and no - // new focusin fires). If the active element is still inside a , - // derive the id from its data-task-id so shortcuts don't silently drop. - if (!focusedTaskId) { - const active = document.activeElement as HTMLElement | null; - const taskEl = active?.closest('task') as HTMLElement | null; - const recoveredId = taskEl?.getAttribute('data-task-id'); - if (recoveredId) { - focusedTaskId = recoveredId; + // Make the DOM authoritative for task focus (#8851). Two problems this + // solves: + // 1. Focus-tracking recovery: a `focusout` can clear focusedTaskId without + // a following `focusin` rebinding it (e.g. focus staying on the task + // host after an inline-edit blur, where `.focus()` is a no-op and no new + // focusin fires). If the active element is still inside a , we + // recover the id so shortcuts don't silently drop. + // 2. Stale-focus guard: navigating to a view with no live (e.g. the + // Planner overdue list) leaves focusedTaskId pointing at a that + // no longer holds focus. Acting on it would mutate the wrong task. If + // the active element is not inside the matching focusedTaskId, + // drop it. + // Only the DOM actively contradicting invalidates focus, so the inline-edit + // recovery path above stays intact. + const active = document.activeElement as HTMLElement | null; + const domFocusedTaskId = + (active?.closest('task') as HTMLElement | null)?.getAttribute('data-task-id') ?? + null; + if (domFocusedTaskId) { + focusedTaskId = domFocusedTaskId; + } else if (focusedTaskId) { + focusedTaskId = null; + } + + // Schedule for today (Shift+T). This is the one task shortcut wired to work + // without a live component, so it also fires from views that render + // (the Planner overdue list). When a real is focused + // we still delegate, so the backlog→regular position-only move (#8592/#8603) + // and the overdue branch in moveToToday() are preserved. (#8851) + if (checkKeyCombo(ev, keys.taskScheduleToday)) { + if (focusedTaskId) { + this._handleTaskShortcut(focusedTaskId, 'moveToTodayWithFocus'); + ev.preventDefault(); + ev.stopPropagation(); + return true; + } + const idBasedTaskId = this._resolveTaskIdFromDom(); + if (idBasedTaskId) { + this._taskService.scheduleForTodayById(idBasedTaskId); + ev.preventDefault(); + ev.stopPropagation(); + return true; } } @@ -244,13 +275,6 @@ export class TaskShortcutService { return true; } - if (checkKeyCombo(ev, keys.taskScheduleToday)) { - this._handleTaskShortcut(focusedTaskId, 'moveToTodayWithFocus'); - ev.preventDefault(); - ev.stopPropagation(); - return true; - } - // Navigation shortcuts - only work if context menu is not open if ( !isContextMenuOpen && @@ -355,6 +379,18 @@ export class TaskShortcutService { return false; } + /** + * Resolves a task id straight from the focused element by walking up to the + * nearest host carrying `data-task-id`. Generic over the host selector (works + * for both `` and ``) so the id-based shortcut path can + * act on a task without a live `` component. (#8851) + */ + private _resolveTaskIdFromDom(): TaskId | null { + const active = document.activeElement as HTMLElement | null; + const host = active?.closest('[data-task-id]') as HTMLElement | null; + return host?.getAttribute('data-task-id') ?? null; + } + /** * Calls a method on the currently focused task component. * diff --git a/src/app/features/tasks/task.service.ts b/src/app/features/tasks/task.service.ts index 4e381c5ceb..06c48e518e 100644 --- a/src/app/features/tasks/task.service.ts +++ b/src/app/features/tasks/task.service.ts @@ -462,6 +462,24 @@ export class TaskService { ); } + /** + * Schedules a task for today by id (same effect as the "Add to My Day" + * button / Schedule → Today). Used by the id-based schedule-today shortcut + * path so it works from views without a live `` component (e.g. the + * Planner overdue list, which renders ``). (#8851) + */ + scheduleForTodayById(taskId: string): void { + const task = this._taskEntities()[taskId]; + this._store.dispatch( + TaskSharedActions.planTasksForToday({ + taskIds: [taskId], + today: this._dateService.todayStr(), + startOfNextDayDiffMs: this._dateService.getStartOfNextDayDiffMs(), + parentTaskMap: task ? { [taskId]: task.parentId } : undefined, + }), + ); + } + remove(task: TaskWithSubTasks): void { this._store.dispatch(TaskSharedActions.deleteTask({ task })); } diff --git a/src/app/features/tasks/task/task-shortcuts.spec.ts b/src/app/features/tasks/task/task-shortcuts.spec.ts index 6f2246e5d9..6e4b66c70d 100644 --- a/src/app/features/tasks/task/task-shortcuts.spec.ts +++ b/src/app/features/tasks/task/task-shortcuts.spec.ts @@ -7,6 +7,7 @@ import { of } from 'rxjs'; import { DialogFullscreenMarkdownComponent } from '../../../ui/dialog-fullscreen-markdown/dialog-fullscreen-markdown.component'; import { DateAdapter } from '@angular/material/core'; import { PlannerActions } from '../../planner/store/planner.actions'; +import { TaskSharedActions } from '../../../root-store/meta/task-shared.actions'; import { DateService } from '../../../core/date/date.service'; import { GlobalTrackingIntervalService } from '../../../core/global-tracking-interval/global-tracking-interval.service'; import { LayoutService } from '../../../core-ui/layout/layout.service'; @@ -557,4 +558,76 @@ describe('TaskComponent shortcut handling', () => { expect(focusByIdSpy).not.toHaveBeenCalled(); })); }); + + describe('moveToToday overdue branch (#8851)', () => { + let dateService: jasmine.SpyObj; + let projectService: jasmine.SpyObj; + + beforeEach(() => { + dateService = TestBed.inject(DateService) as jasmine.SpyObj; + projectService = TestBed.inject(ProjectService) as jasmine.SpyObj; + (dateService as any).todayStr = jasmine + .createSpy('todayStr') + .and.returnValue('2026-06-01'); + (dateService as any).getStartOfNextDayDiffMs = jasmine + .createSpy('getStartOfNextDayDiffMs') + .and.returnValue(0); + }); + + it('schedules an overdue task for today instead of a position-only move', () => { + fixture.componentRef.setInput('task', { + ...createTopLevelTask('Overdue'), + dueDay: '2026-05-30', + }); + storeSpy.dispatch.calls.reset(); + + component.moveToToday(); + + expect(storeSpy.dispatch).toHaveBeenCalledWith( + TaskSharedActions.planTasksForToday({ + taskIds: ['top-1'], + today: '2026-06-01', + startOfNextDayDiffMs: 0, + parentTaskMap: { ['top-1']: undefined }, + }), + ); + expect(projectService.moveTaskToTodayList).not.toHaveBeenCalled(); + }); + + it('keeps the position-only move for a non-overdue task (#8592)', () => { + fixture.componentRef.setInput('task', { + ...createTopLevelTask('Not overdue'), + dueDay: undefined, + dueWithTime: undefined, + }); + storeSpy.dispatch.calls.reset(); + + component.moveToToday(); + + expect(projectService.moveTaskToTodayList).toHaveBeenCalledWith( + 'top-1', + 'project-1', + ); + expect(storeSpy.dispatch).not.toHaveBeenCalled(); + }); + + it('keeps the position-only move for a done task with a stale past dueDay', () => { + // A done task can sit in the backlog with an old dueDay; it must take the + // backlog→regular position-only move, not be re-added to Today. + fixture.componentRef.setInput('task', { + ...createTopLevelTask('Done + overdue'), + isDone: true, + dueDay: '2026-05-30', + }); + storeSpy.dispatch.calls.reset(); + + component.moveToToday(); + + expect(projectService.moveTaskToTodayList).toHaveBeenCalledWith( + 'top-1', + 'project-1', + ); + expect(storeSpy.dispatch).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/app/features/tasks/task/task.component.ts b/src/app/features/tasks/task/task.component.ts index 5de8bd41ab..4cd4739c91 100644 --- a/src/app/features/tasks/task/task.component.ts +++ b/src/app/features/tasks/task/task.component.ts @@ -81,6 +81,7 @@ import { PlannerActions } from '../../planner/store/planner.actions'; import { PlannerService } from '../../planner/planner.service'; import { DialogDeadlineComponent } from '../dialog-deadline/dialog-deadline.component'; import { isDeadlineOverdue as isDeadlineOverdueFn } from '../util/is-deadline-overdue'; +import { isTaskOverdue } from '../util/is-task-overdue'; import { isDeadlineApproaching as isDeadlineApproachingFn } from '../util/is-deadline-approaching'; import { TaskContextMenuComponent } from '../task-context-menu/task-context-menu.component'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; @@ -1362,11 +1363,32 @@ export class TaskComponent implements OnDestroy, AfterViewInit { moveToToday(): void { const t = this.task(); - if (t.projectId) { - // Moving to the regular list is a list-position change only; it must not - // schedule the task for today (#8592). - this._projectService.moveTaskToTodayList(t.id, t.projectId); + if (!t.projectId) { + return; } + // An overdue task is never in the backlog, so the position-only move below + // early-returns for it (moveProjectTaskToRegularListAuto) and Shift+T would + // no-op. Schedule it for today instead — the same thing the "Add to My Day" + // button and Schedule → Today do (#8851). Overdue vs. backlog→regular are + // cleanly separated because overdue tasks are never in the backlog. Exclude + // done tasks: a done task with a stale past dueDay can still sit in the + // backlog, and it should take the position-only move, not be re-added to + // Today. (isTaskOverdue stays done-agnostic — selectOverdueTasks needs + // done tasks included.) + if ( + !t.isDone && + isTaskOverdue( + t, + this._dateService.todayStr(), + this._dateService.getStartOfNextDayDiffMs(), + ) + ) { + this.addToMyDay(); + return; + } + // Moving to the regular list is a list-position change only; it must not + // schedule the task for today (#8592). + this._projectService.moveTaskToTodayList(t.id, t.projectId); } trackByProjectId(i: number, project: Project): string { diff --git a/src/app/features/tasks/util/is-task-overdue.spec.ts b/src/app/features/tasks/util/is-task-overdue.spec.ts new file mode 100644 index 0000000000..6ac24e480a --- /dev/null +++ b/src/app/features/tasks/util/is-task-overdue.spec.ts @@ -0,0 +1,114 @@ +import { + getLogicalTodayStartMs, + isTaskOverdue, + isTaskOverdueByThreshold, +} from './is-task-overdue'; +import { Task } from '../task.model'; + +const createTask = (overrides: Partial = {}): Task => + ({ + id: 'task1', + dueDay: undefined, + dueWithTime: undefined, + ...overrides, + }) as Task; + +describe('isTaskOverdue', () => { + const TODAY_STR = '2026-03-15'; + const NO_OFFSET = 0; + + describe('dueDay', () => { + it('is overdue when dueDay is before todayStr', () => { + expect( + isTaskOverdue(createTask({ dueDay: '2026-03-14' }), TODAY_STR, NO_OFFSET), + ).toBe(true); + }); + + it('is not overdue when dueDay equals todayStr', () => { + expect( + isTaskOverdue(createTask({ dueDay: '2026-03-15' }), TODAY_STR, NO_OFFSET), + ).toBe(false); + }); + + it('is not overdue when dueDay is after todayStr', () => { + expect( + isTaskOverdue(createTask({ dueDay: '2026-03-16' }), TODAY_STR, NO_OFFSET), + ).toBe(false); + }); + + it('is not overdue when dueDay is not a valid YYYY-MM-DD string', () => { + expect( + isTaskOverdue(createTask({ dueDay: '3/14/2026' }), TODAY_STR, NO_OFFSET), + ).toBe(false); + }); + }); + + describe('dueWithTime', () => { + // Boundary is local start-of-day (dateStrToUtcDate returns local midnight), + // so build timestamps with local Date constructors to stay timezone-safe. + it('is overdue when dueWithTime is before the start of today', () => { + const ts = new Date(2026, 2, 14, 23, 0, 0).getTime(); + expect(isTaskOverdue(createTask({ dueWithTime: ts }), TODAY_STR, NO_OFFSET)).toBe( + true, + ); + }); + + it('is not overdue when dueWithTime is later today', () => { + const ts = new Date(2026, 2, 15, 10, 0, 0).getTime(); + expect(isTaskOverdue(createTask({ dueWithTime: ts }), TODAY_STR, NO_OFFSET)).toBe( + false, + ); + }); + + it('respects the start-of-next-day offset', () => { + // A 4h offset pushes the logical start of "today" forward, so a moment + // just after midnight still belongs to the previous logical day → overdue. + const offset = 4 * 60 * 60 * 1000; + const justAfterMidnight = new Date(2026, 2, 15, 1, 0, 0).getTime(); + expect( + isTaskOverdue(createTask({ dueWithTime: justAfterMidnight }), TODAY_STR, offset), + ).toBe(true); + expect( + isTaskOverdue( + createTask({ dueWithTime: justAfterMidnight }), + TODAY_STR, + NO_OFFSET, + ), + ).toBe(false); + }); + }); + + describe('no due date', () => { + it('is not overdue when the task has no due fields', () => { + expect(isTaskOverdue(createTask(), TODAY_STR, NO_OFFSET)).toBe(false); + }); + }); + + describe('shared threshold contract', () => { + it('getLogicalTodayStartMs returns local midnight shifted by the offset', () => { + const localMidnight = new Date(2026, 2, 15).getTime(); + expect(getLogicalTodayStartMs(TODAY_STR, NO_OFFSET)).toBe(localMidnight); + const offset = 4 * 60 * 60 * 1000; + expect(getLogicalTodayStartMs(TODAY_STR, offset)).toBe(localMidnight + offset); + }); + + it('isTaskOverdue delegates to isTaskOverdueByThreshold with the computed threshold', () => { + // Guards against the two overdue definitions drifting: isTaskOverdue must + // equal the threshold predicate fed the same logical start-of-today. + const offset = 4 * 60 * 60 * 1000; + const threshold = getLogicalTodayStartMs(TODAY_STR, offset); + const cases: Partial[] = [ + { dueDay: '2026-03-14' }, + { dueDay: '2026-03-15' }, + { dueWithTime: new Date(2026, 2, 15, 1, 0, 0).getTime() }, + {}, + ]; + cases.forEach((overrides) => { + const task = createTask(overrides); + expect(isTaskOverdue(task, TODAY_STR, offset)).toBe( + isTaskOverdueByThreshold(task, TODAY_STR, threshold), + ); + }); + }); + }); +}); diff --git a/src/app/features/tasks/util/is-task-overdue.ts b/src/app/features/tasks/util/is-task-overdue.ts new file mode 100644 index 0000000000..65f81e3d75 --- /dev/null +++ b/src/app/features/tasks/util/is-task-overdue.ts @@ -0,0 +1,60 @@ +import { Task } from '../task.model'; +import { isDBDateStr } from '../../../util/get-db-date-str'; +import { dateStrToUtcDate } from '../../../util/date-str-to-utc-date'; + +/** + * Compute the logical start-of-today (ms) — local midnight of `todayStr` shifted + * by the start-of-next-day offset. Extracted so the overdue predicate and its + * callers agree on one boundary definition. + */ +export const getLogicalTodayStartMs = ( + todayStr: string, + startOfNextDayDiffMs: number, +): number => { + const today = dateStrToUtcDate(todayStr); + today.setHours(0, 0, 0, 0); + // The logical start of "today" is shifted by the offset. + return today.getTime() + startOfNextDayDiffMs; +}; + +/** + * Overdue comparison against a *precomputed* logical start-of-today threshold. + * This is the single source of truth for "what counts as overdue"; both + * `isTaskOverdue` and `selectOverdueTaskIds` route through it so the two overdue + * definitions can never drift. Callers that iterate many tasks (the selector) + * compute the threshold once and pass it in, instead of per task. + * + * Priority follows the dueWithTime/dueDay mutual-exclusivity pattern. + */ +export const isTaskOverdueByThreshold = ( + task: Pick, + todayStr: string, + todayStartMs: number, +): boolean => + !!( + // String comparison works because dueDay is YYYY-MM-DD (lexicographically + // sortable), avoiding timezone conversion issues. + ( + (task.dueDay && isDBDateStr(task.dueDay) && task.dueDay < todayStr) || + (task.dueWithTime && task.dueWithTime < todayStartMs) + ) + ); + +/** + * Pure predicate for "is this task overdue" — its due date is before the logical + * "today". + * + * Kept clock-free/deterministic: the caller threads in `todayStr` (a DB date + * string, e.g. from `DateService.getLogicalTodayDate()`/`todayStr()`) and the + * start-of-next-day offset so custom start-of-day settings are respected. + */ +export const isTaskOverdue = ( + task: Pick, + todayStr: string, + startOfNextDayDiffMs: number, +): boolean => + isTaskOverdueByThreshold( + task, + todayStr, + getLogicalTodayStartMs(todayStr, startOfNextDayDiffMs), + );