diff --git a/src/renderer/__helpers__/test-utils.tsx b/src/renderer/__helpers__/test-utils.tsx index 8e6f874ff..c22cd8699 100644 --- a/src/renderer/__helpers__/test-utils.tsx +++ b/src/renderer/__helpers__/test-utils.tsx @@ -22,19 +22,41 @@ export function AppContextProvider({ children, value = {}, }: AppContextProviderProps) { - const defaultValue: Partial = useMemo(() => { + const defaultValue: AppContextState = useMemo(() => { return { auth: mockAuth, settings: mockSettings, isLoggedIn: true, notifications: [], + notificationCount: 0, + unreadNotificationCount: 0, + hasNotifications: false, + hasUnreadNotifications: false, status: 'success', globalError: null, + // Default mock implementations for all required methods + loginWithGitHubApp: jest.fn(), + loginWithOAuthApp: jest.fn(), + loginWithPersonalAccessToken: jest.fn(), + logoutFromAccount: jest.fn(), + + fetchNotifications: jest.fn(), + removeAccountNotifications: jest.fn(), + + markNotificationsAsRead: jest.fn(), + markNotificationsAsDone: jest.fn(), + unsubscribeNotification: jest.fn(), + + clearFilters: jest.fn(), + resetSettings: jest.fn(), + updateSetting: jest.fn(), + updateFilter: jest.fn(), + ...value, - } as Partial; + } as AppContextState; }, [value]); return ( diff --git a/src/renderer/__mocks__/state-mocks.ts b/src/renderer/__mocks__/state-mocks.ts index 63ad1f1c0..529aa94c5 100644 --- a/src/renderer/__mocks__/state-mocks.ts +++ b/src/renderer/__mocks__/state-mocks.ts @@ -43,6 +43,7 @@ const mockNotificationSettings: NotificationSettingsState = { showPills: true, showNumber: true, participating: false, + fetchReadNotifications: false, markAsDoneOnOpen: false, markAsDoneOnUnsubscribe: false, delayNotificationState: false, diff --git a/src/renderer/components/Sidebar.tsx b/src/renderer/components/Sidebar.tsx index 0be792987..b3d41a2a1 100644 --- a/src/renderer/components/Sidebar.tsx +++ b/src/renderer/components/Sidebar.tsx @@ -35,7 +35,7 @@ export const Sidebar: FC = () => { status, settings, auth, - unreadNotificationCount, + notificationCount, hasUnreadNotifications, } = useAppContext(); @@ -89,7 +89,7 @@ export const Sidebar: FC = () => { openGitHubNotifications(primaryAccountHostname)} size="small" diff --git a/src/renderer/components/__snapshots__/Sidebar.test.tsx.snap b/src/renderer/components/__snapshots__/Sidebar.test.tsx.snap index 58b56b976..11981260c 100644 --- a/src/renderer/components/__snapshots__/Sidebar.test.tsx.snap +++ b/src/renderer/components/__snapshots__/Sidebar.test.tsx.snap @@ -231,7 +231,7 @@ exports[`renderer/components/Sidebar.tsx should render itself & its children (lo popover="auto" role="tooltip" > - undefined unread notifications ↗ + 0 unread notifications ↗ @@ -1878,20 +1878,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende data-wrap="nowrap" > @@ -2468,20 +2468,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende data-wrap="nowrap" > @@ -2758,20 +2758,20 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende data-wrap="nowrap" > diff --git a/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap index 8227203cc..cf0f804e6 100644 --- a/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap @@ -322,20 +322,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -732,20 +732,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -1147,20 +1147,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -1505,20 +1505,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -1920,20 +1920,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -2278,20 +2278,20 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -2721,35 +2721,6 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its /> - - - - @@ -751,20 +693,20 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should re data-wrap="nowrap" > @@ -917,7 +859,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should to data-portal-root="true" >
@@ -1114,7 +1056,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should to data-portal-root="true" >
@@ -1318,7 +1260,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should to data-portal-root="true" >
@@ -1579,7 +1521,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should us data-portal-root="true" >
@@ -1783,7 +1725,7 @@ exports[`renderer/components/notifications/RepositoryNotifications.tsx should us data-portal-root="true" >
diff --git a/src/renderer/components/settings/NotificationSettings.test.tsx b/src/renderer/components/settings/NotificationSettings.test.tsx index 7645681ff..1aa168199 100644 --- a/src/renderer/components/settings/NotificationSettings.test.tsx +++ b/src/renderer/components/settings/NotificationSettings.test.tsx @@ -271,6 +271,24 @@ describe('renderer/components/settings/NotificationSettings.tsx', () => { ); }); + it('should toggle the fetchReadNotifications checkbox', async () => { + await act(async () => { + renderWithAppContext(, { + updateSetting: updateSettingMock, + }); + }); + + await userEvent.click( + screen.getByTestId('checkbox-fetchReadNotifications'), + ); + + expect(updateSettingMock).toHaveBeenCalledTimes(1); + expect(updateSettingMock).toHaveBeenCalledWith( + 'fetchReadNotifications', + true, + ); + }); + it('should toggle the markAsDoneOnOpen checkbox', async () => { await act(async () => { renderWithAppContext(, { diff --git a/src/renderer/components/settings/NotificationSettings.tsx b/src/renderer/components/settings/NotificationSettings.tsx index 66bb02de7..1507ea1cd 100644 --- a/src/renderer/components/settings/NotificationSettings.tsx +++ b/src/renderer/components/settings/NotificationSettings.tsx @@ -328,6 +328,29 @@ export const NotificationSettings: FC = () => { } /> + + updateSetting('fetchReadNotifications', evt.target.checked) + } + tooltip={ + + Fetch all notifications including read and done. + + ⚠️ GitHub's API does not distinguish between read and done + states, so 'Mark as done' actions will be unavailable when this + setting is enabled. + + + ⚠️ Enabling this setting will increase API usage and may cause + rate limiting for users with many notifications. + + + } + /> + { setUseUnreadActiveIcon(settings.useUnreadActiveIcon); setUseAlternateIdleIcon(settings.useAlternateIdleIcon); - const trayCount = status === 'error' ? -1 : unreadNotificationCount; + const trayCount = status === 'error' ? -1 : notificationCount; setTrayIconColorAndTitle(trayCount, settings); }, [ settings.showNotificationsCountInTray, diff --git a/src/renderer/context/defaults.ts b/src/renderer/context/defaults.ts index b37fc260a..e26199f98 100644 --- a/src/renderer/context/defaults.ts +++ b/src/renderer/context/defaults.ts @@ -36,6 +36,7 @@ const defaultNotificationSettings: NotificationSettingsState = { showPills: true, showNumber: true, participating: false, + fetchReadNotifications: false, markAsDoneOnOpen: false, markAsDoneOnUnsubscribe: false, delayNotificationState: false, diff --git a/src/renderer/hooks/useNotifications.test.ts b/src/renderer/hooks/useNotifications.test.ts index 1d7824232..2ce0423a3 100644 --- a/src/renderer/hooks/useNotifications.test.ts +++ b/src/renderer/hooks/useNotifications.test.ts @@ -3,6 +3,7 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import axios, { AxiosError } from 'axios'; import nock from 'nock'; +import { mockGitHubCloudAccount } from '../__mocks__/account-mocks'; import { mockGitifyNotification } from '../__mocks__/notifications-mocks'; import { mockAuth, mockSettings, mockState } from '../__mocks__/state-mocks'; import { Errors } from '../utils/errors'; @@ -84,11 +85,11 @@ describe('renderer/hooks/useNotifications.ts', () => { ]; nock('https://api.github.com') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .reply(200, notifications); nock('https://github.gitify.io/api/v3') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .reply(200, notifications); const { result } = renderHook(() => useNotifications()); @@ -114,13 +115,218 @@ describe('renderer/hooks/useNotifications.ts', () => { expect(result.current.notifications[1].notifications.length).toBe(2); }); + it('should fetch detailed notifications with success', async () => { + const mockNotificationUser = { + login: 'test-user', + html_url: 'https://github.com/test-user', + avatar_url: 'https://avatars.githubusercontent.com/u/1?v=4', + type: 'User', + }; + + const mockRepository = { + name: 'notifications-test', + full_name: 'gitify-app/notifications-test', + html_url: 'https://github.com/gitify-app/notifications-test', + owner: { + login: 'gitify-app', + avatar_url: 'https://avatar.url', + type: 'Organization', + }, + }; + + const mockNotifications = [ + { + id: '1', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is a check suite workflow.', + type: 'CheckSuite', + url: null, + latest_comment_url: null, + }, + repository: mockRepository, + }, + { + id: '2', + unread: true, + updated_at: '2024-02-26T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is a Discussion.', + type: 'Discussion', + url: null, + latest_comment_url: null, + }, + repository: mockRepository, + }, + { + id: '3', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is an Issue.', + type: 'Issue', + url: 'https://api.github.com/repos/gitify-app/notifications-test/issues/3', + latest_comment_url: + 'https://api.github.com/repos/gitify-app/notifications-test/issues/3/comments', + }, + repository: mockRepository, + }, + { + id: '4', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is a Pull Request.', + type: 'PullRequest', + url: 'https://api.github.com/repos/gitify-app/notifications-test/pulls/4', + latest_comment_url: + 'https://api.github.com/repos/gitify-app/notifications-test/issues/4/comments', + }, + repository: mockRepository, + }, + { + id: '5', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is an invitation.', + type: 'RepositoryInvitation', + url: null, + latest_comment_url: null, + }, + repository: mockRepository, + }, + { + id: '6', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'This is a workflow run.', + type: 'WorkflowRun', + url: null, + latest_comment_url: null, + }, + repository: mockRepository, + }, + ]; + + nock('https://api.github.com') + .get('/notifications?participating=false&all=false') + .reply(200, mockNotifications); + + nock('https://api.github.com') + .post('/graphql') + .reply(200, { + data: { + search: { + nodes: [ + { + title: 'This is a Discussion.', + stateReason: null, + isAnswered: true, + url: 'https://github.com/gitify-app/notifications-test/discussions/612', + author: { + login: 'discussion-creator', + url: 'https://github.com/discussion-creator', + avatar_url: + 'https://avatars.githubusercontent.com/u/133795385?s=200&v=4', + type: 'User', + }, + comments: { + nodes: [ + { + databaseId: 2297637, + createdAt: '2022-03-04T20:39:44Z', + author: { + login: 'comment-user', + url: 'https://github.com/comment-user', + avatar_url: + 'https://avatars.githubusercontent.com/u/1?v=4', + type: 'User', + }, + replies: { + nodes: [], + }, + }, + ], + }, + labels: null, + }, + ], + }, + }, + }); + + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/issues/3') + .reply(200, { + state: 'closed', + merged: true, + user: mockNotificationUser, + labels: [], + }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/issues/3/comments') + .reply(200, { + user: mockNotificationUser, + }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/4') + .reply(200, { + state: 'closed', + merged: false, + user: mockNotificationUser, + labels: [], + }); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/pulls/4/reviews') + .reply(200, {}); + nock('https://api.github.com') + .get('/repos/gitify-app/notifications-test/issues/4/comments') + .reply(200, { + user: mockNotificationUser, + }); + + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.fetchNotifications({ + auth: { + accounts: [mockGitHubCloudAccount], + }, + settings: { + ...mockSettings, + detailedNotifications: true, + }, + }); + }); + + expect(result.current.status).toBe('loading'); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + expect(result.current.notifications[0].account.hostname).toBe( + 'github.com', + ); + expect(result.current.notifications[0].notifications.length).toBe(6); + }); + it('should fetch notifications with same failures', async () => { const code = AxiosError.ERR_BAD_REQUEST; const status = 401; const message = 'Bad credentials'; nock('https://api.github.com/') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .replyWithError({ code, response: { @@ -132,7 +338,7 @@ describe('renderer/hooks/useNotifications.ts', () => { }); nock('https://github.gitify.io/api/v3/') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .replyWithError({ code, response: { @@ -163,7 +369,7 @@ describe('renderer/hooks/useNotifications.ts', () => { const code = AxiosError.ERR_BAD_REQUEST; nock('https://api.github.com/') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .replyWithError({ code, response: { @@ -175,7 +381,7 @@ describe('renderer/hooks/useNotifications.ts', () => { }); nock('https://github.gitify.io/api/v3/') - .get('/notifications?participating=false') + .get('/notifications?participating=false&all=false') .replyWithError({ code, response: { @@ -244,6 +450,89 @@ describe('renderer/hooks/useNotifications.ts', () => { expect(result.current.notifications.length).toBe(0); expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); }); + + it('should keep notifications in list when fetchReadNotifications is enabled', async () => { + const mockNotifications = [ + { + id: mockGitifyNotification.id, + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'Test notification', + type: 'Issue', + url: null, + latest_comment_url: null, + }, + repository: { + name: 'notifications-test', + full_name: 'gitify-app/notifications-test', + html_url: 'https://github.com/gitify-app/notifications-test', + owner: { + login: 'gitify-app', + avatar_url: 'https://avatar.url', + type: 'Organization', + }, + }, + }, + ]; + + // First fetch notifications to populate the state + nock('https://api.github.com') + .get('/notifications?participating=false&all=true') + .reply(200, mockNotifications); + + // Mock the mark as read endpoint + nock('https://api.github.com/') + .patch(`/notifications/threads/${id}`) + .reply(200); + + const stateWithFetchRead = { + auth: { + accounts: [mockGitHubCloudAccount], + }, + settings: { + ...mockSettings, + detailedNotifications: false, + fetchReadNotifications: true, + }, + }; + + const { result } = renderHook(() => useNotifications()); + + // First fetch notifications to populate the state + act(() => { + result.current.fetchNotifications(stateWithFetchRead); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + expect(result.current.notifications.length).toBe(1); + expect(result.current.notifications[0].notifications[0].unread).toBe( + true, + ); + + // Now mark as read with fetchReadNotifications enabled + act(() => { + result.current.markNotificationsAsRead( + stateWithFetchRead, + result.current.notifications[0].notifications, + ); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + // Notifications should still be in the list but marked as read + expect(result.current.notifications.length).toBe(1); + expect(result.current.notifications[0].notifications.length).toBe(1); + expect(result.current.notifications[0].notifications[0].unread).toBe( + false, + ); + }); }); describe('markNotificationsAsDone', () => { @@ -379,5 +668,39 @@ describe('renderer/hooks/useNotifications.ts', () => { expect(result.current.notifications.length).toBe(0); expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); }); + + it('should mark as read (not done) when markAsDoneOnUnsubscribe is true but fetchReadNotifications is enabled', async () => { + // The unsubscribe endpoint call. + nock('https://api.github.com/') + .put(`/notifications/threads/${id}/subscription`) + .reply(200); + + // The mark read endpoint call (NOT mark done). + nock('https://api.github.com/') + .patch(`/notifications/threads/${id}`) + .reply(200); + + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.unsubscribeNotification( + { + ...mockState, + settings: { + ...mockState.settings, + markAsDoneOnUnsubscribe: true, + fetchReadNotifications: true, + }, + }, + mockGitifyNotification, + ); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + expect(result.current.notifications.length).toBe(0); + }); }); }); diff --git a/src/renderer/hooks/useNotifications.ts b/src/renderer/hooks/useNotifications.ts index a13d287d5..fe6980d41 100644 --- a/src/renderer/hooks/useNotifications.ts +++ b/src/renderer/hooks/useNotifications.ts @@ -13,6 +13,7 @@ import { markNotificationThreadAsDone, markNotificationThreadAsRead, } from '../utils/api/client'; +import { getAccountUUID } from '../utils/auth/utils'; import { areAllAccountErrorsSame, doesAllAccountsHaveErrors, @@ -150,12 +151,29 @@ export const useNotifications = (): NotificationsState => { ), ); - const updatedNotifications = removeNotificationsForAccount( - readNotifications[0].account, - state.settings, - readNotifications, - notifications, - ); + // When fetchReadNotifications is enabled, keep notifications in list + // but mark them as read locally (they'll show with reduced opacity) + const updatedNotifications = state.settings.fetchReadNotifications + ? notifications.map((accountNotifications) => + getAccountUUID(accountNotifications.account) === + getAccountUUID(readNotifications[0].account) + ? { + ...accountNotifications, + notifications: accountNotifications.notifications.map( + (n) => + readNotifications.some((rn) => rn.id === n.id) + ? { ...n, unread: false } + : n, + ), + } + : accountNotifications, + ) + : removeNotificationsForAccount( + readNotifications[0].account, + state.settings, + readNotifications, + notifications, + ); setNotifications(updatedNotifications); } catch (err) { @@ -222,7 +240,12 @@ export const useNotifications = (): NotificationsState => { notification.account.token, ); - if (state.settings.markAsDoneOnUnsubscribe) { + // Fall back to mark as read when fetchReadNotifications is enabled + // since marking as done won't work correctly (API limitation) + if ( + state.settings.markAsDoneOnUnsubscribe && + !state.settings.fetchReadNotifications + ) { await markNotificationsAsDone(state, [notification]); } else { await markNotificationsAsRead(state, [notification]); diff --git a/src/renderer/routes/__snapshots__/LoginWithOAuthApp.test.tsx.snap b/src/renderer/routes/__snapshots__/LoginWithOAuthApp.test.tsx.snap index 5fefe358c..b0642180d 100644 --- a/src/renderer/routes/__snapshots__/LoginWithOAuthApp.test.tsx.snap +++ b/src/renderer/routes/__snapshots__/LoginWithOAuthApp.test.tsx.snap @@ -227,7 +227,7 @@ exports[`renderer/routes/LoginWithOAuthApp.tsx renders correctly 1`] = ` >