Skip to content

Commit 04222d4

Browse files
setchyadufr
authored andcommitted
refactor: use state for inferred checksuite status (gitify-app#848)
* refactor: use state to pass checksuite status around * refactor tests
1 parent c3c4a7d commit 04222d4

File tree

6 files changed

+150
-70
lines changed

6 files changed

+150
-70
lines changed

src/hooks/useNotifications.test.ts

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,8 @@ describe('hooks/useNotifications.ts', () => {
192192
{
193193
id: 1,
194194
subject: {
195-
title: 'This is a Discussion.',
196-
type: 'Discussion',
195+
title: 'This is a check suite workflow.',
196+
type: 'CheckSuite',
197197
url: 'https://api.github.com/1',
198198
},
199199
repository: {
@@ -204,25 +204,37 @@ describe('hooks/useNotifications.ts', () => {
204204
{
205205
id: 2,
206206
subject: {
207-
title: 'This is an Issue.',
208-
type: 'Issue',
207+
title: 'This is a Discussion.',
208+
type: 'Discussion',
209209
url: 'https://api.github.com/2',
210210
},
211+
repository: {
212+
full_name: 'some/repo',
213+
},
214+
updated_at: '2024-02-26T00:00:00Z',
211215
},
212216
{
213217
id: 3,
214218
subject: {
215-
title: 'This is a Pull Request.',
216-
type: 'PullRequest',
219+
title: 'This is an Issue.',
220+
type: 'Issue',
217221
url: 'https://api.github.com/3',
218222
},
219223
},
220224
{
221225
id: 4,
226+
subject: {
227+
title: 'This is a Pull Request.',
228+
type: 'PullRequest',
229+
url: 'https://api.github.com/4',
230+
},
231+
},
232+
{
233+
id: 5,
222234
subject: {
223235
title: 'This is an invitation.',
224236
type: 'RepositoryInvitation',
225-
url: 'https://api.github.com/4',
237+
url: 'https://api.github.com/5',
226238
},
227239
},
228240
];
@@ -249,18 +261,15 @@ describe('hooks/useNotifications.ts', () => {
249261
},
250262
},
251263
});
252-
nock('https://api.github.com')
253-
.get('/2')
254-
.reply(200, { state: 'closed', merged: true });
255264
nock('https://api.github.com')
256265
.get('/3')
257-
.reply(200, { state: 'closed', merged: false });
266+
.reply(200, { state: 'closed', merged: true });
258267
nock('https://api.github.com')
259268
.get('/4')
260-
.reply(200, { state: 'open', draft: false });
269+
.reply(200, { state: 'closed', merged: false });
261270
nock('https://api.github.com')
262271
.get('/5')
263-
.reply(200, { state: 'open', draft: true });
272+
.reply(200, { state: 'open', draft: false });
264273

265274
const { result } = renderHook(() => useNotifications(true));
266275

@@ -277,7 +286,7 @@ describe('hooks/useNotifications.ts', () => {
277286
expect(result.current.notifications[0].hostname).toBe('github.com');
278287
});
279288

280-
expect(result.current.notifications[0].notifications.length).toBe(4);
289+
expect(result.current.notifications[0].notifications.length).toBe(5);
281290
});
282291
});
283292
});

src/typesGithub.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export type IssueStateReasonType = 'completed' | 'not_planned' | 'reopened';
4646
export type PullRequestStateType = 'closed' | 'draft' | 'merged' | 'open';
4747

4848
export type StateType =
49+
| CheckSuiteStatus
4950
| DiscussionStateType
5051
| IssueStateType
5152
| IssueStateReasonType

