From 2c21721fefed89bd0be3a144cb9f79a58ea3625f Mon Sep 17 00:00:00 2001 From: Kestutis Kasiulynas Date: Sun, 20 Sep 2026 09:20:18 +0700 Subject: [PATCH 1/2] feat(settings): send a test notification, and make the notifications switch real MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings → Goals & alerts gains a Send test button under the Notifications switch. It sends a notification shaped exactly like a reminder through the platform notifier, checks the permission first and says where to fix a block, and hedges after sending because an OS-level mute of the browser is invisible to the extension. Its id is reminder-test, which the worker's handlers already resolve to dismiss. The Notifications switch was read by nothing. The worker fire path, the page fallback and the Pomodoro completion now honour it via getSettings (field-wise defaults, fail-open on a rejection). The Notifier port gains permission() for all hosts. Four review rounds folded in. Fixes ENG-119 --- .changeset/test-notification-button.md | 8 + CLAUDE.md | 2 +- .../e2e/test-notification.spec.ts | 29 ++++ apps/browser-extension/src/background.test.ts | 22 ++- .../src/platform/chrome-notifier.test.ts | 7 + .../src/platform/chrome-notifier.ts | 11 +- .../src/platform/web-notifier.test.ts | 27 ++- .../src/platform/web-notifier.ts | 22 ++- apps/browser-extension/vitest.setup.ts | 4 +- apps/macos/src/platform/index.test.ts | 117 +++++++++++++ apps/macos/src/platform/index.ts | 29 +++- .../settings/SettingsSections.test.tsx | 22 +++ .../components/settings/SettingsSections.tsx | 4 +- .../settings/TestNotificationRow.test.tsx | 161 ++++++++++++++++++ .../settings/TestNotificationRow.tsx | 97 +++++++++++ .../services/reminder-notifications.test.ts | 57 ++++++- .../src/services/reminder-notifications.ts | 48 +++++- .../app/src/stores/pomodoro-store.test.ts | 34 +++- packages/app/src/stores/pomodoro-store.ts | 19 ++- .../app/src/stores/reminder-store.test.ts | 51 ++++++ packages/app/src/stores/reminder-store.ts | 25 +-- packages/app/vitest.setup.ts | 4 +- packages/shared/src/platform/registry.test.ts | 1 + packages/shared/src/platform/types.ts | 7 + packages/test-utils/src/mocks/index.ts | 1 + .../test-utils/src/mocks/notifier.mock.ts | 17 ++ 26 files changed, 770 insertions(+), 56 deletions(-) create mode 100644 .changeset/test-notification-button.md create mode 100644 apps/browser-extension/e2e/test-notification.spec.ts create mode 100644 apps/macos/src/platform/index.test.ts create mode 100644 packages/app/src/components/settings/TestNotificationRow.test.tsx create mode 100644 packages/app/src/components/settings/TestNotificationRow.tsx create mode 100644 packages/test-utils/src/mocks/notifier.mock.ts diff --git a/.changeset/test-notification-button.md b/.changeset/test-notification-button.md new file mode 100644 index 000000000..1e741763e --- /dev/null +++ b/.changeset/test-notification-button.md @@ -0,0 +1,8 @@ +--- +'@cuewise/browser-extension': patch +'@cuewise/macos': patch +'@cuewise/app': patch +'@cuewise/shared': patch +--- + +A Send test button under Settings → Notifications shows what a reminder looks like, and the Notifications switch now really turns them off. diff --git a/CLAUDE.md b/CLAUDE.md index f8a493e84..441ab96af 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -40,7 +40,7 @@ apps/ // Host variants so a command-only context can't accidentally subscribe. interface Scheduler { scheduleAt(id, when): Promise; cancel(id): Promise; } interface SchedulerHost extends Scheduler { onFire(handler): () => void; } -interface Notifier { notify(opts): Promise; clear(id): Promise; } +interface Notifier { notify(opts): Promise; clear(id): Promise; permission(): Promise; } interface NotifierHost extends Notifier { onClick(handler): () => void; onAction(handler): () => void; } interface KeyValueStore { get(key, area); set(key, value, area); remove(key, area); getUsage(area); } diff --git a/apps/browser-extension/e2e/test-notification.spec.ts b/apps/browser-extension/e2e/test-notification.spec.ts new file mode 100644 index 000000000..ffe878e7e --- /dev/null +++ b/apps/browser-extension/e2e/test-notification.spec.ts @@ -0,0 +1,29 @@ +import { expect, test, type Worker } from '@playwright/test'; +import { buildExtension, launchExtension, openNewTab } from './extension-harness'; + +// ENG-119: the Settings "Send test" button creates a real chrome.notifications entry in the +// REAL built extension — the closest an automated check gets to the OS drawing it. +test.beforeAll(() => { + buildExtension(); +}); + +async function shownNotificationIds(worker: Worker): Promise { + return worker.evaluate(async () => Object.keys(await chrome.notifications.getAll())); +} + +test('Send test creates a notification the worker can see', async () => { + const session = await launchExtension(); + const { context, worker, extensionId } = session; + const page = await openNewTab(session); + + await page.goto(`chrome-extension://${extensionId}/index.html#settings`); + await page.getByPlaceholder('Search settings…').fill('test notification'); + await page.getByRole('button', { name: 'Send test' }).click(); + + await expect(page.getByText(/^Sent\./)).toBeVisible(); + await expect(async () => { + expect(await shownNotificationIds(worker)).toContain('reminder-test'); + }).toPass({ timeout: 5_000 }); + + await context.close(); +}); diff --git a/apps/browser-extension/src/background.test.ts b/apps/browser-extension/src/background.test.ts index 3e7619128..1beda3481 100644 --- a/apps/browser-extension/src/background.test.ts +++ b/apps/browser-extension/src/background.test.ts @@ -11,7 +11,7 @@ vi.mock('@cuewise/app/reminder-activity', async (importOriginal) => ({ ...(await importOriginal()), recordReminderActivity: recordActivityMock, })); -vi.mock('@cuewise/storage', () => ({ +vi.mock('@cuewise/storage', async () => ({ getReminders: getRemindersMock, setReminders: setRemindersMock, // Faithful, not a stub: the read has to happen inside the write, so a mock taking the caller's @@ -22,6 +22,8 @@ vi.mock('@cuewise/storage', () => ({ }), // Runs at module load now, sync or no sync — must resolve, background.ts chains off it. ensureSettingsMigrated: vi.fn(() => Promise.resolve()), + // The fire path checks the Notifications switch before it notifies. + getSettings: vi.fn(async () => (await import('@cuewise/shared')).DEFAULT_SETTINGS), })); type AlarmListener = (alarm: { name: string }) => void; @@ -471,6 +473,24 @@ describe('background: reminder alarm guards', () => { }); }); +// The Settings test notification carries a reminder id nothing is stored under. +describe('background: the test notification', () => { + it.each([ + ['Done', 0], + ['Snooze', 1], + ])('clears it on %s without writing or arming', async (_label, buttonIndex) => { + getRemindersMock.mockResolvedValue([]); + + fireButton('reminder-test', buttonIndex); + + await vi.waitFor(() => { + expect(chromeMock.notifications.clear).toHaveBeenCalledWith('reminder-test'); + }); + expect(setRemindersMock).not.toHaveBeenCalled(); + expect(chromeMock.alarms.create).not.toHaveBeenCalled(); + }); +}); + describe('background: notification click', () => { it('clears the notification and focuses an existing tab', async () => { chromeMock.tabs.query.mockResolvedValueOnce([{ id: 5, windowId: 1 }]); diff --git a/apps/browser-extension/src/platform/chrome-notifier.test.ts b/apps/browser-extension/src/platform/chrome-notifier.test.ts index 601ed533f..e24bb4cfd 100644 --- a/apps/browser-extension/src/platform/chrome-notifier.test.ts +++ b/apps/browser-extension/src/platform/chrome-notifier.test.ts @@ -7,6 +7,7 @@ type ButtonListener = (id: string, buttonIndex: number) => void; const notifications = { create: vi.fn((_id: string, _options: unknown) => Promise.resolve('id')), clear: vi.fn((_id: string) => Promise.resolve(true)), + getPermissionLevel: vi.fn(() => Promise.resolve('granted' as 'granted' | 'denied')), onClicked: { addListener: vi.fn(), removeListener: vi.fn() }, onButtonClicked: { addListener: vi.fn(), removeListener: vi.fn() }, }; @@ -82,4 +83,10 @@ describe('ChromeNotifier', () => { expect(handler).toHaveBeenCalledWith('reminder-9', 1); }); + + it.each(['granted', 'denied'] as const)("reports Chrome's %s permission level", async (level) => { + notifications.getPermissionLevel.mockResolvedValueOnce(level); + + expect(await new ChromeNotifier().permission()).toBe(level); + }); }); diff --git a/apps/browser-extension/src/platform/chrome-notifier.ts b/apps/browser-extension/src/platform/chrome-notifier.ts index 2d7ebcbff..44f8b06ea 100644 --- a/apps/browser-extension/src/platform/chrome-notifier.ts +++ b/apps/browser-extension/src/platform/chrome-notifier.ts @@ -1,4 +1,9 @@ -import { logger, type NotifierHost, type NotifyOptions } from '@cuewise/shared'; +import { + logger, + type NotifierHost, + type NotifierPermission, + type NotifyOptions, +} from '@cuewise/shared'; const ICON_PATH = 'icons/icon-128.png'; @@ -25,6 +30,10 @@ export class ChromeNotifier implements NotifierHost { await chrome.notifications.clear(id); } + async permission(): Promise { + return chrome.notifications.getPermissionLevel(); + } + onClick(handler: (id: string) => void | Promise): () => void { const listener = async (id: string) => { try { diff --git a/apps/browser-extension/src/platform/web-notifier.test.ts b/apps/browser-extension/src/platform/web-notifier.test.ts index c180ffcf1..a16724192 100644 --- a/apps/browser-extension/src/platform/web-notifier.test.ts +++ b/apps/browser-extension/src/platform/web-notifier.test.ts @@ -1,3 +1,4 @@ +import { logger } from '@cuewise/shared'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { WebNotifier } from './web-notifier'; @@ -11,6 +12,7 @@ function stubNotification(permission: NotificationPermission) { describe('WebNotifier', () => { afterEach(() => { vi.unstubAllGlobals(); + vi.restoreAllMocks(); }); it('delivers via the web Notification API when permission is granted', async () => { @@ -21,12 +23,19 @@ describe('WebNotifier', () => { expect(notification).toHaveBeenCalledWith('Pomodoro Timer', { body: 'Done!' }); }); - it('does nothing when permission is not granted', async () => { + // Error, not warn: 'error' is the shipped log level, so anything quieter is never seen. + it('delivers nothing when permission is not granted, and says so', async () => { const notification = stubNotification('denied'); + const errorLog = vi.spyOn(logger, 'error').mockImplementation(() => {}); await new WebNotifier().notify({ id: 'x', title: 'T', body: 'B' }); expect(notification).not.toHaveBeenCalled(); + expect(errorLog).toHaveBeenCalledWith( + 'Web notification not delivered: permission not granted', + undefined, + { id: 'x' } + ); }); it('exposes no-op interaction subscriptions', () => { @@ -35,4 +44,20 @@ describe('WebNotifier', () => { expect(() => notifier.onClick(() => {})()).not.toThrow(); expect(() => notifier.onAction(() => {})()).not.toThrow(); }); + + it.each([ + ['granted', 'granted'], + ['denied', 'denied'], + ['default', 'unknown'], + ] as const)('maps the web permission %s to %s', async (web, expected) => { + stubNotification(web); + + expect(await new WebNotifier().permission()).toBe(expected); + }); + + it('reports unknown where there is no Notification API', async () => { + vi.stubGlobal('Notification', undefined); + + expect(await new WebNotifier().permission()).toBe('unknown'); + }); }); diff --git a/apps/browser-extension/src/platform/web-notifier.ts b/apps/browser-extension/src/platform/web-notifier.ts index 6884d8116..b80220ced 100644 --- a/apps/browser-extension/src/platform/web-notifier.ts +++ b/apps/browser-extension/src/platform/web-notifier.ts @@ -1,4 +1,9 @@ -import type { NotifierHost, NotifyOptions } from '@cuewise/shared'; +import { + logger, + type NotifierHost, + type NotifierPermission, + type NotifyOptions, +} from '@cuewise/shared'; /** * Notifier for contexts without chrome.notifications (dev/web): delivers via the @@ -7,13 +12,24 @@ import type { NotifierHost, NotifyOptions } from '@cuewise/shared'; */ export class WebNotifier implements NotifierHost { async notify(opts: NotifyOptions): Promise { - if (typeof Notification !== 'undefined' && Notification.permission === 'granted') { - new Notification(opts.title, { body: opts.body }); + if (typeof Notification === 'undefined' || Notification.permission !== 'granted') { + logger.error('Web notification not delivered: permission not granted', undefined, { + id: opts.id, + }); + return; } + new Notification(opts.title, { body: opts.body }); } async clear(_id: string): Promise {} + async permission(): Promise { + if (typeof Notification === 'undefined' || Notification.permission === 'default') { + return 'unknown'; + } + return Notification.permission; + } + onClick(_handler: (id: string) => void | Promise): () => void { return () => {}; } diff --git a/apps/browser-extension/vitest.setup.ts b/apps/browser-extension/vitest.setup.ts index 345195644..0fadaec34 100644 --- a/apps/browser-extension/vitest.setup.ts +++ b/apps/browser-extension/vitest.setup.ts @@ -1,5 +1,5 @@ import { configurePlatform } from '@cuewise/shared'; -import { installChromeStorageMock } from '@cuewise/test-utils/mocks'; +import { fakeNotifier, installChromeStorageMock } from '@cuewise/test-utils/mocks'; import { cleanup } from '@testing-library/react'; import { afterEach, beforeEach, vi } from 'vitest'; import '@testing-library/jest-dom'; @@ -39,7 +39,7 @@ beforeEach(() => { scheduleAt: async () => {}, cancel: async () => {}, }, - notifier: { notify: async () => {}, clear: async () => {} }, + notifier: fakeNotifier(), }); }); diff --git a/apps/macos/src/platform/index.test.ts b/apps/macos/src/platform/index.test.ts new file mode 100644 index 000000000..a21dbe77f --- /dev/null +++ b/apps/macos/src/platform/index.test.ts @@ -0,0 +1,117 @@ +import { logger } from '@cuewise/shared'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const isPermissionGrantedMock = vi.fn<() => Promise>(); +const requestPermissionMock = vi.fn<() => Promise<'granted' | 'denied' | 'default'>>(); +const sendNotificationMock = vi.fn(); + +vi.mock('@tauri-apps/plugin-notification', () => ({ + isPermissionGranted: () => isPermissionGrantedMock(), + requestPermission: () => requestPermissionMock(), + sendNotification: (options: unknown) => sendNotificationMock(options), +})); +vi.mock('@tauri-apps/api/core', () => ({ invoke: vi.fn() })); +vi.mock('@tauri-apps/api/event', () => ({ listen: vi.fn() })); + +import { TauriNotifier, WebNotifier } from './index'; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe('TauriNotifier.permission', () => { + it('reports granted when the plugin has permission', async () => { + isPermissionGrantedMock.mockResolvedValue(true); + + expect(await new TauriNotifier().permission()).toBe('granted'); + }); + + it('reports unknown, never denied, when the plugin has no permission', async () => { + isPermissionGrantedMock.mockResolvedValue(false); + + expect(await new TauriNotifier().permission()).toBe('unknown'); + }); +}); + +describe('TauriNotifier.notify', () => { + it('sends without asking when permission is already granted', async () => { + isPermissionGrantedMock.mockResolvedValue(true); + + await new TauriNotifier().notify({ id: 'reminder-1', title: 'T', body: 'B' }); + + expect(requestPermissionMock).not.toHaveBeenCalled(); + expect(sendNotificationMock).toHaveBeenCalledWith({ title: 'T', body: 'B' }); + }); + + it('asks for permission once when it is missing, then sends', async () => { + isPermissionGrantedMock.mockResolvedValue(false); + requestPermissionMock.mockResolvedValue('granted'); + + await new TauriNotifier().notify({ id: 'reminder-1', title: 'T', body: 'B' }); + + expect(requestPermissionMock).toHaveBeenCalledOnce(); + expect(sendNotificationMock).toHaveBeenCalledWith({ title: 'T', body: 'B' }); + }); + + it('sends nothing when permission is refused', async () => { + isPermissionGrantedMock.mockResolvedValue(false); + requestPermissionMock.mockResolvedValue('denied'); + + await new TauriNotifier().notify({ id: 'reminder-1', title: 'T', body: 'B' }); + + expect(sendNotificationMock).not.toHaveBeenCalled(); + }); +}); + +function stubNotification(permission: 'granted' | 'denied' | 'default'): void { + const mock = vi.fn(); + (mock as unknown as { permission: string }).permission = permission; + vi.stubGlobal('Notification', mock); +} + +describe('WebNotifier.notify', () => { + it('delivers through the web API when permission is granted', async () => { + stubNotification('granted'); + + await new WebNotifier().notify({ id: 'reminder-1', title: 'T', body: 'B' }); + + expect(vi.mocked(Notification)).toHaveBeenCalledWith('T', { body: 'B', tag: 'reminder-1' }); + }); + + it('delivers nothing when permission is not granted, and says so', async () => { + stubNotification('default'); + const errorLog = vi.spyOn(logger, 'error').mockImplementation(() => {}); + + await new WebNotifier().notify({ id: 'reminder-1', title: 'T', body: 'B' }); + + expect(vi.mocked(Notification)).not.toHaveBeenCalled(); + expect(errorLog).toHaveBeenCalledWith( + 'Web notification not delivered: permission not granted', + undefined, + { id: 'reminder-1' } + ); + }); +}); + +describe('WebNotifier.permission', () => { + it.each([ + ['granted', 'granted'], + ['denied', 'denied'], + ['default', 'unknown'], + ] as const)('maps the web permission %s to %s', async (web, expected) => { + stubNotification(web); + + expect(await new WebNotifier().permission()).toBe(expected); + }); + + it('reports unknown where there is no Notification API', async () => { + vi.stubGlobal('Notification', undefined); + + expect(await new WebNotifier().permission()).toBe('unknown'); + }); +}); diff --git a/apps/macos/src/platform/index.ts b/apps/macos/src/platform/index.ts index 64f3156e1..834577dde 100644 --- a/apps/macos/src/platform/index.ts +++ b/apps/macos/src/platform/index.ts @@ -1,4 +1,10 @@ -import { logger, type Notifier, type NotifyOptions, type SchedulerHost } from '@cuewise/shared'; +import { + logger, + type Notifier, + type NotifierPermission, + type NotifyOptions, + type SchedulerHost, +} from '@cuewise/shared'; import { invoke } from '@tauri-apps/api/core'; import { listen } from '@tauri-apps/api/event'; import { @@ -17,11 +23,11 @@ import { /** Web Notification API notifier — works inside the Tauri WKWebView. */ export class WebNotifier implements Notifier { async notify(opts: NotifyOptions): Promise { - // Only deliver when permission is already granted. Requesting it must come - // from a user gesture (WebKit errors otherwise), so that belongs in a - // settings action — and real OS notifications move to the Tauri notification - // plugin later. Until then this placeholder no-ops rather than nag. + // Only when already granted: WebKit rejects a permission request outside a user gesture. if (typeof Notification === 'undefined' || Notification.permission !== 'granted') { + logger.error('Web notification not delivered: permission not granted', undefined, { + id: opts.id, + }); return; } new Notification(opts.title, { body: opts.body, tag: opts.id }); @@ -30,6 +36,13 @@ export class WebNotifier implements Notifier { async clear(_id: string): Promise { // Web notifications auto-dismiss; nothing to clear. } + + async permission(): Promise { + if (typeof Notification === 'undefined' || Notification.permission === 'default') { + return 'unknown'; + } + return Notification.permission; + } } /** @@ -52,6 +65,12 @@ export class TauriNotifier implements Notifier { async clear(_id: string): Promise { // The plugin exposes no programmatic clear for delivered notifications. } + + // Never 'denied': the desktop plugin hard-codes granted, so an OS-level denial is invisible + // here, and a false could as well mean unasked. + async permission(): Promise { + return (await isPermissionGranted()) ? 'granted' : 'unknown'; + } } /** diff --git a/packages/app/src/components/settings/SettingsSections.test.tsx b/packages/app/src/components/settings/SettingsSections.test.tsx index ec97a1c0d..e5cd05016 100644 --- a/packages/app/src/components/settings/SettingsSections.test.tsx +++ b/packages/app/src/components/settings/SettingsSections.test.tsx @@ -173,6 +173,28 @@ describe('settings sections', () => { }); }); + describe('Goals & alerts', () => { + it('offers a test notification, gated by the Notifications switch', () => { + renderSection('goals', '', { enableNotifications: false }); + + expect(screen.getByText('Test notification')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Send test' })).toBeDisabled(); + }); + + // Every phrase that opens the section must also match the row, or the panel opens empty. + it.each([ + 'notification', + 'test notification', + 'send test', + ])('renders the test row for a "%s" search', (query) => { + expect(sectionsMatching(query)).toContain('goals'); + renderSection('goals', query); + + expect(screen.getByRole('button', { name: 'Send test' })).toBeEnabled(); + expect(screen.queryByText('Reminders layout')).not.toBeInTheDocument(); + }); + }); + describe('Home', () => { it('renders every catalogued widget row with its help text', () => { renderSection('home'); diff --git a/packages/app/src/components/settings/SettingsSections.tsx b/packages/app/src/components/settings/SettingsSections.tsx index 0e5fd8c7c..06457ce87 100644 --- a/packages/app/src/components/settings/SettingsSections.tsx +++ b/packages/app/src/components/settings/SettingsSections.tsx @@ -57,6 +57,7 @@ import { import { quoteIntervalToSeconds } from './settings-interval'; import { settingsMatch } from './settings-match'; import type { SettingsSectionProps } from './settings-types'; +import { TestNotificationRow } from './TestNotificationRow'; import { ThumbPicker } from './ThumbPicker'; import { pomodoroWorkStep } from './timer-presets'; import { WeatherLocationPicker } from './WeatherLocationPicker'; @@ -741,6 +742,7 @@ function GoalsSection({ s, set, filter }: SettingsSectionProps) { onChange={(v) => set({ enableNotifications: v })} /> + { + vi.clearAllMocks(); + notifier.permission.mockResolvedValue('granted'); + configurePlatform({ notifier }); +}); + +function renderRow(enabled = true, filter = '') { + return render(); +} + +async function clickSend(): Promise { + await userEvent.click(screen.getByRole('button', { name: 'Send test' })); +} + +/** Holds the next notify open; the returned function settles it. */ +function deferNotify(): () => void { + let finish = (): void => {}; + notifier.notify.mockReturnValueOnce( + new Promise((resolve) => { + finish = resolve; + }) + ); + return () => finish(); +} + +describe('TestNotificationRow', () => { + it('is disabled with a hint while the Notifications switch is off', () => { + renderRow(false); + + expect(screen.getByRole('button', { name: 'Send test' })).toBeDisabled(); + expect(screen.getByText('Turn Notifications on first.')).toBeInTheDocument(); + }); + + it('sends a notification shaped exactly like a reminder', async () => { + renderRow(); + + await clickSend(); + + expect(notifier.notify).toHaveBeenCalledWith({ + id: 'reminder-test', + title: '🔔 Reminder', + body: expect.stringContaining('test'), + actions: ['Done', 'Snooze 5 min'], + requireInteraction: true, + }); + expect(await screen.findByText(/^Sent\./)).toBeInTheDocument(); + }); + + // 'unknown' is not a refusal: whether notify can still prompt is the adapter's call. + it('sends when the permission is unknown', async () => { + notifier.permission.mockResolvedValue('unknown'); + renderRow(); + + await clickSend(); + + expect(notifier.notify).toHaveBeenCalled(); + expect(await screen.findByText(/^Sent\./)).toBeInTheDocument(); + }); + + it('does not send when notifications are blocked, and says where to fix it', async () => { + notifier.permission.mockResolvedValue('denied'); + renderRow(); + + await clickSend(); + + expect(notifier.notify).not.toHaveBeenCalled(); + expect(await screen.findByText(/blocked for Cuewise/)).toBeInTheDocument(); + }); + + it('reports a failed send instead of throwing', async () => { + notifier.notify.mockRejectedValueOnce(new Error('no notifications API')); + const errorLog = vi.spyOn(logger, 'error').mockImplementation(() => {}); + renderRow(); + + await clickSend(); + + expect(await screen.findByText(/Couldn't send/)).toBeInTheDocument(); + expect(errorLog).toHaveBeenCalledWith( + 'Failed to send the test notification', + expect.any(Error) + ); + }); + + // On Tauri, notify awaits a native prompt; a second click meanwhile would prompt twice. + it('disables the button while a send is in flight', async () => { + deferNotify(); + renderRow(); + + await clickSend(); + + expect(screen.getByRole('button', { name: 'Sending…' })).toBeDisabled(); + }); + + it('stays in flight across a switch toggle', async () => { + deferNotify(); + const { rerender } = renderRow(); + await clickSend(); + + rerender(); + rerender(); + + expect(screen.getByRole('button', { name: 'Sending…' })).toBeDisabled(); + }); + + it('drops the result of a send that finished while the switch was off', async () => { + const finish = deferNotify(); + const { rerender } = renderRow(); + await clickSend(); + + rerender(); + await act(async () => { + finish(); + }); + rerender(); + + expect(screen.queryByText(/^Sent\./)).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Send test' })).toBeEnabled(); + }); + + it('drops the result of a send that finishes after the switch comes back on', async () => { + const finish = deferNotify(); + const { rerender } = renderRow(); + await clickSend(); + + rerender(); + rerender(); + await act(async () => { + finish(); + }); + + expect(screen.queryByText(/^Sent\./)).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Send test' })).toBeEnabled(); + }); + + it('forgets the last outcome when the switch is turned off', async () => { + const { rerender } = renderRow(); + await clickSend(); + await screen.findByText(/^Sent\./); + + rerender(); + rerender(); + + expect(screen.queryByText(/^Sent\./)).not.toBeInTheDocument(); + }); + + it('hides itself, hint included, when the search filter does not match', () => { + renderRow(false, 'wallpaper'); + + expect(screen.queryByRole('button', { name: 'Send test' })).not.toBeInTheDocument(); + expect(screen.queryByText('Turn Notifications on first.')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/app/src/components/settings/TestNotificationRow.tsx b/packages/app/src/components/settings/TestNotificationRow.tsx new file mode 100644 index 000000000..b13f534fe --- /dev/null +++ b/packages/app/src/components/settings/TestNotificationRow.tsx @@ -0,0 +1,97 @@ +import { getNotifier, logger } from '@cuewise/shared'; +import { BellRing } from 'lucide-react'; +import type React from 'react'; +import { useEffect, useRef, useState } from 'react'; +import { + REMINDER_TEST_NOTIFICATION_ID, + reminderNotification, +} from '../../services/reminder-notifications'; +import { SettingRow } from './SettingControls'; +import { settingsMatch } from './settings-match'; + +const LABEL = 'Test notification'; +const HELP = 'Send one now to check it reaches you'; +const KEYWORDS = 'test notification send test preview check reminder alert'; +const TEST_BODY = 'This is a test reminder. If you can see it, reminders will reach you.'; + +type Result = 'sent' | 'blocked' | 'failed'; + +// No host can see an OS-level mute of the browser or app itself, so "sent" still hedges. +const RESULT_NOTES: Record = { + sent: "Sent. Nothing appeared? Check your system's notification settings — the browser or app itself may be muted.", + blocked: 'Notifications are blocked for Cuewise — allow them in your browser or system settings.', + failed: + "Couldn't send the notification. Reload this page and try again; if it keeps failing, check that notifications are allowed for Cuewise.", +}; +const SWITCH_OFF_NOTE = 'Turn Notifications on first.'; + +interface TestNotificationRowProps { + /** The Settings → Notifications switch; a test while it is off would prove nothing. */ + enabled: boolean; + filter: string; +} + +export const TestNotificationRow: React.FC = ({ enabled, filter }) => { + const [sending, setSending] = useState(false); + const [result, setResult] = useState(null); + // Switching off orphans an in-flight send: its result must not surface on switch-on. + const attempt = useRef(0); + + useEffect(() => { + if (!enabled) { + attempt.current += 1; + setResult(null); + } + }, [enabled]); + + if (!settingsMatch(filter, LABEL, HELP, KEYWORDS)) { + return null; + } + + const send = async () => { + const mine = attempt.current; + const settle = (next: Result) => { + if (mine === attempt.current) { + setResult(next); + } + }; + setSending(true); + setResult(null); + try { + const notifier = getNotifier(); + if ((await notifier.permission()) === 'denied') { + settle('blocked'); + return; + } + await notifier.notify(reminderNotification(REMINDER_TEST_NOTIFICATION_ID, TEST_BODY)); + settle('sent'); + } catch (error) { + logger.error('Failed to send the test notification', error); + settle('failed'); + } finally { + setSending(false); + } + }; + + let note: string | null = SWITCH_OFF_NOTE; + if (enabled) { + note = result === null ? null : RESULT_NOTES[result]; + } + + return ( + <> + + + + {note &&

