Skip to content

Commit 4483a73

Browse files
refactor: fixed issues of review
1 parent 6efa310 commit 4483a73

5 files changed

Lines changed: 101 additions & 142 deletions

File tree

src/Notifications/data/api.js

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import { camelCaseObject, getConfig, snakeCaseObject } from '@edx/frontend-platform';
1+
import { getConfig, snakeCaseObject } from '@edx/frontend-platform';
22
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
33

44
export const getNotificationsCountApiUrl = () => `${getConfig().LMS_BASE_URL}/api/notifications/count/`;
55
export const getNotificationsApiUrl = () => `${getConfig().LMS_BASE_URL}/api/notifications/`;
66
export const markNotificationsSeenApiUrl = (appName) => `${getConfig().LMS_BASE_URL}/api/notifications/mark-notifications-unseen/${appName}/`;
7-
export const markAllNotificationsAsReadpiUrl = (appName, id) => `${getConfig().LMS_BASE_URL}/api/notifications/mark-notifications-read/${appName}/${id}`;
7+
export const markAllNotificationsAsReadpiUrl = () => `${getConfig().LMS_BASE_URL}/api/notifications/read/`;
88

99
export async function getNotifications(appName, page, pageSize) {
1010
const params = snakeCaseObject({ page, pageSize });
@@ -14,29 +14,31 @@ export async function getNotifications(appName, page, pageSize) {
1414
const endIndex = startIndex + pageSize;
1515

1616
const notifications = data.slice(startIndex, endIndex);
17-
return { notifications: camelCaseObject(notifications), numPages: 2, currentPage: page };
17+
return { notifications, numPages: 2, currentPage: page };
1818
}
1919

2020
export async function getNotificationCounts() {
2121
const { data } = await getAuthenticatedHttpClient().get(getNotificationsCountApiUrl());
2222

23-
return camelCaseObject(data);
23+
return data;
2424
}
2525

2626
export async function markNotificationSeen(appName) {
2727
const { data } = await getAuthenticatedHttpClient().put(`${markNotificationsSeenApiUrl(appName)}`);
2828

29-
return camelCaseObject(data);
29+
return data;
3030
}
3131

3232
export async function markAllNotificationRead(appName) {
33-
const { data } = await getAuthenticatedHttpClient().put(`${markAllNotificationsAsReadpiUrl(appName)}`);
33+
const params = snakeCaseObject({ appName });
34+
const { data } = await getAuthenticatedHttpClient().put(markAllNotificationsAsReadpiUrl(), { params });
3435

35-
return camelCaseObject(data);
36+
return data;
3637
}
3738

38-
export async function markNotificationRead(appName, notificationId) {
39-
const { data } = await getAuthenticatedHttpClient().put(`${markAllNotificationsAsReadpiUrl(appName, notificationId)}`);
39+
export async function markNotificationRead(notificationId) {
40+
const params = snakeCaseObject({ notificationId });
41+
const { data } = await getAuthenticatedHttpClient().put(markAllNotificationsAsReadpiUrl(), { params });
4042

41-
return camelCaseObject({ data, id: notificationId });
43+
return { data, id: notificationId };
4244
}

src/Notifications/data/api.test.js

Lines changed: 63 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,19 @@ import { Factory } from 'rosie';
44
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
55
import { initializeMockApp } from '@edx/frontend-platform/testing';
66

7-
import { initializeStore } from '../../store';
8-
import executeThunk from '../../test-utils';
97
import {
108
getNotificationsApiUrl, getNotificationsCountApiUrl, markAllNotificationsAsReadpiUrl, markNotificationsSeenApiUrl,
119
getNotificationCounts, getNotifications, markNotificationSeen, markAllNotificationRead, markNotificationRead,
1210
} from './api';
13-
import {
14-
fetchAppsNotificationCount,
15-
fetchNotificationList,
16-
markAllNotificationsAsRead,
17-
markNotificationsAsRead,
18-
markNotificationsAsSeen,
19-
} from './thunks';
2011

2112
import './__factories__';
2213

2314
const notificationCountsApiUrl = getNotificationsCountApiUrl();
2415
const notificationsApiUrl = getNotificationsApiUrl();
2516
const markedAllNotificationsAsSeenApiUrl = markNotificationsSeenApiUrl('discussions');
26-
const markedAllNotificationsAsReadApiUrl = markAllNotificationsAsReadpiUrl('discussions');
27-
const markedNotificationAsReadApiUrl = markAllNotificationsAsReadpiUrl('discussions', 1);
17+
const markedAllNotificationsAsReadApiUrl = markAllNotificationsAsReadpiUrl();
2818

2919
let axiosMock = null;
30-
let store;
3120

3221
describe('Notifications API', () => {
3322
beforeEach(async () => {
@@ -41,14 +30,13 @@ describe('Notifications API', () => {
4130
});
4231
axiosMock = new MockAdapter(getAuthenticatedHttpClient());
4332
Factory.resetAll();
44-
store = initializeStore();
4533
});
4634

4735
afterEach(() => {
4836
axiosMock.reset();
4937
});
5038

51-
it('successfully get notification counts for different tabs.', async () => {
39+
it('Successfully get notification counts for different tabs.', async () => {
5240
axiosMock.onGet(notificationCountsApiUrl).reply(200, (Factory.build('notificationsCount')));
5341

5442
const { count, countByAppName } = await getNotificationCounts();
@@ -60,21 +48,20 @@ describe('Notifications API', () => {
6048
expect(countByAppName.authoring).toEqual(5);
6149
});
6250

63-
it('failed to get notification counts.', async () => {
64-
axiosMock.onGet(notificationCountsApiUrl).reply(404);
65-
await executeThunk(fetchAppsNotificationCount(), store.dispatch, store.getState);
66-
67-
expect(store.getState().notifications.notificationStatus).toEqual('failed');
68-
});
69-
70-
it('denied to get notification counts.', async () => {
71-
axiosMock.onGet(notificationCountsApiUrl).reply(403, {});
72-
await executeThunk(fetchAppsNotificationCount(), store.dispatch);
73-
74-
expect(store.getState().notifications.notificationStatus).toEqual('denied');
51+
it.each([
52+
{ statusCode: 404, message: 'Failed to get notification counts.' },
53+
{ statusCode: 403, message: 'Denied to get notification counts.' },
54+
])('%s for notification counts API.', async ({ statusCode, message }) => {
55+
axiosMock.onGet(notificationCountsApiUrl).reply(statusCode, { message });
56+
try {
57+
await getNotificationCounts();
58+
} catch (error) {
59+
expect(error.response.status).toEqual(statusCode);
60+
expect(error.response.data.message).toEqual(message);
61+
}
7562
});
7663

77-
it('successfully get notifications.', async () => {
64+
it('Successfully get notifications.', async () => {
7865
axiosMock.onGet(notificationsApiUrl).reply(
7966
200,
8067
(Factory.buildList('notification', 2, null, { createdDate: new Date().toISOString() })),
@@ -85,83 +72,79 @@ describe('Notifications API', () => {
8572
expect(notifications).toHaveLength(2);
8673
});
8774

88-
it('failed to get notifications.', async () => {
89-
axiosMock.onGet(notificationsApiUrl).reply(404);
90-
await executeThunk(fetchNotificationList({ page: 1, pageSize: 10 }), store.dispatch, store.getState);
91-
92-
expect(store.getState().notifications.notificationStatus).toEqual('failed');
75+
it.each([
76+
{ statusCode: 404, message: 'Failed to get notifications.' },
77+
{ statusCode: 403, message: 'Denied to get notifications.' },
78+
])('%s for notification API.', async ({ statusCode, message }) => {
79+
axiosMock.onGet(notificationsApiUrl).reply(statusCode, { message });
80+
try {
81+
await getNotifications({ page: 1, pageSize: 10 });
82+
} catch (error) {
83+
expect(error.response.status).toEqual(statusCode);
84+
expect(error.response.data.message).toEqual(message);
85+
}
9386
});
9487

95-
it('denied to get notifications.', async () => {
96-
axiosMock.onGet(notificationsApiUrl).reply(403, {});
97-
await executeThunk(fetchNotificationList({ page: 1, pageSize: 10 }), store.dispatch);
98-
99-
expect(store.getState().notifications.notificationStatus).toEqual('denied');
100-
});
101-
102-
it('successfully marked all notifications as seen for selected app.', async () => {
88+
it('Successfully marked all notifications as seen for selected app.', async () => {
10389
axiosMock.onPut(markedAllNotificationsAsSeenApiUrl).reply(200, { message: 'Notifications marked seen.' });
10490

10591
const { message } = await markNotificationSeen('discussions');
10692

10793
expect(message).toEqual('Notifications marked seen.');
10894
});
10995

110-
it('failed to mark all notifications as seen for selected app.', async () => {
111-
axiosMock.onPut(markedAllNotificationsAsSeenApiUrl).reply(404);
112-
await executeThunk(markNotificationsAsSeen('discussions'), store.dispatch, store.getState);
113-
114-
expect(store.getState().notifications.notificationStatus).toEqual('failed');
96+
it.each([
97+
{ statusCode: 404, message: 'Failed to mark all notifications as seen for selected app.' },
98+
{ statusCode: 403, message: 'Denied to mark all notifications as seen for selected app.' },
99+
])('%s for notification mark as seen API.', async ({ statusCode, message }) => {
100+
axiosMock.onPut(markedAllNotificationsAsSeenApiUrl).reply(statusCode, { message });
101+
try {
102+
await markNotificationSeen('discussions');
103+
} catch (error) {
104+
expect(error.response.status).toEqual(statusCode);
105+
expect(error.response.data.message).toEqual(message);
106+
}
115107
});
116108

117-
it('denied to mark all notifications as seen for selected app.', async () => {
118-
axiosMock.onPut(markedAllNotificationsAsSeenApiUrl).reply(403, {});
119-
await executeThunk(markNotificationsAsSeen('discussions'), store.dispatch);
120-
121-
expect(store.getState().notifications.notificationStatus).toEqual('denied');
122-
});
123-
124-
it('successfully marked all notifications as read for selected app.', async () => {
109+
it('Successfully marked all notifications as read for selected app.', async () => {
125110
axiosMock.onPut(markedAllNotificationsAsReadApiUrl).reply(200, { message: 'Notifications marked read.' });
126111

127112
const { message } = await markAllNotificationRead('discussions');
128113

129114
expect(message).toEqual('Notifications marked read.');
130115
});
131116

132-
it('failed to mark all notifications as read for selected app.', async () => {
133-
axiosMock.onPut(markedAllNotificationsAsReadApiUrl).reply(404);
134-
await executeThunk(markAllNotificationsAsRead('discussions'), store.dispatch, store.getState);
135-
136-
expect(store.getState().notifications.notificationStatus).toEqual('failed');
137-
});
138-
139-
it('denied to mark all notifications as read for selected app.', async () => {
140-
axiosMock.onPut(markedAllNotificationsAsReadApiUrl).reply(403, {});
141-
await executeThunk(markAllNotificationsAsRead('discussions'), store.dispatch);
142-
143-
expect(store.getState().notifications.notificationStatus).toEqual('denied');
117+
it.each([
118+
{ statusCode: 404, message: 'Failed to mark all notifications as read for selected app.' },
119+
{ statusCode: 403, message: 'Denied to mark all notifications as read for selected app.' },
120+
])('%s for notification mark all as read API.', async ({ statusCode, message }) => {
121+
axiosMock.onPut(markedAllNotificationsAsReadApiUrl).reply(statusCode, { message });
122+
try {
123+
await markAllNotificationRead('discussions');
124+
} catch (error) {
125+
expect(error.response.status).toEqual(statusCode);
126+
expect(error.response.data.message).toEqual(message);
127+
}
144128
});
145129

146-
it('successfully marked notification as read.', async () => {
147-
axiosMock.onPut(markedNotificationAsReadApiUrl).reply(200, { message: 'Notification marked read.' });
130+
it('Successfully marked notification as read.', async () => {
131+
axiosMock.onPut(markedAllNotificationsAsReadApiUrl).reply(200, { message: 'Notification marked read.' });
148132

149-
const { data } = await markNotificationRead('discussions', 1);
133+
const { data } = await markNotificationRead(1);
150134

151135
expect(data.message).toEqual('Notification marked read.');
152136
});
153137

154-
it('failed to mark notification as read .', async () => {
155-
axiosMock.onPut(markedNotificationAsReadApiUrl).reply(404);
156-
await executeThunk(markNotificationsAsRead('discussions', 1), store.dispatch, store.getState);
157-
158-
expect(store.getState().notifications.notificationStatus).toEqual('failed');
159-
});
160-
161-
it('denied to mark notification as read.', async () => {
162-
axiosMock.onPut(markedNotificationAsReadApiUrl).reply(403, {});
163-
await executeThunk(markNotificationsAsRead('discussions', 1), store.dispatch);
164-
165-
expect(store.getState().notifications.notificationStatus).toEqual('denied');
138+
it.each([
139+
{ statusCode: 404, message: 'Failed to mark notification as read.' },
140+
{ statusCode: 403, message: 'Denied to mark notification as read.' },
141+
])('%s for notification mark as read API.', async ({ statusCode, message }) => {
142+
axiosMock.onPut(markedAllNotificationsAsReadApiUrl).reply(statusCode, { message });
143+
try {
144+
await markAllNotificationRead(1);
145+
} catch (error) {
146+
expect(error.response.status).toEqual(statusCode);
147+
expect(error.response.data.message).toEqual(message);
148+
}
166149
});
167150
});

src/Notifications/data/redux.test.js

Lines changed: 9 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ import './__factories__';
1818

1919
const notificationCountsApiUrl = getNotificationsCountApiUrl();
2020
const notificationsApiUrl = getNotificationsApiUrl();
21-
const markedNotificationAsReadApiUrl = markAllNotificationsAsReadpiUrl('discussions', 1);
22-
const markedAllNotificationsAsReadApiUrl = markAllNotificationsAsReadpiUrl('discussions');
21+
const markedAllNotificationsAsReadApiUrl = markAllNotificationsAsReadpiUrl();
2322
const markedAllNotificationsAsSeenApiUrl = markNotificationsSeenApiUrl('discussions');
2423

2524
let axiosMock;
@@ -52,7 +51,7 @@ describe('Notification Redux', () => {
5251
axiosMock.reset();
5352
});
5453

55-
it('successfully loaded initial notification states in the redux.', async () => {
54+
it('Successfully loaded initial notification states in the redux.', async () => {
5655
executeThunk(resetNotificationState(), store.dispatch, store.getState);
5756

5857
const { notifications } = store.getState();
@@ -70,13 +69,13 @@ describe('Notification Redux', () => {
7069
expect(notifications.pagination.nextPage).toBeNull();
7170
});
7271

73-
it('successfully loaded notifications list in the redux.', async () => {
72+
it('Successfully loaded notifications list in the redux.', async () => {
7473
const { notifications: { notifications } } = store.getState();
7574

7675
expect(Object.keys(notifications)).toHaveLength(2);
7776
});
7877

79-
it('successfully loaded notification counts in the redux.', async () => {
78+
it('Successfully loaded notification counts in the redux.', async () => {
8079
const { notifications: { tabsCount } } = store.getState();
8180

8281
expect(tabsCount.count).toEqual(25);
@@ -86,40 +85,14 @@ describe('Notification Redux', () => {
8685
expect(tabsCount.authoring).toEqual(5);
8786
});
8887

89-
it('successfully loaded showNotificationTray status in the redux based on api.', async () => {
90-
const { notifications: { showNotificationTray } } = store.getState();
91-
92-
expect(showNotificationTray).toEqual(true);
93-
});
94-
95-
it('successfully store the count, numPages, currentPage, and nextPage data in redux.', async () => {
96-
const { notifications: { pagination } } = store.getState();
97-
98-
expect(pagination.count).toEqual(10);
99-
expect(pagination.currentPage).toEqual(1);
100-
expect(pagination.numPages).toEqual(2);
101-
});
102-
103-
it('successfully updated the selected app name in redux.', async () => {
104-
const { notifications: { appName } } = store.getState();
105-
106-
expect(appName).toEqual('discussions');
107-
});
108-
109-
it('successfully store notification ids in the selected app in apps.', async () => {
110-
const { notifications: { apps } } = store.getState();
111-
112-
expect(apps.discussions).toHaveLength(2);
113-
});
114-
115-
it('successfully marked all notifications as seen for selected app.', async () => {
88+
it('Successfully marked all notifications as seen for selected app.', async () => {
11689
axiosMock.onPut(markedAllNotificationsAsSeenApiUrl).reply(200);
11790
await executeThunk(markNotificationsAsSeen('discussions'), store.dispatch, store.getState);
11891

11992
expect(store.getState().notifications.notificationStatus).toEqual('successful');
12093
});
12194

122-
it('successfully marked all notifications as read for selected app in the redux.', async () => {
95+
it('Successfully marked all notifications as read for selected app in the redux.', async () => {
12396
axiosMock.onPut(markedAllNotificationsAsReadApiUrl).reply(200);
12497
await executeThunk(markAllNotificationsAsRead('discussions'), store.dispatch, store.getState);
12598

@@ -130,9 +103,9 @@ describe('Notification Redux', () => {
130103
expect(firstNotification.lastRead).not.toBeNull();
131104
});
132105

133-
it('successfully marked notification as read in the redux.', async () => {
134-
axiosMock.onPut(markedNotificationAsReadApiUrl).reply(200);
135-
await executeThunk(markNotificationsAsRead('discussions', 1), store.dispatch, store.getState);
106+
it('Successfully marked notification as read in the redux.', async () => {
107+
axiosMock.onPut(markedAllNotificationsAsReadApiUrl).reply(200);
108+
await executeThunk(markNotificationsAsRead(1), store.dispatch, store.getState);
136109

137110
const { notifications: { notificationStatus, notifications } } = store.getState();
138111
const firstNotification = Object.values(notifications)[0];

0 commit comments

Comments
 (0)