src/utils/github-api.test.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,38 +29,39 @@ describe('formatReason', () => {
2929
describe('getNotificationTypeIcon', () => {
3030
it('should get the notification type icon', () => {
3131
expect(
32-
getNotificationTypeIcon(createSubjectMock({ type: 'CheckSuite' }))
33-
.displayName,
32+
getNotificationTypeIcon(
33+
createSubjectMock({ type: 'CheckSuite', state: null }),
34+
).displayName,
3435
).toBe('SyncIcon');
3536
expect(
3637
getNotificationTypeIcon(
3738
createSubjectMock({
3839
type: 'CheckSuite',
39-
title: 'Workflow cancelled for main branch',
40+
state: 'cancelled',
4041
}),
4142
).displayName,
4243
).toBe('StopIcon');
4344
expect(
4445
getNotificationTypeIcon(
4546
createSubjectMock({
4647
type: 'CheckSuite',
47-
title: 'Workflow failed for main branch',
48+
state: 'failure',
4849
}),
4950
).displayName,
5051
).toBe('XIcon');
5152
expect(
5253
getNotificationTypeIcon(
5354
createSubjectMock({
5455
type: 'CheckSuite',
55-
title: 'Workflow skipped for main branch',
56+
state: 'skipped',
5657
}),
5758
).displayName,
5859
).toBe('SkipIcon');
5960
expect(
6061
getNotificationTypeIcon(
6162
createSubjectMock({
6263
type: 'CheckSuite',
63-
title: 'Workflow succeeded for main branch',
64+
state: 'success',
6465
}),
6566
).displayName,
6667
).toBe('CheckIcon');
@@ -195,15 +196,15 @@ describe('getNotificationTypeIconColor', () => {
195196
getNotificationTypeIconColor(
196197
createSubjectMock({
197198
type: 'CheckSuite',
198-
title: 'Workflow cancelled for main branch',
199+
state: 'cancelled',
199200
}),
200201
),
201202
).toMatchSnapshot();
202203
expect(
203204
getNotificationTypeIconColor(
204205
createSubjectMock({
205206
type: 'CheckSuite',
206-
title: 'Workflow failed for main branch',
207+
state: 'failure',
207208
}),
208209
),
209210
).toMatchSnapshot();
@@ -212,23 +213,23 @@ describe('getNotificationTypeIconColor', () => {
212213
getNotificationTypeIconColor(
213214
createSubjectMock({
214215
type: 'CheckSuite',
215-
title: 'Workflow skipped for main branch',
216+
state: 'skipped',
216217
}),
217218
),
218219
).toMatchSnapshot();
219220
expect(
220221
getNotificationTypeIconColor(
221222
createSubjectMock({
222223
type: 'CheckSuite',
223-
title: 'Workflow succeeded for main branch',
224+
state: 'success',
224225
}),
225226
),
226227
).toMatchSnapshot();
227228
expect(
228229
getNotificationTypeIconColor(
229230
createSubjectMock({
230231
type: 'CheckSuite',
231-
title: 'unknown state',
232+
state: null,
232233
}),
233234
),
234235
).toMatchSnapshot();

src/utils/github-api.ts

Lines changed: 10 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
TagIcon,
2525
XIcon,
2626
} from '@primer/octicons-react';
27-
import { CheckSuiteStatus, Reason, Subject } from '../typesGithub';
27+
import { Reason, Subject } from '../typesGithub';
2828

2929
// prettier-ignore
3030
const DESCRIPTIONS = {
@@ -92,9 +92,7 @@ export function getNotificationTypeIcon(
9292
): React.FC<OcticonProps> {
9393
switch (subject.type) {
9494
case 'CheckSuite':
95-
const checkSuiteState = inferCheckSuiteStatus(subject.title);
96-
97-
switch (checkSuiteState) {
95+
switch (subject.state) {
9896
case 'cancelled':
9997
return StopIcon;
10098
case 'failure':
@@ -158,32 +156,19 @@ export function getNotificationTypeIcon(
158156
}
159157

160158
export function getNotificationTypeIconColor(subject: Subject): string {
161-
if (subject.type === 'CheckSuite') {
162-
const checkSuiteState = inferCheckSuiteStatus(subject.title);
163-
164-
switch (checkSuiteState) {
165-
case 'cancelled':
166-
return 'text-gray-500';
167-
case 'failure':
168-
return 'text-red-500';
169-
case 'skipped':
170-
return 'text-gray-500';
171-
case 'success':
172-
return 'text-green-500';
173-
default:
174-
return 'text-gray-300';
175-
}
176-
}
177-
178159
switch (subject.state) {
179160
case 'ANSWERED':
180161
return 'text-green-500';
162+
case 'cancelled':
163+
return 'text-gray-500';
181164
case 'closed':
182165
return 'text-red-500';
183166
case 'completed':
184167
return 'text-purple-500';
185168
case 'draft':
186169
return 'text-gray-600';
170+
case 'failure':
171+
return 'text-red-500';
187172
case 'merged':
188173
return 'text-purple-500';
189174
case 'not_planned':
@@ -194,31 +179,11 @@ export function getNotificationTypeIconColor(subject: Subject): string {
194179
return 'text-green-500';
195180
case 'RESOLVED':
196181
return 'text-purple-500';
182+
case 'skipped':
183+
return 'text-gray-500';
184+
case 'success':
185+
return 'text-green-500';
197186
default:
198187
return 'text-gray-300';
199188
}
200189
}
201-
202-
export function inferCheckSuiteStatus(title: string): CheckSuiteStatus {
203-
if (title) {
204-
const lowerTitle = title.toLowerCase();
205-
206-
if (lowerTitle.includes('cancelled for')) {
207-
return 'cancelled';
208-
}
209-
210-
if (lowerTitle.includes('failed for')) {
211-
return 'failure';
212-
}
213-
214-
if (lowerTitle.includes('skipped for')) {
215-
return 'skipped';
216-
}
217-
218-
if (lowerTitle.includes('succeeded for')) {
219-
return 'success';
220-
}
221-
}
222-
223-
return null;
224-
}

src/utils/state.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import nock from 'nock';
44
import { mockAccounts } from '../__mocks__/mock-state';
55
import { mockedSingleNotification } from '../__mocks__/mockedData';
66
import {
7+
getCheckSuiteState,
78
getDiscussionState,
89
getIssueState,
910
getPullRequestState,
@@ -15,6 +16,78 @@ describe('utils/state.ts', () => {
1516
axios.defaults.adapter = 'http';
1617
});
1718

19+
describe('getCheckSuiteState', () => {
20+
it('cancelled check suite state', async () => {
21+
const mockNotification = {
22+
...mockedSingleNotification,
23+
subject: {
24+
...mockedSingleNotification.subject,
25+
title: 'Demo workflow run cancelled for main branch',
26+
},
27+
};
28+
29+
const result = getCheckSuiteState(mockNotification);
30+
31+
expect(result).toBe('cancelled');
32+
});
33+
34+
it('failed check suite state', async () => {
35+
const mockNotification = {
36+
...mockedSingleNotification,
37+
subject: {
38+
...mockedSingleNotification.subject,
39+
title: 'Demo workflow run failed for main branch',
40+
},
41+
};
42+
43+
const result = getCheckSuiteState(mockNotification);
44+
45+
expect(result).toBe('failure');
46+
});
47+
48+
it('skipped check suite state', async () => {
49+
const mockNotification = {
50+
...mockedSingleNotification,
51+
subject: {
52+
...mockedSingleNotification.subject,
53+
title: 'Demo workflow run skipped for main branch',
54+
},
55+
};
56+
57+
const result = getCheckSuiteState(mockNotification);
58+
59+
expect(result).toBe('skipped');
60+
});
61+
62+
it('successful check suite state', async () => {
63+
const mockNotification = {
64+
...mockedSingleNotification,
65+
subject: {
66+
...mockedSingleNotification.subject,
67+
title: 'Demo workflow run succeeded for main branch',
68+
},
69+
};
70+
71+
const result = getCheckSuiteState(mockNotification);
72+
73+
expect(result).toBe('success');
74+
});
75+
76+
it('unknown check suite state', async () => {
77+
const mockNotification = {
78+
...mockedSingleNotification,
79+
subject: {
80+
...mockedSingleNotification.subject,
81+
title: 'Demo workflow run for main branch',
82+
},
83+
};
84+
85+
const result = getCheckSuiteState(mockNotification);
86+
87+
expect(result).toBeNull();
88+
});
89+
});
90+
1891
describe('getDiscussionState', () => {
1992
it('answered discussion state', async () => {
2093
const mockNotification = {

0 commit comments

Comments
 (0)