diff --git a/src/renderer/hooks/notifications/useNotificationEffects.ts b/src/renderer/hooks/notifications/useNotificationEffects.ts index 898fd506d..8544f2917 100644 --- a/src/renderer/hooks/notifications/useNotificationEffects.ts +++ b/src/renderer/hooks/notifications/useNotificationEffects.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef } from 'react'; +import { useCallback, useEffect, useEffectEvent, useRef } from 'react'; import { focusManager, type QueryKey, useQueryClient } from '@tanstack/react-query'; @@ -75,6 +75,30 @@ export const useNotificationEffects = ({ const showSystemNotifications = useSettingsStore((s) => s.showNotifications); const notificationVolume = useSettingsStore((s) => s.notificationVolume); + // Read the alert settings at alert time so changing them does not re-run the + // notification diff and does not capture stale values. + const announceNewNotifications = useEffectEvent((notificationsToRaise: GitifyNotification[]) => { + if (playSoundNewNotifications) { + void raiseSoundNotification(notificationVolume); + } + + if (showSystemNotifications) { + void Promise.resolve(raiseNativeNotification(notificationsToRaise)) + .then((published) => { + for (const payload of published ?? []) { + publishedIdsRef.current.add(payload.id); + } + }) + .catch((error) => { + rendererLogError( + 'useNotificationEffects', + 'Failed to publish native notifications', + toError(error), + ); + }); + } + }); + useEffect(() => { actionsRef.current = actions; }, [actions]); @@ -213,40 +237,12 @@ export const useNotificationEffects = ({ ) as GitifyNotification[]; if (filteredDiffNotifications.length > 0) { - if (playSoundNewNotifications) { - void raiseSoundNotification(notificationVolume); - } - - if (showSystemNotifications) { - void Promise.resolve(raiseNativeNotification(filteredDiffNotifications)) - .then((published) => { - for (const payload of published ?? []) { - publishedIdsRef.current.add(payload.id); - } - }) - .catch((error) => { - rendererLogError( - 'useNotificationEffects', - 'Failed to publish native notifications', - toError(error), - ); - }); - } + announceNewNotifications(filteredDiffNotifications); } } previousNotificationsRef.current = unfilteredNotifications; - }, [ - enabled, - notifications, - isLoading, - isError, - playSoundNewNotifications, - showSystemNotifications, - notificationVolume, - queryClient, - notificationsQueryKey, - ]); + }, [enabled, notifications, isLoading, isError, queryClient, notificationsQueryKey]); // Remove published notifications when they become read, leave the cache, // or when no unread notifications remain for the summary. diff --git a/src/renderer/hooks/useNotifications.test.tsx b/src/renderer/hooks/useNotifications.test.tsx index bd15e5f9a..7b1eaec1f 100644 --- a/src/renderer/hooks/useNotifications.test.tsx +++ b/src/renderer/hooks/useNotifications.test.tsx @@ -658,6 +658,48 @@ describe('renderer/hooks/useNotifications.ts', () => { expect(raiseNativeNotificationSpy).toHaveBeenCalledTimes(1); }); + it('uses the latest alert settings for the next alert without re-registering the effect', async () => { + useSettingsStore.setState({ + notificationVolume: 20 as Percentage, + playSound: true, + showNotifications: false, + }); + getAllNotificationsMock.mockResolvedValueOnce(mockSingleAccountNotifications); + + const { result } = renderNotificationsHook(); + await waitFor(() => expect(result.current.hasNotifications).toBe(true)); + await waitFor(() => expect(raiseSoundNotificationSpy).toHaveBeenCalledWith(20)); + expect(raiseNativeNotificationSpy).not.toHaveBeenCalled(); + + act(() => { + useSettingsStore.setState({ + notificationVolume: 60 as Percentage, + showNotifications: true, + }); + }); + + getAllNotificationsMock.mockResolvedValueOnce([ + { + account: mockGitHubCloudAccount, + error: null, + notifications: [ + mockGitifyNotification, + mockGitHubCloudGitifyNotifications[1], + ] as AccountNotifications['notifications'], + }, + ]); + + await act(async () => { + await result.current.refetchNotifications(); + }); + + await waitFor(() => { + expect(raiseSoundNotificationSpy).toHaveBeenCalledTimes(2); + expect(raiseSoundNotificationSpy).toHaveBeenLastCalledWith(60); + expect(raiseNativeNotificationSpy).toHaveBeenCalledTimes(1); + }); + }); + it('does not raise sound or native notifications when disabled', async () => { useSettingsStore.setState({ playSound: false,