diff --git a/res/css/views/settings/_Notifications.pcss b/res/css/views/settings/_Notifications.pcss index 7357ee1c68b..ef6257c704e 100644 --- a/res/css/views/settings/_Notifications.pcss +++ b/res/css/views/settings/_Notifications.pcss @@ -61,6 +61,12 @@ limitations under the License. font-size: $font-12px; font-weight: $font-semi-bold; } +.mx_UserNotifSettings_gridRowError { + // occupy full row + grid-column: 1/-1; + justify-self: start; + padding-right: 30%; +} .mx_UserNotifSettings { color: $primary-content; /* override from default settings page styles */ diff --git a/src/components/views/settings/Notifications.tsx b/src/components/views/settings/Notifications.tsx index 3aa4edf8a43..3f901daeee7 100644 --- a/src/components/views/settings/Notifications.tsx +++ b/src/components/views/settings/Notifications.tsx @@ -47,6 +47,7 @@ import { updateExistingPushRulesWithActions, updatePushRuleActions, } from "../../../utils/pushRules/updatePushRuleActions"; +import { Caption } from "../typography/Caption"; // TODO: this "view" component still has far too much application logic in it, // which should be factored out to other files. @@ -55,7 +56,10 @@ enum Phase { Loading = "loading", Ready = "ready", Persisting = "persisting", // technically a meta-state for Ready, but whatever + // unrecoverable error - eg can't load push rules Error = "error", + // error saving individual rule + SavingError = "savingError", } enum RuleClass { @@ -121,6 +125,8 @@ interface IState { audioNotifications: boolean; clearingNotifications: boolean; + + ruleIdsWithError: Record; } const findInDefaultRules = ( ruleId: RuleId | string, @@ -194,6 +200,7 @@ export default class Notifications extends React.PureComponent { desktopShowBody: SettingsStore.getValue("notificationBodyEnabled"), audioNotifications: SettingsStore.getValue("audioNotificationsEnabled"), clearingNotifications: false, + ruleIdsWithError: {}, }; this.settingWatchers = [ @@ -243,13 +250,9 @@ export default class Notifications extends React.PureComponent { ).reduce((p, c) => Object.assign(c, p), {}); this.setState< - keyof Omit< + keyof Pick< IState, - | "deviceNotificationsEnabled" - | "desktopNotifications" - | "desktopShowBody" - | "audioNotifications" - | "clearingNotifications" + "phase" | "vectorKeywordRuleInfo" | "vectorPushRules" | "pushers" | "threepids" | "masterPushRule" > >({ ...newState, @@ -393,17 +396,28 @@ export default class Notifications extends React.PureComponent { private onMasterRuleChanged = async (checked: boolean): Promise => { this.setState({ phase: Phase.Persisting }); + const masterRule = this.state.masterPushRule!; try { - const masterRule = this.state.masterPushRule!; await MatrixClientPeg.get().setPushRuleEnabled("global", masterRule.kind, masterRule.rule_id, !checked); await this.refreshFromServer(); } catch (e) { - this.setState({ phase: Phase.Error }); + if (masterRule?.rule_id) { + this.setSavingError(masterRule.rule_id); + } else { + this.setState({ phase: Phase.Error }); + } logger.error("Error updating master push rule:", e); this.showSaveError(); } }; + private setSavingError = (ruleId: RuleId | string): void => { + this.setState(({ ruleIdsWithError }) => ({ + phase: Phase.SavingError, + ruleIdsWithError: { ...ruleIdsWithError, [ruleId]: true }, + })); + }; + private updateDeviceNotifications = async (checked: boolean): Promise => { await SettingsStore.setValue("deviceNotificationsEnabled", null, SettingLevel.DEVICE, checked); }; @@ -455,7 +469,10 @@ export default class Notifications extends React.PureComponent { }; private onRadioChecked = async (rule: IVectorPushRule, checkedState: VectorState): Promise => { - this.setState({ phase: Phase.Persisting }); + this.setState(({ ruleIdsWithError }) => ({ + phase: Phase.Persisting, + ruleIdsWithError: { ...ruleIdsWithError, [rule.ruleId]: false }, + })); try { const cli = MatrixClientPeg.get(); @@ -505,9 +522,8 @@ export default class Notifications extends React.PureComponent { await this.refreshFromServer(); } catch (e) { - this.setState({ phase: Phase.Error }); + this.setSavingError(rule.ruleId); logger.error("Error updating push rule:", e); - this.showSaveError(); } }; @@ -768,6 +784,15 @@ export default class Notifications extends React.PureComponent { {makeRadio(r, VectorState.Off)} {makeRadio(r, VectorState.On)} {makeRadio(r, VectorState.Loud)} + {this.state.ruleIdsWithError[r.ruleId] && ( +
+ + {_t( + "An error occurred when updating your notification preferences. Please try to toggle your option again.", + )} + +
+ )} )); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index d5aa83660cc..0dc0c4dcdcf 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1441,6 +1441,7 @@ "On": "On", "Off": "Off", "Noisy": "Noisy", + "An error occurred when updating your notification preferences. Please try to toggle your option again.": "An error occurred when updating your notification preferences. Please try to toggle your option again.", "Global": "Global", "Mentions & keywords": "Mentions & keywords", "Notification targets": "Notification targets", diff --git a/test/components/views/settings/Notifications-test.tsx b/test/components/views/settings/Notifications-test.tsx index 2aa4c93a3b7..f352106db32 100644 --- a/test/components/views/settings/Notifications-test.tsx +++ b/test/components/views/settings/Notifications-test.tsx @@ -28,7 +28,7 @@ import { IPushRuleCondition, } from "matrix-js-sdk/src/matrix"; import { IThreepid, ThreepidMedium } from "matrix-js-sdk/src/@types/threepids"; -import { act, fireEvent, getByTestId, render, screen, waitFor } from "@testing-library/react"; +import { act, fireEvent, getByTestId, render, screen, waitFor, within } from "@testing-library/react"; import Notifications from "../../../../src/components/views/settings/Notifications"; import SettingsStore from "../../../../src/settings/SettingsStore"; @@ -272,7 +272,7 @@ describe("", () => { mockClient.getPushers.mockClear().mockResolvedValue({ pushers: [] }); mockClient.getThreePids.mockClear().mockResolvedValue({ threepids: [] }); mockClient.setPusher.mockClear().mockResolvedValue({}); - mockClient.setPushRuleActions.mockClear().mockResolvedValue({}); + mockClient.setPushRuleActions.mockReset().mockResolvedValue({}); mockClient.pushRules = pushRules; }); @@ -473,6 +473,73 @@ describe("", () => { ); }); + it("adds an error message when updating notification level fails", async () => { + await getComponentAndWait(); + const section = "vector_global"; + + const error = new Error("oups"); + mockClient.setPushRuleEnabled.mockRejectedValue(error); + + // oneToOneRule is set to 'on' + // and is kind: 'underride' + const offToggle = screen.getByTestId(section + oneToOneRule.rule_id).querySelector('input[type="radio"]')!; + fireEvent.click(offToggle); + + await flushPromises(); + + // error message attached to oneToOne rule + const oneToOneRuleElement = screen.getByTestId(section + oneToOneRule.rule_id); + // old value still shown as selected + expect(within(oneToOneRuleElement).getByLabelText("On")).toBeChecked(); + expect( + within(oneToOneRuleElement).getByText( + "An error occurred when updating your notification preferences. Please try to toggle your option again.", + ), + ).toBeInTheDocument(); + }); + + it("clears error message for notification rule on retry", async () => { + await getComponentAndWait(); + const section = "vector_global"; + + const error = new Error("oups"); + mockClient.setPushRuleEnabled.mockRejectedValueOnce(error).mockResolvedValue({}); + + // oneToOneRule is set to 'on' + // and is kind: 'underride' + const offToggle = screen.getByTestId(section + oneToOneRule.rule_id).querySelector('input[type="radio"]')!; + fireEvent.click(offToggle); + + await flushPromises(); + + // error message attached to oneToOne rule + const oneToOneRuleElement = screen.getByTestId(section + oneToOneRule.rule_id); + expect( + within(oneToOneRuleElement).getByText( + "An error occurred when updating your notification preferences. Please try to toggle your option again.", + ), + ).toBeInTheDocument(); + + // retry + fireEvent.click(offToggle); + + // error removed as soon as we start request + expect( + within(oneToOneRuleElement).queryByText( + "An error occurred when updating your notification preferences. Please try to toggle your option again.", + ), + ).not.toBeInTheDocument(); + + await flushPromises(); + + // no error after after successful change + expect( + within(oneToOneRuleElement).queryByText( + "An error occurred when updating your notification preferences. Please try to toggle your option again.", + ), + ).not.toBeInTheDocument(); + }); + describe("synced rules", () => { const pollStartOneToOne = { conditions: [ @@ -554,7 +621,11 @@ describe("", () => { expect(mockClient.setPushRuleActions).toHaveBeenCalledTimes(1); // no error - expect(screen.queryByTestId("error-message")).not.toBeInTheDocument(); + expect( + within(oneToOneRuleElement).queryByText( + "An error occurred when updating your notification preferences. Please try to toggle your option again.", + ), + ).not.toBeInTheDocument(); }); it("updates synced rules when they exist for user", async () => { @@ -585,7 +656,11 @@ describe("", () => { expect(mockClient.setPushRuleActions).toHaveBeenCalledTimes(2); // no error - expect(screen.queryByTestId("error-message")).not.toBeInTheDocument(); + expect( + within(oneToOneRuleElement).queryByText( + "An error occurred when updating your notification preferences. Please try to toggle your option again.", + ), + ).not.toBeInTheDocument(); }); it("does not update synced rules when main rule update fails", async () => { @@ -610,7 +685,11 @@ describe("", () => { // only called for parent rule expect(mockClient.setPushRuleActions).toHaveBeenCalledTimes(1); - expect(screen.queryByTestId("error-message")).toBeInTheDocument(); + expect( + within(oneToOneRuleElement).getByText( + "An error occurred when updating your notification preferences. Please try to toggle your option again.", + ), + ).toBeInTheDocument(); }); it("sets the UI toggle to rule value when no synced rule exist for the user", async () => {