diff --git a/src/renderer/__helpers__/jest.setup.ts b/src/renderer/__helpers__/jest.setup.ts index 4df339369..c4ccc10e3 100644 --- a/src/renderer/__helpers__/jest.setup.ts +++ b/src/renderer/__helpers__/jest.setup.ts @@ -7,6 +7,19 @@ import '@testing-library/jest-dom'; */ import '@primer/react/test-helpers'; +/** + * Custom snapshot serializer to normalize React auto-generated IDs. + * This makes snapshots stable regardless of test execution order. + * React's useId() generates IDs like "_r_X_" where X changes based on + * how many components have rendered before. + */ +expect.addSnapshotSerializer({ + test: (val) => typeof val === 'string' && /_r_[a-z0-9]+_/.test(val), + serialize: (val: string) => { + return `"${val.replace(/_r_[a-z0-9]+_/g, '_r_ID_')}"`; + }, +}); + /** * Gitify context bridge API */ 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 50f54ef2d..9a1ad893c 100644 --- a/src/renderer/components/Sidebar.tsx +++ b/src/renderer/components/Sidebar.tsx @@ -37,7 +37,7 @@ export const Sidebar: FC = () => { status, settings, auth, - unreadNotificationCount, + notificationCount, hasUnreadNotifications, updateSetting, } = useAppContext(); @@ -92,7 +92,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 9e3d83b0b..0fea8e075 100644 --- a/src/renderer/components/__snapshots__/Sidebar.test.tsx.snap +++ b/src/renderer/components/__snapshots__/Sidebar.test.tsx.snap @@ -2,7 +2,7 @@ exports[`renderer/components/Sidebar.tsx Filter notifications highlight filters sidebar if any are saved 1`] = ` @@ -1694,7 +1694,7 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende data-wrap="nowrap" > @@ -2582,7 +2582,7 @@ exports[`renderer/components/notifications/AccountNotifications.tsx should rende data-wrap="nowrap" > @@ -442,7 +442,7 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -913,7 +913,7 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -1273,7 +1273,7 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -1690,7 +1690,7 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -2050,7 +2050,7 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > @@ -2467,7 +2467,7 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-wrap="nowrap" > - - - - @@ -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..06815afc4 100644 --- a/src/renderer/hooks/useNotifications.test.ts +++ b/src/renderer/hooks/useNotifications.test.ts @@ -3,10 +3,16 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import axios, { AxiosError } from 'axios'; import nock from 'nock'; +import { + mockGitHubCloudAccount, + mockGitHubEnterpriseServerAccount, +} from '../__mocks__/account-mocks'; import { mockGitifyNotification } from '../__mocks__/notifications-mocks'; import { mockAuth, mockSettings, mockState } from '../__mocks__/state-mocks'; import { Errors } from '../utils/errors'; import * as logger from '../utils/logger'; +import * as native from '../utils/notifications/native'; +import * as sound from '../utils/notifications/sound'; import { useNotifications } from './useNotifications'; describe('renderer/hooks/useNotifications.ts', () => { @@ -14,11 +20,21 @@ describe('renderer/hooks/useNotifications.ts', () => { .spyOn(logger, 'rendererLogError') .mockImplementation(); + const raiseSoundNotificationSpy = jest + .spyOn(sound, 'raiseSoundNotification') + .mockImplementation(); + + const raiseNativeNotificationSpy = jest + .spyOn(native, 'raiseNativeNotification') + .mockImplementation(); + beforeEach(() => { // axios will default to using the XHR adapter which can't be intercepted // by nock. So, configure axios to use the node adapter. axios.defaults.adapter = 'http'; rendererLogErrorSpy.mockReset(); + raiseSoundNotificationSpy.mockReset(); + raiseNativeNotificationSpy.mockReset(); // Reset mock notification state between tests since it's mutated mockGitifyNotification.unread = true; @@ -84,11 +100,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 +130,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 +353,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 +384,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 +396,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: { @@ -201,104 +422,210 @@ describe('renderer/hooks/useNotifications.ts', () => { expect(result.current.globalError).toBeNull(); expect(rendererLogErrorSpy).toHaveBeenCalledTimes(4); }); - }); - describe('markNotificationsAsRead', () => { - it('should mark notifications as read with success', async () => { - nock('https://api.github.com/') - .patch(`/notifications/threads/${id}`) - .reply(200); + it('should play sound when new notifications arrive and playSound is enabled', async () => { + const notifications = [ + { + id: '1', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'New notification', + type: 'Issue', + url: null, + latest_comment_url: null, + }, + repository: { + name: 'test-repo', + full_name: 'org/test-repo', + html_url: 'https://github.com/org/test-repo', + owner: { + login: 'org', + avatar_url: 'https://avatar.url', + type: 'Organization', + }, + }, + }, + ]; + + nock('https://api.github.com') + .get('/notifications?participating=false&all=false') + .reply(200, notifications); + + const stateWithSound = { + auth: { + accounts: [mockGitHubCloudAccount], + }, + settings: { + ...mockSettings, + detailedNotifications: false, + playSound: true, + showNotifications: false, + }, + }; const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markNotificationsAsRead(mockState, [ - mockGitifyNotification, - ]); + result.current.fetchNotifications(stateWithSound); }); await waitFor(() => { expect(result.current.status).toBe('success'); }); - expect(result.current.notifications.length).toBe(0); + expect(raiseSoundNotificationSpy).toHaveBeenCalledTimes(1); + expect(raiseNativeNotificationSpy).not.toHaveBeenCalled(); }); - it('should mark notifications as read with failure', async () => { - nock('https://api.github.com/') - .patch(`/notifications/threads/${id}`) - .reply(400); + it('should show native notification when new notifications arrive and showNotifications is enabled', async () => { + const notifications = [ + { + id: '2', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'New notification', + type: 'Issue', + url: null, + latest_comment_url: null, + }, + repository: { + name: 'test-repo', + full_name: 'org/test-repo', + html_url: 'https://github.com/org/test-repo', + owner: { + login: 'org', + avatar_url: 'https://avatar.url', + type: 'Organization', + }, + }, + }, + ]; + + nock('https://api.github.com') + .get('/notifications?participating=false&all=false') + .reply(200, notifications); + + const stateWithNotifications = { + auth: { + accounts: [mockGitHubCloudAccount], + }, + settings: { + ...mockSettings, + detailedNotifications: false, + playSound: false, + showNotifications: true, + }, + }; const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markNotificationsAsRead(mockState, [ - mockGitifyNotification, - ]); + result.current.fetchNotifications(stateWithNotifications); }); await waitFor(() => { expect(result.current.status).toBe('success'); }); - expect(result.current.notifications.length).toBe(0); - expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); + expect(raiseSoundNotificationSpy).not.toHaveBeenCalled(); + expect(raiseNativeNotificationSpy).toHaveBeenCalledTimes(1); }); - }); - describe('markNotificationsAsDone', () => { - it('should mark notifications as done with success', async () => { - nock('https://api.github.com/') - .delete(`/notifications/threads/${id}`) - .reply(200); + it('should play sound and show notification when both are enabled', async () => { + const notifications = [ + { + id: '3', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'New notification', + type: 'Issue', + url: null, + latest_comment_url: null, + }, + repository: { + name: 'test-repo', + full_name: 'org/test-repo', + html_url: 'https://github.com/org/test-repo', + owner: { + login: 'org', + avatar_url: 'https://avatar.url', + type: 'Organization', + }, + }, + }, + ]; + + nock('https://api.github.com') + .get('/notifications?participating=false&all=false') + .reply(200, notifications); + + const stateWithBoth = { + auth: { + accounts: [mockGitHubCloudAccount], + }, + settings: { + ...mockSettings, + detailedNotifications: false, + playSound: true, + showNotifications: true, + }, + }; const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markNotificationsAsDone(mockState, [ - mockGitifyNotification, - ]); + result.current.fetchNotifications(stateWithBoth); }); await waitFor(() => { expect(result.current.status).toBe('success'); }); - expect(result.current.notifications.length).toBe(0); + expect(raiseSoundNotificationSpy).toHaveBeenCalledTimes(1); + expect(raiseNativeNotificationSpy).toHaveBeenCalledTimes(1); }); - it('should mark notifications as done with failure', async () => { - nock('https://api.github.com/') - .delete(`/notifications/threads/${id}`) - .reply(400); + it('should not play sound or show notification when no new notifications', async () => { + // Return empty notifications - no new notifications to trigger sound/native + nock('https://api.github.com') + .get('/notifications?participating=false&all=false') + .reply(200, []); + + const stateWithBoth = { + auth: { + accounts: [mockGitHubCloudAccount], + }, + settings: { + ...mockSettings, + detailedNotifications: false, + playSound: true, + showNotifications: true, + }, + }; const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markNotificationsAsDone(mockState, [ - mockGitifyNotification, - ]); + result.current.fetchNotifications(stateWithBoth); }); await waitFor(() => { expect(result.current.status).toBe('success'); }); - expect(result.current.notifications.length).toBe(0); - expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); + expect(raiseSoundNotificationSpy).not.toHaveBeenCalled(); + expect(raiseNativeNotificationSpy).not.toHaveBeenCalled(); }); }); - describe('unsubscribeNotification', () => { - const id = 'notification-123'; - - it('should unsubscribe from a notification with success - markAsDoneOnUnsubscribe = false', async () => { - // The unsubscribe endpoint call. - nock('https://api.github.com/') - .put(`/notifications/threads/${id}/subscription`) - .reply(200); - - // The mark read endpoint call. + describe('markNotificationsAsRead', () => { + it('should mark notifications as read with success', async () => { nock('https://api.github.com/') .patch(`/notifications/threads/${id}`) .reply(200); @@ -306,8 +633,186 @@ describe('renderer/hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.unsubscribeNotification( - mockState, + result.current.markNotificationsAsRead(mockState, [ + mockGitifyNotification, + ]); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + expect(result.current.notifications.length).toBe(0); + }); + + it('should mark notifications as read with failure', async () => { + nock('https://api.github.com/') + .patch(`/notifications/threads/${id}`) + .reply(400); + + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.markNotificationsAsRead(mockState, [ + mockGitifyNotification, + ]); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + 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', () => { + it('should mark notifications as done with success', async () => { + nock('https://api.github.com/') + .delete(`/notifications/threads/${id}`) + .reply(200); + + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.markNotificationsAsDone(mockState, [ + mockGitifyNotification, + ]); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + expect(result.current.notifications.length).toBe(0); + }); + + it('should mark notifications as done with failure', async () => { + nock('https://api.github.com/') + .delete(`/notifications/threads/${id}`) + .reply(400); + + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.markNotificationsAsDone(mockState, [ + mockGitifyNotification, + ]); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + expect(result.current.notifications.length).toBe(0); + expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1); + }); + }); + + describe('unsubscribeNotification', () => { + // Use the actual notification ID from mockGitifyNotification + const unsubscribeId = mockGitifyNotification.id; + + it('should unsubscribe from a notification with success - markAsDoneOnUnsubscribe = false', async () => { + // The unsubscribe endpoint call. + nock('https://api.github.com/') + .put(`/notifications/threads/${unsubscribeId}/subscription`) + .reply(200); + + // The mark read endpoint call. + nock('https://api.github.com/') + .patch(`/notifications/threads/${unsubscribeId}`) + .reply(200); + + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.unsubscribeNotification( + mockState, mockGitifyNotification, ); }); @@ -322,12 +827,12 @@ describe('renderer/hooks/useNotifications.ts', () => { it('should unsubscribe from a notification with success - markAsDoneOnUnsubscribe = true', async () => { // The unsubscribe endpoint call. nock('https://api.github.com/') - .put(`/notifications/threads/${id}/subscription`) + .put(`/notifications/threads/${unsubscribeId}/subscription`) .reply(200); // The mark done endpoint call. nock('https://api.github.com/') - .delete(`/notifications/threads/${id}`) + .delete(`/notifications/threads/${unsubscribeId}`) .reply(200); const { result } = renderHook(() => useNotifications()); @@ -355,12 +860,12 @@ describe('renderer/hooks/useNotifications.ts', () => { it('should unsubscribe from a notification with failure', async () => { // The unsubscribe endpoint call. nock('https://api.github.com/') - .put(`/notifications/threads/${id}/subscription`) + .put(`/notifications/threads/${unsubscribeId}/subscription`) .reply(400); // The mark read endpoint call. nock('https://api.github.com/') - .patch(`/notifications/threads/${id}`) + .patch(`/notifications/threads/${unsubscribeId}`) .reply(400); const { result } = renderHook(() => useNotifications()); @@ -379,5 +884,475 @@ 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/${unsubscribeId}/subscription`) + .reply(200); + + // The mark read endpoint call (NOT mark done). + nock('https://api.github.com/') + .patch(`/notifications/threads/${unsubscribeId}`) + .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); + }); + }); + + describe('removeAccountNotifications', () => { + it('should remove notifications for a specific account', async () => { + const mockNotifications = [ + { + id: '1', + 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=false') + .reply(200, mockNotifications); + + const stateWithSingleAccount = { + auth: { + accounts: [mockGitHubCloudAccount], + }, + settings: { + ...mockSettings, + detailedNotifications: false, + }, + }; + + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.fetchNotifications(stateWithSingleAccount); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + expect(result.current.notifications.length).toBe(1); + + // Now remove the account notifications + act(() => { + result.current.removeAccountNotifications(mockGitHubCloudAccount); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + expect(result.current.notifications.length).toBe(0); + }); + }); + + describe('markNotificationsAsDone', () => { + it('should not mark as done when account does not support the feature', async () => { + // GitHub Enterprise Server without version doesn't support mark as done + const mockEnterpriseNotification = { + ...mockGitifyNotification, + account: mockGitHubEnterpriseServerAccount, // No version set + }; + + const { result } = renderHook(() => useNotifications()); + + // The API should NOT be called when account doesn't support the feature + act(() => { + result.current.markNotificationsAsDone(mockState, [ + mockEnterpriseNotification, + ]); + }); + + // Status should remain 'success' (not change to 'loading' since we return early) + expect(result.current.status).toBe('success'); + // No API calls should have been made - nock will fail if unexpected calls are made + }); + }); + + describe('markNotificationsAsRead - partial marking', () => { + it('should only mark specific notifications as read while keeping others unread', async () => { + const mockNotifications = [ + { + id: '1', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'First 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', + }, + }, + }, + { + id: '2', + unread: true, + updated_at: '2024-01-02T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'Second 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', + }, + }, + }, + ]; + + // Fetch notifications + nock('https://api.github.com') + .get('/notifications?participating=false&all=true') + .reply(200, mockNotifications); + + // Mock the mark as read endpoint for only the first notification + nock('https://api.github.com/') + .patch('/notifications/threads/1') + .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[0].notifications.length).toBe(2); + expect(result.current.notifications[0].notifications[0].unread).toBe( + true, + ); + expect(result.current.notifications[0].notifications[1].unread).toBe( + true, + ); + + // Mark only the first notification as read + act(() => { + result.current.markNotificationsAsRead(stateWithFetchRead, [ + result.current.notifications[0].notifications[0], + ]); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + // First notification should be marked as read + expect(result.current.notifications[0].notifications[0].unread).toBe( + false, + ); + // Second notification should remain unread + expect(result.current.notifications[0].notifications[1].unread).toBe( + true, + ); + }); + }); + + describe('markNotificationsAsRead - multiple accounts', () => { + it('should only update notifications for the correct account when fetchReadNotifications is enabled', async () => { + const cloudNotifications = [ + { + id: '1', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'Cloud 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', + }, + }, + }, + ]; + + const enterpriseNotifications = [ + { + id: '2', + unread: true, + updated_at: '2024-01-01T00:00:00Z', + reason: 'subscribed', + subject: { + title: 'Enterprise notification', + type: 'Issue', + url: null, + latest_comment_url: null, + }, + repository: { + name: 'enterprise-test', + full_name: 'myorg/enterprise-test', + html_url: 'https://github.gitify.io/myorg/enterprise-test', + owner: { + login: 'myorg', + avatar_url: 'https://avatar.url', + type: 'Organization', + }, + }, + }, + ]; + + // Fetch notifications for both accounts + nock('https://api.github.com') + .get('/notifications?participating=false&all=true') + .reply(200, cloudNotifications); + + nock('https://github.gitify.io/api/v3') + .get('/notifications?participating=false&all=true') + .reply(200, enterpriseNotifications); + + // Mock the mark as read endpoint for cloud account + nock('https://api.github.com/') + .patch('/notifications/threads/1') + .reply(200); + + const stateWithMultipleAccounts = { + auth: mockAuth, + settings: { + ...mockSettings, + detailedNotifications: false, + fetchReadNotifications: true, + }, + }; + + const { result } = renderHook(() => useNotifications()); + + // First fetch notifications to populate the state + act(() => { + result.current.fetchNotifications(stateWithMultipleAccounts); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + expect(result.current.notifications.length).toBe(2); + expect(result.current.notifications[0].notifications[0].unread).toBe( + true, + ); + expect(result.current.notifications[1].notifications[0].unread).toBe( + true, + ); + + // Mark only the cloud notification as read + act(() => { + result.current.markNotificationsAsRead( + stateWithMultipleAccounts, + result.current.notifications[0].notifications, + ); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + // Cloud notification should be marked as read + expect(result.current.notifications[0].notifications[0].unread).toBe( + false, + ); + // Enterprise notification should remain unread + expect(result.current.notifications[1].notifications[0].unread).toBe( + true, + ); + }); + }); + + describe('markNotificationsAsRead - fetchReadNotifications disabled', () => { + it('should remove notifications from list when fetchReadNotifications is disabled', 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=false') + .reply(200, mockNotifications); + + // Mock the mark as read endpoint + nock('https://api.github.com/') + .patch(`/notifications/threads/${mockGitifyNotification.id}`) + .reply(200); + + const stateWithFetchReadDisabled = { + auth: { + accounts: [mockGitHubCloudAccount], + }, + settings: { + ...mockSettings, + detailedNotifications: false, + fetchReadNotifications: false, + }, + }; + + const { result } = renderHook(() => useNotifications()); + + // First fetch notifications to populate the state + act(() => { + result.current.fetchNotifications(stateWithFetchReadDisabled); + }); + + 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 DISABLED + act(() => { + result.current.markNotificationsAsRead( + stateWithFetchReadDisabled, + result.current.notifications[0].notifications, + ); + }); + + await waitFor(() => { + expect(result.current.status).toBe('success'); + }); + + // Notifications should be REMOVED from the list (not just marked as read) + expect(result.current.notifications.length).toBe(1); + expect(result.current.notifications[0].notifications.length).toBe(0); + }); + }); + + describe('unsubscribeNotification - additional branch coverage', () => { + it('should mark as read when markAsDoneOnUnsubscribe is false and fetchReadNotifications is enabled', async () => { + // The unsubscribe endpoint call. + nock('https://api.github.com/') + .put(`/notifications/threads/${mockGitifyNotification.id}/subscription`) + .reply(200); + + // The mark read endpoint call (NOT mark done). + nock('https://api.github.com/') + .patch(`/notifications/threads/${mockGitifyNotification.id}`) + .reply(200); + + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.unsubscribeNotification( + { + ...mockState, + settings: { + ...mockState.settings, + markAsDoneOnUnsubscribe: false, + 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__/Filters.test.tsx.snap b/src/renderer/routes/__snapshots__/Filters.test.tsx.snap index 4d7125ce2..3bd247356 100644 --- a/src/renderer/routes/__snapshots__/Filters.test.tsx.snap +++ b/src/renderer/routes/__snapshots__/Filters.test.tsx.snap @@ -2030,7 +2030,7 @@ exports[`renderer/routes/Filters.tsx General should render itself & its children data-wrap="nowrap" >