From 436c52a358738d29ffe5c99bc82e7a08e1bf6d49 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 30 Sep 2022 13:45:24 +0100 Subject: [PATCH 1/7] Revert silencing notifications --- src/Notifier.ts | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/Notifier.ts b/src/Notifier.ts index ed2eaeee598..fcff4381c2f 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -46,7 +46,6 @@ 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: @@ -93,7 +92,6 @@ export const Notifier = { _displayPopupNotification: function(ev: MatrixEvent, room: Room): void { const plaf = PlatformPeg.get(); - const cli = MatrixClientPeg.get(); if (!plaf) { return; } @@ -101,10 +99,6 @@ export const Notifier = { return; } - if (localNotificationsAreSilenced(cli)) { - return; - } - let msg = this.notificationMessageForEvent(ev); if (!msg) return; @@ -177,11 +171,6 @@ export const Notifier = { }, _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}`); @@ -336,7 +325,7 @@ export const Notifier = { } const isGuest = client.isGuest(); return !isGuest && this.supportsDesktopNotifications() && !isPushNotifyDisabled() && - !localNotificationsAreSilenced(client) && !this.isEnabled() && !this._isPromptHidden(); + !this.isEnabled() && !this._isPromptHidden(); }, _isPromptHidden: function() { From 2dc798d792a9b8b027e48936cc3fa1cc6e6d0ed9 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 30 Sep 2022 14:06:12 +0100 Subject: [PATCH 2/7] skip tests --- test/Notifier-test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Notifier-test.ts b/test/Notifier-test.ts index 1178d35bec8..e34dcedc513 100644 --- a/test/Notifier-test.ts +++ b/test/Notifier-test.ts @@ -57,7 +57,7 @@ describe("Notifier", () => { Notifier.isBodyEnabled = jest.fn().mockReturnValue(true); }); - describe("_displayPopupNotification", () => { + describe.skip("_displayPopupNotification", () => { it.each([ { silenced: true, count: 0 }, { silenced: false, count: 1 }, @@ -68,7 +68,7 @@ describe("Notifier", () => { }); }); - describe("_playAudioNotification", () => { + describe.skip("_playAudioNotification", () => { it.each([ { silenced: true, count: 0 }, { silenced: false, count: 1 }, From 482192e10e6c4514382dc0825953e610af9eacad Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 30 Sep 2022 14:34:26 +0100 Subject: [PATCH 3/7] Fix unused import --- src/Notifier.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Notifier.ts b/src/Notifier.ts index fcff4381c2f..fc4d10460f2 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: @@ -171,6 +172,11 @@ export const Notifier = { }, _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}`); From b099fbcc9b5f2d183085a9e8b44178630aae9f88 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 30 Sep 2022 14:57:05 +0100 Subject: [PATCH 4/7] fix notifications --- src/Notifier.ts | 5 +++++ src/toasts/DesktopNotificationsToast.ts | 7 +++++++ test/Notifier-test.ts | 4 ++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Notifier.ts b/src/Notifier.ts index fc4d10460f2..8c7a8e4bed1 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -93,6 +93,7 @@ export const Notifier = { _displayPopupNotification: function(ev: MatrixEvent, room: Room): void { const plaf = PlatformPeg.get(); + const cli = MatrixClientPeg.get(); if (!plaf) { return; } @@ -100,6 +101,10 @@ export const Notifier = { return; } + if (localNotificationsAreSilenced(cli)) { + return; + } + let msg = this.notificationMessageForEvent(ev); if (!msg) return; diff --git a/src/toasts/DesktopNotificationsToast.ts b/src/toasts/DesktopNotificationsToast.ts index e10a6d46c62..ad361174180 100644 --- a/src/toasts/DesktopNotificationsToast.ts +++ b/src/toasts/DesktopNotificationsToast.ts @@ -18,9 +18,16 @@ import { _t } from "../languageHandler"; import Notifier from "../Notifier"; import GenericToast from "../components/views/toasts/GenericToast"; import ToastStore from "../stores/ToastStore"; +import { MatrixClientPeg } from "../MatrixClientPeg"; +import { getLocalNotificationAccountDataEventType } from "../utils/notifications"; const onAccept = () => { Notifier.setEnabled(true); + const cli = MatrixClientPeg.get(); + const eventType = getLocalNotificationAccountDataEventType(cli.deviceId); + cli.setAccountData(eventType, { + is_silenced: false, + }); }; const onReject = () => { diff --git a/test/Notifier-test.ts b/test/Notifier-test.ts index e34dcedc513..1178d35bec8 100644 --- a/test/Notifier-test.ts +++ b/test/Notifier-test.ts @@ -57,7 +57,7 @@ describe("Notifier", () => { Notifier.isBodyEnabled = jest.fn().mockReturnValue(true); }); - describe.skip("_displayPopupNotification", () => { + describe("_displayPopupNotification", () => { it.each([ { silenced: true, count: 0 }, { silenced: false, count: 1 }, @@ -68,7 +68,7 @@ describe("Notifier", () => { }); }); - describe.skip("_playAudioNotification", () => { + describe("_playAudioNotification", () => { it.each([ { silenced: true, count: 0 }, { silenced: false, count: 1 }, From 5054fe192cb005e97a69b1dc07692177fafbec3d Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 30 Sep 2022 15:43:57 +0100 Subject: [PATCH 5/7] Fix default notification settings --- src/components/structures/MatrixChat.tsx | 11 ------- .../views/settings/Notifications.tsx | 2 +- src/settings/Settings.tsx | 2 +- src/utils/notifications.ts | 18 ---------- test/utils/notifications-test.ts | 33 ------------------- 5 files changed, 2 insertions(+), 64 deletions(-) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index e05c4512550..849fcbfd719 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -137,7 +137,6 @@ import { TimelineRenderingType } from "../../contexts/RoomContext"; import { UseCaseSelection } from '../views/elements/UseCaseSelection'; import { ValidatedServerConfig } from '../../utils/ValidatedServerConfig'; import { isLocalRoom } from '../../utils/localRoom/isLocalRoom'; -import { createLocalNotificationSettingsIfNeeded } from '../../utils/notifications'; // legacy export export { default as Views } from "../../Views"; @@ -1260,8 +1259,6 @@ export default class MatrixChat extends React.PureComponent { this.themeWatcher.recheck(); StorageManager.tryPersistStorage(); - this.cli.on(ClientEvent.Sync, this.onInitialSync); - if ( MatrixClientPeg.currentUserIsJustRegistered() && SettingsStore.getValue("FTUE.useCaseSelection") === null @@ -1288,14 +1285,6 @@ export default class MatrixChat extends React.PureComponent { } } - private onInitialSync = (): void => { - if (this.cli.isInitialSyncComplete()) { - this.cli.off(ClientEvent.Sync, this.onInitialSync); - } - - createLocalNotificationSettingsIfNeeded(this.cli); - }; - private async onShowPostLoginScreen(useCase?: UseCase) { if (useCase) { PosthogAnalytics.instance.setProperty("ftueUseCaseSelection", useCase); diff --git a/src/components/views/settings/Notifications.tsx b/src/components/views/settings/Notifications.tsx index 54e7e150516..6c44f9979cb 100644 --- a/src/components/views/settings/Notifications.tsx +++ b/src/components/views/settings/Notifications.tsx @@ -122,7 +122,7 @@ export default class Notifications extends React.PureComponent { this.state = { phase: Phase.Loading, - deviceNotificationsEnabled: SettingsStore.getValue("deviceNotificationsEnabled") ?? false, + deviceNotificationsEnabled: SettingsStore.getValue("deviceNotificationsEnabled") ?? true, desktopNotifications: SettingsStore.getValue("notificationsEnabled"), desktopShowBody: SettingsStore.getValue("notificationBodyEnabled"), audioNotifications: SettingsStore.getValue("audioNotificationsEnabled"), diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index 69edd0b466e..02a80f6dda4 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -792,7 +792,7 @@ export const SETTINGS: {[setting: string]: ISetting} = { }, "deviceNotificationsEnabled": { supportedLevels: [SettingLevel.DEVICE], - default: false, + default: true, }, "notificationSound": { supportedLevels: LEVELS_ROOM_OR_ACCOUNT, diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index ffa346ca2e2..9c3eff4689a 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -30,24 +30,6 @@ export function getLocalNotificationAccountDataEventType(deviceId: string): stri return `${LOCAL_NOTIFICATION_SETTINGS_PREFIX.name}.${deviceId}`; } -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 - // to determine what the state of `is_silenced` - if (!event) { - // If any of the above is true, we fall in the "backwards compat" case, - // and `is_silenced` will be set to `false` - const isSilenced = !deviceNotificationSettingsKeys.some(key => SettingsStore.getValue(key)); - - await cli.setAccountData(eventType, { - is_silenced: isSilenced, - }); - } -} - export function localNotificationsAreSilenced(cli: MatrixClient): boolean { const eventType = getLocalNotificationAccountDataEventType(cli.deviceId); const event = cli.getAccountData(eventType); diff --git a/test/utils/notifications-test.ts b/test/utils/notifications-test.ts index 62e12e6ef83..b27a660ebff 100644 --- a/test/utils/notifications-test.ts +++ b/test/utils/notifications-test.ts @@ -19,7 +19,6 @@ import { mocked } from "jest-mock"; import { localNotificationsAreSilenced, - createLocalNotificationSettingsIfNeeded, getLocalNotificationAccountDataEventType, } from "../../src/utils/notifications"; import SettingsStore from "../../src/settings/SettingsStore"; @@ -47,38 +46,6 @@ describe('notifications', () => { mocked(SettingsStore).getValue.mockReturnValue(false); }); - describe('createLocalNotification', () => { - it('creates account data event', async () => { - await createLocalNotificationSettingsIfNeeded(mockClient); - const event = mockClient.getAccountData(accountDataEventKey); - expect(event?.getContent().is_silenced).toBe(true); - }); - - // Can't figure out why the mock does not override the value here - /*.each(deviceNotificationSettingsKeys) instead of skip */ - it.skip("unsilenced for existing sessions", async (/*settingKey*/) => { - mocked(SettingsStore) - .getValue - .mockImplementation((key) => { - // return key === settingKey; - }); - - await createLocalNotificationSettingsIfNeeded(mockClient); - const event = mockClient.getAccountData(accountDataEventKey); - expect(event?.getContent().is_silenced).toBe(false); - }); - - it("does not override an existing account event data", async () => { - mockClient.setAccountData(accountDataEventKey, { - is_silenced: false, - }); - - await createLocalNotificationSettingsIfNeeded(mockClient); - const event = mockClient.getAccountData(accountDataEventKey); - expect(event?.getContent().is_silenced).toBe(false); - }); - }); - describe('localNotificationsAreSilenced', () => { it('defaults to true when no setting exists', () => { expect(localNotificationsAreSilenced(mockClient)).toBeTruthy(); From 135cb37e45bc0ae47b92b3c21f8638e9f41fd091 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 30 Sep 2022 15:47:45 +0100 Subject: [PATCH 6/7] lint fix --- src/components/structures/MatrixChat.tsx | 2 -- src/utils/notifications.ts | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 849fcbfd719..1d24e1da087 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -400,8 +400,6 @@ export default class MatrixChat extends React.PureComponent { } } - private get cli(): MatrixClient { return MatrixClientPeg.get(); } - public componentDidMount(): void { window.addEventListener("resize", this.onWindowResized); } diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 9c3eff4689a..2eaf9da6abc 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -18,8 +18,6 @@ import { LOCAL_NOTIFICATION_SETTINGS_PREFIX } from "matrix-js-sdk/src/@types/eve import { LocalNotificationSettings } from "matrix-js-sdk/src/@types/local_notifications"; import { MatrixClient } from "matrix-js-sdk/src/client"; -import SettingsStore from "../settings/SettingsStore"; - export const deviceNotificationSettingsKeys = [ "notificationsEnabled", "notificationBodyEnabled", From e79f33c2e0cbef021b1d91a2cfda91574f1272ef Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 30 Sep 2022 15:52:57 +0100 Subject: [PATCH 7/7] Remove dead code --- src/utils/notifications.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 2eaf9da6abc..f41edd24bb7 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -18,12 +18,6 @@ import { LOCAL_NOTIFICATION_SETTINGS_PREFIX } from "matrix-js-sdk/src/@types/eve import { LocalNotificationSettings } from "matrix-js-sdk/src/@types/local_notifications"; import { MatrixClient } from "matrix-js-sdk/src/client"; -export const deviceNotificationSettingsKeys = [ - "notificationsEnabled", - "notificationBodyEnabled", - "audioNotificationsEnabled", -]; - export function getLocalNotificationAccountDataEventType(deviceId: string): string { return `${LOCAL_NOTIFICATION_SETTINGS_PREFIX.name}.${deviceId}`; }