refactor(notifications): read alert settings with useEffectEvent

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.

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,