Skip to content

Commit fdbbad6

Browse files
committed
refactor(handlers): use notification instead of subject
Signed-off-by: Adam Setch <[email protected]>
1 parent 30acefa commit fdbbad6

28 files changed

+155
-167
lines changed

src/renderer/__mocks__/notifications-mocks.ts

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
import type { AccountNotifications, GitifyError } from '../types';
2-
import type {
3-
Notification,
4-
StateType,
5-
Subject,
6-
SubjectType,
7-
} from '../typesGitHub';
2+
import type { Notification, StateType, SubjectType } from '../typesGitHub';
83
import {
94
mockEnterpriseNotifications,
105
mockGitHubNotifications,
@@ -36,18 +31,20 @@ export const mockSingleAccountNotifications: AccountNotifications[] = [
3631
},
3732
];
3833

39-
export function createSubjectMock(mocks: {
34+
export function mockNotificationWithSubject(mocks: {
4035
title?: string;
4136
type?: SubjectType;
4237
state?: StateType;
43-
}): Subject {
38+
}): Notification {
4439
return {
45-
title: mocks.title ?? 'Mock Subject',
46-
type: mocks.type ?? ('Unknown' as SubjectType),
47-
state: mocks.state ?? ('Unknown' as StateType),
48-
url: null,
49-
latest_comment_url: null,
50-
};
40+
subject: {
41+
title: mocks.title ?? 'Mock Subject',
42+
type: mocks.type ?? ('Unknown' as SubjectType),
43+
state: mocks.state ?? ('Unknown' as StateType),
44+
url: null,
45+
latest_comment_url: null,
46+
},
47+
} as Notification;
5148
}
5249

5350
export function mockAccountWithError(error: GitifyError): AccountNotifications {
@@ -58,7 +55,7 @@ export function mockAccountWithError(error: GitifyError): AccountNotifications {
5855
};
5956
}
6057

