From d25b0d5424c04c67e4555bf50b7772d629c5ee54 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 29 Sep 2022 09:01:07 +0100 Subject: [PATCH 1/4] Inhibit desktop and audible notifications when local notifications are silenced --- src/Notifier.ts | 16 +++++++++++++--- src/utils/notifications.ts | 8 +++++++- test/utils/notifications-test.ts | 17 ++++++++++++++++- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/Notifier.ts b/src/Notifier.ts index 80a6661055a..f901669032e 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -46,6 +46,7 @@ import { mediaFromMxc } from "./customisations/Media"; import ErrorDialog from "./components/views/dialogs/ErrorDialog"; import LegacyCallHandler from "./LegacyCallHandler"; import VoipUserMapper from "./VoipUserMapper"; +import { localNotificationsAreSilenced } from "./utils/notifications"; /* * Dispatches: @@ -90,14 +91,18 @@ export const Notifier = { return TextForEvent.textForEvent(ev); }, - _displayPopupNotification: function(ev: MatrixEvent, room: Room) { + _displayPopupNotification: function(ev: MatrixEvent, room: Room): void { const plaf = PlatformPeg.get(); + const cli = MatrixClientPeg.get(); if (!plaf) { return; } if (!plaf.supportsNotifications() || !plaf.maySendNotifications()) { return; } + if (localNotificationsAreSilenced(cli)) { + return; + } let msg = this.notificationMessageForEvent(ev); if (!msg) return; @@ -170,7 +175,12 @@ export const Notifier = { }; }, - _playAudioNotification: async function(ev: MatrixEvent, room: Room) { + _playAudioNotification: async function(ev: MatrixEvent, room: Room): Promise { + const cli = MatrixClientPeg.get(); + if (localNotificationsAreSilenced(cli)) { + return; + } + const sound = this.getSoundForRoom(room.roomId); logger.log(`Got sound ${sound && sound.name || "default"} for ${room.roomId}`); @@ -325,7 +335,7 @@ export const Notifier = { } const isGuest = client.isGuest(); return !isGuest && this.supportsDesktopNotifications() && !isPushNotifyDisabled() && - !this.isEnabled() && !this._isPromptHidden(); + !localNotificationsAreSilenced(client) && !this.isEnabled() && !this._isPromptHidden(); }, _isPromptHidden: function() { diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 088d4232b46..ffa346ca2e2 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -15,6 +15,7 @@ limitations under the License. */ import { LOCAL_NOTIFICATION_SETTINGS_PREFIX } from "matrix-js-sdk/src/@types/event"; +import { LocalNotificationSettings } from "matrix-js-sdk/src/@types/local_notifications"; import { MatrixClient } from "matrix-js-sdk/src/client"; import SettingsStore from "../settings/SettingsStore"; @@ -32,7 +33,6 @@ export function getLocalNotificationAccountDataEventType(deviceId: string): stri export async function createLocalNotificationSettingsIfNeeded(cli: MatrixClient): Promise { const eventType = getLocalNotificationAccountDataEventType(cli.deviceId); const event = cli.getAccountData(eventType); - // New sessions will create an account data event to signify they support // remote toggling of push notifications on this device. Default `is_silenced=true` // For backwards compat purposes, older sessions will need to check settings value @@ -47,3 +47,9 @@ export async function createLocalNotificationSettingsIfNeeded(cli: MatrixClient) }); } } + +export function localNotificationsAreSilenced(cli: MatrixClient): boolean { + const eventType = getLocalNotificationAccountDataEventType(cli.deviceId); + const event = cli.getAccountData(eventType); + return event?.getContent()?.is_silenced ?? true; +} diff --git a/test/utils/notifications-test.ts b/test/utils/notifications-test.ts index 991e36f3a33..62e12e6ef83 100644 --- a/test/utils/notifications-test.ts +++ b/test/utils/notifications-test.ts @@ -18,6 +18,7 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { mocked } from "jest-mock"; import { + localNotificationsAreSilenced, createLocalNotificationSettingsIfNeeded, getLocalNotificationAccountDataEventType, } from "../../src/utils/notifications"; @@ -27,7 +28,7 @@ import { getMockClientWithEventEmitter } from "../test-utils/client"; jest.mock("../../src/settings/SettingsStore"); describe('notifications', () => { - const accountDataStore = {}; + let accountDataStore = {}; const mockClient = getMockClientWithEventEmitter({ isGuest: jest.fn().mockReturnValue(false), getAccountData: jest.fn().mockImplementation(eventType => accountDataStore[eventType]), @@ -42,6 +43,7 @@ describe('notifications', () => { const accountDataEventKey = getLocalNotificationAccountDataEventType(mockClient.deviceId); beforeEach(() => { + accountDataStore = {}; mocked(SettingsStore).getValue.mockReturnValue(false); }); @@ -76,4 +78,17 @@ describe('notifications', () => { expect(event?.getContent().is_silenced).toBe(false); }); }); + + describe('localNotificationsAreSilenced', () => { + it('defaults to true when no setting exists', () => { + expect(localNotificationsAreSilenced(mockClient)).toBeTruthy(); + }); + it('checks the persisted value', () => { + mockClient.setAccountData(accountDataEventKey, { is_silenced: true }); + expect(localNotificationsAreSilenced(mockClient)).toBeTruthy(); + + mockClient.setAccountData(accountDataEventKey, { is_silenced: false }); + expect(localNotificationsAreSilenced(mockClient)).toBeFalsy(); + }); + }); }); From 54910891f61aeeebf57625ea1d6ee7650b18f737 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 29 Sep 2022 10:01:47 +0100 Subject: [PATCH 2/4] Add Notifier tests --- src/Notifier.ts | 6 +++ test/Notifier-test.ts | 86 +++++++++++++++++++++++++++++++++ test/test-utils/MockPlatform.ts | 44 +++++++++++++++++ 3 files changed, 136 insertions(+) create mode 100644 test/Notifier-test.ts create mode 100644 test/test-utils/MockPlatform.ts diff --git a/src/Notifier.ts b/src/Notifier.ts index f901669032e..cca303dbf2e 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -97,12 +97,16 @@ export const Notifier = { if (!plaf) { return; } + console.log("1"); if (!plaf.supportsNotifications() || !plaf.maySendNotifications()) { return; } + + console.log("2"); if (localNotificationsAreSilenced(cli)) { return; } + console.log("3"); let msg = this.notificationMessageForEvent(ev); if (!msg) return; @@ -128,6 +132,7 @@ export const Notifier = { } } + console.log("4"); if (!this.isBodyEnabled()) { msg = ''; } @@ -137,6 +142,7 @@ export const Notifier = { avatarUrl = Avatar.avatarUrlForMember(ev.sender, 40, 40, 'crop'); } + console.log("5"); const notif = plaf.displayNotification(title, msg, avatarUrl, room, ev); // if displayNotification returns non-null, the platform supports diff --git a/test/Notifier-test.ts b/test/Notifier-test.ts new file mode 100644 index 00000000000..3d07a4d1b1f --- /dev/null +++ b/test/Notifier-test.ts @@ -0,0 +1,86 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { MatrixEvent } from "matrix-js-sdk/src/models/event"; + +import Notifier from "../src/Notifier"; +import { getLocalNotificationAccountDataEventType } from "../src/utils/notifications"; +import { getMockClientWithEventEmitter, mkEvent, mkRoom } from "./test-utils"; +import { getMockPlatform } from "./test-utils/MockPlatform"; + +describe("Notifier", () => { + let MockPlatform; + let accountDataStore = {}; + + const mockClient = getMockClientWithEventEmitter({ + getUserId: jest.fn().mockReturnValue("@bob:example.org"), + isGuest: jest.fn().mockReturnValue(false), + getAccountData: jest.fn().mockImplementation(eventType => accountDataStore[eventType]), + setAccountData: jest.fn().mockImplementation((eventType, content) => { + accountDataStore[eventType] = new MatrixEvent({ + type: eventType, + content, + }); + }), + }); + const accountDataEventKey = getLocalNotificationAccountDataEventType(mockClient.deviceId); + const roomId = "!room1:server"; + const testEvent = mkEvent({ + event: true, + type: "m.room.message", + user: "@user1:server", + room: roomId, + content: {}, + }); + const testRoom = mkRoom(mockClient, roomId); + + beforeEach(() => { + accountDataStore = {}; + MockPlatform = getMockPlatform({ + supportsNotifications: jest.fn().mockReturnValue(true), + maySendNotifications: jest.fn().mockReturnValue(true), + displayNotification: jest.fn(), + }); + + Notifier.isBodyEnabled = jest.fn().mockReturnValue(true); + }); + + describe("_displayPopupNotification", () => { + it.each([ + { silenced: true, count: 0 }, + { silenced: false, count: 1 }, + ])("does not dispatch when notifications are silenced", ({ silenced, count }) => { + mockClient.setAccountData(accountDataEventKey, { is_silenced: silenced }); + Notifier._displayPopupNotification(testEvent, testRoom); + expect(MockPlatform.displayNotification).toHaveBeenCalledTimes(count); + }); + }); + + describe("_playAudioNotification", () => { + it.each([ + { silenced: true, count: 0 }, + { silenced: false, count: 1 }, + ])("does not dispatch when notifications are silenced", ({ silenced, count }) => { + // It's not ideal to only look at whether this function has been called + // but avoids starting to look into DOM stuff + Notifier.getSoundForRoom = jest.fn(); + + mockClient.setAccountData(accountDataEventKey, { is_silenced: silenced }); + Notifier._playAudioNotification(testEvent, testRoom); + expect(Notifier.getSoundForRoom).toHaveBeenCalledTimes(count); + }); + }); +}); diff --git a/test/test-utils/MockPlatform.ts b/test/test-utils/MockPlatform.ts new file mode 100644 index 00000000000..05f35fdedb9 --- /dev/null +++ b/test/test-utils/MockPlatform.ts @@ -0,0 +1,44 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { MethodKeysOf, mocked, MockedObject } from "jest-mock"; + +import BasePlatform from "../../src/BasePlatform"; +import PlatformPeg from "../../src/PlatformPeg"; + +export const getMockPlatform = ( + mockProperties: Partial, unknown>>, +): MockedObject => { + const mock = mocked(new MockPlatform(mockProperties) as unknown as BasePlatform); + + jest.spyOn(PlatformPeg, 'get').mockReturnValue(mock); + return mock; +}; + +export class MockPlatform extends BasePlatform { + constructor(mockProperties: Partial, unknown>> = {}) { + super(); + + Object.assign(this, mockProperties); + } + + public getAppVersion = jest.fn().mockResolvedValue("version: test"); + public getConfig = jest.fn().mockResolvedValue({}); + public getHumanReadableName = jest.fn().mockReturnValue("Test Platform"); + public getDefaultDeviceDisplayName = jest.fn().mockReturnValue("Test device"); + public reload = jest.fn(); + public requestNotificationPermission = jest.fn().mockResolvedValue("fine!"); +} From f5de4e1830187aa5e0b8db8fb1baee780a0651a1 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 29 Sep 2022 10:02:48 +0100 Subject: [PATCH 3/4] Remove console.log --- src/Notifier.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Notifier.ts b/src/Notifier.ts index cca303dbf2e..ed2eaeee598 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -97,16 +97,13 @@ export const Notifier = { if (!plaf) { return; } - console.log("1"); if (!plaf.supportsNotifications() || !plaf.maySendNotifications()) { return; } - console.log("2"); if (localNotificationsAreSilenced(cli)) { return; } - console.log("3"); let msg = this.notificationMessageForEvent(ev); if (!msg) return; @@ -132,7 +129,6 @@ export const Notifier = { } } - console.log("4"); if (!this.isBodyEnabled()) { msg = ''; } @@ -142,7 +138,6 @@ export const Notifier = { avatarUrl = Avatar.avatarUrlForMember(ev.sender, 40, 40, 'crop'); } - console.log("5"); const notif = plaf.displayNotification(title, msg, avatarUrl, room, ev); // if displayNotification returns non-null, the platform supports From 72eef5f3c6c18c7a740aabb18749305116801930 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Thu, 29 Sep 2022 11:53:46 +0100 Subject: [PATCH 4/4] Use existing mock platform utility --- test/Notifier-test.ts | 5 ++-- test/test-utils/MockPlatform.ts | 44 --------------------------------- 2 files changed, 2 insertions(+), 47 deletions(-) delete mode 100644 test/test-utils/MockPlatform.ts diff --git a/test/Notifier-test.ts b/test/Notifier-test.ts index 3d07a4d1b1f..1178d35bec8 100644 --- a/test/Notifier-test.ts +++ b/test/Notifier-test.ts @@ -18,8 +18,7 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import Notifier from "../src/Notifier"; import { getLocalNotificationAccountDataEventType } from "../src/utils/notifications"; -import { getMockClientWithEventEmitter, mkEvent, mkRoom } from "./test-utils"; -import { getMockPlatform } from "./test-utils/MockPlatform"; +import { getMockClientWithEventEmitter, mkEvent, mkRoom, mockPlatformPeg } from "./test-utils"; describe("Notifier", () => { let MockPlatform; @@ -49,7 +48,7 @@ describe("Notifier", () => { beforeEach(() => { accountDataStore = {}; - MockPlatform = getMockPlatform({ + MockPlatform = mockPlatformPeg({ supportsNotifications: jest.fn().mockReturnValue(true), maySendNotifications: jest.fn().mockReturnValue(true), displayNotification: jest.fn(), diff --git a/test/test-utils/MockPlatform.ts b/test/test-utils/MockPlatform.ts deleted file mode 100644 index 05f35fdedb9..00000000000 --- a/test/test-utils/MockPlatform.ts +++ /dev/null @@ -1,44 +0,0 @@ -/* -Copyright 2022 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -import { MethodKeysOf, mocked, MockedObject } from "jest-mock"; - -import BasePlatform from "../../src/BasePlatform"; -import PlatformPeg from "../../src/PlatformPeg"; - -export const getMockPlatform = ( - mockProperties: Partial, unknown>>, -): MockedObject => { - const mock = mocked(new MockPlatform(mockProperties) as unknown as BasePlatform); - - jest.spyOn(PlatformPeg, 'get').mockReturnValue(mock); - return mock; -}; - -export class MockPlatform extends BasePlatform { - constructor(mockProperties: Partial, unknown>> = {}) { - super(); - - Object.assign(this, mockProperties); - } - - public getAppVersion = jest.fn().mockResolvedValue("version: test"); - public getConfig = jest.fn().mockResolvedValue({}); - public getHumanReadableName = jest.fn().mockReturnValue("Test Platform"); - public getDefaultDeviceDisplayName = jest.fn().mockReturnValue("Test device"); - public reload = jest.fn(); - public requestNotificationPermission = jest.fn().mockResolvedValue("fine!"); -}