From d1d6949f4391b8d5a85df7e9d87b17cae86017da Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Tue, 23 Jun 2026 13:02:22 +0200 Subject: [PATCH] fix(reminders): stop dismissed reminders from re-opening every 10s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An overdue scheduled reminder kept re-opening its modal every ~10s (the reminder worker's check interval) and on every app resume. Dismissing the dialog via backdrop / Escape / Android back runs none of the clear logic (ngOnDestroy only clears deadline reminders), so the worker re-emitted the still-active reminder and the module re-opened the modal indefinitely. On mobile this reads as a fully frozen app where no controls respond. Add a short in-memory UI cooldown: on a passive dismiss, scheduled reminders are suppressed from re-opening for 5 minutes. The reminder itself stays active — it re-nudges after the cooldown and on cold start — and explicit actions (snooze / done / add-to-today / reschedule) are unaffected. The cooldown is presentation-only; no synced state is touched. Related to #8551 (a dropped done-from-notification leaves a task overdue and active), the common way to reach this state on Android. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../reminders/reminders-dismiss-loop.spec.ts | 56 +++++++++++++++++++ .../features/reminder/reminder.module.spec.ts | 16 ++++-- src/app/features/reminder/reminder.module.ts | 13 ++++- .../reminder/reminder.service.spec.ts | 37 +++++++++++- src/app/features/reminder/reminder.service.ts | 37 ++++++++++++ ...alog-view-task-reminders.component.spec.ts | 56 +++++++++++++++++-- .../dialog-view-task-reminders.component.ts | 13 +++++ 7 files changed, 213 insertions(+), 15 deletions(-) create mode 100644 e2e/tests/reminders/reminders-dismiss-loop.spec.ts diff --git a/e2e/tests/reminders/reminders-dismiss-loop.spec.ts b/e2e/tests/reminders/reminders-dismiss-loop.spec.ts new file mode 100644 index 0000000000..8e5c078871 --- /dev/null +++ b/e2e/tests/reminders/reminders-dismiss-loop.spec.ts @@ -0,0 +1,56 @@ +import { expect, test } from '../../fixtures/test.fixture'; +import { addTaskWithReminder } from '../../utils/schedule-task-helper'; + +// Repro for: "had some problems where no controls were working" (Android, SuperSync). +// Session log showed two OVERDUE scheduled reminders re-opening the reminder modal +// every ~10s and on every app resume. Dismissing the modal via backdrop / Android +// back button (== Escape) runs none of the dialog's clear logic — ngOnDestroy only +// clears DEADLINE reminders, never scheduled ones — so the worker +// (reminder.worker.ts CHECK_INTERVAL_DURATION = 10000) keeps re-emitting the overdue +// reminder and reminder.module.ts re-opens the modal (its only guard is +// openDialogs.length === 0). The modal repeatedly seizing the screen reads, on +// device, as a fully frozen app where no controls respond. + +const DIALOG = 'dialog-view-task-reminder'; +const DIALOG_TASK1 = `${DIALOG} .task:first-of-type`; +const SCHEDULE_MAX_WAIT_TIME = 60000; +// The reminder worker re-checks every 10s; wait out more than one full cycle. +const WORKER_RECHECK_WINDOW = 13000; + +test.describe('Reminders dismiss loop', () => { + test('should not re-open a scheduled reminder after it is dismissed without acting', async ({ + page, + workViewPage, + testPrefix, + }) => { + test.setTimeout(SCHEDULE_MAX_WAIT_TIME + 60000); + + await workViewPage.waitForTaskList(); + + // A scheduled (non-deadline) reminder due ~now: minute-granular time input + // rounds it to the current minute, so it is overdue the instant it fires — + // the exact state from the bug log. + const taskTitle = `${testPrefix}-0 dismiss-loop task`; + await addTaskWithReminder(page, workViewPage, taskTitle, Date.now() + 8000); + + // The reminder fires and the modal appears. + const dialog = page.locator(DIALOG); + await dialog.waitFor({ state: 'visible', timeout: SCHEDULE_MAX_WAIT_TIME }); + await expect(page.locator(DIALOG_TASK1)).toContainText(taskTitle); + + // The user dismisses it without acting (backdrop tap / Android back == Escape). + await page.keyboard.press('Escape'); + await dialog.waitFor({ state: 'hidden', timeout: 10000 }); + + // EXPECTED (correct behaviour, currently fails): a reminder the user has + // actively dismissed must not re-seize the screen on its own. With the bug the + // worker re-emits within ~10s and the modal reopens, so this assertion fails + // because the dialog reappears inside the re-check window. + const reappeared = await dialog + .waitFor({ state: 'visible', timeout: WORKER_RECHECK_WINDOW }) + .then(() => true) + .catch(() => false); + + expect(reappeared).toBe(false); + }); +}); diff --git a/src/app/features/reminder/reminder.module.spec.ts b/src/app/features/reminder/reminder.module.spec.ts index 1e55c7f464..41a6c5f0db 100644 --- a/src/app/features/reminder/reminder.module.spec.ts +++ b/src/app/features/reminder/reminder.module.spec.ts @@ -49,9 +49,11 @@ describe('ReminderModule dialog opening', () => { ReminderModule, { provide: ReminderService, - useValue: jasmine.createSpyObj('ReminderService', ['init'], { - onRemindersActive$: remindersActive$, - }), + useValue: jasmine.createSpyObj( + 'ReminderService', + ['init', 'isReminderUiSuppressed', 'suppressReminderUiAfterDismiss'], + { onRemindersActive$: remindersActive$ }, + ), }, { provide: MatDialog, useValue: matDialogSpy }, { @@ -159,9 +161,11 @@ describe('ReminderModule iOS notification actions', () => { ReminderModule, { provide: ReminderService, - useValue: jasmine.createSpyObj('ReminderService', ['init'], { - onRemindersActive$: NEVER, - }), + useValue: jasmine.createSpyObj( + 'ReminderService', + ['init', 'isReminderUiSuppressed', 'suppressReminderUiAfterDismiss'], + { onRemindersActive$: NEVER }, + ), }, { provide: MatDialog, diff --git a/src/app/features/reminder/reminder.module.ts b/src/app/features/reminder/reminder.module.ts index 9d98b7050f..9a5e9f4d0c 100644 --- a/src/app/features/reminder/reminder.module.ts +++ b/src/app/features/reminder/reminder.module.ts @@ -77,12 +77,19 @@ export class ReminderModule { delay(1000), concatMap(() => this._reminderService.onRemindersActive$.pipe( + // Drop reminders the user dismissed without acting on (backdrop / + // Escape / Android back): they are in a short UI cooldown so the + // worker (~10s tick) cannot immediately re-open the modal and freeze + // the app. The cooldown is in-memory only — a cold start re-nudges. + map((reminders) => + (reminders || []).filter( + (r) => !this._reminderService.isReminderUiSuppressed(r.id), + ), + ), // NOTE: we simply filter for open dialogs, as reminders are re-queried quite often filter( (reminders) => - this._matDialog.openDialogs.length === 0 && - !!reminders && - reminders.length > 0, + this._matDialog.openDialogs.length === 0 && reminders.length > 0, ), // don't show reminders while add task bar is open switchMap((reminders: TaskWithReminderData[]) => { diff --git a/src/app/features/reminder/reminder.service.spec.ts b/src/app/features/reminder/reminder.service.spec.ts index 188c010a8d..a4fc667f9e 100644 --- a/src/app/features/reminder/reminder.service.spec.ts +++ b/src/app/features/reminder/reminder.service.spec.ts @@ -1,7 +1,7 @@ import { TestBed } from '@angular/core/testing'; import { Store } from '@ngrx/store'; import { BehaviorSubject, of } from 'rxjs'; -import { ReminderService } from './reminder.service'; +import { ReminderService, REMINDER_DISMISS_UI_COOLDOWN_MS } from './reminder.service'; import { SnackService } from '../../core/snack/snack.service'; import { ImexViewService } from '../../imex/imex-meta/imex-view.service'; import { GlobalConfigService } from '../config/global-config.service'; @@ -374,6 +374,41 @@ describe('ReminderService', () => { }); }); + describe('reminder UI dismiss cooldown', () => { + it('is not suppressed by default', () => { + expect(service.isReminderUiSuppressed('task1')).toBe(false); + }); + + it('suppresses a reminder after a passive dismiss', () => { + service.suppressReminderUiAfterDismiss('task1'); + expect(service.isReminderUiSuppressed('task1')).toBe(true); + }); + + it('only suppresses the dismissed reminder, not others', () => { + service.suppressReminderUiAfterDismiss('task1'); + expect(service.isReminderUiSuppressed('task1')).toBe(true); + expect(service.isReminderUiSuppressed('task2')).toBe(false); + }); + + it('expires after the cooldown so the reminder re-nudges', () => { + service.suppressReminderUiAfterDismiss('task1'); + const afterCooldown = Date.now() + REMINDER_DISMISS_UI_COOLDOWN_MS + 1000; + expect(service.isReminderUiSuppressed('task1', afterCooldown)).toBe(false); + }); + + it('stays suppressed right up to the cooldown boundary', () => { + const before = Date.now(); + service.suppressReminderUiAfterDismiss('task1'); + // 1s before expiry it must still be suppressed + expect( + service.isReminderUiSuppressed( + 'task1', + before + REMINDER_DISMISS_UI_COOLDOWN_MS - 1000, + ), + ).toBe(true); + }); + }); + // TODO: These tests reference non-existent methods (addReminder, snooze) and missing import (TaskService) // Commented out until the ReminderService API is updated or tests are fixed // describe('duplicate reminder prevention', () => { diff --git a/src/app/features/reminder/reminder.service.ts b/src/app/features/reminder/reminder.service.ts index 028648685c..5e7d55f1c3 100644 --- a/src/app/features/reminder/reminder.service.ts +++ b/src/app/features/reminder/reminder.service.ts @@ -16,6 +16,12 @@ import { Task, TaskWithReminder, TaskWithReminderData } from '../tasks/task.mode import { TaskSharedActions } from '../../root-store/meta/task-shared.actions'; import { LegacyPfDbService } from '../../core/persistence/legacy-pf-db.service'; +// How long to stop auto-reopening the reminder modal for a reminder the user +// dismissed without acting (backdrop / Escape / Android back). Long enough to +// let the user use the app, short enough to still re-nudge so an overdue +// reminder is not forgotten. +export const REMINDER_DISMISS_UI_COOLDOWN_MS = 5 * 60 * 1000; + interface WorkerReminder { id: string; remindAt: number; @@ -54,6 +60,14 @@ export class ReminderService { private _w: Worker; + // In-memory, session-only cooldown for reminders the user dismissed without + // acting (backdrop / Escape / Android back). Read by ReminderModule before + // auto-reopening the modal. Never persisted and never synced — purely a UI + // throttle so an overdue reminder cannot re-grab the screen every worker tick + // (~10s) and freeze the app. Cleared on cold start; an explicit reschedule + // sets a future remindAt that fires past the cooldown anyway. + private _uiSuppressedUntil = new Map(); + constructor() { if (typeof (Worker as unknown) === 'undefined') { throw new Error('No service workers supported :('); @@ -100,6 +114,29 @@ export class ReminderService { }); } + /** + * Throttle re-opening the reminder modal for a reminder the user dismissed + * without acting on it (backdrop / Escape / Android back). Prevents the worker + * (~10s tick) from immediately re-grabbing the screen, which on mobile reads as + * a frozen app where no controls respond. + */ + suppressReminderUiAfterDismiss(taskId: string): void { + this._uiSuppressedUntil.set(taskId, Date.now() + REMINDER_DISMISS_UI_COOLDOWN_MS); + } + + /** Whether the reminder modal should currently stay closed for this task. */ + isReminderUiSuppressed(taskId: string, now: number = Date.now()): boolean { + const until = this._uiSuppressedUntil.get(taskId); + if (until === undefined) { + return false; + } + if (until <= now) { + this._uiSuppressedUntil.delete(taskId); + return false; + } + return true; + } + private async _migrateLegacyReminders(): Promise { try { const legacyReminders = await this._legacyPfDb.load('reminders'); diff --git a/src/app/features/tasks/dialog-view-task-reminders/dialog-view-task-reminders.component.spec.ts b/src/app/features/tasks/dialog-view-task-reminders/dialog-view-task-reminders.component.spec.ts index b029d65216..49ada06a10 100644 --- a/src/app/features/tasks/dialog-view-task-reminders/dialog-view-task-reminders.component.spec.ts +++ b/src/app/features/tasks/dialog-view-task-reminders/dialog-view-task-reminders.component.spec.ts @@ -588,7 +588,10 @@ describe('DialogViewTaskRemindersComponent destroy clears unhandled deadline rem let projectServiceSpy: jasmine.SpyObj; let matDialogSpy: jasmine.SpyObj; let matDialogRefSpy: jasmine.SpyObj>; - let reminderServiceStub: { onRemindersActive$: Subject }; + let reminderServiceStub: { + onRemindersActive$: Subject; + suppressReminderUiAfterDismiss: (taskId: string) => void; + }; const buildTask = (id: string, overrides: Partial = {}): Task => ({ @@ -649,6 +652,7 @@ describe('DialogViewTaskRemindersComponent destroy clears unhandled deadline rem matDialogSpy = jasmine.createSpyObj('MatDialog', ['open']); reminderServiceStub = { onRemindersActive$: new Subject(), + suppressReminderUiAfterDismiss: jasmine.createSpy('suppressReminderUiAfterDismiss'), }; await TestBed.configureTestingModule({ @@ -726,6 +730,32 @@ describe('DialogViewTaskRemindersComponent destroy clears unhandled deadline rem expect(dispatchedClearIds()).toEqual([]); }); + it('puts passively-dismissed schedule reminders into the UI cooldown on destroy', () => { + const reminder = buildReminder('task-1', { isDeadline: false }); + const component = createComponent([reminder], [buildTask('task-1')]); + + // No explicit action — closed via backdrop/Escape. + component.ngOnDestroy(); + + expect(reminderServiceStub.suppressReminderUiAfterDismiss).toHaveBeenCalledOnceWith( + 'task-1', + ); + }); + + it('does NOT cooldown a schedule reminder the user already acted on', () => { + const reminder = buildReminder('task-1', { isDeadline: false }); + const component = createComponent([reminder], [buildTask('task-1')]); + + // Explicit action removes it from the list and marks it dismissed. + component.dismissReminderOnly(reminder); + + component.ngOnDestroy(); + + expect(reminderServiceStub.suppressReminderUiAfterDismiss).not.toHaveBeenCalledWith( + 'task-1', + ); + }); + it('clears every unhandled deadline reminder on destroy, skipping dismissed ones', () => { const r1 = buildReminder('task-1', { isDeadline: true, deadlineDay: '2026-04-25' }); const r2 = buildReminder('task-2', { isDeadline: true, deadlineDay: '2026-04-26' }); @@ -904,7 +934,13 @@ describe('DialogViewTaskRemindersComponent accessibility', () => { provide: MatDialog, useValue: { open: () => ({ afterClosed: () => of(false) }) }, }, - { provide: ReminderService, useValue: { onRemindersActive$: new Subject() } }, + { + provide: ReminderService, + useValue: { + onRemindersActive$: new Subject(), + suppressReminderUiAfterDismiss: () => {}, + }, + }, { provide: DateService, useValue: { todayStr: () => '2026-06-06', getStartOfNextDayDiffMs: () => 0 }, @@ -941,7 +977,10 @@ describe('DialogViewTaskRemindersComponent navigation and focus', () => { let component: DialogViewTaskRemindersComponent; let fixture: any; let taskServiceSpy: jasmine.SpyObj; - let reminderServiceStub: { onRemindersActive$: Subject }; + let reminderServiceStub: { + onRemindersActive$: Subject; + suppressReminderUiAfterDismiss: (taskId: string) => void; + }; const buildTask = (id: string): Task => ({ ...DEFAULT_TASK, id, title: `Task ${id}` }) as Task; @@ -959,7 +998,10 @@ describe('DialogViewTaskRemindersComponent navigation and focus', () => { 'setDone', 'setCurrentId', ]); - reminderServiceStub = { onRemindersActive$: new Subject() }; + reminderServiceStub = { + onRemindersActive$: new Subject(), + suppressReminderUiAfterDismiss: jasmine.createSpy('suppressReminderUiAfterDismiss'), + }; await TestBed.configureTestingModule({ imports: [ @@ -1153,7 +1195,10 @@ describe('DialogViewTaskRemindersComponent reconciles vanished reminders (sync)' let projectServiceSpy: jasmine.SpyObj; let matDialogSpy: jasmine.SpyObj; let matDialogRefSpy: jasmine.SpyObj>; - let reminderServiceStub: { onRemindersActive$: Subject }; + let reminderServiceStub: { + onRemindersActive$: Subject; + suppressReminderUiAfterDismiss: (taskId: string) => void; + }; let storeTasks$: BehaviorSubject; const buildTask = (id: string, overrides: Partial = {}): Task => @@ -1205,6 +1250,7 @@ describe('DialogViewTaskRemindersComponent reconciles vanished reminders (sync)' matDialogSpy = jasmine.createSpyObj('MatDialog', ['open']); reminderServiceStub = { onRemindersActive$: new Subject(), + suppressReminderUiAfterDismiss: jasmine.createSpy('suppressReminderUiAfterDismiss'), }; await TestBed.configureTestingModule({ diff --git a/src/app/features/tasks/dialog-view-task-reminders/dialog-view-task-reminders.component.ts b/src/app/features/tasks/dialog-view-task-reminders/dialog-view-task-reminders.component.ts index c0c73f1ead..64d795bd6d 100644 --- a/src/app/features/tasks/dialog-view-task-reminders/dialog-view-task-reminders.component.ts +++ b/src/app/features/tasks/dialog-view-task-reminders/dialog-view-task-reminders.component.ts @@ -237,6 +237,19 @@ export class DialogViewTaskRemindersComponent implements OnDestroy { this._store.dispatch(TaskSharedActions.clearDeadlineReminder({ taskId })); } }); + // Scheduled (non-deadline) reminders are intentionally NOT cleared on a + // passive dismiss — they should re-nudge later. But re-showing them on the + // very next worker tick (~10s) re-grabs the screen and freezes the app. Put + // them in a short in-memory UI cooldown instead, so the modal stays closed + // long enough to use the app while the reminder itself stays active. + this.taskIds$ + .getValue() + .filter( + (taskId) => + !this._deadlineReminderTaskIds.has(taskId) && + !this._dismissedReminderIds.has(taskId), + ) + .forEach((taskId) => this._reminderService.suppressReminderUiAfterDismiss(taskId)); this._subs.unsubscribe(); }