From 4d139b5d4a1fabea730704cefb1e62f3ea14190c Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 7 Jun 2024 17:53:50 -0400 Subject: [PATCH 1/9] feat: support multiple accounts --- src/components/AccountNotifications.test.tsx | 5 +- src/components/AccountNotifications.tsx | 9 ++- src/components/NotificationRow.tsx | 10 +-- src/components/Repository.test.tsx | 13 ++-- src/components/Repository.tsx | 6 +- src/context/App.test.tsx | 36 +++------ src/context/App.tsx | 34 ++++---- src/hooks/useNotifications.test.ts | 54 +++++++------ src/hooks/useNotifications.ts | 82 ++++++++++---------- src/routes/Accounts.tsx | 6 -- src/routes/Notifications.tsx | 5 +- src/utils/helpers.test.ts | 49 +----------- src/utils/helpers.ts | 15 ---- src/utils/remove-notification.test.ts | 9 +-- src/utils/remove-notification.ts | 11 +-- 15 files changed, 130 insertions(+), 214 deletions(-) diff --git a/src/components/AccountNotifications.test.tsx b/src/components/AccountNotifications.test.tsx index f52955e5b..9a841d397 100644 --- a/src/components/AccountNotifications.test.tsx +++ b/src/components/AccountNotifications.test.tsx @@ -1,4 +1,5 @@ import { render } from '@testing-library/react'; +import { mockGitHubCloudAccount } from '../__mocks__/state-mocks'; import { mockGitHubNotifications } from '../utils/api/__mocks__/response-mocks'; import { AccountNotifications } from './AccountNotifications'; @@ -9,7 +10,7 @@ jest.mock('./Repository', () => ({ describe('components/AccountNotifications.tsx', () => { it('should render itself (github.com with notifications)', () => { const props = { - hostname: 'github.com', + account: mockGitHubCloudAccount, notifications: mockGitHubNotifications, showAccountHostname: true, }; @@ -20,7 +21,7 @@ describe('components/AccountNotifications.tsx', () => { it('should render itself (github.com without notifications)', () => { const props = { - hostname: 'github.com', + account: mockGitHubCloudAccount, notifications: [], showAccountHostname: true, }; diff --git a/src/components/AccountNotifications.tsx b/src/components/AccountNotifications.tsx index 738dd8098..8dabb331a 100644 --- a/src/components/AccountNotifications.tsx +++ b/src/components/AccountNotifications.tsx @@ -1,15 +1,16 @@ import { ChevronDownIcon, ChevronLeftIcon } from '@primer/octicons-react'; +import type { Account } from '../types'; import type { Notification } from '../typesGitHub'; import { RepositoryNotifications } from './Repository'; interface IProps { - hostname: string; + account: Account; notifications: Notification[]; showAccountHostname: boolean; } export const AccountNotifications = (props: IProps) => { - const { hostname, showAccountHostname, notifications } = props; + const { account, showAccountHostname, notifications } = props; const groupedNotifications = Object.values( notifications.reduce( @@ -29,7 +30,7 @@ export const AccountNotifications = (props: IProps) => { <> {showAccountHostname && (
- {hostname} + {account.hostname} - {account.user.login}
)} @@ -40,7 +41,7 @@ export const AccountNotifications = (props: IProps) => { return ( diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 76575e2fc..bb062c90f 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -53,10 +53,10 @@ export const NotificationRow: FC = ({ notification, hostname }) => { openInBrowser(notification); if (settings.markAsDoneOnOpen) { - markNotificationDone(notification.id, hostname); + markNotificationDone(notification); } else { // no need to mark as read, github does it by default when opening it - removeNotificationFromState(settings, notification.id, hostname); + removeNotificationFromState(settings, notification); } }, [notifications, notification, auth, settings]); // notifications required here to prevent weird state issues @@ -64,7 +64,7 @@ export const NotificationRow: FC = ({ notification, hostname }) => { // Don't trigger onClick of parent element. event.stopPropagation(); - unsubscribeNotification(notification.id, hostname); + unsubscribeNotification(notification); }; const openUserProfile = ( @@ -237,7 +237,7 @@ export const NotificationRow: FC = ({ notification, hostname }) => { type="button" className="focus:outline-none h-full hover:text-green-500" title="Mark as Done" - onClick={() => markNotificationDone(notification.id, hostname)} + onClick={() => markNotificationDone(notification)} > @@ -255,7 +255,7 @@ export const NotificationRow: FC = ({ notification, hostname }) => { type="button" className="focus:outline-none h-full hover:text-green-500" title="Mark as Read" - onClick={() => markNotificationRead(notification.id, hostname)} + onClick={() => markNotificationRead(notification)} > diff --git a/src/components/Repository.test.tsx b/src/components/Repository.test.tsx index 888e76584..8afdac5ba 100644 --- a/src/components/Repository.test.tsx +++ b/src/components/Repository.test.tsx @@ -1,7 +1,10 @@ import { fireEvent, render, screen } from '@testing-library/react'; import { shell } from 'electron'; import { AppContext } from '../context/App'; -import { mockGitHubNotifications } from '../utils/api/__mocks__/response-mocks'; +import { + mockGitHubNotifications, + mockSingleNotification, +} from '../utils/api/__mocks__/response-mocks'; import { RepositoryNotifications } from './Repository'; jest.mock('./NotificationRow', () => ({ @@ -57,10 +60,7 @@ describe('components/Repository.tsx', () => { fireEvent.click(screen.getByTitle('Mark Repository as Read')); - expect(markRepoNotifications).toHaveBeenCalledWith( - 'gitify-app/notifications-test', - 'github.com', - ); + expect(markRepoNotifications).toHaveBeenCalledWith(mockSingleNotification); }); it('should mark a repo as done', () => { @@ -73,8 +73,7 @@ describe('components/Repository.tsx', () => { fireEvent.click(screen.getByTitle('Mark Repository as Done')); expect(markRepoNotificationsDone).toHaveBeenCalledWith( - 'gitify-app/notifications-test', - 'github.com', + mockSingleNotification, ); }); diff --git a/src/components/Repository.tsx b/src/components/Repository.tsx index 2b7877617..5120ea4dd 100644 --- a/src/components/Repository.tsx +++ b/src/components/Repository.tsx @@ -26,13 +26,11 @@ export const RepositoryNotifications: FC = ({ }, [repoNotifications]); const markRepoAsRead = useCallback(() => { - const repoSlug = repoNotifications[0].repository.full_name; - markRepoNotifications(repoSlug, hostname); + markRepoNotifications(repoNotifications[0]); }, [repoNotifications, hostname]); const markRepoAsDone = useCallback(() => { - const repoSlug = repoNotifications[0].repository.full_name; - markRepoNotificationsDone(repoSlug, hostname); + markRepoNotificationsDone(repoNotifications[0]); }, [repoNotifications, hostname]); const avatarUrl = repoNotifications[0].repository.owner.avatar_url; diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx index 5773b80e5..876887c62 100644 --- a/src/context/App.test.tsx +++ b/src/context/App.test.tsx @@ -3,6 +3,7 @@ import { useContext } from 'react'; import { mockAuth, mockSettings } from '../__mocks__/state-mocks'; import { useNotifications } from '../hooks/useNotifications'; import type { AuthState, SettingsState } from '../types'; +import { mockSingleNotification } from '../utils/api/__mocks__/response-mocks'; import * as apiRequests from '../utils/api/request'; import * as comms from '../utils/comms'; import Constants from '../utils/constants'; @@ -121,7 +122,7 @@ describe('context/App.tsx', () => { return ( @@ -137,8 +138,7 @@ describe('context/App.tsx', () => { expect(markNotificationReadMock).toHaveBeenCalledTimes(1); expect(markNotificationReadMock).toHaveBeenCalledWith( mockDefaultState, - '123-456', - 'github.com', + mockSingleNotification, ); }); @@ -149,7 +149,7 @@ describe('context/App.tsx', () => { return ( @@ -165,8 +165,7 @@ describe('context/App.tsx', () => { expect(markNotificationDoneMock).toHaveBeenCalledTimes(1); expect(markNotificationDoneMock).toHaveBeenCalledWith( mockDefaultState, - '123-456', - 'github.com', + mockSingleNotification, ); }); @@ -177,7 +176,7 @@ describe('context/App.tsx', () => { return ( @@ -193,8 +192,7 @@ describe('context/App.tsx', () => { expect(unsubscribeNotificationMock).toHaveBeenCalledTimes(1); expect(unsubscribeNotificationMock).toHaveBeenCalledWith( mockDefaultState, - '123-456', - 'github.com', + mockSingleNotification, ); }); @@ -205,12 +203,7 @@ describe('context/App.tsx', () => { return ( @@ -226,8 +219,7 @@ describe('context/App.tsx', () => { expect(markRepoNotificationsMock).toHaveBeenCalledTimes(1); expect(markRepoNotificationsMock).toHaveBeenCalledWith( mockDefaultState, - 'gitify-app/notifications-test', - 'github.com', + mockSingleNotification, ); }); @@ -238,12 +230,7 @@ describe('context/App.tsx', () => { return ( @@ -267,8 +254,7 @@ describe('context/App.tsx', () => { }, settings: mockSettings, }, - 'gitify-app/notifications-test', - 'github.com', + mockSingleNotification, ); }); }); diff --git a/src/context/App.tsx b/src/context/App.tsx index 6f138eaed..0c2404753 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -17,6 +17,7 @@ import { type Status, Theme, } from '../types'; +import type { Notification } from '../typesGitHub'; import { headNotifications } from '../utils/api/client'; import { migrateAuthenticatedAccounts } from '../utils/auth/migration'; import type { @@ -72,15 +73,14 @@ interface AppContextState { errorDetails: GitifyError; removeNotificationFromState: ( settings: SettingsState, - id: string, - hostname: string, + notification: Notification, ) => void; fetchNotifications: () => Promise; - markNotificationRead: (id: string, hostname: string) => Promise; - markNotificationDone: (id: string, hostname: string) => Promise; - unsubscribeNotification: (id: string, hostname: string) => Promise; - markRepoNotifications: (id: string, hostname: string) => Promise; - markRepoNotificationsDone: (id: string, hostname: string) => Promise; + markNotificationRead: (notification: Notification) => Promise; + markNotificationDone: (notification: Notification) => Promise; + unsubscribeNotification: (notification: Notification) => Promise; + markRepoNotifications: (notification: Notification) => Promise; + markRepoNotificationsDone: (notification: Notification) => Promise; settings: SettingsState; updateSetting: ( @@ -231,32 +231,32 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { ); const markNotificationReadWithAccounts = useCallback( - async (id: string, hostname: string) => - await markNotificationRead({ auth, settings }, id, hostname), + async (notification: Notification) => + await markNotificationRead({ auth, settings }, notification), [auth, notifications], ); const markNotificationDoneWithAccounts = useCallback( - async (id: string, hostname: string) => - await markNotificationDone({ auth, settings }, id, hostname), + async (notification: Notification) => + await markNotificationDone({ auth, settings }, notification), [auth, notifications], ); const unsubscribeNotificationWithAccounts = useCallback( - async (id: string, hostname: string) => - await unsubscribeNotification({ auth, settings }, id, hostname), + async (notification: Notification) => + await unsubscribeNotification({ auth, settings }, notification), [auth, notifications], ); const markRepoNotificationsWithAccounts = useCallback( - async (repoSlug: string, hostname: string) => - await markRepoNotifications({ auth, settings }, repoSlug, hostname), + async (notification: Notification) => + await markRepoNotifications({ auth, settings }, notification), [auth, notifications], ); const markRepoNotificationsDoneWithAccounts = useCallback( - async (repoSlug: string, hostname: string) => - await markRepoNotificationsDone({ auth, settings }, repoSlug, hostname), + async (notification: Notification) => + await markRepoNotificationsDone({ auth, settings }, notification), [auth, notifications], ); diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts index 049732824..8d36a9fd4 100644 --- a/src/hooks/useNotifications.test.ts +++ b/src/hooks/useNotifications.test.ts @@ -8,7 +8,10 @@ import { mockSettings, mockState, } from '../__mocks__/state-mocks'; -import { mockNotificationUser } from '../utils/api/__mocks__/response-mocks'; +import { + mockNotificationUser, + mockSingleNotification, +} from '../utils/api/__mocks__/response-mocks'; import { Errors } from '../utils/constants'; import { useNotifications } from './useNotifications'; @@ -19,6 +22,8 @@ describe('hooks/useNotifications.ts', () => { axios.defaults.adapter = 'http'; }); + const id = mockSingleNotification.id; + describe('fetchNotifications', () => { it('should fetch non-detailed notifications with success', async () => { const mockState = { @@ -323,8 +328,7 @@ describe('hooks/useNotifications.ts', () => { act(() => { result.current.removeNotificationFromState( mockSettings, - result.current.notifications[0].notifications[0].id, - result.current.notifications[0].account.hostname, + result.current.notifications[0].notifications[0], ); }); @@ -333,9 +337,6 @@ describe('hooks/useNotifications.ts', () => { }); describe('markNotificationRead', () => { - const hostname = 'github.com'; - const id = 'notification-123'; - it('should mark a notification as read with success', async () => { nock('https://api.github.com/') .patch(`/notifications/threads/${id}`) @@ -344,7 +345,7 @@ describe('hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markNotificationRead(mockState, id, hostname); + result.current.markNotificationRead(mockState, mockSingleNotification); }); await waitFor(() => { @@ -362,7 +363,7 @@ describe('hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markNotificationRead(mockState, id, hostname); + result.current.markNotificationRead(mockState, mockSingleNotification); }); await waitFor(() => { @@ -374,9 +375,6 @@ describe('hooks/useNotifications.ts', () => { }); describe('markNotificationDone', () => { - const hostname = 'github.com'; - const id = 'notification-123'; - it('should mark a notification as done with success', async () => { nock('https://api.github.com/') .delete(`/notifications/threads/${id}`) @@ -385,7 +383,7 @@ describe('hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markNotificationDone(mockState, id, hostname); + result.current.markNotificationDone(mockState, mockSingleNotification); }); await waitFor(() => { @@ -403,7 +401,7 @@ describe('hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markNotificationDone(mockState, id, hostname); + result.current.markNotificationDone(mockState, mockSingleNotification); }); await waitFor(() => { @@ -415,7 +413,6 @@ describe('hooks/useNotifications.ts', () => { }); describe('unsubscribeNotification', () => { - const hostname = 'github.com'; const id = 'notification-123'; it('should unsubscribe from a notification with success', async () => { @@ -432,7 +429,10 @@ describe('hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.unsubscribeNotification(mockState, id, hostname); + result.current.unsubscribeNotification( + mockState, + mockSingleNotification, + ); }); await waitFor(() => { @@ -456,7 +456,10 @@ describe('hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.unsubscribeNotification(mockState, id, hostname); + result.current.unsubscribeNotification( + mockState, + mockSingleNotification, + ); }); await waitFor(() => { @@ -468,7 +471,6 @@ describe('hooks/useNotifications.ts', () => { }); describe('markRepoNotifications', () => { - const hostname = 'github.com'; const repoSlug = 'gitify-app/notifications-test'; it("should mark a repository's notifications as read with success", async () => { @@ -479,7 +481,7 @@ describe('hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markRepoNotifications(mockState, repoSlug, hostname); + result.current.markRepoNotifications(mockState, mockSingleNotification); }); await waitFor(() => { @@ -497,7 +499,7 @@ describe('hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markRepoNotifications(mockState, repoSlug, hostname); + result.current.markRepoNotifications(mockState, mockSingleNotification); }); await waitFor(() => { @@ -509,10 +511,6 @@ describe('hooks/useNotifications.ts', () => { }); describe('markRepoNotificationsDone', () => { - const hostname = 'github.com'; - const repoSlug = 'gitify-app/notifications-test'; - const id = 'notification-123'; - it("should mark a repository's notifications as done with success", async () => { nock('https://api.github.com/') .delete(`/notifications/threads/${id}`) @@ -521,7 +519,10 @@ describe('hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markRepoNotificationsDone(mockState, repoSlug, hostname); + result.current.markRepoNotificationsDone( + mockState, + mockSingleNotification, + ); }); await waitFor(() => { @@ -539,7 +540,10 @@ describe('hooks/useNotifications.ts', () => { const { result } = renderHook(() => useNotifications()); act(() => { - result.current.markRepoNotificationsDone(mockState, repoSlug, hostname); + result.current.markRepoNotificationsDone( + mockState, + mockSingleNotification, + ); }); await waitFor(() => { diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index 7dd93dff4..1d35f5c22 100644 --- a/src/hooks/useNotifications.ts +++ b/src/hooks/useNotifications.ts @@ -6,6 +6,7 @@ import type { SettingsState, Status, } from '../types'; +import type { Notification } from '../typesGitHub'; import { ignoreNotificationThreadSubscription, markNotificationThreadAsDone, @@ -13,7 +14,6 @@ import { markRepositoryNotificationsAsRead, } from '../utils/api/client'; import { determineFailureType } from '../utils/api/errors'; -import { getAccountForHost } from '../utils/helpers'; import { getAllNotifications, setTrayIconColor, @@ -26,34 +26,28 @@ interface NotificationsState { notifications: AccountNotifications[]; removeNotificationFromState: ( settings: SettingsState, - id: string, - hostname: string, + notification: Notification, ) => void; fetchNotifications: (state: GitifyState) => Promise; markNotificationRead: ( state: GitifyState, - id: string, - hostname: string, + notification: Notification, ) => Promise; markNotificationDone: ( state: GitifyState, - id: string, - hostname: string, + notification: Notification, ) => Promise; unsubscribeNotification: ( state: GitifyState, - id: string, - hostname: string, + notification: Notification, ) => Promise; markRepoNotifications: ( state: GitifyState, - repoSlug: string, - hostname: string, + notification: Notification, ) => Promise; markRepoNotificationsDone: ( state: GitifyState, - repoSlug: string, - hostname: string, + notification: Notification, ) => Promise; status: Status; errorDetails: GitifyError; @@ -86,19 +80,20 @@ export const useNotifications = (): NotificationsState => { ); const markNotificationRead = useCallback( - async (state: GitifyState, id: string, hostname: string) => { + async (state: GitifyState, notification: Notification) => { setStatus('loading'); - const account = getAccountForHost(hostname, state.auth); - try { - await markNotificationThreadAsRead(id, hostname, account.token); + await markNotificationThreadAsRead( + notification.id, + notification.account.hostname, + notification.account.token, + ); const updatedNotifications = removeNotification( state.settings, - id, + notification, notifications, - hostname, ); setNotifications(updatedNotifications); @@ -112,19 +107,20 @@ export const useNotifications = (): NotificationsState => { ); const markNotificationDone = useCallback( - async (state: GitifyState, id: string, hostname: string) => { + async (state: GitifyState, notification: Notification) => { setStatus('loading'); - const account = getAccountForHost(hostname, state.auth); - try { - await markNotificationThreadAsDone(id, hostname, account.token); + await markNotificationThreadAsDone( + notification.id, + notification.account.hostname, + notification.account.token, + ); const updatedNotifications = removeNotification( state.settings, - id, + notification, notifications, - hostname, ); setNotifications(updatedNotifications); @@ -138,14 +134,16 @@ export const useNotifications = (): NotificationsState => { ); const unsubscribeNotification = useCallback( - async (state: GitifyState, id: string, hostname: string) => { + async (state: GitifyState, notification: Notification) => { setStatus('loading'); - const account = getAccountForHost(hostname, state.auth); - try { - await ignoreNotificationThreadSubscription(id, hostname, account.token); - await markNotificationRead(state, id, hostname); + await ignoreNotificationThreadSubscription( + notification.id, + notification.account.hostname, + notification.account.token, + ); + await markNotificationRead(state, notification); setStatus('success'); } catch (err) { setStatus('success'); @@ -155,16 +153,17 @@ export const useNotifications = (): NotificationsState => { ); const markRepoNotifications = useCallback( - async (state: GitifyState, repoSlug: string, hostname: string) => { + async (state: GitifyState, notification: Notification) => { setStatus('loading'); - const account = getAccountForHost(hostname, state.auth); + const repoSlug = notification.repository.full_name; + const hostname = notification.account.hostname; try { await markRepositoryNotificationsAsRead( repoSlug, hostname, - account.token, + notification.account.token, ); const updatedNotifications = removeNotifications( repoSlug, @@ -183,9 +182,13 @@ export const useNotifications = (): NotificationsState => { ); const markRepoNotificationsDone = useCallback( - async (state: GitifyState, repoSlug: string, hostname: string) => { + async (state: GitifyState, notification: Notification) => { setStatus('loading'); + const repoSlug = notification.repository.full_name; + const hostname = notification.account.hostname; + + // TODO Adam - FIX ME try { const accountIndex = notifications.findIndex( (accountNotifications) => @@ -201,11 +204,7 @@ export const useNotifications = (): NotificationsState => { await Promise.all( notificationsToRemove.map((notification) => - markNotificationDone( - state, - notification.id, - notifications[accountIndex].account.hostname, - ), + markNotificationDone(state, notification), ), ); } @@ -227,12 +226,11 @@ export const useNotifications = (): NotificationsState => { ); const removeNotificationFromState = useCallback( - (settings: SettingsState, id: string, hostname: string) => { + (settings: SettingsState, notification: Notification) => { const updatedNotifications = removeNotification( settings, - id, + notification, notifications, - hostname, ); setNotifications(updatedNotifications); diff --git a/src/routes/Accounts.tsx b/src/routes/Accounts.tsx index 69c597cc9..61dd9bebe 100644 --- a/src/routes/Accounts.tsx +++ b/src/routes/Accounts.tsx @@ -20,10 +20,6 @@ import { updateTrayIcon, updateTrayTitle, } from '../utils/comms'; -import { - isOAuthAppLoggedIn, - isPersonalAccessTokenLoggedIn, -} from '../utils/helpers'; export const AccountsRoute: FC = () => { const { auth, logoutFromAccount } = useContext(AppContext); @@ -152,7 +148,6 @@ export const AccountsRoute: FC = () => { className={buttonClass} title="Login with Personal Access Token" onClick={loginWithPersonalAccessToken} - hidden={isPersonalAccessTokenLoggedIn(auth)} > @@ -162,7 +157,6 @@ export const AccountsRoute: FC = () => { className={buttonClass} title="Login with OAuth App" onClick={loginWithOAuthApp} - hidden={isOAuthAppLoggedIn(auth)} > diff --git a/src/routes/Notifications.tsx b/src/routes/Notifications.tsx index 626d938d8..13e0dbd47 100644 --- a/src/routes/Notifications.tsx +++ b/src/routes/Notifications.tsx @@ -3,6 +3,7 @@ import { AccountNotifications } from '../components/AccountNotifications'; 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'; @@ -35,8 +36,8 @@ export const NotificationsRoute: FC = () => {
{notifications.map((accountNotifications) => ( { - describe('isPersonalAccessTokenLoggedIn', () => { - it('logged in', () => { - expect( - isPersonalAccessTokenLoggedIn({ - accounts: [mockPersonalAccessTokenAccount], - }), - ).toBe(true); - }); - - it('logged out', () => { - expect( - isPersonalAccessTokenLoggedIn({ - accounts: [], - }), - ).toBe(false); - - expect( - isPersonalAccessTokenLoggedIn({ - accounts: [mockOAuthAccount], - }), - ).toBe(false); - }); - }); - - describe('isOAuthAppLoggedIn', () => { - it('logged in', () => { - expect( - isOAuthAppLoggedIn({ - accounts: [mockOAuthAccount], - }), - ).toBe(true); - }); - - it('logged out', () => { - expect(isOAuthAppLoggedIn({ accounts: [] })).toBe(false); - - expect( - isOAuthAppLoggedIn({ accounts: [mockPersonalAccessTokenAccount] }), - ).toBe(false); - }); - }); - describe('getPlatformFromHostname', () => { it('should return GitHub Cloud', () => { expect(getPlatformFromHostname('github.com')).toBe('GitHub Cloud'); diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index a69d121bb..2fa7220d6 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -1,5 +1,4 @@ import { formatDistanceToNowStrict, parseISO } from 'date-fns'; -import type { Account, AuthState } from '../types'; import type { Notification } from '../typesGitHub'; import { openExternalLink } from '../utils/comms'; import { getHtmlUrl, getLatestDiscussion } from './api/client'; @@ -11,20 +10,6 @@ import { getWorkflowRunAttributes, } from './subject'; -export function isPersonalAccessTokenLoggedIn(auth: AuthState): boolean { - return auth.accounts.some( - (account) => account.method === 'Personal Access Token', - ); -} - -export function isOAuthAppLoggedIn(auth: AuthState): boolean { - return auth.accounts.some((account) => account.method === 'OAuth App'); -} - -export function getAccountForHost(hostname: string, auth: AuthState): Account { - return auth.accounts.find((account) => hostname.endsWith(account.hostname)); -} - export function getPlatformFromHostname(hostname: string): PlatformType { return hostname.endsWith(Constants.DEFAULT_AUTH_OPTIONS.hostname) ? 'GitHub Cloud' diff --git a/src/utils/remove-notification.test.ts b/src/utils/remove-notification.test.ts index d0075cdf5..ab0b02a0b 100644 --- a/src/utils/remove-notification.test.ts +++ b/src/utils/remove-notification.test.ts @@ -5,17 +5,13 @@ import Constants from './constants'; import { removeNotification } from './remove-notification'; describe('utils/remove-notification.ts', () => { - const notificationId = mockSingleNotification.id; - const hostname = mockSingleAccountNotifications[0].account.hostname; - it('should remove a notification if it exists', () => { expect(mockSingleAccountNotifications[0].notifications.length).toBe(1); const result = removeNotification( { ...mockSettings, delayNotificationState: false }, - notificationId, + mockSingleNotification, mockSingleAccountNotifications, - hostname, ); expect(result[0].notifications.length).toBe(0); @@ -30,9 +26,8 @@ describe('utils/remove-notification.ts', () => { const result = removeNotification( { ...mockSettings, delayNotificationState: true }, - notificationId, + mockSingleNotification, mockSingleAccountNotifications, - hostname, ); expect(result[0].notifications.length).toBe(1); diff --git a/src/utils/remove-notification.ts b/src/utils/remove-notification.ts index 03e0fcd15..2e455dd8a 100644 --- a/src/utils/remove-notification.ts +++ b/src/utils/remove-notification.ts @@ -1,21 +1,22 @@ import type { AccountNotifications, SettingsState } from '../types'; +import type { Notification } from '../typesGitHub'; import Constants from './constants'; export const removeNotification = ( settings: SettingsState, - id: string, + notification: Notification, notifications: AccountNotifications[], - hostname: string, ): AccountNotifications[] => { if (settings.delayNotificationState) { - const notificationRow = document.getElementById(id); + const notificationRow = document.getElementById(notification.id); notificationRow.className += ` ${Constants.READ_CLASS_NAME}`; return notifications; } + // TODO Adam - FIX ME const accountIndex = notifications.findIndex( (accountNotifications) => - accountNotifications.account.hostname === hostname, + accountNotifications.account.hostname === notification.account.hostname, ); if (accountIndex !== -1) { @@ -23,7 +24,7 @@ export const removeNotification = ( updatedNotifications[accountIndex] = { ...updatedNotifications[accountIndex], notifications: updatedNotifications[accountIndex].notifications.filter( - (notification) => notification.id !== id, + (notif) => notif.id !== notification.id, ), }; return updatedNotifications; From 4be1daee9392de94b55e46f9b81998021c056fe2 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 8 Jun 2024 06:35:51 -0400 Subject: [PATCH 2/9] feat: update header --- src/components/AccountNotifications.tsx | 19 +- .../AccountNotifications.test.tsx.snap | 214 ++++++++++++++---- src/routes/Accounts.test.tsx | 128 ++++------- src/routes/Accounts.tsx | 7 +- .../__snapshots__/Accounts.test.tsx.snap | 2 - src/utils/helpers.ts | 7 + 6 files changed, 231 insertions(+), 146 deletions(-) diff --git a/src/components/AccountNotifications.tsx b/src/components/AccountNotifications.tsx index 8dabb331a..df3be711f 100644 --- a/src/components/AccountNotifications.tsx +++ b/src/components/AccountNotifications.tsx @@ -1,7 +1,9 @@ import { ChevronDownIcon, ChevronLeftIcon } from '@primer/octicons-react'; import type { Account } from '../types'; import type { Notification } from '../typesGitHub'; +import { openProfile } from '../utils/helpers'; import { RepositoryNotifications } from './Repository'; +import { PlatformIcon } from './icons/PlatformIcon'; interface IProps { account: Account; @@ -29,9 +31,20 @@ export const AccountNotifications = (props: IProps) => { return ( <> {showAccountHostname && ( -
- {account.hostname} - {account.user.login} - +
+
+ + +
+
+ +
)} diff --git a/src/components/__snapshots__/AccountNotifications.test.tsx.snap b/src/components/__snapshots__/AccountNotifications.test.tsx.snap index de9b86718..68e6e303e 100644 --- a/src/components/__snapshots__/AccountNotifications.test.tsx.snap +++ b/src/components/__snapshots__/AccountNotifications.test.tsx.snap @@ -6,9 +6,91 @@ exports[`components/AccountNotifications.tsx should render itself (github.com wi "baseElement":
- github.com +
+ + + + +
+
+ +
+
+
+ Repository +
+
+ , + "container":
+
+
+ + + + +
+
-
- Repository -
-
- , - "container":
-
- github.com -
Repository @@ -113,9 +171,88 @@ exports[`components/AccountNotifications.tsx should render itself (github.com wi "baseElement":
- github.com +
+ + + + +
+
+ +
+
+
+ , + "container":
+
+
+ + + + +
+
- , - "container":
-
- github.com - -
, "debug": [Function], "findAllByAltText": [Function], diff --git a/src/routes/Accounts.test.tsx b/src/routes/Accounts.test.tsx index 1693605ad..94ae2ed98 100644 --- a/src/routes/Accounts.test.tsx +++ b/src/routes/Accounts.test.tsx @@ -178,101 +178,57 @@ describe('routes/Accounts.tsx', () => { }); describe('Add new accounts', () => { - describe('Login with Personal Access Token', () => { - it('should show login with personal access token button if not logged in', async () => { - await act(async () => { - render( - - - - - , - ); - }); - - expect( - screen.getByTitle('Login with Personal Access Token').hidden, - ).toBe(false); - - fireEvent.click(screen.getByTitle('Login with Personal Access Token')); - expect(mockNavigate).toHaveBeenNthCalledWith( - 1, - '/login-personal-access-token', - { - replace: true, - }, + it('should show login with personal access token', async () => { + await act(async () => { + render( + + + + + , ); }); - it('should hide login with personal access token button if already logged in', async () => { - await act(async () => { - render( - - - - - , - ); - }); + expect(screen.getByTitle('Login with Personal Access Token').hidden).toBe( + false, + ); - expect( - screen.getByTitle('Login with Personal Access Token').hidden, - ).toBe(true); - }); + fireEvent.click(screen.getByTitle('Login with Personal Access Token')); + expect(mockNavigate).toHaveBeenNthCalledWith( + 1, + '/login-personal-access-token', + { + replace: true, + }, + ); }); - describe('Login with OAuth App', () => { - it('should show login with oauth app if not logged in', async () => { - await act(async () => { - render( - - - - - , - ); - }); - - expect(screen.getByTitle('Login with OAuth App').hidden).toBe(false); - - fireEvent.click(screen.getByTitle('Login with OAuth App')); - expect(mockNavigate).toHaveBeenNthCalledWith(1, '/login-oauth-app', { - replace: true, - }); + it('should show login with oauth app', async () => { + await act(async () => { + render( + + + + + , + ); }); - it('should hide login with oauth app route if already logged in', async () => { - await act(async () => { - render( - - - - - , - ); - }); + expect(screen.getByTitle('Login with OAuth App').hidden).toBe(false); - expect(screen.getByTitle('Login with OAuth App').hidden).toBe(true); + fireEvent.click(screen.getByTitle('Login with OAuth App')); + expect(mockNavigate).toHaveBeenNthCalledWith(1, '/login-oauth-app', { + replace: true, }); }); }); diff --git a/src/routes/Accounts.tsx b/src/routes/Accounts.tsx index 61dd9bebe..7b288b66d 100644 --- a/src/routes/Accounts.tsx +++ b/src/routes/Accounts.tsx @@ -20,6 +20,7 @@ import { updateTrayIcon, updateTrayTitle, } from '../utils/comms'; +import { openProfile } from '../utils/helpers'; export const AccountsRoute: FC = () => { const { auth, logoutFromAccount } = useContext(AppContext); @@ -32,12 +33,6 @@ export const AccountsRoute: FC = () => { updateTrayTitle(); }, []); - const openProfile = (account: Account) => { - const url = new URL(`https://${account.hostname}`); - url.pathname = account.user.login; - openExternalLink(url.toString()); - }; - const openHost = (hostname: string) => { openExternalLink(`https://${hostname}`); }; diff --git a/src/routes/__snapshots__/Accounts.test.tsx.snap b/src/routes/__snapshots__/Accounts.test.tsx.snap index 352796a66..a13ecd202 100644 --- a/src/routes/__snapshots__/Accounts.test.tsx.snap +++ b/src/routes/__snapshots__/Accounts.test.tsx.snap @@ -351,7 +351,6 @@ exports[`routes/Accounts.tsx General should render itself & its children 1`] = `