From b3dbfdbd7be2fb654d5cfe5ca1ae341fdf2234e9 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:17:40 -0400 Subject: [PATCH] fix(gui): route push taps through the stored notification A tap from the worker names only a notification; the desktop and the dashboard read the signed-in user's own row and route from its app, target and data, as a tap from the URL already did. Without a row the dashboard opens the list and the desktop does nothing. --- .../UI/Dashboard/UIDashboardNotifications.js | 21 ++--- src/gui/src/UI/UIDesktop.js | 31 ++++--- src/gui/src/helpers/notificationTap.js | 84 ++++++++----------- src/gui/src/helpers/notificationTap.test.js | 81 +++++++++++++----- src/gui/src/helpers/pushWorker.js | 3 +- src/gui/src/helpers/pushWorker.test.js | 10 +-- 6 files changed, 129 insertions(+), 101 deletions(-) diff --git a/src/gui/src/UI/Dashboard/UIDashboardNotifications.js b/src/gui/src/UI/Dashboard/UIDashboardNotifications.js index 1d5d6b6a1..36ff9a775 100644 --- a/src/gui/src/UI/Dashboard/UIDashboardNotifications.js +++ b/src/gui/src/UI/Dashboard/UIDashboardNotifications.js @@ -26,7 +26,7 @@ import { handleNotificationTap, openAppForTap, setPushTapHandler, - storedTapItem, + storedTapHandler, tapRoute, } from '../../helpers/notificationTap.js'; import { showLocalBanner, startPushWorker } from '../../helpers/pushWorker.js'; @@ -498,6 +498,15 @@ export default function UIDashboardNotifications ({ $el_window, socket }) { void handleNotificationTap({ uid: entry.uid, notification: entry.notification, appUid: entry.appUid }, tapSurface); }; + const actOnStored = storedTapHandler({ + fetchRow: getNotification, + surface: tapSurface, + found: (uid) => void acknowledge(uid).catch((err) => { + console.warn('Could not acknowledge notification:', err); + }), + missing: () => open(), + }); + const actOnPush = async (push) => { await reveal_dashboard(); if ( ! loaded ) await refresh(); @@ -506,15 +515,7 @@ export default function UIDashboardNotifications ({ $el_window, socket }) { actOn(entry); return; } - const tap = push.stored ? await storedTapItem(push.uid, getNotification) : { uid: push.uid, push }; - if ( ! tap ) { - open(); - return; - } - void acknowledge(push.uid).catch((err) => { - console.warn('Could not acknowledge notification:', err); - }); - await handleNotificationTap(tap, tapSurface); + await actOnStored(push); }; // -- Toasts ------------------------------------------------------------- diff --git a/src/gui/src/UI/UIDesktop.js b/src/gui/src/UI/UIDesktop.js index cd6a0da91..7d8552ef2 100644 --- a/src/gui/src/UI/UIDesktop.js +++ b/src/gui/src/UI/UIDesktop.js @@ -51,7 +51,7 @@ import { handleNotificationTap, openAppForTap, setPushTapHandler, - storedTapItem, + storedTapHandler, } from '../helpers/notificationTap.js'; import { showLocalBanner, startPushWorker } from '../helpers/pushWorker.js'; import { get_app_by_uid } from '../helpers/getAppByUid.js'; @@ -251,26 +251,25 @@ async function UIDesktop (options) { }; // The desktop has no notification list, so other types only clear. - const tap_notification = (tap) => handleNotificationTap(tap, { + const tap_surface = { share: (notification) => share_notification_click(notification)?.(), openApp: (route) => openAppForTap(route, { getApp: get_app_by_uid, launch: launch_app }), - }); + }; + const tap_notification = (tap) => handleNotificationTap(tap, tap_surface); const toasts_for = (uid) => $(elementsWithData($('.notification').get(), 'uid', uid)); - setPushTapHandler(async (push) => { - let tap = { uid: push.uid, push }; - if ( push.stored ) { - tap = await storedTapItem(push.uid, getNotification); - if ( ! tap ) return; - } - toasts_for(push.uid).remove(); - update_tab_notif_count_badge(); - void markNotificationAcknowledged(push.uid).catch((err) => { - console.warn('Could not acknowledge notification:', err); - }); - return tap_notification(tap); - }); + setPushTapHandler(storedTapHandler({ + fetchRow: getNotification, + surface: tap_surface, + found: (uid) => { + toasts_for(uid).remove(); + update_tab_notif_count_badge(); + void markNotificationAcknowledged(uid).catch((err) => { + console.warn('Could not acknowledge notification:', err); + }); + }, + })); startPushWorker(); /** diff --git a/src/gui/src/helpers/notificationTap.js b/src/gui/src/helpers/notificationTap.js index 31b3056f0..36dbbb866 100644 --- a/src/gui/src/helpers/notificationTap.js +++ b/src/gui/src/helpers/notificationTap.js @@ -38,11 +38,10 @@ const nonEmpty = (value) => (typeof value === 'string' && value !== '' ? value : */ /** + * A tap from the worker or the URL names only a notification; what it leads + * to is read off the signed-in user's own row. * @typedef {Object} PushTap * @property {string} uid - * @property {{ app?: string, path?: string }} [target] - * @property {unknown} [data] - * @property {true} [stored] - Only the uid is known; act on the stored row */ /** @@ -52,60 +51,37 @@ const nonEmpty = (value) => (typeof value === 'string' && value !== '' ? value : */ /** - * @param {{ uid?: string, notification?: Object|null, appUid?: string|null, push?: PushTap }} tap + * @param {{ uid?: string, notification?: Object|null, appUid?: string|null }} tap * @returns {TapRoute} */ -export const tapRoute = ({ uid, notification, appUid, push }) => { - if ( notification && typeof notification === 'object' ) { - if ( notification.type === APP_MESSAGE_TYPE ) { - const app = nonEmpty(appUid) ?? nonEmpty(notification.app?.uid); - if ( ! app ) return { kind: 'panel' }; - const target = typeof notification.target === 'string' ? notification.target : undefined; - return { kind: 'app', appUid: app, click: { uid, target, data: notification.data } }; - } - if ( isShareNotification(notification) ) return { kind: 'share', notification }; - return { kind: 'panel' }; - } - const app = nonEmpty(push?.target?.app); - if ( app ) { - return { kind: 'app', appUid: app, click: { uid, target: push.target.path, data: push.data } }; - } - const path = push?.target?.path; - if ( typeof path === 'string' && path.startsWith('/') ) { - return { - kind: 'share', - notification: { type: 'share.received', fields: { target: { path } } }, - }; +export const tapRoute = ({ uid, notification, appUid }) => { + if ( ! notification || typeof notification !== 'object' ) return { kind: 'panel' }; + if ( notification.type === APP_MESSAGE_TYPE ) { + const app = nonEmpty(appUid) ?? nonEmpty(notification.app?.uid); + if ( ! app ) return { kind: 'panel' }; + const target = typeof notification.target === 'string' ? notification.target : undefined; + return { kind: 'app', appUid: app, click: { uid, target, data: notification.data } }; } + if ( isShareNotification(notification) ) return { kind: 'share', notification }; return { kind: 'panel' }; }; +/** @returns {PushTap|null} */ +const tapFor = (uid) => ( + typeof uid === 'string' && uid.length <= UID_MAX && UID_PATTERN.test(uid) ? { uid } : null +); + /** * @param {unknown} raw - The worker's `notification` object - * @returns {PushTap|null} + * @returns {PushTap|null} The notification it names; nothing else it carries */ -export const normalizePushTap = (raw) => { - if ( ! raw || typeof raw !== 'object' ) return null; - if ( ! nonEmpty(raw.uid) || raw.uid.length > UID_MAX ) return null; - const target = {}; - if ( nonEmpty(raw.target?.app) ) target.app = raw.target.app; - if ( nonEmpty(raw.target?.path) ) target.path = raw.target.path; - return { - uid: raw.uid, - ...(Object.keys(target).length ? { target } : {}), - ...(raw.data !== undefined ? { data: raw.data } : {}), - }; -}; +export const normalizePushTap = (raw) => (raw && typeof raw === 'object' ? tapFor(raw.uid) : null); /** * @param {URLSearchParams} [params] - * @returns {PushTap|null} A tap naming only a notification uid + * @returns {PushTap|null} */ -export const pushTapFromParams = (params) => { - const uid = params?.get?.(PUSH_TAP_PARAM); - if ( typeof uid !== 'string' || uid.length > UID_MAX || ! UID_PATTERN.test(uid) ) return null; - return { uid, stored: true }; -}; +export const pushTapFromParams = (params) => tapFor(params?.get?.(PUSH_TAP_PARAM)); /** * @param {string} uid @@ -205,6 +181,21 @@ export const handleNotificationTap = async (tap, { share, panel, openApp }) => { return panel?.(); }; +/** + * @param {{ fetchRow: (uid: string) => Promise, + * surface: { share: Function, panel?: Function, openApp: Function }, + * found?: (uid: string) => unknown, missing?: () => unknown }} deps + * `found` runs before routing a row that was read; `missing` instead of + * routing when there is none + * @returns {(tap: PushTap) => Promise} + */ +export const storedTapHandler = ({ fetchRow, surface, found, missing }) => async ({ uid }) => { + const tap = await storedTapItem(uid, fetchRow); + if ( ! tap ) return missing?.(); + found?.(uid); + return handleNotificationTap(tap, surface); +}; + /** @type {((tap: PushTap) => unknown) | null} */ let pushTapHandler = null; /** @type {PushTap[]} */ @@ -223,8 +214,5 @@ const deliverPushTap = (tap) => { else pendingPushTaps.push(tap); }; -/** @param {unknown} raw - The worker's `notification` object */ +/** @param {unknown} raw - The worker's `notification` object, or a tap from the URL */ export const dispatchPushTap = (raw) => deliverPushTap(normalizePushTap(raw)); - -/** @param {PushTap} tap - From {@link pushTapFromParams} */ -export const dispatchStoredTap = (tap) => deliverPushTap(tap?.stored ? tap : null); diff --git a/src/gui/src/helpers/notificationTap.test.js b/src/gui/src/helpers/notificationTap.test.js index e8d9bffd2..f84063c51 100644 --- a/src/gui/src/helpers/notificationTap.test.js +++ b/src/gui/src/helpers/notificationTap.test.js @@ -24,12 +24,15 @@ import { normalizePushTap, openAppForTap, pushTapFromParams, + storedTapHandler, storedTapItem, tapRoute, withoutNotificationClick, } from './notificationTap.js'; const UID = '0b6a2c1e-6f1d-4a57-9c1e-3d2f8a9b7c10'; +const OTHER_UID = '7d3e9f10-2b4c-4d5e-8f6a-1b2c3d4e5f60'; +const THIRD_UID = 'c1d2e3f4-5a6b-4c7d-8e9f-0a1b2c3d4e5f'; const appMessage = (over = {}) => ({ type: 'app.message', @@ -75,24 +78,22 @@ describe('tapRoute', () => { .toEqual({ kind: 'panel' }); }); - it('reads a push tap: an app target is the app, a path is a shared item', () => { + it('is the list without a notification, whatever else the tap carries', () => { expect(tapRoute({ uid: 'n1', push: { uid: 'n1', target: { app: 'app-1', path: '/rooms/7' }, data: 5 } })) - .toEqual({ kind: 'app', appUid: 'app-1', click: { uid: 'n1', target: '/rooms/7', data: 5 } }); - expect(tapRoute({ uid: 'n1', push: { uid: 'n1', target: { path: '/alice/Shared/a.txt' } } })).toEqual({ - kind: 'share', - notification: { type: 'share.received', fields: { target: { path: '/alice/Shared/a.txt' } } }, - }); - expect(tapRoute({ uid: 'n1', push: { uid: 'n1' } })).toEqual({ kind: 'panel' }); + .toEqual({ kind: 'panel' }); + expect(tapRoute({ uid: 'n1', target: { app: 'app-1' }, notification: null })).toEqual({ kind: 'panel' }); + expect(tapRoute({ uid: 'n1' })).toEqual({ kind: 'panel' }); }); }); describe('normalizePushTap', () => { - it('keeps the expected fields and drops the rest', () => { - expect(normalizePushTap({ uid: 'n1', target: { app: 'a', path: 7, x: 1 }, data: null, extra: 1 })) - .toEqual({ uid: 'n1', target: { app: 'a' }, data: null }); + it('keeps the notification uid and nothing else', () => { + expect(normalizePushTap({ uid: UID, target: { app: 'a', path: '/x' }, data: null, extra: 1 })) + .toEqual({ uid: UID }); expect(normalizePushTap({ uid: '' })).toBeNull(); - expect(normalizePushTap({ uid: 'x'.repeat(65) })).toBeNull(); - expect(normalizePushTap('n1')).toBeNull(); + expect(normalizePushTap({ uid: 'n1' })).toBeNull(); + expect(normalizePushTap({ uid: `${UID}${'0'.repeat(40)}` })).toBeNull(); + expect(normalizePushTap(UID)).toBeNull(); }); }); @@ -100,7 +101,7 @@ describe('pushTapFromParams', () => { const params = (value) => new URLSearchParams({ 'puter.notification': value }); it('reads a notification uid, and nothing else, from the URL', () => { - expect(pushTapFromParams(params(UID))).toEqual({ uid: UID, stored: true }); + expect(pushTapFromParams(params(UID))).toEqual({ uid: UID }); }); it('takes nothing that is not a notification uid', () => { @@ -251,17 +252,57 @@ describe('handleNotificationTap', () => { }); }); +describe('storedTapHandler', () => { + const surface = () => ({ share: vi.fn(), panel: vi.fn(), openApp: vi.fn() }); + const row = { uid: UID, value: appMessage({ target: '/rooms/9', data: { room: 9 } }), app_uid: 'app-row' }; + const rows = async (uid) => (uid === UID ? row : null); + + it('routes from the row the tap names, after `found`', async () => { + const deps = { fetchRow: vi.fn(rows), surface: surface(), found: vi.fn(), missing: vi.fn() }; + await storedTapHandler(deps)({ uid: UID }); + expect(deps.fetchRow).toHaveBeenCalledWith(UID); + expect(deps.found).toHaveBeenCalledWith(UID); + expect(deps.surface.openApp).toHaveBeenCalledWith({ + kind: 'app', + appUid: 'app-row', + click: { uid: UID, target: '/rooms/9', data: { room: 9 } }, + }); + expect(deps.missing).not.toHaveBeenCalled(); + }); + + it('opens nothing for a row the user cannot read, running `missing` instead', async () => { + const deps = { fetchRow: vi.fn(rows), surface: surface(), found: vi.fn(), missing: vi.fn() }; + await storedTapHandler(deps)({ uid: OTHER_UID }); + expect(deps.missing).toHaveBeenCalledOnce(); + expect(deps.found).not.toHaveBeenCalled(); + expect(deps.surface.openApp).not.toHaveBeenCalled(); + expect(deps.surface.panel).not.toHaveBeenCalled(); + expect(deps.surface.share).not.toHaveBeenCalled(); + }); +}); + describe('push tap dispatch', () => { it('holds taps until a handler is set, then hands them over in order', async () => { - const { dispatchPushTap, dispatchStoredTap, setPushTapHandler } = await import('./notificationTap.js'); - dispatchPushTap({ uid: 'n1' }); + const { dispatchPushTap, setPushTapHandler } = await import('./notificationTap.js'); + dispatchPushTap({ uid: UID }); dispatchPushTap({ uid: '' }); - dispatchStoredTap({ uid: UID, stored: true }); - dispatchStoredTap({ uid: UID }); + dispatchPushTap({ uid: 'n2' }); + dispatchPushTap({ uid: OTHER_UID, stored: true }); const handler = vi.fn(); setPushTapHandler(handler); - expect(handler.mock.calls).toEqual([[{ uid: 'n1' }], [{ uid: UID, stored: true }]]); - dispatchPushTap({ uid: 'n3' }); - expect(handler).toHaveBeenLastCalledWith({ uid: 'n3' }); + expect(handler.mock.calls).toEqual([[{ uid: UID }], [{ uid: OTHER_UID }]]); + dispatchPushTap({ uid: THIRD_UID }); + expect(handler).toHaveBeenLastCalledWith({ uid: THIRD_UID }); + }); + + it('hands a worker tap over as the notification it names, nothing more', async () => { + const { dispatchPushTap, setPushTapHandler, storedTapHandler: make } = await import('./notificationTap.js'); + const openApp = vi.fn(); + const fetchRow = vi.fn(async () => null); + setPushTapHandler(make({ fetchRow, surface: { share: vi.fn(), openApp } })); + dispatchPushTap({ uid: OTHER_UID, target: { app: 'app-1', path: '/rooms/7' }, data: { room: 7 } }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(fetchRow).toHaveBeenCalledWith(OTHER_UID); + expect(openApp).not.toHaveBeenCalled(); }); }); diff --git a/src/gui/src/helpers/pushWorker.js b/src/gui/src/helpers/pushWorker.js index e0783189c..b1236a6d9 100644 --- a/src/gui/src/helpers/pushWorker.js +++ b/src/gui/src/helpers/pushWorker.js @@ -23,7 +23,6 @@ import { httpsIconFor, isShareNotification } from '../UI/Dashboard/notificationC import { APP_MESSAGE_TYPE, dispatchPushTap, - dispatchStoredTap, normalizePushTap, pushTapFromParams, PUSH_TAP_PARAM, @@ -144,7 +143,7 @@ export const startPushWorker = async ({ if ( query?.has?.(PUSH_TAP_PARAM) ) { const opened = pushTapFromParams(query); clearParam(PUSH_TAP_PARAM); - if ( opened ) dispatchStoredTap(opened); + if ( opened ) dispatchPushTap(opened); } // Leave an existing worker: removing it ends this browser's subscription. diff --git a/src/gui/src/helpers/pushWorker.test.js b/src/gui/src/helpers/pushWorker.test.js index 7e2359f27..7cd5d25d3 100644 --- a/src/gui/src/helpers/pushWorker.test.js +++ b/src/gui/src/helpers/pushWorker.test.js @@ -210,14 +210,14 @@ describe('startPushWorker', () => { expect(nav.serviceWorker.register).toHaveBeenCalledWith('/sw.js', { scope: '/' }); const source = { postMessage: vi.fn() }; - nav.listeners.message({ data: { type: 'other', notification: { uid: 'x' } }, source }); - nav.listeners.message({ data: { type: 'puter-notification-click', id: 'click-0', notification: { uid: '' } }, source }); + nav.listeners.message({ data: { type: 'other', notification: { uid: UID } }, source }); + nav.listeners.message({ data: { type: 'puter-notification-click', id: 'click-0', notification: { uid: 'n1' } }, source }); nav.listeners.message({ - data: { type: 'puter-notification-click', id: 'click-1', notification: { uid: 'n1', target: { app: 'a' } } }, + data: { type: 'puter-notification-click', id: 'click-1', notification: { uid: UID, target: { app: 'a' } } }, source, }); expect(source.postMessage.mock.calls).toEqual([[{ type: 'puter-notification-click-ack', id: 'click-1' }]]); - expect(handler.mock.calls).toEqual([[{ uid: 'n1', target: { app: 'a' } }]]); + expect(handler.mock.calls).toEqual([[{ uid: UID }]]); }); it('registers nothing inside a frame', async () => { @@ -245,7 +245,7 @@ describe('startPushWorker', () => { clearParam, }); expect(clearParam).toHaveBeenCalledWith('puter.notification'); - expect(handler).toHaveBeenCalledWith({ uid: UID, stored: true }); + expect(handler).toHaveBeenCalledWith({ uid: UID }); }); it('clears a malformed parameter and hands nothing over', async () => {