diff --git a/android/app/src/main/res/values/strings.xml b/android/app/src/main/res/values/strings.xml index 751f61cf5c..73c7fc919e 100644 --- a/android/app/src/main/res/values/strings.xml +++ b/android/app/src/main/res/values/strings.xml @@ -3,8 +3,8 @@ Super Prod Please update Android System WebView to version %1$d or newer. Super Productivity could not start Android System WebView. - This is not an outdated WebView version. Android reports WebView provider details, but the provider fails to start. This is most often fixed by resetting WebView: open Android System WebView\'s app settings below, then tap Storage → Clear storage and reopen Super Productivity. Updating Android System WebView or switching away from a beta provider can also help. Clearing WebView storage does not delete your Super Productivity tasks, but exporting a backup first is a sensible precaution. - This is not an outdated WebView version warning. Android could not start WebView before provider details were available. This is most often fixed by resetting WebView: open Android System WebView\'s app settings below, then tap Storage → Clear storage and reopen Super Productivity. Updating Android System WebView or switching away from a beta provider can also help. Clearing WebView storage does not delete your Super Productivity tasks, but exporting a backup first is a sensible precaution. + This is not an outdated WebView version. Android reports WebView provider details, but the provider fails to start. This is most often fixed by resetting the active WebView provider app: open the provider app settings below, then tap Storage → Clear storage and reopen Super Productivity. Updating Android System WebView or switching away from a beta provider can also help. Clearing provider storage does not delete your Super Productivity tasks, but it may affect that provider app\'s own browser or app data. Exporting a backup first is a sensible precaution. + This is not an outdated WebView version warning. Android could not start WebView before provider details were available. This is most often fixed by resetting the WebView provider app: open the provider app settings below, then tap Storage → Clear storage and reopen Super Productivity. Updating Android System WebView or switching away from a beta provider can also help. Clearing provider storage does not delete your Super Productivity tasks, but it may affect that provider app\'s own browser or app data. Exporting a backup first is a sensible precaution. Detected Chromium major version: %1$d Detected WebView version: %1$s Provider package: %1$s @@ -18,8 +18,8 @@ Manage Android WebView? Android may show options such as removing WebView updates. Removing updates can clear WebView storage on some devices. Continue only if you have exported or backed up your Super Productivity data; synced data may still require reconnecting accounts and local unsynced changes may be lost. Continue - Reset Android System WebView - This opens Android System WebView\'s app settings. There, tap Storage → Clear storage, then reopen Super Productivity.\n\nYour Super Productivity tasks are stored separately and will not be deleted, but export a backup first if you can. + Reset WebView provider app + This opens the active WebView provider app settings. There, tap Storage → Clear storage, then reopen Super Productivity.\n\nYour Super Productivity tasks are stored separately and will not be deleted, but clearing storage may affect that provider app\'s own browser or app data. Export a backup first if you can. Continue at your own risk Your Android System WebView appears to be older than version %1$d.\n\nIf the version reported above is correct, continuing may cause:\n• The app to crash or fail to load\n• Data to render incorrectly or not at all\n• Sync errors and possible data loss\n\nVersion detection is occasionally wrong on certain devices. If you are sure your WebView is up to date, you can continue — but please update Android System WebView from the Play Store first whenever possible. I understand the risk, continue diff --git a/src/app/features/task-repeat-cfg/store/task-repeat-cleanup.effects.spec.ts b/src/app/features/task-repeat-cfg/store/task-repeat-cleanup.effects.spec.ts index 4110928886..3f7c29c013 100644 --- a/src/app/features/task-repeat-cfg/store/task-repeat-cleanup.effects.spec.ts +++ b/src/app/features/task-repeat-cfg/store/task-repeat-cleanup.effects.spec.ts @@ -8,11 +8,19 @@ import { SyncWrapperService } from '../../../imex/sync/sync-wrapper.service'; import { HydrationStateService } from '../../../op-log/apply/hydration-state.service'; import { DeletedTaskIssueSidecarService } from '../../issue/two-way-sync/deleted-task-issue-sidecar.service'; import { TaskSharedActions } from '../../../root-store/meta/task-shared.actions'; -import { DEFAULT_TASK, Task, TaskWithSubTasks } from '../../tasks/task.model'; +import { + DEFAULT_TASK, + Task, + TaskReminderOptionId, + TaskWithSubTasks, +} from '../../tasks/task.model'; import { getDbDateStr } from '../../../util/get-db-date-str'; import { selectAllTaskRepeatCfgs } from './task-repeat-cfg.selectors'; import { DEFAULT_TASK_REPEAT_CFG, TaskRepeatCfg } from '../task-repeat-cfg.model'; import { DateService } from '../../../core/date/date.service'; +import { getDateTimeFromClockString } from '../../../util/get-date-time-from-clock-string'; +import { remindOptionToMilliseconds } from '../../tasks/util/remind-option-to-milliseconds'; +import { TODAY_TAG } from '../../tag/tag.const'; describe('TaskRepeatCleanupEffects', () => { let effects: TaskRepeatCleanupEffects; @@ -235,11 +243,21 @@ describe('TaskRepeatCleanupEffects', () => { }); describe('skipOverdue cross-day cleanup (#7977)', () => { - const skipOverdueCfg = (id: string, notes = ''): TaskRepeatCfg => - ({ ...DEFAULT_TASK_REPEAT_CFG, id, skipOverdue: true, notes }) as TaskRepeatCfg; + const skipOverdueCfg = ( + id: string, + notes = '', + overrides: Partial = {}, + ): TaskRepeatCfg => + ({ + ...DEFAULT_TASK_REPEAT_CFG, + id, + skipOverdue: true, + notes, + ...overrides, + }) as TaskRepeatCfg; it("deletes yesterday's EMPTY overdue instance once today's instance exists", fakeAsync(() => { - repeatCfgs$.next([skipOverdueCfg('cfg-so')]); + repeatCfgs$.next([skipOverdueCfg('cfg-so', '', { title: 'Water plants' })]); const yesterdayInstance: Task = { ...DEFAULT_TASK, projectId: 'p1', @@ -278,6 +296,442 @@ describe('TaskRepeatCleanupEffects', () => { sub.unsubscribe(); })); + it("deletes yesterday's EMPTY timed overdue instance when schedule matches the template", fakeAsync(() => { + const dueWithTime = getDateTimeFromClockString('09:00', yesterdayMs); + repeatCfgs$.next([ + skipOverdueCfg('cfg-timed-template', '', { + title: 'Water plants', + startTime: '09:00', + remindAt: TaskReminderOptionId.m15, + }), + ]); + const yesterdayInstance: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'timed-template-yesterday', + title: 'Water plants', + repeatCfgId: 'cfg-timed-template', + created: yesterdayMs, + dueDay: undefined, + dueWithTime, + remindAt: remindOptionToMilliseconds(dueWithTime, TaskReminderOptionId.m15), + isDone: false, + timeSpent: 0, + }; + const todayInstance: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'timed-template-today', + title: 'Water plants', + repeatCfgId: 'cfg-timed-template', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + }; + + repeatableTasks$.next([ + wrapWithSubTasks(yesterdayInstance), + wrapWithSubTasks(todayInstance), + ]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()).toEqual(['timed-template-yesterday']); + + sub.unsubscribe(); + })); + + it('keeps an overdue instance whose schedule differs from the template', fakeAsync(() => { + const editedDueWithTime = getDateTimeFromClockString('10:00', yesterdayMs); + repeatCfgs$.next([ + skipOverdueCfg('cfg-schedule', '', { + title: 'Water plants', + startTime: '09:00', + remindAt: TaskReminderOptionId.m15, + }), + ]); + const yesterdayEdited: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'schedule-yesterday', + title: 'Water plants', + repeatCfgId: 'cfg-schedule', + created: yesterdayMs, + dueDay: undefined, + dueWithTime: editedDueWithTime, + remindAt: remindOptionToMilliseconds(editedDueWithTime, TaskReminderOptionId.m15), + isDone: false, + timeSpent: 0, + }; + const todayInstance: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'schedule-today', + title: 'Water plants', + repeatCfgId: 'cfg-schedule', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + }; + + repeatableTasks$.next([ + wrapWithSubTasks(yesterdayEdited), + wrapWithSubTasks(todayInstance), + ]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()) + .withContext('an instance with a user-edited schedule must be preserved') + .toEqual([]); + + sub.unsubscribe(); + })); + + it('keeps an overdue instance with deadline metadata', fakeAsync(() => { + repeatCfgs$.next([skipOverdueCfg('cfg-deadline', '', { title: 'Water plants' })]); + const yesterdayEdited: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'deadline-yesterday', + title: 'Water plants', + repeatCfgId: 'cfg-deadline', + created: yesterdayMs, + dueDay: getDbDateStr(yesterdayMs), + deadlineDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + }; + const todayInstance: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'deadline-today', + title: 'Water plants', + repeatCfgId: 'cfg-deadline', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + }; + + repeatableTasks$.next([ + wrapWithSubTasks(yesterdayEdited), + wrapWithSubTasks(todayInstance), + ]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()) + .withContext('an instance with user-added deadline metadata must be preserved') + .toEqual([]); + + sub.unsubscribe(); + })); + + it('ignores TODAY_TAG when checking whether tags match the template', fakeAsync(() => { + repeatCfgs$.next([ + skipOverdueCfg('cfg-today-tag', '', { + title: 'Water plants', + tagIds: [TODAY_TAG.id, 'tag-a'], + }), + ]); + const yesterdayInstance: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'today-tag-yesterday', + title: 'Water plants', + repeatCfgId: 'cfg-today-tag', + created: yesterdayMs, + dueDay: getDbDateStr(yesterdayMs), + isDone: false, + timeSpent: 0, + tagIds: ['tag-a'], + }; + const todayInstance: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'today-tag-today', + title: 'Water plants', + repeatCfgId: 'cfg-today-tag', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + tagIds: ['tag-a'], + }; + + repeatableTasks$.next([ + wrapWithSubTasks(yesterdayInstance), + wrapWithSubTasks(todayInstance), + ]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()).toEqual(['today-tag-yesterday']); + + sub.unsubscribe(); + })); + + it('keeps an overdue instance whose title differs from the template', fakeAsync(() => { + repeatCfgs$.next([skipOverdueCfg('cfg-title', '', { title: 'Water plants' })]); + const yesterdayEdited: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'title-yesterday', + title: 'Water plants - balcony', + repeatCfgId: 'cfg-title', + created: yesterdayMs, + dueDay: getDbDateStr(yesterdayMs), + isDone: false, + timeSpent: 0, + }; + const todayInstance: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'title-today', + title: 'Water plants', + repeatCfgId: 'cfg-title', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + }; + + repeatableTasks$.next([ + wrapWithSubTasks(yesterdayEdited), + wrapWithSubTasks(todayInstance), + ]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()) + .withContext('an instance with a user-edited title must be preserved') + .toEqual([]); + + sub.unsubscribe(); + })); + + it('keeps an overdue instance whose estimate differs from the template', fakeAsync(() => { + repeatCfgs$.next([ + skipOverdueCfg('cfg-estimate', '', { + title: 'Water plants', + defaultEstimate: 15 * 60 * 1000, + }), + ]); + const yesterdayEdited: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'estimate-yesterday', + title: 'Water plants', + repeatCfgId: 'cfg-estimate', + created: yesterdayMs, + dueDay: getDbDateStr(yesterdayMs), + isDone: false, + timeSpent: 0, + timeEstimate: 30 * 60 * 1000, + }; + const todayInstance: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'estimate-today', + title: 'Water plants', + repeatCfgId: 'cfg-estimate', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + timeEstimate: 15 * 60 * 1000, + }; + + repeatableTasks$.next([ + wrapWithSubTasks(yesterdayEdited), + wrapWithSubTasks(todayInstance), + ]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()) + .withContext('an instance with a user-edited estimate must be preserved') + .toEqual([]); + + sub.unsubscribe(); + })); + + it('keeps an overdue instance whose tags differ from the template', fakeAsync(() => { + repeatCfgs$.next([ + skipOverdueCfg('cfg-tags', '', { title: 'Water plants', tagIds: ['tag-a'] }), + ]); + const yesterdayEdited: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'tags-yesterday', + title: 'Water plants', + repeatCfgId: 'cfg-tags', + created: yesterdayMs, + dueDay: getDbDateStr(yesterdayMs), + isDone: false, + timeSpent: 0, + tagIds: ['tag-a', 'tag-user-added'], + }; + const todayInstance: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'tags-today', + title: 'Water plants', + repeatCfgId: 'cfg-tags', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + tagIds: ['tag-a'], + }; + + repeatableTasks$.next([ + wrapWithSubTasks(yesterdayEdited), + wrapWithSubTasks(todayInstance), + ]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()) + .withContext('an instance with user-edited tags must be preserved') + .toEqual([]); + + sub.unsubscribe(); + })); + + it('keeps an overdue instance whose subtask template was edited', fakeAsync(() => { + repeatCfgs$.next([ + skipOverdueCfg('cfg-subtasks', '', { + title: 'Water plants', + shouldInheritSubtasks: true, + subTaskTemplates: [{ title: 'Check soil', timeEstimate: 0, notes: '' }], + }), + ]); + const yesterdayEdited = wrapWithSubTasks({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'subtasks-yesterday', + title: 'Water plants', + repeatCfgId: 'cfg-subtasks', + created: yesterdayMs, + dueDay: getDbDateStr(yesterdayMs), + isDone: false, + timeSpent: 0, + } as Task); + yesterdayEdited.subTasks.push({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'edited-subtask', + title: 'Check dry soil', + parentId: 'subtasks-yesterday', + isDone: false, + timeSpent: 0, + } as Task); + const todayInstance = wrapWithSubTasks({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'subtasks-today', + title: 'Water plants', + repeatCfgId: 'cfg-subtasks', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + } as Task); + todayInstance.subTasks.push({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'template-subtask', + title: 'Check soil', + parentId: 'subtasks-today', + isDone: false, + timeSpent: 0, + } as Task); + + repeatableTasks$.next([yesterdayEdited, todayInstance]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()) + .withContext('an instance with user-edited subtasks must be preserved') + .toEqual([]); + + sub.unsubscribe(); + })); + + it('keeps an overdue instance whose subtask has user metadata', fakeAsync(() => { + repeatCfgs$.next([ + skipOverdueCfg('cfg-subtask-meta', '', { + title: 'Water plants', + shouldInheritSubtasks: true, + subTaskTemplates: [{ title: 'Check soil', timeEstimate: 0, notes: '' }], + }), + ]); + const yesterdayEdited = wrapWithSubTasks({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'subtask-meta-yesterday', + title: 'Water plants', + repeatCfgId: 'cfg-subtask-meta', + created: yesterdayMs, + dueDay: getDbDateStr(yesterdayMs), + isDone: false, + timeSpent: 0, + } as Task); + yesterdayEdited.subTasks.push({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'subtask-with-attachment', + title: 'Check soil', + parentId: 'subtask-meta-yesterday', + isDone: false, + timeSpent: 0, + attachments: [{ id: 'sub-a1', type: 'LINK', path: 'https://x', title: 'x' }], + } as Task); + const todayInstance = wrapWithSubTasks({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'subtask-meta-today', + title: 'Water plants', + repeatCfgId: 'cfg-subtask-meta', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + } as Task); + todayInstance.subTasks.push({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'subtask-template', + title: 'Check soil', + parentId: 'subtask-meta-today', + isDone: false, + timeSpent: 0, + } as Task); + + repeatableTasks$.next([yesterdayEdited, todayInstance]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()) + .withContext('a parent with subtask user metadata must be preserved') + .toEqual([]); + + sub.unsubscribe(); + })); + it("keeps yesterday's TRACKED instance even with skipOverdue (never destroy work)", fakeAsync(() => { repeatCfgs$.next([skipOverdueCfg('cfg-so2')]); const yesterdayTracked: Task = { @@ -461,6 +915,113 @@ describe('TaskRepeatCleanupEffects', () => { sub.unsubscribe(); })); + it('keeps an overdue instance whose subtask was completed by the user', fakeAsync(() => { + repeatCfgs$.next([ + skipOverdueCfg('cfg-sub-done', '', { + title: 'Water plants', + shouldInheritSubtasks: true, + subTaskTemplates: [{ title: 'Check soil', timeEstimate: 0, notes: '' }], + }), + ]); + const yesterdayWithDoneSub = wrapWithSubTasks({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'sub-done-yesterday', + title: 'Water plants', + repeatCfgId: 'cfg-sub-done', + created: yesterdayMs, + dueDay: getDbDateStr(yesterdayMs), + isDone: false, + timeSpent: 0, + } as Task); + // Title/notes/estimate match the template — only difference is the user + // marked the subtask done. hasSubtaskProgress upstream of the unmodified + // gate must keep the parent alive. + yesterdayWithDoneSub.subTasks.push({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'done-subtask', + title: 'Check soil', + parentId: 'sub-done-yesterday', + isDone: true, + timeSpent: 0, + } as Task); + const todayInstance = wrapWithSubTasks({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'sub-done-today', + title: 'Water plants', + repeatCfgId: 'cfg-sub-done', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + } as Task); + todayInstance.subTasks.push({ + ...DEFAULT_TASK, + projectId: 'p1', + id: 'today-subtask', + title: 'Check soil', + parentId: 'sub-done-today', + isDone: false, + timeSpent: 0, + } as Task); + + repeatableTasks$.next([yesterdayWithDoneSub, todayInstance]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()) + .withContext('a parent with a completed subtask must be preserved') + .toEqual([]); + + sub.unsubscribe(); + })); + + it('keeps an overdue instance the user moved to a different project', fakeAsync(() => { + // cfg has no projectId → the "did the user move it?" check falls back + // to comparing against the newest instance's projectId. Moving the older + // instance must look like a user edit and be preserved. + repeatCfgs$.next([skipOverdueCfg('cfg-moved', '', { title: 'Water plants' })]); + const yesterdayMoved: Task = { + ...DEFAULT_TASK, + projectId: 'p2', + id: 'moved-yesterday', + title: 'Water plants', + repeatCfgId: 'cfg-moved', + created: yesterdayMs, + dueDay: getDbDateStr(yesterdayMs), + isDone: false, + timeSpent: 0, + }; + const todayInstance: Task = { + ...DEFAULT_TASK, + projectId: 'p1', + id: 'moved-today', + title: 'Water plants', + repeatCfgId: 'cfg-moved', + created: todayMs, + dueDay: getDbDateStr(todayMs), + isDone: false, + timeSpent: 0, + }; + + repeatableTasks$.next([ + wrapWithSubTasks(yesterdayMoved), + wrapWithSubTasks(todayInstance), + ]); + + const sub = effects.cleanupDuplicateRepeatInstances$.subscribe(); + tick(3001); + + expect(getDispatchedDeleteIds()) + .withContext('an instance moved to a different project must be preserved') + .toEqual([]); + + sub.unsubscribe(); + })); + it('does not touch instances of configs WITHOUT skipOverdue (#7718 still holds)', fakeAsync(() => { // cfg present but skipOverdue is false -> default behavior, keep overdue repeatCfgs$.next([ diff --git a/src/app/features/task-repeat-cfg/store/task-repeat-cleanup.effects.ts b/src/app/features/task-repeat-cfg/store/task-repeat-cleanup.effects.ts index 02d3c83cea..a507458743 100644 --- a/src/app/features/task-repeat-cfg/store/task-repeat-cleanup.effects.ts +++ b/src/app/features/task-repeat-cfg/store/task-repeat-cleanup.effects.ts @@ -17,6 +17,98 @@ import { selectAllTaskRepeatCfgs } from './task-repeat-cfg.selectors'; import { DateService } from '../../../core/date/date.service'; import { Log } from '../../../core/log'; import { DeletedTaskIssueSidecarService } from '../../issue/two-way-sync/deleted-task-issue-sidecar.service'; +import { TODAY_TAG } from '../../tag/tag.const'; +import { isValidSplitTime } from '../../../util/is-valid-split-time'; +import { getDateTimeFromClockString } from '../../../util/get-date-time-from-clock-string'; +import { dateStrToUtcDate } from '../../../util/date-str-to-utc-date'; +import { remindOptionToMilliseconds } from '../../tasks/util/remind-option-to-milliseconds'; + +const _sameStringSet = (a: readonly string[], b: readonly string[]): boolean => { + if (a.length !== b.length) { + return false; + } + const sortedA = [...a].sort(); + const sortedB = [...b].sort(); + return sortedA.every((value, index) => value === sortedB[index]); +}; + +const _isNil = (v: unknown): boolean => v === null || v === undefined; + +const _hasNoDeadlineFields = ( + task: Pick, +): boolean => + _isNil(task.deadlineDay) && + _isNil(task.deadlineWithTime) && + _isNil(task.deadlineRemindAt); + +const _hasTemplateSchedule = ( + task: TaskWithSubTasks, + cfg: TaskRepeatCfg, + dueStr: string, +): boolean => { + if (isValidSplitTime(cfg.startTime)) { + const expectedDueWithTime = getDateTimeFromClockString( + cfg.startTime, + dateStrToUtcDate(dueStr), + ); + const expectedRemindAt = cfg.remindAt + ? remindOptionToMilliseconds(expectedDueWithTime, cfg.remindAt) + : undefined; + const isScheduledTemplate = + task.dueWithTime === expectedDueWithTime && + _isNil(task.dueDay) && + task.remindAt === expectedRemindAt; + const isBeforeScheduleActionTemplate = + _isNil(task.dueWithTime) && task.dueDay === dueStr && _isNil(task.remindAt); + + return isScheduledTemplate || isBeforeScheduleActionTemplate; + } + + return _isNil(task.dueWithTime) && task.dueDay === dueStr && _isNil(task.remindAt); +}; + +const _hasTemplateSubTasks = (task: TaskWithSubTasks, cfg: TaskRepeatCfg): boolean => { + const templates = cfg.shouldInheritSubtasks ? (cfg.subTaskTemplates ?? []) : []; + if (task.subTasks.length !== templates.length) { + return false; + } + return templates.every((template, index) => { + const subTask = task.subTasks[index]; + return ( + !!subTask && + subTask.title === template.title && + (subTask.timeEstimate ?? 0) === (template.timeEstimate ?? 0) && + (subTask.notes ?? '').trim() === (template.notes ?? '').trim() && + (subTask.attachments?.length ?? 0) === 0 && + _sameStringSet(subTask.tagIds ?? [], []) && + subTask.parentId === task.id && + subTask.projectId === (cfg.projectId || task.projectId) && + _isNil(subTask.remindAt) && + _hasNoDeadlineFields(subTask) + ); + }); +}; + +const _isUnmodifiedSkipOverdueInstance = ( + task: TaskWithSubTasks, + cfg: TaskRepeatCfg, + newestInstanceProjectId: string, + dueStr: string, +): boolean => + task.title === (cfg.title ?? '') && + (task.timeEstimate ?? 0) === (cfg.defaultEstimate ?? 0) && + _sameStringSet( + task.tagIds ?? [], + (cfg.tagIds ?? []).filter((tagId) => tagId !== TODAY_TAG.id), + ) && + (task.notes ?? '').trim() === (cfg.notes ?? '').trim() && + (task.attachments?.length ?? 0) === 0 && + (cfg.projectId + ? task.projectId === cfg.projectId + : task.projectId === newestInstanceProjectId) && + _hasTemplateSchedule(task, cfg, dueStr) && + _hasNoDeadlineFields(task) && + _hasTemplateSubTasks(task, cfg); @Injectable() export class TaskRepeatCleanupEffects { @@ -141,8 +233,12 @@ export class TaskRepeatCleanupEffects { // creation time than today's, so only remove instances that // are genuinely OVERDUE (due before today) — never today's or // a future instance. And never discard a prior-day instance - // the user actually edited (attachments or notes that differ - // from the template). + // the user actually edited. _isUnmodifiedSkipOverdueInstance + // compares: title, timeEstimate, tagIds (excluding TODAY_TAG), + // notes, attachments (must be empty), projectId, and the + // subtask templates (title/timeEstimate/notes). + // Subtask *progress* (completion / timeSpent) is caught + // upstream by hasSubtaskProgress before this gate runs. if (isSkipOverdueGroup) { const dueStr = task.dueWithTime ? getDbDateStr(task.dueWithTime) @@ -150,10 +246,15 @@ export class TaskRepeatCleanupEffects { if (!dueStr || dueStr >= todayStr) { continue; } - if (task.attachments?.length) { - continue; - } - if ((task.notes ?? '').trim() !== (cfg?.notes ?? '').trim()) { + if ( + !cfg || + !_isUnmodifiedSkipOverdueInstance( + task, + cfg, + tasks[0].projectId, + dueStr, + ) + ) { continue; } } diff --git a/src/app/features/tasks/add-task-bar/add-task-bar-actions/add-task-bar-actions.component.spec.ts b/src/app/features/tasks/add-task-bar/add-task-bar-actions/add-task-bar-actions.component.spec.ts index a1ed46cb6f..b62f98ea57 100644 --- a/src/app/features/tasks/add-task-bar/add-task-bar-actions/add-task-bar-actions.component.spec.ts +++ b/src/app/features/tasks/add-task-bar/add-task-bar-actions/add-task-bar-actions.component.spec.ts @@ -19,6 +19,7 @@ import { Store } from '@ngrx/store'; import { GlobalConfigService } from 'src/app/features/config/global-config.service'; import { DateTimeLocale, DateTimeLocales } from 'src/app/core/locale.constants'; import { DateService } from '../../../../core/date/date.service'; +import { TaskReminderOptionId } from '../../task.model'; const expectedLocaleTime = (timeStr: string, locale: string): string => { const [hours, minutes] = timeStr.split(':').map(Number); @@ -98,7 +99,10 @@ describe('AddTaskBarActionsComponent', () => { 'updateDate', 'updateEstimate', 'updateRemindOption', + 'updateDeadline', + 'updateDeadlineRemindOption', 'clearDate', + 'clearDeadline', 'clearTags', 'clearEstimate', 'toggleTag', @@ -1024,6 +1028,28 @@ describe('AddTaskBarActionsComponent', () => { }); describe('Schedule Dialog Timezone Handling', () => { + it('should pass the existing deadline reminder option when opening deadline dialog', () => { + const stateWithDeadline = { + ...mockState, + deadlineDate: '2025-07-20', + deadlineTime: '09:15', + deadlineRemindOption: TaskReminderOptionId.m30, + }; + (mockStateService as any)._mockStateSignal.set(stateWithDeadline); + fixture.detectChanges(); + + component.openDeadlineDialog(); + + expect(mockMatDialog.open).toHaveBeenCalledWith(jasmine.any(Function), { + data: { + targetDeadlineDay: '2025-07-20', + targetDeadlineTime: '09:15', + targetDeadlineRemindOption: TaskReminderOptionId.m30, + isSelectDeadlineOnly: true, + }, + }); + }); + it('should handle dialog results with dates from different timezones', () => { // Simulate a dialog result with a Date object that might come from a date picker const selectedDate = new Date(2025, 2, 15, 10, 30, 0); // March 15, 2025 at 10:30 AM diff --git a/src/app/features/tasks/add-task-bar/add-task-bar-actions/add-task-bar-actions.component.ts b/src/app/features/tasks/add-task-bar/add-task-bar-actions/add-task-bar-actions.component.ts index 61fa1e463b..b9da7bbe11 100644 --- a/src/app/features/tasks/add-task-bar/add-task-bar-actions/add-task-bar-actions.component.ts +++ b/src/app/features/tasks/add-task-bar/add-task-bar-actions/add-task-bar-actions.component.ts @@ -247,6 +247,7 @@ export class AddTaskBarActionsComponent { data: { targetDeadlineDay: state.deadlineDate || undefined, targetDeadlineTime: state.deadlineTime || undefined, + targetDeadlineRemindOption: state.deadlineRemindOption ?? undefined, isSelectDeadlineOnly: true, }, }); diff --git a/src/app/features/tasks/add-task-bar/add-task-bar-parser.service.spec.ts b/src/app/features/tasks/add-task-bar/add-task-bar-parser.service.spec.ts index a2a43a3762..fcf9daeda1 100644 --- a/src/app/features/tasks/add-task-bar/add-task-bar-parser.service.spec.ts +++ b/src/app/features/tasks/add-task-bar/add-task-bar-parser.service.spec.ts @@ -4,6 +4,7 @@ import { AddTaskBarStateService } from './add-task-bar-state.service'; import { ShortSyntaxConfig } from '../../config/global-config.model'; import { Project } from '../../project/project.model'; import { Tag } from '../../tag/tag.model'; +import { TaskReminderOptionId } from '../task.model'; describe('AddTaskBarParserService', () => { let service: AddTaskBarParserService; @@ -335,6 +336,209 @@ describe('AddTaskBarParserService', () => { expect(typeof deadlineDate).toBe('string'); expect(deadlineTime).toBe('12:00'); }); + + it('should preserve a manually selected deadline when unrelated syntax is parsed', async () => { + mockStateService.state.and.returnValue({ + projectId: null, + tagIds: [], + tagIdsFromTxt: [], + newTagTitles: [], + date: null, + time: null, + estimate: null, + cleanText: null, + deadlineDate: '2026-06-10', + deadlineTime: '10:30', + deadlineRemindOption: TaskReminderOptionId.m15, + } as any); + + await service.parseAndUpdateText( + 'Prepare report #urgent', + mockConfig, + mockProjects, + mockTags, + mockDefaultProject, + ); + + expect(mockStateService.updateDeadline).toHaveBeenCalledWith( + '2026-06-10', + '10:30', + ); + expect(mockStateService.updateDeadlineRemindOption).toHaveBeenCalledWith( + TaskReminderOptionId.m15, + ); + }); + + it('should preserve a user deadline added after a parsed deadline was already cleared', async () => { + // Run 1: parsed deadline syntax populates the deadline. + await service.parseAndUpdateText( + 'Do taxes !friday', + mockConfig, + mockProjects, + mockTags, + mockDefaultProject, + ); + + // Run 2: user removes the syntax — previousParseResult flips + // isDeadlineFromSyntax to false, so a manually-set deadline that + // appears in state on the NEXT run must be preserved. + mockStateService.updateDeadline.calls.reset(); + mockStateService.updateDeadlineRemindOption.calls.reset(); + mockStateService.state.and.returnValue({ + projectId: null, + tagIds: [], + tagIdsFromTxt: [], + newTagTitles: [], + date: null, + time: null, + estimate: null, + cleanText: null, + deadlineDate: null, + deadlineTime: null, + deadlineRemindOption: null, + } as any); + await service.parseAndUpdateText( + 'Do taxes', + mockConfig, + mockProjects, + mockTags, + mockDefaultProject, + ); + + // Run 3: user has now manually set a deadline via the dialog and + // adds more text WITHOUT introducing any deadline syntax. + mockStateService.updateDeadline.calls.reset(); + mockStateService.updateDeadlineRemindOption.calls.reset(); + mockStateService.state.and.returnValue({ + projectId: null, + tagIds: [], + tagIdsFromTxt: [], + newTagTitles: [], + date: null, + time: null, + estimate: null, + cleanText: null, + deadlineDate: '2026-07-01', + deadlineTime: '09:00', + deadlineRemindOption: TaskReminderOptionId.m15, + } as any); + await service.parseAndUpdateText( + 'Do taxes #urgent', + mockConfig, + mockProjects, + mockTags, + mockDefaultProject, + ); + + expect(mockStateService.updateDeadline).toHaveBeenCalledWith( + '2026-07-01', + '09:00', + ); + expect(mockStateService.updateDeadlineRemindOption).toHaveBeenCalledWith( + TaskReminderOptionId.m15, + ); + }); + + it('should clear a previously parsed deadline when deadline syntax is removed', async () => { + await service.parseAndUpdateText( + 'Do taxes !friday', + mockConfig, + mockProjects, + mockTags, + mockDefaultProject, + ); + const [parsedDeadlineDate, parsedDeadlineTime] = + mockStateService.updateDeadline.calls.mostRecent().args; + + mockStateService.updateDeadline.calls.reset(); + mockStateService.updateDeadlineRemindOption.calls.reset(); + mockStateService.state.and.returnValue({ + projectId: null, + tagIds: [], + tagIdsFromTxt: [], + newTagTitles: [], + date: null, + time: null, + estimate: null, + cleanText: null, + deadlineDate: parsedDeadlineDate, + deadlineTime: parsedDeadlineTime, + deadlineRemindOption: null, + } as any); + + await service.parseAndUpdateText( + 'Do taxes #urgent', + mockConfig, + mockProjects, + mockTags, + mockDefaultProject, + ); + + expect(mockStateService.updateDeadline).toHaveBeenCalledWith(null, null); + }); + + it('should clear a syntax-owned deadline when the input is cleared', async () => { + await service.parseAndUpdateText( + 'Do taxes !friday', + mockConfig, + mockProjects, + mockTags, + mockDefaultProject, + ); + + mockStateService.updateDeadline.calls.reset(); + mockStateService.updateDeadlineRemindOption.calls.reset(); + + await service.parseAndUpdateText( + '', + mockConfig, + mockProjects, + mockTags, + mockDefaultProject, + ); + + expect(mockStateService.updateDeadline).toHaveBeenCalledWith(null, null); + expect(mockStateService.updateDeadlineRemindOption).toHaveBeenCalledWith(null); + }); + + it('should clear a stale reminder option when deadline syntax is removed', async () => { + await service.parseAndUpdateText( + 'Do taxes !friday', + mockConfig, + mockProjects, + mockTags, + mockDefaultProject, + ); + const [parsedDeadlineDate, parsedDeadlineTime] = + mockStateService.updateDeadline.calls.mostRecent().args; + + mockStateService.updateDeadline.calls.reset(); + mockStateService.updateDeadlineRemindOption.calls.reset(); + mockStateService.state.and.returnValue({ + projectId: null, + tagIds: [], + tagIdsFromTxt: [], + newTagTitles: [], + date: null, + time: null, + estimate: null, + cleanText: null, + deadlineDate: parsedDeadlineDate, + deadlineTime: parsedDeadlineTime, + deadlineRemindOption: TaskReminderOptionId.m15, + } as any); + + await service.parseAndUpdateText( + 'Do taxes #urgent', + mockConfig, + mockProjects, + mockTags, + mockDefaultProject, + ); + + expect(mockStateService.updateDeadline).toHaveBeenCalledWith(null, null); + expect(mockStateService.updateDeadlineRemindOption).toHaveBeenCalledWith(null); + }); }); describe('Parsing Integration', () => { diff --git a/src/app/features/tasks/add-task-bar/add-task-bar-parser.service.ts b/src/app/features/tasks/add-task-bar/add-task-bar-parser.service.ts index 8904c5398b..1ba9cb51f6 100644 --- a/src/app/features/tasks/add-task-bar/add-task-bar-parser.service.ts +++ b/src/app/features/tasks/add-task-bar/add-task-bar-parser.service.ts @@ -22,6 +22,7 @@ interface PreviousParseResult { deadlineDate: string | null; deadlineTime: string | null; deadlineRemindOption: TaskReminderOptionId | null; + isDeadlineFromSyntax: boolean; } @Injectable() @@ -51,7 +52,16 @@ export class AddTaskBarParserService { ): Promise { const parseRunId = ++this._parseRunId; - if (!text || !config) { + if (!text) { + if (this._previousParseResult?.isDeadlineFromSyntax) { + this._stateService.updateDeadline(null, null); + this._stateService.updateDeadlineRemindOption(null); + } + this._previousParseResult = null; + return; + } + + if (!config) { this._previousParseResult = null; return; } @@ -74,6 +84,12 @@ export class AddTaskBarParserService { // Create current parse result data structure let currentResult: PreviousParseResult; + // On the first run _previousParseResult is null. Treat "no previous run" + // as "user owns the deadline" so an existing user-set deadline isn't + // wiped on first parse. + const wasDeadlineFromSyntax = + this._previousParseResult?.isDeadlineFromSyntax ?? false; + if (!parseResult) { // No parse result means no short syntax found // Preserve current user-selected values instead of falling back to defaults @@ -91,9 +107,12 @@ export class AddTaskBarParserService { dueDate: currentState.date || (defaultDate ? defaultDate : null), dueTime: currentState.time || defaultTime || null, attachments: [], - deadlineDate: currentState.deadlineDate || null, - deadlineTime: currentState.deadlineTime || null, - deadlineRemindOption: currentState.deadlineRemindOption || null, + deadlineDate: wasDeadlineFromSyntax ? null : currentState.deadlineDate || null, + deadlineTime: wasDeadlineFromSyntax ? null : currentState.deadlineTime || null, + deadlineRemindOption: wasDeadlineFromSyntax + ? null + : currentState.deadlineRemindOption || null, + isDeadlineFromSyntax: false, }; } else { // Extract parsed values @@ -124,6 +143,9 @@ export class AddTaskBarParserService { let deadlineDate: string | null = null; let deadlineTime: string | null = null; let deadlineRemindOption: TaskReminderOptionId | null = null; + const hasParsedDeadline = + parseResult.taskChanges.deadlineWithTime !== undefined || + parseResult.taskChanges.deadlineDay !== undefined; if (parseResult.taskChanges.deadlineWithTime) { const deadlineDateObj = new Date(parseResult.taskChanges.deadlineWithTime); @@ -147,6 +169,10 @@ export class AddTaskBarParserService { } } else if (parseResult.taskChanges.deadlineDay) { deadlineDate = parseResult.taskChanges.deadlineDay; + } else if (!wasDeadlineFromSyntax) { + deadlineDate = currentState.deadlineDate || null; + deadlineTime = currentState.deadlineTime || null; + deadlineRemindOption = currentState.deadlineRemindOption || null; } currentResult = { @@ -162,6 +188,7 @@ export class AddTaskBarParserService { deadlineDate: deadlineDate, deadlineTime: deadlineTime, deadlineRemindOption: deadlineRemindOption, + isDeadlineFromSyntax: hasParsedDeadline, }; } @@ -267,7 +294,8 @@ export class AddTaskBarParserService { if ( !this._previousParseResult || this._previousParseResult.deadlineRemindOption !== - currentResult.deadlineRemindOption + currentResult.deadlineRemindOption || + currentState.deadlineRemindOption !== currentResult.deadlineRemindOption ) { this._stateService.updateDeadlineRemindOption(currentResult.deadlineRemindOption); } diff --git a/src/app/features/tasks/dialog-deadline/dialog-deadline.component.spec.ts b/src/app/features/tasks/dialog-deadline/dialog-deadline.component.spec.ts index e148822e09..5986ca96b5 100644 --- a/src/app/features/tasks/dialog-deadline/dialog-deadline.component.spec.ts +++ b/src/app/features/tasks/dialog-deadline/dialog-deadline.component.spec.ts @@ -4,7 +4,7 @@ import { provideMockStore, MockStore } from '@ngrx/store/testing'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { TranslateModule, TranslateService, TranslateStore } from '@ngx-translate/core'; import { DateAdapter } from '@angular/material/core'; -import { DEFAULT_TASK, Task } from '../task.model'; +import { DEFAULT_TASK, Task, TaskReminderOptionId } from '../task.model'; import { DialogDeadlineComponent } from './dialog-deadline.component'; import { TaskSharedActions } from '../../../root-store/meta/task-shared.actions'; import { DateService } from '../../../core/date/date.service'; @@ -51,6 +51,15 @@ describe('DialogDeadlineComponent.submit() input validation', () => { return TestBed.createComponent(DialogDeadlineComponent).componentInstance; }; + const createSelectOnlyComponent = (): DialogDeadlineComponent => { + TestBed.overrideProvider(MAT_DIALOG_DATA, { + useValue: { isSelectDeadlineOnly: true }, + }); + store = TestBed.inject(MockStore); + dispatchSpy = spyOn(store, 'dispatch').and.callThrough(); + return TestBed.createComponent(DialogDeadlineComponent).componentInstance; + }; + beforeEach(async () => { matDialogRefSpy = jasmine.createSpyObj('MatDialogRef', ['close']); // ngAfterViewInit reads viewChild.required(MatCalendar). With the template @@ -119,6 +128,26 @@ describe('DialogDeadlineComponent.submit() input validation', () => { expect(calls[0].autoPlanToday).toBe('2026-05-06'); }); + // Mirrors the non-select-only parameterized test below — same garbage + // inputs must round-trip to `time: null` instead of throwing or persisting + // a malformed string back to the caller. + ['abc', '13:60', '25:00'].forEach((badTime) => { + it(`returns null time for malformed select-only deadline time "${badTime}"`, () => { + const component = createSelectOnlyComponent(); + component.selectedDate = new Date(2026, 4, 6); + component.selectedTime = badTime; + + expect(() => component.submit()).not.toThrow(); + + expect(dispatchSpy).not.toHaveBeenCalled(); + expect(matDialogRefSpy.close).toHaveBeenCalledWith({ + date: component.selectedDate, + time: null, + remindOption: TaskReminderOptionId.DoNotRemind, + }); + }); + }); + // Proves issue #7490. submit() must not throw on a malformed selectedTime and // must fall back to a date-only deadline for genuinely invalid values. // Note: '1:' is intentionally NOT in this list — isValidSplitTime parses it diff --git a/src/app/features/tasks/dialog-deadline/dialog-deadline.component.ts b/src/app/features/tasks/dialog-deadline/dialog-deadline.component.ts index cdb503b61a..763270af6c 100644 --- a/src/app/features/tasks/dialog-deadline/dialog-deadline.component.ts +++ b/src/app/features/tasks/dialog-deadline/dialog-deadline.component.ts @@ -85,6 +85,7 @@ export class DialogDeadlineComponent implements AfterViewInit { task?: Task; targetDeadlineDay?: string; targetDeadlineTime?: string; + targetDeadlineRemindOption?: TaskReminderOptionId; isSelectDeadlineOnly?: boolean; }>(MAT_DIALOG_DATA); private _matDialogRef = inject>(MatDialogRef); @@ -149,7 +150,11 @@ export class DialogDeadlineComponent implements AfterViewInit { } } else { this.selectedReminderCfgId = this._defaultTaskRemindCfgId(); - if (this.data.targetDeadlineDay || this.data.targetDeadlineTime) { + if ( + this.data.targetDeadlineDay || + this.data.targetDeadlineTime || + this.data.targetDeadlineRemindOption + ) { this.hasExistingDeadline = true; } } @@ -160,6 +165,12 @@ export class DialogDeadlineComponent implements AfterViewInit { if (this.data.targetDeadlineTime) { this.selectedTime = this.data.targetDeadlineTime; } + if ( + this.data.isSelectDeadlineOnly && + this.data.targetDeadlineRemindOption !== undefined + ) { + this.selectedReminderCfgId = this.data.targetDeadlineRemindOption; + } this.calendar().activeDate = new Date(this.selectedDate || new Date()); this._cd.detectChanges(); @@ -264,9 +275,10 @@ export class DialogDeadlineComponent implements AfterViewInit { const time = this.selectedTime ? normalizeClockStr(this.selectedTime) : null; if (this.data.isSelectDeadlineOnly) { + const validTime = time && isValidSplitTime(time) ? time : null; this._matDialogRef.close({ date: this.selectedDate, - time, + time: validTime, remindOption: this.selectedReminderCfgId, }); return; diff --git a/src/app/imex/sync/sync-wrapper.service.spec.ts b/src/app/imex/sync/sync-wrapper.service.spec.ts index 5289a705f5..c4beacf0b9 100644 --- a/src/app/imex/sync/sync-wrapper.service.spec.ts +++ b/src/app/imex/sync/sync-wrapper.service.spec.ts @@ -2205,6 +2205,153 @@ describe('SyncWrapperService', () => { expect(result).toBe(SyncStatus.UpdateRemote); }); + it('should pass the pre-sync never-synced snapshot into LWW re-upload retries', async () => { + mockSyncService.hasSyncedOps.and.resolveTo(false); + mockSyncService.downloadRemoteOps.and.returnValue( + Promise.resolve({ + kind: 'no_new_ops' as const, + }), + ); + + let uploadCallCount = 0; + mockSyncService.uploadPendingOps.and.callFake(async () => { + uploadCallCount++; + return { + kind: 'completed' as const, + uploadedCount: 1, + piggybackedOpsCount: 0, + localWinOpsCreated: uploadCallCount === 1 ? 1 : 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], + }; + }); + + await service.sync(); + + expect(mockSyncService.uploadPendingOps).toHaveBeenCalledTimes(2); + expect(mockSyncService.uploadPendingOps.calls.argsFor(0)[1]).toEqual({ + isNeverSynced: true, + }); + expect(mockSyncService.uploadPendingOps.calls.argsFor(1)[1]).toEqual({ + isNeverSynced: true, + }); + }); + + it('should stop sync when an LWW re-upload is cancelled', async () => { + mockSyncService.downloadRemoteOps.and.returnValue( + Promise.resolve({ + kind: 'no_new_ops' as const, + }), + ); + + let uploadCallCount = 0; + mockSyncService.uploadPendingOps.and.callFake(async () => { + uploadCallCount++; + if (uploadCallCount === 1) { + return { + kind: 'completed' as const, + uploadedCount: 1, + piggybackedOpsCount: 0, + localWinOpsCreated: 1, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], + }; + } + return { kind: 'cancelled' as const }; + }); + + const result = await service.sync(); + + expect(result).toBe('HANDLED_ERROR'); + expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith( + 'UNKNOWN_OR_CHANGED', + ); + expect(mockProviderManager.setSyncStatus).not.toHaveBeenCalledWith('IN_SYNC'); + }); + + it('should stop sync when an LWW re-upload has permanent rejections', async () => { + mockSyncService.downloadRemoteOps.and.returnValue( + Promise.resolve({ + kind: 'no_new_ops' as const, + }), + ); + + let uploadCallCount = 0; + mockSyncService.uploadPendingOps.and.callFake(async () => { + uploadCallCount++; + if (uploadCallCount === 1) { + return { + kind: 'completed' as const, + uploadedCount: 1, + piggybackedOpsCount: 0, + localWinOpsCreated: 1, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], + }; + } + return { + kind: 'completed' as const, + uploadedCount: 0, + piggybackedOpsCount: 0, + localWinOpsCreated: 0, + permanentRejectionCount: 1, + hasMorePiggyback: false, + rejectedOps: [{ opId: 'lww-op', error: 'Validation failed' }], + }; + }); + + const result = await service.sync(); + + expect(mockSyncService.uploadPendingOps).toHaveBeenCalledTimes(2); + expect(result).toBe('HANDLED_ERROR'); + expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR'); + expect(mockProviderManager.setSyncStatus).not.toHaveBeenCalledWith('IN_SYNC'); + }); + + it('should stop sync when LWW re-upload is cancelled and the pending ops came from download', async () => { + // downloadResult.localWinOpsCreated > 0 means LWW work originated from + // the download path. The initial upload produces no LWW ops, so the + // very first retry is what cancels — guard against a regression where + // the cancel-path only triggers on upload-originated LWW work. + mockSyncService.downloadRemoteOps.and.returnValue( + Promise.resolve({ + kind: 'ops_processed' as const, + newOpsCount: 5, + localWinOpsCreated: 2, + }), + ); + + let uploadCallCount = 0; + mockSyncService.uploadPendingOps.and.callFake(async () => { + uploadCallCount++; + if (uploadCallCount === 1) { + return { + kind: 'completed' as const, + uploadedCount: 1, + piggybackedOpsCount: 0, + localWinOpsCreated: 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], + }; + } + return { kind: 'cancelled' as const }; + }); + + const result = await service.sync(); + + // initial upload + 1 retry that cancels = 2 calls, no further retries + expect(mockSyncService.uploadPendingOps).toHaveBeenCalledTimes(2); + expect(result).toBe('HANDLED_ERROR'); + expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith( + 'UNKNOWN_OR_CHANGED', + ); + expect(mockProviderManager.setSyncStatus).not.toHaveBeenCalledWith('IN_SYNC'); + }); + it('should treat blocked_fresh_client reupload result as 0 localWinOpsCreated and exit loop', async () => { mockSyncService.downloadRemoteOps.and.returnValue( Promise.resolve({ diff --git a/src/app/imex/sync/sync-wrapper.service.ts b/src/app/imex/sync/sync-wrapper.service.ts index 9032e4cdb7..09ff196f40 100644 --- a/src/app/imex/sync/sync-wrapper.service.ts +++ b/src/app/imex/sync/sync-wrapper.service.ts @@ -79,6 +79,9 @@ import { WrappedProviderService } from '../../op-log/sync-providers/wrapped-prov import { isSuperSyncWebSocketAccess } from '@sp/sync-providers/super-sync'; import { isTransientNetworkError } from '@sp/sync-providers/http'; import { HydrationStateService } from '../../op-log/apply/hydration-state.service'; +import type { UploadOutcome } from '../../op-log/core/types/sync-results.types'; + +type CompletedUploadOutcome = Extract; /** * Identifies which error or UI path triggered a destructive forceUpload. @@ -494,6 +497,8 @@ export class SyncWrapperService { syncCapableProvider, { isNeverSynced: isNeverSyncedAtSyncStart }, ); + const completedUploadResults: CompletedUploadOutcome[] = + uploadResult.kind === 'completed' ? [uploadResult] : []; if (uploadResult.kind === 'completed') { SyncLog.log( `SyncWrapperService: Upload complete. uploaded=${uploadResult.uploadedCount}, piggybacked=${uploadResult.piggybackedOpsCount}`, @@ -522,8 +527,28 @@ export class SyncWrapperService { `SyncWrapperService: Re-uploading ${pendingLwwOps} local-win op(s) from LWW ` + `(attempt ${lwwRetries}/${MAX_LWW_REUPLOAD_RETRIES})...`, ); - const reuploadResult = - await this._opLogSyncService.uploadPendingOps(syncCapableProvider); + // Re-thread isNeverSyncedAtSyncStart (the snapshot captured BEFORE the + // initial upload ran) instead of letting uploadPendingOps re-read live + // state — the initial batch has already flipped hasSyncedOps() to true + // and a live read here would mis-classify a still-fresh client. Mirrors + // the orchestrator-snapshot rationale at the top of uploadPendingOps. + const reuploadResult = await this._opLogSyncService.uploadPendingOps( + syncCapableProvider, + { isNeverSynced: isNeverSyncedAtSyncStart }, + ); + if (reuploadResult.kind === 'cancelled') { + // Mirror the initial-upload cancel path: a cancelled LWW re-upload + // means downloaded localWinOpsCreated stay pending in the op-log. + // UNKNOWN_OR_CHANGED forces a retry on the next sync tick. + SyncLog.log( + 'SyncWrapperService: LWW re-upload cancelled by user. Skipping remaining sync work.', + ); + this._providerManager.setSyncStatus('UNKNOWN_OR_CHANGED'); + return 'HANDLED_ERROR'; + } + if (reuploadResult.kind === 'completed') { + completedUploadResults.push(reuploadResult); + } pendingLwwOps = reuploadResult.kind === 'completed' ? reuploadResult.localWinOpsCreated : 0; } @@ -542,6 +567,9 @@ export class SyncWrapperService { this._providerManager.setSyncStatus('ERROR'); return 'HANDLED_ERROR'; } + if (this._handlePermanentUploadRejections(completedUploadResults)) { + return 'HANDLED_ERROR'; + } // Don't claim IN_SYNC — there are known unuploaded ops. this._providerManager.setSyncStatus('UNKNOWN_OR_CHANGED'); return SyncStatus.UpdateRemote; @@ -559,28 +587,7 @@ export class SyncWrapperService { } // 4. Check for permanent rejection failures - if (uploadResult.kind === 'completed' && uploadResult.permanentRejectionCount > 0) { - const hasPayloadError = uploadResult.rejectedOps.some( - (r) => - r.error?.includes('Payload too complex') || - r.error?.includes('Payload too large'), - ); - - if (hasPayloadError) { - SyncLog.err( - 'SyncWrapperService: Upload rejected - payload too large/complex', - uploadResult.rejectedOps, - ); - this._providerManager.setSyncStatus('ERROR'); - alertDialog(this._translateService.instant(T.F.SYNC.S.ERROR_PAYLOAD_TOO_LARGE)); - return 'HANDLED_ERROR'; - } - - SyncLog.err( - `SyncWrapperService: Upload had ${uploadResult.permanentRejectionCount} permanent rejection(s), not marking as IN_SYNC`, - uploadResult.rejectedOps, - ); - this._providerManager.setSyncStatus('ERROR'); + if (this._handlePermanentUploadRejections(completedUploadResults)) { return 'HANDLED_ERROR'; } @@ -895,6 +902,38 @@ export class SyncWrapperService { } } + private _handlePermanentUploadRejections( + uploadResults: readonly CompletedUploadOutcome[], + ): boolean { + const rejectedResult = uploadResults.find((r) => r.permanentRejectionCount > 0); + if (!rejectedResult) { + return false; + } + + const hasPayloadError = rejectedResult.rejectedOps.some( + (r) => + r.error?.includes('Payload too complex') || + r.error?.includes('Payload too large'), + ); + + if (hasPayloadError) { + SyncLog.err( + 'SyncWrapperService: Upload rejected - payload too large/complex', + rejectedResult.rejectedOps, + ); + this._providerManager.setSyncStatus('ERROR'); + alertDialog(this._translateService.instant(T.F.SYNC.S.ERROR_PAYLOAD_TOO_LARGE)); + return true; + } + + SyncLog.err( + `SyncWrapperService: Upload had ${rejectedResult.permanentRejectionCount} permanent rejection(s), not marking as IN_SYNC`, + rejectedResult.rejectedOps, + ); + this._providerManager.setSyncStatus('ERROR'); + return true; + } + async forceUpload(triggerSource: ForceUploadTriggerSource = 'unknown'): Promise { if (!this._c(this._translateService.instant(T.F.SYNC.C.FORCE_UPLOAD))) { return; diff --git a/src/app/op-log/sync/operation-log-sync.service.spec.ts b/src/app/op-log/sync/operation-log-sync.service.spec.ts index ae06333496..c2f1b34e6e 100644 --- a/src/app/op-log/sync/operation-log-sync.service.spec.ts +++ b/src/app/op-log/sync/operation-log-sync.service.spec.ts @@ -417,7 +417,7 @@ describe('OperationLogSyncService', () => { }), ); - await service.uploadPendingOps(mockProvider); + await service.uploadPendingOps(mockProvider, { isNeverSynced: true }); expect(rejectedOpsHandlerServiceSpy.handleRejectedOps).toHaveBeenCalledWith( [{ opId: 'local-op-1', error: 'Some error', errorCode: 'VALIDATION_ERROR' }], @@ -426,8 +426,10 @@ describe('OperationLogSyncService', () => { }); it('should pass download callback that calls downloadRemoteOps', async () => { - uploadServiceSpy.uploadPendingOps.and.returnValue( - Promise.resolve({ + opLogStoreSpy.hasSyncedOps.and.resolveTo(false); + uploadServiceSpy.uploadPendingOps.and.callFake(async () => { + opLogStoreSpy.hasSyncedOps.and.resolveTo(true); + return { uploadedCount: 0, piggybackedOps: [], rejectedCount: 1, @@ -438,8 +440,8 @@ describe('OperationLogSyncService', () => { errorCode: 'CONFLICT_CONCURRENT', }, ], - }), - ); + }; + }); // Capture the callback passed to handleRejectedOps let capturedCallback: any; @@ -463,11 +465,16 @@ describe('OperationLogSyncService', () => { // Call the callback and verify it delegates to downloadRemoteOps await capturedCallback(); - expect(downloadSpy).toHaveBeenCalledWith(mockProvider, undefined); + expect(downloadSpy).toHaveBeenCalledWith(mockProvider, { + isNeverSynced: true, + }); // Test with forceFromSeq0 option await capturedCallback({ forceFromSeq0: true }); - expect(downloadSpy).toHaveBeenCalledWith(mockProvider, { forceFromSeq0: true }); + expect(downloadSpy).toHaveBeenCalledWith(mockProvider, { + forceFromSeq0: true, + isNeverSynced: true, + }); }); it('should add mergedOpsFromRejection to localWinOpsCreated in result', async () => { @@ -1174,6 +1181,54 @@ describe('OperationLogSyncService', () => { await expectAsync(service.downloadRemoteOps(mockProvider)).toBeResolved(); expect(syncHydrationServiceSpy.hydrateFromRemoteSync).toHaveBeenCalled(); + expect(opLogStoreSpy.markRejected).toHaveBeenCalledWith(['ex-op-ex-task-1']); + // markRejected must run AFTER hydrateFromRemoteSync — otherwise a + // hydration failure would drop the example ops while leaving the + // user without the remote snapshot. + const hydrateOrder = ( + syncHydrationServiceSpy.hydrateFromRemoteSync.calls.mostRecent() as unknown as { + invocationOrder: number; + } + ).invocationOrder; + const markRejectedOrder = ( + opLogStoreSpy.markRejected.calls.mostRecent() as unknown as { + invocationOrder: number; + } + ).invocationOrder; + expect(markRejectedOrder).toBeGreaterThan(hydrateOrder); + }); + + it('does NOT call markRejected when hydrateFromRemoteSync rejects (#7985)', async () => { + opLogStoreSpy.getUnsynced.and.returnValue( + Promise.resolve([exampleCreateEntry('ex-task-1')]), + ); + stateSnapshotServiceSpy.getStateSnapshot.and.returnValue({ + task: { ids: ['ex-task-1'] }, + project: { ids: [INBOX_PROJECT.id] }, + tag: { ids: [TODAY_TAG.id] }, + note: { ids: [] }, + } as any); + + const syncHydrationServiceSpy = TestBed.inject( + SyncHydrationService, + ) as jasmine.SpyObj; + syncHydrationServiceSpy.hydrateFromRemoteSync.and.rejectWith( + new Error('hydrate failed'), + ); + + downloadServiceSpy.downloadRemoteOps.and.returnValue( + Promise.resolve(fileSnapshotDownloadResult as any), + ); + + const mockProvider = { + isReady: () => Promise.resolve(true), + supportsOperationSync: true, + setLastServerSeq: jasmine.createSpy('setLastServerSeq').and.resolveTo(), + } as any; + + await expectAsync(service.downloadRemoteOps(mockProvider)).toBeRejected(); + expect(syncHydrationServiceSpy.hydrateFromRemoteSync).toHaveBeenCalled(); + expect(opLogStoreSpy.markRejected).not.toHaveBeenCalled(); }); it('still throws LocalDataConflictError when a real task exists alongside example tasks (#7985)', async () => { diff --git a/src/app/op-log/sync/operation-log-sync.service.ts b/src/app/op-log/sync/operation-log-sync.service.ts index fd9018fb43..c98df25727 100644 --- a/src/app/op-log/sync/operation-log-sync.service.ts +++ b/src/app/op-log/sync/operation-log-sync.service.ts @@ -308,7 +308,10 @@ export class OperationLogSyncService { const downloadCallback = async (downloadOptions?: { forceFromSeq0?: boolean; }): Promise => { - const outcome = await this.downloadRemoteOps(syncProvider, downloadOptions); + const outcome = await this.downloadRemoteOps(syncProvider, { + ...downloadOptions, + isNeverSynced: isNeverSyncedAtSyncStart, + }); // Validation failure (if any during the nested download) is on the // session-validation latch — no need to thread the boolean back. (#7330) switch (outcome.kind) { @@ -468,6 +471,11 @@ export class OperationLogSyncService { const unsyncedOps = await this.opLogStore.getUnsynced(); const hasLocalChanges = unsyncedOps.length > 0; + // Collected here, applied AFTER hydrateFromRemoteSync succeeds so a + // hydration failure doesn't permanently drop the pending example-create + // ops while leaving the user without the remote snapshot. + let exampleTaskOpIdsToDiscard: string[] = []; + if (hasLocalChanges) { // Throw LocalDataConflictError if unsynced ops contain meaningful user data // OR if the NgRx store has meaningful data (tasks, projects, tags, notes). @@ -484,12 +492,13 @@ export class OperationLogSyncService { // while the example create ops are still pending (a never-synced file client) — // exactly the reachable scenario. A real (non-example) task / non-INBOX project / // non-system tag / note still reads as meaningful and shows the dialog. + const exampleTaskEntries = unsyncedOps.filter(isExampleTaskCreateOp); const exampleTaskIds = new Set( - unsyncedOps - .filter(isExampleTaskCreateOp) + exampleTaskEntries .map((entry) => entry.op.entityId) .filter((id): id is string => id !== undefined), ); + const exampleTaskOpIds = exampleTaskEntries.map((entry) => entry.op.id); const hasMeaningfulUserData = this.syncImportConflictGateService.hasMeaningfulPendingOps(unsyncedOps) || this.syncLocalStateService.hasMeaningfulStoreData(exampleTaskIds); @@ -508,6 +517,9 @@ export class OperationLogSyncService { result.snapshotVectorClock, ); } else { + // Defer the markRejected call until hydration has succeeded — see + // the declaration of exampleTaskOpIdsToDiscard above for rationale. + exampleTaskOpIdsToDiscard = exampleTaskOpIds; // Only system/config ops AND no meaningful store data - proceed with download OpLog.normal( `OperationLogSyncService: Client has ${unsyncedOps.length} unsynced ops but no meaningful user data. ` + @@ -568,6 +580,12 @@ export class OperationLogSyncService { false, // Don't create SYNC_IMPORT for file-based bootstrap ); + // Now that the remote snapshot is applied, it's safe to drop the + // example-create ops we previously decided were obsolete. Doing this + // after hydration ensures a hydration failure leaves the queue intact + // so the next attempt can retry. + await this._discardExampleTaskOps(exampleTaskOpIdsToDiscard); + // CRITICAL FIX: Write recentOps to IndexedDB after snapshot hydration. // File-based providers return ALL recentOps on every download, relying on // getAppliedOpIds() (from IndexedDB) to filter already-applied ops.