fix: remove notification from state on open (#789)

* fix: remove notification from state on open

* tests: add test for `removeNotificationFromState`

Author: adufr

src/components/NotificationRow.test.tsx

@@ -31,7 +31,7 @@ describe('components/Notification.js', () => {
   });
 
   it('should open a notification in the browser', () => {
-    const markNotification = jest.fn();
+    const removeNotificationFromState = jest.fn();
 
     const props = {
       notification: mockedSingleNotification,
@@ -42,7 +42,7 @@ describe('components/Notification.js', () => {
       <AppContext.Provider
         value={{
           settings: { ...mockSettings, markAsDoneOnOpen: false },
-          markNotification,
+          removeNotificationFromState,
           accounts: mockAccounts,
         }}
       >
@@ -52,6 +52,7 @@ describe('components/Notification.js', () => {
 
     fireEvent.click(getByRole('main'));
     expect(shell.openExternal).toHaveBeenCalledTimes(1);
+    expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
   });
 
   it('should open a notification in browser & mark it as done', () => {

src/components/NotificationRow.tsx

@@ -23,6 +23,7 @@ export const NotificationRow: React.FC<IProps> = ({
   const {
     settings,
     accounts,
+    removeNotificationFromState,
     markNotification,
     markNotificationDone,
     unsubscribeNotification,
@@ -33,6 +34,9 @@ export const NotificationRow: React.FC<IProps> = ({
 
     if (settings.markAsDoneOnOpen) {
       markNotificationDone(notification.id, hostname);
+    } else {
+      // no need to mark as read, github does it by default when opening it
+      removeNotificationFromState(notification.id, hostname);
     }
   }, [settings]);
 

src/context/App.tsx

@@ -51,6 +51,7 @@ interface AppContextState {
   notifications: AccountNotifications[];
   isFetching: boolean;
   requestFailed: boolean;
+  removeNotificationFromState: (id: string, hostname: string) => void;
   fetchNotifications: () => Promise<void>;
   markNotification: (id: string, hostname: string) => Promise<void>;
   markNotificationDone: (id: string, hostname: string) => Promise<void>;
@@ -71,6 +72,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
     notifications,
     requestFailed,
     isFetching,
+    removeNotificationFromState,
     markNotification,
     markNotificationDone,
     unsubscribeNotification,
@@ -210,6 +212,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
         notifications,
         isFetching,
         requestFailed,
+        removeNotificationFromState,
         fetchNotifications: fetchNotificationsWithAccounts,
         markNotification: markNotificationWithAccounts,
         markNotificationDone: markNotificationDoneWithAccounts,

src/hooks/useNotifications.test.ts

@@ -269,6 +269,42 @@ describe('hooks/useNotifications.ts', () => {
     });
   });
 
+  describe('removeNotificationFromState', () => {
+    it('should remove a notification from state', async () => {
+      const notifications = [
+        { id: 1, title: 'This is a notification.' },
+        { id: 2, title: 'This is another one.' },
+      ];
+
+      nock('https://api.github.com')
+        .get('/notifications?participating=false')
+        .reply(200, notifications);
+
+      nock('https://github.gitify.io/api/v3')
+        .get('/notifications?participating=false')
+        .reply(200, notifications);
+
+      const { result } = renderHook(() => useNotifications(false));
+
+      act(() => {
+        result.current.fetchNotifications(mockAccounts, mockSettings);
+      });
+
+      await waitFor(() => {
+        expect(result.current.isFetching).toBe(false);
+      });
+
+      act(() => {
+        result.current.removeNotificationFromState(
+          result.current.notifications[0].notifications[0].id,
+          result.current.notifications[0].hostname,
+        );
+      });
+
+      expect(result.current.notifications[0].notifications.length).toBe(1);
+    });
+  });
+
   describe('markNotification', () => {
     const id = 'notification-123';
 

src/hooks/useNotifications.ts

@@ -19,6 +19,7 @@ import { removeNotifications } from '../utils/remove-notifications';
 
 interface NotificationsState {
   notifications: AccountNotifications[];
+  removeNotificationFromState: (id: string, hostname: string) => void;
   fetchNotifications: (
     accounts: AuthState,
     settings: SettingsState,
@@ -312,11 +313,26 @@ export const useNotifications = (colors: boolean): NotificationsState => {
     [notifications],
   );
 
+  const removeNotificationFromState = useCallback(
+    (id, hostname) => {
+      const updatedNotifications = removeNotification(
+        id,
+        notifications,
+        hostname,
+      );
+
+      setNotifications(updatedNotifications);
+      setTrayIconColor(updatedNotifications);
+    },
+    [notifications],
+  );
+
   return {
     isFetching,
     requestFailed,
     notifications,
 
+    removeNotificationFromState,
     fetchNotifications,
     markNotification,
     markNotificationDone,