diff --git a/src/renderer/context/App.test.tsx b/src/renderer/context/App.test.tsx index f0ba95845..6e91a4316 100644 --- a/src/renderer/context/App.test.tsx +++ b/src/renderer/context/App.test.tsx @@ -179,7 +179,10 @@ describe('renderer/context/App.tsx', () => { }); describe('authentication methods', () => { - const apiRequestAuthSpy = jest.spyOn(apiRequests, 'apiRequestAuth'); + const performAuthenticatedRESTRequestSpy = jest.spyOn( + apiRequests, + 'performAuthenticatedRESTRequest', + ); it('should call loginWithGitHubApp', async () => { const { button } = renderContextButton('loginWithGitHubApp'); @@ -202,7 +205,7 @@ describe('renderer/context/App.tsx', () => { }); it('should call loginWithPersonalAccessToken', async () => { - apiRequestAuthSpy.mockResolvedValueOnce(null); + performAuthenticatedRESTRequestSpy.mockResolvedValueOnce(null); const { button } = renderContextButton('loginWithPersonalAccessToken', { hostname: 'github.com' as Hostname, @@ -215,8 +218,8 @@ describe('renderer/context/App.tsx', () => { expect(mockFetchNotifications).toHaveBeenCalledTimes(1), ); - expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1); - expect(apiRequestAuthSpy).toHaveBeenCalledWith( + expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledTimes(1); + expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith( 'https://api.github.com/notifications', 'HEAD', 'encrypted', diff --git a/src/renderer/utils/api/client.test.ts b/src/renderer/utils/api/client.test.ts index 524c76288..f23eb822a 100644 --- a/src/renderer/utils/api/client.test.ts +++ b/src/renderer/utils/api/client.test.ts @@ -1,4 +1,4 @@ -import axios, { type AxiosPromise, type AxiosResponse } from 'axios'; +import axios from 'axios'; import { mockGitHubCloudAccount } from '../../__mocks__/account-mocks'; import { @@ -7,13 +7,20 @@ import { } from '../../__mocks__/notifications-mocks'; import { mockToken } from '../../__mocks__/state-mocks'; import { Constants } from '../../constants'; -import type { Hostname, Link, SettingsState, Token } from '../../types'; +import type { + AuthCode, + Hostname, + Link, + SettingsState, + Token, +} from '../../types'; import * as logger from '../../utils/logger'; import { mockAuthHeaders, mockNonCachedAuthHeaders, } from './__mocks__/request-mocks'; import { + exchangeAuthCodeForAccessToken, fetchAuthenticatedUserDetails, fetchDiscussionByNumber, fetchIssueByNumber, @@ -36,19 +43,63 @@ import { FetchPullRequestByNumberDocument, type FetchPullRequestByNumberQuery, } from './graphql/generated/graphql'; -import type { ExecutionResultWithHeaders } from './request'; import * as apiRequests from './request'; +import type { + GitHubGraphQLResponse, + GitHubHtmlUrlResponse, + LoginOAuthResponse, +} from './types'; jest.mock('axios'); const mockGitHubHostname = 'github.com' as Hostname; const mockThreadId = '1234'; +const mockAuthCode = 'some-auth-code' as AuthCode; + describe('renderer/utils/api/client.ts', () => { afterEach(() => { jest.clearAllMocks(); }); + it('exchangeAuthCodeForAccessToken - should exchange code', async () => { + const performUnauthenticatedRESTRequestSpy = jest.spyOn( + apiRequests, + 'performUnauthenticatedRESTRequest', + ); + + const requestPromise = Promise.resolve({ + access_token: mockToken, + } as LoginOAuthResponse); + + performUnauthenticatedRESTRequestSpy.mockResolvedValue(requestPromise); + + const { + hostname: mockHostname, + clientId: mockClientId, + clientSecret: mockClientSecret, + } = Constants.DEFAULT_AUTH_OPTIONS; + + const response = await exchangeAuthCodeForAccessToken( + mockHostname, + Constants.DEFAULT_AUTH_OPTIONS.clientId, + Constants.DEFAULT_AUTH_OPTIONS.clientSecret, + mockAuthCode, + ); + + expect(apiRequests.performUnauthenticatedRESTRequest).toHaveBeenCalledWith( + 'https://github.com/login/oauth/access_token', + 'POST', + { + client_id: mockClientId, + client_secret: mockClientSecret, + code: mockAuthCode, + }, + ); + + expect(response.access_token).toBe(mockToken); + }); + it('headNotifications - should fetch notifications head', async () => { await headNotifications(mockGitHubHostname, mockToken); @@ -185,16 +236,16 @@ describe('renderer/utils/api/client.ts', () => { describe('getHtmlUrl', () => { it('should return the HTML URL', async () => { - const apiRequestAuthSpy = jest.spyOn(apiRequests, 'apiRequestAuth'); + const performAuthenticatedRESTRequestSpy = jest.spyOn( + apiRequests, + 'performAuthenticatedRESTRequest', + ); const requestPromise = Promise.resolve({ - data: { - html_url: - 'https://github.com/gitify-app/notifications-test/issues/785', - }, - } as AxiosResponse) as AxiosPromise; + html_url: 'https://github.com/gitify-app/notifications-test/issues/785', + } as GitHubHtmlUrlResponse); - apiRequestAuthSpy.mockResolvedValue(requestPromise); + performAuthenticatedRESTRequestSpy.mockResolvedValue(requestPromise); const result = await getHtmlUrl( 'https://api.github.com/repos/gitify-app/notifications-test/issues/785' as Link, @@ -210,11 +261,14 @@ describe('renderer/utils/api/client.ts', () => { .spyOn(logger, 'rendererLogError') .mockImplementation(); - const apiRequestAuthSpy = jest.spyOn(apiRequests, 'apiRequestAuth'); + const performAuthenticatedRESTRequestSpy = jest.spyOn( + apiRequests, + 'performAuthenticatedRESTRequest', + ); const mockError = new Error('Test error'); - apiRequestAuthSpy.mockRejectedValue(mockError); + performAuthenticatedRESTRequestSpy.mockRejectedValue(mockError); await getHtmlUrl( 'https://api.github.com/repos/gitify-app/gitify/issues/785' as Link, @@ -234,7 +288,7 @@ describe('renderer/utils/api/client.ts', () => { performGraphQLRequestSpy.mockResolvedValue({ data: {}, headers: {}, - } as ExecutionResultWithHeaders); + } as GitHubGraphQLResponse); await fetchAuthenticatedUserDetails(mockGitHubHostname, mockToken); @@ -260,7 +314,7 @@ describe('renderer/utils/api/client.ts', () => { performGraphQLRequestSpy.mockResolvedValue({ data: {}, headers: {}, - } as ExecutionResultWithHeaders); + } as GitHubGraphQLResponse); await fetchDiscussionByNumber(mockNotification); @@ -295,7 +349,7 @@ describe('renderer/utils/api/client.ts', () => { performGraphQLRequestSpy.mockResolvedValue({ data: {}, headers: {}, - } as ExecutionResultWithHeaders); + } as GitHubGraphQLResponse); await fetchIssueByNumber(mockNotification); @@ -328,7 +382,7 @@ describe('renderer/utils/api/client.ts', () => { performGraphQLRequestSpy.mockResolvedValue({ data: {}, headers: {}, - } as ExecutionResultWithHeaders); + } as GitHubGraphQLResponse); await fetchPullByNumber(mockNotification); @@ -364,7 +418,7 @@ describe('renderer/utils/api/client.ts', () => { performGraphQLRequestStringSpy.mockResolvedValue({ data: {}, headers: {}, - } as ExecutionResultWithHeaders); + } as GitHubGraphQLResponse); await fetchNotificationDetailsForList([mockNotification]); @@ -380,7 +434,7 @@ describe('renderer/utils/api/client.ts', () => { performGraphQLRequestStringSpy.mockResolvedValue({ data: {}, headers: {}, - } as ExecutionResultWithHeaders); + } as GitHubGraphQLResponse); await fetchNotificationDetailsForList([]); @@ -396,7 +450,7 @@ describe('renderer/utils/api/client.ts', () => { performGraphQLRequestStringSpy.mockResolvedValue({ data: {}, headers: {}, - } as ExecutionResultWithHeaders); + } as GitHubGraphQLResponse); await fetchNotificationDetailsForList(mockGitHubCloudGitifyNotifications); diff --git a/src/renderer/utils/api/client.ts b/src/renderer/utils/api/client.ts index c77ddbc80..ab72ac0c9 100644 --- a/src/renderer/utils/api/client.ts +++ b/src/renderer/utils/api/client.ts @@ -1,9 +1,11 @@ import type { AxiosPromise } from 'axios'; -import type { ExecutionResult } from 'graphql'; import { Constants } from '../../constants'; import type { Account, + AuthCode, + ClientID, + ClientSecret, GitifyNotification, Hostname, Link, @@ -26,12 +28,15 @@ import { } from './graphql/generated/graphql'; import { MergeQueryBuilder } from './graphql/MergeQueryBuilder'; import { - apiRequestAuth, - type ExecutionResultWithHeaders, + performAuthenticatedRESTRequest, performGraphQLRequest, performGraphQLRequestString, + performUnauthenticatedRESTRequest, } from './request'; import type { + GitHubGraphQLResponse, + GitHubHtmlUrlResponse, + LoginOAuthResponse, NotificationThreadSubscription, RawCommit, RawCommitComment, @@ -44,6 +49,29 @@ import { getNumberFromUrl, } from './utils'; +/** + * Exchange an OAuth Auth Code for an Access Token + */ +export function exchangeAuthCodeForAccessToken( + hostname: Hostname, + clientId: ClientID, + clientSecret: ClientSecret, + authCode: AuthCode, +) { + const url = `https://${hostname}/login/oauth/access_token` as Link; + const data = { + client_id: clientId, + client_secret: clientSecret, + code: authCode, + }; + + return performUnauthenticatedRESTRequest( + 'POST', + url, + data, + ); +} + /** * Perform a HEAD operation, used to validate that connectivity is established. * @@ -56,7 +84,7 @@ export function headNotifications( const url = getGitHubAPIBaseUrl(hostname); url.pathname += 'notifications'; - return apiRequestAuth(url.toString() as Link, 'HEAD', token); + return performAuthenticatedRESTRequest('HEAD', url.toString() as Link, token); } /** @@ -73,9 +101,9 @@ export function listNotificationsForAuthenticatedUser( url.searchParams.append('participating', String(settings.participating)); url.searchParams.append('all', String(settings.fetchReadNotifications)); - return apiRequestAuth( - url.toString() as Link, + return performAuthenticatedRESTRequest( 'GET', + url.toString() as Link, account.token, {}, settings.fetchAllNotifications, @@ -96,7 +124,12 @@ export function markNotificationThreadAsRead( const url = getGitHubAPIBaseUrl(hostname); url.pathname += `notifications/threads/${threadId}`; - return apiRequestAuth(url.toString() as Link, 'PATCH', token, {}); + return performAuthenticatedRESTRequest( + 'PATCH', + url.toString() as Link, + token, + {}, + ); } /** @@ -115,7 +148,12 @@ export function markNotificationThreadAsDone( const url = getGitHubAPIBaseUrl(hostname); url.pathname += `notifications/threads/${threadId}`; - return apiRequestAuth(url.toString() as Link, 'DELETE', token, {}); + return performAuthenticatedRESTRequest( + 'DELETE', + url.toString() as Link, + token, + {}, + ); } /** @@ -131,7 +169,7 @@ export function ignoreNotificationThreadSubscription( const url = getGitHubAPIBaseUrl(hostname); url.pathname += `notifications/threads/${threadId}/subscription`; - return apiRequestAuth(url.toString() as Link, 'PUT', token, { + return performAuthenticatedRESTRequest('PUT', url.toString() as Link, token, { ignored: true, }); } @@ -142,7 +180,7 @@ export function ignoreNotificationThreadSubscription( * Endpoint documentation: https://docs.github.com/en/rest/commits/commits#get-a-commit */ export function getCommit(url: Link, token: Token): AxiosPromise { - return apiRequestAuth(url, 'GET', token); + return performAuthenticatedRESTRequest('GET', url, token); } /** @@ -155,7 +193,7 @@ export function getCommitComment( url: Link, token: Token, ): AxiosPromise { - return apiRequestAuth(url, 'GET', token); + return performAuthenticatedRESTRequest('GET', url, token); } /** @@ -164,7 +202,7 @@ export function getCommitComment( * Endpoint documentation: https://docs.github.com/en/rest/releases/releases#get-a-release */ export function getRelease(url: Link, token: Token): AxiosPromise { - return apiRequestAuth(url, 'GET', token); + return performAuthenticatedRESTRequest('GET', url, token); } /** @@ -172,7 +210,12 @@ export function getRelease(url: Link, token: Token): AxiosPromise { */ export async function getHtmlUrl(url: Link, token: Token): Promise { try { - const response = (await apiRequestAuth(url, 'GET', token)).data; + const response = + await performAuthenticatedRESTRequest( + 'GET', + url, + token, + ); return response.html_url; } catch (err) { @@ -190,7 +233,7 @@ export async function getHtmlUrl(url: Link, token: Token): Promise { export async function fetchAuthenticatedUserDetails( hostname: Hostname, token: Token, -): Promise> { +): Promise> { const url = getGitHubGraphQLUrl(hostname); return performGraphQLRequest( @@ -205,7 +248,7 @@ export async function fetchAuthenticatedUserDetails( */ export async function fetchDiscussionByNumber( notification: GitifyNotification, -): Promise> { +): Promise> { const url = getGitHubGraphQLUrl(notification.account.hostname); const number = getNumberFromUrl(notification.subject.url); @@ -232,7 +275,7 @@ export async function fetchDiscussionByNumber( */ export async function fetchIssueByNumber( notification: GitifyNotification, -): Promise> { +): Promise> { const url = getGitHubGraphQLUrl(notification.account.hostname); const number = getNumberFromUrl(notification.subject.url); @@ -255,7 +298,7 @@ export async function fetchIssueByNumber( */ export async function fetchPullByNumber( notification: GitifyNotification, -): Promise> { +): Promise> { const url = getGitHubGraphQLUrl(notification.account.hostname); const number = getNumberFromUrl(notification.subject.url); diff --git a/src/renderer/utils/api/errors.test.ts b/src/renderer/utils/api/errors.test.ts index 268e40719..6cae1382c 100644 --- a/src/renderer/utils/api/errors.test.ts +++ b/src/renderer/utils/api/errors.test.ts @@ -1,10 +1,12 @@ import { AxiosError, type AxiosResponse } from 'axios'; +import type { ExecutionResult } from 'graphql'; import { EVENTS } from '../../../shared/events'; import type { Link } from '../../types'; import { Errors } from '../errors'; -import { determineFailureType } from './errors'; +import * as rendererLogger from '../logger'; +import { assertNoGraphQLErrors, determineFailureType } from './errors'; import type { GitHubRESTError } from './types'; describe('renderer/utils/api/errors.ts', () => { @@ -118,6 +120,42 @@ describe('renderer/utils/api/errors.ts', () => { expect(result).toBe(Errors.UNKNOWN); }); + + describe('assertNoGraphQLErrors', () => { + it('throws and logs when GraphQL errors are present', () => { + const logSpy = jest + .spyOn(rendererLogger, 'rendererLogError') + .mockImplementation(); + + const payload = { + data: {}, + errors: [{}], + } as unknown as ExecutionResult; + + expect(() => assertNoGraphQLErrors('test-context', payload)).toThrow( + 'GraphQL request returned errors', + ); + + expect(logSpy).toHaveBeenCalled(); + }); + + it('does not throw when errors array is empty or undefined', () => { + const payloadNoErrors: ExecutionResult = { + data: {}, + }; + const payloadEmptyErrors: ExecutionResult = { + data: {}, + errors: [], + }; + + expect(() => + assertNoGraphQLErrors('test-context', payloadNoErrors), + ).not.toThrow(); + expect(() => + assertNoGraphQLErrors('test-context', payloadEmptyErrors), + ).not.toThrow(); + }); + }); }); function createMockResponse( diff --git a/src/renderer/utils/api/errors.ts b/src/renderer/utils/api/errors.ts index 649af6439..c90507fb0 100644 --- a/src/renderer/utils/api/errors.ts +++ b/src/renderer/utils/api/errors.ts @@ -1,7 +1,9 @@ import { AxiosError } from 'axios'; +import type { ExecutionResult } from 'graphql'; import type { GitifyError } from '../../types'; import { Errors } from '../errors'; +import { rendererLogError } from '../logger'; import type { GitHubRESTError } from './types'; export function determineFailureType( @@ -43,3 +45,18 @@ export function determineFailureType( return Errors.UNKNOWN; } + +/** + * Assert that a GraphQL response does not contain errors. + * Logs and throws if `errors` array is present and non-empty. + */ +export function assertNoGraphQLErrors( + context: string, + payload: ExecutionResult, +) { + if (Array.isArray(payload.errors) && payload.errors.length > 0) { + const err = new Error('GraphQL request returned errors'); + rendererLogError(context, 'GraphQL errors present in response', err); + throw err; + } +} diff --git a/src/renderer/utils/api/graphql/generated/graphql.ts b/src/renderer/utils/api/graphql/generated/graphql.ts index fda34b59c..9802e1530 100644 --- a/src/renderer/utils/api/graphql/generated/graphql.ts +++ b/src/renderer/utils/api/graphql/generated/graphql.ts @@ -4662,60 +4662,6 @@ export type CreateSponsorshipsPayload = { sponsorables?: Maybe>; }; -/** Autogenerated input type of CreateTeamDiscussion */ -export type CreateTeamDiscussionInput = { - /** - * The content of the discussion. This field is required. - * - * **Upcoming Change on 2024-07-01 UTC** - * **Description:** `body` will be removed. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. - * **Reason:** The Team Discussions feature is deprecated in favor of Organization Discussions. - * - */ - body?: InputMaybe; - /** A unique identifier for the client performing the mutation. */ - clientMutationId?: InputMaybe; - /** - * If true, restricts the visibility of this discussion to team members and organization owners. If false or not specified, allows any organization member to view this discussion. - * - * **Upcoming Change on 2024-07-01 UTC** - * **Description:** `private` will be removed. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. - * **Reason:** The Team Discussions feature is deprecated in favor of Organization Discussions. - * - */ - private?: InputMaybe; - /** - * The ID of the team to which the discussion belongs. This field is required. - * - * **Upcoming Change on 2024-07-01 UTC** - * **Description:** `teamId` will be removed. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. - * **Reason:** The Team Discussions feature is deprecated in favor of Organization Discussions. - * - */ - teamId?: InputMaybe; - /** - * The title of the discussion. This field is required. - * - * **Upcoming Change on 2024-07-01 UTC** - * **Description:** `title` will be removed. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. - * **Reason:** The Team Discussions feature is deprecated in favor of Organization Discussions. - * - */ - title?: InputMaybe; -}; - -/** Autogenerated return type of CreateTeamDiscussion. */ -export type CreateTeamDiscussionPayload = { - __typename?: 'CreateTeamDiscussionPayload'; - /** A unique identifier for the client performing the mutation. */ - clientMutationId?: Maybe; - /** - * The new discussion. - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - teamDiscussion?: Maybe; -}; - /** Autogenerated input type of CreateUserList */ export type CreateUserListInput = { /** A unique identifier for the client performing the mutation. */ @@ -5535,21 +5481,6 @@ export type DeleteRepositoryRulesetPayload = { clientMutationId?: Maybe; }; -/** Autogenerated input type of DeleteTeamDiscussion */ -export type DeleteTeamDiscussionInput = { - /** A unique identifier for the client performing the mutation. */ - clientMutationId?: InputMaybe; - /** The discussion ID to delete. */ - id: Scalars['ID']['input']; -}; - -/** Autogenerated return type of DeleteTeamDiscussion. */ -export type DeleteTeamDiscussionPayload = { - __typename?: 'DeleteTeamDiscussionPayload'; - /** A unique identifier for the client performing the mutation. */ - clientMutationId?: Maybe; -}; - /** Autogenerated input type of DeleteUserList */ export type DeleteUserListInput = { /** A unique identifier for the client performing the mutation. */ @@ -7575,10 +7506,11 @@ export type EnterpriseOwnerInfo = { samlIdentityProviderSettingOrganizations: OrganizationConnection; /** A list of members with a support entitlement. */ supportEntitlements: EnterpriseMemberConnection; - /** The setting value for whether team discussions are enabled for organizations in this enterprise. */ + /** + * The setting value for whether team discussions are enabled for organizations in this enterprise. + * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. + */ teamDiscussionsSetting: EnterpriseEnabledDisabledSettingValue; - /** A list of enterprise organizations configured with the provided team discussions setting value. */ - teamDiscussionsSettingOrganizations: OrganizationConnection; /** The setting value for what methods of two-factor authentication the enterprise prevents its users from having. */ twoFactorDisallowedMethodsSetting: EnterpriseDisallowedMethodsSettingValue; /** The setting value for whether the enterprise requires two-factor authentication for its organizations and users. */ @@ -7870,17 +7802,6 @@ export type EnterpriseOwnerInfoSupportEntitlementsArgs = { }; -/** Enterprise information visible to enterprise owners or enterprise owners' personal access tokens (classic) with read:enterprise or admin:enterprise scope. */ -export type EnterpriseOwnerInfoTeamDiscussionsSettingOrganizationsArgs = { - after?: InputMaybe; - before?: InputMaybe; - first?: InputMaybe; - last?: InputMaybe; - orderBy?: InputMaybe; - value: Scalars['Boolean']['input']; -}; - - /** Enterprise information visible to enterprise owners or enterprise owners' personal access tokens (classic) with read:enterprise or admin:enterprise scope. */ export type EnterpriseOwnerInfoTwoFactorRequiredSettingOrganizationsArgs = { after?: InputMaybe; @@ -12313,8 +12234,6 @@ export type Mutation = { createSponsorship?: Maybe; /** Make many sponsorships for different sponsorable users or organizations at once. Can only sponsor those who have a public GitHub Sponsors profile. */ createSponsorships?: Maybe; - /** Creates a new team discussion. */ - createTeamDiscussion?: Maybe; /** Creates a new user list. */ createUserList?: Maybe; /** Rejects a suggested topic for the repository. */ @@ -12378,8 +12297,6 @@ export type Mutation = { deleteRepositoryCustomProperty?: Maybe; /** Delete a repository ruleset */ deleteRepositoryRuleset?: Maybe; - /** Deletes a team discussion. */ - deleteTeamDiscussion?: Maybe; /** Deletes a user list. */ deleteUserList?: Maybe; /** Deletes a verifiable domain. */ @@ -12609,8 +12526,6 @@ export type Mutation = { updateEnterpriseProfile?: Maybe; /** Sets whether repository projects are enabled for a enterprise. */ updateEnterpriseRepositoryProjectsSetting?: Maybe; - /** Sets whether team discussions are enabled for an enterprise. */ - updateEnterpriseTeamDiscussionsSetting?: Maybe; /** Sets the two-factor authentication methods that users of an enterprise may not use. */ updateEnterpriseTwoFactorAuthenticationDisallowedMethodsSetting?: Maybe; /** Sets whether two factor authentication is required for all users in an enterprise. */ @@ -12714,8 +12629,6 @@ export type Mutation = { updateSponsorshipPreferences?: Maybe; /** Updates the state for subscribable subjects. */ updateSubscription?: Maybe; - /** Updates a team discussion. */ - updateTeamDiscussion?: Maybe; /** Updates team review assignment. */ updateTeamReviewAssignment?: Maybe; /** Update team repository. */ @@ -13181,12 +13094,6 @@ export type MutationCreateSponsorshipsArgs = { }; -/** The root query for implementing GraphQL mutations. */ -export type MutationCreateTeamDiscussionArgs = { - input: CreateTeamDiscussionInput; -}; - - /** The root query for implementing GraphQL mutations. */ export type MutationCreateUserListArgs = { input: CreateUserListInput; @@ -13349,12 +13256,6 @@ export type MutationDeleteRepositoryRulesetArgs = { }; -/** The root query for implementing GraphQL mutations. */ -export type MutationDeleteTeamDiscussionArgs = { - input: DeleteTeamDiscussionInput; -}; - - /** The root query for implementing GraphQL mutations. */ export type MutationDeleteUserListArgs = { input: DeleteUserListInput; @@ -13997,12 +13898,6 @@ export type MutationUpdateEnterpriseRepositoryProjectsSettingArgs = { }; -/** The root query for implementing GraphQL mutations. */ -export type MutationUpdateEnterpriseTeamDiscussionsSettingArgs = { - input: UpdateEnterpriseTeamDiscussionsSettingInput; -}; - - /** The root query for implementing GraphQL mutations. */ export type MutationUpdateEnterpriseTwoFactorAuthenticationDisallowedMethodsSettingArgs = { input: UpdateEnterpriseTwoFactorAuthenticationDisallowedMethodsSettingInput; @@ -14225,12 +14120,6 @@ export type MutationUpdateSubscriptionArgs = { }; -/** The root query for implementing GraphQL mutations. */ -export type MutationUpdateTeamDiscussionArgs = { - input: UpdateTeamDiscussionInput; -}; - - /** The root query for implementing GraphQL mutations. */ export type MutationUpdateTeamReviewAssignmentArgs = { input: UpdateTeamReviewAssignmentInput; @@ -31254,14 +31143,6 @@ export type Team = MemberStatusable & Node & Subscribable & { databaseId?: Maybe; /** The description of the team. */ description?: Maybe; - /** Find a team discussion by its number. */ - discussion?: Maybe; - /** A list of team discussions. */ - discussions: TeamDiscussionConnection; - /** The HTTP path for team discussions */ - discussionsResourcePath: Scalars['URI']['output']; - /** The HTTP URL for team discussions */ - discussionsUrl: Scalars['URI']['output']; /** The HTTP path for editing this team */ editTeamResourcePath: Scalars['URI']['output']; /** The HTTP URL for editing this team */ @@ -31358,23 +31239,6 @@ export type TeamChildTeamsArgs = { }; -/** A team of users in an organization. */ -export type TeamDiscussionArgs = { - number: Scalars['Int']['input']; -}; - - -/** A team of users in an organization. */ -export type TeamDiscussionsArgs = { - after?: InputMaybe; - before?: InputMaybe; - first?: InputMaybe; - isPinned?: InputMaybe; - last?: InputMaybe; - orderBy?: InputMaybe; -}; - - /** A team of users in an organization. */ export type TeamInvitationsArgs = { after?: InputMaybe; @@ -31824,172 +31688,6 @@ export type TeamConnection = { totalCount: Scalars['Int']['output']; }; -/** A team discussion. */ -export type TeamDiscussion = Comment & Deletable & Node & Reactable & Subscribable & UniformResourceLocatable & Updatable & UpdatableComment & { - __typename?: 'TeamDiscussion'; - /** The actor who authored the comment. */ - author?: Maybe; - /** - * Author's association with the discussion's team. - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - authorAssociation: CommentAuthorAssociation; - /** The body as Markdown. */ - body: Scalars['String']['output']; - /** The body rendered to HTML. */ - bodyHTML: Scalars['HTML']['output']; - /** The body rendered to text. */ - bodyText: Scalars['String']['output']; - /** - * Identifies the discussion body hash. - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - bodyVersion: Scalars['String']['output']; - /** - * The HTTP path for discussion comments - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - commentsResourcePath: Scalars['URI']['output']; - /** - * The HTTP URL for discussion comments - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - commentsUrl: Scalars['URI']['output']; - /** Identifies the date and time when the object was created. */ - createdAt: Scalars['DateTime']['output']; - /** Check if this comment was created via an email reply. */ - createdViaEmail: Scalars['Boolean']['output']; - /** Identifies the primary key from the database. */ - databaseId?: Maybe; - /** The actor who edited the comment. */ - editor?: Maybe; - /** The Node ID of the TeamDiscussion object */ - id: Scalars['ID']['output']; - /** Check if this comment was edited and includes an edit with the creation data */ - includesCreatedEdit: Scalars['Boolean']['output']; - /** - * Whether or not the discussion is pinned. - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - isPinned: Scalars['Boolean']['output']; - /** - * Whether or not the discussion is only visible to team members and organization owners. - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - isPrivate: Scalars['Boolean']['output']; - /** The moment the editor made the last edit */ - lastEditedAt?: Maybe; - /** - * Identifies the discussion within its team. - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - number: Scalars['Int']['output']; - /** Identifies when the comment was published at. */ - publishedAt?: Maybe; - /** A list of reactions grouped by content left on the subject. */ - reactionGroups?: Maybe>; - /** A list of Reactions left on the Issue. */ - reactions: ReactionConnection; - /** - * The HTTP path for this discussion - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - resourcePath: Scalars['URI']['output']; - /** - * The team that defines the context of this discussion. - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - team: Team; - /** - * The title of the discussion - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - title: Scalars['String']['output']; - /** Identifies the date and time when the object was last updated. */ - updatedAt: Scalars['DateTime']['output']; - /** - * The HTTP URL for this discussion - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - url: Scalars['URI']['output']; - /** A list of edits to this content. */ - userContentEdits?: Maybe; - /** Check if the current viewer can delete this object. */ - viewerCanDelete: Scalars['Boolean']['output']; - /** - * Whether or not the current viewer can pin this discussion. - * @deprecated The Team Discussions feature is deprecated in favor of Organization Discussions. Follow the guide at https://github.blog/changelog/2023-02-08-sunset-notice-team-discussions/ to find a suitable replacement. Removal on 2024-07-01 UTC. - */ - viewerCanPin: Scalars['Boolean']['output']; - /** Can user react to this subject */ - viewerCanReact: Scalars['Boolean']['output']; - /** Check if the viewer is able to change their subscription status for the repository. */ - viewerCanSubscribe: Scalars['Boolean']['output']; - /** Check if the current viewer can update this object. */ - viewerCanUpdate: Scalars['Boolean']['output']; - /** Reasons why the current viewer can not update this comment. */ - viewerCannotUpdateReasons: Array; - /** Did the viewer author this comment. */ - viewerDidAuthor: Scalars['Boolean']['output']; - /** Identifies if the viewer is watching, not watching, or ignoring the subscribable entity. */ - viewerSubscription?: Maybe; -}; - - -/** A team discussion. */ -export type TeamDiscussionReactionsArgs = { - after?: InputMaybe; - before?: InputMaybe; - content?: InputMaybe; - first?: InputMaybe; - last?: InputMaybe; - orderBy?: InputMaybe; -}; - - -/** A team discussion. */ -export type TeamDiscussionUserContentEditsArgs = { - after?: InputMaybe; - before?: InputMaybe; - first?: InputMaybe; - last?: InputMaybe; -}; - -/** The connection type for TeamDiscussion. */ -export type TeamDiscussionConnection = { - __typename?: 'TeamDiscussionConnection'; - /** A list of edges. */ - edges?: Maybe>>; - /** A list of nodes. */ - nodes?: Maybe>>; - /** Information to aid in pagination. */ - pageInfo: PageInfo; - /** Identifies the total count of items in the connection. */ - totalCount: Scalars['Int']['output']; -}; - -/** An edge in a connection. */ -export type TeamDiscussionEdge = { - __typename?: 'TeamDiscussionEdge'; - /** A cursor for use in pagination. */ - cursor: Scalars['String']['output']; - /** The item at the end of the edge. */ - node?: Maybe; -}; - -/** Ways in which team discussion connections can be ordered. */ -export type TeamDiscussionOrder = { - /** The direction in which to order nodes. */ - direction: OrderDirection; - /** The field by which to order nodes. */ - field: TeamDiscussionOrderField; -}; - -/** Properties by which team discussion connections can be ordered. */ -export type TeamDiscussionOrderField = - /** Allows chronological ordering of team discussions. */ - | 'CREATED_AT'; - /** An edge in a connection. */ export type TeamEdge = { __typename?: 'TeamEdge'; @@ -33543,27 +33241,6 @@ export type UpdateEnterpriseRepositoryProjectsSettingPayload = { message?: Maybe; }; -/** Autogenerated input type of UpdateEnterpriseTeamDiscussionsSetting */ -export type UpdateEnterpriseTeamDiscussionsSettingInput = { - /** A unique identifier for the client performing the mutation. */ - clientMutationId?: InputMaybe; - /** The ID of the enterprise on which to set the team discussions setting. */ - enterpriseId: Scalars['ID']['input']; - /** The value for the team discussions setting on the enterprise. */ - settingValue: EnterpriseEnabledDisabledSettingValue; -}; - -/** Autogenerated return type of UpdateEnterpriseTeamDiscussionsSetting. */ -export type UpdateEnterpriseTeamDiscussionsSettingPayload = { - __typename?: 'UpdateEnterpriseTeamDiscussionsSettingPayload'; - /** A unique identifier for the client performing the mutation. */ - clientMutationId?: Maybe; - /** The enterprise with the updated team discussions setting. */ - enterprise?: Maybe; - /** A message confirming the result of updating the team discussions setting. */ - message?: Maybe; -}; - /** Autogenerated input type of UpdateEnterpriseTwoFactorAuthenticationDisallowedMethodsSetting */ export type UpdateEnterpriseTwoFactorAuthenticationDisallowedMethodsSettingInput = { /** A unique identifier for the client performing the mutation. */ @@ -34446,31 +34123,6 @@ export type UpdateSubscriptionPayload = { subscribable?: Maybe; }; -/** Autogenerated input type of UpdateTeamDiscussion */ -export type UpdateTeamDiscussionInput = { - /** The updated text of the discussion. */ - body?: InputMaybe; - /** The current version of the body content. If provided, this update operation will be rejected if the given version does not match the latest version on the server. */ - bodyVersion?: InputMaybe; - /** A unique identifier for the client performing the mutation. */ - clientMutationId?: InputMaybe; - /** The Node ID of the discussion to modify. */ - id: Scalars['ID']['input']; - /** If provided, sets the pinned state of the updated discussion. */ - pinned?: InputMaybe; - /** The updated title of the discussion. */ - title?: InputMaybe; -}; - -/** Autogenerated return type of UpdateTeamDiscussion. */ -export type UpdateTeamDiscussionPayload = { - __typename?: 'UpdateTeamDiscussionPayload'; - /** A unique identifier for the client performing the mutation. */ - clientMutationId?: Maybe; - /** The updated discussion. */ - teamDiscussion?: Maybe; -}; - /** Autogenerated input type of UpdateTeamReviewAssignment */ export type UpdateTeamReviewAssignmentInput = { /** The algorithm to use for review assignment */ @@ -35892,7 +35544,7 @@ export type WorkflowsParametersInput = { workflows: Array; }; -export type _Entity = Issue; +export type _Entity = Issue | Repository; type AuthorFields_Bot_Fragment = { __typename?: 'Bot', login: string, htmlUrl: any, avatarUrl: any, type: 'Bot' }; diff --git a/src/renderer/utils/api/request.test.ts b/src/renderer/utils/api/request.test.ts index dce087d4b..9c2882d65 100644 --- a/src/renderer/utils/api/request.test.ts +++ b/src/renderer/utils/api/request.test.ts @@ -2,6 +2,7 @@ import axios from 'axios'; import { mockToken } from '../../__mocks__/state-mocks'; import type { Link } from '../../types'; +import * as rendererLogger from '../logger'; import { mockAuthHeaders, mockNoAuthHeaders, @@ -9,11 +10,11 @@ import { } from './__mocks__/request-mocks'; import { FetchAuthenticatedUserDetailsDocument } from './graphql/generated/graphql'; import { - apiRequest, - apiRequestAuth, getHeaders, + performAuthenticatedRESTRequest, performGraphQLRequest, performGraphQLRequestString, + performUnauthenticatedRESTRequest, shouldRequestWithNoCache, } from './request'; @@ -31,7 +32,7 @@ describe('renderer/utils/api/request.ts', () => { it('should make a request with the correct parameters', async () => { const data = { key: 'value' }; - await apiRequest(url, method, data); + await performUnauthenticatedRESTRequest(method, url, data); expect(axios).toHaveBeenCalledWith({ method, @@ -43,7 +44,7 @@ describe('renderer/utils/api/request.ts', () => { it('should make a request with the correct parameters and default data', async () => { const data = {}; - await apiRequest(url, method); + await performUnauthenticatedRESTRequest(method, url); expect(axios).toHaveBeenCalledWith({ method, @@ -62,7 +63,7 @@ describe('renderer/utils/api/request.ts', () => { it('should make an authenticated request with the correct parameters', async () => { const data = { key: 'value' }; - await apiRequestAuth(url, method, mockToken, data); + await performAuthenticatedRESTRequest(method, url, mockToken, data); expect(axios).toHaveBeenCalledWith({ method, @@ -75,7 +76,7 @@ describe('renderer/utils/api/request.ts', () => { it('should make an authenticated request with the correct parameters and default data', async () => { const data = {}; - await apiRequestAuth(url, method, mockToken); + await performAuthenticatedRESTRequest(method, url, mockToken); expect(axios).toHaveBeenCalledWith({ method, @@ -110,6 +111,50 @@ describe('renderer/utils/api/request.ts', () => { headers: mockAuthHeaders, }); }); + + it('throws on non-2xx HTTP status', async () => { + const logSpy = jest + .spyOn(rendererLogger, 'rendererLogError') + .mockImplementation(); + + (axios as unknown as jest.Mock).mockResolvedValue({ + status: 500, + data: { data: {}, errors: [] }, + headers: {}, + }); + + await expect( + performGraphQLRequest( + url, + mockToken, + FetchAuthenticatedUserDetailsDocument, + ), + ).rejects.toThrow('HTTP error 500'); + + expect(logSpy).toHaveBeenCalled(); + }); + + it('throws when GraphQL errors are present', async () => { + const logSpy = jest + .spyOn(rendererLogger, 'rendererLogError') + .mockImplementation(); + + (axios as unknown as jest.Mock).mockResolvedValue({ + status: 200, + data: { data: {}, errors: [{ message: 'boom' }] }, + headers: {}, + }); + + await expect( + performGraphQLRequest( + url, + mockToken, + FetchAuthenticatedUserDetailsDocument, + ), + ).rejects.toThrow('GraphQL request returned errors'); + + expect(logSpy).toHaveBeenCalled(); + }); }); describe('performGraphQLRequestString', () => { @@ -133,26 +178,136 @@ describe('renderer/utils/api/request.ts', () => { headers: mockAuthHeaders, }); }); + + it('throws on non-2xx HTTP status', async () => { + const logSpy = jest + .spyOn(rendererLogger, 'rendererLogError') + .mockImplementation(); + + const queryString = 'query Foo { repository { issue { title } } }'; + + (axios as unknown as jest.Mock).mockResolvedValue({ + status: 502, + data: { data: {}, errors: [] }, + headers: {}, + }); + + await expect( + performGraphQLRequestString(url, mockToken, queryString), + ).rejects.toThrow('HTTP error 502'); + + expect(logSpy).toHaveBeenCalled(); + }); + + it('throws when GraphQL errors are present', async () => { + const logSpy = jest + .spyOn(rendererLogger, 'rendererLogError') + .mockImplementation(); + + const queryString = 'query Foo { repository { issue { title } } }'; + + (axios as unknown as jest.Mock).mockResolvedValue({ + status: 200, + data: { data: {}, errors: [{ message: 'graphql-error' }] }, + headers: {}, + }); + + await expect( + performGraphQLRequestString(url, mockToken, queryString), + ).rejects.toThrow('GraphQL request returned errors'); + + expect(logSpy).toHaveBeenCalled(); + }); + }); + + describe('performAuthenticatedRESTRequest pagination', () => { + it('combines paginated results on success', async () => { + // First page -> next link + (axios as unknown as jest.Mock) + .mockResolvedValueOnce({ + status: 200, + data: [1, 2], + headers: { + link: '; rel="next"', + }, + }) + .mockResolvedValueOnce({ + status: 200, + data: [3], + headers: { + link: undefined, + }, + }); + + const result = await performAuthenticatedRESTRequest( + method, + url, + mockToken, + {}, + true, + ); + + expect(result).toEqual([1, 2, 3]); + }); + + it('throws on non-2xx status in pagination', async () => { + const logSpy = jest + .spyOn(rendererLogger, 'rendererLogError') + .mockImplementation(); + + // First page ok, second page error + (axios as unknown as jest.Mock) + .mockResolvedValueOnce({ + status: 200, + data: [1], + headers: { + link: '; rel="next"', + }, + }) + .mockResolvedValueOnce({ + status: 500, + data: [], + headers: { + link: undefined, + }, + }); + + await expect( + performAuthenticatedRESTRequest( + method, + url, + mockToken, + {}, + true, + ), + ).rejects.toThrow('HTTP error 500'); + + expect(logSpy).toHaveBeenCalled(); + }); }); describe('shouldRequestWithNoCache', () => { it('shouldRequestWithNoCache', () => { expect( - shouldRequestWithNoCache('https://example.com/api/v3/notifications'), + shouldRequestWithNoCache( + 'https://example.com/api/v3/notifications' as Link, + ), ).toBe(true); expect( shouldRequestWithNoCache( - 'https://example.com/login/oauth/access_token', + 'https://example.com/login/oauth/access_token' as Link, ), ).toBe(true); expect( - shouldRequestWithNoCache('https://example.com/notifications'), + shouldRequestWithNoCache('https://example.com/notifications' as Link), ).toBe(true); expect( - shouldRequestWithNoCache('https://example.com/some/other/endpoint'), + shouldRequestWithNoCache( + 'https://example.com/some/other/endpoint' as Link, + ), ).toBe(false); }); }); diff --git a/src/renderer/utils/api/request.ts b/src/renderer/utils/api/request.ts index 201f4fe2a..ac2b63fe2 100644 --- a/src/renderer/utils/api/request.ts +++ b/src/renderer/utils/api/request.ts @@ -1,66 +1,64 @@ -import axios, { - type AxiosPromise, - type AxiosResponse, - type Method, -} from 'axios'; +import axios, { type AxiosResponse, type Method } from 'axios'; import type { ExecutionResult } from 'graphql'; import type { Link, Token } from '../../types'; import { decryptValue } from '../comms'; import { rendererLogError } from '../logger'; +import { assertNoGraphQLErrors } from './errors'; import type { TypedDocumentString } from './graphql/generated/graphql'; +import type { GitHubGraphQLResponse } from './types'; import { getNextURLFromLinkHeader } from './utils'; /** - * ExecutionResult with HTTP response headers - */ -export type ExecutionResultWithHeaders = ExecutionResult & { - headers: Record; -}; - -/** - * Perform an unauthenticated API request + * Perform an unauthenticated REST API request * - * @param url - * @param method - * @param data - * @returns + * @param method The REST http method + * @param url The API url + * @param data The API request body + * @returns Resolves to a GitHub REST response */ -export async function apiRequest( - url: Link, +export async function performUnauthenticatedRESTRequest( method: Method, - data = {}, -): Promise { + url: Link, + data: Record = {}, +): Promise { const headers = await getHeaders(url); - return axios({ method, url, data, headers }); + return axios({ + method, + url, + data, + headers, + }).then((response) => { + return response.data; + }) as Promise; } /** - * Perform an authenticated API request + * Perform an authenticated REST API request * - * @param url - * @param method - * @param token - * @param data - * @param fetchAllRecords whether to fetch all records or just the first page - * @returns + * @param method The REST http method + * @param url The API url + * @param token A GitHub token (decrypted) + * @param data The API request body + * @param fetchAllRecords Whether to fetch all records or just the first page + * @returns Resolves to a GitHub REST response */ -export async function apiRequestAuth( - url: Link, +export async function performAuthenticatedRESTRequest( method: Method, + url: Link, token: Token, - data = {}, + data: Record = {}, fetchAllRecords = false, -): AxiosPromise | null { +): Promise { const headers = await getHeaders(url, token); if (!fetchAllRecords) { return axios({ method, url, data, headers }); } - let response: AxiosResponse | null = null; - let combinedData = []; + let response: AxiosResponse | null = null; + let combinedData: unknown[] = []; try { let nextUrl: string | null = url; @@ -68,34 +66,48 @@ export async function apiRequestAuth( while (nextUrl) { response = await axios({ method, url: nextUrl, data, headers }); + // Ensure HTTP status indicates success + if (response.status < 200 || response.status >= 300) { + const err = new Error(`HTTP error ${response.status}`); + rendererLogError( + 'performAuthenticatedRESTRequest', + `HTTP error ${response.status} for ${nextUrl}`, + err, + ); + throw err; + } + // If no data is returned, break the loop if (!response?.data) { break; } - combinedData = combinedData.concat(response.data); // Accumulate data + // Accumulate paginated array results + combinedData = combinedData.concat(response.data as unknown as unknown[]); nextUrl = getNextURLFromLinkHeader(response); } } catch (err) { - rendererLogError('apiRequestAuth', 'API request failed:', err); + rendererLogError( + 'performAuthenticatedRESTRequest', + 'API request failed:', + err, + ); throw err; } - return { - ...response, - data: combinedData, - } as AxiosResponse; + // Return the combined array of records as the result data + return combinedData as TResult; } /** - * Perform a GraphQL API request for account + * Perform a GraphQL API request with typed operation document * - * @param account - * @param query - * @param variables - * @returns + * @param url The API url + * @param query The GraphQL operation/query statement + * @param variables The GraphQL operation variables + * @returns Resolves to a GitHub GraphQL response */ export async function performGraphQLRequest( url: Link, @@ -114,24 +126,36 @@ export async function performGraphQLRequest( }, headers: headers, }).then((response) => { + // Check GraphQL errors + assertNoGraphQLErrors( + 'performGraphQLRequest', + response.data as ExecutionResult, + ); + return { ...response.data, headers: response.headers, }; - }) as Promise>; + }) as Promise>; } /** * Perform a GraphQL API request using a raw query string instead of a TypedDocumentString. * - * Useful for dynamically composed queries (e.g., merged queries built at runtime). + * Useful for dynamically composed queries (e.g: merged queries built at runtime). + * + * @param url The API url + * @param token The GitHub token (decrypted) + * @param query The GraphQL operation/query statement + * @param variables The GraphQL operation variables + * @returns Resolves to a GitHub GraphQL response */ export async function performGraphQLRequestString( url: Link, token: Token, query: string, variables?: Record, -): Promise> { +): Promise> { const headers = await getHeaders(url, token); return axios({ @@ -143,20 +167,27 @@ export async function performGraphQLRequestString( }, headers: headers, }).then((response) => { + // Check GraphQL errors + assertNoGraphQLErrors( + 'performGraphQLRequestString', + response.data as ExecutionResult, + ); + return { ...response.data, headers: response.headers, - } as ExecutionResultWithHeaders; + } as GitHubGraphQLResponse; }); } /** - * Return true if the request should be made with no-cache + * Determine if the API request should be made with no-cache header + * based on the API url path * - * @param url - * @returns boolean + * @param url The API url + * @returns Whether a cache heading should be set or not */ -export function shouldRequestWithNoCache(url: string) { +export function shouldRequestWithNoCache(url: Link) { const parsedUrl = new URL(url); switch (parsedUrl.pathname) { @@ -172,9 +203,9 @@ export function shouldRequestWithNoCache(url: string) { /** * Construct headers for API requests * - * @param username - * @param token - * @returns + * @param username A GitHub account username + * @param token A GitHub token (decrypted) + * @returns A headers object to use with API requests */ export async function getHeaders(url: Link, token?: Token) { const headers: Record = { diff --git a/src/renderer/utils/api/types.ts b/src/renderer/utils/api/types.ts index 2ada18d0b..18bb68b32 100644 --- a/src/renderer/utils/api/types.ts +++ b/src/renderer/utils/api/types.ts @@ -1,7 +1,18 @@ import type { components } from '@octokit/openapi-types'; +import type { ExecutionResult } from 'graphql'; import type { Link } from '../../types'; +/** + * GitHub GraphQL API response type with HTTP response headers + */ +export type GitHubGraphQLResponse = ExecutionResult & { + headers: Record; +}; + +/** + * GitHub REST Error + */ export interface GitHubRESTError { message: string; documentation_url: Link; @@ -19,3 +30,19 @@ export type RawGitHubNotification = components['schemas']['thread']; export type RawRelease = components['schemas']['release']; export type RawUser = components['schemas']['simple-user']; + +/** + * Minimal response for endpoints where we're only interested in the `html_url`. + * + * Used when following a notification thread's subject URL or latest comment URL. + */ +export type GitHubHtmlUrlResponse = { + html_url: string; +}; + +/** + * Minimal response for login/oauth/access_token + */ +export type LoginOAuthResponse = { + access_token: string; +}; diff --git a/src/renderer/utils/auth/utils.test.ts b/src/renderer/utils/auth/utils.test.ts index e008b078d..36956ca18 100644 --- a/src/renderer/utils/auth/utils.test.ts +++ b/src/renderer/utils/auth/utils.test.ts @@ -115,10 +115,13 @@ describe('renderer/utils/auth/utils.ts', () => { describe('getToken', () => { const authCode = '123-456' as AuthCode; - const apiRequestSpy = jest.spyOn(apiRequests, 'apiRequest'); + const performUnauthenticatedRESTRequestSpy = jest.spyOn( + apiRequests, + 'performUnauthenticatedRESTRequest', + ); it('should get a token', async () => { - apiRequestSpy.mockResolvedValueOnce( + performUnauthenticatedRESTRequestSpy.mockResolvedValueOnce( Promise.resolve({ data: { access_token: 'this-is-a-token' }, } as AxiosResponse), @@ -126,7 +129,9 @@ describe('renderer/utils/auth/utils.ts', () => { const res = await authUtils.getToken(authCode); - expect(apiRequests.apiRequest).toHaveBeenCalledWith( + expect( + apiRequests.performUnauthenticatedRESTRequest, + ).toHaveBeenCalledWith( 'https://github.com/login/oauth/access_token', 'POST', { diff --git a/src/renderer/utils/auth/utils.ts b/src/renderer/utils/auth/utils.ts index 3e894568f..ccabe2349 100644 --- a/src/renderer/utils/auth/utils.ts +++ b/src/renderer/utils/auth/utils.ts @@ -13,8 +13,10 @@ import type { Link, Token, } from '../../types'; -import { fetchAuthenticatedUserDetails } from '../api/client'; -import { apiRequest } from '../api/request'; +import { + exchangeAuthCodeForAccessToken, + fetchAuthenticatedUserDetails, +} from '../api/client'; import { encryptValue, openExternalLink } from '../comms'; import { getPlatformFromHostname } from '../helpers'; import { rendererLogError, rendererLogInfo, rendererLogWarn } from '../logger'; @@ -75,18 +77,16 @@ export async function getToken( authCode: AuthCode, authOptions = Constants.DEFAULT_AUTH_OPTIONS, ): Promise { - const url = - `https://${authOptions.hostname}/login/oauth/access_token` as Link; - const data = { - client_id: authOptions.clientId, - client_secret: authOptions.clientSecret, - code: authCode, - }; + const response = await exchangeAuthCodeForAccessToken( + authOptions.hostname, + authOptions.clientId, + authOptions.clientSecret, + authCode, + ); - const response = await apiRequest(url, 'POST', data); return { hostname: authOptions.hostname, - token: response.data.access_token, + token: response.access_token as Token, }; }