fix: correctly set count when using delayNotificationState

Signed-off-by: Adam Setch <[email protected]>

Author: setchy

src/renderer/components/Sidebar.tsx

@@ -23,20 +23,20 @@ import {
   openGitHubPulls,
 } from '../utils/links';
 import { hasActiveFilters } from '../utils/notifications/filters/filter';
-import { getUnreadNotificationCount } from '../utils/notifications/notifications';
 import { LogoIcon } from './icons/LogoIcon';
 
 export const Sidebar: FC = () => {
   const navigate = useNavigate();
   const location = useLocation();
 
   const {
-    notifications,
     fetchNotifications,
     isLoggedIn,
     status,
     settings,
     auth,
+    unreadCount,
+    hasNotifications,
   } = useContext(AppContext);
 
   // We naively assume that the first account is the primary account for the purposes of our sidebar quick links
@@ -65,8 +65,6 @@ export const Sidebar: FC = () => {
     fetchNotifications();
   };
 
-  const notificationsCount = getUnreadNotificationCount(notifications);
-
   const sidebarButtonStyle = { color: 'white' };
 
   return (
@@ -96,14 +94,14 @@ export const Sidebar: FC = () => {
         <IconButton
           aria-label="Notifications"
           data-testid="sidebar-notifications"
-          description={`${notificationsCount} unread notifications ↗`}
+          description={`${unreadCount} unread notifications ↗`}
           icon={BellIcon}
           onClick={() => openGitHubNotifications(primaryAccountHostname)}
           size="small"
           sx={sidebarButtonStyle}
           tooltipDirection="e"
           unsafeDisableTooltip={false}
-          variant={notificationsCount > 0 ? 'primary' : 'invisible'}
+          variant={hasNotifications ? 'primary' : 'invisible'}
         />
 
         {isLoggedIn && (

src/renderer/components/__snapshots__/Sidebar.test.tsx.snap

@@ -42,6 +42,10 @@ describe('renderer/context/App.tsx', () => {
       .spyOn(notifications, 'setTrayIconColorAndTitle')
       .mockImplementation(jest.fn());
 
+    jest
+      .spyOn(notifications, 'getUnreadNotificationCount')
+      .mockImplementation(jest.fn());
+
     const fetchNotificationsMock = jest.fn();
     const markNotificationsAsReadMock = jest.fn();
     const markNotificationsAsDoneMock = jest.fn();
@@ -70,8 +74,6 @@ describe('renderer/context/App.tsx', () => {
     it('fetch notifications every minute', async () => {
       customRender(null);
 
-      // Wait for the useEffects, for settings.participating and accounts, to run.
-      // Those aren't what we're testing
       await waitFor(() =>
         expect(fetchNotificationsMock).toHaveBeenCalledTimes(1),
       );
@@ -80,23 +82,20 @@ describe('renderer/context/App.tsx', () => {
         jest.advanceTimersByTime(
           Constants.DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS,
         );
-        return;
       });
       expect(fetchNotificationsMock).toHaveBeenCalledTimes(2);
 
       act(() => {
         jest.advanceTimersByTime(
           Constants.DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS,
         );
-        return;
       });
       expect(fetchNotificationsMock).toHaveBeenCalledTimes(3);
 
       act(() => {
         jest.advanceTimersByTime(
           Constants.DEFAULT_FETCH_NOTIFICATIONS_INTERVAL_MS,
         );
-        return;
       });
       expect(fetchNotificationsMock).toHaveBeenCalledTimes(4);
     });

src/renderer/components/layout/__snapshots__/AppLayout.test.tsx.snap

@@ -48,7 +48,10 @@ import {
   setUseAlternateIdleIcon,
   setUseUnreadActiveIcon,
 } from '../utils/comms';
-import { setTrayIconColorAndTitle } from '../utils/notifications/notifications';
+import {
+  getUnreadNotificationCount,
+  setTrayIconColorAndTitle,
+} from '../utils/notifications/notifications';
 import { clearState, loadState, saveState } from '../utils/storage';
 import {
   DEFAULT_DAY_COLOR_SCHEME,
@@ -73,6 +76,8 @@ interface AppContextState {
   globalError: GitifyError;
 
   notifications: AccountNotifications[];
+  unreadCount: number;
+  hasNotifications: boolean;
   fetchNotifications: () => Promise<void>;
   removeAccountNotifications: (account: Account) => Promise<void>;
 
@@ -108,6 +113,10 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
     unsubscribeNotification,
   } = useNotifications();
 
+  const unreadCount = getUnreadNotificationCount(notifications);
+
+  const hasNotifications = useMemo(() => unreadCount > 0, [unreadCount]);
+
   // biome-ignore lint/correctness/useExhaustiveDependencies: restoreSettings is stable and should run only once
   useEffect(() => {
     restoreSettings();
@@ -346,6 +355,8 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
       globalError,
 
       notifications,
+      unreadCount,
+      hasNotifications,
       fetchNotifications: fetchNotificationsWithAccounts,
 
       markNotificationsAsRead: markNotificationsAsReadWithAccounts,
@@ -370,6 +381,8 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
       globalError,
 
       notifications,
+      unreadCount,
+      hasNotifications,
       fetchNotificationsWithAccounts,
       markNotificationsAsReadWithAccounts,
       markNotificationsAsDoneWithAccounts,

src/renderer/context/App.test.tsx

@@ -83,6 +83,7 @@ export const useNotifications = (): NotificationsState => {
       setStatus('loading');
       setGlobalError(null);
 
+      const previousNotifications = notifications;
       const fetchedNotifications = await getAllNotifications(state);
 
       // Set Global Error if all accounts have the same error
@@ -109,8 +110,13 @@ export const useNotifications = (): NotificationsState => {
         return;
       }
 
+      triggerNativeNotifications(
+        previousNotifications,
+        fetchedNotifications,
+        state,
+      );
+
       setNotifications(fetchedNotifications);
-      triggerNativeNotifications(notifications, fetchedNotifications, state);
 
       setStatus('success');
     },

src/renderer/context/App.tsx

@@ -7,10 +7,9 @@ import { AccountNotifications } from '../components/notifications/AccountNotific
 import { Oops } from '../components/Oops';
 import { AppContext } from '../context/App';
 import { getAccountUUID } from '../utils/auth/utils';
-import { getUnreadNotificationCount } from '../utils/notifications/notifications';
 
 export const NotificationsRoute: FC = () => {
-  const { notifications, status, globalError, settings } =
+  const { notifications, status, globalError, settings, hasNotifications } =
     useContext(AppContext);
 
   const hasMultipleAccounts = useMemo(
@@ -23,11 +22,6 @@ export const NotificationsRoute: FC = () => {
     [notifications],
   );
 
-  const hasNotifications = useMemo(
-    () => getUnreadNotificationCount(notifications) > 0,
-    [notifications],
-  );
-
   if (status === 'error') {
     return <Oops error={globalError} />;
   }

src/renderer/hooks/useNotifications.ts

@@ -28,6 +28,7 @@ describe('renderer/utils/notifications/native.ts', () => {
         auth: mockAuth,
         settings,
       });
+
       // wait for async native handling (generateGitHubWebUrl) to complete
       await waitFor(() =>
         expect(window.gitify.raiseNativeNotification).toHaveBeenCalledTimes(1),
@@ -64,6 +65,7 @@ describe('renderer/utils/notifications/native.ts', () => {
         auth: mockAuth,
         settings,
       });
+
       await waitFor(() =>
         expect(window.gitify.raiseNativeNotification).toHaveBeenCalledTimes(1),
       );