Skip to content

Commit 2856135

Browse files
authored
feat: enable unsubscribing notifications (#407)
* feat: enable unsubscribing notifications * test: fixup and add unsubscribe tests * fix: make icon smaller and secondary * fix: remove notification on unsubscribe
1 parent 1c62602 commit 2856135

File tree

7 files changed

+220
-23
lines changed

7 files changed

+220
-23
lines changed

src/js/actions/index.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,81 @@ describe('actions/index.js', () => {
373373
});
374374
});
375375

376+
it('should unsubscribe from a notification thread with success - github.com', () => {
377+
const id = 123;
378+
const hostname = 'github.com';
379+
380+
// The unsubscribe endpoint call.
381+
nock('https://api.github.com/')
382+
.delete(`/notifications/threads/${id}/subscription`)
383+
.reply(204);
384+
385+
// The mark read endpoint call.
386+
nock('https://api.github.com/')
387+
.patch(`/notifications/threads/${id}`)
388+
.reply(200);
389+
390+
const expectedActions = [
391+
{ type: actions.UNSUBSCRIBE_NOTIFICATION.REQUEST },
392+
{
393+
type: actions.UNSUBSCRIBE_NOTIFICATION.SUCCESS,
394+
meta: { id, hostname },
395+
},
396+
];
397+
398+
const store = createMockStore(
399+
{
400+
auth: {
401+
token: 'THISISATOKEN',
402+
enterpriseAccounts: mockedEnterpriseAccounts,
403+
},
404+
notifications: { response: [] },
405+
},
406+
expectedActions
407+
);
408+
409+
return store
410+
.dispatch(actions.unsubscribeNotification(id, hostname))
411+
.then(() => {
412+
expect(store.getActions()).toEqual(expectedActions);
413+
});
414+
});
415+
416+
it('should unsubscribe from a notification thread with failure - github.com', () => {
417+
const id = 123;
418+
const hostname = 'github.com';
419+
const message = 'Oops! Something went wrong.';
420+
421+
nock('https://api.github.com/')
422+
.delete(`/notifications/threads/${id}/subscription`)
423+
.reply(400, { message });
424+
425+
const expectedActions = [
426+
{ type: actions.UNSUBSCRIBE_NOTIFICATION.REQUEST },
427+
{ type: actions.UNSUBSCRIBE_NOTIFICATION.FAILURE, payload: { message } },
428+
];
429+
430+
const store = createMockStore(
431+
{
432+
auth: {
433+
token: 'THISISATOKEN',
434+
enterpriseAccounts: mockedEnterpriseAccounts,
435+
},
436+
settings: {
437+
participating: false,
438+
},
439+
notifications: { response: [] },
440+
},
441+
expectedActions
442+
);
443+
444+
return store
445+
.dispatch(actions.unsubscribeNotification(id, hostname))
446+
.then(() => {
447+
expect(store.getActions()).toEqual(expectedActions);
448+
});
449+
});
450+
376451
it('should mark a notification as read with success - github.com', () => {
377452
const id = 123;
378453
const hostname = 'github.com';

src/js/actions/index.ts

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,14 @@ export function makeAsyncActionSet(actionName) {
1818
};
1919
}
2020

21+
enum Methods {
22+
GET = 'GET',
23+
POST = 'POST',
24+
PUT = 'PUT',
25+
PATCH = 'PATCH',
26+
DELETE = 'DELETE',
27+
}
28+
2129
// Authentication
2230

2331
export const LOGIN = makeAsyncActionSet('LOGIN');
@@ -27,7 +35,6 @@ export function loginUser(authOptions, code) {
2735

2836
return (dispatch) => {
2937
const url = `https://${hostname}/login/oauth/access_token`;
30-
const method = 'POST';
3138
const data = {
3239
client_id: authOptions.clientId,
3340
client_secret: authOptions.clientSecret,
@@ -36,7 +43,7 @@ export function loginUser(authOptions, code) {
3643

3744
dispatch({ type: LOGIN.REQUEST });
3845

39-
return apiRequest(url, method, data)
46+
return apiRequest(url, Methods.POST, data)
4047
.then(function (response) {
4148
dispatch({
4249
type: LOGIN.SUCCESS,
@@ -60,7 +67,6 @@ export function logout(): LogoutAction {
6067
export const NOTIFICATIONS = makeAsyncActionSet('NOTIFICATIONS');
6168
export function fetchNotifications() {
6269
return (dispatch, getState: () => AppState) => {
63-
const method = 'GET';
6470
const { settings }: { settings: SettingsState } = getState();
6571
const isGitHubLoggedIn = getState().auth.token !== null;
6672
const endpointSuffix = `notifications?participating=${settings.participating}`;
@@ -72,7 +78,7 @@ export function fetchNotifications() {
7278

7379
const url = `https://api.${Constants.DEFAULT_AUTH_OPTIONS.hostname}/${endpointSuffix}`;
7480
const token = getState().auth.token;
75-
return apiRequestAuth(url, method, token);
81+
return apiRequestAuth(url, Methods.GET, token);
7682
}
7783

7884
function getEnterpriseNotifications() {
@@ -81,7 +87,7 @@ export function fetchNotifications() {
8187
const hostname = account.hostname;
8288
const token = account.token;
8389
const url = `https://${hostname}/api/v3/${endpointSuffix}`;
84-
return apiRequestAuth(url, method, token);
90+
return apiRequestAuth(url, Methods.GET, token);
8591
});
8692
}
8793

@@ -130,7 +136,6 @@ export const MARK_NOTIFICATION = makeAsyncActionSet('MARK_NOTIFICATION');
130136
export function markNotification(id, hostname) {
131137
return (dispatch, getState: () => AppState) => {
132138
const url = `${generateGitHubAPIUrl(hostname)}notifications/threads/${id}`;
133-
const method = 'PATCH';
134139

135140
const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
136141
const entAccounts = getState().auth.enterpriseAccounts;
@@ -140,7 +145,7 @@ export function markNotification(id, hostname) {
140145

141146
dispatch({ type: MARK_NOTIFICATION.REQUEST });
142147

143-
return apiRequestAuth(url, method, token, {})
148+
return apiRequestAuth(url, Methods.PATCH, token, {})
144149
.then(function (response) {
145150
dispatch({
146151
type: MARK_NOTIFICATION.SUCCESS,
@@ -156,6 +161,48 @@ export function markNotification(id, hostname) {
156161
};
157162
}
158163

164+
export const UNSUBSCRIBE_NOTIFICATION = makeAsyncActionSet(
165+
'UNSUBSCRIBE_NOTIFICATION'
166+
);
167+
export function unsubscribeNotification(id, hostname) {
168+
return (dispatch, getState: () => AppState) => {
169+
const markReadURL = `${generateGitHubAPIUrl(
170+
hostname
171+
)}notifications/threads/${id}`;
172+
const unsubscribeURL = `${generateGitHubAPIUrl(
173+
hostname
174+
)}notifications/threads/${id}/subscription`;
175+
176+
const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
177+
const entAccounts = getState().auth.enterpriseAccounts;
178+
const token = isEnterprise
179+
? getEnterpriseAccountToken(hostname, entAccounts)
180+
: getState().auth.token;
181+
182+
dispatch({ type: UNSUBSCRIBE_NOTIFICATION.REQUEST });
183+
184+
return apiRequestAuth(unsubscribeURL, Methods.DELETE, token, {})
185+
.then((response) => {
186+
// The GitHub notifications API doesn't automatically mark things as read
187+
// like it does in the UI, so after unsubscribing we also need to hit the
188+
// endpoint to mark it as read.
189+
return apiRequestAuth(markReadURL, Methods.PATCH, token, {});
190+
})
191+
.then((response) => {
192+
dispatch({
193+
type: UNSUBSCRIBE_NOTIFICATION.SUCCESS,
194+
meta: { id, hostname },
195+
});
196+
})
197+
.catch((error) => {
198+
dispatch({
199+
type: UNSUBSCRIBE_NOTIFICATION.FAILURE,
200+
payload: error.response.data,
201+
});
202+
});
203+
};
204+
}
205+
159206
// Repo's Notification
160207

161208
export const MARK_REPO_NOTIFICATION = makeAsyncActionSet(
@@ -166,7 +213,6 @@ export function markRepoNotifications(repoSlug, hostname) {
166213
const url = `${generateGitHubAPIUrl(
167214
hostname
168215
)}repos/${repoSlug}/notifications`;
169-
const method = 'PUT';
170216

171217
const isEnterprise = hostname !== Constants.DEFAULT_AUTH_OPTIONS.hostname;
172218
const entAccounts = getState().auth.enterpriseAccounts;
@@ -176,7 +222,7 @@ export function markRepoNotifications(repoSlug, hostname) {
176222

177223
dispatch({ type: MARK_REPO_NOTIFICATION.REQUEST });
178224

179-
return apiRequestAuth(url, method, token, {})
225+
return apiRequestAuth(url, Methods.PUT, token, {})
180226
.then(function (response) {
181227
dispatch({
182228
type: MARK_REPO_NOTIFICATION.SUCCESS,

src/js/components/__snapshots__/notification.test.tsx.snap

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,34 @@ exports[`components/notification.js should render itself & its children 1`] = `
5151
- Updated
5252
5353
in over 3 years
54+
<button
55+
className="sc-AxheI hMkKFy"
56+
onClick={[Function]}
57+
title="Unsubscribe"
58+
>
59+
<svg
60+
aria-hidden="false"
61+
aria-label="Unsubscribe"
62+
className="octicon"
63+
height={13}
64+
role="img"
65+
style={
66+
Object {
67+
"display": "inline-block",
68+
"fill": "currentColor",
69+
"userSelect": "none",
70+
"verticalAlign": "text-bottom",
71+
}
72+
}
73+
viewBox="0 0 16 16"
74+
width={13}
75+
>
76+
<path
77+
d="M8 2.81v10.38c0 .67-.81 1-1.28.53L3 10H1c-.55 0-1-.45-1-1V7c0-.55.45-1 1-1h2l3.72-3.72C7.19 1.81 8 2.14 8 2.81zm7.53 3.22l-1.06-1.06-1.97 1.97-1.97-1.97-1.06 1.06L11.44 8 9.47 9.97l1.06 1.06 1.97-1.97 1.97 1.97 1.06-1.06L13.56 8l1.97-1.97z"
78+
fillRule="evenodd"
79+
/>
80+
</svg>
81+
</button>
5482
</div>
5583
</div>
5684
<div

src/js/components/notification.test.tsx

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ describe('components/notification.js', () => {
3232

3333
const props = {
3434
markNotification: jest.fn(),
35+
unsubscribeNotification: jest.fn(),
3536
markOnClick: false,
3637
notification: notification,
3738
hostname: 'github.com',
@@ -44,6 +45,7 @@ describe('components/notification.js', () => {
4445
it('should open a notification in the browser', () => {
4546
const props = {
4647
markNotification: jest.fn(),
48+
unsubscribeNotification: jest.fn(),
4749
markOnClick: false,
4850
notification: notification,
4951
hostname: 'github.com',
@@ -54,30 +56,46 @@ describe('components/notification.js', () => {
5456
expect(shell.openExternal).toHaveBeenCalledTimes(1);
5557
});
5658

57-
it('should mark a notification as read', () => {
59+
it('should open a notification in browser & mark it as read', () => {
5860
const props = {
5961
markNotification: jest.fn(),
60-
markOnClick: false,
62+
unsubscribeNotification: jest.fn(),
63+
markOnClick: true,
6164
notification: notification,
6265
hostname: 'github.com',
6366
};
6467

6568
const { getByRole } = render(<NotificationItem {...props} />);
66-
fireEvent.click(getByRole('button'));
69+
fireEvent.click(getByRole('main'));
70+
expect(shell.openExternal).toHaveBeenCalledTimes(1);
6771
expect(props.markNotification).toHaveBeenCalledTimes(1);
6872
});
6973

70-
it('should open a notification in browser & mark it as read', () => {
74+
it('should mark a notification as read', () => {
7175
const props = {
7276
markNotification: jest.fn(),
73-
markOnClick: true,
77+
unsubscribeNotification: jest.fn(),
78+
markOnClick: false,
7479
notification: notification,
7580
hostname: 'github.com',
7681
};
7782

78-
const { getByRole } = render(<NotificationItem {...props} />);
79-
fireEvent.click(getByRole('main'));
80-
expect(shell.openExternal).toHaveBeenCalledTimes(1);
83+
const { getByLabelText } = render(<NotificationItem {...props} />);
84+
fireEvent.click(getByLabelText('Mark as Read'));
8185
expect(props.markNotification).toHaveBeenCalledTimes(1);
8286
});
87+
88+
it('should unsubscribe from a notification thread', () => {
89+
const props = {
90+
markNotification: jest.fn(),
91+
unsubscribeNotification: jest.fn(),
92+
markOnClick: false,
93+
notification: notification,
94+
hostname: 'github.com',
95+
};
96+
97+
const { getByLabelText } = render(<NotificationItem {...props} />);
98+
fireEvent.click(getByLabelText('Unsubscribe'));
99+
expect(props.unsubscribeNotification).toHaveBeenCalledTimes(1);
100+
});
83101
});

0 commit comments

Comments
 (0)