Skip to content

Commit a2faefa

Browse files
fix: improve mark as read (#31824)
## **Description** Improves notification mark as read performance by not passing down all notifications to our API. Now we only send up notifications that have not been read yet. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/31824?quickstart=1) ## **Related issues** Tentative Fix For: #31422 I still need to investigate to see if there are still issues related to this. ## **Manual testing steps** 1. Have unread notifications 2. Press "Mark All As Read" button 3. See unread notifications become marked as read. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
1 parent a720eda commit a2faefa

File tree

2 files changed

+47
-16
lines changed

2 files changed

+47
-16
lines changed
Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,57 @@
11
import React from 'react';
2-
import { render } from '@testing-library/react';
3-
import { Provider } from 'react-redux';
4-
import configureStore from 'redux-mock-store';
5-
import thunk from 'redux-thunk';
2+
import { fireEvent } from '@testing-library/react';
3+
import { createMockNotificationEthSent } from '@metamask/notification-services-controller/notification-services/mocks';
4+
import { processNotification } from '@metamask/notification-services-controller/notification-services';
5+
6+
import * as UseNotificationModule from '../../hooks/metamask-notifications/useNotifications';
7+
import { renderWithProvider } from '../../../test/lib/render-helpers';
8+
import configureStore from '../../store/store';
9+
import mockState from '../../../test/data/mock-state.json';
610
import { NotificationsListReadAllButton } from './notifications-list-read-all-button';
711

8-
const mockStore = configureStore([thunk]);
9-
const store = mockStore({
10-
metamask: {
11-
metamaskNotificationsList: [],
12-
},
13-
});
12+
const mockNotification = (isRead: boolean) => {
13+
const n = processNotification(createMockNotificationEthSent());
14+
n.isRead = isRead;
15+
return n;
16+
};
1417

1518
describe('NotificationsListReadAllButton', () => {
19+
const store = configureStore(mockState);
20+
1621
it('renders correctly and handles click', () => {
17-
const { getByTestId } = render(
18-
<Provider store={store}>
19-
<NotificationsListReadAllButton notifications={[]} />
20-
</Provider>,
22+
const { getByTestId } = renderWithProvider(
23+
<NotificationsListReadAllButton notifications={[]} />,
24+
store,
2125
);
2226

2327
const button = getByTestId('notifications-list-read-all-button');
2428
expect(button).toBeInTheDocument();
2529
});
30+
31+
it('presses and marks all unread notifications as read', async () => {
32+
const mockMarkAsReadCallback = jest.fn();
33+
jest
34+
.spyOn(UseNotificationModule, 'useMarkNotificationAsRead')
35+
.mockReturnValue({ markNotificationAsRead: mockMarkAsReadCallback });
36+
37+
const notificationList = [
38+
mockNotification(true),
39+
mockNotification(false),
40+
mockNotification(false),
41+
mockNotification(false),
42+
mockNotification(true),
43+
];
44+
45+
const { getByTestId } = renderWithProvider(
46+
<NotificationsListReadAllButton notifications={notificationList} />,
47+
store,
48+
);
49+
50+
const button = getByTestId('notifications-list-read-all-button');
51+
fireEvent.click(button);
52+
53+
expect(mockMarkAsReadCallback).toHaveBeenCalled();
54+
const mockCall = mockMarkAsReadCallback.mock.lastCall[0];
55+
expect(mockCall.length).toBe(3); // 3 unread notifications sent
56+
});
2657
});

ui/pages/notifications/notifications-list-read-all-button.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ export const NotificationsListReadAllButton = ({
3434
if (notifications && notifications.length > 0) {
3535
notificationsRead = notifications
3636
.filter(
37-
(notification): notification is Notification =>
38-
(notification as Notification).id !== undefined,
37+
(notification) =>
38+
notification?.id !== undefined && !notification.isRead,
3939
)
4040
.map((notification: Notification) => ({
4141
id: notification.id,

0 commit comments

Comments
 (0)