From 10a765472b9bac77bfc4ac2e29fc5deb2132397c Mon Sep 17 00:00:00 2001 From: Kerry Date: Thu, 2 Mar 2023 18:33:19 +1300 Subject: [PATCH] Polls push rules: synchronise poll rules with message rules (#10263) * basic sync setup * formatting * get loudest value for synced rules * more types * test synced rules in notifications settings * type fixes * noimplicitany fixes * remove debug * tidying --- .../views/settings/Notifications.tsx | 122 ++++++- .../VectorPushRulesDefinitions.ts | 16 +- .../views/settings/Notifications-test.tsx | 315 +++++++++++++++--- 3 files changed, 403 insertions(+), 50 deletions(-) diff --git a/src/components/views/settings/Notifications.tsx b/src/components/views/settings/Notifications.tsx index 3fb902ba5e..35c7fa3d49 100644 --- a/src/components/views/settings/Notifications.tsx +++ b/src/components/views/settings/Notifications.tsx @@ -15,10 +15,18 @@ limitations under the License. */ import React, { ReactNode } from "react"; -import { IAnnotatedPushRule, IPusher, PushRuleAction, PushRuleKind, RuleId } from "matrix-js-sdk/src/@types/PushRules"; +import { + IAnnotatedPushRule, + IPusher, + PushRuleAction, + IPushRule, + PushRuleKind, + RuleId, +} from "matrix-js-sdk/src/@types/PushRules"; import { IThreepid, ThreepidMedium } from "matrix-js-sdk/src/@types/threepids"; import { logger } from "matrix-js-sdk/src/logger"; import { LocalNotificationSettings } from "matrix-js-sdk/src/@types/local_notifications"; +import { PushProcessor } from "matrix-js-sdk/src/pushprocessor"; import Spinner from "../elements/Spinner"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; @@ -92,6 +100,9 @@ interface IVectorPushRule { rule?: IAnnotatedPushRule; description: TranslatedString | string; vectorState: VectorState; + // loudest vectorState of a rule and its synced rules + // undefined when rule has no synced rules + syncedVectorState?: VectorState; } interface IProps {} @@ -115,9 +126,68 @@ interface IState { clearingNotifications: boolean; } +const findInDefaultRules = ( + ruleId: RuleId | string, + defaultRules: { + [k in RuleClass]: IAnnotatedPushRule[]; + }, +): IAnnotatedPushRule | undefined => { + for (const category in defaultRules) { + const rule: IAnnotatedPushRule | undefined = defaultRules[category as RuleClass].find( + (rule) => rule.rule_id === ruleId, + ); + if (rule) { + return rule; + } + } +}; + +// Vector notification states ordered by loudness in ascending order +const OrderedVectorStates = [VectorState.Off, VectorState.On, VectorState.Loud]; + +/** + * Find the 'loudest' vector state assigned to a rule + * and it's synced rules + * If rules have fallen out of sync, + * the loudest rule can determine the display value + * @param defaultRules + * @param rule - parent rule + * @param definition - definition of parent rule + * @returns VectorState - the maximum/loudest state for the parent and synced rules + */ +const maximumVectorState = ( + defaultRules: { + [k in RuleClass]: IAnnotatedPushRule[]; + }, + rule: IAnnotatedPushRule, + definition: VectorPushRuleDefinition, +): VectorState | undefined => { + if (!definition.syncedRuleIds?.length) { + return undefined; + } + const vectorState = definition.syncedRuleIds.reduce((maxVectorState, ruleId) => { + // already set to maximum + if (maxVectorState === VectorState.Loud) { + return maxVectorState; + } + const syncedRule = findInDefaultRules(ruleId, defaultRules); + if (syncedRule) { + const syncedRuleVectorState = definition.ruleToVectorState(syncedRule); + // if syncedRule is 'louder' than current maximum + // set maximum to louder vectorState + if (OrderedVectorStates.indexOf(syncedRuleVectorState) > OrderedVectorStates.indexOf(maxVectorState)) { + return syncedRuleVectorState; + } + } + return maxVectorState; + }, definition.ruleToVectorState(rule)); + + return vectorState; +}; export default class Notifications extends React.PureComponent { private settingWatchers: string[]; + private pushProcessor: PushProcessor; public constructor(props: IProps) { super(props); @@ -145,6 +215,8 @@ export default class Notifications extends React.PureComponent { this.setState({ audioNotifications: value as boolean }), ), ]; + + this.pushProcessor = new PushProcessor(MatrixClientPeg.get()); } private get isInhibited(): boolean { @@ -281,6 +353,7 @@ export default class Notifications extends React.PureComponent { ruleId: rule.rule_id, rule, vectorState, + syncedVectorState: maximumVectorState(defaultRules, rule, definition), description: _t(definition.description), }); } @@ -388,6 +461,43 @@ export default class Notifications extends React.PureComponent { await SettingsStore.setValue("audioNotificationsEnabled", null, SettingLevel.DEVICE, checked); }; + private setPushRuleActions = async ( + ruleId: IPushRule["rule_id"], + kind: PushRuleKind, + actions?: PushRuleAction[], + ): Promise => { + const cli = MatrixClientPeg.get(); + if (!actions) { + await cli.setPushRuleEnabled("global", kind, ruleId, false); + } else { + await cli.setPushRuleActions("global", kind, ruleId, actions); + await cli.setPushRuleEnabled("global", kind, ruleId, true); + } + }; + + /** + * Updated syncedRuleIds from rule definition + * If a rule does not exist it is ignored + * Synced rules are updated sequentially + * and stop at first error + */ + private updateSyncedRules = async ( + syncedRuleIds: VectorPushRuleDefinition["syncedRuleIds"], + actions?: PushRuleAction[], + ): Promise => { + // get synced rules that exist for user + const syncedRules: ReturnType[] = syncedRuleIds + ?.map((ruleId) => this.pushProcessor.getPushRuleAndKindById(ruleId)) + .filter(Boolean); + + if (!syncedRules?.length) { + return; + } + for (const { kind, rule: syncedRule } of syncedRules) { + await this.setPushRuleActions(syncedRule.rule_id, kind, actions); + } + }; + private onRadioChecked = async (rule: IVectorPushRule, checkedState: VectorState): Promise => { this.setState({ phase: Phase.Persisting }); @@ -428,12 +538,8 @@ export default class Notifications extends React.PureComponent { } else { const definition: VectorPushRuleDefinition = VectorPushRulesDefinitions[rule.ruleId]; const actions = definition.vectorStateToActions[checkedState]; - if (!actions) { - await cli.setPushRuleEnabled("global", rule.rule.kind, rule.rule.rule_id, false); - } else { - await cli.setPushRuleActions("global", rule.rule.kind, rule.rule.rule_id, actions); - await cli.setPushRuleEnabled("global", rule.rule.kind, rule.rule.rule_id, true); - } + await this.setPushRuleActions(rule.rule.rule_id, rule.rule.kind, actions); + await this.updateSyncedRules(definition.syncedRuleIds, actions); } await this.refreshFromServer(); @@ -684,7 +790,7 @@ export default class Notifications extends React.PureComponent { ", () => { mockClient.getPushers.mockClear().mockResolvedValue({ pushers: [] }); mockClient.getThreePids.mockClear().mockResolvedValue({ threepids: [] }); mockClient.setPusher.mockClear().mockResolvedValue({}); + mockClient.setPushRuleActions.mockClear().mockResolvedValue({}); + mockClient.pushRules = pushRules; }); it("renders spinner while loading", async () => { @@ -429,6 +472,196 @@ describe("", () => { StandardActions.ACTION_DONT_NOTIFY, ); }); + + describe("synced rules", () => { + const pollStartOneToOne = { + conditions: [ + { + kind: ConditionKind.RoomMemberCount, + is: "2", + } as IPushRuleCondition, + { + kind: ConditionKind.EventMatch, + key: "type", + pattern: "org.matrix.msc3381.poll.start", + } as IPushRuleCondition, + ], + actions: [PushRuleActionName.DontNotify], + rule_id: ".org.matrix.msc3930.rule.poll_start_one_to_one", + default: true, + enabled: true, + } as IPushRule; + const pollStartGroup = { + conditions: [ + { + kind: ConditionKind.EventMatch, + key: "type", + pattern: "org.matrix.msc3381.poll.start", + }, + ], + actions: [PushRuleActionName.Notify], + rule_id: ".org.matrix.msc3930.rule.poll_start", + default: true, + enabled: true, + } as IPushRule; + const pollEndOneToOne = { + conditions: [ + { + kind: ConditionKind.RoomMemberCount, + is: "2", + }, + { + kind: ConditionKind.EventMatch, + key: "type", + pattern: "org.matrix.msc3381.poll.end", + }, + ], + actions: [ + PushRuleActionName.Notify, + { set_tweak: TweakName.Highlight, value: false }, + { set_tweak: TweakName.Sound, value: "default" }, + ], + rule_id: ".org.matrix.msc3930.rule.poll_end_one_to_one", + default: true, + enabled: true, + } as IPushRule; + + const setPushRuleMock = (rules: IPushRule[] = []): void => { + const combinedRules = { + ...pushRules, + global: { + ...pushRules.global, + underride: [...pushRules.global.underride!, ...rules], + }, + }; + mockClient.getPushRules.mockClear().mockResolvedValue(combinedRules); + mockClient.pushRules = combinedRules; + }; + + // ".m.rule.room_one_to_one" and ".m.rule.message" have synced rules + it("succeeds when no synced rules exist for user", async () => { + await getComponentAndWait(); + const section = "vector_global"; + + const oneToOneRuleElement = screen.getByTestId(section + oneToOneRule.rule_id); + + const offToggle = oneToOneRuleElement.querySelector('input[type="radio"]')!; + fireEvent.click(offToggle); + + await flushPromises(); + + // didnt attempt to update any non-existant rules + expect(mockClient.setPushRuleActions).toHaveBeenCalledTimes(1); + + // no error + expect(screen.queryByTestId("error-message")).not.toBeInTheDocument(); + }); + + it("updates synced rules when they exist for user", async () => { + setPushRuleMock([pollStartOneToOne, pollStartGroup]); + await getComponentAndWait(); + const section = "vector_global"; + const oneToOneRuleElement = screen.getByTestId(section + oneToOneRule.rule_id); + + const offToggle = oneToOneRuleElement.querySelector('input[type="radio"]')!; + fireEvent.click(offToggle); + + await flushPromises(); + + // updated synced rule + expect(mockClient.setPushRuleActions).toHaveBeenCalledWith( + "global", + "underride", + oneToOneRule.rule_id, + [PushRuleActionName.DontNotify], + ); + expect(mockClient.setPushRuleActions).toHaveBeenCalledWith( + "global", + "underride", + pollStartOneToOne.rule_id, + [PushRuleActionName.DontNotify], + ); + // only called for parent rule and one existing synced rule + expect(mockClient.setPushRuleActions).toHaveBeenCalledTimes(2); + + // no error + expect(screen.queryByTestId("error-message")).not.toBeInTheDocument(); + }); + + it("does not update synced rules when main rule update fails", async () => { + setPushRuleMock([pollStartOneToOne]); + await getComponentAndWait(); + const section = "vector_global"; + const oneToOneRuleElement = screen.getByTestId(section + oneToOneRule.rule_id); + // have main rule update fail + mockClient.setPushRuleActions.mockRejectedValue("oups"); + + const offToggle = oneToOneRuleElement.querySelector('input[type="radio"]')!; + fireEvent.click(offToggle); + + await flushPromises(); + + expect(mockClient.setPushRuleActions).toHaveBeenCalledWith( + "global", + "underride", + oneToOneRule.rule_id, + [PushRuleActionName.DontNotify], + ); + // only called for parent rule + expect(mockClient.setPushRuleActions).toHaveBeenCalledTimes(1); + + expect(screen.queryByTestId("error-message")).toBeInTheDocument(); + }); + + it("sets the UI toggle to rule value when no synced rule exist for the user", async () => { + setPushRuleMock([]); + await getComponentAndWait(); + const section = "vector_global"; + const oneToOneRuleElement = screen.getByTestId(section + oneToOneRule.rule_id); + + // loudest state of synced rules should be the toggle value + expect(oneToOneRuleElement.querySelector('input[aria-label="On"]')).toBeChecked(); + }); + + it("sets the UI toggle to the loudest synced rule value", async () => { + // oneToOneRule is set to 'On' + // pollEndOneToOne is set to 'Loud' + setPushRuleMock([pollStartOneToOne, pollEndOneToOne]); + await getComponentAndWait(); + const section = "vector_global"; + const oneToOneRuleElement = screen.getByTestId(section + oneToOneRule.rule_id); + + // loudest state of synced rules should be the toggle value + expect(oneToOneRuleElement.querySelector('input[aria-label="Noisy"]')).toBeChecked(); + + const onToggle = oneToOneRuleElement.querySelector('input[aria-label="On"]')!; + fireEvent.click(onToggle); + + await flushPromises(); + + // called for all 3 rules + expect(mockClient.setPushRuleActions).toHaveBeenCalledTimes(3); + const expectedActions = [PushRuleActionName.Notify, { set_tweak: TweakName.Highlight, value: false }]; + expect(mockClient.setPushRuleActions).toHaveBeenCalledWith( + "global", + "underride", + oneToOneRule.rule_id, + expectedActions, + ); + expect(mockClient.setPushRuleActions).toHaveBeenCalledWith( + "global", + "underride", + pollStartOneToOne.rule_id, + expectedActions, + ); + expect(mockClient.setPushRuleActions).toHaveBeenCalledWith( + "global", + "underride", + pollEndOneToOne.rule_id, + expectedActions, + ); + }); + }); }); describe("clear all notifications", () => {