From 1c21ae3a0f887537d91a8ef70c5e2f2d68ae82e9 Mon Sep 17 00:00:00 2001 From: Andy Date: Thu, 26 Mar 2026 20:05:27 +0100 Subject: [PATCH 1/4] fix(push): fix Base64 encoding mismatch and pass deviceId as query param The interaso/webpush library decodes p256dh/auth keys with URL-safe Base64 (Base64.getUrlDecoder) but the frontend encoded them with btoa() which produces standard Base64. Keys containing + or / caused silent decode failures, preventing all push notifications from being delivered. Also pass deviceId as a query parameter on mutation requests instead of the X-Device-Id header (which was removed earlier due to CORS preflight failures), so the backend can exclude the originating device from self-notifications. --- .../com/shoppinglist/routes/ListRoutes.kt | 8 +++---- .../src/context/PushNotificationContext.tsx | 2 ++ frontend/src/services/api.ts | 23 +++++++++++++++---- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/backend/src/main/kotlin/com/shoppinglist/routes/ListRoutes.kt b/backend/src/main/kotlin/com/shoppinglist/routes/ListRoutes.kt index 82d784f..86b1bc0 100644 --- a/backend/src/main/kotlin/com/shoppinglist/routes/ListRoutes.kt +++ b/backend/src/main/kotlin/com/shoppinglist/routes/ListRoutes.kt @@ -170,7 +170,7 @@ fun Route.listRoutes(pushNotificationService: PushNotificationService? = null) { call.application.log.warn("Error broadcasting item addition: ${e.message}") } - val deviceId = call.request.header("X-Device-Id") + val deviceId = call.request.queryParameters["deviceId"] call.application.launch(Dispatchers.Default) { pushNotificationService?.notifyListChange(listId, deviceId, "ITEM_ADDED", newItem.text) } @@ -273,7 +273,7 @@ fun Route.listRoutes(pushNotificationService: PushNotificationService? = null) { call.application.log.warn("Error broadcasting item update: ${e.message}") } - val deviceId = call.request.header("X-Device-Id") + val deviceId = call.request.queryParameters["deviceId"] call.application.launch(Dispatchers.Default) { pushNotificationService?.notifyListChange(listId, deviceId, "ITEM_UPDATED", updatedItem.text) } @@ -351,7 +351,7 @@ fun Route.listRoutes(pushNotificationService: PushNotificationService? = null) { call.application.log.warn("Error broadcasting item deletion: ${e.message}") } - val deviceId = call.request.header("X-Device-Id") + val deviceId = call.request.queryParameters["deviceId"] call.application.launch(Dispatchers.Default) { pushNotificationService?.notifyListChange(listId, deviceId, "ITEM_DELETED", null) } @@ -409,7 +409,7 @@ fun Route.listRoutes(pushNotificationService: PushNotificationService? = null) { call.application.log.warn("Error broadcasting items cleared: ${e.message}") } - val deviceId = call.request.header("X-Device-Id") + val deviceId = call.request.queryParameters["deviceId"] call.application.launch(Dispatchers.Default) { pushNotificationService?.notifyListChange(listId, deviceId, "ITEMS_CLEARED", null) } diff --git a/frontend/src/context/PushNotificationContext.tsx b/frontend/src/context/PushNotificationContext.tsx index eb10981..608359f 100644 --- a/frontend/src/context/PushNotificationContext.tsx +++ b/frontend/src/context/PushNotificationContext.tsx @@ -96,9 +96,11 @@ export const PushNotificationProvider: React.FC<{ children: ReactNode }> = ({ ch const p256dh = p256dhKey ? btoa(String.fromCharCode(...new Uint8Array(p256dhKey as ArrayBuffer))) + .replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') : ''; const auth = authKey ? btoa(String.fromCharCode(...new Uint8Array(authKey as ArrayBuffer))) + .replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') : ''; await fetch(`${apiBase}/push/subscribe`, { diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index 330d58e..8a0fed5 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -13,6 +13,10 @@ const getApiUrl = () => { const API_BASE_URL = `${getApiUrl()}/api`; class ApiClient { + private getDeviceId(): string | null { + return localStorage.getItem('shoppimo_device_id'); + } + private async request(endpoint: string, options: RequestInit = {}): Promise { const url = `${API_BASE_URL}${endpoint}`; @@ -64,35 +68,44 @@ class ApiClient { return this.request(`/lists/${listId}`); } + private withDeviceId(endpoint: string): string { + const deviceId = this.getDeviceId(); + if (deviceId) { + const separator = endpoint.includes('?') ? '&' : '?'; + return `${endpoint}${separator}deviceId=${encodeURIComponent(deviceId)}`; + } + return endpoint; + } + async addItem(listId: string, text: string): Promise { - return this.request(`/lists/${listId}/items`, { + return this.request(this.withDeviceId(`/lists/${listId}/items`), { method: 'POST', body: JSON.stringify({ text }), }); } async updateItem(listId: string, itemId: string, text: string): Promise { - return this.request(`/lists/${listId}/items/${itemId}`, { + return this.request(this.withDeviceId(`/lists/${listId}/items/${itemId}`), { method: 'PUT', body: JSON.stringify({ text }), }); } async toggleItem(listId: string, itemId: string, completed: boolean): Promise { - return this.request(`/lists/${listId}/items/${itemId}`, { + return this.request(this.withDeviceId(`/lists/${listId}/items/${itemId}`), { method: 'PUT', body: JSON.stringify({ completed }), }); } async deleteItem(listId: string, itemId: string): Promise { - return this.request(`/lists/${listId}/items/${itemId}`, { + return this.request(this.withDeviceId(`/lists/${listId}/items/${itemId}`), { method: 'DELETE', }); } async clearCompleted(listId: string): Promise { - return this.request(`/lists/${listId}/clear-completed`, { + return this.request(this.withDeviceId(`/lists/${listId}/clear-completed`), { method: 'POST', }); } From ed5027dabfeb7a1e8937ea8000d21fdff77a4877 Mon Sep 17 00:00:00 2001 From: Andy Date: Thu, 26 Mar 2026 20:07:16 +0100 Subject: [PATCH 2/4] introduce new github workflow for branch development --- .github/workflows/feature-docker.yml | 70 ++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 .github/workflows/feature-docker.yml diff --git a/.github/workflows/feature-docker.yml b/.github/workflows/feature-docker.yml new file mode 100644 index 0000000..222786e --- /dev/null +++ b/.github/workflows/feature-docker.yml @@ -0,0 +1,70 @@ +name: Feature Branch Docker + +on: + push: + branches-ignore: + - main + +permissions: + contents: read + packages: write + +jobs: + build-and-push: + name: Build & push Docker images + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Sanitize branch name for Docker tag + id: branch + run: | + RAW="${GITHUB_REF_NAME}" + SANITIZED=$(echo "$RAW" | sed 's/[^a-zA-Z0-9._-]/-/g' | sed 's/^-//;s/-$//' | cut -c1-128) + echo "TAG=$SANITIZED" >> "$GITHUB_OUTPUT" + + - name: Log in to GitHub Container Registry + uses: docker/login-action@v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + - name: Build and push frontend image + uses: docker/build-push-action@v5 + with: + context: ./frontend + file: ./frontend/Dockerfile.prod + push: true + tags: ghcr.io/${{ github.repository }}/frontend:${{ steps.branch.outputs.TAG }} + cache-from: type=gha + cache-to: type=gha,mode=max + + - name: Build and push backend image + uses: docker/build-push-action@v5 + with: + context: ./backend + file: ./backend/Dockerfile.prod + push: true + tags: ghcr.io/${{ github.repository }}/backend:${{ steps.branch.outputs.TAG }} + cache-from: type=gha + cache-to: type=gha,mode=max + + - name: Summary + run: | + echo "### 🐳 Docker images pushed" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "| Service | Image |" >> "$GITHUB_STEP_SUMMARY" + echo "|---------|-------|" >> "$GITHUB_STEP_SUMMARY" + echo "| Frontend | \`ghcr.io/${{ github.repository }}/frontend:${{ steps.branch.outputs.TAG }}\` |" >> "$GITHUB_STEP_SUMMARY" + echo "| Backend | \`ghcr.io/${{ github.repository }}/backend:${{ steps.branch.outputs.TAG }}\` |" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "Pull with:" >> "$GITHUB_STEP_SUMMARY" + echo "\`\`\`bash" >> "$GITHUB_STEP_SUMMARY" + echo "docker pull ghcr.io/${{ github.repository }}/frontend:${{ steps.branch.outputs.TAG }}" >> "$GITHUB_STEP_SUMMARY" + echo "docker pull ghcr.io/${{ github.repository }}/backend:${{ steps.branch.outputs.TAG }}" >> "$GITHUB_STEP_SUMMARY" + echo "\`\`\`" >> "$GITHUB_STEP_SUMMARY" From 7fe2b481efcfb67118cbf86ba77a35b0f1e67831 Mon Sep 17 00:00:00 2001 From: Andy Date: Thu, 26 Mar 2026 20:37:52 +0100 Subject: [PATCH 3/4] feat(push): add detailed notification messages and auto-subscribe with opt-out Replace generic 'List updated' notifications with action-specific messages (added, checked, unchecked, edited, deleted, cleared). Auto-subscribe users on list open with per-list opt-out stored in localStorage. --- .../com/shoppinglist/routes/ListRoutes.kt | 9 +- .../services/PushNotificationService.kt | 28 ++-- frontend/src/components/NotificationBell.tsx | 17 ++- .../__tests__/NotificationBell.test.tsx | 130 ++++++++---------- .../src/context/PushNotificationContext.tsx | 37 ++++- 5 files changed, 137 insertions(+), 84 deletions(-) diff --git a/backend/src/main/kotlin/com/shoppinglist/routes/ListRoutes.kt b/backend/src/main/kotlin/com/shoppinglist/routes/ListRoutes.kt index 86b1bc0..bd956e7 100644 --- a/backend/src/main/kotlin/com/shoppinglist/routes/ListRoutes.kt +++ b/backend/src/main/kotlin/com/shoppinglist/routes/ListRoutes.kt @@ -274,8 +274,12 @@ fun Route.listRoutes(pushNotificationService: PushNotificationService? = null) { } val deviceId = call.request.queryParameters["deviceId"] + val pushChangeType = when { + request.completed != null -> if (updatedItem.completed) "ITEM_CHECKED" else "ITEM_UNCHECKED" + else -> "ITEM_UPDATED" + } call.application.launch(Dispatchers.Default) { - pushNotificationService?.notifyListChange(listId, deviceId, "ITEM_UPDATED", updatedItem.text) + pushNotificationService?.notifyListChange(listId, deviceId, pushChangeType, updatedItem.text) } call.respond(HttpStatusCode.OK, updatedItem) @@ -330,6 +334,7 @@ fun Route.listRoutes(pushNotificationService: PushNotificationService? = null) { return@delete } + val itemToDelete = itemRepository.getItemById(itemId) val deleted = itemRepository.deleteItem(itemId) if (!deleted) { call.respond( @@ -353,7 +358,7 @@ fun Route.listRoutes(pushNotificationService: PushNotificationService? = null) { val deviceId = call.request.queryParameters["deviceId"] call.application.launch(Dispatchers.Default) { - pushNotificationService?.notifyListChange(listId, deviceId, "ITEM_DELETED", null) + pushNotificationService?.notifyListChange(listId, deviceId, "ITEM_DELETED", itemToDelete?.text) } call.respond(HttpStatusCode.NoContent) diff --git a/backend/src/main/kotlin/com/shoppinglist/services/PushNotificationService.kt b/backend/src/main/kotlin/com/shoppinglist/services/PushNotificationService.kt index 0b8acf0..2a71546 100644 --- a/backend/src/main/kotlin/com/shoppinglist/services/PushNotificationService.kt +++ b/backend/src/main/kotlin/com/shoppinglist/services/PushNotificationService.kt @@ -54,16 +54,26 @@ class PushNotificationService( } private fun buildPayload(changeType: String, listId: UUID, itemText: String?): String { - val bodyText = when { - itemText != null -> when (changeType) { - "ITEM_ADDED" -> "$itemText added" - "ITEM_UPDATED" -> "$itemText updated" - "ITEM_DELETED" -> "$itemText removed" - else -> "List updated" - } + val title = when (changeType) { + "ITEM_ADDED" -> "Item added" + "ITEM_CHECKED" -> "Item checked off" + "ITEM_UNCHECKED" -> "Item unchecked" + "ITEM_UPDATED" -> "Item updated" + "ITEM_DELETED" -> "Item removed" + "ITEMS_CLEARED" -> "Completed items cleared" else -> "List updated" - }.take(40) - return """{"type":"$changeType","listId":"$listId","title":"List updated","body":"$bodyText","url":"/list/$listId"}""" + } + val bodyText = when (changeType) { + "ITEM_ADDED" -> itemText?.let { "✚ $it" } ?: "New item added" + "ITEM_CHECKED" -> itemText?.let { "✓ $it" } ?: "An item was checked off" + "ITEM_UNCHECKED" -> itemText?.let { "↩ $it" } ?: "An item was unchecked" + "ITEM_UPDATED" -> itemText?.let { "✎ $it" } ?: "An item was edited" + "ITEM_DELETED" -> itemText?.let { "✕ $it" } ?: "An item was removed" + "ITEMS_CLEARED" -> "All completed items were removed" + else -> "The list was updated" + }.take(60) + val escaped = bodyText.replace("\\", "\\\\").replace("\"", "\\\"") + return """{"type":"$changeType","listId":"$listId","title":"$title","body":"$escaped","url":"/list/$listId"}""" } companion object { diff --git a/frontend/src/components/NotificationBell.tsx b/frontend/src/components/NotificationBell.tsx index 98580e3..c23e51d 100644 --- a/frontend/src/components/NotificationBell.tsx +++ b/frontend/src/components/NotificationBell.tsx @@ -1,3 +1,4 @@ +import { useEffect, useRef } from 'react'; import { useNotifications } from '../context/PushNotificationContext'; import { useI18n } from '../context/I18nContext'; @@ -6,8 +7,15 @@ interface NotificationBellProps { } const NotificationBell = ({ listId }: NotificationBellProps) => { - const { permissionStatus, isSubscribed, isLoading, requestPermission, subscribe, unsubscribe } = useNotifications(); + const { permissionStatus, isSubscribed, isLoading, subscribe, unsubscribe, autoSubscribe, setOptedOut, isOptedOut } = useNotifications(); const { t } = useI18n(); + const autoSubscribeAttempted = useRef(false); + + useEffect(() => { + if (autoSubscribeAttempted.current) return; + autoSubscribeAttempted.current = true; + autoSubscribe(listId); + }, [listId, autoSubscribe]); const handleClick = async () => { if (permissionStatus === 'denied') { @@ -15,10 +23,13 @@ const NotificationBell = ({ listId }: NotificationBellProps) => { } if (isSubscribed) { + setOptedOut(listId, true); await unsubscribe(listId); } else { + setOptedOut(listId, false); if (permissionStatus !== 'granted') { - await requestPermission(); + const result = await Notification.requestPermission(); + if (result !== 'granted') return; } await subscribe(listId); } @@ -82,7 +93,7 @@ const NotificationBell = ({ listId }: NotificationBellProps) => { ? 'opacity-50 cursor-not-allowed bg-gray-200 dark:bg-gray-700 text-gray-500 dark:text-gray-400' : isSubscribed ? 'bg-green-100 dark:bg-green-900/50 text-green-800 dark:text-green-300 border border-green-200 dark:border-green-800 hover:bg-green-200 dark:hover:bg-green-800/50' - : 'bg-blue-600 dark:bg-blue-500 hover:bg-blue-700 dark:hover:bg-blue-600 text-white' + : 'bg-gray-100 dark:bg-gray-700 text-gray-600 dark:text-gray-400 border border-gray-200 dark:border-gray-600 hover:bg-gray-200 dark:hover:bg-gray-600' }`} > {getBellIcon()} diff --git a/frontend/src/components/__tests__/NotificationBell.test.tsx b/frontend/src/components/__tests__/NotificationBell.test.tsx index aec86a6..7d44405 100644 --- a/frontend/src/components/__tests__/NotificationBell.test.tsx +++ b/frontend/src/components/__tests__/NotificationBell.test.tsx @@ -10,23 +10,43 @@ vi.mock('../../context/PushNotificationContext', () => ({ import { useNotifications } from '../../context/PushNotificationContext' const mockUseNotifications = useNotifications as any +// Mock global Notification API (component calls Notification.requestPermission() directly) +const mockGlobalRequestPermission = vi.fn().mockResolvedValue('granted') +Object.defineProperty(window, 'Notification', { + writable: true, + value: { + permission: 'default', + requestPermission: mockGlobalRequestPermission, + }, +}) + describe('NotificationBell', () => { const listId = 'test-list-id' const mockRequestPermission = vi.fn() const mockSubscribe = vi.fn() const mockUnsubscribe = vi.fn() + const mockAutoSubscribe = vi.fn() + const mockSetOptedOut = vi.fn() + const mockIsOptedOut = vi.fn().mockReturnValue(false) + + const defaultMock = { + permissionStatus: 'default' as const, + isSubscribed: false, + isLoading: false, + deviceId: 'test-device-id', + requestPermission: mockRequestPermission, + subscribe: mockSubscribe, + unsubscribe: mockUnsubscribe, + autoSubscribe: mockAutoSubscribe, + setOptedOut: mockSetOptedOut, + isOptedOut: mockIsOptedOut, + } beforeEach(() => { vi.clearAllMocks() - mockUseNotifications.mockReturnValue({ - permissionStatus: 'default', - isSubscribed: false, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, - }) + mockIsOptedOut.mockReturnValue(false) + mockGlobalRequestPermission.mockResolvedValue('granted') + mockUseNotifications.mockReturnValue(defaultMock) }) describe('Rendering', () => { @@ -45,13 +65,9 @@ describe('NotificationBell', () => { describe('Visual State: Default (not subscribed, permission default/granted)', () => { it('shows outline bell icon when not subscribed and permission is default', () => { mockUseNotifications.mockReturnValue({ + ...defaultMock, permissionStatus: 'default', isSubscribed: false, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) render() @@ -63,13 +79,9 @@ describe('NotificationBell', () => { it('shows outline bell icon when not subscribed and permission is granted', () => { mockUseNotifications.mockReturnValue({ + ...defaultMock, permissionStatus: 'granted', isSubscribed: false, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) render() @@ -89,13 +101,9 @@ describe('NotificationBell', () => { describe('Visual State: Subscribed', () => { beforeEach(() => { mockUseNotifications.mockReturnValue({ + ...defaultMock, permissionStatus: 'granted', isSubscribed: true, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) }) @@ -125,13 +133,9 @@ describe('NotificationBell', () => { describe('Visual State: Permission Denied', () => { beforeEach(() => { mockUseNotifications.mockReturnValue({ + ...defaultMock, permissionStatus: 'denied', isSubscribed: false, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) }) @@ -160,13 +164,8 @@ describe('NotificationBell', () => { describe('Visual State: Loading', () => { beforeEach(() => { mockUseNotifications.mockReturnValue({ - permissionStatus: 'default', - isSubscribed: false, + ...defaultMock, isLoading: true, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) }) @@ -187,17 +186,12 @@ describe('NotificationBell', () => { describe('Click Behavior: Subscribe Flow', () => { it('calls requestPermission and subscribe when clicking unsubscribed button with default permission', async () => { const user = userEvent.setup() - mockRequestPermission.mockResolvedValue(undefined) mockSubscribe.mockResolvedValue(undefined) mockUseNotifications.mockReturnValue({ + ...defaultMock, permissionStatus: 'default', isSubscribed: false, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) render() @@ -205,7 +199,8 @@ describe('NotificationBell', () => { await user.click(button) - expect(mockRequestPermission).toHaveBeenCalled() + expect(mockSetOptedOut).toHaveBeenCalledWith(listId, false) + expect(mockGlobalRequestPermission).toHaveBeenCalled() expect(mockSubscribe).toHaveBeenCalledWith(listId) }) @@ -214,13 +209,9 @@ describe('NotificationBell', () => { mockSubscribe.mockResolvedValue(undefined) mockUseNotifications.mockReturnValue({ + ...defaultMock, permissionStatus: 'granted', isSubscribed: false, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) render() @@ -228,7 +219,8 @@ describe('NotificationBell', () => { await user.click(button) - expect(mockRequestPermission).not.toHaveBeenCalled() + expect(mockSetOptedOut).toHaveBeenCalledWith(listId, false) + expect(mockGlobalRequestPermission).not.toHaveBeenCalled() expect(mockSubscribe).toHaveBeenCalledWith(listId) }) @@ -236,13 +228,9 @@ describe('NotificationBell', () => { const user = userEvent.setup() mockUseNotifications.mockReturnValue({ + ...defaultMock, permissionStatus: 'denied', isSubscribed: false, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) render() @@ -258,23 +246,21 @@ describe('NotificationBell', () => { mockUnsubscribe.mockResolvedValue(undefined) mockUseNotifications.mockReturnValue({ + ...defaultMock, permissionStatus: 'granted', isSubscribed: true, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) + + mockUnsubscribe.mockResolvedValue(undefined) render() const button = screen.getByTestId('push-subscribe-button') await user.click(button) + expect(mockSetOptedOut).toHaveBeenCalledWith(listId, true) expect(mockUnsubscribe).toHaveBeenCalledWith(listId) expect(mockSubscribe).not.toHaveBeenCalled() - expect(mockRequestPermission).not.toHaveBeenCalled() }) }) @@ -288,13 +274,9 @@ describe('NotificationBell', () => { it('uses i18n for tooltip text "Disable notifications"', () => { mockUseNotifications.mockReturnValue({ + ...defaultMock, permissionStatus: 'granted', isSubscribed: true, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) render() @@ -321,13 +303,9 @@ describe('NotificationBell', () => { const customListId = 'custom-list-id-12345' mockUseNotifications.mockReturnValue({ + ...defaultMock, permissionStatus: 'granted', isSubscribed: true, - isLoading: false, - deviceId: 'test-device-id', - requestPermission: mockRequestPermission, - subscribe: mockSubscribe, - unsubscribe: mockUnsubscribe, }) mockUnsubscribe.mockResolvedValue(undefined) @@ -337,7 +315,21 @@ describe('NotificationBell', () => { await user.click(button) + expect(mockSetOptedOut).toHaveBeenCalledWith(customListId, true) expect(mockUnsubscribe).toHaveBeenCalledWith(customListId) }) }) + + describe('Auto-Subscribe', () => { + it('calls autoSubscribe with listId on mount', () => { + render() + expect(mockAutoSubscribe).toHaveBeenCalledWith(listId) + }) + + it('calls autoSubscribe only once even on re-render', () => { + const { rerender } = render() + rerender() + expect(mockAutoSubscribe).toHaveBeenCalledTimes(1) + }) + }) }) diff --git a/frontend/src/context/PushNotificationContext.tsx b/frontend/src/context/PushNotificationContext.tsx index 608359f..6216434 100644 --- a/frontend/src/context/PushNotificationContext.tsx +++ b/frontend/src/context/PushNotificationContext.tsx @@ -1,4 +1,4 @@ -import React, { createContext, useContext, useState, ReactNode } from 'react'; +import React, { createContext, useContext, useState, useCallback, ReactNode } from 'react'; type PermissionStatus = 'default' | 'granted' | 'denied'; @@ -10,11 +10,15 @@ interface PushNotificationContextType { requestPermission: () => Promise; subscribe: (listId: string) => Promise; unsubscribe: (listId: string) => Promise; + autoSubscribe: (listId: string) => Promise; + setOptedOut: (listId: string, optedOut: boolean) => void; + isOptedOut: (listId: string) => boolean; } const PushNotificationContext = createContext(undefined); const DEVICE_ID_KEY = 'shoppimo_device_id'; +const OPT_OUT_KEY_PREFIX = 'shoppimo_push_optout_'; const getApiUrl = () => { // APP_CONFIG is injected at runtime by nginx/config.js for production deployments @@ -59,6 +63,18 @@ export const PushNotificationProvider: React.FC<{ children: ReactNode }> = ({ ch const [isLoading, setIsLoading] = useState(false); const [deviceId] = useState(() => getOrCreateDeviceId()); + const isOptedOut = useCallback((listId: string): boolean => { + return localStorage.getItem(`${OPT_OUT_KEY_PREFIX}${listId}`) === 'true'; + }, []); + + const setOptedOut = useCallback((listId: string, optedOut: boolean): void => { + if (optedOut) { + localStorage.setItem(`${OPT_OUT_KEY_PREFIX}${listId}`, 'true'); + } else { + localStorage.removeItem(`${OPT_OUT_KEY_PREFIX}${listId}`); + } + }, []); + const requestPermission = async (): Promise => { const result = await Notification.requestPermission(); setPermissionStatus(result as PermissionStatus); @@ -123,6 +139,22 @@ export const PushNotificationProvider: React.FC<{ children: ReactNode }> = ({ ch } }; + const autoSubscribe = async (listId: string): Promise => { + if (isSubscribed || isLoading || isOptedOut(listId) || permissionStatus === 'denied') { + return; + } + + if (permissionStatus === 'default') { + const result = await Notification.requestPermission(); + setPermissionStatus(result as PermissionStatus); + if (result !== 'granted') { + return; + } + } + + await subscribe(listId); + }; + const unsubscribe = async (listId: string): Promise => { setIsLoading(true); try { @@ -169,6 +201,9 @@ export const PushNotificationProvider: React.FC<{ children: ReactNode }> = ({ ch requestPermission, subscribe, unsubscribe, + autoSubscribe, + setOptedOut, + isOptedOut, }; return ( From 877eaab198283935a1532c5c6101ae2d09ab7b8d Mon Sep 17 00:00:00 2001 From: Andy Date: Thu, 26 Mar 2026 20:44:23 +0100 Subject: [PATCH 4/4] fix(push): remove unused isOptedOut destructure from NotificationBell --- frontend/src/components/NotificationBell.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/components/NotificationBell.tsx b/frontend/src/components/NotificationBell.tsx index c23e51d..89d00f5 100644 --- a/frontend/src/components/NotificationBell.tsx +++ b/frontend/src/components/NotificationBell.tsx @@ -7,7 +7,7 @@ interface NotificationBellProps { } const NotificationBell = ({ listId }: NotificationBellProps) => { - const { permissionStatus, isSubscribed, isLoading, subscribe, unsubscribe, autoSubscribe, setOptedOut, isOptedOut } = useNotifications(); + const { permissionStatus, isSubscribed, isLoading, subscribe, unsubscribe, autoSubscribe, setOptedOut } = useNotifications(); const { t } = useI18n(); const autoSubscribeAttempted = useRef(false);