From c2e2f406afa1142590f92cb3ad6513ffa547179a Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 30 Sep 2022 16:11:04 +0100 Subject: [PATCH] Fixes silenced notification preventing notification prompt to be shown (#9336) --- src/Notifier.ts | 2 +- src/components/structures/MatrixChat.tsx | 13 -------- .../views/settings/Notifications.tsx | 2 +- src/settings/Settings.tsx | 2 +- src/toasts/DesktopNotificationsToast.ts | 7 ++++ src/utils/notifications.ts | 26 --------------- test/utils/notifications-test.ts | 33 ------------------- 7 files changed, 10 insertions(+), 75 deletions(-) diff --git a/src/Notifier.ts b/src/Notifier.ts index ed2eaeee59..8c7a8e4bed 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -336,7 +336,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() { diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index e05c451255..1d24e1da08 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"; @@ -401,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); } @@ -1260,8 +1257,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 +1283,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 54e7e15051..6c44f9979c 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 69edd0b466..02a80f6dda 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/toasts/DesktopNotificationsToast.ts b/src/toasts/DesktopNotificationsToast.ts index e10a6d46c6..ad36117418 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/src/utils/notifications.ts b/src/utils/notifications.ts index ffa346ca2e..f41edd24bb 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -18,36 +18,10 @@ 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", - "audioNotificationsEnabled", -]; - export function getLocalNotificationAccountDataEventType(deviceId: string): string { 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 62e12e6ef8..b27a660ebf 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();