From 4f507d4b764de2b53eab5a291af4fd0985501e2d Mon Sep 17 00:00:00 2001 From: Arthur Dufour Date: Thu, 18 Jan 2024 08:59:16 +0100 Subject: [PATCH 1/5] feat: add button to mark a notification as done --- src/components/NotificationRow.tsx | 23 +++++++++++++++--- src/context/App.tsx | 9 +++++++ src/hooks/useNotifications.ts | 39 ++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 07f533f81..acbf4266a 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -20,8 +20,13 @@ export const NotificationRow: React.FC = ({ notification, hostname, }) => { - const { settings, accounts, markNotification, unsubscribeNotification } = - useContext(AppContext); + const { + settings, + accounts, + markNotification, + markNotificationDone, + unsubscribeNotification, + } = useContext(AppContext); const pressTitle = useCallback(() => { openBrowser(); @@ -85,7 +90,7 @@ export const NotificationRow: React.FC = ({ -
+
+ +
); diff --git a/src/context/App.tsx b/src/context/App.tsx index 34543cf38..bf0799e1f 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -53,6 +53,7 @@ interface AppContextState { requestFailed: boolean; fetchNotifications: () => Promise; markNotification: (id: string, hostname: string) => Promise; + markNotificationDone: (id: string, hostname: string) => Promise; unsubscribeNotification: (id: string, hostname: string) => Promise; markRepoNotifications: (id: string, hostname: string) => Promise; @@ -71,6 +72,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => { requestFailed, isFetching, markNotification, + markNotificationDone, unsubscribeNotification, markRepoNotifications, } = useNotifications(settings.colors); @@ -177,6 +179,12 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => { [accounts, notifications], ); + const markNotificationDoneWithAccounts = useCallback( + async (id: string, hostname: string) => + await markNotificationDone(accounts, id, hostname), + [accounts, notifications], + ); + const unsubscribeNotificationWithAccounts = useCallback( async (id: string, hostname: string) => await unsubscribeNotification(accounts, id, hostname), @@ -204,6 +212,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => { requestFailed, fetchNotifications: fetchNotificationsWithAccounts, markNotification: markNotificationWithAccounts, + markNotificationDone: markNotificationDoneWithAccounts, unsubscribeNotification: unsubscribeNotificationWithAccounts, markRepoNotifications: markRepoNotificationsWithAccounts, diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index 50cf161b1..41875fb8b 100644 --- a/src/hooks/useNotifications.ts +++ b/src/hooks/useNotifications.ts @@ -28,6 +28,11 @@ interface NotificationsState { id: string, hostname: string, ) => Promise; + markNotificationDone: ( + accounts: AuthState, + id: string, + hostname: string, + ) => Promise; unsubscribeNotification: ( accounts: AuthState, id: string, @@ -215,6 +220,39 @@ export const useNotifications = (colors: boolean): NotificationsState => { [notifications], ); + const markNotificationDone = useCallback( + async (accounts, id, hostname) => { + setIsFetching(true); + + const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname; + const token = isEnterprise + ? getEnterpriseAccountToken(hostname, accounts.enterpriseAccounts) + : accounts.token; + + try { + await apiRequestAuth( + `${generateGitHubAPIUrl(hostname)}notifications/threads/${id}`, + 'DELETE', + token, + {}, + ); + + const updatedNotifications = removeNotification( + id, + notifications, + hostname, + ); + + setNotifications(updatedNotifications); + setTrayIconColor(updatedNotifications); + setIsFetching(false); + } catch (err) { + setIsFetching(false); + } + }, + [notifications], + ); + const unsubscribeNotification = useCallback( async (accounts, id, hostname) => { setIsFetching(true); @@ -281,6 +319,7 @@ export const useNotifications = (colors: boolean): NotificationsState => { fetchNotifications, markNotification, + markNotificationDone, unsubscribeNotification, markRepoNotifications, }; From 43e57e1d4fb5a7687568e197838a3bf4bd8b9632 Mon Sep 17 00:00:00 2001 From: Arthur Dufour Date: Thu, 18 Jan 2024 09:00:48 +0100 Subject: [PATCH 2/5] tests: add tests for `markNotificationDone()` --- src/components/NotificationRow.test.tsx | 25 +++++++ src/context/App.test.tsx | 27 ++++++++ src/hooks/useNotifications.test.ts | 86 +++++++++++++++++++++++++ 3 files changed, 138 insertions(+) diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx index e9bfe16a6..e0b907ef9 100644 --- a/src/components/NotificationRow.test.tsx +++ b/src/components/NotificationRow.test.tsx @@ -104,6 +104,31 @@ describe('components/Notification.js', () => { expect(markNotification).toHaveBeenCalledTimes(1); }); + it('should mark a notification as done', () => { + const markNotificationDone = jest.fn(); + + const props = { + notification: mockedSingleNotification, + hostname: 'github.com', + }; + + const { getByTitle } = render( + + + + + , + ); + + fireEvent.click(getByTitle('Mark as Done')); + expect(markNotificationDone).toHaveBeenCalledTimes(1); + }); + it('should unsubscribe from a notification thread', () => { const unsubscribeNotification = jest.fn(); diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx index 05e00520c..a943b5ea4 100644 --- a/src/context/App.test.tsx +++ b/src/context/App.test.tsx @@ -38,6 +38,7 @@ describe('context/App.tsx', () => { const fetchNotificationsMock = jest.fn(); const markNotificationMock = jest.fn(); + const markNotificationDoneMock = jest.fn(); const unsubscribeNotificationMock = jest.fn(); const markRepoNotificationsMock = jest.fn(); @@ -45,6 +46,7 @@ describe('context/App.tsx', () => { (useNotifications as jest.Mock).mockReturnValue({ fetchNotifications: fetchNotificationsMock, markNotification: markNotificationMock, + markNotificationDone: markNotificationDoneMock, unsubscribeNotification: unsubscribeNotificationMock, markRepoNotifications: markRepoNotificationsMock, }); @@ -121,6 +123,31 @@ describe('context/App.tsx', () => { ); }); + it('should call markNotificationDone', async () => { + const TestComponent = () => { + const { markNotificationDone } = useContext(AppContext); + + return ( + + ); + }; + + const { getByText } = customRender(); + + markNotificationDoneMock.mockReset(); + + fireEvent.click(getByText('Test Case')); + + expect(markNotificationDoneMock).toHaveBeenCalledTimes(1); + expect(markNotificationDoneMock).toHaveBeenCalledWith( + { enterpriseAccounts: [], token: null, user: null }, + '123-456', + 'github.com', + ); + }); + it('should call unsubscribeNotification', async () => { const TestComponent = () => { const { unsubscribeNotification } = useContext(AppContext); diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts index 66e40f26a..c6f2b753d 100644 --- a/src/hooks/useNotifications.test.ts +++ b/src/hooks/useNotifications.test.ts @@ -355,6 +355,92 @@ describe('hooks/useNotifications.ts', () => { }); }); + describe('markNotificationDone', () => { + const id = 'notification-123'; + + describe('github.com', () => { + const accounts = { ...mockAccounts, enterpriseAccounts: [] }; + const hostname = 'github.com'; + + it('should mark a notification as done with success - github.com', async () => { + nock('https://api.github.com/') + .delete(`/notifications/threads/${id}`) + .reply(200); + + const { result } = renderHook(() => useNotifications(false)); + + act(() => { + result.current.markNotificationDone(accounts, id, hostname); + }); + + await waitFor(() => { + expect(result.current.isFetching).toBe(false); + }); + + expect(result.current.notifications.length).toBe(0); + }); + + it('should mark a notification as done with failure - github.com', async () => { + nock('https://api.github.com/') + .delete(`/notifications/threads/${id}`) + .reply(400); + + const { result } = renderHook(() => useNotifications(false)); + + act(() => { + result.current.markNotificationDone(accounts, id, hostname); + }); + + await waitFor(() => { + expect(result.current.isFetching).toBe(false); + }); + + expect(result.current.notifications.length).toBe(0); + }); + }); + + describe('enterprise', () => { + const accounts = { ...mockAccounts, token: null }; + const hostname = 'github.gitify.io'; + + it('should mark a notification as done with success - enterprise', async () => { + nock('https://github.gitify.io/') + .delete(`/notifications/threads/${id}`) + .reply(200); + + const { result } = renderHook(() => useNotifications(false)); + + act(() => { + result.current.markNotificationDone(accounts, id, hostname); + }); + + await waitFor(() => { + expect(result.current.isFetching).toBe(false); + }); + + expect(result.current.notifications.length).toBe(0); + }); + + it('should mark a notification as done with failure - enterprise', async () => { + nock('https://github.gitify.io/') + .delete(`/notifications/threads/${id}`) + .reply(400); + + const { result } = renderHook(() => useNotifications(false)); + + act(() => { + result.current.markNotificationDone(accounts, id, hostname); + }); + + await waitFor(() => { + expect(result.current.isFetching).toBe(false); + }); + + expect(result.current.notifications.length).toBe(0); + }); + }); + }); + describe('unsubscribeNotification', () => { const id = 'notification-123'; From a0299465907c1e1a43458eb6e931bd18c39a9b38 Mon Sep 17 00:00:00 2001 From: Arthur Dufour Date: Thu, 18 Jan 2024 09:01:30 +0100 Subject: [PATCH 3/5] feat: update "mark notification as read" icon --- src/components/NotificationRow.tsx | 6 +++--- src/components/Repository.tsx | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index acbf4266a..6a28bb369 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -1,6 +1,6 @@ import React, { useCallback, useContext } from 'react'; import { formatDistanceToNow, parseISO } from 'date-fns'; -import { CheckIcon, MuteIcon } from '@primer/octicons-react'; +import { CheckIcon, MuteIcon, ReadIcon } from '@primer/octicons-react'; import { formatReason, @@ -96,9 +96,9 @@ export const NotificationRow: React.FC = ({ title="Mark as Read" onClick={() => markNotification(notification.id, hostname)} > - diff --git a/src/components/Repository.tsx b/src/components/Repository.tsx index 14ee53c2b..ddf1ef4a1 100644 --- a/src/components/Repository.tsx +++ b/src/components/Repository.tsx @@ -1,7 +1,7 @@ const { shell } = require('electron'); import React, { useCallback, useContext } from 'react'; -import { CheckIcon } from '@primer/octicons-react'; +import { ReadIcon } from '@primer/octicons-react'; import { CSSTransition, TransitionGroup } from 'react-transition-group'; import { AppContext } from '../context/App'; @@ -43,9 +43,9 @@ export const RepositoryNotifications: React.FC = ({
From 8ad7e1e3f21116950f292330f26272743edd079e Mon Sep 17 00:00:00 2001 From: Arthur Dufour Date: Thu, 18 Jan 2024 09:01:47 +0100 Subject: [PATCH 4/5] tests: update snaps --- .../NotificationRow.test.tsx.snap | 35 +++++++++++++++++-- .../__snapshots__/Repository.test.tsx.snap | 6 ++-- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/components/__snapshots__/NotificationRow.test.tsx.snap b/src/components/__snapshots__/NotificationRow.test.tsx.snap index 8a922c507..9d9ed5042 100644 --- a/src/components/__snapshots__/NotificationRow.test.tsx.snap +++ b/src/components/__snapshots__/NotificationRow.test.tsx.snap @@ -87,7 +87,7 @@ exports[`components/Notification.js should render itself & its children 1`] = `
+ From b4d9561c573c142cbc2a5984eec9660467f453a6 Mon Sep 17 00:00:00 2001 From: Arthur Dufour Date: Wed, 31 Jan 2024 09:32:11 +0100 Subject: [PATCH 5/5] fix: remove now obsolete `markOnClick` --- src/components/NotificationRow.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx index 65e59221b..0359f0eef 100644 --- a/src/components/NotificationRow.test.tsx +++ b/src/components/NotificationRow.test.tsx @@ -90,7 +90,7 @@ describe('components/Notification.js', () => { const { getByTitle } = render(