Skip to content

Commit 2565d21

Browse files
setchysetchy
authored
refactor(api): request and client types (#2545)
* refactor(api): request function typing --------- Signed-off-by: Adam Setch <adam.setch@outlook.com>
1 parent 048be62 commit 2565d21

File tree

12 files changed

+467
-291
lines changed

12 files changed

+467
-291
lines changed

‎src/renderer/context/App.test.tsx‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,6 @@ describe('renderer/context/App.tsx', () => {
184184
});
185185

186186
describe('authentication methods', () => {
187-
const apiRequestAuthSpy = jest.spyOn(apiRequests, 'apiRequestAuth');
188-
189187
it('should call loginWithGitHubApp', async () => {
190188
const { button } = renderContextButton('loginWithGitHubApp');
191189

@@ -207,7 +205,9 @@ describe('renderer/context/App.tsx', () => {
207205
});
208206

209207
it('should call loginWithPersonalAccessToken', async () => {
210-
apiRequestAuthSpy.mockResolvedValueOnce(null);
208+
const performAuthenticatedRESTRequestSpy = jest
209+
.spyOn(apiRequests, 'performAuthenticatedRESTRequest')
210+
.mockResolvedValueOnce(null);
211211

212212
const { button } = renderContextButton('loginWithPersonalAccessToken', {
213213
hostname: 'github.com' as Hostname,
@@ -220,10 +220,10 @@ describe('renderer/context/App.tsx', () => {
220220
expect(mockFetchNotifications).toHaveBeenCalledTimes(1),
221221
);
222222

223-
expect(apiRequestAuthSpy).toHaveBeenCalledTimes(1);
224-
expect(apiRequestAuthSpy).toHaveBeenCalledWith(
225-
'https://api.github.com/notifications',
223+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledTimes(1);
224+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith(
226225
'HEAD',
226+
'https://api.github.com/notifications',
227227
'encrypted',
228228
);
229229
});

‎src/renderer/utils/api/client.test.ts‎

Lines changed: 82 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,15 @@
1-
import axios, { type AxiosPromise, type AxiosResponse } from 'axios';
2-
31
import { mockGitHubCloudAccount } from '../../__mocks__/account-mocks';
42
import {
53
mockGitHubCloudGitifyNotifications,
64
mockPartialGitifyNotification,
75
} from '../../__mocks__/notifications-mocks';
86
import { mockToken } from '../../__mocks__/state-mocks';
9-
import {
10-
mockAuthHeaders,
11-
mockNonCachedAuthHeaders,
12-
} from './__mocks__/request-mocks';
137

148
import { Constants } from '../../constants';
159

1610
import type { Hostname, Link, SettingsState, Token } from '../../types';
11+
import type { GitHubGraphQLResponse } from './graphql/types';
1712

18-
import * as logger from '../../utils/logger';
1913
import {
2014
fetchAuthenticatedUserDetails,
2115
fetchDiscussionByNumber,
@@ -39,7 +33,6 @@ import {
3933
FetchPullRequestByNumberDocument,
4034
type FetchPullRequestByNumberQuery,
4135
} from './graphql/generated/graphql';
42-
import type { ExecutionResultWithHeaders } from './request';
4336
import * as apiRequests from './request';
4437

4538
jest.mock('axios');
@@ -48,96 +41,110 @@ const mockGitHubHostname = 'github.com' as Hostname;
4841
const mockThreadId = '1234';
4942

5043
describe('renderer/utils/api/client.ts', () => {
44+
const performAuthenticatedRESTRequestSpy = jest.spyOn(
45+
apiRequests,
46+
'performAuthenticatedRESTRequest',
47+
);
48+
5149
afterEach(() => {
5250
jest.clearAllMocks();
5351
});
5452

5553
it('headNotifications - should fetch notifications head', async () => {
5654
await headNotifications(mockGitHubHostname, mockToken);
5755

58-
expect(axios).toHaveBeenCalledWith({
59-
url: 'https://api.github.com/notifications',
60-
headers: mockNonCachedAuthHeaders,
61-
method: 'HEAD',
62-
data: {},
63-
});
56+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith(
57+
'HEAD',
58+
'https://api.github.com/notifications',
59+
mockToken,
60+
);
6461
});
6562

6663
describe('listNotificationsForAuthenticatedUser', () => {
6764
it('should list only participating notifications for user', async () => {
6865
const mockSettings: Partial<SettingsState> = {
6966
participating: true,
7067
fetchReadNotifications: false,
68+
fetchAllNotifications: false,
7169
};
7270

7371
await listNotificationsForAuthenticatedUser(
7472
mockGitHubCloudAccount,
7573
mockSettings as SettingsState,
7674
);
7775

78-
expect(axios).toHaveBeenCalledWith({
79-
url: 'https://api.github.com/notifications?participating=true&all=false',
80-
headers: mockNonCachedAuthHeaders,
81-
method: 'GET',
82-
data: {},
83-
});
76+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith(
77+
'GET',
78+
'https://api.github.com/notifications?participating=true&all=false',
79+
mockGitHubCloudAccount.token,
80+
{},
81+
false,
82+
);
8483
});
8584

8685
it('should list participating and watching notifications for user', async () => {
8786
const mockSettings: Partial<SettingsState> = {
8887
participating: false,
8988
fetchReadNotifications: false,
89+
fetchAllNotifications: false,
9090
};
9191

9292
await listNotificationsForAuthenticatedUser(
9393
mockGitHubCloudAccount,
9494
mockSettings as SettingsState,
9595
);
9696

97-
expect(axios).toHaveBeenCalledWith({
98-
url: 'https://api.github.com/notifications?participating=false&all=false',
99-
headers: mockNonCachedAuthHeaders,
100-
method: 'GET',
101-
data: {},
102-
});
97+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith(
98+
'GET',
99+
'https://api.github.com/notifications?participating=false&all=false',
100+
mockGitHubCloudAccount.token,
101+
{},
102+
false,
103+
);
103104
});
104105

105-
it('should include all=true when fetchReadNotifications is enabled', async () => {
106+
it('should list read and done notifications for user', async () => {
106107
const mockSettings: Partial<SettingsState> = {
107108
participating: false,
108109
fetchReadNotifications: true,
110+
fetchAllNotifications: false,
109111
};
110112

111113
await listNotificationsForAuthenticatedUser(
112114
mockGitHubCloudAccount,
113115
mockSettings as SettingsState,
114116
);
115117

116-
expect(axios).toHaveBeenCalledWith({
117-
url: 'https://api.github.com/notifications?participating=false&all=true',
118-
headers: mockNonCachedAuthHeaders,
119-
method: 'GET',
120-
data: {},
121-
});
118+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith(
119+
'GET',
120+
'https://api.github.com/notifications?participating=false&all=true',
121+
mockGitHubCloudAccount.token,
122+
{},
123+
false,
124+
);
122125
});
123126

124-
it('should include all=false when fetchReadNotifications is disabled', async () => {
127+
it('should unpaginate notifications list for user', async () => {
128+
performAuthenticatedRESTRequestSpy.mockImplementation();
129+
125130
const mockSettings: Partial<SettingsState> = {
126131
participating: false,
127132
fetchReadNotifications: false,
133+
fetchAllNotifications: true,
128134
};
129135

130136
await listNotificationsForAuthenticatedUser(
131137
mockGitHubCloudAccount,
132138
mockSettings as SettingsState,
133139
);
134140

135-
expect(axios).toHaveBeenCalledWith({
136-
url: 'https://api.github.com/notifications?participating=false&all=false',
137-
headers: mockNonCachedAuthHeaders,
138-
method: 'GET',
139-
data: {},
140-
});
141+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith(
142+
'GET',
143+
'https://api.github.com/notifications?participating=false&all=false',
144+
mockGitHubCloudAccount.token,
145+
{},
146+
true,
147+
);
141148
});
142149
});
143150

@@ -148,12 +155,12 @@ describe('renderer/utils/api/client.ts', () => {
148155
mockToken,
149156
);
150157

151-
expect(axios).toHaveBeenCalledWith({
152-
url: `https://api.github.com/notifications/threads/${mockThreadId}`,
153-
headers: mockAuthHeaders,
154-
method: 'PATCH',
155-
data: {},
156-
});
158+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith(
159+
'PATCH',
160+
`https://api.github.com/notifications/threads/${mockThreadId}`,
161+
mockToken,
162+
{},
163+
);
157164
});
158165

159166
it('markNotificationThreadAsDone - should mark notification thread as done', async () => {
@@ -163,12 +170,12 @@ describe('renderer/utils/api/client.ts', () => {
163170
mockToken,
164171
);
165172

166-
expect(axios).toHaveBeenCalledWith({
167-
url: `https://api.github.com/notifications/threads/${mockThreadId}`,
168-
headers: mockAuthHeaders,
169-
method: 'DELETE',
170-
data: {},
171-
});
173+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith(
174+
'DELETE',
175+
`https://api.github.com/notifications/threads/${mockThreadId}`,
176+
mockToken,
177+
{},
178+
);
172179
});
173180

174181
it('ignoreNotificationThreadSubscription - should ignore notification thread subscription', async () => {
@@ -178,54 +185,25 @@ describe('renderer/utils/api/client.ts', () => {
178185
mockToken,
179186
);
180187

181-
expect(axios).toHaveBeenCalledWith({
182-
url: `https://api.github.com/notifications/threads/${mockThreadId}/subscription`,
183-
headers: mockAuthHeaders,
184-
method: 'PUT',
185-
data: { ignored: true },
186-
});
188+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith(
189+
'PUT',
190+
`https://api.github.com/notifications/threads/${mockThreadId}/subscription`,
191+
mockToken,
192+
{ ignored: true },
193+
);
187194
});
188195

189-
describe('getHtmlUrl', () => {
190-
it('should return the HTML URL', async () => {
191-
const apiRequestAuthSpy = jest.spyOn(apiRequests, 'apiRequestAuth');
192-
193-
const requestPromise = Promise.resolve({
194-
data: {
195-
html_url:
196-
'https://github.com/gitify-app/notifications-test/issues/785',
197-
},
198-
} as AxiosResponse) as AxiosPromise;
199-
200-
apiRequestAuthSpy.mockResolvedValue(requestPromise);
201-
202-
const result = await getHtmlUrl(
203-
'https://api.github.com/repos/gitify-app/notifications-test/issues/785' as Link,
204-
'123' as Token,
205-
);
206-
expect(result).toBe(
207-
'https://github.com/gitify-app/notifications-test/issues/785',
208-
);
209-
});
210-
211-
it('should handle error', async () => {
212-
const rendererLogErrorSpy = jest
213-
.spyOn(logger, 'rendererLogError')
214-
.mockImplementation();
215-
216-
const apiRequestAuthSpy = jest.spyOn(apiRequests, 'apiRequestAuth');
217-
218-
const mockError = new Error('Test error');
219-
220-
apiRequestAuthSpy.mockRejectedValue(mockError);
221-
222-
await getHtmlUrl(
223-
'https://api.github.com/repos/gitify-app/gitify/issues/785' as Link,
224-
'123' as Token,
225-
);
196+
it('getHtmlUrl - should return the HTML URL', async () => {
197+
await getHtmlUrl(
198+
'https://api.github.com/repos/gitify-app/notifications-test/issues/785' as Link,
199+
'123' as Token,
200+
);
226201

227-
expect(rendererLogErrorSpy).toHaveBeenCalledTimes(1);
228-
});
202+
expect(performAuthenticatedRESTRequestSpy).toHaveBeenCalledWith(
203+
'GET',
204+
'https://api.github.com/repos/gitify-app/notifications-test/issues/785',
205+
'123',
206+
);
229207
});
230208

231209
it('fetchAuthenticatedUserDetails calls performGraphQLRequest with correct args', async () => {
@@ -237,7 +215,7 @@ describe('renderer/utils/api/client.ts', () => {
237215
performGraphQLRequestSpy.mockResolvedValue({
238216
data: {},
239217
headers: {},
240-
} as ExecutionResultWithHeaders<FetchAuthenticatedUserDetailsQuery>);
218+
} as GitHubGraphQLResponse<FetchAuthenticatedUserDetailsQuery>);
241219

242220
await fetchAuthenticatedUserDetails(mockGitHubHostname, mockToken);
243221

@@ -263,7 +241,7 @@ describe('renderer/utils/api/client.ts', () => {
263241
performGraphQLRequestSpy.mockResolvedValue({
264242
data: {},
265243
headers: {},
266-
} as ExecutionResultWithHeaders<FetchDiscussionByNumberQuery>);
244+
} as GitHubGraphQLResponse<FetchDiscussionByNumberQuery>);
267245

268246
await fetchDiscussionByNumber(mockNotification);
269247

@@ -298,7 +276,7 @@ describe('renderer/utils/api/client.ts', () => {
298276
performGraphQLRequestSpy.mockResolvedValue({
299277
data: {},
300278
headers: {},
301-
} as ExecutionResultWithHeaders<FetchIssueByNumberQuery>);
279+
} as GitHubGraphQLResponse<FetchIssueByNumberQuery>);
302280

303281
await fetchIssueByNumber(mockNotification);
304282

@@ -331,7 +309,7 @@ describe('renderer/utils/api/client.ts', () => {
331309
performGraphQLRequestSpy.mockResolvedValue({
332310
data: {},
333311
headers: {},
334-
} as ExecutionResultWithHeaders<FetchPullRequestByNumberQuery>);
312+
} as GitHubGraphQLResponse<FetchPullRequestByNumberQuery>);
335313

336314
await fetchPullByNumber(mockNotification);
337315

@@ -367,7 +345,7 @@ describe('renderer/utils/api/client.ts', () => {
367345
performGraphQLRequestStringSpy.mockResolvedValue({
368346
data: {},
369347
headers: {},
370-
} as ExecutionResultWithHeaders<unknown>);
348+
} as GitHubGraphQLResponse<unknown>);
371349

372350
await fetchNotificationDetailsForList([mockNotification]);
373351

@@ -383,7 +361,7 @@ describe('renderer/utils/api/client.ts', () => {
383361
performGraphQLRequestStringSpy.mockResolvedValue({
384362
data: {},
385363
headers: {},
386-
} as ExecutionResultWithHeaders<unknown>);
364+
} as GitHubGraphQLResponse<unknown>);
387365

388366
await fetchNotificationDetailsForList([]);
389367

@@ -399,7 +377,7 @@ describe('renderer/utils/api/client.ts', () => {
399377
performGraphQLRequestStringSpy.mockResolvedValue({
400378
data: {},
401379
headers: {},
402-
} as ExecutionResultWithHeaders<unknown>);
380+
} as GitHubGraphQLResponse<unknown>);
403381

404382
await fetchNotificationDetailsForList(mockGitHubCloudGitifyNotifications);
405383

0 commit comments

Comments
 (0)