fix: correctly set count when using delayNotificationState

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

Author: setchy

src/renderer/components/Sidebar.tsx

@@ -1,4 +1,4 @@
-import { type FC, useContext, useMemo } from 'react';
+import { type FC, useContext } from 'react';
 import { useLocation, useNavigate } from 'react-router-dom';
 
 import {
@@ -23,7 +23,7 @@ import {
   openGitHubPulls,
 } from '../utils/links';
 import { hasActiveFilters } from '../utils/notifications/filters/filter';
-import { getNotificationCount } from '../utils/notifications/notifications';
+import { getUnreadNotificationCount } from '../utils/notifications/notifications';
 import { LogoIcon } from './icons/LogoIcon';
 
 export const Sidebar: FC = () => {
@@ -65,9 +65,7 @@ export const Sidebar: FC = () => {
     fetchNotifications();
   };
 
-  const notificationsCount = useMemo(() => {
-    return getNotificationCount(notifications);
-  }, [notifications]);
+  const notificationsCount = getUnreadNotificationCount(notifications);
 
   const sidebarButtonStyle = { color: 'white' };
 

src/renderer/context/App.test.tsx

@@ -38,16 +38,15 @@ describe('renderer/context/App.tsx', () => {
   });
 
   describe('notification methods', () => {
-    const getNotificationCountMock = jest.spyOn(
-      notifications,
-      'getNotificationCount',
-    );
-    getNotificationCountMock.mockReturnValue(1);
+    const setTrayIconColorAndTitleMock = jest
+      .spyOn(notifications, 'setTrayIconColorAndTitle')
+      .mockImplementation(jest.fn());
 
     const fetchNotificationsMock = jest.fn();
     const markNotificationsAsReadMock = jest.fn();
     const markNotificationsAsDoneMock = jest.fn();
     const unsubscribeNotificationMock = jest.fn();
+    const setTrayIconColorAndTitle = jest.fn();
 
     const mockDefaultState = {
       auth: { accounts: [] },
@@ -60,6 +59,7 @@ describe('renderer/context/App.tsx', () => {
         markNotificationsAsRead: markNotificationsAsReadMock,
         markNotificationsAsDone: markNotificationsAsDoneMock,
         unsubscribeNotification: unsubscribeNotificationMock,
+        setTrayIconColorAndTitle,
       });
     });
 
@@ -144,6 +144,7 @@ describe('renderer/context/App.tsx', () => {
         mockDefaultState,
         [mockSingleNotification],
       );
+      expect(setTrayIconColorAndTitleMock).toHaveBeenCalledTimes(1);
     });
 
     it('should call markNotificationsAsDone', async () => {
@@ -169,6 +170,7 @@ describe('renderer/context/App.tsx', () => {
         mockDefaultState,
         [mockSingleNotification],
       );
+      expect(setTrayIconColorAndTitleMock).toHaveBeenCalledTimes(1);
     });
 
     it('should call unsubscribeNotification', async () => {
@@ -194,6 +196,7 @@ describe('renderer/context/App.tsx', () => {
         mockDefaultState,
         mockSingleNotification,
       );
+      expect(setTrayIconColorAndTitleMock).toHaveBeenCalledTimes(1);
     });
   });
 

src/renderer/context/App.tsx

@@ -47,10 +47,8 @@ import {
   setKeyboardShortcut,
   setUseAlternateIdleIcon,
   setUseUnreadActiveIcon,
-  updateTrayColor,
-  updateTrayTitle,
 } from '../utils/comms';
-import { getNotificationCount } from '../utils/notifications/notifications';
+import { setTrayIconColorAndTitle } from '../utils/notifications/notifications';
 import { clearState, loadState, saveState } from '../utils/storage';
 import {
   DEFAULT_DAY_COLOR_SCHEME,
@@ -164,19 +162,11 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
     }
   }, Constants.REFRESH_ACCOUNTS_INTERVAL_MS);
 