61-
export function createMockNotificationForRepoName(
58+
export function mockNotificationWithRepoName(
6259
id: string,
6360
repoFullName: string | null,
6461
): Notification {

src/renderer/components/notifications/NotificationRow.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ export const NotificationRow: FC<INotificationRow> = ({
6363
};
6464

6565
const handler = createNotificationHandler(notification);
66-
const NotificationIcon = handler.iconType(notification.subject);
67-
const iconColor = handler.iconColor(notification.subject);
66+
const NotificationIcon = handler.iconType(notification);
67+
const iconColor = handler.iconColor(notification);
6868
const notificationType = handler.formattedNotificationType(notification);
6969
const notificationNumber = handler.formattedNotificationNumber(notification);
7070
const notificationTitle = handler.formattedNotificationTitle(notification);

src/renderer/utils/notifications/group.test.ts

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createMockNotificationForRepoName } from '../../__mocks__/notifications-mocks';
1+
import { mockNotificationWithRepoName } from '../../__mocks__/notifications-mocks';
22
import { mockSettings } from '../../__mocks__/state-mocks';
33
import { GroupBy } from '../../types';
44
import type { Notification } from '../../typesGitHub';
@@ -24,9 +24,9 @@ describe('renderer/utils/notifications/group.ts', () => {
2424
describe('groupNotificationsByRepository', () => {
2525
it('groups notifications by repository full_name', () => {
2626
const notifications: Notification[] = [
27-
createMockNotificationForRepoName('1', 'owner/repo-a'),
28-
createMockNotificationForRepoName('2', 'owner/repo-b'),
29-
createMockNotificationForRepoName('3', 'owner/repo-a'),
27+
mockNotificationWithRepoName('1', 'owner/repo-a'),
28+
mockNotificationWithRepoName('2', 'owner/repo-b'),
29+
mockNotificationWithRepoName('3', 'owner/repo-a'),
3030
];
3131

3232
const result = groupNotificationsByRepository(notifications);
@@ -38,10 +38,10 @@ describe('renderer/utils/notifications/group.ts', () => {
3838

3939
it('preserves first-seen repository order', () => {
4040
const notifications: Notification[] = [
41-
createMockNotificationForRepoName('1', 'owner/repo-c'),
42-
createMockNotificationForRepoName('2', 'owner/repo-a'),
43-
createMockNotificationForRepoName('3', 'owner/repo-b'),
44-
createMockNotificationForRepoName('4', 'owner/repo-a'),
41+
mockNotificationWithRepoName('1', 'owner/repo-c'),
42+
mockNotificationWithRepoName('2', 'owner/repo-a'),
43+
mockNotificationWithRepoName('3', 'owner/repo-b'),
44+
mockNotificationWithRepoName('4', 'owner/repo-a'),
4545
];
4646

4747
const result = groupNotificationsByRepository(notifications);
@@ -52,9 +52,9 @@ describe('renderer/utils/notifications/group.ts', () => {
5252

5353
it('skips notifications without repository data', () => {
5454
const notifications: Notification[] = [
55-
createMockNotificationForRepoName('1', 'owner/repo-a'),
56-
createMockNotificationForRepoName('2', null),
57-
createMockNotificationForRepoName('3', 'owner/repo-a'),
55+
mockNotificationWithRepoName('1', 'owner/repo-a'),
56+
mockNotificationWithRepoName('2', null),
57+
mockNotificationWithRepoName('3', 'owner/repo-a'),
5858
];
5959

6060
const result = groupNotificationsByRepository(notifications);
@@ -73,8 +73,8 @@ describe('renderer/utils/notifications/group.ts', () => {
7373

7474
it('returns empty map when all notifications lack repository data', () => {
7575
const notifications: Notification[] = [
76-
createMockNotificationForRepoName('1', null),
77-
createMockNotificationForRepoName('2', null),
76+
mockNotificationWithRepoName('1', null),
77+
mockNotificationWithRepoName('2', null),
7878
];
7979

8080
const result = groupNotificationsByRepository(notifications);
@@ -87,10 +87,10 @@ describe('renderer/utils/notifications/group.ts', () => {
8787
it('returns repository-grouped order when groupBy is REPOSITORY', () => {
8888
const settings = { ...mockSettings, groupBy: GroupBy.REPOSITORY };
8989
const notifications: Notification[] = [
90-
createMockNotificationForRepoName('1', 'owner/repo-b'),
91-
createMockNotificationForRepoName('2', 'owner/repo-a'),
92-
createMockNotificationForRepoName('3', 'owner/repo-b'),
93-
createMockNotificationForRepoName('4', 'owner/repo-a'),
90+
mockNotificationWithRepoName('1', 'owner/repo-b'),
91+
mockNotificationWithRepoName('2', 'owner/repo-a'),
92+
mockNotificationWithRepoName('3', 'owner/repo-b'),
93+
mockNotificationWithRepoName('4', 'owner/repo-a'),
9494
];
9595

9696
const result = getFlattenedNotificationsByRepo(notifications, settings);
@@ -102,9 +102,9 @@ describe('renderer/utils/notifications/group.ts', () => {
102102
it('returns natural account order when groupBy is DATE', () => {
103103
const settings = { ...mockSettings, groupBy: GroupBy.DATE };
104104
const notifications: Notification[] = [
105-
createMockNotificationForRepoName('1', 'owner/repo-b'),
106-
createMockNotificationForRepoName('2', 'owner/repo-a'),
107-
createMockNotificationForRepoName('3', 'owner/repo-b'),
105+
mockNotificationWithRepoName('1', 'owner/repo-b'),
106+
mockNotificationWithRepoName('2', 'owner/repo-a'),
107+
mockNotificationWithRepoName('3', 'owner/repo-b'),
108108
];
109109

110110
const result = getFlattenedNotificationsByRepo(notifications, settings);
@@ -125,9 +125,9 @@ describe('renderer/utils/notifications/group.ts', () => {
125125
it('handles notifications without repository data when grouped', () => {
126126
const settings = { ...mockSettings, groupBy: GroupBy.REPOSITORY };
127127
const notifications: Notification[] = [
128-
createMockNotificationForRepoName('1', 'owner/repo-a'),
129-
createMockNotificationForRepoName('2', null),
130-
createMockNotificationForRepoName('3', 'owner/repo-a'),
128+
mockNotificationWithRepoName('1', 'owner/repo-a'),
129+
mockNotificationWithRepoName('2', null),
130+
mockNotificationWithRepoName('3', 'owner/repo-a'),
131131
];
132132

133133
const result = getFlattenedNotificationsByRepo(notifications, settings);
@@ -139,9 +139,9 @@ describe('renderer/utils/notifications/group.ts', () => {
139139
it('preserves notifications without repository data when not grouped', () => {
140140
const settings = { ...mockSettings, groupBy: GroupBy.DATE };
141141
const notifications: Notification[] = [
142-
createMockNotificationForRepoName('1', 'owner/repo-a'),
143-
createMockNotificationForRepoName('2', null),
144-
createMockNotificationForRepoName('3', 'owner/repo-a'),
142+
mockNotificationWithRepoName('1', 'owner/repo-a'),
143+
mockNotificationWithRepoName('2', null),
144+
mockNotificationWithRepoName('3', 'owner/repo-a'),
145145
];
146146

147147
const result = getFlattenedNotificationsByRepo(notifications, settings);

src/renderer/utils/notifications/handlers/checkSuite.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createSubjectMock } from '../../../__mocks__/notifications-mocks';
1+
import { mockNotificationWithSubject } from '../../../__mocks__/notifications-mocks';
22
import { partialMockNotification } from '../../../__mocks__/partial-mocks';
33
import { mockSettings } from '../../../__mocks__/state-mocks';
44
import { checkSuiteHandler, getCheckSuiteAttributes } from './checkSuite';
@@ -139,13 +139,13 @@ describe('renderer/utils/notifications/handlers/checkSuite.ts', () => {
139139
it('iconType', () => {
140140
expect(
141141
checkSuiteHandler.iconType(
142-
createSubjectMock({ type: 'CheckSuite', state: null }),
142+
mockNotificationWithSubject({ type: 'CheckSuite', state: null }),
143143
).displayName,
144144
).toBe('RocketIcon');
145145

146146
expect(
147147
checkSuiteHandler.iconType(
148-
createSubjectMock({
148+
mockNotificationWithSubject({
149149
type: 'CheckSuite',
150150
state: 'cancelled',
151151
}),
@@ -154,7 +154,7 @@ describe('renderer/utils/notifications/handlers/checkSuite.ts', () => {
154154

155155
expect(
156156
checkSuiteHandler.iconType(
157-
createSubjectMock({
157+
mockNotificationWithSubject({
158158
type: 'CheckSuite',
159159
state: 'failure',
160160
}),
@@ -163,7 +163,7 @@ describe('renderer/utils/notifications/handlers/checkSuite.ts', () => {
163163

164164
expect(
165165
checkSuiteHandler.iconType(
166-
createSubjectMock({
166+
mockNotificationWithSubject({
167167
type: 'CheckSuite',
168168
state: 'skipped',
169169
}),
@@ -172,7 +172,7 @@ describe('renderer/utils/notifications/handlers/checkSuite.ts', () => {
172172

173173
expect(
174174
checkSuiteHandler.iconType(
175-
createSubjectMock({
175+
mockNotificationWithSubject({
176176
type: 'CheckSuite',
177177
state: 'success',
178178
}),

src/renderer/utils/notifications/handlers/checkSuite.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import type {
1515
CheckSuiteStatus,
1616
GitifySubject,
1717
Notification,
18-
Subject,
1918
} from '../../../typesGitHub';
2019
import { DefaultHandler } from './default';
2120

@@ -38,8 +37,8 @@ class CheckSuiteHandler extends DefaultHandler {
3837
return null;
3938
}
4039

41-
iconType(subject: Subject): FC<OcticonProps> | null {
42-
switch (subject.state) {
40+
iconType(notification: Notification): FC<OcticonProps> | null {
41+
switch (notification.subject.state) {
4342
case 'cancelled':
4443
return StopIcon;
4544
case 'failure':

src/renderer/utils/notifications/handlers/commit.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import axios from 'axios';
22
import nock from 'nock';
33

4-
import { createSubjectMock } from '../../../__mocks__/notifications-mocks';
4+
import { mockNotificationWithSubject } from '../../../__mocks__/notifications-mocks';
55
import {
66
partialMockNotification,
77
partialMockUser,
@@ -99,7 +99,8 @@ describe('renderer/utils/notifications/handlers/commit.ts', () => {
9999

100100
it('iconType', () => {
101101
expect(
102-
commitHandler.iconType(createSubjectMock({ type: 'Commit' })).displayName,
102+
commitHandler.iconType(mockNotificationWithSubject({ type: 'Commit' }))
103+
.displayName,
103104
).toBe('GitCommitIcon');
104105
});
105106
});

src/renderer/utils/notifications/handlers/commit.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import type {
88
GitifySubject,
99
Notification,
1010
StateType,
11-
Subject,
1211
User,
1312
} from '../../../typesGitHub';
1413
import { getCommit, getCommitComment } from '../../api/client';
@@ -55,7 +54,7 @@ class CommitHandler extends DefaultHandler {
5554
};
5655
}
5756

58-
iconType(_subject: Subject): FC<OcticonProps> | null {
57+
iconType(_notification: Notification): FC<OcticonProps> | null {
5958
return GitCommitIcon;
6059
}
6160
}

src/renderer/utils/notifications/handlers/default.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createSubjectMock } from '../../../__mocks__/notifications-mocks';
1+
import { mockNotificationWithSubject } from '../../../__mocks__/notifications-mocks';
22
import { partialMockNotification } from '../../../__mocks__/partial-mocks';
33
import { mockSettings } from '../../../__mocks__/state-mocks';
44
import { IconColor } from '../../../types';
@@ -24,9 +24,9 @@ describe('renderer/utils/notifications/handlers/default.ts', () => {
2424
});
2525

2626
it('iconType', () => {
27-
expect(defaultHandler.iconType(createSubjectMock({})).displayName).toBe(
28-
'QuestionIcon',
29-
);
27+
expect(
28+
defaultHandler.iconType(mockNotificationWithSubject({})).displayName,
29+
).toBe('QuestionIcon');
3030
});
3131

3232
describe('iconColor', () => {
@@ -50,7 +50,7 @@ describe('renderer/utils/notifications/handlers/default.ts', () => {
5050
];
5151

5252
it.each(cases)('returns correct color for state %s', (state, expected) => {
53-
const subject = createSubjectMock({ state });
53+
const subject = mockNotificationWithSubject({ state });
5454
expect(defaultHandler.iconColor(subject)).toBe(expected);
5555
});
5656
});

src/renderer/utils/notifications/handlers/default.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { IconColor } from '../../../types';
88
import type {
99
GitifySubject,
1010
Notification,
11-
Subject,
1211
SubjectType,
1312
} from '../../../typesGitHub';
1413
import type { NotificationTypeHandler } from './types';
@@ -24,12 +23,12 @@ export class DefaultHandler implements NotificationTypeHandler {
2423
return null;
2524
}
2625

27-
iconType(_subject: Subject): FC<OcticonProps> | null {
26+
iconType(_notification: Notification): FC<OcticonProps> | null {
2827
return QuestionIcon;
2928
}
3029

31-
iconColor(subject: Subject): IconColor {
32-
switch (subject.state) {
30+
iconColor(notification: Notification): IconColor {
31+
switch (notification.subject.state) {
3332
case 'open':
3433
case 'reopened':
3534
case 'ANSWERED':

src/renderer/utils/notifications/handlers/discussion.test.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import axios from 'axios';
22
import nock from 'nock';
33

4-
import { createSubjectMock } from '../../../__mocks__/notifications-mocks';
4+
import { mockNotificationWithSubject } from '../../../__mocks__/notifications-mocks';
55
import { partialMockNotification } from '../../../__mocks__/partial-mocks';
66
import { mockSettings } from '../../../__mocks__/state-mocks';
77
import type { Link } from '../../../types';
@@ -281,25 +281,26 @@ describe('renderer/utils/notifications/handlers/discussion.ts', () => {
281281

282282
it('iconType', () => {
283283
expect(
284-
discussionHandler.iconType(createSubjectMock({ type: 'Discussion' }))
285-
.displayName,
284+
discussionHandler.iconType(
285+
mockNotificationWithSubject({ type: 'Discussion' }),
286+
).displayName,
286287
).toBe('CommentDiscussionIcon');
287288

288289
expect(
289290
discussionHandler.iconType(
290-
createSubjectMock({ type: 'Discussion', state: 'DUPLICATE' }),
291+
mockNotificationWithSubject({ type: 'Discussion', state: 'DUPLICATE' }),
291292
).displayName,
292293
).toBe('DiscussionDuplicateIcon');
293294

294295
expect(
295296
discussionHandler.iconType(
296-
createSubjectMock({ type: 'Discussion', state: 'OUTDATED' }),
297+
mockNotificationWithSubject({ type: 'Discussion', state: 'OUTDATED' }),
297298
).displayName,
298299
).toBe('DiscussionOutdatedIcon');
299300

300301
expect(
301302
discussionHandler.iconType(
302-
createSubjectMock({ type: 'Discussion', state: 'RESOLVED' }),
303+
mockNotificationWithSubject({ type: 'Discussion', state: 'RESOLVED' }),
303304
).displayName,
304305
).toBe('DiscussionClosedIcon');
305306
});

0 commit comments

Comments
 (0)