Skip to content

Commit 0514a43

Browse files
adufradufr
committed
Merge remote-tracking branch 'origin/main' into feat/mark-repository-as-done
2 parents bb609b9 + 7efe0d9 commit 0514a43

16 files changed

+203
-28
lines changed

‎src/components/NotificationRow.test.tsx

+3-2
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ describe('components/Notification.js', () => {
3131
});
3232

3333
it('should open a notification in the browser', () => {
34-
const markNotification = jest.fn();
34+
const removeNotificationFromState = jest.fn();
3535

3636
const props = {
3737
notification: mockedSingleNotification,
@@ -42,7 +42,7 @@ describe('components/Notification.js', () => {
4242
<AppContext.Provider
4343
value={{
4444
settings: { ...mockSettings, markAsDoneOnOpen: false },
45-
markNotification,
45+
removeNotificationFromState,
4646
accounts: mockAccounts,
4747
}}
4848
>
@@ -52,6 +52,7 @@ describe('components/Notification.js', () => {
5252

5353
fireEvent.click(getByRole('main'));
5454
expect(shell.openExternal).toHaveBeenCalledTimes(1);
55+
expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
5556
});
5657

5758
it('should open a notification in browser & mark it as done', () => {

‎src/components/NotificationRow.tsx

+4
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export const NotificationRow: React.FC<IProps> = ({
2323
const {
2424
settings,
2525
accounts,
26+
removeNotificationFromState,
2627
markNotification,
2728
markNotificationDone,
2829
unsubscribeNotification,
@@ -33,6 +34,9 @@ export const NotificationRow: React.FC<IProps> = ({
3334

3435
if (settings.markAsDoneOnOpen) {
3536
markNotificationDone(notification.id, hostname);
37+
} else {
38+
// no need to mark as read, github does it by default when opening it
39+
removeNotificationFromState(notification.id, hostname);
3640
}
3741
}, [settings]);
3842

‎src/context/App.test.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ describe('context/App.tsx', () => {
290290
participating: true,
291291
playSound: true,
292292
showNotifications: true,
293-
colors: false,
293+
colors: null,
294294
markAsDoneOnOpen: false,
295295
},
296296
);
@@ -328,7 +328,7 @@ describe('context/App.tsx', () => {
328328
participating: false,
329329
playSound: true,
330330
showNotifications: true,
331-
colors: false,
331+
colors: null,
332332
markAsDoneOnOpen: false,
333333
},
334334
);

‎src/context/App.tsx

+4-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export const defaultSettings: SettingsState = {
3636
showNotifications: true,
3737
openAtStartup: false,
3838
appearance: Appearance.SYSTEM,
39-
colors: false,
39+
colors: null,
4040
markAsDoneOnOpen: false,
4141
};
4242

@@ -51,6 +51,7 @@ interface AppContextState {
5151
notifications: AccountNotifications[];
5252
isFetching: boolean;
5353
requestFailed: boolean;
54+
removeNotificationFromState: (id: string, hostname: string) => void;
5455
fetchNotifications: () => Promise<void>;
5556
markNotification: (id: string, hostname: string) => Promise<void>;
5657
markNotificationDone: (id: string, hostname: string) => Promise<void>;
@@ -72,6 +73,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
7273
notifications,
7374
requestFailed,
7475
isFetching,
76+
removeNotificationFromState,
7577
markNotification,
7678
markNotificationDone,
7779
unsubscribeNotification,
@@ -218,6 +220,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
218220
notifications,
219221
isFetching,
220222
requestFailed,
223+
removeNotificationFromState,
221224
fetchNotifications: fetchNotificationsWithAccounts,
222225
markNotification: markNotificationWithAccounts,
223226
markNotificationDone: markNotificationDoneWithAccounts,

‎src/hooks/useNotifications.test.ts

+36
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,42 @@ describe('hooks/useNotifications.ts', () => {
269269
});
270270
});
271271

272+
describe('removeNotificationFromState', () => {
273+
it('should remove a notification from state', async () => {
274+
const notifications = [
275+
{ id: 1, title: 'This is a notification.' },
276+
{ id: 2, title: 'This is another one.' },
277+
];
278+
279+
nock('https://api.github.com')
280+
.get('/notifications?participating=false')
281+
.reply(200, notifications);
282+
283+
nock('https://github.gitify.io/api/v3')
284+
.get('/notifications?participating=false')
285+
.reply(200, notifications);
286+
287+
const { result } = renderHook(() => useNotifications(false));
288+
289+
act(() => {
290+
result.current.fetchNotifications(mockAccounts, mockSettings);
291+
});
292+
293+
await waitFor(() => {
294+
expect(result.current.isFetching).toBe(false);
295+
});
296+
297+
act(() => {
298+
result.current.removeNotificationFromState(
299+
result.current.notifications[0].notifications[0].id,
300+
result.current.notifications[0].hostname,
301+
);
302+
});
303+
304+
expect(result.current.notifications[0].notifications.length).toBe(1);
305+
});
306+
});
307+
272308
describe('markNotification', () => {
273309
const id = 'notification-123';
274310

‎src/hooks/useNotifications.ts

+25-8
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { apiRequestAuth } from '../utils/api-requests';
88
import {
99
getEnterpriseAccountToken,
1010
generateGitHubAPIUrl,
11+
isEnterpriseHost,
1112
} from '../utils/helpers';
1213
import { removeNotification } from '../utils/remove-notification';
1314
import {
@@ -19,6 +20,7 @@ import { removeNotifications } from '../utils/remove-notifications';
1920

2021
interface NotificationsState {
2122
notifications: AccountNotifications[];
23+
removeNotificationFromState: (id: string, hostname: string) => void;
2224
fetchNotifications: (
2325
accounts: AuthState,
2426
settings: SettingsState,
@@ -109,7 +111,7 @@ export const useNotifications = (colors: boolean): NotificationsState => {
109111
]
110112
: [...enterpriseNotifications];
111113

112-
if (!colors) {
114+
if (colors === false) {
113115
setNotifications(data);
114116
triggerNativeNotifications(
115117
notifications,
@@ -134,9 +136,9 @@ export const useNotifications = (colors: boolean): NotificationsState => {
134136
) {
135137
return notification;
136138
}
137-
const isEnterprise =
138-
accountNotifications.hostname !==
139-
Constants.DEFAULT_AUTH_OPTIONS.hostname;
139+
const isEnterprise = isEnterpriseHost(
140+
accountNotifications.hostname,
141+
);
140142
const token = isEnterprise
141143
? getEnterpriseAccountToken(
142144
accountNotifications.hostname,
@@ -196,7 +198,7 @@ export const useNotifications = (colors: boolean): NotificationsState => {
196198
async (accounts, id, hostname) => {
197199
setIsFetching(true);
198200

199-
const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
201+
const isEnterprise = isEnterpriseHost(hostname);
200202
const token = isEnterprise
201203
? getEnterpriseAccountToken(hostname, accounts.enterpriseAccounts)
202204
: accounts.token;
@@ -229,7 +231,7 @@ export const useNotifications = (colors: boolean): NotificationsState => {
229231
async (accounts, id, hostname) => {
230232
setIsFetching(true);
231233

232-
const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
234+
const isEnterprise = isEnterpriseHost(hostname);
233235
const token = isEnterprise
234236
? getEnterpriseAccountToken(hostname, accounts.enterpriseAccounts)
235237
: accounts.token;
@@ -262,7 +264,7 @@ export const useNotifications = (colors: boolean): NotificationsState => {
262264
async (accounts, id, hostname) => {
263265
setIsFetching(true);
264266

265-
const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
267+
const isEnterprise = isEnterpriseHost(hostname);
266268
const token = isEnterprise
267269
? getEnterpriseAccountToken(hostname, accounts.enterpriseAccounts)
268270
: accounts.token;
@@ -288,7 +290,7 @@ export const useNotifications = (colors: boolean): NotificationsState => {
288290
async (accounts, repoSlug, hostname) => {
289291
setIsFetching(true);
290292

291-
const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
293+
const isEnterprise = isEnterpriseHost(hostname);
292294
const token = isEnterprise
293295
? getEnterpriseAccountToken(hostname, accounts.enterpriseAccounts)
294296
: accounts.token;
@@ -360,11 +362,26 @@ export const useNotifications = (colors: boolean): NotificationsState => {
360362
[notifications],
361363
);
362364

365+
const removeNotificationFromState = useCallback(
366+
(id, hostname) => {
367+
const updatedNotifications = removeNotification(
368+
id,
369+
notifications,
370+
hostname,
371+
);
372+
373+
setNotifications(updatedNotifications);
374+
setTrayIconColor(updatedNotifications);
375+
},
376+
[notifications],
377+
);
378+
363379
return {
364380
isFetching,
365381
requestFailed,
366382
notifications,
367383

384+
removeNotificationFromState,
368385
fetchNotifications,
369386
markNotification,
370387
markNotificationDone,

‎src/types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export interface SettingsState {
1212
showNotifications: boolean;
1313
openAtStartup: boolean;
1414
appearance: Appearance;
15-
colors: boolean;
15+
colors: boolean | null;
1616
markAsDoneOnOpen: boolean;
1717
}
1818

‎src/typesGithub.ts

+3
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,11 @@ export type IssueStateType =
2828
| 'completed'
2929
| 'reopened'
3030
| 'not_planned';
31+
3132
export type PullRequestStateType = 'closed' | 'open' | 'merged' | 'draft';
33+
3234
export type StateType = IssueStateType | PullRequestStateType;
35+
3336
export type ViewerSubscription = 'IGNORED' | 'SUBSCRIBED' | 'UNSUBSCRIBED';
3437

3538
export interface Notification {

‎src/utils/__snapshots__/api-requests.test.ts.snap

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`apiRequest should make a request with the correct parameters 1`] = `
4+
{
5+
"Accept": "application/json",
6+
"Cache-Control": "no-cache",
7+
"Content-Type": "application/json",
8+
}
9+
`;
10+
11+
exports[`apiRequestAuth should make an authenticated request with the correct parameters 1`] = `
12+
{
13+
"Accept": "application/json",
14+
"Authorization": "token yourAuthToken",
15+
"Cache-Control": "no-cache",
16+
"Content-Type": "application/json",
17+
}
18+
`;

‎src/utils/__snapshots__/github-api.test.ts.snap

+21
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,26 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`./utils/github-api.ts should format the notification color 1`] = `"text-red-500"`;
4+
5+
exports[`./utils/github-api.ts should format the notification color 2`] = `"text-purple-500"`;
6+
7+
exports[`./utils/github-api.ts should format the notification color 3`] = `"text-gray-600"`;
8+
9+
exports[`./utils/github-api.ts should format the notification color 4`] = `"text-purple-500"`;
10+
11+
exports[`./utils/github-api.ts should format the notification color 5`] = `"text-gray-300"`;
12+
13+
exports[`./utils/github-api.ts should format the notification color 6`] = `"text-green-500"`;
14+
15+
exports[`./utils/github-api.ts should format the notification color 7`] = `"text-green-500"`;
16+
17+
exports[`./utils/github-api.ts should format the notification color 8`] = `
18+
{
19+
"description": "The reason for this notification is not supported by the app.",
20+
"type": "Unknown",
21+
}
22+
`;
23+
324
exports[`./utils/github-api.ts should format the notification reason 1`] = `
425
{
526
"description": "You were assigned to the issue.",

‎src/utils/api-requests.test.ts

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import axios from 'axios';
2+
import { apiRequest, apiRequestAuth } from './api-requests';
3+
4+
jest.mock('axios');
5+
6+
describe('apiRequest', () => {
7+
it('should make a request with the correct parameters', async () => {
8+
const url = 'https://example.com';
9+
const method = 'get';
10+
const data = { key: 'value' };
11+
12+
await apiRequest(url, method, data);
13+
14+
expect(axios).toHaveBeenCalledWith({
15+
method,
16+
url,
17+
data,
18+
});
19+
20+
expect(axios.defaults.headers.common).toMatchSnapshot();
21+
});
22+
});
23+
24+
describe('apiRequestAuth', () => {
25+
it('should make an authenticated request with the correct parameters', async () => {
26+
const url = 'https://example.com';
27+
const method = 'get';
28+
const token = 'yourAuthToken';
29+
const data = { key: 'value' };
30+
31+
await apiRequestAuth(url, method, token, data);
32+
33+
expect(axios).toHaveBeenCalledWith({
34+
method,
35+
url,
36+
data,
37+
});
38+
39+
expect(axios.defaults.headers.common).toMatchSnapshot();
40+
});
41+
});

‎src/utils/auth.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { BrowserWindow } from '@electron/remote';
22

3-
import { generateGitHubAPIUrl } from './helpers';
3+
import { generateGitHubAPIUrl, isEnterpriseHost } from './helpers';
44
import { apiRequest, apiRequestAuth } from '../utils/api-requests';
55
import { AuthResponse, AuthState, AuthTokenResponse } from '../types';
66
import { Constants } from '../utils/constants';
@@ -114,7 +114,7 @@ export const addAccount = (
114114
hostname,
115115
user?: User,
116116
): AuthState => {
117-
if (hostname === Constants.DEFAULT_AUTH_OPTIONS.hostname) {
117+
if (!isEnterpriseHost(hostname)) {
118118
return {
119119
...accounts,
120120
token,

‎src/utils/github-api.test.ts

+16-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { formatReason, getNotificationTypeIcon } from './github-api';
1+
import {
2+
formatReason,
3+
getNotificationTypeIcon,
4+
getNotificationTypeIconColor,
5+
} from './github-api';
26
import { Reason, SubjectType } from '../typesGithub';
37

48
describe('./utils/github-api.ts', () => {
@@ -62,4 +66,15 @@ describe('./utils/github-api.ts', () => {
6266
'QuestionIcon',
6367
);
6468
});
69+
70+
it('should format the notification color', () => {
71+
expect(getNotificationTypeIconColor('closed')).toMatchSnapshot();
72+
expect(getNotificationTypeIconColor('completed')).toMatchSnapshot();
73+
expect(getNotificationTypeIconColor('draft')).toMatchSnapshot();
74+
expect(getNotificationTypeIconColor('merged')).toMatchSnapshot();
75+
expect(getNotificationTypeIconColor('not_planned')).toMatchSnapshot();
76+
expect(getNotificationTypeIconColor('open')).toMatchSnapshot();
77+
expect(getNotificationTypeIconColor('reopened')).toMatchSnapshot();
78+
expect(formatReason('something_else_unknown' as Reason)).toMatchSnapshot();
79+
});
6580
});

0 commit comments

Comments
 (0)