From 6734e35127a7a2620e0b64aa7ef3ee64840ba8e0 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 9 Jun 2024 07:03:17 -0400 Subject: [PATCH 1/2] refactor: consolidate open links fns --- src/components/NotificationRow.test.tsx | 10 +- src/components/NotificationRow.tsx | 40 +++--- src/components/Repository.tsx | 11 +- src/components/Sidebar.tsx | 18 +-- .../NotificationRow.test.tsx.snap | 20 +-- src/routes/Accounts.tsx | 30 ++--- src/routes/Settings.tsx | 31 ++--- src/utils/auth/utils.ts | 2 +- src/utils/constants.ts | 2 + src/utils/helpers.ts | 7 -- src/utils/links.test.ts | 118 ++++++++++++++++++ src/utils/links.ts | 52 ++++++++ src/utils/notifications.test.ts | 8 +- src/utils/notifications.ts | 4 +- 14 files changed, 237 insertions(+), 116 deletions(-) create mode 100644 src/utils/links.test.ts create mode 100644 src/utils/links.ts diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx index 515386923..2d5f85044 100644 --- a/src/components/NotificationRow.test.tsx +++ b/src/components/NotificationRow.test.tsx @@ -4,12 +4,12 @@ import { mockAuth, mockSettings } from '../__mocks__/state-mocks'; import { AppContext } from '../context/App'; import type { Milestone, UserType } from '../typesGitHub'; import { mockSingleNotification } from '../utils/api/__mocks__/response-mocks'; -import * as helpers from '../utils/helpers'; +import * as links from '../utils/links'; import { NotificationRow } from './NotificationRow'; describe('components/NotificationRow.tsx', () => { beforeEach(() => { - jest.spyOn(helpers, 'openInBrowser'); + jest.spyOn(links, 'openNotification'); }); afterEach(() => { @@ -341,7 +341,7 @@ describe('components/NotificationRow.tsx', () => { ); fireEvent.click(screen.getByRole('main')); - expect(helpers.openInBrowser).toHaveBeenCalledTimes(1); + expect(links.openNotification).toHaveBeenCalledTimes(1); expect(removeNotificationFromState).toHaveBeenCalledTimes(1); }); @@ -366,7 +366,7 @@ describe('components/NotificationRow.tsx', () => { ); fireEvent.keyDown(screen.getByRole('main')); - expect(helpers.openInBrowser).toHaveBeenCalledTimes(1); + expect(links.openNotification).toHaveBeenCalledTimes(1); expect(removeNotificationFromState).toHaveBeenCalledTimes(1); }); @@ -391,7 +391,7 @@ describe('components/NotificationRow.tsx', () => { ); fireEvent.click(screen.getByRole('main')); - expect(helpers.openInBrowser).toHaveBeenCalledTimes(1); + expect(links.openNotification).toHaveBeenCalledTimes(1); expect(markNotificationDone).toHaveBeenCalledTimes(1); }); diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 76575e2fc..d7e6e3450 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -8,29 +8,22 @@ import { ReadIcon, TagIcon, } from '@primer/octicons-react'; -import { - type FC, - type KeyboardEvent, - type MouseEvent, - useCallback, - useContext, -} from 'react'; +import { type FC, type MouseEvent, useCallback, useContext } from 'react'; import { AppContext } from '../context/App'; import { IconColor } from '../types'; import type { Notification } from '../typesGitHub'; -import { openExternalLink } from '../utils/comms'; import Constants from '../utils/constants'; import { formatForDisplay, formatNotificationUpdatedAt, - openInBrowser, } from '../utils/helpers'; import { getNotificationTypeIcon, getNotificationTypeIconColor, getPullRequestReviewIcon, } from '../utils/icons'; +import { openNotification, openUserProfile } from '../utils/links'; import { formatReason } from '../utils/reason'; interface IProps { @@ -49,8 +42,8 @@ export const NotificationRow: FC = ({ notification, hostname }) => { notifications, } = useContext(AppContext); - const openNotification = useCallback(() => { - openInBrowser(notification); + const handleNotification = useCallback(() => { + openNotification(notification); if (settings.markAsDoneOnOpen) { markNotificationDone(notification.id, hostname); @@ -67,15 +60,6 @@ export const NotificationRow: FC = ({ notification, hostname }) => { unsubscribeNotification(notification.id, hostname); }; - const openUserProfile = ( - event: MouseEvent | KeyboardEvent, - ) => { - // Don't trigger onClick of parent element. - event.stopPropagation(); - - openExternalLink(notification.subject.user.html_url); - }; - const reason = formatReason(notification.reason); const NotificationIcon = getNotificationTypeIcon(notification.subject); const iconColor = getNotificationTypeIconColor(notification.subject); @@ -116,8 +100,8 @@ export const NotificationRow: FC = ({ notification, hostname }) => {
openNotification()} - onKeyDown={() => openNotification()} + onClick={() => handleNotification()} + onKeyDown={() => handleNotification()} >
= ({ notification, hostname }) => {
{notification.subject.user ? ( -
) => { + // Don't trigger onClick of parent element. + event.stopPropagation(); + openUserProfile(notification.subject.user); + }} className="flex-shrink-0" > = ({ notification, hostname }) => { title={notification.subject.user.login} alt={`${notification.subject.user.login}'s avatar`} /> -
+ ) : (
= ({ const { markRepoNotificationsRead, markRepoNotificationsDone } = useContext(AppContext); - const openBrowser = useCallback(() => { - const url = repoNotifications[0].repository.html_url; - openExternalLink(url); - }, [repoNotifications]); - const markRepoAsRead = useCallback(() => { const repoSlug = repoNotifications[0].repository.full_name; markRepoNotificationsRead(repoSlug, hostname); @@ -53,8 +48,8 @@ export const RepositoryNotifications: FC = ({ )} openRepository(repoNotifications[0].repository)} + onKeyDown={() => openRepository(repoNotifications[0].repository)} > {repoName} diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx index a6f10faa5..0025ffe8e 100644 --- a/src/components/Sidebar.tsx +++ b/src/components/Sidebar.tsx @@ -4,12 +4,12 @@ import { SyncIcon, XCircleIcon, } from '@primer/octicons-react'; -import { type FC, useCallback, useContext, useMemo } from 'react'; +import { type FC, useContext, useMemo } from 'react'; import { useLocation, useNavigate } from 'react-router-dom'; import { Logo } from '../components/Logo'; import { AppContext } from '../context/App'; -import { openExternalLink, quitApp } from '../utils/comms'; -import { Constants } from '../utils/constants'; +import { quitApp } from '../utils/comms'; +import { openGitHubNotifications, openGitifyRepository } from '../utils/links'; import { getNotificationCount } from '../utils/notifications'; export const Sidebar: FC = () => { @@ -19,14 +19,6 @@ export const Sidebar: FC = () => { const { notifications, fetchNotifications, isLoggedIn, status } = useContext(AppContext); - const onOpenBrowser = useCallback(() => { - openExternalLink(`https://github.com/${Constants.REPO_SLUG}`); - }, []); - - const onOpenGitHubNotifications = useCallback(() => { - openExternalLink('https://github.com/notifications'); - }, []); - const toggleSettings = () => { if (location.pathname.startsWith('/settings')) { navigate('/', { replace: true }); @@ -49,7 +41,7 @@ export const Sidebar: FC = () => { type="button" className="w-5 my-3 mx-auto cursor-pointer outline-none" title="Open Gitify on GitHub" - onClick={onOpenBrowser} + onClick={() => openGitifyRepository()} data-testid="gitify-logo" > @@ -60,7 +52,7 @@ export const Sidebar: FC = () => { className={`flex justify-around self-stretch items-center my-1 py-1 px-2 text-xs font-extrabold cursor-pointer ${ notificationsCount > 0 ? 'text-green-500' : 'text-white' }`} - onClick={onOpenGitHubNotifications} + onClick={() => openGitHubNotifications()} title={`${notificationsCount} Unread Notifications`} > -
gitify-app's avatar -
+
@@ -5038,9 +5039,10 @@ exports[`components/NotificationRow.tsx should render itself & its children 1`]
-
gitify-app's avatar -
+
@@ -5281,9 +5283,10 @@ exports[`components/NotificationRow.tsx should render itself & its children when
-
gitify-app's avatar -
+
@@ -5467,9 +5470,10 @@ exports[`components/NotificationRow.tsx should render itself & its children when
-
gitify-app's avatar -
+
diff --git a/src/routes/Accounts.tsx b/src/routes/Accounts.tsx index eb2ac487a..d582842de 100644 --- a/src/routes/Accounts.tsx +++ b/src/routes/Accounts.tsx @@ -14,16 +14,17 @@ import { AppContext } from '../context/App'; import { AuthMethodIcon } from '../components/icons/AuthMethodIcon'; import { PlatformIcon } from '../components/icons/PlatformIcon'; import type { Account } from '../types'; -import { getAccountUUID, getDeveloperSettingsURL } from '../utils/auth/utils'; -import { - openExternalLink, - updateTrayIcon, - updateTrayTitle, -} from '../utils/comms'; +import { getAccountUUID } from '../utils/auth/utils'; +import { updateTrayIcon, updateTrayTitle } from '../utils/comms'; import { isOAuthAppLoggedIn, isPersonalAccessTokenLoggedIn, } from '../utils/helpers'; +import { + openAccountProfile, + openDeveloperSettings, + openHost, +} from '../utils/links'; export const AccountsRoute: FC = () => { const { auth, logoutFromAccount } = useContext(AppContext); @@ -36,21 +37,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}`); - }; - - const openDeveloperSettings = (account: Account) => { - const url = getDeveloperSettingsURL(account); - openExternalLink(url); - }; - const loginWithPersonalAccessToken = useCallback(() => { return navigate('/login-personal-access-token', { replace: true }); }, []); @@ -97,7 +83,7 @@ export const AccountsRoute: FC = () => { type="button" className="cursor-pointer font-semibold mb-1 text-sm" title="Open Profile" - onClick={() => openProfile(account)} + onClick={() => openAccountProfile(account)} > @{account.user.login} { const [appVersion, setAppVersion] = useState(null); - const openGitHubReleaseNotes = useCallback((version) => { - openExternalLink( - `https://github.com/${Constants.REPO_SLUG}/releases/tag/v${version}`, - ); - }, []); - - const openGitHubParticipatingDocs = (event: MouseEvent) => { - // Don't trigger onClick of parent element. - event.stopPropagation(); - - openExternalLink( - 'https://docs.github.com/en/account-and-profile/managing-subscriptions-and-notifications-on-github/setting-up-notifications/configuring-notifications#about-participating-and-watching-notifications', - ); - }; - useEffect(() => { (async () => { const result = await getAppVersion(); @@ -188,7 +175,11 @@ export const SettingsRoute: FC = () => { type="button" className="text-blue-500 mx-1" title="Open GitHub documentation for participating and watching notifications" - onClick={openGitHubParticipatingDocs} + onClick={(event: MouseEvent) => { + // Don't trigger onClick of parent element. + event.stopPropagation(); + openGitHubParticipatingDocs(); + }} > official docs @@ -292,7 +283,7 @@ export const SettingsRoute: FC = () => { type="button" className="font-semibold cursor-pointer" title="View release notes" - onClick={() => openGitHubReleaseNotes(appVersion)} + onClick={() => openGitifyReleaseNotes(appVersion)} > Gitify v{appVersion} diff --git a/src/utils/auth/utils.ts b/src/utils/auth/utils.ts index f4ab7fe4a..a6c3f2122 100644 --- a/src/utils/auth/utils.ts +++ b/src/utils/auth/utils.ts @@ -149,7 +149,7 @@ export function getDeveloperSettingsURL(account: Account): string { settingsURL.pathname = '/settings/developers'; break; case 'Personal Access Token': - settingsURL.pathname = 'settings/tokens'; + settingsURL.pathname = '/settings/tokens'; break; } return settingsURL.toString(); diff --git a/src/utils/constants.ts b/src/utils/constants.ts index a3d607f94..f4af91f77 100644 --- a/src/utils/constants.ts +++ b/src/utils/constants.ts @@ -33,6 +33,8 @@ export const Constants = { 'https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authenticating-to-the-rest-api-with-an-oauth-app', PAT_URL: 'https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens', + PARTICIPATING_URL: + 'https://docs.github.com/en/account-and-profile/managing-subscriptions-and-notifications-on-github/setting-up-notifications/configuring-notifications#about-participating-and-watching-notifications', }, }; diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index a69d121bb..51c054c48 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -1,7 +1,6 @@ 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'; import type { PlatformType } from './auth/types'; import { Constants } from './constants'; @@ -183,9 +182,3 @@ export function formatNotificationUpdatedAt( return ''; } - -export async function openInBrowser(notification: Notification) { - const url = await generateGitHubWebUrl(notification); - - openExternalLink(url); -} diff --git a/src/utils/links.test.ts b/src/utils/links.test.ts new file mode 100644 index 000000000..04dc88063 --- /dev/null +++ b/src/utils/links.test.ts @@ -0,0 +1,118 @@ +import { partialMockUser } from '../__mocks__/partial-mocks'; +import { mockGitHubCloudAccount } from '../__mocks__/state-mocks'; +import type { Repository } from '../typesGitHub'; +import { mockSingleNotification } from './api/__mocks__/response-mocks'; +import * as authUtils from './auth/utils'; +import * as comms from './comms'; +import Constants from './constants'; +import * as helpers from './helpers'; +import { + openAccountProfile, + openDeveloperSettings, + openGitHubNotifications, + openGitHubParticipatingDocs, + openGitifyReleaseNotes, + openGitifyRepository, + openHost, + openNotification, + openRepository, + openUserProfile, +} from './links'; + +describe('utils/links.ts', () => { + beforeEach(() => { + jest.spyOn(comms, 'openExternalLink'); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('openGitifyRepository', () => { + openGitifyRepository(); + expect(comms.openExternalLink).toHaveBeenCalledWith( + 'https://github.com/gitify-app/gitify', + ); + }); + + it('openGitifyReleaseNotes', () => { + openGitifyReleaseNotes('1.0.0'); + expect(comms.openExternalLink).toHaveBeenCalledWith( + 'https://github.com/gitify-app/gitify/releases/tag/v1.0.0', + ); + }); + + it('openGitHubNotifications', () => { + openGitHubNotifications(); + expect(comms.openExternalLink).toHaveBeenCalledWith( + 'https://github.com/notifications', + ); + }); + + it('openAccountProfile', () => { + openAccountProfile(mockGitHubCloudAccount); + expect(comms.openExternalLink).toHaveBeenCalledWith( + 'https://github.com/octocat', + ); + }); + + it('openUserProfile', () => { + const mockUser = partialMockUser('mock-user'); + openUserProfile(mockUser); + expect(comms.openExternalLink).toHaveBeenCalledWith( + 'https://github.com/mock-user', + ); + }); + + it('openHost', () => { + openHost('github.com'); + expect(comms.openExternalLink).toHaveBeenCalledWith('https://github.com'); + }); + + it('openDeveloperSettings', () => { + const mockSettingsURL = 'https://github.com/settings/tokens'; + + jest + .spyOn(authUtils, 'getDeveloperSettingsURL') + .mockReturnValue(mockSettingsURL); + openDeveloperSettings(mockGitHubCloudAccount); + expect(comms.openExternalLink).toHaveBeenCalledWith(mockSettingsURL); + }); + + it('openRepository', () => { + const mockHtmlUrl = 'https://github.com/gitify-app/gitify'; + + const repo = { + html_url: mockHtmlUrl, + } as Repository; + + openRepository(repo); + expect(comms.openExternalLink).toHaveBeenCalledWith(mockHtmlUrl); + }); + + it('openNotification', async () => { + const mockNotificationUrl = mockSingleNotification.repository.html_url; + jest + .spyOn(helpers, 'generateGitHubWebUrl') + .mockResolvedValue(mockNotificationUrl); + await openNotification(mockSingleNotification); + expect(comms.openExternalLink).toHaveBeenCalledWith(mockNotificationUrl); + }); + + it('openParticipatingDocs', () => { + openGitHubParticipatingDocs(); + expect(comms.openExternalLink).toHaveBeenCalledWith( + Constants.GITHUB_DOCS.PARTICIPATING_URL, + ); + }); +}); + +// export function openDeveloperSettings(account: Account) { +// const url = getDeveloperSettingsURL(account); +// openExternalLink(url); +// } + +// export async function openNotification(notification: Notification) { +// const url = await generateGitHubWebUrl(notification); +// openExternalLink(url); +// } diff --git a/src/utils/links.ts b/src/utils/links.ts new file mode 100644 index 000000000..69cf8f8f2 --- /dev/null +++ b/src/utils/links.ts @@ -0,0 +1,52 @@ +import type { Account } from '../types'; +import type { Notification, Repository, SubjectUser } from '../typesGitHub'; +import { getDeveloperSettingsURL } from './auth/utils'; +import { openExternalLink } from './comms'; +import Constants from './constants'; +import { generateGitHubWebUrl } from './helpers'; + +export function openGitifyRepository() { + openExternalLink(`https://github.com/${Constants.REPO_SLUG}`); +} + +export function openGitifyReleaseNotes(version: string) { + openExternalLink( + `https://github.com/${Constants.REPO_SLUG}/releases/tag/v${version}`, + ); +} + +export function openGitHubNotifications() { + openExternalLink('https://github.com/notifications'); +} + +export function openAccountProfile(account: Account) { + const url = new URL(`https://${account.hostname}`); + url.pathname = account.user.login; + openExternalLink(url.toString()); +} + +export function openUserProfile(user: SubjectUser) { + openExternalLink(user.html_url); +} + +export function openHost(hostname: string) { + openExternalLink(`https://${hostname}`); +} + +export function openDeveloperSettings(account: Account) { + const url = getDeveloperSettingsURL(account); + openExternalLink(url); +} + +export function openRepository(repository: Repository) { + openExternalLink(repository.html_url); +} + +export async function openNotification(notification: Notification) { + const url = await generateGitHubWebUrl(notification); + openExternalLink(url); +} + +export function openGitHubParticipatingDocs() { + openExternalLink(Constants.GITHUB_DOCS.PARTICIPATING_URL); +} diff --git a/src/utils/notifications.test.ts b/src/utils/notifications.test.ts index 79a06bff9..dbfc0f91c 100644 --- a/src/utils/notifications.test.ts +++ b/src/utils/notifications.test.ts @@ -8,7 +8,7 @@ import { defaultSettings } from '../context/App'; import type { SettingsState } from '../types'; import { mockGitHubNotifications } from './api/__mocks__/response-mocks'; import * as comms from './comms'; -import * as helpers from './helpers'; +import * as links from './links'; import * as notificationsHelpers from './notifications'; import { filterNotifications } from './notifications'; @@ -112,7 +112,7 @@ describe('utils/notifications.ts', () => { it('should click on a native notification (with 1 notification)', () => { const hideWindowMock = jest.spyOn(comms, 'hideWindow'); - jest.spyOn(helpers, 'openInBrowser'); + jest.spyOn(links, 'openNotification'); const nativeNotification: Notification = notificationsHelpers.raiseNativeNotification( @@ -121,8 +121,8 @@ describe('utils/notifications.ts', () => { ); nativeNotification.onclick(null); - expect(helpers.openInBrowser).toHaveBeenCalledTimes(1); - expect(helpers.openInBrowser).toHaveBeenLastCalledWith( + expect(links.openNotification).toHaveBeenCalledTimes(1); + expect(links.openNotification).toHaveBeenLastCalledWith( mockGitHubNotifications[0], ); expect(hideWindowMock).toHaveBeenCalledTimes(1); diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 1c7ac7426..850eb34b4 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -5,9 +5,9 @@ import type { SettingsState, } from '../types'; import { Notification } from '../typesGitHub'; -import { openInBrowser } from '../utils/helpers'; import { listNotificationsForAuthenticatedUser } from './api/client'; import { hideWindow, showWindow, updateTrayIcon } from './comms'; +import { openNotification } from './links'; import { isWindows } from './platform'; import { getGitifySubjectDetails } from './subject'; @@ -95,7 +95,7 @@ export const raiseNativeNotification = ( nativeNotification.onclick = () => { if (notifications.length === 1) { hideWindow(); - openInBrowser(notifications[0]); + openNotification(notifications[0]); } else { showWindow(); } From f6ccf2a62d5f25bf99fee8866521367396f83fe3 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 10 Jun 2024 12:58:28 -0400 Subject: [PATCH 2/2] Update src/utils/links.test.ts Co-authored-by: Afonso Jorge Ramos --- src/utils/links.test.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/utils/links.test.ts b/src/utils/links.test.ts index 04dc88063..1a5279b8b 100644 --- a/src/utils/links.test.ts +++ b/src/utils/links.test.ts @@ -106,13 +106,3 @@ describe('utils/links.ts', () => { ); }); }); - -// export function openDeveloperSettings(account: Account) { -// const url = getDeveloperSettingsURL(account); -// openExternalLink(url); -// } - -// export async function openNotification(notification: Notification) { -// const url = await generateGitHubWebUrl(notification); -// openExternalLink(url); -// }