mirror of
https://github.com/johannesjo/super-productivity.git
synced 2026-07-17 16:37:43 +00:00
* fix(tasks): respect auto-add-to-today setting when completing tasks Completing an unscheduled top-level task stamped a completion-day dueDay even when "Automatically add worked-on tasks to today" was off (discussion #8463). The meta-reducer updateDoneOnForTask synthesizes that dueDay ungated by the setting, and the gated effect autoAddTodayTagOnMarkAsDone had become dead (the reducer set dueDay before its \!task.dueDay filter ran). Gating the reducer on the synced config would diverge on replay since ops apply in causal arrival order, so the decision is frozen into the op at completion time instead: getMarkDoneTaskChanges passes an explicit dueDay: null for an unscheduled top-level task when the setting is off, which trips the reducer's hasScheduleInUpdate guard and skips synthesis. null (not undefined) survives serialization for deterministic replay; the reducer itself is left untouched so legacy/replay behavior is unchanged. Applied at every completion producer: TaskService.setDone (the funnel for the context menu, reminders, bulk project-done, Android, etc.), the four components that dispatched updateTask directly (task-list, schedule-event, focus-mode-main/break now route through setDone), and the moveTaskToDone$ and auto-mark-parent effects. The dead autoAddTodayTagOnMarkAsDone effect is removed so completion-dating has a single source of truth. Only future completions are affected; already-dated tasks are unchanged. * fix(tasks): freeze offset-correct completion day into the op Follow-up to the auto-add-to-today fix. When auto-add is on, the completion day was still synthesized by the reducer, which derives it from doneOn via the offset-blind getDbDateStr during replay while the live path used the offset-adjusted todayStr. For users with a custom "start of next day" this made the stamped dueDay differ between the originating device (logical day) and a replaying device (calendar day). getMarkDoneTaskChanges now freezes the offset-adjusted logical day (DateService.todayStr) into the op for unscheduled top-level completions (dueDay: todayStr when on, dueDay: null when off). Producers pass todayStr; the reducer applies the explicit value and its synthesis stays only as a legacy fallback. Replay reproduces the frozen value, so the day is both offset-correct and identical across devices. TODAY_TAG membership is unchanged (the dueDay-change branch fires identically for an explicit vs synthesized today). * refactor(tasks): record only doneOn on completion, never stamp dueDay Completion no longer synthesizes or freezes a dueDay (reverting the Option A gating and the May-18 completion-day stamp). The Today "Done" list is driven by isDone/doneOn, not dueDay, so completed tasks still show there. Existing schedules are preserved on completion; the isAutoAddWorkedOnToToday setting now gates only the time-tracking auto-add path. Removes the dead autoAddTodayTagOnMarkAsDone effect, the reducer dueDay synthesis, and the converter's legacy dueDay back-fill (keeping the doneOn back-fill). * refactor(tasks): simplify done-today counter, document legacy-op sanitizer Multi-review follow-up. The done-today counter now counts via the flat selectAllTasks instead of selectAllTasksWithSubTasks + flattenTasks (same count, no per-emission nesting/allocation). Adds a comment clarifying that sanitizeDoneScheduleChanges now exists only as legacy-op replay defense. * test(workflow): pin finish-day archive of unscheduled completed tasks Regression guard for discussion #8463. Since completion now records only doneOn (no dueDay), this e2e proves an unscheduled completed task still (a) shows in the Today Done list and (b) is cleared to the archive on Finish Day — both driven by isDone/doneOn, not dueDay. Fails if a future change scopes the Today Done list or the finish-day archive by dueDay. * chore: drop stray plugin-dev lockfile changes from e2e build The e2e plugin build bumped vite in packages/plugin-dev lockfiles; that churn was unintentionally swept into the test commit and is unrelated to this PR. Restore both lockfiles to the base version. * test(archive): archived completion no longer stamps dueDay The TaskArchiveService.updateTask path runs the same task-shared meta-reducer as live tasks, so #8463's "completion records only doneOn, never dueDay" now applies there too. Update the assertion: completing an archived task sets doneOn but leaves dueDay undefined.
This commit is contained in:
Johannes Millan
GitHub
parent
c5651f52c7
commit
52f9cb167d
11 changed files with 118 additions and 266 deletions
55
e2e/tests/workflow/finish-day-archives-unscheduled.spec.ts
Normal file
55
e2e/tests/workflow/finish-day-archives-unscheduled.spec.ts
Normal file
|
|
@ -0,0 +1,55 @@
|
|||
import { expect, test } from '../../fixtures/test.fixture';
|
||||
|
||||
/**
|
||||
* Regression guard for discussion #8463.
|
||||
*
|
||||
* Completing a task now records only `doneOn` and no longer stamps a `dueDay`.
|
||||
* An UNSCHEDULED completed task (one that was never on Today — e.g. a project
|
||||
* task, which gets no `dueDay` at creation) must therefore still:
|
||||
* (a) appear in the Today "Done" list, and
|
||||
* (b) be cleared to the archive on Finish Day.
|
||||
* Both are driven by `isDone`/`doneOn`, not `dueDay`. If a future change scopes
|
||||
* the Today Done list or the finish-day archive by `dueDay`, this test fails.
|
||||
*/
|
||||
|
||||
const FINISH_DAY_BTN = '.e2e-finish-day';
|
||||
const SAVE_AND_GO_HOME_BTN =
|
||||
'daily-summary button[mat-flat-button][color="primary"]:last-of-type';
|
||||
|
||||
test.describe('Finish Day archives unscheduled completed tasks (#8463)', () => {
|
||||
test('completed inbox task shows in Today Done and is archived on finish day', async ({
|
||||
page,
|
||||
workViewPage,
|
||||
taskPage,
|
||||
testPrefix,
|
||||
}) => {
|
||||
await workViewPage.waitForTaskList();
|
||||
|
||||
// A task created in a project context (here: the Inbox) gets no `dueDay` —
|
||||
// only the Today context stamps one at creation. So this exercises the
|
||||
// "unscheduled" completion path.
|
||||
await page.goto('/#/project/INBOX_PROJECT/tasks');
|
||||
await workViewPage.waitForTaskList();
|
||||
const taskName = `${testPrefix}-Unscheduled Done`;
|
||||
await workViewPage.addTask(taskName);
|
||||
await taskPage.markTaskAsDone(taskPage.getTaskByText(taskName));
|
||||
await expect(taskPage.getDoneTasks().filter({ hasText: taskName })).toHaveCount(1);
|
||||
|
||||
// (a) It appears in the Today "Done" list despite having no dueDay.
|
||||
await page.goto('/#/tag/TODAY/tasks');
|
||||
await workViewPage.waitForTaskList();
|
||||
await expect(taskPage.getDoneTasks().filter({ hasText: taskName })).toHaveCount(1);
|
||||
|
||||
// (b) Finish Day moves it to the archive.
|
||||
await page.locator(FINISH_DAY_BTN).click();
|
||||
await page.waitForURL(/daily-summary/);
|
||||
await expect(page.locator('daily-summary')).toContainText('Unscheduled Done');
|
||||
await page.locator(SAVE_AND_GO_HOME_BTN).click();
|
||||
await page.waitForURL(/tag\/TODAY/);
|
||||
await workViewPage.waitForTaskList();
|
||||
|
||||
// The Today Done list is the global active-isDone list, so the task being gone
|
||||
// from it proves it was archived, not merely hidden.
|
||||
await expect(taskPage.getTaskByText(taskName)).toHaveCount(0);
|
||||
});
|
||||
});
|
||||
|
|
@ -309,7 +309,9 @@ describe('TaskArchiveService', () => {
|
|||
|
||||
expect(updatedTask.isDone).toBe(true);
|
||||
expect(updatedTask.doneOn).toBeGreaterThan(0);
|
||||
expect(updatedTask.dueDay).toBeDefined();
|
||||
// #8463: completion records only doneOn and never stamps a dueDay (the
|
||||
// archive path runs the same meta-reducer as live tasks).
|
||||
expect(updatedTask.dueDay).toBeUndefined();
|
||||
|
||||
// Mark as undone
|
||||
archiveDbAdapterMock.loadArchiveYoung.and.returnValue(
|
||||
|
|
|
|||
|
|
@ -4,11 +4,9 @@ import { provideMockStore, MockStore } from '@ngrx/store/testing';
|
|||
import { of, Subject } from 'rxjs';
|
||||
import { Action } from '@ngrx/store';
|
||||
import { TaskRelatedModelEffects } from './task-related-model.effects';
|
||||
import { TaskService } from '../task.service';
|
||||
import { GlobalConfigService } from '../../config/global-config.service';
|
||||
import { HydrationStateService } from '../../../op-log/apply/hydration-state.service';
|
||||
import { LOCAL_ACTIONS } from '../../../util/local-actions.token';
|
||||
import { TaskSharedActions } from '../../../root-store/meta/task-shared.actions';
|
||||
import { DEFAULT_TASK, Task } from '../task.model';
|
||||
import { getDbDateStr } from '../../../util/get-db-date-str';
|
||||
import { selectTodayTaskIds } from '../../work-context/store/work-context.selectors';
|
||||
|
|
@ -19,8 +17,6 @@ describe('TaskRelatedModelEffects', () => {
|
|||
let effects: TaskRelatedModelEffects;
|
||||
let actions$: Subject<Action>;
|
||||
let store: MockStore;
|
||||
let taskService: jasmine.SpyObj<TaskService>;
|
||||
let hydrationStateService: jasmine.SpyObj<HydrationStateService>;
|
||||
let dateService: jasmine.SpyObj<DateService>;
|
||||
|
||||
const createTask = (id: string, partial: Partial<Task> = {}): Task => ({
|
||||
|
|
@ -35,7 +31,6 @@ describe('TaskRelatedModelEffects', () => {
|
|||
beforeEach(() => {
|
||||
actions$ = new Subject<Action>();
|
||||
|
||||
const taskServiceSpy = jasmine.createSpyObj('TaskService', ['getByIdOnce$']);
|
||||
const hydrationStateServiceSpy = jasmine.createSpyObj('HydrationStateService', [
|
||||
'isApplyingRemoteOps',
|
||||
]);
|
||||
|
|
@ -54,7 +49,6 @@ describe('TaskRelatedModelEffects', () => {
|
|||
provideMockStore({
|
||||
selectors: [{ selector: selectTodayTaskIds, value: [] }],
|
||||
}),
|
||||
{ provide: TaskService, useValue: taskServiceSpy },
|
||||
{ provide: DateService, useValue: dateServiceSpy },
|
||||
{
|
||||
provide: GlobalConfigService,
|
||||
|
|
@ -69,10 +63,6 @@ describe('TaskRelatedModelEffects', () => {
|
|||
|
||||
effects = TestBed.inject(TaskRelatedModelEffects);
|
||||
store = TestBed.inject(MockStore);
|
||||
taskService = TestBed.inject(TaskService) as jasmine.SpyObj<TaskService>;
|
||||
hydrationStateService = TestBed.inject(
|
||||
HydrationStateService,
|
||||
) as jasmine.SpyObj<HydrationStateService>;
|
||||
dateService = TestBed.inject(DateService) as jasmine.SpyObj<DateService>;
|
||||
});
|
||||
|
||||
|
|
@ -150,176 +140,4 @@ describe('TaskRelatedModelEffects', () => {
|
|||
subscription.unsubscribe();
|
||||
}));
|
||||
});
|
||||
|
||||
describe('autoAddTodayTagOnMarkAsDone', () => {
|
||||
it('should dispatch planTasksForToday when an unscheduled task is marked done', (done) => {
|
||||
const task = createTask('task-1', {
|
||||
parentId: undefined,
|
||||
dueDay: undefined,
|
||||
dueWithTime: undefined,
|
||||
});
|
||||
taskService.getByIdOnce$.and.returnValue(of(task));
|
||||
|
||||
effects.autoAddTodayTagOnMarkAsDone.subscribe({
|
||||
next: (action) => {
|
||||
expect(action).toEqual(
|
||||
jasmine.objectContaining({
|
||||
taskIds: ['task-1'],
|
||||
today: dateService.todayStr(),
|
||||
startOfNextDayDiffMs: dateService.getStartOfNextDayDiffMs(),
|
||||
}),
|
||||
);
|
||||
done();
|
||||
},
|
||||
error: done.fail,
|
||||
});
|
||||
|
||||
actions$.next(
|
||||
TaskSharedActions.updateTask({
|
||||
task: { id: 'task-1', changes: { isDone: true } },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should NOT dispatch planTasksForToday when marked done task has an existing dueDay', fakeAsync(() => {
|
||||
const task = createTask('task-1', {
|
||||
parentId: undefined,
|
||||
dueDay: '2026-05-16',
|
||||
});
|
||||
taskService.getByIdOnce$.and.returnValue(of(task));
|
||||
|
||||
let emitted = false;
|
||||
const subscription = effects.autoAddTodayTagOnMarkAsDone.subscribe(() => {
|
||||
emitted = true;
|
||||
});
|
||||
|
||||
actions$.next(
|
||||
TaskSharedActions.updateTask({
|
||||
task: { id: 'task-1', changes: { isDone: true } },
|
||||
}),
|
||||
);
|
||||
tick();
|
||||
|
||||
expect(emitted).toBe(false);
|
||||
subscription.unsubscribe();
|
||||
}));
|
||||
|
||||
it('should NOT dispatch planTasksForToday when marked done task has dueWithTime', fakeAsync(() => {
|
||||
const task = createTask('task-1', {
|
||||
parentId: undefined,
|
||||
dueWithTime: Date.now(),
|
||||
});
|
||||
taskService.getByIdOnce$.and.returnValue(of(task));
|
||||
|
||||
let emitted = false;
|
||||
const subscription = effects.autoAddTodayTagOnMarkAsDone.subscribe(() => {
|
||||
emitted = true;
|
||||
});
|
||||
|
||||
actions$.next(
|
||||
TaskSharedActions.updateTask({
|
||||
task: { id: 'task-1', changes: { isDone: true } },
|
||||
}),
|
||||
);
|
||||
tick();
|
||||
|
||||
expect(emitted).toBe(false);
|
||||
subscription.unsubscribe();
|
||||
}));
|
||||
|
||||
it('should NOT dispatch planTasksForToday when task is marked done but already has dueDay set to today', fakeAsync(() => {
|
||||
const today = getDbDateStr();
|
||||
const task = createTask('task-1', {
|
||||
parentId: undefined,
|
||||
dueDay: today, // Already set to today
|
||||
});
|
||||
taskService.getByIdOnce$.and.returnValue(of(task));
|
||||
|
||||
let emitted = false;
|
||||
const subscription = effects.autoAddTodayTagOnMarkAsDone.subscribe(() => {
|
||||
emitted = true;
|
||||
});
|
||||
|
||||
actions$.next(
|
||||
TaskSharedActions.updateTask({
|
||||
task: { id: 'task-1', changes: { isDone: true } },
|
||||
}),
|
||||
);
|
||||
tick();
|
||||
|
||||
expect(emitted).toBe(false);
|
||||
subscription.unsubscribe();
|
||||
}));
|
||||
|
||||
it('should NOT dispatch planTasksForToday when task has a parent', fakeAsync(() => {
|
||||
const task = createTask('task-1', {
|
||||
parentId: 'parent-task', // Has parent
|
||||
dueDay: undefined,
|
||||
});
|
||||
taskService.getByIdOnce$.and.returnValue(of(task));
|
||||
|
||||
let emitted = false;
|
||||
const subscription = effects.autoAddTodayTagOnMarkAsDone.subscribe(() => {
|
||||
emitted = true;
|
||||
});
|
||||
|
||||
actions$.next(
|
||||
TaskSharedActions.updateTask({
|
||||
task: { id: 'task-1', changes: { isDone: true } },
|
||||
}),
|
||||
);
|
||||
tick();
|
||||
|
||||
expect(emitted).toBe(false);
|
||||
subscription.unsubscribe();
|
||||
}));
|
||||
|
||||
it('should NOT dispatch when hydration is in progress', fakeAsync(() => {
|
||||
hydrationStateService.isApplyingRemoteOps.and.returnValue(true);
|
||||
|
||||
const task = createTask('task-1', {
|
||||
parentId: undefined,
|
||||
dueDay: undefined,
|
||||
});
|
||||
taskService.getByIdOnce$.and.returnValue(of(task));
|
||||
|
||||
let emitted = false;
|
||||
const subscription = effects.autoAddTodayTagOnMarkAsDone.subscribe(() => {
|
||||
emitted = true;
|
||||
});
|
||||
|
||||
actions$.next(
|
||||
TaskSharedActions.updateTask({
|
||||
task: { id: 'task-1', changes: { isDone: true } },
|
||||
}),
|
||||
);
|
||||
tick();
|
||||
|
||||
expect(emitted).toBe(false);
|
||||
subscription.unsubscribe();
|
||||
}));
|
||||
|
||||
it('should NOT dispatch when isDone is false', fakeAsync(() => {
|
||||
const task = createTask('task-1', {
|
||||
parentId: undefined,
|
||||
dueDay: undefined,
|
||||
});
|
||||
taskService.getByIdOnce$.and.returnValue(of(task));
|
||||
|
||||
let emitted = false;
|
||||
const subscription = effects.autoAddTodayTagOnMarkAsDone.subscribe(() => {
|
||||
emitted = true;
|
||||
});
|
||||
|
||||
actions$.next(
|
||||
TaskSharedActions.updateTask({
|
||||
task: { id: 'task-1', changes: { isDone: false } },
|
||||
}),
|
||||
);
|
||||
tick();
|
||||
|
||||
expect(emitted).toBe(false);
|
||||
subscription.unsubscribe();
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,11 +1,9 @@
|
|||
import { inject, Injectable } from '@angular/core';
|
||||
import { createEffect, ofType } from '@ngrx/effects';
|
||||
import { TaskSharedActions } from '../../../root-store/meta/task-shared.actions';
|
||||
import { filter, map, mergeMap, switchMap, withLatestFrom } from 'rxjs/operators';
|
||||
import { Task } from '../task.model';
|
||||
import { filter, map, switchMap, withLatestFrom } from 'rxjs/operators';
|
||||
import { moveTaskInTodayList } from '../../work-context/store/work-context-meta.actions';
|
||||
import { GlobalConfigService } from '../../config/global-config.service';
|
||||
import { TaskService } from '../task.service';
|
||||
import { EMPTY, Observable } from 'rxjs';
|
||||
import { moveProjectTaskToRegularList } from '../../project/store/project.actions';
|
||||
import { TimeTrackingActions } from '../../time-tracking/store/time-tracking.actions';
|
||||
|
|
@ -18,7 +16,6 @@ import { DateService } from '../../../core/date/date.service';
|
|||
@Injectable()
|
||||
export class TaskRelatedModelEffects {
|
||||
private _actions$ = inject(LOCAL_ACTIONS);
|
||||
private _taskService = inject(TaskService);
|
||||
private _globalConfigService = inject(GlobalConfigService);
|
||||
private _store = inject(Store);
|
||||
private _hydrationState = inject(HydrationStateService);
|
||||
|
|
@ -57,36 +54,11 @@ export class TaskRelatedModelEffects {
|
|||
),
|
||||
);
|
||||
|
||||
autoAddTodayTagOnMarkAsDone = createEffect(() =>
|
||||
this.ifAutoAddTodayEnabled$(
|
||||
this._actions$.pipe(
|
||||
ofType(TaskSharedActions.updateTask),
|
||||
filter((a) => a.task.changes.isDone === true),
|
||||
// PERF: Skip during hydration/sync to avoid service calls
|
||||
filter(() => !this._hydrationState.isApplyingRemoteOps()),
|
||||
// Use mergeMap instead of switchMap to ensure ALL mark-as-done actions
|
||||
// trigger planTasksForToday, not just the last one. switchMap would cancel
|
||||
// previous inner subscriptions when new actions arrive quickly.
|
||||
mergeMap(({ task }) => this._taskService.getByIdOnce$(task.id as string)),
|
||||
// Only auto-plan unscheduled tasks. Completion records doneOn, but must not
|
||||
// rewrite an existing dueDay/dueWithTime schedule.
|
||||
filter(
|
||||
(task: Task) =>
|
||||
!!task &&
|
||||
!task.parentId &&
|
||||
!task.dueDay &&
|
||||
typeof task.dueWithTime !== 'number',
|
||||
),
|
||||
map((task) =>
|
||||
TaskSharedActions.planTasksForToday({
|
||||
taskIds: [task.id],
|
||||
today: this._dateService.todayStr(),
|
||||
startOfNextDayDiffMs: this._dateService.getStartOfNextDayDiffMs(),
|
||||
}),
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
// NOTE: Completing a task no longer auto-dates it. Completion records only
|
||||
// `doneOn`; it never synthesizes or freezes a `dueDay`. The Today "Done" list
|
||||
// is driven by `isDone`/`doneOn`, so completed tasks still show there without a
|
||||
// schedule. The `isAutoAddWorkedOnToToday` setting now gates ONLY the
|
||||
// time-tracking auto-add path above (`autoAddTodayTagOnTracking`).
|
||||
|
||||
// EXTERNAL ===> TASKS
|
||||
// -------------------
|
||||
|
|
|
|||
|
|
@ -13,7 +13,6 @@ import { filterOutId } from '../../../util/filter-out-id';
|
|||
import { Update } from '@ngrx/entity';
|
||||
import { TaskLog } from '../../../core/log';
|
||||
import { devError } from '../../../util/dev-error';
|
||||
import { getDbDateStr } from '../../../util/get-db-date-str';
|
||||
|
||||
export const getTaskById = (taskId: string, state: TaskState): Task => {
|
||||
if (!state.entities[taskId]) {
|
||||
|
|
@ -127,35 +126,18 @@ export const reCalcTimeEstimateForParentIfParent = (
|
|||
);
|
||||
};
|
||||
|
||||
export const updateDoneOnForTask = (
|
||||
upd: Update<Task>,
|
||||
state: TaskState,
|
||||
todayStr: string,
|
||||
): TaskState => {
|
||||
export const updateDoneOnForTask = (upd: Update<Task>, state: TaskState): TaskState => {
|
||||
const task = state.entities[upd.id] as Task;
|
||||
const isToDone = upd.changes.isDone === true;
|
||||
const isToUnDone = upd.changes.isDone === false;
|
||||
if (isToDone || isToUnDone) {
|
||||
const hasExistingSchedule =
|
||||
typeof task.dueDay === 'string' || typeof task.dueWithTime === 'number';
|
||||
const hasScheduleInUpdate =
|
||||
Object.prototype.hasOwnProperty.call(upd.changes, 'dueDay') ||
|
||||
Object.prototype.hasOwnProperty.call(upd.changes, 'dueWithTime');
|
||||
// Completion records ONLY `doneOn`. It never synthesizes a `dueDay`: that
|
||||
// stays a pure planning field. Any existing schedule is left untouched, and
|
||||
// the Today "Done" list is driven by `isDone`/`doneOn`, not `dueDay`.
|
||||
const doneOn =
|
||||
typeof upd.changes.doneOn === 'number' ? upd.changes.doneOn : Date.now();
|
||||
const completionDay =
|
||||
typeof upd.changes.doneOn === 'number'
|
||||
? getDbDateStr(upd.changes.doneOn)
|
||||
: todayStr;
|
||||
const changes = {
|
||||
...(isToDone
|
||||
? {
|
||||
doneOn,
|
||||
...(!task.parentId && !hasExistingSchedule && !hasScheduleInUpdate
|
||||
? { dueDay: completionDay }
|
||||
: {}),
|
||||
}
|
||||
: {}),
|
||||
...(isToDone ? { doneOn } : {}),
|
||||
...(isToUnDone ? { doneOn: undefined } : {}),
|
||||
};
|
||||
return taskAdapter.updateOne(
|
||||
|
|
|
|||
|
|
@ -392,12 +392,28 @@ export class WorkContextService {
|
|||
// ]))
|
||||
// );
|
||||
|
||||
flatDoneTodayNr$: Observable<number> = this.mainListTasks$.pipe(
|
||||
map((tasks) => flattenTasks(tasks)),
|
||||
map((tasks) => {
|
||||
const done = tasks.filter((task) => task.isDone);
|
||||
return done.length;
|
||||
}),
|
||||
// Counts tasks completed today (feeds the done-sound pitch). On the Today list a
|
||||
// completion records only `doneOn` and no `dueDay`, so unscheduled done tasks are
|
||||
// not in `mainListTasks$`; count them via `doneOn` is-today across all tasks
|
||||
// instead. Other contexts keep counting the context's own done main-list tasks.
|
||||
flatDoneTodayNr$: Observable<number> = this.isTodayList$.pipe(
|
||||
switchMap((isToday) =>
|
||||
isToday
|
||||
? // `selectAllTasks` is already flat (parents + subtasks) — no flattenTasks needed
|
||||
this._store$
|
||||
.select(selectAllTasks)
|
||||
.pipe(
|
||||
map(
|
||||
(tasks) =>
|
||||
tasks.filter(
|
||||
(t) => t.isDone && t.doneOn && this._dateService.isToday(t.doneOn),
|
||||
).length,
|
||||
),
|
||||
)
|
||||
: this.mainListTasks$.pipe(
|
||||
map((tasks) => flattenTasks(tasks).filter((t) => t.isDone).length),
|
||||
),
|
||||
),
|
||||
distinctUntilChanged(), // Only emit when count actually changes
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -400,7 +400,7 @@ describe('operation-converter utility', () => {
|
|||
});
|
||||
|
||||
describe('updateTask done replay date backfill', () => {
|
||||
it('injects doneOn and legacy dueDay from the originating operation timestamp when both are missing', () => {
|
||||
it('injects only doneOn (never a dueDay) from the operation timestamp when missing', () => {
|
||||
const timestamp = new Date(2024, 5, 14, 12, 0, 0, 0).getTime();
|
||||
const op = createMockOperation({
|
||||
actionType: ActionType.TASK_SHARED_UPDATE,
|
||||
|
|
@ -415,7 +415,8 @@ describe('operation-converter utility', () => {
|
|||
const action = convertOpToAction(op) as any;
|
||||
|
||||
expect(action.task.changes.doneOn).toBe(timestamp);
|
||||
expect(action.task.changes.dueDay).toBe('2024-06-14');
|
||||
// Completion records only doneOn; it must not synthesize a dueDay on replay.
|
||||
expect(action.task.changes.dueDay).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not inject dueDay when doneOn exists and dueDay is intentionally absent', () => {
|
||||
|
|
|
|||
|
|
@ -91,16 +91,15 @@ const addReplaySafeDoneFields = (
|
|||
}
|
||||
|
||||
const hasDoneOn = typeof taskChanges['doneOn'] === 'number';
|
||||
const hasDueDay = Object.prototype.hasOwnProperty.call(taskChanges, 'dueDay');
|
||||
const isLegacyDoneOpWithoutDateFields = !hasDoneOn && !hasDueDay;
|
||||
|
||||
const replaySafeChanges = {
|
||||
...taskChanges,
|
||||
// Back-fill `doneOn` for older done ops that didn't store it. We never inject a
|
||||
// `dueDay`: completion records only the completion timestamp and must not
|
||||
// synthesize a schedule, so local apply and replay both yield no `dueDay` for
|
||||
// an unscheduled completion (replay determinism). Any `dueDay` the op already
|
||||
// carries (an explicit schedule) is preserved via the spread above.
|
||||
doneOn: hasDoneOn ? taskChanges['doneOn'] : op.timestamp,
|
||||
// Older done ops did not store the logical day, timezone, or start-of-next-day
|
||||
// offset. This timestamp fallback is replay-stable, but still uses the replaying
|
||||
// device's local calendar and can be off near custom day-start boundaries.
|
||||
...(isLegacyDoneOpWithoutDateFields ? { dueDay: getDbDateStr(op.timestamp) } : {}),
|
||||
};
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -199,7 +199,7 @@ describe('done task operation replay', () => {
|
|||
TestBed.resetTestingModule();
|
||||
});
|
||||
|
||||
it('replays completed tasks with the operation date instead of the replay date', () => {
|
||||
it('records only doneOn (never a dueDay) when replaying an unscheduled completion', () => {
|
||||
const op: Operation = {
|
||||
id: 'op-done-yesterday',
|
||||
actionType: ActionType.TASK_SHARED_UPDATE,
|
||||
|
|
@ -223,8 +223,9 @@ describe('done task operation replay', () => {
|
|||
|
||||
expect(task.isDone).toBe(true);
|
||||
expect(task.doneOn).toBe(DONE_TIMESTAMP);
|
||||
expect(task.dueDay).toBe(ACTION_TODAY);
|
||||
expect(task.dueDay).not.toBe(REPLAY_TODAY);
|
||||
// Completion never synthesizes a dueDay: dueDay stays a pure planning field,
|
||||
// so local apply and replay both yield no dueDay for an unscheduled completion.
|
||||
expect(task.dueDay).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not move an already scheduled task to the completion day during replay', () => {
|
||||
|
|
@ -269,7 +270,7 @@ describe('done task operation replay', () => {
|
|||
expect(task.dueDay).toBe(scheduledDay);
|
||||
});
|
||||
|
||||
it('replays doneOn-only completed unscheduled tasks with the completion day', () => {
|
||||
it('replays a doneOn-carrying unscheduled completion without synthesizing a dueDay', () => {
|
||||
const op: Operation = {
|
||||
id: 'op-done-on-only',
|
||||
actionType: ActionType.TASK_SHARED_UPDATE,
|
||||
|
|
@ -293,8 +294,7 @@ describe('done task operation replay', () => {
|
|||
|
||||
expect(task.isDone).toBe(true);
|
||||
expect(task.doneOn).toBe(DONE_TIMESTAMP);
|
||||
expect(task.dueDay).toBe(ACTION_TODAY);
|
||||
expect(task.dueDay).not.toBe(REPLAY_TODAY);
|
||||
expect(task.dueDay).toBeUndefined();
|
||||
});
|
||||
|
||||
it('ignores legacy synthetic completion dueDay when replaying onto a scheduled task', () => {
|
||||
|
|
|
|||
|
|
@ -1530,7 +1530,7 @@ describe('taskSharedCrudMetaReducer', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should handle isDone updates and set doneOn timestamp and completion dueDay for unscheduled tasks', () => {
|
||||
it('sets doneOn but no dueDay when completing an unscheduled task', () => {
|
||||
const testState = createStateWithExistingTasks(['task1'], [], ['task1']);
|
||||
const action = createUpdateTaskAction('task1', {
|
||||
isDone: true,
|
||||
|
|
@ -1541,10 +1541,13 @@ describe('taskSharedCrudMetaReducer', () => {
|
|||
const updatedTask = resultState[TASK_FEATURE_NAME].entities['task1'] as Task;
|
||||
expect(updatedTask.isDone).toBe(true);
|
||||
expect(updatedTask.doneOn).toEqual(jasmine.any(Number));
|
||||
expect(updatedTask.dueDay).toBe(getDbDateStr());
|
||||
expect((resultState[TAG_FEATURE_NAME].entities['TODAY'] as Tag).taskIds).toContain(
|
||||
'task1',
|
||||
);
|
||||
// Completion records only doneOn; it never synthesizes a dueDay, so the task
|
||||
// is not added to TODAY_TAG via completion (the Today "Done" list is driven by
|
||||
// isDone/doneOn, not dueDay).
|
||||
expect(updatedTask.dueDay).toBeUndefined();
|
||||
expect(
|
||||
(resultState[TAG_FEATURE_NAME].entities['TODAY'] as Tag).taskIds,
|
||||
).not.toContain('task1');
|
||||
});
|
||||
|
||||
it('should add completed task to TODAY_TAG.taskIds', () => {
|
||||
|
|
|
|||
|
|
@ -589,6 +589,10 @@ const handleRestoreDeletedTask = (
|
|||
return updatedState;
|
||||
};
|
||||
|
||||
// Legacy-op replay defense: current clients never emit a synthetic completion
|
||||
// `dueDay` (completion records only `doneOn`), but ops captured by OLDER clients
|
||||
// can carry `{ isDone: true, dueDay: <completionDay> }`. This strips that synthetic
|
||||
// day so replaying such an op can't overwrite an existing schedule.
|
||||
const sanitizeDoneScheduleChanges = (
|
||||
taskUpdate: Update<Task>,
|
||||
currentTask: Task,
|
||||
|
|
@ -683,7 +687,7 @@ const handleUpdateTask = (state: RootState, taskUpdate: Update<Task>): RootState
|
|||
? updateTimeSpentForTask(taskId, timeSpentOnDay, taskState)
|
||||
: taskState;
|
||||
taskState = updateTimeEstimateForTask(cleanedTaskUpdate, timeEstimate, taskState);
|
||||
taskState = updateDoneOnForTask(cleanedTaskUpdate, taskState, todayStr);
|
||||
taskState = updateDoneOnForTask(cleanedTaskUpdate, taskState);
|
||||
taskState = taskAdapter.updateOne(
|
||||
{
|
||||
...cleanedTaskUpdate,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue