diff --git a/src/__mocks__/notifications-mocks.ts b/src/__mocks__/notifications-mocks.ts index 438ab37b0..0abbe39cc 100644 --- a/src/__mocks__/notifications-mocks.ts +++ b/src/__mocks__/notifications-mocks.ts @@ -12,10 +12,12 @@ export const mockAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: mockGitHubNotifications, + error: null, }, { account: mockGitHubEnterpriseServerAccount, notifications: mockEnterpriseNotifications, + error: null, }, ]; @@ -23,5 +25,6 @@ export const mockSingleAccountNotifications: AccountNotifications[] = [ { account: mockGitHubCloudAccount, notifications: [mockGitHubNotifications[0]], + error: null, }, ]; diff --git a/src/components/AccountNotifications.test.tsx b/src/components/AccountNotifications.test.tsx index 3b970f4b0..7df675406 100644 --- a/src/components/AccountNotifications.test.tsx +++ b/src/components/AccountNotifications.test.tsx @@ -11,11 +11,18 @@ jest.mock('./RepositoryNotifications', () => ({ })); describe('components/AccountNotifications.tsx', () => { - it('should render itself (github.com with notifications) - group by repositories', () => { + // The read emoji randomly rotates, but then the snapshots would never match + // Have to make it consistent so the emojis are always the same + beforeEach(() => { + global.Math.random = jest.fn(() => 0.1); + }); + + it('should render itself - group notifications by repositories', () => { const props = { account: mockGitHubCloudAccount, notifications: mockGitHubNotifications, showAccountHostname: true, + error: null, }; const tree = render( @@ -28,11 +35,12 @@ describe('components/AccountNotifications.tsx', () => { expect(tree).toMatchSnapshot(); }); - it('should render itself (github.com with notifications) - group by date', () => { + it('should render itself - group notifications by date', () => { const props = { account: mockGitHubCloudAccount, notifications: mockGitHubNotifications, showAccountHostname: true, + error: null, }; const tree = render( @@ -45,10 +53,31 @@ describe('components/AccountNotifications.tsx', () => { expect(tree).toMatchSnapshot(); }); - it('should render itself (github.com without notifications)', () => { + it('should render itself - no notifications', () => { + const props = { + account: mockGitHubCloudAccount, + notifications: [], + showAccountHostname: true, + error: null, + }; + + const tree = render( + + + , + ); + expect(tree).toMatchSnapshot(); + }); + + it('should render itself - account error', () => { const props = { account: mockGitHubCloudAccount, notifications: [], + error: { + title: 'Error title', + descriptions: ['Error description'], + emojis: ['🔥'], + }, showAccountHostname: true, }; @@ -69,6 +98,7 @@ describe('components/AccountNotifications.tsx', () => { account: mockGitHubCloudAccount, notifications: [], showAccountHostname: true, + error: null, }; await act(async () => { @@ -90,6 +120,7 @@ describe('components/AccountNotifications.tsx', () => { account: mockGitHubCloudAccount, notifications: mockGitHubNotifications, showAccountHostname: true, + error: null, }; await act(async () => { diff --git a/src/components/AccountNotifications.tsx b/src/components/AccountNotifications.tsx index 5d9f1552a..3714bc649 100644 --- a/src/components/AccountNotifications.tsx +++ b/src/components/AccountNotifications.tsx @@ -3,14 +3,16 @@ import { ChevronLeftIcon, ChevronUpIcon, } from '@primer/octicons-react'; -import { type FC, type MouseEvent, useContext, useState } from 'react'; +import { type FC, type MouseEvent, useContext, useMemo, useState } from 'react'; import { AppContext } from '../context/App'; -import { type Account, Opacity, Size } from '../types'; +import { type Account, type GitifyError, Opacity, Size } from '../types'; import type { Notification } from '../typesGitHub'; import { cn } from '../utils/cn'; import { openAccountProfile } from '../utils/links'; +import { AllRead } from './AllRead'; import { HoverGroup } from './HoverGroup'; import { NotificationRow } from './NotificationRow'; +import { Oops } from './Oops'; import { RepositoryNotifications } from './RepositoryNotifications'; import { InteractionButton } from './buttons/InteractionButton'; import { PlatformIcon } from './icons/PlatformIcon'; @@ -18,6 +20,7 @@ import { PlatformIcon } from './icons/PlatformIcon'; interface IAccountNotifications { account: Account; notifications: Notification[]; + error: GitifyError | null; showAccountHostname: boolean; } @@ -40,6 +43,11 @@ export const AccountNotifications: FC = ( ), ); + const hasNotifications = useMemo( + () => notifications.length > 0, + [notifications], + ); + const [showAccountNotifications, setShowAccountNotifications] = useState(true); @@ -68,8 +76,10 @@ export const AccountNotifications: FC = ( {showAccountHostname && (
= ( {showAccountNotifications && ( <> + {props.error && } + {!hasNotifications && !props.error && } {groupByRepository ? Object.values(groupedNotifications).map((repoNotifications) => { const repoSlug = repoNotifications[0].repository.full_name; diff --git a/src/components/AllRead.tsx b/src/components/AllRead.tsx index 4ba62b0e5..b34c932b9 100644 --- a/src/components/AllRead.tsx +++ b/src/components/AllRead.tsx @@ -12,7 +12,7 @@ export const AllRead: FC = () => { return (
-

{emoji}

+

{emoji}

No new notifications.

diff --git a/src/components/Oops.tsx b/src/components/Oops.tsx index 791424c8f..7f7b21ea4 100644 --- a/src/components/Oops.tsx +++ b/src/components/Oops.tsx @@ -13,7 +13,7 @@ export const Oops: FC = ({ error }: IOops) => { return (
-

{emoji}

+

{emoji}

{error.title}

{error.descriptions.map((description, i) => { diff --git a/src/components/__snapshots__/AccountNotifications.test.tsx.snap b/src/components/__snapshots__/AccountNotifications.test.tsx.snap index a466615a7..f37cb8bcd 100644 --- a/src/components/__snapshots__/AccountNotifications.test.tsx.snap +++ b/src/components/__snapshots__/AccountNotifications.test.tsx.snap @@ -1,12 +1,239 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`components/AccountNotifications.tsx should render itself (github.com with notifications) - group by date 1`] = ` +exports[`components/AccountNotifications.tsx should render itself - account error 1`] = ` { "asFragment": [Function], "baseElement":
+
+
+ + + +
+ +
+
+ +
+
+
+

+ 🔥 +

+

+ Error title +

+
+ Error description +
+
+
+ , + "container":
+
+
+
+ + + +
+ +
+
+ +
+
+
+

+ 🔥 +

+

+ Error title +

+
+ Error description +
+
+
, + "debug": [Function], + "findAllByAltText": [Function], + "findAllByDisplayValue": [Function], + "findAllByLabelText": [Function], + "findAllByPlaceholderText": [Function], + "findAllByRole": [Function], + "findAllByTestId": [Function], + "findAllByText": [Function], + "findAllByTitle": [Function], + "findByAltText": [Function], + "findByDisplayValue": [Function], + "findByLabelText": [Function], + "findByPlaceholderText": [Function], + "findByRole": [Function], + "findByTestId": [Function], + "findByText": [Function], + "findByTitle": [Function], + "getAllByAltText": [Function], + "getAllByDisplayValue": [Function], + "getAllByLabelText": [Function], + "getAllByPlaceholderText": [Function], + "getAllByRole": [Function], + "getAllByTestId": [Function], + "getAllByText": [Function], + "getAllByTitle": [Function], + "getByAltText": [Function], + "getByDisplayValue": [Function], + "getByLabelText": [Function], + "getByPlaceholderText": [Function], + "getByRole": [Function], + "getByTestId": [Function], + "getByText": [Function], + "getByTitle": [Function], + "queryAllByAltText": [Function], + "queryAllByDisplayValue": [Function], + "queryAllByLabelText": [Function], + "queryAllByPlaceholderText": [Function], + "queryAllByRole": [Function], + "queryAllByTestId": [Function], + "queryAllByText": [Function], + "queryAllByTitle": [Function], + "queryByAltText": [Function], + "queryByDisplayValue": [Function], + "queryByLabelText": [Function], + "queryByPlaceholderText": [Function], + "queryByRole": [Function], + "queryByTestId": [Function], + "queryByText": [Function], + "queryByTitle": [Function], + "rerender": [Function], + "unmount": [Function], +} +`; + +exports[`components/AccountNotifications.tsx should render itself - group notifications by date 1`] = ` +{ + "asFragment": [Function], + "baseElement": +
+
, "container":
, "container":
+
+

+ 🎊 +

+

+ No new notifications. +

+
, "container":
+
+

+ 🎊 +

+

+ No new notifications. +

+
, "debug": [Function], "findAllByAltText": [Function], @@ -1311,7 +1566,7 @@ exports[`components/AccountNotifications.tsx should toggle account notifications "baseElement":
, "container":

🎊

@@ -26,7 +26,7 @@ exports[`components/AllRead.tsx should render itself & its children 1`] = ` class="flex flex-1 flex-col items-center justify-center bg-white p-4 text-black dark:bg-gray-dark dark:text-white" >

🎊

diff --git a/src/components/__snapshots__/Oops.test.tsx.snap b/src/components/__snapshots__/Oops.test.tsx.snap index 3c3a7b930..654158f4c 100644 --- a/src/components/__snapshots__/Oops.test.tsx.snap +++ b/src/components/__snapshots__/Oops.test.tsx.snap @@ -9,7 +9,7 @@ exports[`components/Oops.tsx should render itself & its children 1`] = ` class="flex flex-1 flex-col items-center justify-center p-4" >

🔥

@@ -31,7 +31,7 @@ exports[`components/Oops.tsx should render itself & its children 1`] = ` class="flex flex-1 flex-col items-center justify-center p-4" >

🔥

diff --git a/src/context/App.tsx b/src/context/App.tsx index 5bd6ddad0..90cf29a1d 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -100,7 +100,7 @@ interface AppContextState { notifications: AccountNotifications[]; status: Status; - errorDetails: GitifyError; + globalError: GitifyError; fetchNotifications: () => Promise; markNotificationRead: (notification: Notification) => Promise; markNotificationDone: (notification: Notification) => Promise; @@ -122,7 +122,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { const { fetchNotifications, notifications, - errorDetails, + globalError, status, markNotificationRead, markNotificationDone, @@ -311,7 +311,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { notifications, status, - errorDetails, + globalError, fetchNotifications: fetchNotificationsWithAccounts, markNotificationRead: markNotificationReadWithAccounts, markNotificationDone: markNotificationDoneWithAccounts, diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts index e4b3af7cc..730364ea7 100644 --- a/src/hooks/useNotifications.test.ts +++ b/src/hooks/useNotifications.test.ts @@ -252,10 +252,10 @@ describe('hooks/useNotifications.ts', () => { expect(result.current.notifications[0].notifications.length).toBe(6); }); - it('should fetch notifications with failures', async () => { + it('should fetch notifications with same failures', async () => { const code = AxiosError.ERR_BAD_REQUEST; - const status = 400; - const message = 'Oops! Something went wrong.'; + const status = 401; + const message = 'Bad credentials'; nock('https://api.github.com/') .get('/notifications?participating=false') @@ -293,7 +293,49 @@ describe('hooks/useNotifications.ts', () => { expect(result.current.status).toBe('error'); }); - expect(result.current.errorDetails).toBe(Errors.UNKNOWN); + expect(result.current.globalError).toBe(Errors.BAD_CREDENTIALS); + }); + + it('should fetch notifications with different failures', async () => { + const code = AxiosError.ERR_BAD_REQUEST; + + nock('https://api.github.com/') + .get('/notifications?participating=false') + .replyWithError({ + code, + response: { + status: 400, + data: { + message: 'Oops! Something went wrong.', + }, + }, + }); + + nock('https://github.gitify.io/api/v3/') + .get('/notifications?participating=false') + .replyWithError({ + code, + response: { + status: 401, + data: { + message: 'Bad credentials', + }, + }, + }); + + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.fetchNotifications(mockState); + }); + + expect(result.current.status).toBe('loading'); + + await waitFor(() => { + expect(result.current.status).toBe('error'); + }); + + expect(result.current.globalError).toBeNull(); }); }); diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index b8fb28499..76415d384 100644 --- a/src/hooks/useNotifications.ts +++ b/src/hooks/useNotifications.ts @@ -12,7 +12,6 @@ import { markNotificationThreadAsRead, markRepositoryNotificationsAsRead, } from '../utils/api/client'; -import { determineFailureType } from '../utils/api/errors'; import { getAccountUUID } from '../utils/auth/utils'; import { getAllNotifications, @@ -46,12 +45,12 @@ interface NotificationsState { notification: Notification, ) => Promise; status: Status; - errorDetails: GitifyError; + globalError: GitifyError; } export const useNotifications = (): NotificationsState => { const [status, setStatus] = useState('success'); - const [errorDetails, setErrorDetails] = useState(); + const [globalError, setGlobalError] = useState(); const [notifications, setNotifications] = useState( [], @@ -61,16 +60,30 @@ export const useNotifications = (): NotificationsState => { async (state: GitifyState) => { setStatus('loading'); - try { - const fetchedNotifications = await getAllNotifications(state); + const fetchedNotifications = await getAllNotifications(state); + + // Set Global Error if all accounts have the same error + const allAccountsHaveErrors = fetchedNotifications.every((account) => { + return account.error !== null; + }); + let accountErrorsAreAllSame = true; + const accountError = fetchedNotifications[0]?.error; + for (const fetchedNotification of fetchedNotifications) { + if (accountError !== fetchedNotification.error) { + accountErrorsAreAllSame = false; + break; + } + } - setNotifications(fetchedNotifications); - triggerNativeNotifications(notifications, fetchedNotifications, state); - setStatus('success'); - } catch (err) { + if (allAccountsHaveErrors) { setStatus('error'); - setErrorDetails(determineFailureType(err)); + setGlobalError(accountErrorsAreAllSame ? accountError : null); + return; } + + setNotifications(fetchedNotifications); + triggerNativeNotifications(notifications, fetchedNotifications, state); + setStatus('success'); }, [notifications], ); @@ -223,7 +236,7 @@ export const useNotifications = (): NotificationsState => { return { status, - errorDetails, + globalError, notifications, fetchNotifications, diff --git a/src/routes/Notifications.test.tsx b/src/routes/Notifications.test.tsx index 6fb0edc39..207083b45 100644 --- a/src/routes/Notifications.test.tsx +++ b/src/routes/Notifications.test.tsx @@ -58,7 +58,7 @@ describe('routes/Notifications.tsx', () => { value={{ notifications: [], status: 'error', - errorDetails: Errors.BAD_CREDENTIALS, + globalError: Errors.BAD_CREDENTIALS, }} > @@ -74,7 +74,7 @@ describe('routes/Notifications.tsx', () => { value={{ notifications: [], status: 'error', - errorDetails: Errors.MISSING_SCOPES, + globalError: Errors.MISSING_SCOPES, }} > @@ -90,7 +90,7 @@ describe('routes/Notifications.tsx', () => { value={{ notifications: [], status: 'error', - errorDetails: Errors.RATE_LIMITED, + globalError: Errors.RATE_LIMITED, }} > @@ -106,7 +106,7 @@ describe('routes/Notifications.tsx', () => { value={{ notifications: [], status: 'error', - errorDetails: Errors.UNKNOWN, + globalError: Errors.UNKNOWN, }} > @@ -122,7 +122,7 @@ describe('routes/Notifications.tsx', () => { value={{ notifications: [], status: 'error', - errorDetails: null, + globalError: null, }} > diff --git a/src/routes/Notifications.tsx b/src/routes/Notifications.tsx index 6d8019c23..17543e543 100644 --- a/src/routes/Notifications.tsx +++ b/src/routes/Notifications.tsx @@ -4,31 +4,31 @@ import { AllRead } from '../components/AllRead'; import { Oops } from '../components/Oops'; import { AppContext } from '../context/App'; import { getAccountUUID } from '../utils/auth/utils'; -import { Errors } from '../utils/constants'; import { getNotificationCount } from '../utils/notifications'; export const NotificationsRoute: FC = () => { - const { notifications, status, errorDetails, settings } = - useContext(AppContext); + const { notifications, globalError, settings } = useContext(AppContext); const hasMultipleAccounts = useMemo( () => notifications.length > 1, [notifications], ); - const notificationsCount = useMemo(() => { - return getNotificationCount(notifications); - }, [notifications]); + + const hasNoAccountErrors = useMemo( + () => notifications.every((account) => account.error === null), + [notifications], + ); const hasNotifications = useMemo( - () => notificationsCount > 0, - [notificationsCount], + () => getNotificationCount(notifications) > 0, + [notifications], ); - if (status === 'error') { - return ; + if (globalError) { + return ; } - if (!hasNotifications) { + if (!hasNotifications && hasNoAccountErrors) { return ; } @@ -39,6 +39,7 @@ export const NotificationsRoute: FC = () => { key={getAccountUUID(accountNotifications.account)} account={accountNotifications.account} notifications={accountNotifications.notifications} + error={accountNotifications.error} showAccountHostname={ hasMultipleAccounts || settings.showAccountHostname } diff --git a/src/routes/__snapshots__/Notifications.test.tsx.snap b/src/routes/__snapshots__/Notifications.test.tsx.snap index 8bb99d300..5dbd73147 100644 --- a/src/routes/__snapshots__/Notifications.test.tsx.snap +++ b/src/routes/__snapshots__/Notifications.test.tsx.snap @@ -144,13 +144,13 @@ exports[`routes/Notifications.tsx should render itself & its children (error con "baseElement":

- Oops + AllRead

, "container":

- Oops + AllRead

, "debug": [Function], diff --git a/src/types.ts b/src/types.ts index a51c648e5..e7cac771f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -123,6 +123,7 @@ export type RadioGroupItem = { export interface AccountNotifications { account: Account; notifications: Notification[]; + error: GitifyError | null; } export interface GitifyUser { diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 44ffcc0e2..35ad9d2a5 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -5,6 +5,7 @@ import type { } from '../types'; import { Notification } from '../typesGitHub'; import { listNotificationsForAuthenticatedUser } from './api/client'; +import { determineFailureType } from './api/errors'; import { getAccountUUID } from './auth/utils'; import { hideWindow, showWindow, updateTrayIcon } from './comms'; import { openNotification } from './links'; @@ -129,21 +130,30 @@ export async function getAllNotifications( responses .filter((response) => !!response) .map(async (accountNotifications) => { - let notifications = (await accountNotifications.notifications).data.map( - (notification: Notification) => ({ + try { + let notifications = ( + await accountNotifications.notifications + ).data.map((notification: Notification) => ({ ...notification, account: accountNotifications.account, - }), - ); + })); - notifications = await enrichNotifications(notifications, state); + notifications = await enrichNotifications(notifications, state); - notifications = filterNotifications(notifications, state.settings); + notifications = filterNotifications(notifications, state.settings); - return { - account: accountNotifications.account, - notifications: notifications, - }; + return { + account: accountNotifications.account, + notifications: notifications, + error: null, + }; + } catch (error) { + return { + account: accountNotifications.account, + notifications: [], + error: determineFailureType(error), + }; + } }), );