From d4cca243fe9168ea630a5fb26d5bda6a65b0f5d2 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 21 Apr 2024 16:37:30 -0400 Subject: [PATCH 1/5] reafactor: api client --- src/context/App.tsx | 11 ++++--- src/hooks/useNotifications.ts | 54 ++++++++++++++++------------------- src/routes/Settings.tsx | 8 ++++-- src/utils/api/client.ts | 25 ++++++++++++++++ src/utils/auth.ts | 7 +++-- src/utils/helpers.test.ts | 10 +++---- src/utils/helpers.ts | 6 ++-- 7 files changed, 74 insertions(+), 47 deletions(-) create mode 100644 src/utils/api/client.ts diff --git a/src/context/App.tsx b/src/context/App.tsx index 3907fb975..bf8d4a73d 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -22,7 +22,7 @@ import { apiRequestAuth } from '../utils/api-requests'; import { addAccount, authGitHub, getToken, getUserData } from '../utils/auth'; import { setAutoLaunch, updateTrayTitle } from '../utils/comms'; import Constants from '../utils/constants'; -import { generateGitHubAPIUrl } from '../utils/helpers'; +import { getGitHubAPIBaseUrl } from '../utils/helpers'; import { getNotificationCount } from '../utils/notifications'; import { clearState, loadState, saveState } from '../utils/storage'; import { setTheme } from '../utils/theme'; @@ -166,11 +166,10 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { const validateToken = useCallback( async ({ token, hostname }: AuthTokenOptions) => { - await apiRequestAuth( - `${generateGitHubAPIUrl(hostname)}notifications`, - 'HEAD', - token, - ); + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/notifications`); + await apiRequestAuth(url.toString(), 'HEAD', token); + const user = await getUserData(token, hostname); const updatedAccounts = addAccount(accounts, token, hostname, user); setAccounts(updatedAccounts); diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index 112b6db13..a27a5f1cd 100644 --- a/src/hooks/useNotifications.ts +++ b/src/hooks/useNotifications.ts @@ -12,8 +12,8 @@ import { apiRequestAuth } from '../utils/api-requests'; import { determineFailureType } from '../utils/api/errors'; import Constants from '../utils/constants'; import { - generateGitHubAPIUrl, getEnterpriseAccountToken, + getGitHubAPIBaseUrl, getTokenForHost, isEnterpriseHost, isGitHubLoggedIn, @@ -75,9 +75,16 @@ export const useNotifications = (): NotificationsState => { const fetchNotifications = useCallback( async (accounts: AuthState, settings: SettingsState) => { function getNotifications(hostname: string, token: string): AxiosPromise { - const endpointSuffix = `notifications?participating=${settings.participating}`; - const url = `${generateGitHubAPIUrl(hostname)}${endpointSuffix}`; - return apiRequestAuth(url, 'GET', token); + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/notifications`); + url.searchParams.append( + 'participating', + String(settings.participating), + ); + + console.log('url', url.toString()); + + return apiRequestAuth(url.toString(), 'GET', token); } function getGitHubNotifications() { @@ -217,12 +224,9 @@ export const useNotifications = (): NotificationsState => { : accounts.token; try { - await apiRequestAuth( - `${generateGitHubAPIUrl(hostname)}notifications/threads/${id}`, - 'PATCH', - token, - {}, - ); + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/notifications/threads/${id}`); + await apiRequestAuth(url.toString(), 'PATCH', token, {}); const updatedNotifications = removeNotification( id, @@ -250,12 +254,9 @@ export const useNotifications = (): NotificationsState => { : accounts.token; try { - await apiRequestAuth( - `${generateGitHubAPIUrl(hostname)}notifications/threads/${id}`, - 'DELETE', - token, - {}, - ); + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/notifications/threads/${id}`); + await apiRequestAuth(url.toString(), 'DELETE', token, {}); const updatedNotifications = removeNotification( id, @@ -283,14 +284,12 @@ export const useNotifications = (): NotificationsState => { : accounts.token; try { - await apiRequestAuth( - `${generateGitHubAPIUrl( - hostname, - )}notifications/threads/${id}/subscription`, - 'PUT', - token, - { ignored: true }, + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL( + `${baseUrl}/notifications/threads/${id}/subscriptions`, ); + await apiRequestAuth(url.toString(), 'PUT', token, { ignored: true }); + await markNotificationRead(accounts, id, hostname); } catch (err) { setIsFetching(false); @@ -309,12 +308,9 @@ export const useNotifications = (): NotificationsState => { : accounts.token; try { - await apiRequestAuth( - `${generateGitHubAPIUrl(hostname)}repos/${repoSlug}/notifications`, - 'PUT', - token, - {}, - ); + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/repos/${repoSlug}/notifications`); + await apiRequestAuth(url.toString(), 'PUT', token, {}); const updatedNotifications = removeNotifications( repoSlug, diff --git a/src/routes/Settings.tsx b/src/routes/Settings.tsx index 77c0e00e4..bfaa305e8 100644 --- a/src/routes/Settings.tsx +++ b/src/routes/Settings.tsx @@ -28,7 +28,7 @@ import { updateTrayTitle, } from '../utils/comms'; import Constants from '../utils/constants'; -import { generateGitHubAPIUrl } from '../utils/helpers'; +import { getGitHubAPIBaseUrl } from '../utils/helpers'; import { setTheme } from '../utils/theme'; export const SettingsRoute: FC = () => { @@ -74,8 +74,12 @@ export const SettingsRoute: FC = () => { useMemo(() => { (async () => { + const baseUrl = getGitHubAPIBaseUrl( + Constants.DEFAULT_AUTH_OPTIONS.hostname, + ); + const url = new URL(baseUrl); const response = await apiRequestAuth( - `${generateGitHubAPIUrl(Constants.DEFAULT_AUTH_OPTIONS.hostname)}`, + url.toString(), 'GET', accounts.token, ); diff --git a/src/utils/api/client.ts b/src/utils/api/client.ts new file mode 100644 index 000000000..40a347537 --- /dev/null +++ b/src/utils/api/client.ts @@ -0,0 +1,25 @@ +import axios, { type AxiosPromise, type Method } from 'axios'; + +function apiRequest( + url: string, + method: Method, + data = {}, +): AxiosPromise | null { + axios.defaults.headers.common.Accept = 'application/json'; + axios.defaults.headers.common['Content-Type'] = 'application/json'; + axios.defaults.headers.common['Cache-Control'] = 'no-cache'; + return axios({ method, url, data }); +} + +function apiRequestAuth( + url: string, + method: Method, + token: string, + data = {}, +): AxiosPromise | null { + axios.defaults.headers.common.Accept = 'application/json'; + axios.defaults.headers.common.Authorization = `token ${token}`; + axios.defaults.headers.common['Cache-Control'] = 'no-cache'; + axios.defaults.headers.common['Content-Type'] = 'application/json'; + return axios({ method, url, data }); +} diff --git a/src/utils/auth.ts b/src/utils/auth.ts index 68fd9e5b1..d69545258 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -9,7 +9,7 @@ import type { import type { UserDetails } from '../typesGithub'; import { apiRequest, apiRequestAuth } from '../utils/api-requests'; import { Constants } from '../utils/constants'; -import { generateGitHubAPIUrl, isEnterpriseHost } from './helpers'; +import { getGitHubAPIBaseUrl, isEnterpriseHost } from './helpers'; export const authGitHub = ( authOptions = Constants.DEFAULT_AUTH_OPTIONS, @@ -82,8 +82,11 @@ export const getUserData = async ( token: string, hostname: string, ): Promise => { + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/user`); + const response: UserDetails = ( - await apiRequestAuth(`${generateGitHubAPIUrl(hostname)}user`, 'GET', token) + await apiRequestAuth(url.toString(), 'GET', token) ).data; return { diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts index b4019e3ff..59f75cbb2 100644 --- a/src/utils/helpers.test.ts +++ b/src/utils/helpers.test.ts @@ -12,9 +12,9 @@ import { addNotificationReferrerIdToUrl, formatForDisplay, formatSearchQueryString, - generateGitHubAPIUrl, generateGitHubWebUrl, generateNotificationReferrerId, + getGitHubAPIBaseUrl, getHtmlUrl, isEnterpriseHost, isGitHubLoggedIn, @@ -93,13 +93,13 @@ describe('utils/helpers.ts', () => { describe('generateGitHubAPIUrl', () => { it('should generate a GitHub API url - non enterprise', () => { - const result = generateGitHubAPIUrl('github.com'); - expect(result).toBe('https://api.github.com/'); + const result = getGitHubAPIBaseUrl('github.com'); + expect(result).toBe('https://api.github.com'); }); it('should generate a GitHub API url - enterprise', () => { - const result = generateGitHubAPIUrl('github.manos.im'); - expect(result).toBe('https://github.manos.im/api/v3/'); + const result = getGitHubAPIBaseUrl('github.manos.im'); + expect(result).toBe('https://github.manos.im/api/v3'); }); }); diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index 814023fa1..5f534cb01 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -37,11 +37,11 @@ export function isEnterpriseHost(hostname: string): boolean { return !hostname.endsWith(Constants.DEFAULT_AUTH_OPTIONS.hostname); } -export function generateGitHubAPIUrl(hostname) { +export function getGitHubAPIBaseUrl(hostname) { const isEnterprise = isEnterpriseHost(hostname); return isEnterprise - ? `https://${hostname}/api/v3/` - : `https://api.${hostname}/`; + ? `https://${hostname}/api/v3` + : `https://api.${hostname}`; } export function addNotificationReferrerIdToUrl( From d1b9b158a5a5a22f5964d6fd1749fda871054467 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 22 Apr 2024 05:55:13 -0400 Subject: [PATCH 2/5] reafactor: api client --- src/context/App.test.tsx | 2 +- src/context/App.tsx | 7 +- src/hooks/useNotifications.ts | 55 ++++----- src/routes/Settings.test.tsx | 2 +- src/routes/Settings.tsx | 10 +- src/typesGithub.ts | 44 +++++++ .../api/__snapshots__/request.test.ts.snap | 51 ++++++++ src/utils/api/client.ts | 113 ++++++++++++++---- .../request.test.ts} | 4 +- src/utils/{api-requests.ts => api/request.ts} | 0 src/utils/auth.test.ts | 2 +- src/utils/auth.ts | 13 +- src/utils/helpers.test.ts | 2 +- src/utils/helpers.ts | 2 +- src/utils/subject.ts | 2 +- 15 files changed, 222 insertions(+), 87 deletions(-) create mode 100644 src/utils/api/__snapshots__/request.test.ts.snap rename src/utils/{api-requests.test.ts => api/request.test.ts} (93%) rename src/utils/{api-requests.ts => api/request.ts} (100%) diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx index 3280d5773..6e004344c 100644 --- a/src/context/App.test.tsx +++ b/src/context/App.test.tsx @@ -4,7 +4,7 @@ import { useContext } from 'react'; import { mockAccounts, mockSettings } from '../__mocks__/mock-state'; import { useNotifications } from '../hooks/useNotifications'; import type { AuthState, SettingsState } from '../types'; -import * as apiRequests from '../utils/api-requests'; +import * as apiRequests from '../utils/api/request'; import * as comms from '../utils/comms'; import Constants from '../utils/constants'; import * as notifications from '../utils/notifications'; diff --git a/src/context/App.tsx b/src/context/App.tsx index bf8d4a73d..3f4f1464c 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -18,11 +18,10 @@ import { type SettingsState, Theme, } from '../types'; -import { apiRequestAuth } from '../utils/api-requests'; +import { headNotifications } from '../utils/api/client'; import { addAccount, authGitHub, getToken, getUserData } from '../utils/auth'; import { setAutoLaunch, updateTrayTitle } from '../utils/comms'; import Constants from '../utils/constants'; -import { getGitHubAPIBaseUrl } from '../utils/helpers'; import { getNotificationCount } from '../utils/notifications'; import { clearState, loadState, saveState } from '../utils/storage'; import { setTheme } from '../utils/theme'; @@ -166,9 +165,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { const validateToken = useCallback( async ({ token, hostname }: AuthTokenOptions) => { - const baseUrl = getGitHubAPIBaseUrl(hostname); - const url = new URL(`${baseUrl}/notifications`); - await apiRequestAuth(url.toString(), 'HEAD', token); + await headNotifications(hostname, token); const user = await getUserData(token, hostname); const updatedAccounts = addAccount(accounts, token, hostname, user); diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index a27a5f1cd..e5c71ac82 100644 --- a/src/hooks/useNotifications.ts +++ b/src/hooks/useNotifications.ts @@ -1,4 +1,4 @@ -import axios, { type AxiosError, type AxiosPromise } from 'axios'; +import axios, { type AxiosError } from 'axios'; import { useCallback, useState } from 'react'; import type { @@ -8,12 +8,18 @@ import type { SettingsState, } from '../types'; import type { GithubRESTError, Notification } from '../typesGithub'; -import { apiRequestAuth } from '../utils/api-requests'; +import { + ignoreNotificationThreadSubscription, + listNotificationsForAuthenticatedUser, + markNotificationThreadAsDone, + markRepositoryNotificationsAsRead, + markThreadAsRead, +} from '../utils/api/client'; import { determineFailureType } from '../utils/api/errors'; +import { apiRequestAuth } from '../utils/api/request'; import Constants from '../utils/constants'; import { getEnterpriseAccountToken, - getGitHubAPIBaseUrl, getTokenForHost, isEnterpriseHost, isGitHubLoggedIn, @@ -74,32 +80,25 @@ export const useNotifications = (): NotificationsState => { const fetchNotifications = useCallback( async (accounts: AuthState, settings: SettingsState) => { - function getNotifications(hostname: string, token: string): AxiosPromise { - const baseUrl = getGitHubAPIBaseUrl(hostname); - const url = new URL(`${baseUrl}/notifications`); - url.searchParams.append( - 'participating', - String(settings.participating), - ); - - console.log('url', url.toString()); - - return apiRequestAuth(url.toString(), 'GET', token); - } - function getGitHubNotifications() { if (!isGitHubLoggedIn(accounts)) { return; } - return getNotifications( + + return listNotificationsForAuthenticatedUser( Constants.DEFAULT_AUTH_OPTIONS.hostname, accounts.token, + settings, ); } function getEnterpriseNotifications() { return accounts.enterpriseAccounts.map((account) => { - return getNotifications(account.hostname, account.token); + return listNotificationsForAuthenticatedUser( + account.hostname, + account.token, + settings, + ); }); } @@ -224,9 +223,7 @@ export const useNotifications = (): NotificationsState => { : accounts.token; try { - const baseUrl = getGitHubAPIBaseUrl(hostname); - const url = new URL(`${baseUrl}/notifications/threads/${id}`); - await apiRequestAuth(url.toString(), 'PATCH', token, {}); + await markThreadAsRead(id, hostname, token); const updatedNotifications = removeNotification( id, @@ -254,9 +251,7 @@ export const useNotifications = (): NotificationsState => { : accounts.token; try { - const baseUrl = getGitHubAPIBaseUrl(hostname); - const url = new URL(`${baseUrl}/notifications/threads/${id}`); - await apiRequestAuth(url.toString(), 'DELETE', token, {}); + await markNotificationThreadAsDone(id, hostname, token); const updatedNotifications = removeNotification( id, @@ -284,12 +279,7 @@ export const useNotifications = (): NotificationsState => { : accounts.token; try { - const baseUrl = getGitHubAPIBaseUrl(hostname); - const url = new URL( - `${baseUrl}/notifications/threads/${id}/subscriptions`, - ); - await apiRequestAuth(url.toString(), 'PUT', token, { ignored: true }); - + await ignoreNotificationThreadSubscription(id, hostname, token); await markNotificationRead(accounts, id, hostname); } catch (err) { setIsFetching(false); @@ -308,10 +298,7 @@ export const useNotifications = (): NotificationsState => { : accounts.token; try { - const baseUrl = getGitHubAPIBaseUrl(hostname); - const url = new URL(`${baseUrl}/repos/${repoSlug}/notifications`); - await apiRequestAuth(url.toString(), 'PUT', token, {}); - + await markRepositoryNotificationsAsRead(repoSlug, hostname, token); const updatedNotifications = removeNotifications( repoSlug, notifications, diff --git a/src/routes/Settings.test.tsx b/src/routes/Settings.test.tsx index f19b2955b..94b8e5e2f 100644 --- a/src/routes/Settings.test.tsx +++ b/src/routes/Settings.test.tsx @@ -8,7 +8,7 @@ import type { AxiosResponse } from 'axios'; import { shell } from 'electron'; import { mockAccounts, mockSettings } from '../__mocks__/mock-state'; import { AppContext } from '../context/App'; -import * as apiRequests from '../utils/api-requests'; +import * as apiRequests from '../utils/api/request'; import Constants from '../utils/constants'; import { SettingsRoute } from './Settings'; diff --git a/src/routes/Settings.tsx b/src/routes/Settings.tsx index bfaa305e8..ef87f8d0c 100644 --- a/src/routes/Settings.tsx +++ b/src/routes/Settings.tsx @@ -21,14 +21,13 @@ import { Checkbox } from '../components/fields/Checkbox'; import { RadioGroup } from '../components/fields/RadioGroup'; import { AppContext } from '../context/App'; import { Theme } from '../types'; -import { apiRequestAuth } from '../utils/api-requests'; +import { getRootHypermediaLinks } from '../utils/api/client'; import { openExternalLink, updateTrayIcon, updateTrayTitle, } from '../utils/comms'; import Constants from '../utils/constants'; -import { getGitHubAPIBaseUrl } from '../utils/helpers'; import { setTheme } from '../utils/theme'; export const SettingsRoute: FC = () => { @@ -74,13 +73,8 @@ export const SettingsRoute: FC = () => { useMemo(() => { (async () => { - const baseUrl = getGitHubAPIBaseUrl( + const response = await getRootHypermediaLinks( Constants.DEFAULT_AUTH_OPTIONS.hostname, - ); - const url = new URL(baseUrl); - const response = await apiRequestAuth( - url.toString(), - 'GET', accounts.token, ); diff --git a/src/typesGithub.ts b/src/typesGithub.ts index 0a462d6af..60d832aa2 100644 --- a/src/typesGithub.ts +++ b/src/typesGithub.ts @@ -444,3 +444,47 @@ export interface GithubRESTError { message: string; documentation_url: string; } + +export interface NotificationThreadSubscription { + subscribed: boolean; + ignored: boolean; + reason: string | null; + created_at: string; + url: string; + thread_url: string; +} + +export interface RootHypermediaLinks { + current_user_url: string; + current_user_authorizations_html_url: string; + authorizations_url: string; + code_search_url: string; + commit_search_url: string; + emails_url: string; + emojis_url: string; + events_url: string; + feeds_url: string; + followers_url: string; + following_url: string; + gists_url: string; + hub_url: string; + issue_search_url: string; + issues_url: string; + keys_url: string; + notifications_url: string; + organization_url: string; + organization_repositories_url: string; + organization_teams_url: string; + public_gists_url: string; + rate_limit_url: string; + repository_url: string; + repository_search_url: string; + current_user_repositories_url: string; + starred_url: string; + starred_gists_url: string; + topic_search_url: string; + user_url: string; + user_organizations_url: string; + user_repositories_url: string; + user_search_url: string; +} diff --git a/src/utils/api/__snapshots__/request.test.ts.snap b/src/utils/api/__snapshots__/request.test.ts.snap new file mode 100644 index 000000000..05f61a958 --- /dev/null +++ b/src/utils/api/__snapshots__/request.test.ts.snap @@ -0,0 +1,51 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`apiRequestAuth should make an authenticated request with the correct parameters 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`apiRequestAuth should make an authenticated request with the correct parameters and default data 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/request.ts should make a request with the correct parameters 1`] = ` +{ + "Accept": "application/json", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/request.ts should make a request with the correct parameters and default data 1`] = ` +{ + "Accept": "application/json", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api-requests.ts should make a request with the correct parameters 1`] = ` +{ + "Accept": "application/json", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api-requests.ts should make a request with the correct parameters and default data 1`] = ` +{ + "Accept": "application/json", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; diff --git a/src/utils/api/client.ts b/src/utils/api/client.ts index 40a347537..2f070e8a9 100644 --- a/src/utils/api/client.ts +++ b/src/utils/api/client.ts @@ -1,25 +1,90 @@ -import axios, { type AxiosPromise, type Method } from 'axios'; - -function apiRequest( - url: string, - method: Method, - data = {}, -): AxiosPromise | null { - axios.defaults.headers.common.Accept = 'application/json'; - axios.defaults.headers.common['Content-Type'] = 'application/json'; - axios.defaults.headers.common['Cache-Control'] = 'no-cache'; - return axios({ method, url, data }); -} - -function apiRequestAuth( - url: string, - method: Method, - token: string, - data = {}, -): AxiosPromise | null { - axios.defaults.headers.common.Accept = 'application/json'; - axios.defaults.headers.common.Authorization = `token ${token}`; - axios.defaults.headers.common['Cache-Control'] = 'no-cache'; - axios.defaults.headers.common['Content-Type'] = 'application/json'; - return axios({ method, url, data }); +import type { Axios, AxiosPromise } from 'axios'; +import type { SettingsState } from '../../types'; +import type { + NotificationThreadSubscription, + RootHypermediaLinks, + UserDetails, +} from '../../typesGithub'; +import { getGitHubAPIBaseUrl } from '../helpers'; +import { apiRequestAuth } from './request'; + +export function getRootHypermediaLinks( + hostname: string, + token: string, +): AxiosPromise { + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(baseUrl); + return apiRequestAuth(url.toString(), 'GET', token); +} + +export function getAuthenticatedUser( + hostname: string, + token: string, +): AxiosPromise { + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/user`); + return apiRequestAuth(url.toString(), 'GET', token); +} + +export function headNotifications( + hostname: string, + token: string, +): AxiosPromise { + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/notifications`); + return apiRequestAuth(url.toString(), 'HEAD', token); +} + +export function listNotificationsForAuthenticatedUser( + hostname: string, + token: string, + settings: SettingsState, +): AxiosPromise { + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/notifications`); + url.searchParams.append('participating', String(settings.participating)); + + return apiRequestAuth(url.toString(), 'GET', token); +} + +export function markNotificationThreadAsRead( + threadId: string, + hostname: string, + token: string, +): AxiosPromise { + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/notifications/threads/${threadId}`); + return apiRequestAuth(url.toString(), 'PATCH', token, {}); +} + +export function markNotificationThreadAsDone( + threadId: string, + hostname: string, + token: string, +): AxiosPromise { + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/notifications/threads/${threadId}`); + return apiRequestAuth(url.toString(), 'DELETE', token, {}); +} + +export function ignoreNotificationThreadSubscription( + threadId: string, + hostname: string, + token: string, +): AxiosPromise { + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL( + `${baseUrl}/notifications/threads/${threadId}/subscriptions`, + ); + return apiRequestAuth(url.toString(), 'PUT', token, { ignored: true }); +} + +export function markRepositoryNotificationsAsRead( + repoSlug: string, + hostname: string, + token: string, +): AxiosPromise { + const baseUrl = getGitHubAPIBaseUrl(hostname); + const url = new URL(`${baseUrl}/repos/${repoSlug}/notifications`); + return apiRequestAuth(url.toString(), 'PUT', token, {}); } diff --git a/src/utils/api-requests.test.ts b/src/utils/api/request.test.ts similarity index 93% rename from src/utils/api-requests.test.ts rename to src/utils/api/request.test.ts index 7d0bc6637..45b5ee7a7 100644 --- a/src/utils/api-requests.test.ts +++ b/src/utils/api/request.test.ts @@ -1,12 +1,12 @@ import axios from 'axios'; -import { apiRequest, apiRequestAuth } from './api-requests'; +import { apiRequest, apiRequestAuth } from './request'; jest.mock('axios'); const url = 'https://example.com'; const method = 'get'; -describe('utils/api-requests.ts', () => { +describe('utils/api/request.ts', () => { afterEach(() => { jest.resetAllMocks(); }); diff --git a/src/utils/api-requests.ts b/src/utils/api/request.ts similarity index 100% rename from src/utils/api-requests.ts rename to src/utils/api/request.ts diff --git a/src/utils/auth.test.ts b/src/utils/auth.test.ts index 0365677fd..ab7080967 100644 --- a/src/utils/auth.test.ts +++ b/src/utils/auth.test.ts @@ -4,7 +4,7 @@ import remote from '@electron/remote'; const browserWindow = new remote.BrowserWindow(); import type { AuthState } from '../types'; -import * as apiRequests from './api-requests'; +import * as apiRequests from './api/request'; import * as auth from './auth'; describe('utils/auth.tsx', () => { diff --git a/src/utils/auth.ts b/src/utils/auth.ts index d69545258..187b352fd 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -7,9 +7,10 @@ import type { GitifyUser, } from '../types'; import type { UserDetails } from '../typesGithub'; -import { apiRequest, apiRequestAuth } from '../utils/api-requests'; import { Constants } from '../utils/constants'; -import { getGitHubAPIBaseUrl, isEnterpriseHost } from './helpers'; +import { getAuthenticatedUser } from './api/client'; +import { apiRequest } from './api/request'; +import { isEnterpriseHost } from './helpers'; export const authGitHub = ( authOptions = Constants.DEFAULT_AUTH_OPTIONS, @@ -82,12 +83,8 @@ export const getUserData = async ( token: string, hostname: string, ): Promise => { - const baseUrl = getGitHubAPIBaseUrl(hostname); - const url = new URL(`${baseUrl}/user`); - - const response: UserDetails = ( - await apiRequestAuth(url.toString(), 'GET', token) - ).data; + const response: UserDetails = (await getAuthenticatedUser(hostname, token)) + .data; return { id: response.id, diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts index 59f75cbb2..07c25d694 100644 --- a/src/utils/helpers.test.ts +++ b/src/utils/helpers.test.ts @@ -6,7 +6,7 @@ import { mockedUser, } from '../__mocks__/mockedData'; import type { SubjectType } from '../typesGithub'; -import * as apiRequests from './api-requests'; +import * as apiRequests from './api/request'; import { addHours, addNotificationReferrerIdToUrl, diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index 5f534cb01..553f46a03 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -8,8 +8,8 @@ import type { Notification, PullRequest, } from '../typesGithub'; -import { apiRequestAuth } from '../utils/api-requests'; import { openExternalLink } from '../utils/comms'; +import { apiRequestAuth } from './api/request'; import { Constants } from './constants'; import { getCheckSuiteAttributes, getWorkflowRunAttributes } from './subject'; diff --git a/src/utils/subject.ts b/src/utils/subject.ts index 2eda8469f..cf0b9895d 100644 --- a/src/utils/subject.ts +++ b/src/utils/subject.ts @@ -14,7 +14,7 @@ import type { User, WorkflowRunAttributes, } from '../typesGithub'; -import { apiRequestAuth } from './api-requests'; +import { apiRequestAuth } from './api/request'; import { fetchDiscussion, getLatestDiscussionComment } from './helpers'; export async function getGitifySubjectDetails( From d84887bfee4409edec9e3ebdd8a5a9e56db0ca5a Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 22 Apr 2024 07:12:07 -0400 Subject: [PATCH 3/5] refactor: api client --- src/hooks/useNotifications.ts | 11 +- .../__snapshots__/api-requests.test.ts.snap | 35 --- .../api/__snapshots__/client.test.ts.snap | 145 +++++++++ .../api/__snapshots__/request.test.ts.snap | 16 - src/utils/api/client.test.ts | 278 ++++++++++++++++++ src/utils/api/client.ts | 32 +- src/utils/auth.ts | 1 - src/utils/helpers.ts | 1 - src/utils/subject.ts | 23 +- 9 files changed, 460 insertions(+), 82 deletions(-) delete mode 100644 src/utils/__snapshots__/api-requests.test.ts.snap create mode 100644 src/utils/api/__snapshots__/client.test.ts.snap create mode 100644 src/utils/api/client.test.ts diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index b1d5fa460..a2841a823 100644 --- a/src/hooks/useNotifications.ts +++ b/src/hooks/useNotifications.ts @@ -12,11 +12,10 @@ import { ignoreNotificationThreadSubscription, listNotificationsForAuthenticatedUser, markNotificationThreadAsDone, + markNotificationThreadAsRead, markRepositoryNotificationsAsRead, - markThreadAsRead, } from '../utils/api/client'; import { determineFailureType } from '../utils/api/errors'; -import { apiRequestAuth } from '../utils/api/request'; import Constants from '../utils/constants'; import { getEnterpriseAccountToken, @@ -80,12 +79,6 @@ export const useNotifications = (): NotificationsState => { const fetchNotifications = useCallback( async (accounts: AuthState, settings: SettingsState) => { - function getNotifications(hostname: string, token: string): AxiosPromise { - const endpointSuffix = `notifications?participating=${settings.participating}`; - const url = `${generateGitHubAPIUrl(hostname)}${endpointSuffix}`; - return apiRequestAuth(url, 'GET', token); - } - function getGitHubNotifications() { if (!isGitHubLoggedIn(accounts)) { return; @@ -229,7 +222,7 @@ export const useNotifications = (): NotificationsState => { : accounts.token; try { - await markThreadAsRead(id, hostname, token); + await markNotificationThreadAsRead(id, hostname, token); const updatedNotifications = removeNotification( id, diff --git a/src/utils/__snapshots__/api-requests.test.ts.snap b/src/utils/__snapshots__/api-requests.test.ts.snap deleted file mode 100644 index 420db2eb8..000000000 --- a/src/utils/__snapshots__/api-requests.test.ts.snap +++ /dev/null @@ -1,35 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`apiRequestAuth should make an authenticated request with the correct parameters 1`] = ` -{ - "Accept": "application/json", - "Authorization": "token yourAuthToken", - "Cache-Control": "no-cache", - "Content-Type": "application/json", -} -`; - -exports[`apiRequestAuth should make an authenticated request with the correct parameters and default data 1`] = ` -{ - "Accept": "application/json", - "Authorization": "token yourAuthToken", - "Cache-Control": "no-cache", - "Content-Type": "application/json", -} -`; - -exports[`utils/api-requests.ts should make a request with the correct parameters 1`] = ` -{ - "Accept": "application/json", - "Cache-Control": "no-cache", - "Content-Type": "application/json", -} -`; - -exports[`utils/api-requests.ts should make a request with the correct parameters and default data 1`] = ` -{ - "Accept": "application/json", - "Cache-Control": "no-cache", - "Content-Type": "application/json", -} -`; diff --git a/src/utils/api/__snapshots__/client.test.ts.snap b/src/utils/api/__snapshots__/client.test.ts.snap new file mode 100644 index 000000000..85d09f88f --- /dev/null +++ b/src/utils/api/__snapshots__/client.test.ts.snap @@ -0,0 +1,145 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`utils/api/client.ts getAuthenticatedUser should fetch authenticated user - enterprise 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts getAuthenticatedUser should fetch authenticated user - github 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts getRootHypermediaLinks should fetch root hypermedia links - enterprise 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts getRootHypermediaLinks should fetch root hypermedia links - github 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts headNotifications should fetch notifications head - enterprise 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts headNotifications should fetch notifications head - github 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts ignoreNotificationThreadSubscription should ignore notification thread subscription - github 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts ignoreNotificationThreadSubscription should ignore notification thread subscription- enterprise 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts listNotificationsForAuthenticatedUser should list notifications for user - enterprise 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts listNotificationsForAuthenticatedUser should list notifications for user - github 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts markNotificationThreadAsDone should mark notification thread as done - github 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts markNotificationThreadAsDone should mark notification thread as done- enterprise 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts markNotificationThreadAsRead should mark notification thread as read - github 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts markNotificationThreadAsRead should mark notification thread as read- enterprise 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts markRepositoryNotificationsAsRead should mark repository notifications as read - enterprise 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; + +exports[`utils/api/client.ts markRepositoryNotificationsAsRead should mark repository notifications as read - github 1`] = ` +{ + "Accept": "application/json", + "Authorization": "token yourAuthToken", + "Cache-Control": "no-cache", + "Content-Type": "application/json", +} +`; diff --git a/src/utils/api/__snapshots__/request.test.ts.snap b/src/utils/api/__snapshots__/request.test.ts.snap index 05f61a958..bc9cdb84a 100644 --- a/src/utils/api/__snapshots__/request.test.ts.snap +++ b/src/utils/api/__snapshots__/request.test.ts.snap @@ -33,19 +33,3 @@ exports[`utils/api/request.ts should make a request with the correct parameters "Content-Type": "application/json", } `; - -exports[`utils/api-requests.ts should make a request with the correct parameters 1`] = ` -{ - "Accept": "application/json", - "Cache-Control": "no-cache", - "Content-Type": "application/json", -} -`; - -exports[`utils/api-requests.ts should make a request with the correct parameters and default data 1`] = ` -{ - "Accept": "application/json", - "Cache-Control": "no-cache", - "Content-Type": "application/json", -} -`; diff --git a/src/utils/api/client.test.ts b/src/utils/api/client.test.ts new file mode 100644 index 000000000..9684523a9 --- /dev/null +++ b/src/utils/api/client.test.ts @@ -0,0 +1,278 @@ +import axios from 'axios'; +import type { SettingsState } from '../../types'; +import { + getAuthenticatedUser, + getRootHypermediaLinks, + headNotifications, + ignoreNotificationThreadSubscription, + listNotificationsForAuthenticatedUser, + markNotificationThreadAsDone, + markNotificationThreadAsRead, + markRepositoryNotificationsAsRead, +} from './client'; + +jest.mock('axios'); + +const mockGitHubHostname = 'github.com'; +const mockEnterpriseHostname = 'example.com'; +const mockToken = 'yourAuthToken'; +const mockThreadId = '1234'; +const mockRepoSlug = 'gitify-app/notification-test'; + +describe('utils/api/client.ts', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('getRootHypermediaLinks', () => { + it('should fetch root hypermedia links - github', async () => { + await getRootHypermediaLinks(mockGitHubHostname, mockToken); + + expect(axios).toHaveBeenCalledWith({ + url: 'https://api.github.com/', + method: 'GET', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + + it('should fetch root hypermedia links - enterprise', async () => { + await getRootHypermediaLinks(mockEnterpriseHostname, mockToken); + + expect(axios).toHaveBeenCalledWith({ + url: 'https://example.com/api/v3', + method: 'GET', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + }); + + describe('getAuthenticatedUser', () => { + it('should fetch authenticated user - github', async () => { + await getAuthenticatedUser(mockGitHubHostname, mockToken); + + expect(axios).toHaveBeenCalledWith({ + url: 'https://api.github.com/user', + method: 'GET', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + + it('should fetch authenticated user - enterprise', async () => { + await getAuthenticatedUser(mockEnterpriseHostname, mockToken); + + expect(axios).toHaveBeenCalledWith({ + url: 'https://example.com/api/v3/user', + method: 'GET', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + }); + + describe('headNotifications', () => { + it('should fetch notifications head - github', async () => { + await headNotifications(mockGitHubHostname, mockToken); + + expect(axios).toHaveBeenCalledWith({ + url: 'https://api.github.com/notifications', + method: 'HEAD', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + + it('should fetch notifications head - enterprise', async () => { + await headNotifications(mockEnterpriseHostname, mockToken); + + expect(axios).toHaveBeenCalledWith({ + url: 'https://example.com/api/v3/notifications', + method: 'HEAD', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + }); + + describe('listNotificationsForAuthenticatedUser', () => { + const mockSettings: Partial = { + participating: true, + }; + + it('should list notifications for user - github', async () => { + await listNotificationsForAuthenticatedUser( + mockGitHubHostname, + mockToken, + mockSettings as SettingsState, + ); + + expect(axios).toHaveBeenCalledWith({ + url: 'https://api.github.com/notifications?participating=true', + method: 'GET', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + + it('should list notifications for user - enterprise', async () => { + await listNotificationsForAuthenticatedUser( + mockEnterpriseHostname, + mockToken, + mockSettings as SettingsState, + ); + + expect(axios).toHaveBeenCalledWith({ + url: 'https://example.com/api/v3/notifications?participating=true', + method: 'GET', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + }); + + describe('markNotificationThreadAsRead', () => { + it('should mark notification thread as read - github', async () => { + await markNotificationThreadAsRead( + mockThreadId, + mockGitHubHostname, + mockToken, + ); + + expect(axios).toHaveBeenCalledWith({ + url: `https://api.github.com/notifications/threads/${mockThreadId}`, + method: 'PATCH', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + + it('should mark notification thread as read- enterprise', async () => { + await markNotificationThreadAsRead( + mockThreadId, + mockEnterpriseHostname, + mockToken, + ); + + expect(axios).toHaveBeenCalledWith({ + url: `https://example.com/api/v3/notifications/threads/${mockThreadId}`, + method: 'PATCH', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + }); + + describe('markNotificationThreadAsDone', () => { + it('should mark notification thread as done - github', async () => { + await markNotificationThreadAsDone( + mockThreadId, + mockGitHubHostname, + mockToken, + ); + + expect(axios).toHaveBeenCalledWith({ + url: `https://api.github.com/notifications/threads/${mockThreadId}`, + method: 'DELETE', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + + it('should mark notification thread as done- enterprise', async () => { + await markNotificationThreadAsDone( + mockThreadId, + mockEnterpriseHostname, + mockToken, + ); + + expect(axios).toHaveBeenCalledWith({ + url: `https://example.com/api/v3/notifications/threads/${mockThreadId}`, + method: 'DELETE', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + }); + + describe('ignoreNotificationThreadSubscription', () => { + it('should ignore notification thread subscription - github', async () => { + await ignoreNotificationThreadSubscription( + mockThreadId, + mockGitHubHostname, + mockToken, + ); + + expect(axios).toHaveBeenCalledWith({ + url: `https://api.github.com/notifications/threads/${mockThreadId}/subscriptions`, + method: 'PUT', + data: { ignored: true }, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + + it('should ignore notification thread subscription- enterprise', async () => { + await ignoreNotificationThreadSubscription( + mockThreadId, + mockEnterpriseHostname, + mockToken, + ); + + expect(axios).toHaveBeenCalledWith({ + url: `https://example.com/api/v3/notifications/threads/${mockThreadId}/subscriptions`, + method: 'PUT', + data: { ignored: true }, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + }); + + describe('markRepositoryNotificationsAsRead', () => { + it('should mark repository notifications as read - github', async () => { + await markRepositoryNotificationsAsRead( + mockRepoSlug, + mockGitHubHostname, + mockToken, + ); + + expect(axios).toHaveBeenCalledWith({ + url: `https://api.github.com/repos/${mockRepoSlug}/notifications`, + method: 'PUT', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + + it('should mark repository notifications as read - enterprise', async () => { + await markRepositoryNotificationsAsRead( + mockRepoSlug, + mockEnterpriseHostname, + mockToken, + ); + + expect(axios).toHaveBeenCalledWith({ + url: `https://example.com/api/v3/repos/${mockRepoSlug}/notifications`, + method: 'PUT', + data: {}, + }); + + expect(axios.defaults.headers.common).toMatchSnapshot(); + }); + }); +}); diff --git a/src/utils/api/client.ts b/src/utils/api/client.ts index 2f070e8a9..292b42d1b 100644 --- a/src/utils/api/client.ts +++ b/src/utils/api/client.ts @@ -1,10 +1,16 @@ -import type { Axios, AxiosPromise } from 'axios'; +import type { AxiosPromise } from 'axios'; import type { SettingsState } from '../../types'; import type { + Commit, + Issue, + IssueComments, + Notification, NotificationThreadSubscription, + PullRequest, + ReleaseComments, RootHypermediaLinks, UserDetails, -} from '../../typesGithub'; +} from '../../typesGitHub'; import { getGitHubAPIBaseUrl } from '../helpers'; import { apiRequestAuth } from './request'; @@ -88,3 +94,25 @@ export function markRepositoryNotificationsAsRead( const url = new URL(`${baseUrl}/repos/${repoSlug}/notifications`); return apiRequestAuth(url.toString(), 'PUT', token, {}); } + +export function getCommit(url: string, token: string): AxiosPromise { + return apiRequestAuth(url, 'GET', token); +} + +export function getIssue(url: string, token: string): AxiosPromise { + return apiRequestAuth(url, 'GET', token); +} + +export function getPullRequest( + url: string, + token: string, +): AxiosPromise { + return apiRequestAuth(url, 'GET', token); +} + +export function getComments( + url: string, + token: string, +): AxiosPromise { + return apiRequestAuth(url, 'GET', token); +} diff --git a/src/utils/auth.ts b/src/utils/auth.ts index 7cfb08701..222e1900d 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -7,7 +7,6 @@ import type { GitifyUser, } from '../types'; import type { UserDetails } from '../typesGitHub'; -import { apiRequest, apiRequestAuth } from '../utils/api-requests'; import { Constants } from '../utils/constants'; import { getAuthenticatedUser } from './api/client'; import { apiRequest } from './api/request'; diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index c12589283..c1e96d1fa 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -8,7 +8,6 @@ import type { Notification, PullRequest, } from '../typesGitHub'; -import { apiRequestAuth } from '../utils/api-requests'; import { openExternalLink } from '../utils/comms'; import { apiRequestAuth } from './api/request'; import { Constants } from './constants'; diff --git a/src/utils/subject.ts b/src/utils/subject.ts index 469ea3b97..74fce35e8 100644 --- a/src/utils/subject.ts +++ b/src/utils/subject.ts @@ -1,20 +1,17 @@ import type { CheckSuiteAttributes, CheckSuiteStatus, - Commit, DiscussionStateType, GitifySubject, - Issue, IssueComments, Notification, - PullRequest, PullRequestStateType, ReleaseComments, SubjectUser, User, WorkflowRunAttributes, } from '../typesGitHub'; -import { apiRequestAuth } from './api-requests'; +import { getComments, getCommit, getIssue, getPullRequest } from './api/client'; import { fetchDiscussion, getLatestDiscussionComment } from './helpers'; export async function getGitifySubjectDetails( @@ -105,9 +102,7 @@ async function getGitifySubjectForCommit( token: string, ): Promise { try { - const commit: Commit = ( - await apiRequestAuth(notification.subject.url, 'GET', token) - ).data; + const commit = (await getCommit(notification.subject.url, token)).data; const commitCommentUser = await getLatestCommentUser(notification, token); @@ -172,9 +167,7 @@ async function getGitifySubjectForIssue( token: string, ): Promise { try { - const issue: Issue = ( - await apiRequestAuth(notification.subject.url, 'GET', token) - ).data; + const issue = (await getIssue(notification.subject.url, token)).data; const issueCommentUser = await getLatestCommentUser(notification, token); @@ -197,9 +190,7 @@ async function getGitifySubjectForPullRequest( token: string, ): Promise { try { - const pr: PullRequest = ( - await apiRequestAuth(notification.subject.url, 'GET', token) - ).data; + const pr = (await getPullRequest(notification.subject.url, token)).data; let prState: PullRequestStateType = pr.state; if (pr.merged) { @@ -300,11 +291,7 @@ async function getLatestCommentUser( try { const response: IssueComments | ReleaseComments = ( - await apiRequestAuth( - notification.subject.latest_comment_url, - 'GET', - token, - ) + await getComments(notification.subject.latest_comment_url, token) )?.data; return ( From 892860ddde16226dfb9e5cf2dc78a0740a2b2f18 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 22 Apr 2024 09:13:53 -0400 Subject: [PATCH 4/5] refactor: api client --- src/typesGitHub.ts | 14 +++-- src/utils/api/client.test.ts | 26 ++++++++- src/utils/api/client.ts | 109 +++++++++++++++++++++++++++++++++-- src/utils/helpers.test.ts | 27 --------- src/utils/helpers.ts | 15 +---- src/utils/subject.ts | 90 +++++++++++++++++------------ 6 files changed, 189 insertions(+), 92 deletions(-) diff --git a/src/typesGitHub.ts b/src/typesGitHub.ts index 9a7a004a3..7e1d6187c 100644 --- a/src/typesGitHub.ts +++ b/src/typesGitHub.ts @@ -330,7 +330,7 @@ interface CommitFiles { contents_url: string; patch: string; } -export interface CommitComments { +export interface CommitComment { url: string; html_url: string; issue_url: string; @@ -365,7 +365,7 @@ export interface Issue { state_reason: IssueStateReasonType | null; } -export interface IssueComments { +export interface IssueOrPullRequestComment { url: string; html_url: string; issue_url: string; @@ -377,20 +377,22 @@ export interface IssueComments { body: string; } -export interface ReleaseComments { +export interface Release { url: string; assets_url: string; + upload_url: string; html_url: string; id: number; author: User; node_id: string; tag_name: string; - name: string; + target_commitish: string; + name: string | null; + body: string | null; draft: boolean; prerelease: boolean; created_at: string; - published_at: string; - body: string; + published_at: string | null; } export interface GraphQLSearch { diff --git a/src/utils/api/client.test.ts b/src/utils/api/client.test.ts index 9684523a9..f0cda8baa 100644 --- a/src/utils/api/client.test.ts +++ b/src/utils/api/client.test.ts @@ -1,7 +1,8 @@ -import axios from 'axios'; +import axios, { type AxiosPromise, type AxiosResponse } from 'axios'; import type { SettingsState } from '../../types'; import { getAuthenticatedUser, + getHtmlUrl, getRootHypermediaLinks, headNotifications, ignoreNotificationThreadSubscription, @@ -10,6 +11,7 @@ import { markNotificationThreadAsRead, markRepositoryNotificationsAsRead, } from './client'; +import * as apiRequests from './request'; jest.mock('axios'); @@ -275,4 +277,26 @@ describe('utils/api/client.ts', () => { expect(axios.defaults.headers.common).toMatchSnapshot(); }); }); + + describe('getHtmlUrl', () => { + it('should return the HTML URL', async () => { + const apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth'); + + const requestPromise = new Promise((resolve) => + resolve({ + data: { + html_url: 'https://github.com/gitify-app/gitify/issues/785', + }, + } as AxiosResponse), + ) as AxiosPromise; + + apiRequestAuthMock.mockResolvedValue(requestPromise); + + const result = await getHtmlUrl( + 'https://api.github.com/repos/gitify-app/gitify/issues/785', + '123', + ); + expect(result).toBe('https://github.com/gitify-app/gitify/issues/785'); + }); + }); }); diff --git a/src/utils/api/client.ts b/src/utils/api/client.ts index 292b42d1b..c0b02b7c9 100644 --- a/src/utils/api/client.ts +++ b/src/utils/api/client.ts @@ -2,18 +2,24 @@ import type { AxiosPromise } from 'axios'; import type { SettingsState } from '../../types'; import type { Commit, + CommitComment, Issue, - IssueComments, + IssueOrPullRequestComment, Notification, NotificationThreadSubscription, PullRequest, - ReleaseComments, + Release, RootHypermediaLinks, UserDetails, } from '../../typesGitHub'; import { getGitHubAPIBaseUrl } from '../helpers'; import { apiRequestAuth } from './request'; +/** + * Get Hypermedia links to resources accessible in GitHub's REST API + * + * Endpoint documentation: https://docs.github.com/en/rest/meta/meta#github-api-root + */ export function getRootHypermediaLinks( hostname: string, token: string, @@ -23,6 +29,11 @@ export function getRootHypermediaLinks( return apiRequestAuth(url.toString(), 'GET', token); } +/** + * Get the authenticated user + * + * Endpoint documentation: https://docs.github.com/en/rest/users/users#get-the-authenticated-user + */ export function getAuthenticatedUser( hostname: string, token: string, @@ -32,6 +43,7 @@ export function getAuthenticatedUser( return apiRequestAuth(url.toString(), 'GET', token); } +// export function headNotifications( hostname: string, token: string, @@ -41,6 +53,11 @@ export function headNotifications( return apiRequestAuth(url.toString(), 'HEAD', token); } +/** + * List all notifications for the current user, sorted by most recently updated. + * + * Endpoint documentation: https://docs.github.com/en/rest/activity/notifications#list-notifications-for-the-authenticated-user + */ export function listNotificationsForAuthenticatedUser( hostname: string, token: string, @@ -53,6 +70,12 @@ export function listNotificationsForAuthenticatedUser( return apiRequestAuth(url.toString(), 'GET', token); } +/** + * Marks a thread as "read." Marking a thread as "read" is equivalent to + * clicking a notification in your notification inbox on GitHub. + * + * Endpoint documentation: https://docs.github.com/en/rest/activity/notifications#mark-a-thread-as-read + */ export function markNotificationThreadAsRead( threadId: string, hostname: string, @@ -63,6 +86,12 @@ export function markNotificationThreadAsRead( return apiRequestAuth(url.toString(), 'PATCH', token, {}); } +/** + * Marks a thread as "done." Marking a thread as "done" is equivalent to marking a + * notification in your notification inbox on GitHub as done. + * + * Endpoint documentation: https://docs.github.com/en/rest/activity/notifications#mark-a-thread-as-done + */ export function markNotificationThreadAsDone( threadId: string, hostname: string, @@ -73,6 +102,11 @@ export function markNotificationThreadAsDone( return apiRequestAuth(url.toString(), 'DELETE', token, {}); } +/** + * Ignore future notifications for threads until you comment on the thread or get an`@mention`. + * + * Endpoint documentation: https://docs.github.com/en/rest/activity/notifications#delete-a-thread-subscription + */ export function ignoreNotificationThreadSubscription( threadId: string, hostname: string, @@ -85,6 +119,14 @@ export function ignoreNotificationThreadSubscription( return apiRequestAuth(url.toString(), 'PUT', token, { ignored: true }); } +/** + * Marks all notifications in a repository as "read" for the current user. + * If the number of notifications is too large to complete in one request, + * you will receive a 202 Accepted status and GitHub will run an asynchronous + * process to mark notifications as "read." + * + * Endpoint documentation: https://docs.github.com/en/rest/activity/notifications#mark-repository-notifications-as-read + */ export function markRepositoryNotificationsAsRead( repoSlug: string, hostname: string, @@ -95,24 +137,79 @@ export function markRepositoryNotificationsAsRead( return apiRequestAuth(url.toString(), 'PUT', token, {}); } +/** + * Returns the contents of a single commit reference. + * + * Endpoint documentation: https://docs.github.com/en/rest/commits/commits#get-a-commit + */ export function getCommit(url: string, token: string): AxiosPromise { return apiRequestAuth(url, 'GET', token); } +/** + * Gets a specified commit comment. + * + * Endpoint documentation: https://docs.github.com/en/rest/commits/comments#get-a-commit-comment + + */ +export function getCommitComment( + url: string, + token: string, +): AxiosPromise { + return apiRequestAuth(url, 'GET', token); +} + +/** + * Get details of an issue. + * + * Endpoint documentation: https://docs.github.com/en/rest/issues/issues#get-an-issue + */ export function getIssue(url: string, token: string): AxiosPromise { return apiRequestAuth(url, 'GET', token); } -export function getPullRequest( +/** + * Get comments on issues and pull requests. + * Every pull request is an issue, but not every issue is a pull request. + * + * Endpoint documentation: https://docs.github.com/en/rest/issues/comments#get-an-issue-comment + */ +export function getIssueOrPullRequestComment( url: string, token: string, -): AxiosPromise { +): AxiosPromise { return apiRequestAuth(url, 'GET', token); } -export function getComments( +/** + * Get details of a pull request. + * + * Endpoint documentation: https://docs.github.com/en/rest/pulls/pulls#get-a-pull-request + */ +export function getPullRequest( url: string, token: string, -): AxiosPromise { +): AxiosPromise { return apiRequestAuth(url, 'GET', token); } + +/** + * Gets a public release with the specified release ID. + * + * Endpoint documentation: https://docs.github.com/en/rest/releases/releases#get-a-release + */ +export function getRelease(url: string, token: string): AxiosPromise { + return apiRequestAuth(url, 'GET', token); +} + +/** + * Get the `html_url` from the GitHub response + */ +export async function getHtmlUrl(url: string, token: string): Promise { + try { + const response = (await apiRequestAuth(url, 'GET', token)).data; + return response.html_url; + } catch (err) { + console.error('Failed to get html url'); + } +} diff --git a/src/utils/helpers.test.ts b/src/utils/helpers.test.ts index b1936fec5..54a1909a2 100644 --- a/src/utils/helpers.test.ts +++ b/src/utils/helpers.test.ts @@ -15,7 +15,6 @@ import { generateGitHubWebUrl, generateNotificationReferrerId, getGitHubAPIBaseUrl, - getHtmlUrl, isEnterpriseHost, isGitHubLoggedIn, } from './helpers'; @@ -130,32 +129,6 @@ describe('utils/helpers.ts', () => { }); }); - describe('getHtmlUrl', () => { - const apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth'); - - afterEach(() => { - jest.clearAllMocks(); - }); - - it('should return the HTML URL', async () => { - const requestPromise = new Promise((resolve) => - resolve({ - data: { - html_url: 'https://github.com/gitify-app/gitify/issues/785', - }, - } as AxiosResponse), - ) as AxiosPromise; - - apiRequestAuthMock.mockResolvedValue(requestPromise); - - const result = await getHtmlUrl( - 'https://api.github.com/repos/gitify-app/gitify/issues/785', - '123', - ); - expect(result).toBe('https://github.com/gitify-app/gitify/issues/785'); - }); - }); - describe('generateGitHubWebUrl', () => { const mockedHtmlUrl = 'https://github.com/gitify-app/gitify/issues/785'; const mockedNotificationReferrer = diff --git a/src/utils/helpers.ts b/src/utils/helpers.ts index c1e96d1fa..1161ac25d 100644 --- a/src/utils/helpers.ts +++ b/src/utils/helpers.ts @@ -3,12 +3,10 @@ import type { Discussion, DiscussionComment, GraphQLSearch, - Issue, - IssueComments, Notification, - PullRequest, } from '../typesGitHub'; import { openExternalLink } from '../utils/comms'; +import { getHtmlUrl } from './api/client'; import { apiRequestAuth } from './api/request'; import { Constants } from './constants'; import { getCheckSuiteAttributes, getWorkflowRunAttributes } from './subject'; @@ -81,17 +79,6 @@ export function formatSearchQueryString( return `${title} in:title repo:${repo} updated:>${addHours(lastUpdated, -2)}`; } -export async function getHtmlUrl(url: string, token: string): Promise { - try { - const response: Issue | IssueComments | PullRequest = ( - await apiRequestAuth(url, 'GET', token) - ).data; - return response.html_url; - } catch (err) { - console.error('Failed to get html url'); - } -} - export function getCheckSuiteUrl(notification: Notification) { let url = `${notification.repository.html_url}/actions`; const filters = []; diff --git a/src/utils/subject.ts b/src/utils/subject.ts index 74fce35e8..0a49fc311 100644 --- a/src/utils/subject.ts +++ b/src/utils/subject.ts @@ -3,15 +3,20 @@ import type { CheckSuiteStatus, DiscussionStateType, GitifySubject, - IssueComments, Notification, PullRequestStateType, - ReleaseComments, SubjectUser, User, WorkflowRunAttributes, } from '../typesGitHub'; -import { getComments, getCommit, getIssue, getPullRequest } from './api/client'; +import { + getCommit, + getCommitComment, + getIssue, + getIssueOrPullRequestComment, + getPullRequest, + getRelease, +} from './api/client'; import { fetchDiscussion, getLatestDiscussionComment } from './helpers'; export async function getGitifySubjectDetails( @@ -102,21 +107,31 @@ async function getGitifySubjectForCommit( token: string, ): Promise { try { - const commit = (await getCommit(notification.subject.url, token)).data; + let user: User; - const commitCommentUser = await getLatestCommentUser(notification, token); + if (notification.subject.latest_comment_url) { + const commitComment = ( + await getCommitComment(notification.subject.latest_comment_url, token) + ).data; + + user = commitComment.user; + } else { + const commit = (await getCommit(notification.subject.url, token)).data; + + user = commit.author; + } return { state: null, user: { - login: commitCommentUser?.login ?? commit.author.login, - html_url: commitCommentUser?.html_url ?? commit.author.html_url, - avatar_url: commitCommentUser?.avatar_url ?? commit.author.avatar_url, - type: commitCommentUser?.type ?? commit.author.type, + login: user.login, + html_url: user.html_url, + avatar_url: user.avatar_url, + type: user.type, }, }; } catch (err) { - console.error('Issue subject retrieval failed'); + console.error('Commit subject retrieval failed'); } } @@ -169,7 +184,17 @@ async function getGitifySubjectForIssue( try { const issue = (await getIssue(notification.subject.url, token)).data; - const issueCommentUser = await getLatestCommentUser(notification, token); + let issueCommentUser: User; + + if (notification.subject.latest_comment_url) { + const issueComment = ( + await getIssueOrPullRequestComment( + notification.subject.latest_comment_url, + token, + ) + ).data; + issueCommentUser = issueComment.user; + } return { state: issue.state_reason ?? issue.state, @@ -199,7 +224,17 @@ async function getGitifySubjectForPullRequest( prState = 'draft'; } - const prCommentUser = await getLatestCommentUser(notification, token); + let prCommentUser: User; + + if (notification.subject.latest_comment_url) { + const prComment = ( + await getIssueOrPullRequestComment( + notification.subject.latest_comment_url, + token, + ) + ).data; + prCommentUser = prComment.user; + } return { state: prState, @@ -219,15 +254,15 @@ async function getGitifySubjectForRelease( notification: Notification, token: string, ): Promise { - const releaseCommentUser = await getLatestCommentUser(notification, token); + const release = (await getRelease(notification.subject.url, token)).data; return { state: null, user: { - login: releaseCommentUser.login, - html_url: releaseCommentUser.html_url, - avatar_url: releaseCommentUser.avatar_url, - type: releaseCommentUser.type, + login: release.author.login, + html_url: release.author.html_url, + avatar_url: release.author.avatar_url, + type: release.author.type, }, }; } @@ -280,24 +315,3 @@ function getWorkflowRunStatus(statusDisplayName: string): CheckSuiteStatus { return null; } } - -async function getLatestCommentUser( - notification: Notification, - token: string, -): Promise | null { - if (!notification.subject.latest_comment_url) { - return null; - } - - try { - const response: IssueComments | ReleaseComments = ( - await getComments(notification.subject.latest_comment_url, token) - )?.data; - - return ( - (response as IssueComments)?.user ?? (response as ReleaseComments).author - ); - } catch (err) { - console.error('Discussion latest comment retrieval failed'); - } -} From 5d13f32d0350132f8de955efacdd0f09bca6ce55 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Mon, 22 Apr 2024 09:15:01 -0400 Subject: [PATCH 5/5] refactor: api client --- src/typesGitHub.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/typesGitHub.ts b/src/typesGitHub.ts index 7e1d6187c..164033fec 100644 --- a/src/typesGitHub.ts +++ b/src/typesGitHub.ts @@ -107,6 +107,7 @@ export interface UserProfile { two_factor_authentication: boolean; plan: Plan; } + export interface Plan { name: string; space: number; @@ -330,6 +331,7 @@ interface CommitFiles { contents_url: string; patch: string; } + export interface CommitComment { url: string; html_url: string;