diff --git a/src/components/views/settings/Notifications.tsx b/src/components/views/settings/Notifications.tsx index 2f05b5e4dc..f758022fc4 100644 --- a/src/components/views/settings/Notifications.tsx +++ b/src/components/views/settings/Notifications.tsx @@ -105,15 +105,36 @@ interface IState { }; pushers?: IPusher[]; threepids?: IThreepid[]; + + desktopNotifications: boolean; + desktopShowBody: boolean; + audioNotifications: boolean; } export default class Notifications extends React.PureComponent { + private settingWatchers: string[]; + public constructor(props: IProps) { super(props); this.state = { phase: Phase.Loading, + desktopNotifications: SettingsStore.getValue("notificationsEnabled"), + desktopShowBody: SettingsStore.getValue("notificationBodyEnabled"), + audioNotifications: SettingsStore.getValue("audioNotificationsEnabled"), }; + + this.settingWatchers = [ + SettingsStore.watchSetting("notificationsEnabled", null, (...[,,,, value]) => + this.setState({ desktopNotifications: value as boolean }), + ), + SettingsStore.watchSetting("notificationBodyEnabled", null, (...[,,,, value]) => + this.setState({ desktopShowBody: value as boolean }), + ), + SettingsStore.watchSetting("audioNotificationsEnabled", null, (...[,,,, value]) => + this.setState({ audioNotifications: value as boolean }), + ), + ]; } private get isInhibited(): boolean { @@ -129,6 +150,10 @@ export default class Notifications extends React.PureComponent { this.refreshFromServer(); } + public componentWillUnmount() { + this.settingWatchers.forEach(watcher => SettingsStore.unwatchSetting(watcher)); + } + private async refreshFromServer() { try { const newState = (await Promise.all([ @@ -137,7 +162,7 @@ export default class Notifications extends React.PureComponent { this.refreshThreepids(), ])).reduce((p, c) => Object.assign(c, p), {}); - this.setState({ + this.setState>({ ...newState, phase: Phase.Ready, }); @@ -308,17 +333,14 @@ export default class Notifications extends React.PureComponent { private onDesktopNotificationsChanged = async (checked: boolean) => { await SettingsStore.setValue("notificationsEnabled", null, SettingLevel.DEVICE, checked); - this.forceUpdate(); // the toggle is controlled by SettingsStore#getValue() }; private onDesktopShowBodyChanged = async (checked: boolean) => { await SettingsStore.setValue("notificationBodyEnabled", null, SettingLevel.DEVICE, checked); - this.forceUpdate(); // the toggle is controlled by SettingsStore#getValue() }; private onAudioNotificationsChanged = async (checked: boolean) => { await SettingsStore.setValue("audioNotificationsEnabled", null, SettingLevel.DEVICE, checked); - this.forceUpdate(); // the toggle is controlled by SettingsStore#getValue() }; private onRadioChecked = async (rule: IVectorPushRule, checkedState: VectorState) => { @@ -499,7 +521,7 @@ export default class Notifications extends React.PureComponent { { { (); + private static itemCache = new Map(); private static objectCache = new Map(); private static storageListenerBound = false; @@ -51,7 +51,7 @@ export default abstract class AbstractLocalStorageSettingsHandler extends Settin } } - protected getItem(key: string): any { + protected getItem(key: string): string { if (!AbstractLocalStorageSettingsHandler.itemCache.has(key)) { const value = localStorage.getItem(key); AbstractLocalStorageSettingsHandler.itemCache.set(key, value); @@ -61,6 +61,14 @@ export default abstract class AbstractLocalStorageSettingsHandler extends Settin return AbstractLocalStorageSettingsHandler.itemCache.get(key); } + protected getBoolean(key: string): boolean | null { + const item = this.getItem(key); + if (item === "true") return true; + if (item === "false") return false; + // Fall back to the next config level + return null; + } + protected getObject(key: string): T | null { if (!AbstractLocalStorageSettingsHandler.objectCache.has(key)) { try { @@ -76,11 +84,15 @@ export default abstract class AbstractLocalStorageSettingsHandler extends Settin return AbstractLocalStorageSettingsHandler.objectCache.get(key) as T; } - protected setItem(key: string, value: any): void { + protected setItem(key: string, value: string): void { AbstractLocalStorageSettingsHandler.itemCache.set(key, value); localStorage.setItem(key, value); } + protected setBoolean(key: string, value: boolean | null): void { + this.setItem(key, `${value}`); + } + protected setObject(key: string, value: object): void { AbstractLocalStorageSettingsHandler.objectCache.set(key, value); localStorage.setItem(key, JSON.stringify(value)); diff --git a/src/settings/handlers/DeviceSettingsHandler.ts b/src/settings/handlers/DeviceSettingsHandler.ts index 25c75c67a1..138457d5a9 100644 --- a/src/settings/handlers/DeviceSettingsHandler.ts +++ b/src/settings/handlers/DeviceSettingsHandler.ts @@ -43,17 +43,11 @@ export default class DeviceSettingsHandler extends AbstractLocalStorageSettingsH // Special case notifications if (settingName === "notificationsEnabled") { - const value = this.getItem("notifications_enabled"); - if (typeof(value) === "string") return value === "true"; - return null; // wrong type or otherwise not set + return this.getBoolean("notifications_enabled"); } else if (settingName === "notificationBodyEnabled") { - const value = this.getItem("notifications_body_enabled"); - if (typeof(value) === "string") return value === "true"; - return null; // wrong type or otherwise not set + return this.getBoolean("notifications_body_enabled"); } else if (settingName === "audioNotificationsEnabled") { - const value = this.getItem("audio_notifications_enabled"); - if (typeof(value) === "string") return value === "true"; - return null; // wrong type or otherwise not set + return this.getBoolean("audio_notifications_enabled"); } const settings = this.getSettings() || {}; @@ -68,15 +62,15 @@ export default class DeviceSettingsHandler extends AbstractLocalStorageSettingsH // Special case notifications if (settingName === "notificationsEnabled") { - this.setItem("notifications_enabled", newValue); + this.setBoolean("notifications_enabled", newValue); this.watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue); return Promise.resolve(); } else if (settingName === "notificationBodyEnabled") { - this.setItem("notifications_body_enabled", newValue); + this.setBoolean("notifications_body_enabled", newValue); this.watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue); return Promise.resolve(); } else if (settingName === "audioNotificationsEnabled") { - this.setItem("audio_notifications_enabled", newValue); + this.setBoolean("audio_notifications_enabled", newValue); this.watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue); return Promise.resolve(); } @@ -126,15 +120,11 @@ export default class DeviceSettingsHandler extends AbstractLocalStorageSettingsH return false; } - const value = this.getItem("mx_labs_feature_" + featureName); - if (value === "true") return true; - if (value === "false") return false; - // Try to read the next config level for the feature. - return null; + return this.getBoolean("mx_labs_feature_" + featureName); } private writeFeature(featureName: string, enabled: boolean | null) { - this.setItem("mx_labs_feature_" + featureName, `${enabled}`); + this.setBoolean("mx_labs_feature_" + featureName, enabled); this.watchers.notifyUpdate(featureName, null, SettingLevel.DEVICE, enabled); } } diff --git a/test/components/views/settings/Notifications-test.tsx b/test/components/views/settings/Notifications-test.tsx index b1abbebed4..ab1f7aa6f0 100644 --- a/test/components/views/settings/Notifications-test.tsx +++ b/test/components/views/settings/Notifications-test.tsx @@ -13,7 +13,7 @@ limitations under the License. */ import React from 'react'; -import { mount } from 'enzyme'; +import { mount, ReactWrapper } from 'enzyme'; import { IPushRule, IPushRules, RuleId, IPusher } from 'matrix-js-sdk/src/matrix'; import { IThreepid, ThreepidMedium } from 'matrix-js-sdk/src/@types/threepids'; import { act } from 'react-dom/test-utils'; @@ -23,16 +23,11 @@ import SettingsStore from "../../../../src/settings/SettingsStore"; import { StandardActions } from '../../../../src/notifications/StandardActions'; import { getMockClientWithEventEmitter } from '../../../test-utils'; -jest.mock('../../../../src/settings/SettingsStore', () => ({ - monitorSetting: jest.fn(), - getValue: jest.fn(), - setValue: jest.fn(), -})); - // don't pollute test output with error logs from mock rejections jest.mock("matrix-js-sdk/src/logger"); -jest.useRealTimers(); +// Avoid indirectly importing any eagerly created stores that would require extra setup +jest.mock("../../../../src/Notifier"); const masterRule = { actions: ["dont_notify"], @@ -81,9 +76,6 @@ describe('', () => { mockClient.getPushers.mockClear().mockResolvedValue({ pushers: [] }); mockClient.getThreePids.mockClear().mockResolvedValue({ threepids: [] }); mockClient.setPusher.mockClear().mockResolvedValue({}); - - (SettingsStore.getValue as jest.Mock).mockClear().mockReturnValue(true); - (SettingsStore.setValue as jest.Mock).mockClear().mockResolvedValue(true); }); it('renders spinner while loading', () => { @@ -91,11 +83,6 @@ describe('', () => { expect(component.find('.mx_Spinner').length).toBeTruthy(); }); - it('renders error message when fetching push rules fails', async () => { - mockClient.getPushRules.mockRejectedValue({}); - const component = await getComponentAndWait(); - expect(findByTestId(component, 'error-message').length).toBeTruthy(); - }); it('renders error message when fetching push rules fails', async () => { mockClient.getPushRules.mockRejectedValue({}); const component = await getComponentAndWait(); @@ -221,17 +208,24 @@ describe('', () => { }); }); - it('sets settings value on toggle click', async () => { + it('toggles and sets settings correctly', async () => { const component = await getComponentAndWait(); + let audioNotifsToggle: ReactWrapper; - const audioNotifsToggle = findByTestId(component, 'notif-setting-audioNotificationsEnabled') - .find('div[role="switch"]'); + const update = () => { + audioNotifsToggle = findByTestId(component, 'notif-setting-audioNotificationsEnabled') + .find('div[role="switch"]'); + }; + update(); - await act(async () => { - audioNotifsToggle.simulate('click'); - }); + expect(audioNotifsToggle.getDOMNode().getAttribute("aria-checked")).toEqual("true"); + expect(SettingsStore.getValue("audioNotificationsEnabled")).toEqual(true); - expect(SettingsStore.setValue).toHaveBeenCalledWith('audioNotificationsEnabled', null, "device", false); + act(() => { audioNotifsToggle.simulate('click'); }); + update(); + + expect(audioNotifsToggle.getDOMNode().getAttribute("aria-checked")).toEqual("false"); + expect(SettingsStore.getValue("audioNotificationsEnabled")).toEqual(false); }); });