From 8c34e6190e94c3153f6ab8b17705eabc2f724f98 Mon Sep 17 00:00:00 2001 From: Shawn Jackson Date: Tue, 18 Aug 2026 21:43:14 -0700 Subject: [PATCH 1/2] RG-T132 Push linking fix --- .../push-notification-chat-deeplink.test.ts | 206 ++++++++++++++++++ .../__tests__/push-notification.test.ts | 145 +++++++++++- src/services/push-notification.ts | 102 +++++++-- .../push-notification/__tests__/store.test.ts | 56 ++++- src/stores/push-notification/store.ts | 71 +++--- 5 files changed, 531 insertions(+), 49 deletions(-) create mode 100644 src/services/__tests__/push-notification-chat-deeplink.test.ts diff --git a/src/services/__tests__/push-notification-chat-deeplink.test.ts b/src/services/__tests__/push-notification-chat-deeplink.test.ts new file mode 100644 index 00000000..8dfa1fc5 --- /dev/null +++ b/src/services/__tests__/push-notification-chat-deeplink.test.ts @@ -0,0 +1,206 @@ +import { router } from 'expo-router'; + +import { logger } from '@/lib/logging'; + +import { extractPushNotificationData, handleChatDeepLink } from '../push-notification'; + +jest.mock('expo-router', () => ({ + router: { push: jest.fn() }, +})); + +jest.mock('expo-device', () => ({ + isDevice: true, +})); + +jest.mock('expo-notifications', () => ({ + setNotificationHandler: jest.fn(), + addNotificationReceivedListener: jest.fn(() => ({ remove: jest.fn() })), + addNotificationResponseReceivedListener: jest.fn(() => ({ remove: jest.fn() })), + getLastNotificationResponseAsync: jest.fn(() => Promise.resolve(null)), + getPermissionsAsync: jest.fn(() => Promise.resolve({ status: 'granted' })), + requestPermissionsAsync: jest.fn(() => Promise.resolve({ status: 'granted' })), + getDevicePushTokenAsync: jest.fn(() => Promise.resolve({ data: 'test-token' })), + setBadgeCountAsync: jest.fn(() => Promise.resolve()), + dismissAllNotificationsAsync: jest.fn(() => Promise.resolve()), +})); + +jest.mock('@notifee/react-native', () => ({ + __esModule: true, + default: { + createChannel: jest.fn(() => Promise.resolve()), + deleteChannel: jest.fn(() => Promise.resolve()), + setNotificationCategories: jest.fn(() => Promise.resolve()), + requestPermission: jest.fn(() => Promise.resolve({ authorizationStatus: 1 })), + onForegroundEvent: jest.fn(() => jest.fn()), + onBackgroundEvent: jest.fn(), + cancelAllNotifications: jest.fn(() => Promise.resolve()), + }, + AndroidImportance: { HIGH: 4 }, + AndroidVisibility: { PUBLIC: 1 }, + AuthorizationStatus: { AUTHORIZED: 1, DENIED: 2 }, + EventType: { PRESS: 1, ACTION_PRESS: 2 }, +})); + +jest.mock('@/api/devices/push', () => ({ + registerUnitDevice: jest.fn(), +})); + +jest.mock('@/stores/auth/store', () => { + // handleChatDeepLink gates the cold-start push on a hydrated session, so the mock has to + // answer getState() as well as being callable as a selector hook. + const state = { status: 'signedIn', accessToken: 'test-access-token' }; + const store: any = jest.fn((selector: any) => (selector ? selector(state) : state)); + store.getState = () => state; + return { __esModule: true, default: store }; +}); + +jest.mock('@/lib/logging', () => ({ + logger: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + debug: jest.fn(), + }, +})); + +jest.mock('@/lib/storage/app', () => ({ + getDeviceUuid: jest.fn(() => 'test-uuid'), + getBaseApiUrl: jest.fn(() => ''), +})); + +jest.mock('@/lib/storage/notification-prefs', () => ({ + getModernNotificationSoundsEnabled: jest.fn(() => true), + getAppliedNotificationSoundMode: jest.fn(() => undefined), + setAppliedNotificationSoundMode: jest.fn(), +})); + +jest.mock('@/stores/app/core-store', () => ({ + useCoreStore: Object.assign( + jest.fn((selector: any) => { + const state = { activeUnitId: 'test-unit', activeCall: null, activeUnit: null }; + return selector ? selector(state) : state; + }), + { getState: jest.fn(() => ({ activeCall: null, activeUnit: null })) } + ), +})); + +jest.mock('@/stores/app/location-store', () => ({ + useLocationStore: { + getState: jest.fn(() => ({ latitude: null, longitude: null })), + }, +})); + +jest.mock('@/stores/check-in-timers/store', () => ({ + useCheckInTimerStore: { + getState: jest.fn(() => ({ performCheckIn: jest.fn() })), + }, +})); + +jest.mock('@/stores/security/store', () => ({ + securityStore: jest.fn((selector: any) => { + const state = { rights: { DepartmentCode: 'TEST' } }; + return selector ? selector(state) : state; + }), +})); + +// The real push-notification store pulls in the sound service (expo-audio); stub it out. +jest.mock('@/services/notification-sound.service', () => ({ + notificationSoundService: { + playNotificationSound: jest.fn(() => Promise.resolve()), + }, +})); + +describe('handleChatDeepLink', () => { + const push = router.push as jest.Mock; + const logError = logger.error as jest.Mock; + + beforeEach(() => { + jest.useFakeTimers(); + push.mockReset(); + logError.mockClear(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it.each([ + ['t:channel-1', 'channel-1'], + ['g:9101', '9101'], + ['T:channel-1', 'channel-1'], + ['G:9101', '9101'], + ])('navigates with explicit route params for %s', (eventCode, channelId) => { + expect(handleChatDeepLink(eventCode)).toBe(true); + expect(push).toHaveBeenCalledWith({ pathname: '/chat/[channelId]', params: { channelId } }); + }); + + it.each(['t:a/b', 't:a\\b', 'g:a?x=1', 'g:a#fragment', 'x:123', 't:', 'notacode', ':missingprefix'])('rejects invalid payload %s', (eventCode) => { + expect(handleChatDeepLink(eventCode)).toBe(false); + expect(push).not.toHaveBeenCalled(); + }); + + it('retries navigation when the router is not ready yet', async () => { + push + .mockImplementationOnce(() => { + throw new Error('router not ready'); + }) + .mockImplementationOnce(() => undefined); + + expect(handleChatDeepLink('t:channel-1')).toBe(true); + expect(push).toHaveBeenCalledTimes(1); + + await jest.advanceTimersByTimeAsync(300); + + expect(push).toHaveBeenCalledTimes(2); + expect(logError).not.toHaveBeenCalled(); + }); + + it('logs an error after exhausting navigation retries', async () => { + push.mockImplementation(() => { + throw new Error('router not ready'); + }); + + expect(handleChatDeepLink('t:channel-1')).toBe(true); + + // Budget is 40 attempts x 250ms so a cold start has ~10s to mount and hydrate. + await jest.advanceTimersByTimeAsync(250 * 40); + + expect(push).toHaveBeenCalledTimes(40); + expect(logError).toHaveBeenCalledWith(expect.objectContaining({ message: 'Failed to deep-link to chat channel' })); + }); +}); + +describe('extractPushNotificationData', () => { + const makeRequest = (data: unknown, triggerPayload?: unknown): any => ({ + identifier: 'req-1', + content: { title: 'T', body: 'B', data }, + trigger: triggerPayload === undefined ? { type: 'push' } : { type: 'push', payload: triggerPayload }, + }); + + it('reads eventCode from content.data (Android FCM path)', () => { + const { eventCode, data } = extractPushNotificationData(makeRequest({ eventCode: 'g:123', other: 1 })); + expect(eventCode).toBe('g:123'); + expect(data).toEqual({ eventCode: 'g:123', other: 1 }); + }); + + it('falls back to a top-level trigger payload key (iOS APNs custom key)', () => { + const { eventCode } = extractPushNotificationData(makeRequest(undefined, { aps: { alert: {} }, eventCode: 't:abc', type: '13' })); + expect(eventCode).toBe('t:abc'); + }); + + it('falls back to the trigger payload body dict (iOS expo-style body key)', () => { + const { eventCode } = extractPushNotificationData(makeRequest(null, { aps: {}, body: { eventCode: 'C:55' } })); + expect(eventCode).toBe('C:55'); + }); + + it('falls back to an aps-nested eventCode (FCM-relayed APNs override)', () => { + const { eventCode } = extractPushNotificationData(makeRequest({}, { aps: { category: 'chats', eventCode: 'g:77' } })); + expect(eventCode).toBe('g:77'); + }); + + it('returns undefined when no eventCode exists anywhere', () => { + const { eventCode, data } = extractPushNotificationData(makeRequest({ foo: 'bar' }, { aps: {} })); + expect(eventCode).toBeUndefined(); + expect(data).toEqual({ foo: 'bar' }); + }); +}); diff --git a/src/services/__tests__/push-notification.test.ts b/src/services/__tests__/push-notification.test.ts index 4ff64863..7e43f6b0 100644 --- a/src/services/__tests__/push-notification.test.ts +++ b/src/services/__tests__/push-notification.test.ts @@ -1,12 +1,21 @@ import { usePushNotificationModalStore } from '@/stores/push-notification/store'; -// Mock the store +// Mock the store — keep the real (pure) parseNotificationData so the service's +// tap routing decisions are exercised against the actual parser. jest.mock('@/stores/push-notification/store', () => ({ + parseNotificationData: jest.requireActual('@/stores/push-notification/store').parseNotificationData, usePushNotificationModalStore: { getState: jest.fn(), }, })); +// The real store module pulls in the sound service (expo-audio); stub it out. +jest.mock('@/services/notification-sound.service', () => ({ + notificationSoundService: { + playNotificationSound: jest.fn(() => Promise.resolve()), + }, +})); + // Mock expo-device jest.mock('expo-device', () => ({ isDevice: true, @@ -443,6 +452,7 @@ describe('Push Notification Service Integration', () => { describe('cold-start tap dedupe', () => { let pushNotificationService: typeof import('../push-notification').pushNotificationService; + let freshRouterPushWithRetry: jest.Mock; beforeEach(() => { jest.clearAllMocks(); @@ -458,6 +468,10 @@ describe('Push Notification Service Integration', () => { (freshStore.getState as jest.Mock).mockReturnValue({ showNotificationModal: mockShowNotificationModal, }); + + // Same story for the navigation mock the fresh service module captured. + freshRouterPushWithRetry = require('@/lib/navigation').routerPushWithRetry as jest.Mock; + freshRouterPushWithRetry.mockResolvedValue(undefined); }); afterEach(() => { @@ -489,8 +503,10 @@ describe('Push Notification Service Integration', () => { await Promise.resolve(); jest.advanceTimersByTime(1000); - expect(mockShowNotificationModal).toHaveBeenCalledTimes(1); - expect(mockShowNotificationModal).toHaveBeenCalledWith(expect.objectContaining({ eventCode: 'C:77' })); + // A call tap deep-links to the call detail exactly once; no modal fallback. + expect(freshRouterPushWithRetry).toHaveBeenCalledTimes(1); + expect(freshRouterPushWithRetry).toHaveBeenCalledWith({ pathname: '/call/[id]', params: { id: '77' } }, expect.objectContaining({ maxAttempts: 40, retryDelayMs: 250 })); + expect(mockShowNotificationModal).not.toHaveBeenCalled(); }); it('still handles distinct notification taps separately', async () => { @@ -509,7 +525,128 @@ describe('Push Notification Service Integration', () => { responseHandler(makeResponse('tap-b', 'C:2')); jest.advanceTimersByTime(400); - expect(mockShowNotificationModal).toHaveBeenCalledTimes(2); + expect(freshRouterPushWithRetry).toHaveBeenCalledTimes(2); + expect(freshRouterPushWithRetry).toHaveBeenCalledWith({ pathname: '/call/[id]', params: { id: '1' } }, expect.anything()); + expect(freshRouterPushWithRetry).toHaveBeenCalledWith({ pathname: '/call/[id]', params: { id: '2' } }, expect.anything()); + }); + }); + + describe('tap deep-linking', () => { + let pushNotificationService: typeof import('../push-notification').pushNotificationService; + let freshRouterPushWithRetry: jest.Mock; + let responseHandler: (r: unknown) => void; + + const makeResponse = (id: string, data: Record | undefined, triggerPayload?: Record): unknown => ({ + actionIdentifier: 'default', + notification: { + request: { + identifier: id, + content: { title: 'T', body: 'B', data }, + trigger: triggerPayload === undefined ? { type: 'push' } : { type: 'push', payload: triggerPayload }, + }, + }, + }); + + beforeEach(async () => { + jest.clearAllMocks(); + jest.resetModules(); + + jest.unmock('../push-notification'); + const module = require('../push-notification'); + pushNotificationService = module.pushNotificationService; + + const freshStore = require('@/stores/push-notification/store').usePushNotificationModalStore; + (freshStore.getState as jest.Mock).mockReturnValue({ + showNotificationModal: mockShowNotificationModal, + }); + + freshRouterPushWithRetry = require('@/lib/navigation').routerPushWithRetry as jest.Mock; + freshRouterPushWithRetry.mockResolvedValue(undefined); + + jest.useFakeTimers(); + await pushNotificationService.initialize(); + responseHandler = (mockAddNotificationResponseReceivedListener.mock.calls as unknown[][])[0]?.[0] as (r: unknown) => void; + }); + + afterEach(() => { + pushNotificationService.cleanup(); + jest.useRealTimers(); + }); + + it('navigates to the call detail for a call tap', () => { + responseHandler(makeResponse('tap-call', { eventCode: 'C:123' })); + jest.advanceTimersByTime(400); + + expect(freshRouterPushWithRetry).toHaveBeenCalledWith({ pathname: '/call/[id]', params: { id: '123' } }, expect.objectContaining({ maxAttempts: 40, retryDelayMs: 250 })); + expect(mockShowNotificationModal).not.toHaveBeenCalled(); + }); + + it.each([ + ['t:chan-1', 'chan-1'], + ['g:group-2', 'group-2'], + ['T:chan-3', 'chan-3'], + ['G:group-4', 'group-4'], + ])('navigates to the chat conversation for %s', (eventCode, channelId) => { + responseHandler(makeResponse(`tap-${eventCode}`, { eventCode })); + jest.advanceTimersByTime(400); + + expect(freshRouterPushWithRetry).toHaveBeenCalledWith({ pathname: '/chat/[channelId]', params: { channelId } }, expect.objectContaining({ maxAttempts: 40, retryDelayMs: 250 })); + expect(mockShowNotificationModal).not.toHaveBeenCalled(); + }); + + it('falls back to the modal for a message tap', () => { + responseHandler(makeResponse('tap-msg', { eventCode: 'M:5' })); + jest.advanceTimersByTime(400); + + expect(freshRouterPushWithRetry).not.toHaveBeenCalled(); + expect(mockShowNotificationModal).toHaveBeenCalledWith(expect.objectContaining({ eventCode: 'M:5' })); + }); + + it('falls back to the modal when the call id is not a safe route param', () => { + responseHandler(makeResponse('tap-bad-call', { eventCode: 'C:1/2' })); + jest.advanceTimersByTime(400); + + expect(freshRouterPushWithRetry).not.toHaveBeenCalled(); + expect(mockShowNotificationModal).toHaveBeenCalledWith(expect.objectContaining({ eventCode: 'C:1/2' })); + }); + + it('routes an iOS tap whose eventCode only exists on the raw trigger payload', () => { + responseHandler(makeResponse('tap-ios', {}, { aps: { alert: {} }, eventCode: 'C:55' })); + jest.advanceTimersByTime(400); + + expect(freshRouterPushWithRetry).toHaveBeenCalledWith({ pathname: '/call/[id]', params: { id: '55' } }, expect.anything()); + expect(mockShowNotificationModal).not.toHaveBeenCalled(); + }); + + describe('notifee press events', () => { + const getNotifeeForegroundHandler = (): ((event: unknown) => Promise) => (mockOnForegroundEvent.mock.calls as unknown[][])[0]?.[0] as (event: unknown) => Promise; + + const makePressEvent = (data: Record): unknown => ({ + // EventType.PRESS + type: 1, + detail: { notification: { id: 'notifee-1', title: 'T', body: 'B', data } }, + }); + + it('deep-links a notifee press with a chat eventCode', async () => { + await getNotifeeForegroundHandler()(makePressEvent({ eventCode: 'g:room-1' })); + + expect(freshRouterPushWithRetry).toHaveBeenCalledWith({ pathname: '/chat/[channelId]', params: { channelId: 'room-1' } }, expect.anything()); + expect(mockShowNotificationModal).not.toHaveBeenCalled(); + }); + + it('deep-links a notifee press with a call eventCode', async () => { + await getNotifeeForegroundHandler()(makePressEvent({ eventCode: 'C:42' })); + + expect(freshRouterPushWithRetry).toHaveBeenCalledWith({ pathname: '/call/[id]', params: { id: '42' } }, expect.anything()); + expect(mockShowNotificationModal).not.toHaveBeenCalled(); + }); + + it('shows the modal for a notifee press without a routable eventCode', async () => { + await getNotifeeForegroundHandler()(makePressEvent({ eventCode: 'M:9' })); + + expect(freshRouterPushWithRetry).not.toHaveBeenCalled(); + expect(mockShowNotificationModal).toHaveBeenCalledWith(expect.objectContaining({ eventCode: 'M:9' })); + }); }); }); diff --git a/src/services/push-notification.ts b/src/services/push-notification.ts index 3975ef49..025faf1e 100644 --- a/src/services/push-notification.ts +++ b/src/services/push-notification.ts @@ -13,7 +13,7 @@ import { useCoreStore } from '@/stores/app/core-store'; import { useLocationStore } from '@/stores/app/location-store'; import useAuthStore from '@/stores/auth/store'; import { useCheckInTimerStore } from '@/stores/check-in-timers/store'; -import { usePushNotificationModalStore } from '@/stores/push-notification/store'; +import { parseNotificationData, usePushNotificationModalStore } from '@/stores/push-notification/store'; import { securityStore } from '@/stores/security/store'; // Numeric values for the CheckInType field expected by the API. @@ -34,13 +34,45 @@ export interface PushNotificationData { data?: Record; } +/** + * Pulls the Resgrid eventCode (and the data record that carried it) out of a + * notification request. On Android the FCM data payload is surfaced as + * content.data, but on iOS expo-notifications only maps the APNs custom key + * "body" to content.data — Core sends eventCode as a top-level custom key (or + * nested under aps for FCM-relayed APNs), so content.data is empty there and we + * must fall back to the raw push payload exposed on the trigger. + */ +export function extractPushNotificationData(request: Notifications.NotificationRequest): { eventCode: string | undefined; data: Record } { + const contentData = request.content.data; + if (contentData && typeof contentData === 'object' && typeof (contentData as Record).eventCode === 'string') { + return { eventCode: (contentData as Record).eventCode as string, data: contentData as Record }; + } + + const trigger = request.trigger as { payload?: Record } | null | undefined; + const payload = trigger && typeof trigger === 'object' ? trigger.payload : undefined; + if (payload && typeof payload === 'object') { + const candidates = [payload, payload.body, payload.aps]; + for (const candidate of candidates) { + if (candidate && typeof candidate === 'object' && typeof (candidate as Record).eventCode === 'string') { + return { eventCode: (candidate as Record).eventCode as string, data: candidate as Record }; + } + } + } + + return { + eventCode: undefined, + data: contentData && typeof contentData === 'object' ? (contentData as Record) : {}, + }; +} + /** * Handles chat push deep-links. Chat notifications carry an eventCode of * "t:{channelId}" (direct message) or "g:{channelId}" (group/channel); both - * navigate to the chat conversation route. + * navigate to the chat conversation route. Case-insensitive so the legacy + * uppercase prefixes deep-link too. */ export function handleChatDeepLink(eventCode: string): boolean { - const match = /^([tg]):(.+)$/.exec(eventCode); + const match = /^([tg]):(.+)$/i.exec(eventCode); if (!match) return false; const channelId = match[2]; if (/[/\\?#]/.test(channelId)) return false; @@ -238,13 +270,51 @@ class PushNotificationService { void usePushNotificationModalStore.getState().showNotificationModal(notificationData); } + /** + * Deep-links a tapped notification straight to its screen when the eventCode + * routes somewhere: chat ("t:"/"g:") to the conversation, calls ("C:{id}") to + * the call detail. Returns true when navigation was started so the caller can + * skip the modal fallback. + */ + private tryDeepLinkForData(data: Record | undefined): boolean { + const eventCode = data?.eventCode; + if (typeof eventCode !== 'string') { + return false; + } + + // Chat notifications: eventCode "t:{channelId}" (DM) / "g:{channelId}" (group). + if (handleChatDeepLink(eventCode)) { + return true; + } + + const parsed = parseNotificationData({ eventCode, data }); + if (parsed.type === 'call' && parsed.id !== '' && !/[/\\?#]/.test(parsed.id)) { + void routerPushWithRetry( + { pathname: '/call/[id]', params: { id: parsed.id } }, + { + maxAttempts: 40, + retryDelayMs: 250, + // On a cold start the session is still hydrating — see handleChatDeepLink. + waitUntil: () => useAuthStore.getState().status === 'signedIn', + } + ).catch((error) => { + logger.error({ message: 'Failed to deep-link to call from push notification', context: { error, eventCode } }); + }); + return true; + } + + return false; + } + // Foreground push received via expo-notifications. private handleNotificationReceived = (notification: Notifications.Notification): void => { - const data = notification.request.content.data as Record | undefined; + // iOS remote pushes don't surface Core's custom keys on content.data — pull + // the eventCode out of the raw trigger payload so the modal still shows. + const { eventCode, data } = extractPushNotificationData(notification.request); logger.info({ message: 'Notification received', - context: { data }, + context: { eventCode }, }); this.showModalForData(data, notification.request.content.title, notification.request.content.body); @@ -277,18 +347,20 @@ class PushNotificationService { } const content = request.content; - const data = content.data as Record | undefined; + // iOS remote pushes don't surface Core's custom keys on content.data — the + // extractor falls back to the raw trigger payload so taps still route. + const { eventCode, data } = extractPushNotificationData(request); logger.info({ message: 'Notification response received (tap)', - context: { data, actionIdentifier: response.actionIdentifier, source }, + context: { eventCode, actionIdentifier: response.actionIdentifier, source }, }); // Delay so the React tree is mounted and the modal store is ready. setTimeout(() => { - // Deep-link chat notifications: eventCode "t:{channelId}" (DM) / "g:{channelId}" (group). - const eventCode = data?.eventCode; - if (typeof eventCode === 'string' && handleChatDeepLink(eventCode)) { + // Deep-link chat and call notifications straight to their screen; anything + // else falls back to the persistent modal. + if (this.tryDeepLinkForData(data)) { return; } this.showModalForData(data, content.title, content.body); @@ -314,9 +386,11 @@ class PushNotificationService { await this.handleCheckInAction(); } - // Handle notification press → modal + // Handle notification press → deep link when the eventCode routes somewhere, modal otherwise if (type === EventType.PRESS && detail.notification) { - this.showModalForData(detail.notification.data, detail.notification.title, detail.notification.body); + if (!this.tryDeepLinkForData(detail.notification.data)) { + this.showModalForData(detail.notification.data, detail.notification.title, detail.notification.body); + } } }); @@ -331,7 +405,9 @@ class PushNotificationService { } if (type === EventType.PRESS && detail.notification) { - this.showModalForData(detail.notification.data, detail.notification.title, detail.notification.body); + if (!this.tryDeepLinkForData(detail.notification.data)) { + this.showModalForData(detail.notification.data, detail.notification.title, detail.notification.body); + } } }); } diff --git a/src/stores/push-notification/__tests__/store.test.ts b/src/stores/push-notification/__tests__/store.test.ts index f2e2dd93..7246ca46 100644 --- a/src/stores/push-notification/__tests__/store.test.ts +++ b/src/stores/push-notification/__tests__/store.test.ts @@ -1,6 +1,6 @@ import { logger } from '@/lib/logging'; import { notificationSoundService } from '@/services/notification-sound.service'; -import { usePushNotificationModalStore } from '../store'; +import { parseNotificationData, usePushNotificationModalStore } from '../store'; // Mock logger service jest.mock('@/lib/logging', () => ({ @@ -367,4 +367,58 @@ describe('usePushNotificationModalStore', () => { expect(parsed.eventCode).toBe(''); }); }); + + describe('parseNotificationData (colon-form event codes)', () => { + it.each([ + ['C:1234', 'call', '1234'], + ['M:5678', 'message', '5678'], + ['T:9101', 'chat', '9101'], + ['G:1121', 'group-chat', '1121'], + // New chat system sends lowercase prefixes WITH a colon. + ['t:chan-abc', 'chat', 'chan-abc'], + ['g:group-xyz', 'group-chat', 'group-xyz'], + ['c:42', 'call', '42'], + ['m:7', 'message', '7'], + ])('parses %s as %s with id %s', (eventCode, type, id) => { + const parsed = parseNotificationData({ eventCode }); + + expect(parsed.type).toBe(type); + expect(parsed.id).toBe(id); + expect(parsed.eventCode).toBe(eventCode); + }); + + it('splits on the first colon only so an id containing a colon survives intact', () => { + const parsed = parseNotificationData({ eventCode: 'T:9:10' }); + + expect(parsed.type).toBe('chat'); + expect(parsed.id).toBe('9:10'); + }); + + it('parses an unknown colon prefix as unknown but keeps the id', () => { + const parsed = parseNotificationData({ eventCode: 'X:9999' }); + + expect(parsed.type).toBe('unknown'); + expect(parsed.id).toBe('9999'); + }); + + it('still parses the legacy colon-less form', () => { + const parsed = parseNotificationData({ eventCode: 'C1234' }); + + expect(parsed.type).toBe('call'); + expect(parsed.id).toBe('1234'); + }); + + it('carries title, body and data through unchanged', () => { + const parsed = parseNotificationData({ + eventCode: 'C:1', + title: 'Call', + body: 'Dispatch', + data: { eventCode: 'C:1', extra: 'x' }, + }); + + expect(parsed.title).toBe('Call'); + expect(parsed.body).toBe('Dispatch'); + expect(parsed.data).toEqual({ eventCode: 'C:1', extra: 'x' }); + }); + }); }); diff --git a/src/stores/push-notification/store.ts b/src/stores/push-notification/store.ts index 50dd9a73..f9dcc28d 100644 --- a/src/stores/push-notification/store.ts +++ b/src/stores/push-notification/store.ts @@ -29,41 +29,50 @@ interface PushNotificationModalState { parseNotification: (notificationData: PushNotificationData) => ParsedNotification; } +// First character of the event code prefix sent by the Resgrid backend, e.g. +// "C:1234" call, "M:5678" message, "t:9012" chat, "g:3456" group chat. +const EVENT_CODE_PREFIXES: Record = { + c: 'call', + m: 'message', + t: 'chat', + g: 'group-chat', +}; + +export const parseNotificationData = (notificationData: PushNotificationData): ParsedNotification => { + const eventCode = notificationData.eventCode || ''; + let type: NotificationType = 'unknown'; + let id = ''; + + const separatorIndex = eventCode.indexOf(':'); + + if (separatorIndex > 0) { + // Colon form ("C:1234", "t:{channelId}"): split on the FIRST colon only, so + // an id that itself contains one survives intact. + const lowerPrefix = eventCode.slice(0, separatorIndex).toLowerCase(); + type = EVENT_CODE_PREFIXES[lowerPrefix.charAt(0)] ?? 'unknown'; + id = eventCode.slice(separatorIndex + 1); + } else if (eventCode.length > 1) { + // Legacy colon-less form ("C1234"): first character is the type prefix, the + // rest is the id. + type = EVENT_CODE_PREFIXES[eventCode.charAt(0).toLowerCase()] ?? 'unknown'; + id = eventCode.slice(1); + } + + return { + type, + id, + eventCode, + title: notificationData.title, + body: notificationData.body, + data: notificationData.data, + }; +}; + export const usePushNotificationModalStore = create((set, get) => ({ isOpen: false, notification: null, - parseNotification: (notificationData: PushNotificationData): ParsedNotification => { - const eventCode = notificationData.eventCode || ''; - let type: NotificationType = 'unknown'; - let id = ''; - - // Parse event code format like "C1234", "M5678", "T9012", "G3456" - // First character is the type prefix, rest is the ID - if (eventCode && eventCode.length > 1) { - const prefix = eventCode[0].toLowerCase(); - id = eventCode.slice(1); - - if (prefix === 'c') { - type = 'call'; - } else if (prefix === 'm') { - type = 'message'; - } else if (prefix === 't') { - type = 'chat'; - } else if (prefix === 'g') { - type = 'group-chat'; - } - } - - return { - type, - id, - eventCode, - title: notificationData.title, - body: notificationData.body, - data: notificationData.data, - }; - }, + parseNotification: (notificationData: PushNotificationData): ParsedNotification => parseNotificationData(notificationData), showNotificationModal: async (notificationData: PushNotificationData) => { const parsedNotification = get().parseNotification(notificationData); From 36085fa32e6e1fe6cc50a87cc34420e7ffb79876 Mon Sep 17 00:00:00 2001 From: Shawn Jackson Date: Tue, 18 Aug 2026 22:37:11 -0700 Subject: [PATCH 2/2] RG-T132 PR#275 fixes --- .../push-notification-modal.test.tsx | 30 +++++ .../push-notification-modal.tsx | 6 +- .../push-notification-chat-deeplink.test.ts | 17 +-- .../__tests__/push-notification.test.ts | 36 ++++-- src/services/push-notification.ts | 111 ++++++++++-------- src/stores/push-notification/store.ts | 8 ++ 6 files changed, 145 insertions(+), 63 deletions(-) diff --git a/src/components/push-notification/__tests__/push-notification-modal.test.tsx b/src/components/push-notification/__tests__/push-notification-modal.test.tsx index f11fc5f7..5e273b2f 100644 --- a/src/components/push-notification/__tests__/push-notification-modal.test.tsx +++ b/src/components/push-notification/__tests__/push-notification-modal.test.tsx @@ -71,6 +71,9 @@ jest.mock('@/hooks/use-analytics', () => ({ })); jest.mock('@/stores/push-notification/store', () => ({ + // Keep the real isSafeRouteId so the tests exercise the same call id validation the + // deep-link handler uses, rather than a stub that always says yes. + ...jest.requireActual('@/stores/push-notification/store'), usePushNotificationModalStore: jest.fn(), })); @@ -277,6 +280,33 @@ describe('PushNotificationModal', () => { }); }); + // The call id comes from the push payload and is interpolated into the route path, so the same + // separator/query/fragment rejection the deep-link handler applies has to hold here too. + it.each(['../chat/9999', 'a\\b', 'a?x=1', 'a#fragment', ''])('should not offer the call button for unsafe call id %s', (id) => { + const hideNotificationModalMock = jest.fn(); + const state = { + ...mockStore, + isOpen: true, + notification: { + type: 'call' as const, + id, + eventCode: `C:${id}`, + title: 'Emergency Call', + body: 'Structure fire', + }, + hideNotificationModal: hideNotificationModalMock, + }; + + (usePushNotificationModalStore as unknown as jest.Mock).mockImplementation((selector: any) => (typeof selector === 'function' ? selector(state) : state)); + + render(); + + expect(screen.queryByText('View call')).toBeNull(); + expect(router.push).not.toHaveBeenCalled(); + expect(hideNotificationModalMock).not.toHaveBeenCalled(); + expect(mockAnalytics.trackEvent).not.toHaveBeenCalledWith('push_notification_view_call_pressed', expect.any(Object)); + }); + it('should handle view call button press', async () => { const hideNotificationModalMock = jest.fn(); diff --git a/src/components/push-notification/push-notification-modal.tsx b/src/components/push-notification/push-notification-modal.tsx index 099b39b9..b10951a0 100644 --- a/src/components/push-notification/push-notification-modal.tsx +++ b/src/components/push-notification/push-notification-modal.tsx @@ -9,7 +9,7 @@ import { Modal, ModalBackdrop, ModalBody, ModalContent, ModalFooter, ModalHeader import { Text } from '@/components/ui/text'; import { VStack } from '@/components/ui/vstack'; import { useAnalytics } from '@/hooks/use-analytics'; -import { type NotificationType, usePushNotificationModalStore } from '@/stores/push-notification/store'; +import { isSafeRouteId, type NotificationType, usePushNotificationModalStore } from '@/stores/push-notification/store'; const NotificationIcon = ({ type }: { type: NotificationType }) => { const iconSize = 24; @@ -47,7 +47,7 @@ export const PushNotificationModal: React.FC = () => { }; const handleViewCall = () => { - if (notification?.type === 'call' && notification.id) { + if (notification?.type === 'call' && isSafeRouteId(notification.id)) { trackEvent('push_notification_view_call_pressed', { id: notification.id, eventCode: notification.eventCode, @@ -124,7 +124,7 @@ export const PushNotificationModal: React.FC = () => { {t('common.dismiss')} - {notification.type === 'call' && notification.id ? ( + {notification.type === 'call' && isSafeRouteId(notification.id) ? ( diff --git a/src/services/__tests__/push-notification-chat-deeplink.test.ts b/src/services/__tests__/push-notification-chat-deeplink.test.ts index 8dfa1fc5..10c26e68 100644 --- a/src/services/__tests__/push-notification-chat-deeplink.test.ts +++ b/src/services/__tests__/push-notification-chat-deeplink.test.ts @@ -129,13 +129,13 @@ describe('handleChatDeepLink', () => { ['g:9101', '9101'], ['T:channel-1', 'channel-1'], ['G:9101', '9101'], - ])('navigates with explicit route params for %s', (eventCode, channelId) => { - expect(handleChatDeepLink(eventCode)).toBe(true); + ])('navigates with explicit route params for %s', async (eventCode, channelId) => { + await expect(handleChatDeepLink(eventCode)).resolves.toBe(true); expect(push).toHaveBeenCalledWith({ pathname: '/chat/[channelId]', params: { channelId } }); }); - it.each(['t:a/b', 't:a\\b', 'g:a?x=1', 'g:a#fragment', 'x:123', 't:', 'notacode', ':missingprefix'])('rejects invalid payload %s', (eventCode) => { - expect(handleChatDeepLink(eventCode)).toBe(false); + it.each(['t:a/b', 't:a\\b', 'g:a?x=1', 'g:a#fragment', 'x:123', 't:', 'notacode', ':missingprefix'])('rejects invalid payload %s', async (eventCode) => { + await expect(handleChatDeepLink(eventCode)).resolves.toBe(false); expect(push).not.toHaveBeenCalled(); }); @@ -146,26 +146,29 @@ describe('handleChatDeepLink', () => { }) .mockImplementationOnce(() => undefined); - expect(handleChatDeepLink('t:channel-1')).toBe(true); + const navigated = handleChatDeepLink('t:channel-1'); expect(push).toHaveBeenCalledTimes(1); await jest.advanceTimersByTimeAsync(300); expect(push).toHaveBeenCalledTimes(2); + await expect(navigated).resolves.toBe(true); expect(logError).not.toHaveBeenCalled(); }); - it('logs an error after exhausting navigation retries', async () => { + it('resolves false and logs after exhausting navigation retries', async () => { push.mockImplementation(() => { throw new Error('router not ready'); }); - expect(handleChatDeepLink('t:channel-1')).toBe(true); + const navigated = handleChatDeepLink('t:channel-1'); // Budget is 40 attempts x 250ms so a cold start has ~10s to mount and hydrate. await jest.advanceTimersByTimeAsync(250 * 40); expect(push).toHaveBeenCalledTimes(40); + // Resolving false is what tells the tap handler to fall back to the notification modal. + await expect(navigated).resolves.toBe(false); expect(logError).toHaveBeenCalledWith(expect.objectContaining({ message: 'Failed to deep-link to chat channel' })); }); }); diff --git a/src/services/__tests__/push-notification.test.ts b/src/services/__tests__/push-notification.test.ts index 7e43f6b0..c86755b9 100644 --- a/src/services/__tests__/push-notification.test.ts +++ b/src/services/__tests__/push-notification.test.ts @@ -1,9 +1,9 @@ import { usePushNotificationModalStore } from '@/stores/push-notification/store'; -// Mock the store — keep the real (pure) parseNotificationData so the service's -// tap routing decisions are exercised against the actual parser. +// Mock the store — keep the real (pure) parseNotificationData and isSafeRouteId so the +// service's tap routing decisions are exercised against the actual parser and validation. jest.mock('@/stores/push-notification/store', () => ({ - parseNotificationData: jest.requireActual('@/stores/push-notification/store').parseNotificationData, + ...jest.requireActual('@/stores/push-notification/store'), usePushNotificationModalStore: { getState: jest.fn(), }, @@ -594,22 +594,44 @@ describe('Push Notification Service Integration', () => { expect(mockShowNotificationModal).not.toHaveBeenCalled(); }); - it('falls back to the modal for a message tap', () => { + // The tap handler awaits the deep-link before falling back, so the modal lands a + // microtask after the timer — advanceTimersByTimeAsync flushes both. + it('falls back to the modal for a message tap', async () => { responseHandler(makeResponse('tap-msg', { eventCode: 'M:5' })); - jest.advanceTimersByTime(400); + await jest.advanceTimersByTimeAsync(400); expect(freshRouterPushWithRetry).not.toHaveBeenCalled(); expect(mockShowNotificationModal).toHaveBeenCalledWith(expect.objectContaining({ eventCode: 'M:5' })); }); - it('falls back to the modal when the call id is not a safe route param', () => { + it('falls back to the modal when the call id is not a safe route param', async () => { responseHandler(makeResponse('tap-bad-call', { eventCode: 'C:1/2' })); - jest.advanceTimersByTime(400); + await jest.advanceTimersByTimeAsync(400); expect(freshRouterPushWithRetry).not.toHaveBeenCalled(); expect(mockShowNotificationModal).toHaveBeenCalledWith(expect.objectContaining({ eventCode: 'C:1/2' })); }); + it('falls back to the modal when the call deep-link never lands', async () => { + freshRouterPushWithRetry.mockRejectedValueOnce(new Error('navigation never became ready')); + + responseHandler(makeResponse('tap-call-failed', { eventCode: 'C:123' })); + await jest.advanceTimersByTimeAsync(400); + + expect(freshRouterPushWithRetry).toHaveBeenCalledWith({ pathname: '/call/[id]', params: { id: '123' } }, expect.objectContaining({ maxAttempts: 40 })); + expect(mockShowNotificationModal).toHaveBeenCalledWith(expect.objectContaining({ eventCode: 'C:123' })); + }); + + it('falls back to the modal when the chat deep-link never lands', async () => { + freshRouterPushWithRetry.mockRejectedValueOnce(new Error('navigation never became ready')); + + responseHandler(makeResponse('tap-chat-failed', { eventCode: 'g:group-9' })); + await jest.advanceTimersByTimeAsync(400); + + expect(freshRouterPushWithRetry).toHaveBeenCalledWith({ pathname: '/chat/[channelId]', params: { channelId: 'group-9' } }, expect.objectContaining({ maxAttempts: 40 })); + expect(mockShowNotificationModal).toHaveBeenCalledWith(expect.objectContaining({ eventCode: 'g:group-9' })); + }); + it('routes an iOS tap whose eventCode only exists on the raw trigger payload', () => { responseHandler(makeResponse('tap-ios', {}, { aps: { alert: {} }, eventCode: 'C:55' })); jest.advanceTimersByTime(400); diff --git a/src/services/push-notification.ts b/src/services/push-notification.ts index 025faf1e..de2bd8fd 100644 --- a/src/services/push-notification.ts +++ b/src/services/push-notification.ts @@ -1,19 +1,20 @@ import notifee, { AndroidImportance, AndroidVisibility, AuthorizationStatus, EventType } from '@notifee/react-native'; import * as Device from 'expo-device'; import * as Notifications from 'expo-notifications'; +import type { Href } from 'expo-router'; import { useEffect, useRef } from 'react'; import { Platform } from 'react-native'; import { registerUnitDevice } from '@/api/devices/push'; import { logger } from '@/lib/logging'; -import { routerPushWithRetry } from '@/lib/navigation'; +import { type RouterPushRetryOptions, routerPushWithRetry } from '@/lib/navigation'; import { getDeviceUuid } from '@/lib/storage/app'; import { getAppliedNotificationSoundMode, getModernNotificationSoundsEnabled, setAppliedNotificationSoundMode } from '@/lib/storage/notification-prefs'; import { useCoreStore } from '@/stores/app/core-store'; import { useLocationStore } from '@/stores/app/location-store'; import useAuthStore from '@/stores/auth/store'; import { useCheckInTimerStore } from '@/stores/check-in-timers/store'; -import { parseNotificationData, usePushNotificationModalStore } from '@/stores/push-notification/store'; +import { isSafeRouteId, parseNotificationData, usePushNotificationModalStore } from '@/stores/push-notification/store'; import { securityStore } from '@/stores/security/store'; // Numeric values for the CheckInType field expected by the API. @@ -66,30 +67,57 @@ export function extractPushNotificationData(request: Notifications.NotificationR } /** - * Handles chat push deep-links. Chat notifications carry an eventCode of + * Recognises chat push deep-links. Chat notifications carry an eventCode of * "t:{channelId}" (direct message) or "g:{channelId}" (group/channel); both * navigate to the chat conversation route. Case-insensitive so the legacy - * uppercase prefixes deep-link too. + * uppercase prefixes deep-link too. Returns the channel id, or null when the + * eventCode is not a chat deep-link. */ -export function handleChatDeepLink(eventCode: string): boolean { +export function parseChatDeepLink(eventCode: string): string | null { const match = /^([tg]):(.+)$/i.exec(eventCode); - if (!match) return false; + if (!match) return null; const channelId = match[2]; - if (/[/\\?#]/.test(channelId)) return false; - void routerPushWithRetry( - { pathname: '/chat/[channelId]', params: { channelId } }, - { - maxAttempts: 40, - retryDelayMs: 250, - // On a cold start the session is still hydrating. Pushing a protected route before - // it settles gets the route replaced by the auth guard, which is indistinguishable - // from the tap doing nothing at all. - waitUntil: () => useAuthStore.getState().status === 'signedIn', - } - ).catch((error) => { - logger.error({ message: 'Failed to deep-link to chat channel', context: { error, eventCode } }); - }); - return true; + if (!isSafeRouteId(channelId)) return null; + return channelId; +} + +/** + * Retry budget for every push deep-link: 40 x 250ms gives a cold start ~10s to mount the + * root layout and hydrate the session before the push is given up on. + * + * On a cold start the session is still hydrating. Pushing a protected route before + * it settles gets the route replaced by the auth guard, which is indistinguishable + * from the tap doing nothing at all. + */ +const DEEP_LINK_RETRY_OPTIONS: RouterPushRetryOptions = { + maxAttempts: 40, + retryDelayMs: 250, + waitUntil: () => useAuthStore.getState().status === 'signedIn', +}; + +/** + * Resolves true when the push landed on its route, false once the retry budget is spent so + * the caller can fall back to the modal. + */ +async function deepLinkWithRetry(href: Href, failureMessage: string, eventCode: string): Promise { + try { + await routerPushWithRetry(href, DEEP_LINK_RETRY_OPTIONS); + return true; + } catch (error) { + logger.error({ message: failureMessage, context: { error, eventCode } }); + return false; + } +} + +/** + * Resolves true when the tap was navigated, false when it was not a chat deep-link or the + * navigation never landed. A false result means the caller still owes the user a fallback — + * silently giving up leaves the app sitting on whatever screen it opened to. + */ +export async function handleChatDeepLink(eventCode: string): Promise { + const channelId = parseChatDeepLink(eventCode); + if (!channelId) return false; + return deepLinkWithRetry({ pathname: '/chat/[channelId]', params: { channelId } }, 'Failed to deep-link to chat channel', eventCode); } // Configure how notifications are presented while the app is in the foreground. @@ -273,34 +301,24 @@ class PushNotificationService { /** * Deep-links a tapped notification straight to its screen when the eventCode * routes somewhere: chat ("t:"/"g:") to the conversation, calls ("C:{id}") to - * the call detail. Returns true when navigation was started so the caller can - * skip the modal fallback. + * the call detail. Resolves true only once the navigation has landed, so a + * deep-link that never arrives (cold start where the session never hydrates) + * still leaves the caller its modal fallback instead of a logged no-op. */ - private tryDeepLinkForData(data: Record | undefined): boolean { + private async tryDeepLinkForData(data: Record | undefined): Promise { const eventCode = data?.eventCode; if (typeof eventCode !== 'string') { return false; } // Chat notifications: eventCode "t:{channelId}" (DM) / "g:{channelId}" (group). - if (handleChatDeepLink(eventCode)) { - return true; + if (parseChatDeepLink(eventCode)) { + return handleChatDeepLink(eventCode); } const parsed = parseNotificationData({ eventCode, data }); - if (parsed.type === 'call' && parsed.id !== '' && !/[/\\?#]/.test(parsed.id)) { - void routerPushWithRetry( - { pathname: '/call/[id]', params: { id: parsed.id } }, - { - maxAttempts: 40, - retryDelayMs: 250, - // On a cold start the session is still hydrating — see handleChatDeepLink. - waitUntil: () => useAuthStore.getState().status === 'signedIn', - } - ).catch((error) => { - logger.error({ message: 'Failed to deep-link to call from push notification', context: { error, eventCode } }); - }); - return true; + if (parsed.type === 'call' && isSafeRouteId(parsed.id)) { + return deepLinkWithRetry({ pathname: '/call/[id]', params: { id: parsed.id } }, 'Failed to deep-link to call from push notification', eventCode); } return false; @@ -359,11 +377,12 @@ class PushNotificationService { // Delay so the React tree is mounted and the modal store is ready. setTimeout(() => { // Deep-link chat and call notifications straight to their screen; anything - // else falls back to the persistent modal. - if (this.tryDeepLinkForData(data)) { - return; - } - this.showModalForData(data, content.title, content.body); + // else — and any deep-link that never lands — falls back to the persistent modal. + void this.tryDeepLinkForData(data).then((navigated) => { + if (!navigated) { + this.showModalForData(data, content.title, content.body); + } + }); }, delayMs); } @@ -388,7 +407,7 @@ class PushNotificationService { // Handle notification press → deep link when the eventCode routes somewhere, modal otherwise if (type === EventType.PRESS && detail.notification) { - if (!this.tryDeepLinkForData(detail.notification.data)) { + if (!(await this.tryDeepLinkForData(detail.notification.data))) { this.showModalForData(detail.notification.data, detail.notification.title, detail.notification.body); } } @@ -405,7 +424,7 @@ class PushNotificationService { } if (type === EventType.PRESS && detail.notification) { - if (!this.tryDeepLinkForData(detail.notification.data)) { + if (!(await this.tryDeepLinkForData(detail.notification.data))) { this.showModalForData(detail.notification.data, detail.notification.title, detail.notification.body); } } diff --git a/src/stores/push-notification/store.ts b/src/stores/push-notification/store.ts index f9dcc28d..104eb2a0 100644 --- a/src/stores/push-notification/store.ts +++ b/src/stores/push-notification/store.ts @@ -38,6 +38,14 @@ const EVENT_CODE_PREFIXES: Record = { g: 'group-chat', }; +/** + * Ids parsed out of a push event code are attacker-influenced and land straight in a route + * path, so anything that could steer the router elsewhere — a path separator, a query, a + * fragment — disqualifies the id. Shared by the deep-link handlers and the notification modal + * so a tap and a cold-start deep-link accept exactly the same ids. + */ +export const isSafeRouteId = (id: string): boolean => id.length > 0 && !/[/\\?#]/.test(id); + export const parseNotificationData = (notificationData: PushNotificationData): ParsedNotification => { const eventCode = notificationData.eventCode || ''; let type: NotificationType = 'unknown';