From e0eae895016257ef7989819f4a19bf380158646c Mon Sep 17 00:00:00 2001 From: Arthur Dufour Date: Sun, 18 Feb 2024 14:47:28 +0100 Subject: [PATCH 1/2] fix: remove notification from state on open --- src/components/NotificationRow.test.tsx | 5 +++-- src/components/NotificationRow.tsx | 4 ++++ src/context/App.tsx | 3 +++ src/hooks/useNotifications.ts | 16 ++++++++++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx index dcd601b16..064261881 100644 --- a/src/components/NotificationRow.test.tsx +++ b/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', () => { @@ -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', () => { diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 7f1dbc4a5..360df2840 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -23,6 +23,7 @@ export const NotificationRow: React.FC = ({ const { settings, accounts, + removeNotificationFromState, markNotification, markNotificationDone, unsubscribeNotification, @@ -33,6 +34,9 @@ export const NotificationRow: React.FC = ({ 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]); diff --git a/src/context/App.tsx b/src/context/App.tsx index d009a1604..c9d466000 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -51,6 +51,7 @@ interface AppContextState { notifications: AccountNotifications[]; isFetching: boolean; requestFailed: boolean; + removeNotificationFromState: (id: string, hostname: string) => void; fetchNotifications: () => Promise; markNotification: (id: string, hostname: string) => Promise; markNotificationDone: (id: string, hostname: string) => Promise; @@ -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, diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index 41875fb8b..08235acfb 100644 --- a/src/hooks/useNotifications.ts +++ b/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, From edac01787cd4dc55f46e576f4b7ae01cd5305777 Mon Sep 17 00:00:00 2001 From: Arthur Dufour Date: Sun, 18 Feb 2024 15:27:46 +0100 Subject: [PATCH 2/2] tests: add test for `removeNotificationFromState` --- src/hooks/useNotifications.test.ts | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts index c6f2b753d..ca789184e 100644 --- a/src/hooks/useNotifications.test.ts +++ b/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';