{note}

} + + ); +}; diff --git a/packages/app/src/services/reminder-notifications.test.ts b/packages/app/src/services/reminder-notifications.test.ts index d7d8f9a1b..fa6c5934b 100644 --- a/packages/app/src/services/reminder-notifications.test.ts +++ b/packages/app/src/services/reminder-notifications.test.ts @@ -1,6 +1,7 @@ -import { configurePlatform, logger, type Reminder } from '@cuewise/shared'; +import { configurePlatform, DEFAULT_SETTINGS, logger, type Reminder } from '@cuewise/shared'; import * as storage from '@cuewise/storage'; import { recurringReminderFactory, reminderFactory } from '@cuewise/test-utils/factories'; +import { fakeNotifier } from '@cuewise/test-utils/mocks'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { recordReminderActivity } from './reminder-activity'; import { armMissingReminderAlarms, handleReminderFire } from './reminder-notifications'; @@ -14,6 +15,7 @@ const recordActivity = vi.mocked(recordReminderActivity); vi.mock('@cuewise/storage', () => ({ getReminders: vi.fn(), setReminders: vi.fn(), + getSettings: vi.fn(), // Faithful, not a stub: reading inside the write is the property under test, so a mock that // took the caller's list would let a read hoisted back out of the lock pass. updateReminders: vi.fn(async (mutate: (reminders: Reminder[]) => Reminder[]) => { @@ -24,17 +26,20 @@ vi.mock('@cuewise/storage', () => ({ const getRemindersMock = vi.mocked(storage.getReminders); const setRemindersMock = vi.mocked(storage.setReminders); +const getSettingsMock = vi.mocked(storage.getSettings); // Spy notifier/scheduler injected via the platform ports — assert against these // instead of any concrete adapter. -const notify = vi.fn(() => Promise.resolve()); +const notifier = fakeNotifier(); +const notify = notifier.notify; const scheduleAt = vi.fn(() => Promise.resolve()); beforeEach(() => { vi.clearAllMocks(); setRemindersMock.mockResolvedValue({ success: true }); + getSettingsMock.mockResolvedValue(DEFAULT_SETTINGS); configurePlatform({ - notifier: { notify, clear: async () => {} }, + notifier, scheduler: { deliversInBackground: true, persistsAcrossRestarts: false, @@ -184,6 +189,52 @@ describe('handleReminderFire', () => { expect(notify).not.toHaveBeenCalled(); }); + it('skips the notification but still advances when notifications are switched off', async () => { + getSettingsMock.mockResolvedValue({ ...DEFAULT_SETTINGS, enableNotifications: false }); + getRemindersMock.mockResolvedValue([ + recurringReminderFactory.build({ + id: 'r6', + recurring: { frequency: 'interval', intervalMinutes: 30 }, + }), + ]); + + await handleReminderFire('reminder-r6'); + + expect(notify).not.toHaveBeenCalled(); + expect(scheduleAt).toHaveBeenCalledWith('reminder-r6', expect.any(Date)); + }); + + it('spends a one-off, rather than deferring it, when notifications are switched off', async () => { + getSettingsMock.mockResolvedValue({ ...DEFAULT_SETTINGS, enableNotifications: false }); + getRemindersMock.mockResolvedValue([reminderFactory.build({ id: 'r8', text: 'Stretch' })]); + + await handleReminderFire('reminder-r8'); + + expect(notify).not.toHaveBeenCalled(); + expect(setRemindersMock.mock.calls[0][0][0].notified).toBe(true); + expect(recordActivity).toHaveBeenCalledWith({ + event: 'fired', + reminderId: 'r8', + text: 'Stretch', + detail: 'notifications off', + }); + }); + + // A storage hiccup must not silence reminders: the default is on, so unknown means on. + it('notifies, and says so, when the settings read rejects', async () => { + getSettingsMock.mockRejectedValue(new Error('storage unavailable')); + getRemindersMock.mockResolvedValue([reminderFactory.build({ id: 'r7' })]); + const errorLog = vi.spyOn(logger, 'error').mockImplementation(() => {}); + + await handleReminderFire('reminder-r7'); + + expect(notify).toHaveBeenCalled(); + expect(errorLog).toHaveBeenCalledWith( + 'Could not read the Notifications switch; notifying anyway', + expect.any(Error) + ); + }); + it('records a fired one-off in the activity log', async () => { getRemindersMock.mockResolvedValue([reminderFactory.build({ id: 'r1', text: 'Stretch' })]); diff --git a/packages/app/src/services/reminder-notifications.ts b/packages/app/src/services/reminder-notifications.ts index fda13f64e..668e66b9c 100644 --- a/packages/app/src/services/reminder-notifications.ts +++ b/packages/app/src/services/reminder-notifications.ts @@ -8,12 +8,13 @@ import { getNotifier, getScheduler, logger, + type NotifyOptions, nextReminderDueDate, type Reminder, reminderAlarmId, reminderIdFromAlarm, } from '@cuewise/shared'; -import { getReminders, updateReminders } from '@cuewise/storage'; +import { getReminders, getSettings, updateReminders } from '@cuewise/storage'; import { activitySubject, recordReminderActivity } from './reminder-activity'; export interface ReminderAlarmReconcile { @@ -60,6 +61,34 @@ export async function armMissingReminderAlarms( return tally; } +// A reminder-prefixed id with no stored reminder: the extension's button handler resolves it to +// dismiss, so Done / Snooze just close the test; a click opens Cuewise like any reminder. +export const REMINDER_TEST_NOTIFICATION_ID = reminderAlarmId('test'); + +/** The one shape every reminder notification takes, so a test notification is a real preview. */ +export function reminderNotification(id: string, body: string): NotifyOptions { + return { + id, + title: '🔔 Reminder', + body, + actions: ['Done', 'Snooze 5 min'], + requireInteraction: true, + }; +} + +/** + * Read from storage, not the settings store: the service worker has none. getSettings defaults + * field-wise, so an unreadable switch is on and a readable "off" is honoured; a rejection is on too. + */ +export async function notificationsEnabled(): Promise { + try { + return (await getSettings()).enableNotifications; + } catch (error) { + logger.error('Could not read the Notifications switch; notifying anyway', error); + return true; + } +} + /** * Deliver a reminder's notification when its scheduled wake fires. Looks the * reminder up by the alarm id, notifies (with Done/Snooze actions), marks it @@ -105,13 +134,10 @@ export async function handleReminderFire(alarmId: string): Promise { } step = 'notify'; - await getNotifier().notify({ - id: reminderAlarmId(reminderId), - title: '🔔 Reminder', - body: reminder.text, - actions: ['Done', 'Snooze 5 min'], - requireInteraction: true, - }); + const delivered = await notificationsEnabled(); + if (delivered) { + await getNotifier().notify(reminderNotification(reminderAlarmId(reminderId), reminder.text)); + } // One locked section reading fresh, not the list from before the notify: that round trip is // long enough for a pull to land, and every decision below has to be made against what it left. @@ -147,10 +173,14 @@ export async function handleReminderFire(alarmId: string): Promise { step = 're-arm'; await getScheduler().scheduleAt(reminderAlarmId(reminderId), nextDueDate); } + const details = [ + ...(delivered ? [] : ['notifications off']), + ...(nextDueDate !== null ? [`next ${nextDueDate.toISOString()}`] : []), + ]; await recordReminderActivity({ event: 'fired', ...activitySubject(reminder), - ...(nextDueDate !== null ? { detail: `next ${nextDueDate.toISOString()}` } : {}), + ...(details.length > 0 ? { detail: details.join(', ') } : {}), }); } catch (error) { logger.error('Error handling reminder fire', error); diff --git a/packages/app/src/stores/pomodoro-store.test.ts b/packages/app/src/stores/pomodoro-store.test.ts index 701a99e7a..1252f57b7 100644 --- a/packages/app/src/stores/pomodoro-store.test.ts +++ b/packages/app/src/stores/pomodoro-store.test.ts @@ -2,6 +2,7 @@ import { configurePlatform, type Settings } from '@cuewise/shared'; import type { SettingsRead } from '@cuewise/storage'; import * as storage from '@cuewise/storage'; import { defaultSettings } from '@cuewise/test-utils/fixtures'; +import { fakeNotifier } from '@cuewise/test-utils/mocks'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import * as sounds from '../utils/sounds'; import { usePomodoroStore } from './pomodoro-store'; @@ -54,10 +55,7 @@ vi.mock('./celebration-store', () => ({ })); // The Notifier is injected; assert against it instead of the global Notification. -const fakeNotifier = { - notify: vi.fn(() => Promise.resolve()), - clear: vi.fn(() => Promise.resolve()), -}; +const notifier = fakeNotifier(); // completeSession awaits readSettings() before it writes, so a fire-and-forget tick has not // reached the write when the caller's next statement runs. @@ -141,7 +139,7 @@ describe('Pomodoro Store - Auto-Start Breaks', () => { vi.mocked(storage.getSettings).mockResolvedValue(defaultSettings); vi.mocked(storage.readSettings).mockResolvedValue(settingsRead(defaultSettings)); - configurePlatform({ notifier: fakeNotifier }); + configurePlatform({ notifier }); }); describe('completeSession notification', () => { @@ -150,7 +148,7 @@ describe('Pomodoro Store - Auto-Start Breaks', () => { await usePomodoroStore.getState().completeSession(); - expect(fakeNotifier.notify).toHaveBeenCalledWith( + expect(notifier.notify).toHaveBeenCalledWith( expect.objectContaining({ title: 'Pomodoro Timer', body: expect.stringContaining('complete'), @@ -159,7 +157,7 @@ describe('Pomodoro Store - Auto-Start Breaks', () => { }); it('still saves the session when the notification fails', async () => { - fakeNotifier.notify.mockRejectedValueOnce(new Error('notify failed')); + notifier.notify.mockRejectedValueOnce(new Error('notify failed')); setupWorkSession(); await expect(usePomodoroStore.getState().completeSession()).resolves.toBeUndefined(); @@ -167,6 +165,19 @@ describe('Pomodoro Store - Auto-Start Breaks', () => { expect(storage.setPomodoroSessions).toHaveBeenCalled(); expect(toastError).not.toHaveBeenCalled(); }); + + it('does not notify when notifications are switched off in settings', async () => { + vi.mocked(storage.getSettings).mockResolvedValue({ + ...defaultSettings, + enableNotifications: false, + }); + setupWorkSession(); + + await usePomodoroStore.getState().completeSession(); + + expect(notifier.notify).not.toHaveBeenCalled(); + expect(storage.setPomodoroSessions).toHaveBeenCalled(); + }); }); describe('completeSession with auto-start enabled', () => { @@ -255,6 +266,13 @@ describe('Pomodoro Store - Auto-Start Breaks', () => { expect.stringContaining('pomodoroAutoStartBreaks') ); }); + + it('still notifies that the session ended', async () => { + setupWorkSession(); + await usePomodoroStore.getState().completeSession(); + + expect(notifier.notify).toHaveBeenCalled(); + }); }); describe('completeSession - break to work transitions with auto-start enabled', () => { @@ -473,7 +491,7 @@ describe('Pomodoro Store - tick wall-clock reconciliation (#159)', () => { vi.mocked(storage.setPomodoroSessions).mockResolvedValue({ success: true }); vi.mocked(storage.getSettings).mockResolvedValue(defaultSettings); vi.mocked(storage.readSettings).mockResolvedValue(settingsRead(defaultSettings)); - configurePlatform({ notifier: fakeNotifier }); + configurePlatform({ notifier }); }); it('decrements by one second on a normal ~1s tick', () => { diff --git a/packages/app/src/stores/pomodoro-store.ts b/packages/app/src/stores/pomodoro-store.ts index 39db80c8a..6b6634b3e 100644 --- a/packages/app/src/stores/pomodoro-store.ts +++ b/packages/app/src/stores/pomodoro-store.ts @@ -17,6 +17,7 @@ import { useEffect } from 'react'; import { create } from 'zustand'; import { createJSONStorage, persist } from 'zustand/middleware'; import { chromeLocalStorage } from '../adapters/zustand-chrome-adapter'; +import { notificationsEnabled } from '../services/reminder-notifications'; import { playCompletionSound, playStartSound } from '../utils/sounds'; import { useCelebrationStore } from './celebration-store'; import { useFocusModeStore } from './focus-mode-store'; @@ -568,14 +569,16 @@ export const usePomodoroStore = create()( } // Fire-and-forget: a notification failure — async rejection OR a // synchronous getNotifier() throw — must not fail the already-saved session. - try { - getNotifier() - .notify({ id: 'pomodoro-complete', title: 'Pomodoro Timer', body: message }) - .catch((error) => { - logger.error('Failed to show pomodoro completion notification', error); - }); - } catch (error) { - logger.error('Failed to show pomodoro completion notification', error); + if (await notificationsEnabled()) { + try { + getNotifier() + .notify({ id: 'pomodoro-complete', title: 'Pomodoro Timer', body: message }) + .catch((error) => { + logger.error('Failed to show pomodoro completion notification', error); + }); + } catch (error) { + logger.error('Failed to show pomodoro completion notification', error); + } } } catch (error) { logger.error('Error finishing a completed pomodoro session', error); diff --git a/packages/app/src/stores/reminder-store.test.ts b/packages/app/src/stores/reminder-store.test.ts index e717a8ec6..83855e8b9 100644 --- a/packages/app/src/stores/reminder-store.test.ts +++ b/packages/app/src/stores/reminder-store.test.ts @@ -1,5 +1,6 @@ import { configurePlatform, + DEFAULT_SETTINGS, logger, type Reminder, resetPlatform, @@ -7,8 +8,10 @@ import { } from '@cuewise/shared'; import * as storage from '@cuewise/storage'; import { recurringReminderFactory, reminderFactory } from '@cuewise/test-utils/factories'; +import { fakeNotifier } from '@cuewise/test-utils/mocks'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { recordReminderActivity } from '../services/reminder-activity'; +import { reminderNotification } from '../services/reminder-notifications'; import { fakeObservableStore } from './__fixtures__/storage-changes.fixtures'; import { useReminderStore } from './reminder-store'; @@ -22,6 +25,7 @@ const recordActivity = vi.mocked(recordReminderActivity); vi.mock('@cuewise/storage', () => ({ getReminders: vi.fn(), setReminders: vi.fn(), + getSettings: vi.fn(), // Faithful, not a stub: reading inside the write is the property under test, so a mock that // took the caller's list would let a read hoisted back out of the lock pass. updateReminders: vi.fn(async (mutate: (reminders: Reminder[]) => Reminder[]) => { @@ -47,6 +51,7 @@ vi.mock('./toast-store', () => ({ const getRemindersMock = vi.mocked(storage.getReminders); const setRemindersMock = vi.mocked(storage.setReminders); +const getSettingsMock = vi.mocked(storage.getSettings); // The Scheduler is injected; assert against it instead of poking chrome.alarms. const fakeScheduler = { @@ -82,6 +87,7 @@ beforeEach(() => { // seed with setState or an earlier action and still exercise the read-inside-the-write. getRemindersMock.mockImplementation(async () => useReminderStore.getState().reminders); setRemindersMock.mockResolvedValue({ success: true }); + getSettingsMock.mockResolvedValue(DEFAULT_SETTINGS); configurePlatform({ scheduler: fakeScheduler }); useReminderStore.setState({ reminders: [], @@ -390,6 +396,51 @@ describe('fireDueReminders', () => { expect(logged).toHaveBeenCalledWith('Fired due reminders', { count: 1 }); }); + + describe('raising the OS notification from the page', () => { + const notifier = fakeNotifier(); + const DUE_TEXT = 'Stand up'; + + beforeEach(() => { + configurePlatform({ scheduler: fakeScheduler, notifier }); + useReminderStore.setState({ + reminders: [ + reminderFactory.build({ + id: 'due-1', + text: DUE_TEXT, + dueDate: new Date(Date.now() - 60_000).toISOString(), + notified: false, + }), + ], + }); + }); + + it('notifies when the switch is on', async () => { + await useReminderStore.getState().fireDueReminders(); + + expect(notifier.notify).toHaveBeenCalledWith( + reminderNotification('reminder-due-1', DUE_TEXT) + ); + }); + + it('still toasts, but does not notify, when the switch is off', async () => { + getSettingsMock.mockResolvedValue({ ...DEFAULT_SETTINGS, enableNotifications: false }); + + await useReminderStore.getState().fireDueReminders(); + + expect(notifier.notify).not.toHaveBeenCalled(); + expect(toastWarning).toHaveBeenCalledTimes(1); + }); + + it('leaves delivery to a background host', async () => { + configurePlatform({ scheduler: { ...fakeScheduler, deliversInBackground: true }, notifier }); + + await useReminderStore.getState().fireDueReminders(); + + expect(notifier.notify).not.toHaveBeenCalled(); + expect(getSettingsMock).not.toHaveBeenCalled(); + }); + }); }); describe('addReminder with an interval recurrence', () => { diff --git a/packages/app/src/stores/reminder-store.ts b/packages/app/src/stores/reminder-store.ts index 077db60a4..d71bac1e1 100644 --- a/packages/app/src/stores/reminder-store.ts +++ b/packages/app/src/stores/reminder-store.ts @@ -24,7 +24,11 @@ import { } from '@cuewise/storage'; import { create } from 'zustand'; import { activitySubject, recordReminderActivity } from '../services/reminder-activity'; -import { armMissingReminderAlarms } from '../services/reminder-notifications'; +import { + armMissingReminderAlarms, + notificationsEnabled, + reminderNotification, +} from '../services/reminder-notifications'; import { createStaleLatch, createStorageObserver, sameEntities } from './storage-changes'; import { useToastStore } from './toast-store'; @@ -647,20 +651,19 @@ export const useReminderStore = create((set, get) => ({ // reminder that fired but never reached the user would otherwise leave no trace. logger.error('Fired due reminders', { count: dueNow.length }); + // Toasts first: `notified` is already persisted, so nothing fallible may sit between that + // write and the one delivery a page without a background host is guaranteed to make. for (const r of dueNow) { useToastStore.getState().warning(`Reminder: ${r.text}`); await recordReminderActivity({ event: 'toasted', ...activitySubject(r) }); - // No background worker to raise the OS notification, so deliver it here via the port. - // Where a resident host owns delivery, it notifies instead. - if (!getScheduler().deliversInBackground) { + } + + // No background worker to raise the OS notification, so deliver it here via the port. + // Where a resident host owns delivery, it notifies instead. + if (!getScheduler().deliversInBackground && (await notificationsEnabled())) { + for (const r of dueNow) { getNotifier() - .notify({ - id: reminderAlarmId(r.id), - title: '🔔 Reminder', - body: r.text, - actions: ['Done', 'Snooze 5 min'], - requireInteraction: true, - }) + .notify(reminderNotification(reminderAlarmId(r.id), r.text)) .catch((error) => logger.error('Failed to deliver reminder notification', error)); } } diff --git a/packages/app/vitest.setup.ts b/packages/app/vitest.setup.ts index b11f29ee2..1f19eb05b 100644 --- a/packages/app/vitest.setup.ts +++ b/packages/app/vitest.setup.ts @@ -1,5 +1,5 @@ import { configureLogger, configurePlatform } from '@cuewise/shared'; -import { installChromeStorageMock } from '@cuewise/test-utils/mocks'; +import { fakeNotifier, installChromeStorageMock } from '@cuewise/test-utils/mocks'; import { cleanup } from '@testing-library/react'; import { afterEach, beforeEach, vi } from 'vitest'; import '@testing-library/jest-dom'; @@ -43,7 +43,7 @@ beforeEach(() => { scheduleAt: async () => {}, cancel: async () => {}, }, - notifier: { notify: async () => {}, clear: async () => {} }, + notifier: fakeNotifier(), }); }); diff --git a/packages/shared/src/platform/registry.test.ts b/packages/shared/src/platform/registry.test.ts index 116b00092..080dd9f86 100644 --- a/packages/shared/src/platform/registry.test.ts +++ b/packages/shared/src/platform/registry.test.ts @@ -19,6 +19,7 @@ const fakeScheduler: Scheduler = { const fakeNotifier: Notifier = { notify: async () => {}, clear: async () => {}, + permission: async () => 'unknown', }; const fakeStorage: KeyValueStore = { supportsSync: false, diff --git a/packages/shared/src/platform/types.ts b/packages/shared/src/platform/types.ts index 4ae9f8747..3baf1b565 100644 --- a/packages/shared/src/platform/types.ts +++ b/packages/shared/src/platform/types.ts @@ -47,10 +47,17 @@ export interface NotifyOptions { requireInteraction?: boolean; } +/** + * Not `NotificationPermission`: lib.dom owns that name with a different member set. `unknown` is + * "the host cannot tell, or has not asked"; whether a notify then prompts is up to the adapter. + */ +export type NotifierPermission = 'granted' | 'denied' | 'unknown'; + /** Command surface: deliver/clear an OS notification, keyed by id. */ export interface Notifier { notify(opts: NotifyOptions): Promise; clear(id: string): Promise; + permission(): Promise; } /** A resident context that also routes notification clicks/actions back to handlers. */ diff --git a/packages/test-utils/src/mocks/index.ts b/packages/test-utils/src/mocks/index.ts index ef95d4538..6dea24d21 100644 --- a/packages/test-utils/src/mocks/index.ts +++ b/packages/test-utils/src/mocks/index.ts @@ -1,3 +1,4 @@ export * from './chrome-storage.mock'; export * from './lock-manager.mock'; +export * from './notifier.mock'; export * from './zustand.mock'; diff --git a/packages/test-utils/src/mocks/notifier.mock.ts b/packages/test-utils/src/mocks/notifier.mock.ts new file mode 100644 index 000000000..b16448728 --- /dev/null +++ b/packages/test-utils/src/mocks/notifier.mock.ts @@ -0,0 +1,17 @@ +import type { Notifier } from '@cuewise/shared'; +import { type Mock, vi } from 'vitest'; + +export interface FakeNotifier extends Notifier { + notify: Mock; + clear: Mock; + permission: Mock; +} + +/** A Notifier of spies. Script a member per test: `notifier.permission.mockResolvedValue('denied')`. */ +export function fakeNotifier(): FakeNotifier { + return { + notify: vi.fn(async () => {}), + clear: vi.fn(async () => {}), + permission: vi.fn(async () => 'unknown' as const), + }; +} From 972a36609ad131ad226939f39a0a32188650c58e Mon Sep 17 00:00:00 2001 From: Kestutis Kasiulynas Date: Sun, 20 Sep 2026 09:32:46 +0700 Subject: [PATCH 2/2] test(app): pin the withheld-fire trace and the toast-first ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-rebase review round 1. A withheld recurring fire records 'notifications off, next …'; every due reminder toasts before anything that can fail; the macOS notifier says when a refused permission drops a notification, as the web ones do; handleReminderFire's doc mentions the switch. --- apps/browser-extension/src/background.test.ts | 2 +- apps/macos/src/platform/index.test.ts | 8 ++++- apps/macos/src/platform/index.ts | 8 +++-- .../services/reminder-notifications.test.ts | 30 ++++++++++++++++++- .../src/services/reminder-notifications.ts | 6 ++-- packages/app/src/stores/pomodoro-store.ts | 4 +-- .../app/src/stores/reminder-store.test.ts | 27 +++++++++++++++++ 7 files changed, 75 insertions(+), 10 deletions(-) diff --git a/apps/browser-extension/src/background.test.ts b/apps/browser-extension/src/background.test.ts index 1beda3481..6119e70e8 100644 --- a/apps/browser-extension/src/background.test.ts +++ b/apps/browser-extension/src/background.test.ts @@ -11,7 +11,7 @@ vi.mock('@cuewise/app/reminder-activity', async (importOriginal) => ({ ...(await importOriginal()), recordReminderActivity: recordActivityMock, })); -vi.mock('@cuewise/storage', async () => ({ +vi.mock('@cuewise/storage', () => ({ getReminders: getRemindersMock, setReminders: setRemindersMock, // Faithful, not a stub: the read has to happen inside the write, so a mock taking the caller's diff --git a/apps/macos/src/platform/index.test.ts b/apps/macos/src/platform/index.test.ts index a21dbe77f..235b15a59 100644 --- a/apps/macos/src/platform/index.test.ts +++ b/apps/macos/src/platform/index.test.ts @@ -58,13 +58,19 @@ describe('TauriNotifier.notify', () => { expect(sendNotificationMock).toHaveBeenCalledWith({ title: 'T', body: 'B' }); }); - it('sends nothing when permission is refused', async () => { + it('sends nothing when permission is refused, and says so', async () => { isPermissionGrantedMock.mockResolvedValue(false); requestPermissionMock.mockResolvedValue('denied'); + const errorLog = vi.spyOn(logger, 'error').mockImplementation(() => {}); await new TauriNotifier().notify({ id: 'reminder-1', title: 'T', body: 'B' }); expect(sendNotificationMock).not.toHaveBeenCalled(); + expect(errorLog).toHaveBeenCalledWith( + 'Native notification not delivered: permission refused', + undefined, + { id: 'reminder-1' } + ); }); }); diff --git a/apps/macos/src/platform/index.ts b/apps/macos/src/platform/index.ts index 834577dde..302c701ce 100644 --- a/apps/macos/src/platform/index.ts +++ b/apps/macos/src/platform/index.ts @@ -57,9 +57,13 @@ export class TauriNotifier implements Notifier { if (!granted) { granted = (await requestPermission()) === 'granted'; } - if (granted) { - sendNotification({ title: opts.title, body: opts.body }); + if (!granted) { + logger.error('Native notification not delivered: permission refused', undefined, { + id: opts.id, + }); + return; } + sendNotification({ title: opts.title, body: opts.body }); } async clear(_id: string): Promise { diff --git a/packages/app/src/services/reminder-notifications.test.ts b/packages/app/src/services/reminder-notifications.test.ts index fa6c5934b..b262fef92 100644 --- a/packages/app/src/services/reminder-notifications.test.ts +++ b/packages/app/src/services/reminder-notifications.test.ts @@ -206,12 +206,20 @@ describe('handleReminderFire', () => { it('spends a one-off, rather than deferring it, when notifications are switched off', async () => { getSettingsMock.mockResolvedValue({ ...DEFAULT_SETTINGS, enableNotifications: false }); - getRemindersMock.mockResolvedValue([reminderFactory.build({ id: 'r8', text: 'Stretch' })]); + getRemindersMock.mockResolvedValue([reminderFactory.build({ id: 'r8' })]); await handleReminderFire('reminder-r8'); expect(notify).not.toHaveBeenCalled(); expect(setRemindersMock.mock.calls[0][0][0].notified).toBe(true); + }); + + it('records a withheld one-off as fired with the switch named', async () => { + getSettingsMock.mockResolvedValue({ ...DEFAULT_SETTINGS, enableNotifications: false }); + getRemindersMock.mockResolvedValue([reminderFactory.build({ id: 'r8', text: 'Stretch' })]); + + await handleReminderFire('reminder-r8'); + expect(recordActivity).toHaveBeenCalledWith({ event: 'fired', reminderId: 'r8', @@ -220,6 +228,26 @@ describe('handleReminderFire', () => { }); }); + it('records a withheld recurring fire together with its next occurrence', async () => { + getSettingsMock.mockResolvedValue({ ...DEFAULT_SETTINGS, enableNotifications: false }); + getRemindersMock.mockResolvedValue([ + recurringReminderFactory.build({ + id: 'r2', + text: 'Water', + recurring: { frequency: 'interval', intervalMinutes: 30 }, + }), + ]); + + await handleReminderFire('reminder-r2'); + + expect(recordActivity).toHaveBeenCalledWith({ + event: 'fired', + reminderId: 'r2', + text: 'Water', + detail: expect.stringMatching(/^notifications off, next \d{4}-/), + }); + }); + // A storage hiccup must not silence reminders: the default is on, so unknown means on. it('notifies, and says so, when the settings read rejects', async () => { getSettingsMock.mockRejectedValue(new Error('storage unavailable')); diff --git a/packages/app/src/services/reminder-notifications.ts b/packages/app/src/services/reminder-notifications.ts index 668e66b9c..58d326c2b 100644 --- a/packages/app/src/services/reminder-notifications.ts +++ b/packages/app/src/services/reminder-notifications.ts @@ -90,9 +90,9 @@ export async function notificationsEnabled(): Promise { } /** - * Deliver a reminder's notification when its scheduled wake fires. Looks the - * reminder up by the alarm id, notifies (with Done/Snooze actions), marks it - * notified, and re-arms the next occurrence for recurring reminders. A no-op for + * Deliver a reminder's notification when its scheduled wake fires. Looks the reminder up by the + * alarm id, notifies (with Done/Snooze actions) unless the Notifications switch is off, and in + * either case marks it notified and re-arms the next occurrence of a recurring one. A no-op for * non-reminder alarm ids, or reminders that are gone / completed / paused. */ export async function handleReminderFire(alarmId: string): Promise { diff --git a/packages/app/src/stores/pomodoro-store.ts b/packages/app/src/stores/pomodoro-store.ts index 6b6634b3e..12823ad26 100644 --- a/packages/app/src/stores/pomodoro-store.ts +++ b/packages/app/src/stores/pomodoro-store.ts @@ -567,9 +567,9 @@ export const usePomodoroStore = create()( } else { message = 'Break complete! Ready to focus?'; } - // Fire-and-forget: a notification failure — async rejection OR a - // synchronous getNotifier() throw — must not fail the already-saved session. if (await notificationsEnabled()) { + // Fire-and-forget: a notification failure — async rejection OR a + // synchronous getNotifier() throw — must not fail the already-saved session. try { getNotifier() .notify({ id: 'pomodoro-complete', title: 'Pomodoro Timer', body: message }) diff --git a/packages/app/src/stores/reminder-store.test.ts b/packages/app/src/stores/reminder-store.test.ts index 83855e8b9..ebd4f9a68 100644 --- a/packages/app/src/stores/reminder-store.test.ts +++ b/packages/app/src/stores/reminder-store.test.ts @@ -440,6 +440,33 @@ describe('fireDueReminders', () => { expect(notifier.notify).not.toHaveBeenCalled(); expect(getSettingsMock).not.toHaveBeenCalled(); }); + + // `notified` is persisted before the toasts, so every due reminder must reach the user even + // when the notifier is unusable — the toast is the delivery nothing fallible may precede. + it('toasts every due reminder before anything that can fail', async () => { + resetPlatform(); + configurePlatform({ scheduler: fakeScheduler }); + useReminderStore.setState({ + reminders: [ + reminderFactory.build({ + id: 'due-1', + dueDate: new Date(Date.now() - 60_000).toISOString(), + notified: false, + }), + reminderFactory.build({ + id: 'due-2', + dueDate: new Date(Date.now() - 30_000).toISOString(), + notified: false, + }), + ], + }); + const errorLog = vi.spyOn(logger, 'error').mockImplementation(() => {}); + + await useReminderStore.getState().fireDueReminders(); + + expect(toastWarning).toHaveBeenCalledTimes(2); + expect(errorLog).toHaveBeenCalledWith('Error firing due reminders', expect.anything()); + }); }); });