From ab52d314ff49f474d62079337a99c6219beaf96a Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 15 Nov 2022 10:27:13 +0000 Subject: [PATCH] Make clear notifications work with threads (#9575) --- .../views/settings/Notifications.tsx | 52 +++--- src/utils/notifications.ts | 30 ++++ .../views/settings/Notifications-test.tsx | 164 +++++++++-------- .../__snapshots__/Notifications-test.tsx.snap | 169 +++--------------- test/utils/notifications-test.ts | 63 +++++++ 5 files changed, 241 insertions(+), 237 deletions(-) diff --git a/src/components/views/settings/Notifications.tsx b/src/components/views/settings/Notifications.tsx index 6c44f9979cb..fdb8aba9f14 100644 --- a/src/components/views/settings/Notifications.tsx +++ b/src/components/views/settings/Notifications.tsx @@ -42,7 +42,7 @@ import AccessibleButton from "../elements/AccessibleButton"; import TagComposer from "../elements/TagComposer"; import { objectClone } from "../../../utils/objects"; import { arrayDiff } from "../../../utils/arrays"; -import { getLocalNotificationAccountDataEventType } from "../../../utils/notifications"; +import { clearAllNotifications, getLocalNotificationAccountDataEventType } from "../../../utils/notifications"; // TODO: this "view" component still has far too much application logic in it, // which should be factored out to other files. @@ -112,6 +112,8 @@ interface IState { desktopNotifications: boolean; desktopShowBody: boolean; audioNotifications: boolean; + + clearingNotifications: boolean; } export default class Notifications extends React.PureComponent { @@ -126,6 +128,7 @@ export default class Notifications extends React.PureComponent { desktopNotifications: SettingsStore.getValue("notificationsEnabled"), desktopShowBody: SettingsStore.getValue("notificationBodyEnabled"), audioNotifications: SettingsStore.getValue("audioNotificationsEnabled"), + clearingNotifications: false, }; this.settingWatchers = [ @@ -177,8 +180,12 @@ export default class Notifications extends React.PureComponent { ])).reduce((p, c) => Object.assign(c, p), {}); this.setState - >({ + "deviceNotificationsEnabled" | + "desktopNotifications" | + "desktopShowBody" | + "audioNotifications" | + "clearingNotifications" + >>({ ...newState, phase: Phase.Ready, }); @@ -433,17 +440,14 @@ export default class Notifications extends React.PureComponent { } }; - private onClearNotificationsClicked = () => { - const client = MatrixClientPeg.get(); - client.getRooms().forEach(r => { - if (r.getUnreadNotificationCount() > 0) { - const events = r.getLiveTimeline().getEvents(); - if (events.length) { - // noinspection JSIgnoredPromiseFromCall - client.sendReadReceipt(events[events.length - 1]); - } - } - }); + private onClearNotificationsClicked = async (): Promise => { + try { + this.setState({ clearingNotifications: true }); + const client = MatrixClientPeg.get(); + await clearAllNotifications(client); + } finally { + this.setState({ clearingNotifications: false }); + } }; private async setKeywords(keywords: string[], originalRules: IAnnotatedPushRule[]) { @@ -531,7 +535,7 @@ export default class Notifications extends React.PureComponent { private renderTopSection() { const masterSwitch = { const emailSwitches = (this.state.threepids || []).filter(t => t.medium === ThreepidMedium.Email) .map(e => p.kind === "email" && p.pushkey === e.address)} label={_t("Enable email notifications for %(email)s", { email: e.address })} @@ -558,7 +562,7 @@ export default class Notifications extends React.PureComponent { { masterSwitch } this.updateDeviceNotifications(checked)} @@ -567,21 +571,21 @@ export default class Notifications extends React.PureComponent { { this.state.deviceNotificationsEnabled && (<> { ) { clearNotifsButton = { _t("Clear notifications") }; } @@ -653,7 +659,7 @@ export default class Notifications extends React.PureComponent { const fieldsetRows = this.state.vectorPushRules[category].map(r =>
{ r.description } @@ -678,7 +684,7 @@ export default class Notifications extends React.PureComponent { } return <> -
+
{ sectionName } { VectorStateToLabel[VectorState.Off] } { VectorStateToLabel[VectorState.On] } @@ -715,7 +721,7 @@ export default class Notifications extends React.PureComponent { // Ends up default centered return ; } else if (this.state.phase === Phase.Error) { - return

{ _t("There was an error loading your notification settings.") }

; + return

{ _t("There was an error loading your notification settings.") }

; } return
diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 32296d62e6e..2b08f406dc8 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -17,6 +17,8 @@ limitations under the License. import { MatrixClient } from "matrix-js-sdk/src/client"; import { LOCAL_NOTIFICATION_SETTINGS_PREFIX } from "matrix-js-sdk/src/@types/event"; import { LocalNotificationSettings } from "matrix-js-sdk/src/@types/local_notifications"; +import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts"; +import { Room } from "matrix-js-sdk/src/models/room"; import SettingsStore from "../settings/SettingsStore"; @@ -56,3 +58,31 @@ export function localNotificationsAreSilenced(cli: MatrixClient): boolean { const event = cli.getAccountData(eventType); return event?.getContent()?.is_silenced ?? false; } + +export function clearAllNotifications(client: MatrixClient): Promise> { + const receiptPromises = client.getRooms().reduce((promises, room: Room) => { + if (room.getUnreadNotificationCount() > 0) { + const roomEvents = room.getLiveTimeline().getEvents(); + const lastThreadEvents = room.lastThread?.events; + + const lastRoomEvent = roomEvents?.[roomEvents?.length - 1]; + const lastThreadLastEvent = lastThreadEvents?.[lastThreadEvents?.length - 1]; + + const lastEvent = (lastRoomEvent?.getTs() ?? 0) > (lastThreadLastEvent?.getTs() ?? 0) + ? lastRoomEvent + : lastThreadLastEvent; + + if (lastEvent) { + const receiptType = SettingsStore.getValue("sendReadReceipts", room.roomId) + ? ReceiptType.Read + : ReceiptType.ReadPrivate; + const promise = client.sendReadReceipt(lastEvent, receiptType, true); + promises.push(promise); + } + } + + return promises; + }, []); + + return Promise.all(receiptPromises); +} diff --git a/test/components/views/settings/Notifications-test.tsx b/test/components/views/settings/Notifications-test.tsx index 88deaa2c0f6..da2bc9f3f81 100644 --- a/test/components/views/settings/Notifications-test.tsx +++ b/test/components/views/settings/Notifications-test.tsx @@ -13,8 +13,6 @@ limitations under the License. */ import React from 'react'; -// eslint-disable-next-line deprecate/import -import { mount, ReactWrapper } from 'enzyme'; import { IPushRule, IPushRules, @@ -22,14 +20,17 @@ import { IPusher, LOCAL_NOTIFICATION_SETTINGS_PREFIX, MatrixEvent, + Room, + NotificationCountType, } from 'matrix-js-sdk/src/matrix'; import { IThreepid, ThreepidMedium } from 'matrix-js-sdk/src/@types/threepids'; import { act } from 'react-dom/test-utils'; +import { fireEvent, getByTestId, render, screen, waitFor } from '@testing-library/react'; import Notifications from '../../../../src/components/views/settings/Notifications'; import SettingsStore from "../../../../src/settings/SettingsStore"; import { StandardActions } from '../../../../src/notifications/StandardActions'; -import { getMockClientWithEventEmitter } from '../../../test-utils'; +import { getMockClientWithEventEmitter, mkMessage } from '../../../test-utils'; // don't pollute test output with error logs from mock rejections jest.mock("matrix-js-sdk/src/logger"); @@ -56,13 +57,12 @@ const pushRules: IPushRules = { "global": { "underride": [{ "conditions": [{ "ki const flushPromises = async () => await new Promise(resolve => setTimeout(resolve)); describe('', () => { - const getComponent = () => mount(); + const getComponent = () => render(); // get component, wait for async data and force a render const getComponentAndWait = async () => { const component = getComponent(); await flushPromises(); - component.setProps({}); return component; }; @@ -85,11 +85,11 @@ describe('', () => { } }), setAccountData: jest.fn(), + sendReadReceipt: jest.fn(), + supportsExperimentalThreads: jest.fn().mockReturnValue(true), }); mockClient.getPushRules.mockResolvedValue(pushRules); - const findByTestId = (component, id) => component.find(`[data-test-id="${id}"]`); - beforeEach(() => { mockClient.getPushRules.mockClear().mockResolvedValue(pushRules); mockClient.getPushers.mockClear().mockResolvedValue({ pushers: [] }); @@ -97,25 +97,25 @@ describe('', () => { mockClient.setPusher.mockClear().mockResolvedValue({}); }); - it('renders spinner while loading', () => { - const component = getComponent(); - expect(component.find('.mx_Spinner').length).toBeTruthy(); + it('renders spinner while loading', async () => { + getComponent(); + expect(screen.getByTestId('spinner')).toBeInTheDocument(); }); it('renders error message when fetching push rules fails', async () => { mockClient.getPushRules.mockRejectedValue({}); - const component = await getComponentAndWait(); - expect(findByTestId(component, 'error-message').length).toBeTruthy(); + await getComponentAndWait(); + expect(screen.getByTestId('error-message')).toBeInTheDocument(); }); it('renders error message when fetching pushers fails', async () => { mockClient.getPushers.mockRejectedValue({}); - const component = await getComponentAndWait(); - expect(findByTestId(component, 'error-message').length).toBeTruthy(); + await getComponentAndWait(); + expect(screen.getByTestId('error-message')).toBeInTheDocument(); }); it('renders error message when fetching threepids fails', async () => { mockClient.getThreePids.mockRejectedValue({}); - const component = await getComponentAndWait(); - expect(findByTestId(component, 'error-message').length).toBeTruthy(); + await getComponentAndWait(); + expect(screen.getByTestId('error-message')).toBeInTheDocument(); }); describe('main notification switches', () => { @@ -127,18 +127,18 @@ describe('', () => { }, } as unknown as IPushRules; mockClient.getPushRules.mockClear().mockResolvedValue(disableNotificationsPushRules); - const component = await getComponentAndWait(); + const { container } = await getComponentAndWait(); - expect(component).toMatchSnapshot(); + expect(container).toMatchSnapshot(); }); it('renders switches correctly', async () => { - const component = await getComponentAndWait(); + await getComponentAndWait(); - expect(findByTestId(component, 'notif-master-switch').length).toBeTruthy(); - expect(findByTestId(component, 'notif-device-switch').length).toBeTruthy(); - expect(findByTestId(component, 'notif-setting-notificationsEnabled').length).toBeTruthy(); - expect(findByTestId(component, 'notif-setting-notificationBodyEnabled').length).toBeTruthy(); - expect(findByTestId(component, 'notif-setting-audioNotificationsEnabled').length).toBeTruthy(); + expect(screen.getByTestId('notif-master-switch')).toBeInTheDocument(); + expect(screen.getByTestId('notif-device-switch')).toBeInTheDocument(); + expect(screen.getByTestId('notif-setting-notificationsEnabled')).toBeInTheDocument(); + expect(screen.getByTestId('notif-setting-notificationBodyEnabled')).toBeInTheDocument(); + expect(screen.getByTestId('notif-setting-audioNotificationsEnabled')).toBeInTheDocument(); }); describe('email switches', () => { @@ -156,9 +156,8 @@ describe('', () => { }); it('renders email switches correctly when email 3pids exist', async () => { - const component = await getComponentAndWait(); - - expect(findByTestId(component, 'notif-email-switch')).toMatchSnapshot(); + await getComponentAndWait(); + expect(screen.getByTestId('notif-email-switch')).toBeInTheDocument(); }); it('renders email switches correctly when notifications are on for email', async () => { @@ -167,19 +166,20 @@ describe('', () => { { kind: 'email', pushkey: testEmail } as unknown as IPusher, ], }); - const component = await getComponentAndWait(); + await getComponentAndWait(); - expect(findByTestId(component, 'notif-email-switch').props().value).toEqual(true); + const emailSwitch = screen.getByTestId('notif-email-switch'); + expect(emailSwitch.querySelector('[aria-checked="true"]')).toBeInTheDocument(); }); it('enables email notification when toggling on', async () => { - const component = await getComponentAndWait(); + await getComponentAndWait(); - const emailToggle = findByTestId(component, 'notif-email-switch') - .find('div[role="switch"]'); + const emailToggle = screen.getByTestId('notif-email-switch') + .querySelector('div[role="switch"]'); await act(async () => { - emailToggle.simulate('click'); + fireEvent.click(emailToggle); }); expect(mockClient.setPusher).toHaveBeenCalledWith(expect.objectContaining({ @@ -194,32 +194,31 @@ describe('', () => { it('displays error when pusher update fails', async () => { mockClient.setPusher.mockRejectedValue({}); - const component = await getComponentAndWait(); + await getComponentAndWait(); - const emailToggle = findByTestId(component, 'notif-email-switch') - .find('div[role="switch"]'); + const emailToggle = screen.getByTestId('notif-email-switch') + .querySelector('div[role="switch"]'); await act(async () => { - emailToggle.simulate('click'); + fireEvent.click(emailToggle); }); // force render await flushPromises(); - await component.setProps({}); - expect(findByTestId(component, 'error-message').length).toBeTruthy(); + expect(screen.getByTestId('error-message')).toBeInTheDocument(); }); it('enables email notification when toggling off', async () => { const testPusher = { kind: 'email', pushkey: 'tester@test.com' } as unknown as IPusher; mockClient.getPushers.mockResolvedValue({ pushers: [testPusher] }); - const component = await getComponentAndWait(); + await getComponentAndWait(); - const emailToggle = findByTestId(component, 'notif-email-switch') - .find('div[role="switch"]'); + const emailToggle = screen.getByTestId('notif-email-switch') + .querySelector('div[role="switch"]'); await act(async () => { - emailToggle.simulate('click'); + fireEvent.click(emailToggle); }); expect(mockClient.setPusher).toHaveBeenCalledWith({ @@ -229,67 +228,64 @@ describe('', () => { }); it('toggles and sets settings correctly', async () => { - const component = await getComponentAndWait(); - let audioNotifsToggle: ReactWrapper; + await getComponentAndWait(); + let audioNotifsToggle; const update = () => { - audioNotifsToggle = findByTestId(component, 'notif-setting-audioNotificationsEnabled') - .find('div[role="switch"]'); + audioNotifsToggle = screen.getByTestId('notif-setting-audioNotificationsEnabled') + .querySelector('div[role="switch"]'); }; update(); - expect(audioNotifsToggle.getDOMNode().getAttribute("aria-checked")).toEqual("true"); + expect(audioNotifsToggle.getAttribute("aria-checked")).toEqual("true"); expect(SettingsStore.getValue("audioNotificationsEnabled")).toEqual(true); - act(() => { audioNotifsToggle.simulate('click'); }); + act(() => { fireEvent.click(audioNotifsToggle); }); update(); - expect(audioNotifsToggle.getDOMNode().getAttribute("aria-checked")).toEqual("false"); + expect(audioNotifsToggle.getAttribute("aria-checked")).toEqual("false"); expect(SettingsStore.getValue("audioNotificationsEnabled")).toEqual(false); }); }); describe('individual notification level settings', () => { - const getCheckedRadioForRule = (ruleEl) => - ruleEl.find('input[type="radio"][checked=true]').props()['aria-label']; it('renders categories correctly', async () => { - const component = await getComponentAndWait(); + await getComponentAndWait(); - expect(findByTestId(component, 'notif-section-vector_global').length).toBeTruthy(); - expect(findByTestId(component, 'notif-section-vector_mentions').length).toBeTruthy(); - expect(findByTestId(component, 'notif-section-vector_other').length).toBeTruthy(); + expect(screen.getByTestId('notif-section-vector_global')).toBeInTheDocument(); + expect(screen.getByTestId('notif-section-vector_mentions')).toBeInTheDocument(); + expect(screen.getByTestId('notif-section-vector_other')).toBeInTheDocument(); }); it('renders radios correctly', async () => { - const component = await getComponentAndWait(); + await getComponentAndWait(); const section = 'vector_global'; - const globalSection = findByTestId(component, `notif-section-${section}`); + const globalSection = screen.getByTestId(`notif-section-${section}`); // 4 notification rules with class 'global' - expect(globalSection.find('fieldset').length).toEqual(4); + expect(globalSection.querySelectorAll('fieldset').length).toEqual(4); // oneToOneRule is set to 'on' - const oneToOneRuleElement = findByTestId(component, section + oneToOneRule.rule_id); - expect(getCheckedRadioForRule(oneToOneRuleElement)).toEqual('On'); + const oneToOneRuleElement = screen.getByTestId(section + oneToOneRule.rule_id); + expect(oneToOneRuleElement.querySelector("[aria-label='On']")).toBeInTheDocument(); // encryptedOneToOneRule is set to 'loud' - const encryptedOneToOneElement = findByTestId(component, section + encryptedOneToOneRule.rule_id); - expect(getCheckedRadioForRule(encryptedOneToOneElement)).toEqual('Noisy'); + const encryptedOneToOneElement = screen.getByTestId(section + encryptedOneToOneRule.rule_id); + expect(encryptedOneToOneElement.querySelector("[aria-label='Noisy']")).toBeInTheDocument(); // encryptedGroupRule is set to 'off' - const encryptedGroupElement = findByTestId(component, section + encryptedGroupRule.rule_id); - expect(getCheckedRadioForRule(encryptedGroupElement)).toEqual('Off'); + const encryptedGroupElement = screen.getByTestId(section + encryptedGroupRule.rule_id); + expect(encryptedGroupElement.querySelector("[aria-label='Off']")).toBeInTheDocument(); }); it('updates notification level when changed', async () => { - const component = await getComponentAndWait(); + await getComponentAndWait(); const section = 'vector_global'; // oneToOneRule is set to 'on' // and is kind: 'underride' - const oneToOneRuleElement = findByTestId(component, section + oneToOneRule.rule_id); + const oneToOneRuleElement = screen.getByTestId(section + oneToOneRule.rule_id); await act(async () => { - // toggle at 0 is 'off' - const offToggle = oneToOneRuleElement.find('input[type="radio"]').at(0); - offToggle.simulate('change'); + const offToggle = oneToOneRuleElement.querySelector('input[type="radio"]'); + fireEvent.click(offToggle); }); expect(mockClient.setPushRuleEnabled).toHaveBeenCalledWith( @@ -300,4 +296,32 @@ describe('', () => { 'global', 'underride', oneToOneRule.rule_id, StandardActions.ACTION_DONT_NOTIFY); }); }); + + describe("clear all notifications", () => { + it("clears all notifications", async () => { + const room = new Room("room123", mockClient, "@alice:example.org"); + mockClient.getRooms.mockReset().mockReturnValue([room]); + + const message = mkMessage({ + event: true, + room: "room123", + user: "@alice:example.org", + ts: 1, + }); + room.addLiveEvents([message]); + room.setUnreadNotificationCount(NotificationCountType.Total, 1); + + const { container } = await getComponentAndWait(); + const clearNotificationEl = getByTestId(container, "clear-notifications"); + + fireEvent.click(clearNotificationEl); + + expect(clearNotificationEl.className).toContain("mx_AccessibleButton_disabled"); + expect(mockClient.sendReadReceipt).toHaveBeenCalled(); + + await waitFor(() => { + expect(clearNotificationEl.className).not.toContain("mx_AccessibleButton_disabled"); + }); + }); + }); }); diff --git a/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap b/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap index c00d74b56aa..23fec442b25 100644 --- a/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap +++ b/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap @@ -1,157 +1,38 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[` main notification switches email switches renders email switches correctly when email 3pids exist 1`] = ` - -
- - Enable email notifications for tester@test.com - - <_default - checked={false} - disabled={false} - onChange={[Function]} - title="Enable email notifications for tester@test.com" - > - - -
-
-
- - - -
- -`; - exports[` main notification switches renders only enable notifications switch when notifications are disabled 1`] = ` - +
- -
+ Enable notifications for this account +
- Enable notifications for this account -
- - - Turn off to disable notifications on all your devices and sessions - - + Turn off to disable notifications on all your devices and sessions - <_default - checked={false} - disabled={false} - onChange={[Function]} - title="Enable notifications for this account" - > - - -
-
-
- - - + +
+
- +
- +
`; diff --git a/test/utils/notifications-test.ts b/test/utils/notifications-test.ts index df7134cb1c0..4d5ec53249c 100644 --- a/test/utils/notifications-test.ts +++ b/test/utils/notifications-test.ts @@ -16,15 +16,21 @@ limitations under the License. import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { mocked } from "jest-mock"; +import { MatrixClient } from "matrix-js-sdk/src/client"; +import { NotificationCountType, Room } from "matrix-js-sdk/src/models/room"; +import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts"; import { localNotificationsAreSilenced, getLocalNotificationAccountDataEventType, createLocalNotificationSettingsIfNeeded, deviceNotificationSettingsKeys, + clearAllNotifications, } from "../../src/utils/notifications"; import SettingsStore from "../../src/settings/SettingsStore"; import { getMockClientWithEventEmitter } from "../test-utils/client"; +import { mkMessage, stubClient } from "../test-utils/test-utils"; +import { MatrixClientPeg } from "../../src/MatrixClientPeg"; jest.mock("../../src/settings/SettingsStore"); @@ -99,4 +105,61 @@ describe('notifications', () => { expect(localNotificationsAreSilenced(mockClient)).toBeFalsy(); }); }); + + describe("clearAllNotifications", () => { + let client: MatrixClient; + let room: Room; + let sendReadReceiptSpy; + + const ROOM_ID = "123"; + const USER_ID = "@bob:example.org"; + + beforeEach(() => { + stubClient(); + client = mocked(MatrixClientPeg.get()); + room = new Room(ROOM_ID, client, USER_ID); + sendReadReceiptSpy = jest.spyOn(client, "sendReadReceipt").mockResolvedValue({}); + jest.spyOn(client, "getRooms").mockReturnValue([room]); + jest.spyOn(SettingsStore, "getValue").mockImplementation((name) => { + return name === "sendReadReceipts"; + }); + }); + + it("does not send any requests if everything has been read", () => { + clearAllNotifications(client); + expect(sendReadReceiptSpy).not.toBeCalled(); + }); + + it("sends unthreaded receipt requests", () => { + const message = mkMessage({ + event: true, + room: ROOM_ID, + user: USER_ID, + ts: 1, + }); + room.addLiveEvents([message]); + room.setUnreadNotificationCount(NotificationCountType.Total, 1); + + clearAllNotifications(client); + + expect(sendReadReceiptSpy).toBeCalledWith(message, ReceiptType.Read, true); + }); + + it("sends private read receipts", () => { + const message = mkMessage({ + event: true, + room: ROOM_ID, + user: USER_ID, + ts: 1, + }); + room.addLiveEvents([message]); + room.setUnreadNotificationCount(NotificationCountType.Total, 1); + + jest.spyOn(SettingsStore, "getValue").mockReset().mockReturnValue(false); + + clearAllNotifications(client); + + expect(sendReadReceiptSpy).toBeCalledWith(message, ReceiptType.ReadPrivate, true); + }); + }); });