From e3338c3c5cb73203f4b1a18adec9897d3b9e85fb Mon Sep 17 00:00:00 2001 From: Hel Nershing Thapa Date: Tue, 6 Jun 2023 16:29:40 +0545 Subject: [PATCH 1/9] Send `setPopoutFocus` to `InboxNavMini` component Resolves an issue preventing an uncaught error when routing to a location using `Link` --- frontend/src/views/notifications.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/src/views/notifications.js b/frontend/src/views/notifications.js index 6506c2115d..d89742d1fc 100644 --- a/frontend/src/views/notifications.js +++ b/frontend/src/views/notifications.js @@ -48,7 +48,10 @@ export const NotificationPopout = (props) => { }} className={`fr ${props.isPopoutFocus ? '' : 'dn '}br2 absolute bg-white`} > - + Date: Tue, 6 Jun 2023 16:33:57 +0545 Subject: [PATCH 2/9] Construct `generateSampleNotifications` function for sample data he `generateSampleNotifications` function is added, which takes the number of samples as a parameter and generates an array of sample notification objects. --- .../network/tests/mockData/notifications.js | 114 ++++++++---------- 1 file changed, 50 insertions(+), 64 deletions(-) diff --git a/frontend/src/network/tests/mockData/notifications.js b/frontend/src/network/tests/mockData/notifications.js index e12a97f1f5..fc5ec7dd4f 100644 --- a/frontend/src/network/tests/mockData/notifications.js +++ b/frontend/src/network/tests/mockData/notifications.js @@ -1,3 +1,52 @@ +export function generateSampleNotifications(numSamples) { + const sampleData = []; + + for (let i = 0; i < numSamples; i++) { + const messageId = Math.floor(Math.random() * 1000000); + const subject = `Sample subject ${i} Sample Team`; + const message = `Sample message ${i} Example.com`; + const fromUsername = `User${i}`; + const displayPictureUrl = `https://www.example.com/avatar${i}.jpg`; + const messageType = + i % 3 === 0 + ? 'REQUEST_TEAM_NOTIFICATION' + : i % 3 === 1 + ? 'INVITATION_NOTIFICATION' + : 'TEAM_BROADCAST'; + const sentDate = new Date().toISOString(); + const read = i % 3 !== 0; + + let projectId = null; + let projectTitle = null; + let taskId = null; + + if (i % 2 === 0) { + projectId = Math.floor(Math.random() * 1000); + projectTitle = `Sample Project ${projectId}`; + } else { + taskId = Math.floor(Math.random() * 1000); + } + + const sample = { + messageId, + subject, + message, + fromUsername, + displayPictureUrl, + projectId, + projectTitle, + taskId, + messageType, + sentDate, + read, + }; + + sampleData.push(sample); + } + + return sampleData; +} + export const ownCountUnread = { newMessages: true, unread: 2, @@ -14,68 +63,5 @@ export const notifications = { perPage: 10, total: 208, }, - userMessages: [ - { - messageId: 759710, - subject: - 'Hel Nershing Thapa requested to join Sample Team', - message: - 'Hel Nershing Thapa has requested to join the Sample Team team. Access the team management page to accept or reject that request.', - fromUsername: 'Hel Nershing Thapa', - displayPictureUrl: - 'https://www.openstreetmap.org/rails/active_storage/representations/redirect/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBBeFJheFE9PSIsImV4cCI6bnVsbCwicHVyIjoiYmxvYl9pZCJ9fQ==--a765e2377a288bccae85da6604300251d9de6d39/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaDdCem9MWm05eWJXRjBTU0lJYW5CbkJqb0dSVlE2RkhKbGMybDZaVjkwYjE5c2FXMXBkRnNIYVdscGFRPT0iLCJleHAiOm51bGwsInB1ciI6InZhcmlhdGlvbiJ9fQ==--1d22b8d446683a272d1a9ff04340453ca7c374b4/bitmoji.jpg', - projectId: null, - projectTitle: null, - taskId: null, - messageType: 'REQUEST_TEAM_NOTIFICATION', - sentDate: '2023-01-09T16:42:16.168030Z', - read: false, - }, - { - messageId: 759703, - subject: - 'You have been added to team Notification Test', - message: - 'You have been added to the team Notification Test as MEMBER by helnershingthapa. Access the Notification Test\'s page to view more info about this team.', - fromUsername: 'helnershingthapa', - displayPictureUrl: - 'https://www.openstreetmap.org/rails/active_storage/representations/redirect/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBBNXQ2Q3c9PSIsImV4cCI6bnVsbCwicHVyIjoiYmxvYl9pZCJ9fQ==--fe41f1b2a5d6cf492a7133f15c81f105dec06ff7/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaDdCem9MWm05eWJXRjBPZ2h3Ym1jNkZISmxjMmw2WlY5MGIxOXNhVzFwZEZzSGFXbHBhUT09IiwiZXhwIjpudWxsLCJwdXIiOiJ2YXJpYXRpb24ifX0=--058ac785867b32287d598a314311e2253bd879a3/unnamed.webp', - projectId: null, - projectTitle: null, - taskId: null, - messageType: 'INVITATION_NOTIFICATION', - sentDate: '2023-01-02T07:59:27.129271Z', - read: true, - }, - { - messageId: 759606, - subject: 'Test', - message: - 'A message from Aadesh Baral, manager of Notification Test team:

Hello

', - fromUsername: 'Aadesh Baral', - displayPictureUrl: - 'https://www.openstreetmap.org/rails/active_storage/representations/redirect/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBBNXFHQ1E9PSIsImV4cCI6bnVsbCwicHVyIjoiYmxvYl9pZCJ9fQ==--aed68fa4deb4e4eeba02e233b68881c4c9d84ab8/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaDdCem9MWm05eWJXRjBTU0lJYW5CbkJqb0dSVlE2RkhKbGMybDZaVjkwYjE5c2FXMXBkRnNIYVdscGFRPT0iLCJleHAiOm51bGwsInB1ciI6InZhcmlhdGlvbiJ9fQ==--1d22b8d446683a272d1a9ff04340453ca7c374b4/89986176_1424302844399685_1705928282320404480_n.jpg', - projectId: null, - projectTitle: null, - taskId: null, - messageType: 'TEAM_BROADCAST', - sentDate: '2022-12-02T09:53:52.231791Z', - read: true, - }, - { - messageId: 759600, - subject: 'Test', - message: - 'A message from Aadesh Baral, manager of Notification Test team:

Hello

', - fromUsername: 'Aadesh Baral', - displayPictureUrl: - 'https://www.openstreetmap.org/rails/active_storage/representations/redirect/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBBNXFHQ1E9PSIsImV4cCI6bnVsbCwicHVyIjoiYmxvYl9pZCJ9fQ==--aed68fa4deb4e4eeba02e233b68881c4c9d84ab8/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaDdCem9MWm05eWJXRjBTU0lJYW5CbkJqb0dSVlE2RkhKbGMybDZaVjkwYjE5c2FXMXBkRnNIYVdscGFRPT0iLCJleHAiOm51bGwsInB1ciI6InZhcmlhdGlvbiJ9fQ==--1d22b8d446683a272d1a9ff04340453ca7c374b4/89986176_1424302844399685_1705928282320404480_n.jpg', - projectId: null, - projectTitle: null, - taskId: null, - messageType: 'TEAM_BROADCAST', - sentDate: '2022-12-02T09:53:51.664913Z', - read: true, - }, - ], + userMessages: generateSampleNotifications(10), }; From f6fbd2a848fa1a65749c2fbca931020f175583c8 Mon Sep 17 00:00:00 2001 From: Hel Nershing Thapa Date: Tue, 6 Jun 2023 16:35:43 +0545 Subject: [PATCH 3/9] Moved the updateUnreadCount function to the ActionButtons component to eliminate prop drilling and improve code organization maybe. --- .../src/components/notifications/actionButtons.js | 11 +++++++++-- .../components/notifications/notificationResults.js | 13 ------------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/frontend/src/components/notifications/actionButtons.js b/frontend/src/components/notifications/actionButtons.js index 987172e33e..d2f2c96f33 100644 --- a/frontend/src/components/notifications/actionButtons.js +++ b/frontend/src/components/notifications/actionButtons.js @@ -6,7 +6,7 @@ import deletionMessages from '../deleteModal/messages'; import messages from './messages'; import { EyeIcon, WasteIcon } from '../svgIcons'; import { Button } from '../button'; -import { pushToLocalJSONAPI } from '../../network/genericJSONRequest'; +import { fetchLocalJSONAPI, pushToLocalJSONAPI } from '../../network/genericJSONRequest'; export const ActionButtons = ({ selected, @@ -16,7 +16,6 @@ export const ActionButtons = ({ isAllSelected, inboxQuery, setInboxQuery, - updateUnreadCount, pageOfCards, totalPages, }) => { @@ -25,6 +24,14 @@ export const ActionButtons = ({ const param = inboxQuery.types ? `?messageType=${inboxQuery.types?.join(',')}` : ''; const payload = JSON.stringify({ messageIds: selected }); + const updateUnreadCount = () => { + fetchLocalJSONAPI(`notifications/?status=unread`, token) + .then((notifications) => + dispatch({ type: 'SET_UNREAD_COUNT', payload: notifications.pagination?.total }), + ) + .catch((e) => console.log(e)); + }; + const deleteMessages = () => { if (isAllSelected) { pushToLocalJSONAPI(`/api/v2/notifications/delete-all/${param}`, null, token, 'DELETE') diff --git a/frontend/src/components/notifications/notificationResults.js b/frontend/src/components/notifications/notificationResults.js index c5fea44096..a248e29c41 100644 --- a/frontend/src/components/notifications/notificationResults.js +++ b/frontend/src/components/notifications/notificationResults.js @@ -1,6 +1,5 @@ import React, { useEffect, useState } from 'react'; import ReactPlaceholder from 'react-placeholder'; -import { useDispatch, useSelector } from 'react-redux'; import { FormattedMessage, FormattedNumber } from 'react-intl'; import 'react-placeholder/lib/reactPlaceholder.css'; @@ -10,7 +9,6 @@ import NotificationPlaceholder from './notificationPlaceholder'; import { RefreshIcon } from '../svgIcons'; import { SelectAll } from '../formInputs'; import { SelectAllNotifications } from './selectAllNotifications'; -import { fetchLocalJSONAPI } from '../../network/genericJSONRequest'; import { ActionButtons } from './actionButtons'; export const NotificationResultsMini = (props) => { @@ -110,8 +108,6 @@ const NotificationCards = ({ inboxQuery, setInboxQuery, }) => { - const dispatch = useDispatch(); - const token = useSelector((state) => state.auth.token); const [selected, setSelected] = useState([]); const [isAllSelected, setIsAllSelected] = useState(false); @@ -138,14 +134,6 @@ const NotificationCards = ({ return !msg.read && selected.includes(msg.messageId) ? acc + 1 : acc; }, 0); - const updateUnreadCount = () => { - fetchLocalJSONAPI(`notifications/?status=unread`, token) - .then((notifications) => - dispatch({ type: 'SET_UNREAD_COUNT', payload: notifications.pagination?.total }), - ) - .catch((e) => console.log(e)); - }; - return ( <> {!useMiniCard && ( @@ -165,7 +153,6 @@ const NotificationCards = ({ isAllSelected={isAllSelected} inboxQuery={inboxQuery} setInboxQuery={setInboxQuery} - updateUnreadCount={updateUnreadCount} pageOfCards={pageOfCards} totalPages={totalPages} /> From 6a51e93e4fd62b2a4daa59c85f070873fcb0c4ea Mon Sep 17 00:00:00 2001 From: Hel Nershing Thapa Date: Tue, 6 Jun 2023 16:37:31 +0545 Subject: [PATCH 4/9] Update the error message text when loading notification details fail Will now show error loading notification instead of error loading message --- frontend/src/components/notifications/messages.js | 4 ++++ frontend/src/components/notifications/notificationBodyCard.js | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/notifications/messages.js b/frontend/src/components/notifications/messages.js index 9c33e9fe6c..6f42bc2215 100644 --- a/frontend/src/components/notifications/messages.js +++ b/frontend/src/components/notifications/messages.js @@ -8,6 +8,10 @@ export default defineMessages({ id: 'notifications.mainSection.title', defaultMessage: 'Notifications', }, + notification: { + id: 'notifications.singular.notification', + defaultMessage: 'notification', + }, all: { id: 'notifications.filter.all', defaultMessage: 'All', diff --git a/frontend/src/components/notifications/notificationBodyCard.js b/frontend/src/components/notifications/notificationBodyCard.js index 0200f1cbed..2b55f65dd2 100644 --- a/frontend/src/components/notifications/notificationBodyCard.js +++ b/frontend/src/components/notifications/notificationBodyCard.js @@ -63,7 +63,7 @@ export const NotificationBodyModal = (props) => { , + xWord: , }} /> From 122a3daa15bab00594c575b4502780ba477fef99 Mon Sep 17 00:00:00 2001 From: Hel Nershing Thapa Date: Tue, 6 Jun 2023 16:38:23 +0545 Subject: [PATCH 5/9] Removed unnecessary else conditional statements from the onSortSelect function since the dropdown selection is single select? --- .../notifications/notificationOrderBy.js | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/frontend/src/components/notifications/notificationOrderBy.js b/frontend/src/components/notifications/notificationOrderBy.js index 5a9e56c156..e770a49e0e 100644 --- a/frontend/src/components/notifications/notificationOrderBy.js +++ b/frontend/src/components/notifications/notificationOrderBy.js @@ -38,21 +38,16 @@ export function NotificationOrderBySelector(props) { }, ]; - const onSortSelect = (arr) => { - if (arr.length === 1) { - props.setQuery( - { - ...props.allQueryParams, - page: undefined, - orderBy: arr[0].sort, - orderByType: arr[0].type, - }, - 'pushIn', - ); - } else if (arr.length > 1) { - throw new Error('filter select array is bigger.'); - } - }; + const onSortSelect = (arr) => + props.setQuery( + { + ...props.allQueryParams, + page: undefined, + orderBy: arr[0].sort, + orderByType: arr[0].type, + }, + 'pushIn', + ); return ( Date: Tue, 6 Jun 2023 16:44:28 +0545 Subject: [PATCH 6/9] Accomodate test cases for previous commits the previous commits I'm taking about here being the use of function to generate n number of notifications and removal of updateUnreadCount as props to ActionButtons --- .../header/tests/notificationBell.test.js | 6 +++--- .../notifications/tests/actionButtons.test.js | 14 -------------- .../notifications/tests/notificationCard.test.js | 14 +++----------- frontend/src/views/tests/notifications.test.js | 2 +- 4 files changed, 7 insertions(+), 29 deletions(-) diff --git a/frontend/src/components/header/tests/notificationBell.test.js b/frontend/src/components/header/tests/notificationBell.test.js index 54e861b2ad..afe72abc57 100644 --- a/frontend/src/components/header/tests/notificationBell.test.js +++ b/frontend/src/components/header/tests/notificationBell.test.js @@ -21,8 +21,8 @@ describe('Notification Bell', () => { ); const inboxLink = screen.getAllByRole('link')[0]; expect(within(inboxLink).getByLabelText(/notifications/i)).toBeInTheDocument(); - expect(await screen.findByText(/You have been added to team/i)).toBeInTheDocument(); - expect(screen.getAllByRole('article').length).toBe(4); + expect(await screen.findByText(/Sample subject 1/i)).toBeInTheDocument(); + expect(screen.getAllByRole('article').length).toBe(5); await waitFor(() => { expect(container.getElementsByClassName('redicon')[0]).toBeInTheDocument(); }); @@ -36,7 +36,7 @@ describe('Notification Bell', () => { , ); expect(screen.getAllByRole('link')[0]).not.toHaveClass('bb b--blue-dark bw1 pv2'); - expect(await screen.findByText(/You have been added to team/i)).toBeInTheDocument(); + expect(await screen.findByText(/Sample subject 1/i)).toBeInTheDocument(); await waitFor(() => { expect(container.getElementsByClassName('redicon')[0]).toBeInTheDocument(); }); diff --git a/frontend/src/components/notifications/tests/actionButtons.test.js b/frontend/src/components/notifications/tests/actionButtons.test.js index 5fb0eaf47a..03602ffe74 100644 --- a/frontend/src/components/notifications/tests/actionButtons.test.js +++ b/frontend/src/components/notifications/tests/actionButtons.test.js @@ -10,7 +10,6 @@ import { ReduxIntlProviders } from '../../../utils/testWithIntl'; describe('Action Buttons', () => { const retryFnMock = jest.fn(); const setSelectedMock = jest.fn(); - const updateUnreadCountMock = jest.fn(); it('should return nothing if no notification is selected', () => { act(() => { store.dispatch({ type: 'SET_TOKEN', token: 'validToken' }); @@ -22,7 +21,6 @@ describe('Action Buttons', () => { selected={[]} retryFn={retryFnMock} setSelected={setSelectedMock} - updateUnreadCount={updateUnreadCountMock} /> , ); @@ -40,7 +38,6 @@ describe('Action Buttons', () => { retryFn={retryFnMock} isAllSelected={false} setSelected={setSelectedMock} - updateUnreadCount={updateUnreadCountMock} unreadCountInSelected={1} /> , @@ -52,7 +49,6 @@ describe('Action Buttons', () => { ); await waitFor(() => expect(setSelectedMock).toHaveBeenCalledTimes(1)); await waitFor(() => expect(retryFnMock).toHaveBeenCalledTimes(1)); - expect(updateUnreadCountMock).not.toHaveBeenCalled(); }); it('should fetch unread count if all notifications are selected upon marking notifications as read', async () => { @@ -65,7 +61,6 @@ describe('Action Buttons', () => { retryFn={retryFnMock} isAllSelected={true} setSelected={setSelectedMock} - updateUnreadCount={updateUnreadCountMock} /> , ); @@ -76,7 +71,6 @@ describe('Action Buttons', () => { ); await waitFor(() => expect(setSelectedMock).toHaveBeenCalledTimes(1)); await waitFor(() => expect(retryFnMock).toHaveBeenCalledTimes(1)); - expect(updateUnreadCountMock).toHaveBeenCalled(); }); it('should decrement unread count in redux store if all notifications are not selected upon deleting notifications', async () => { @@ -89,7 +83,6 @@ describe('Action Buttons', () => { retryFn={retryFnMock} isAllSelected={false} setSelected={setSelectedMock} - updateUnreadCount={updateUnreadCountMock} unreadCountInSelected={1} pageOfCards={6} /> @@ -102,7 +95,6 @@ describe('Action Buttons', () => { ); await waitFor(() => expect(setSelectedMock).toHaveBeenCalledTimes(1)); await waitFor(() => expect(retryFnMock).toHaveBeenCalledTimes(1)); - expect(updateUnreadCountMock).not.toHaveBeenCalled(); }); it('should fetch unread count if all notifications are selected upon deleting notifications', async () => { @@ -115,7 +107,6 @@ describe('Action Buttons', () => { retryFn={retryFnMock} isAllSelected={true} setSelected={setSelectedMock} - updateUnreadCount={updateUnreadCountMock} /> , ); @@ -126,7 +117,6 @@ describe('Action Buttons', () => { ); await waitFor(() => expect(setSelectedMock).toHaveBeenCalledTimes(1)); await waitFor(() => expect(retryFnMock).toHaveBeenCalledTimes(1)); - expect(updateUnreadCountMock).toHaveBeenCalled(); }); // Error are consoled in all cases of POST error @@ -141,7 +131,6 @@ describe('Action Buttons', () => { retryFn={retryFnMock} isAllSelected={false} setSelected={setSelectedMock} - updateUnreadCount={updateUnreadCountMock} unreadCountInSelected={1} /> , @@ -165,7 +154,6 @@ describe('Action Buttons', () => { retryFn={retryFnMock} isAllSelected={true} setSelected={setSelectedMock} - updateUnreadCount={updateUnreadCountMock} /> , ); @@ -189,7 +177,6 @@ describe('Action Buttons', () => { retryFn={retryFnMock} isAllSelected={false} setSelected={setSelectedMock} - updateUnreadCount={updateUnreadCountMock} unreadCountInSelected={1} /> , @@ -213,7 +200,6 @@ describe('Action Buttons', () => { retryFn={retryFnMock} isAllSelected={true} setSelected={setSelectedMock} - updateUnreadCount={updateUnreadCountMock} /> , ); diff --git a/frontend/src/components/notifications/tests/notificationCard.test.js b/frontend/src/components/notifications/tests/notificationCard.test.js index 4ab28b8639..2abf3cbfc3 100644 --- a/frontend/src/components/notifications/tests/notificationCard.test.js +++ b/frontend/src/components/notifications/tests/notificationCard.test.js @@ -112,13 +112,9 @@ describe('Notification Card', () => { /> , ); - await user.click(screen.getByText(/requested to join/i)); + await user.click(screen.getByText(/sample subject/i)); // Awaiting portion of the notification message inside the dialog - await waitFor(() => - expect( - screen.getByText(/Access the team management page to accept or reject that request./i), - ).toBeInTheDocument(), - ); + await waitFor(() => expect(screen.getByText(/Sample message/i)).toBeInTheDocument()); }); }); @@ -131,11 +127,7 @@ describe('Notification Card Mini', () => { , ); await user.click(screen.getByRole('article')); - await waitFor(() => - expect( - screen.getByText(/Access the team management page to accept or reject that request./i), - ).toBeInTheDocument(), - ); + await waitFor(() => expect(screen.getByText(/Sample message/i)).toBeInTheDocument()); await user.click( screen.getByRole('button', { name: /close/i, diff --git a/frontend/src/views/tests/notifications.test.js b/frontend/src/views/tests/notifications.test.js index 5ed76d59ee..edb16cf584 100644 --- a/frontend/src/views/tests/notifications.test.js +++ b/frontend/src/views/tests/notifications.test.js @@ -32,6 +32,6 @@ describe('Notifications Page', () => { ); await waitFor(() => expect(screen.getAllByText('Team announcement')[0]).toBeInTheDocument()); - expect(screen.getAllByRole('article').length).toBe(4); + expect(screen.getAllByRole('article').length).toBe(10); }); }); From 879b4c37fe7165ad58c1ea06892adf3cea2afd1f Mon Sep 17 00:00:00 2001 From: Hel Nershing Thapa Date: Tue, 6 Jun 2023 16:46:35 +0545 Subject: [PATCH 7/9] Add test case for decrementing page query when deleting all notifications on the last page --- .../notifications/tests/actionButtons.test.js | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/frontend/src/components/notifications/tests/actionButtons.test.js b/frontend/src/components/notifications/tests/actionButtons.test.js index 03602ffe74..ba20f35263 100644 --- a/frontend/src/components/notifications/tests/actionButtons.test.js +++ b/frontend/src/components/notifications/tests/actionButtons.test.js @@ -6,6 +6,7 @@ import { setupFaultyHandlers } from '../../../network/tests/server'; import { store } from '../../../store'; import { ActionButtons } from '../actionButtons'; import { ReduxIntlProviders } from '../../../utils/testWithIntl'; +import { generateSampleNotifications } from '../../../network/tests/mockData/notifications'; describe('Action Buttons', () => { const retryFnMock = jest.fn(); @@ -210,4 +211,35 @@ describe('Action Buttons', () => { ); // Error is then consoled }); + + it('should decrement the page query by 1 if the user deletes all notifications on the last page', async () => { + // ACT: there are 3 notifications pages in total, and we're trying to delete + // all the six notifications in the last page + const setInboxQueryMock = jest.fn(); + render( + + + , + ); + const user = userEvent.setup(); + await user.click( + screen.getByRole('button', { + name: /delete/i, + }), + ); + await waitFor(() => + expect(setInboxQueryMock).toHaveBeenCalledWith({ page: 2, types: undefined }, 'pushIn'), + ); + await waitFor(() => expect(setSelectedMock).toHaveBeenCalledWith([])); + expect(retryFnMock).not.toBeCalled(); + }); }); From f85c46758c55cbacf56a5babb17832843662a34e Mon Sep 17 00:00:00 2001 From: Hel Nershing Thapa Date: Tue, 6 Jun 2023 16:47:47 +0545 Subject: [PATCH 8/9] Update overall test cases for notifications page --- .../header/tests/notificationBell.test.js | 17 ++- .../notifications/tests/inboxNav.test.js | 45 +++++++- .../tests/notificationBodyCard.test.js | 103 ++++++++++++++++++ .../tests/notificationOrderBy.test.js | 42 +++++++ .../tests/notificationResults.test.js | 15 +++ .../notifications/tests/paginator.test.js | 25 +++++ frontend/src/network/tests/server-handlers.js | 1 + 7 files changed, 244 insertions(+), 4 deletions(-) create mode 100644 frontend/src/components/notifications/tests/notificationBodyCard.test.js create mode 100644 frontend/src/components/notifications/tests/notificationOrderBy.test.js create mode 100644 frontend/src/components/notifications/tests/paginator.test.js diff --git a/frontend/src/components/header/tests/notificationBell.test.js b/frontend/src/components/header/tests/notificationBell.test.js index afe72abc57..aee0154255 100644 --- a/frontend/src/components/header/tests/notificationBell.test.js +++ b/frontend/src/components/header/tests/notificationBell.test.js @@ -3,7 +3,11 @@ import { act, screen, waitFor, within } from '@testing-library/react'; import '../../../utils/mockMatchMedia'; import { store } from '../../../store'; -import { ReduxIntlProviders, renderWithRouter } from '../../../utils/testWithIntl'; +import { + ReduxIntlProviders, + createComponentWithMemoryRouter, + renderWithRouter, +} from '../../../utils/testWithIntl'; import { NotificationBell } from '../notificationBell'; describe('Notification Bell', () => { @@ -45,4 +49,15 @@ describe('Notification Bell', () => { expect(container.querySelector('redicon')).not.toBeInTheDocument(); }); }); + + it('should navigate to the notifications page', async () => { + const { router } = createComponentWithMemoryRouter( + + + , + ); + const user = userEvent.setup(); + await user.click(await screen.findByText(/208 unread/i)); + await waitFor(() => expect(router.state.location.pathname).toBe('/inbox')); + }); }); diff --git a/frontend/src/components/notifications/tests/inboxNav.test.js b/frontend/src/components/notifications/tests/inboxNav.test.js index fef21ce309..9c855271b3 100644 --- a/frontend/src/components/notifications/tests/inboxNav.test.js +++ b/frontend/src/components/notifications/tests/inboxNav.test.js @@ -1,10 +1,16 @@ import '@testing-library/jest-dom'; -import { screen } from '@testing-library/react'; +import { act, screen } from '@testing-library/react'; import { QueryParamProvider } from 'use-query-params'; import { ReactRouter6Adapter } from 'use-query-params/adapters/react-router-6'; +import userEvent from '@testing-library/user-event'; -import { InboxNav } from '../inboxNav'; -import { IntlProviders, createComponentWithMemoryRouter } from '../../../utils/testWithIntl'; +import { InboxNav, InboxNavMini, InboxNavMiniBottom } from '../inboxNav'; +import { + IntlProviders, + ReduxIntlProviders, + createComponentWithMemoryRouter, +} from '../../../utils/testWithIntl'; +import { store } from '../../../store'; describe('Inbox Nav', () => { it('should display styles for active tab and clear filters button', () => { @@ -37,4 +43,37 @@ describe('Inbox Nav', () => { }), ).toBeInTheDocument(); }); + + it('should set the popout focus of notification popover to false on navigating to the notifications page', async () => { + const setPopoutFocusMock = jest.fn(); + createComponentWithMemoryRouter( + + + , + ); + const user = userEvent.setup(); + await user.click( + screen.getByRole('link', { + name: /go to notifications/i, + }), + ); + expect(setPopoutFocusMock).toHaveBeenCalledTimes(1); + expect(setPopoutFocusMock).toHaveBeenCalledWith(false); + }); + + it("should show '1 new notification' text for single message", () => { + act(() => { + store.dispatch({ type: 'SET_UNREAD_COUNT', payload: 1 }); + }); + createComponentWithMemoryRouter( + + + , + ); + expect( + screen.getByRole('link', { + name: /1 unread notification/i, + }), + ).toBeInTheDocument(); + }); }); diff --git a/frontend/src/components/notifications/tests/notificationBodyCard.test.js b/frontend/src/components/notifications/tests/notificationBodyCard.test.js new file mode 100644 index 0000000000..1089e15310 --- /dev/null +++ b/frontend/src/components/notifications/tests/notificationBodyCard.test.js @@ -0,0 +1,103 @@ +import '@testing-library/jest-dom'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { NotificationBodyCard, NotificationBodyModal } from '../notificationBodyCard'; +import { ReduxIntlProviders, renderWithRouter } from '../../../utils/testWithIntl'; +import { setupFaultyHandlers } from '../../../network/tests/server'; +import { generateSampleNotifications } from '../../../network/tests/mockData/notifications'; +import { ORG_NAME } from '../../../config'; + +describe('Notification Body Modal', () => { + it('should close the notification bell popup if it is open', () => { + const setPopoutFocusMock = jest.fn(); + render( + + + , + ); + expect(setPopoutFocusMock).toHaveBeenCalled(); + }); + + it('should open link on the notification text', async () => { + const originalOpen = window.open; + window.open = jest.fn(); + renderWithRouter( + + + , + ); + const user = userEvent.setup(); + await user.click( + await screen.findByRole('link', { + name: /example.com/i, + }), + ); + expect(window.open).toHaveBeenCalledTimes(1); + expect(window.open).toHaveBeenCalledWith('https://example.com/'); + // Restore the original window.open method + window.open = originalOpen; + }); + + it('should display error message when notification cannnot be fetched', async () => { + setupFaultyHandlers(); + renderWithRouter( + + + , + ); + await waitFor(() => + expect(screen.getByText(/error loading the notification/i)).toBeInTheDocument(), + ); + }); +}); + +describe('Notification Body Card Deletion', () => { + it('should delete the notification and call close notification modal function prop', async () => { + const sampleNotification = generateSampleNotifications(1)[0]; + const retryFnMock = jest.fn(); + const closeModalMock = jest.fn(); + renderWithRouter( + + + , + ); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /delete/i })); + await waitFor(() => { + expect(retryFnMock).toHaveBeenCalled(); + expect(closeModalMock).toHaveBeenCalled(); + }); + }); + + it('should log the error message when notification deletion error occurs', async () => { + const logSpy = jest.spyOn(console, 'log'); + const sampleNotification = generateSampleNotifications(1)[0]; + setupFaultyHandlers(); + renderWithRouter( + + + , + ); + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /delete/i })); + await waitFor(() => expect(logSpy).toHaveBeenCalled()); + }); + + it('should display organization name on system generate notifications', async () => { + let sampleNotification = generateSampleNotifications(1)[0]; + sampleNotification.fromUsername = null; + renderWithRouter( + + + , + ); + if (ORG_NAME) { + expect(screen.getByText(ORG_NAME)).toBeInTheDocument(); + } + }); +}); diff --git a/frontend/src/components/notifications/tests/notificationOrderBy.test.js b/frontend/src/components/notifications/tests/notificationOrderBy.test.js new file mode 100644 index 0000000000..3c18b474cc --- /dev/null +++ b/frontend/src/components/notifications/tests/notificationOrderBy.test.js @@ -0,0 +1,42 @@ +import '@testing-library/jest-dom'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { IntlProviders, createComponentWithMemoryRouter } from '../../../utils/testWithIntl'; +import { NotificationOrderBySelector } from '../notificationOrderBy'; + +describe('Notification Order By', () => { + it('should display the sort by button when no query param is available', () => { + render( + + + , + ); + expect( + screen.getByRole('button', { + name: /sort by/i, + }), + ).toBeInTheDocument(); + }); + + it('should display the sort by button when no query param is available', async () => { + const setQueryMock = jest.fn(); + createComponentWithMemoryRouter( + + + , + ); + const user = userEvent.setup(); + await user.click( + screen.getByRole('button', { + name: /sort by/i, + }), + ); + await user.click(screen.getByText(/new notifications first/i)); + expect(setQueryMock).toHaveBeenCalledTimes(1); + expect(setQueryMock).toHaveBeenCalledWith( + { orderBy: 'DESC', orderByType: 'date', page: undefined }, + 'pushIn', + ); + }); +}); diff --git a/frontend/src/components/notifications/tests/notificationResults.test.js b/frontend/src/components/notifications/tests/notificationResults.test.js index 98180e1c2c..76615c3c6d 100644 --- a/frontend/src/components/notifications/tests/notificationResults.test.js +++ b/frontend/src/components/notifications/tests/notificationResults.test.js @@ -68,4 +68,19 @@ describe('Notifications Results', () => { ); expect(screen.getByText(messages.noMessages.defaultMessage)).toBeInTheDocument(); }); + + it('should display select all notifications button', async () => { + createComponentWithMemoryRouter( + + + , + ); + const user = userEvent.setup(); + await user.click(screen.getAllByRole('checkbox')[0]); + expect( + screen.getByRole('button', { + name: /Select all 208 notifications/i, + }), + ).toBeInTheDocument(); + }); }); diff --git a/frontend/src/components/notifications/tests/paginator.test.js b/frontend/src/components/notifications/tests/paginator.test.js new file mode 100644 index 0000000000..032d9b5024 --- /dev/null +++ b/frontend/src/components/notifications/tests/paginator.test.js @@ -0,0 +1,25 @@ +import { render, screen } from '@testing-library/react'; +import Paginator from '../paginator'; +import userEvent from '@testing-library/user-event'; + +test('should set the active page', async () => { + const setInboxQueryMock = jest.fn(); + render( + , + ); + const user = userEvent.setup(); + await user.click( + screen.getByRole('button', { + name: /2/i, + }), + ); + expect(setInboxQueryMock).toBeCalledWith({ page: 2 }, 'pushIn'); +}); diff --git a/frontend/src/network/tests/server-handlers.js b/frontend/src/network/tests/server-handlers.js index f71d78cd3e..993e02e77e 100644 --- a/frontend/src/network/tests/server-handlers.js +++ b/frontend/src/network/tests/server-handlers.js @@ -400,6 +400,7 @@ const faultyHandlers = [ rest.post(API_URL + 'notifications/mark-as-read-all/', failedToConnectError), rest.post(API_URL + 'notifications/mark-as-read-all/:types', failedToConnectError), rest.post(API_URL + 'notifications/mark-as-read-multiple/', failedToConnectError), + rest.get(API_URL + 'notifications/:id/', failedToConnectError), rest.delete(API_URL + 'notifications/:id/', failedToConnectError), rest.patch(API_URL + 'users/:username/actions/set-level/:level', failedToConnectError), rest.patch(API_URL + 'users/:username/actions/set-role/:role', failedToConnectError), From a64180f963dfa6866ed8a6f90c95b85f6d04a59c Mon Sep 17 00:00:00 2001 From: Hel Nershing Thapa Date: Tue, 6 Jun 2023 21:27:25 +0545 Subject: [PATCH 9/9] Refactor userEvent implementation to adhere to recommendations --- .../components/header/tests/notificationBell.test.js | 3 +-- .../notifications/tests/actionButtons.test.js | 10 +++++----- .../notifications/tests/notificationCard.test.js | 3 --- .../notifications/tests/notificationResults.test.js | 3 +-- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/frontend/src/components/header/tests/notificationBell.test.js b/frontend/src/components/header/tests/notificationBell.test.js index aee0154255..f82e68b668 100644 --- a/frontend/src/components/header/tests/notificationBell.test.js +++ b/frontend/src/components/header/tests/notificationBell.test.js @@ -51,12 +51,11 @@ describe('Notification Bell', () => { }); it('should navigate to the notifications page', async () => { - const { router } = createComponentWithMemoryRouter( + const { router, user } = createComponentWithMemoryRouter( , ); - const user = userEvent.setup(); await user.click(await screen.findByText(/208 unread/i)); await waitFor(() => expect(router.state.location.pathname).toBe('/inbox')); }); diff --git a/frontend/src/components/notifications/tests/actionButtons.test.js b/frontend/src/components/notifications/tests/actionButtons.test.js index ba20f35263..8bd32c0af0 100644 --- a/frontend/src/components/notifications/tests/actionButtons.test.js +++ b/frontend/src/components/notifications/tests/actionButtons.test.js @@ -122,8 +122,8 @@ describe('Action Buttons', () => { // Error are consoled in all cases of POST error it('should catch error when marking multiple selected notifications as read', async () => { - setupFaultyHandlers(); const user = userEvent.setup(); + setupFaultyHandlers(); render( { }); it('should catch error when marking all notifications in a category as read', async () => { - setupFaultyHandlers(); const user = userEvent.setup(); + setupFaultyHandlers(); render( { }); it('should catch error when deleting multiple selected notifications', async () => { + const user = userEvent.setup(); act(() => {}); setupFaultyHandlers(); - const user = userEvent.setup(); render( { }); it('should catch error when deleting all notifications in a category', async () => { - setupFaultyHandlers(); const user = userEvent.setup(); + setupFaultyHandlers(); render( { // ACT: there are 3 notifications pages in total, and we're trying to delete // all the six notifications in the last page const setInboxQueryMock = jest.fn(); + const user = userEvent.setup(); render( { /> , ); - const user = userEvent.setup(); await user.click( screen.getByRole('button', { name: /delete/i, diff --git a/frontend/src/components/notifications/tests/notificationCard.test.js b/frontend/src/components/notifications/tests/notificationCard.test.js index 2abf3cbfc3..d4be1fd5ee 100644 --- a/frontend/src/components/notifications/tests/notificationCard.test.js +++ b/frontend/src/components/notifications/tests/notificationCard.test.js @@ -30,7 +30,6 @@ describe('Notification Card', () => { /> , ); - await user.click( screen.getByRole('button', { name: /Mark notification as read/i, @@ -51,7 +50,6 @@ describe('Notification Card', () => { /> , ); - await user.click(screen.getAllByRole('button')[1]); await waitFor(() => { expect(fetchNotificationsMock).toHaveBeenCalledTimes(1); @@ -100,7 +98,6 @@ describe('Notification Card', () => { it('should open notification modal', async () => { global.open = jest.fn(); - const setSelectedMock = jest.fn(); const { user } = renderWithRouter( diff --git a/frontend/src/components/notifications/tests/notificationResults.test.js b/frontend/src/components/notifications/tests/notificationResults.test.js index 76615c3c6d..ff1ea66aa6 100644 --- a/frontend/src/components/notifications/tests/notificationResults.test.js +++ b/frontend/src/components/notifications/tests/notificationResults.test.js @@ -70,12 +70,11 @@ describe('Notifications Results', () => { }); it('should display select all notifications button', async () => { - createComponentWithMemoryRouter( + const { user } = createComponentWithMemoryRouter( , ); - const user = userEvent.setup(); await user.click(screen.getAllByRole('checkbox')[0]); expect( screen.getByRole('button', {