+  // biome-ignore lint/correctness/useExhaustiveDependencies: We also want to update the tray on setting changes
   useEffect(() => {
-    const count = getNotificationCount(notifications);
-
-    let title = '';
-    if (settings.showNotificationsCountInTray && count > 0) {
-      title = count.toString();
-    }
-
     setUseUnreadActiveIcon(settings.useUnreadActiveIcon);
     setUseAlternateIdleIcon(settings.useAlternateIdleIcon);
-
-    updateTrayColor(count);
-    updateTrayTitle(title);
+    setTrayIconColorAndTitle(notifications, settings);
   }, [
     settings.showNotificationsCountInTray,
     settings.useUnreadActiveIcon,

src/renderer/hooks/useNotifications.ts

@@ -19,7 +19,7 @@ import { rendererLogError } from '../utils/logger';
 import { triggerNativeNotifications } from '../utils/notifications/native';
 import {
   getAllNotifications,
-  setTrayIconColor,
+  setTrayIconColorAndTitle,
 } from '../utils/notifications/notifications';
 import { removeNotifications } from '../utils/notifications/remove';
 
@@ -34,9 +34,13 @@ function markNotificationsAsReadLocally(targetNotifications: Notification[]) {
 }
 
 interface NotificationsState {
+  status: Status;
+  globalError: GitifyError;
+
   notifications: AccountNotifications[];
-  removeAccountNotifications: (account: Account) => Promise<void>;
   fetchNotifications: (state: GitifyState) => Promise<void>;
+  removeAccountNotifications: (account: Account) => Promise<void>;
+
   markNotificationsAsRead: (
     state: GitifyState,
     notifications: Notification[],
@@ -49,8 +53,6 @@ interface NotificationsState {
     state: GitifyState,
     notification: Notification,
   ) => Promise<void>;
-  status: Status;
-  globalError: GitifyError;
 }
 
 export const useNotifications = (): NotificationsState => {
@@ -70,7 +72,7 @@ export const useNotifications = (): NotificationsState => {
       );
 
       setNotifications(updatedNotifications);
-      setTrayIconColor(updatedNotifications);
+
       setStatus('success');
     },
     [notifications],
@@ -109,6 +111,7 @@ export const useNotifications = (): NotificationsState => {
 
       setNotifications(fetchedNotifications);
       triggerNativeNotifications(notifications, fetchedNotifications, state);
+
       setStatus('success');
     },
     [notifications],
@@ -138,7 +141,7 @@ export const useNotifications = (): NotificationsState => {
         markNotificationsAsReadLocally(readNotifications);
 
         setNotifications(updatedNotifications);
-        setTrayIconColor(updatedNotifications);
+        setTrayIconColorAndTitle(updatedNotifications, state.settings);
       } catch (err) {
         rendererLogError(
           'markNotificationsAsRead',
@@ -180,7 +183,7 @@ export const useNotifications = (): NotificationsState => {
         markNotificationsAsReadLocally(doneNotifications);
 
         setNotifications(updatedNotifications);
-        setTrayIconColor(updatedNotifications);
+        setTrayIconColorAndTitle(updatedNotifications, state.settings);
       } catch (err) {
         rendererLogError(
           'markNotificationsAsDone',

src/renderer/routes/Notifications.tsx

@@ -7,7 +7,7 @@ import { AccountNotifications } from '../components/notifications/AccountNotific
 import { Oops } from '../components/Oops';
 import { AppContext } from '../context/App';
 import { getAccountUUID } from '../utils/auth/utils';
-import { getNotificationCount } from '../utils/notifications/notifications';
+import { getUnreadNotificationCount } from '../utils/notifications/notifications';
 
 export const NotificationsRoute: FC = () => {
   const { notifications, status, globalError, settings } =
@@ -24,7 +24,7 @@ export const NotificationsRoute: FC = () => {
   );
 
   const hasNotifications = useMemo(
-    () => getNotificationCount(notifications) > 0,
+    () => getUnreadNotificationCount(notifications) > 0,
     [notifications],
   );
 

src/renderer/utils/notifications/native.ts

@@ -4,7 +4,7 @@ import type { AccountNotifications, GitifyState } from '../../types';
 import type { Notification } from '../../typesGitHub';
 import { getAccountUUID } from '../auth/utils';
 import { generateGitHubWebUrl } from '../helpers';
-import { setTrayIconColor } from './notifications';
+import { setTrayIconColorAndTitle } from './notifications';
 
 export const triggerNativeNotifications = (
   previousNotifications: AccountNotifications[],
@@ -35,7 +35,7 @@ export const triggerNativeNotifications = (
     return accountNewNotifications;
   });
 
-  setTrayIconColor(newNotifications);
+  setTrayIconColorAndTitle(newNotifications, state.settings);
 
   // If there are no new notifications just stop there
   if (!diffNotifications.length) {

src/renderer/utils/notifications/notifications.test.ts

@@ -7,7 +7,10 @@ import { mockSettings } from '../../__mocks__/state-mocks';
 import type { Link } from '../../types';
 import type { Repository } from '../../typesGitHub';
 import * as logger from '../../utils/logger';
-import { enrichNotification, getNotificationCount } from './notifications';
+import {
+  enrichNotification,
+  getUnreadNotificationCount,
+} from './notifications';
 
 describe('renderer/utils/notifications/notifications.ts', () => {
   beforeEach(() => {
@@ -20,8 +23,8 @@ describe('renderer/utils/notifications/notifications.ts', () => {
     jest.clearAllMocks();
   });
 
-  it('getNotificationCount', () => {
-    const result = getNotificationCount(mockSingleAccountNotifications);
+  it('getUnreadNotificationCount', () => {
+    const result = getUnreadNotificationCount(mockSingleAccountNotifications);
 
     expect(result).toBe(1);
   });

src/renderer/utils/notifications/notifications.ts

@@ -6,7 +6,7 @@ import type {
 import type { GitifySubject, Notification } from '../../typesGitHub';
 import { listNotificationsForAuthenticatedUser } from '../api/client';
 import { determineFailureType } from '../api/errors';
-import { updateTrayColor } from '../comms';
+import { updateTrayColor, updateTrayTitle } from '../comms';
 import { rendererLogError, rendererLogWarn } from '../logger';
 import {
   filterBaseNotifications,
@@ -15,15 +15,32 @@ import {
 import { getFlattenedNotificationsByRepo } from './group';
 import { createNotificationHandler } from './handlers';
 
-export function setTrayIconColor(notifications: AccountNotifications[]) {
-  const allNotificationsCount = getNotificationCount(notifications);
+/**
+ * Sets the tray icon color and title based on the number of unread notifications.
+ * @param notifications
+ * @param settings
+ */
+export function setTrayIconColorAndTitle(
+  notifications: AccountNotifications[],
+  settings: SettingsState,
+) {
+  const totalUnreadNotifications = getUnreadNotificationCount(notifications);
+
+  let title = '';
+  if (settings.showNotificationsCountInTray && totalUnreadNotifications > 0) {
+    title = totalUnreadNotifications.toString();
+  }
 
-  updateTrayColor(allNotificationsCount);
+  updateTrayColor(totalUnreadNotifications);
+  updateTrayTitle(title);
 }
 
-export function getNotificationCount(notifications: AccountNotifications[]) {
+export function getUnreadNotificationCount(
+  notifications: AccountNotifications[],
+) {
   return notifications.reduce(
-    (sum, account) => sum + account.notifications.length,
+    (sum, account) =>
+      sum + account.notifications.filter((n) => n.unread === true).length,
     0,
   );
 }