Polls: sync push rules on changes to account_data (#10287)

* 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

* extract updatePushRuleActions fn to utils

* extract update synced rules

* just synchronise in one place?

* monitor account data changes AND trigger changes sync in notifications form

* lint

* setup LoggedInView test with enough mocks

* test rule syncing in LoggedInView

* strict fixes

* more comments

* one more comment

src/components/structures/LoggedInView.tsx

@@ -70,6 +70,7 @@ import { IConfigOptions } from "../../IConfigOptions";
 import LeftPanelLiveShareWarning from "../views/beacon/LeftPanelLiveShareWarning";
 import { UserOnboardingPage } from "../views/user-onboarding/UserOnboardingPage";
 import { PipContainer } from "./PipContainer";
+import { monitorSyncedPushRules } from "../../utils/pushRules/monitorSyncedPushRules";
 
 // We need to fetch each pinned message individually (if we don't already have it)
 // so each pinned message may trigger a request. Limit the number per room for sanity.
@@ -165,6 +166,8 @@ class LoggedInView extends React.Component<IProps, IState> {
         this.updateServerNoticeEvents();
 
         this._matrixClient.on(ClientEvent.AccountData, this.onAccountData);
+        // check push rules on start up as well
+        monitorSyncedPushRules(this._matrixClient.getAccountData("m.push_rules"), this._matrixClient);
         this._matrixClient.on(ClientEvent.Sync, this.onSync);
         // Call `onSync` with the current state as well
         this.onSync(this._matrixClient.getSyncState(), null, this._matrixClient.getSyncStateData());
@@ -279,6 +282,7 @@ class LoggedInView extends React.Component<IProps, IState> {
         if (event.getType() === "m.ignored_user_list") {
             dis.dispatch({ action: "ignore_state_changed" });
         }
+        monitorSyncedPushRules(event, this._matrixClient);
     };
 
     private onCompactLayoutChanged = (): void => {

src/components/views/settings/Notifications.tsx

@@ -15,18 +15,10 @@ limitations under the License.
 */
 
 import React, { ReactNode } from "react";
-import {
-    IAnnotatedPushRule,
-    IPusher,
-    PushRuleAction,
-    IPushRule,
-    PushRuleKind,
-    RuleId,
-} from "matrix-js-sdk/src/@types/PushRules";
+import { IAnnotatedPushRule, IPusher, PushRuleAction, 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";
@@ -51,6 +43,10 @@ import TagComposer from "../elements/TagComposer";
 import { objectClone } from "../../../utils/objects";
 import { arrayDiff } from "../../../utils/arrays";
 import { clearAllNotifications, getLocalNotificationAccountDataEventType } from "../../../utils/notifications";
+import {
+    updateExistingPushRulesWithActions,
+    updatePushRuleActions,
+} from "../../../utils/pushRules/updatePushRuleActions";
 
 // TODO: this "view" component still has far too much application logic in it,
 // which should be factored out to other files.
@@ -187,7 +183,6 @@ const maximumVectorState = (
 
 export default class Notifications extends React.PureComponent<IProps, IState> {
     private settingWatchers: string[];
-    private pushProcessor: PushProcessor;
 
     public constructor(props: IProps) {
         super(props);
@@ -215,8 +210,6 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
                 this.setState({ audioNotifications: value as boolean }),
             ),
         ];
-
-        this.pushProcessor = new PushProcessor(MatrixClientPeg.get());
     }
 
     private get isInhibited(): boolean {
@@ -461,43 +454,6 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
         await SettingsStore.setValue("audioNotificationsEnabled", null, SettingLevel.DEVICE, checked);
     };
 
-    private setPushRuleActions = async (
-        ruleId: IPushRule["rule_id"],
-        kind: PushRuleKind,
-        actions?: PushRuleAction[],
-    ): Promise<void> => {
-        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<void> => {
-        // get synced rules that exist for user
-        const syncedRules: ReturnType<PushProcessor["getPushRuleAndKindById"]>[] = 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<void> => {
         this.setState({ phase: Phase.Persisting });
 
@@ -538,8 +494,13 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
             } else {
                 const definition: VectorPushRuleDefinition = VectorPushRulesDefinitions[rule.ruleId];
                 const actions = definition.vectorStateToActions[checkedState];
-                await this.setPushRuleActions(rule.rule.rule_id, rule.rule.kind, actions);
-                await this.updateSyncedRules(definition.syncedRuleIds, actions);
+                // we should not encounter this
+                // satisfies types
+                if (!rule.rule) {
+                    throw new Error("Cannot update rule: push rule data is incomplete.");
+                }
+                await updatePushRuleActions(cli, rule.rule.rule_id, rule.rule.kind, actions);
+                await updateExistingPushRulesWithActions(cli, definition.syncedRuleIds, actions);
             }
 
             await this.refreshFromServer();

src/utils/pushRules/monitorSyncedPushRules.ts

@@ -0,0 +1,108 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { MatrixClient } from "matrix-js-sdk/src/client";
+import { MatrixEvent, EventType } from "matrix-js-sdk/src/matrix";
+import { PushProcessor } from "matrix-js-sdk/src/pushprocessor";
+import { RuleId, IAnnotatedPushRule } from "matrix-js-sdk/src/@types/PushRules";
+import { logger } from "matrix-js-sdk/src/logger";
+
+import { VectorPushRulesDefinitions, VectorPushRuleDefinition } from "../../notifications";
+import { updateExistingPushRulesWithActions } from "./updatePushRuleActions";
+
+const pushRuleAndKindToAnnotated = (
+    ruleAndKind: ReturnType<PushProcessor["getPushRuleAndKindById"]>,
+): IAnnotatedPushRule | undefined =>
+    ruleAndKind
+        ? {
+              ...ruleAndKind.rule,
+              kind: ruleAndKind.kind,
+          }
+        : undefined;
+
+/**
+ * Checks that any synced rules that exist a given rule are in sync
+ * And updates any that are out of sync
+ * Ignores ruleIds that do not exist for the user
+ * @param matrixClient - cli
+ * @param pushProcessor - processor used to retrieve current state of rules
+ * @param ruleId - primary rule
+ * @param definition - VectorPushRuleDefinition of the primary rule
+ */
+const monitorSyncedRule = async (
+    matrixClient: MatrixClient,
+    pushProcessor: PushProcessor,
+    ruleId: RuleId | string,
+    definition: VectorPushRuleDefinition,
+): Promise<void> => {
+    const primaryRule = pushRuleAndKindToAnnotated(pushProcessor.getPushRuleAndKindById(ruleId));
+
+    if (!primaryRule) {
+        return;
+    }
+    const syncedRules: IAnnotatedPushRule[] | undefined = definition.syncedRuleIds
+        ?.map((ruleId) => pushRuleAndKindToAnnotated(pushProcessor.getPushRuleAndKindById(ruleId)))
+        .filter((n?: IAnnotatedPushRule): n is IAnnotatedPushRule => Boolean(n));
+
+    // no synced rules to manage
+    if (!syncedRules?.length) {
+        return;
+    }
+
+    const primaryRuleVectorState = definition.ruleToVectorState(primaryRule);
+
+    const outOfSyncRules = syncedRules.filter(
+        (syncedRule) => definition.ruleToVectorState(syncedRule) !== primaryRuleVectorState,
+    );
+
+    if (outOfSyncRules.length) {
+        await updateExistingPushRulesWithActions(
+            matrixClient,
+            // eslint-disable-next-line camelcase, @typescript-eslint/naming-convention
+            outOfSyncRules.map(({ rule_id }) => rule_id),
+            primaryRule.actions,
+        );
+    }
+};
+
+/**
+ * On changes to m.push_rules account data,
+ * check that synced push rules are in sync with their primary rule,
+ * and update any out of sync rules.
+ * synced rules are defined in VectorPushRulesDefinitions
+ * If updating a rule fails for any reason,
+ * the error is caught and handled silently
+ * @param accountDataEvent - MatrixEvent
+ * @param matrixClient - cli
+ * @returns Resolves when updates are complete
+ */
+export const monitorSyncedPushRules = async (
+    accountDataEvent: MatrixEvent | undefined,
+    matrixClient: MatrixClient,
+): Promise<void> => {
+    if (accountDataEvent?.getType() !== EventType.PushRules) {
+        return;
+    }
+    const pushProcessor = new PushProcessor(matrixClient);
+
+    Object.entries(VectorPushRulesDefinitions).forEach(async ([ruleId, definition]) => {
+        try {
+            await monitorSyncedRule(matrixClient, pushProcessor, ruleId, definition);
+        } catch (error) {
+            logger.error(`Failed to fully synchronise push rules for ${ruleId}`, error);
+        }
+    });
+};

src/utils/pushRules/updatePushRuleActions.ts

@@ -0,0 +1,75 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { MatrixClient } from "matrix-js-sdk/src/client";
+import { IPushRule, PushRuleAction, PushRuleKind } from "matrix-js-sdk/src/matrix";
+import { PushProcessor } from "matrix-js-sdk/src/pushprocessor";
+
+/**
+ * Sets the actions for a given push rule id and kind
+ * When actions are falsy, disables the rule
+ * @param matrixClient - cli
+ * @param ruleId - rule id to update
+ * @param kind - PushRuleKind
+ * @param actions - push rule actions to set for rule
+ */
+export const updatePushRuleActions = async (
+    matrixClient: MatrixClient,
+    ruleId: IPushRule["rule_id"],
+    kind: PushRuleKind,
+    actions?: PushRuleAction[],
+): Promise<void> => {
+    if (!actions) {
+        await matrixClient.setPushRuleEnabled("global", kind, ruleId, false);
+    } else {
+        await matrixClient.setPushRuleActions("global", kind, ruleId, actions);
+        await matrixClient.setPushRuleEnabled("global", kind, ruleId, true);
+    }
+};
+
+interface PushRuleAndKind {
+    rule: IPushRule;
+    kind: PushRuleKind;
+}
+
+/**
+ * Update push rules with given actions
+ * Where they already exist for current user
+ * Rules are updated sequentially and stop at first error
+ * @param matrixClient - cli
+ * @param ruleIds - RuleIds of push rules to attempt to set actions for
+ * @param actions - push rule actions to set for rule
+ * @returns resolves when all rules have been updated
+ * @returns rejects when a rule update fails
+ */
+export const updateExistingPushRulesWithActions = async (
+    matrixClient: MatrixClient,
+    ruleIds?: IPushRule["rule_id"][],
+    actions?: PushRuleAction[],
+): Promise<void> => {
+    const pushProcessor = new PushProcessor(matrixClient);
+
+    const rules: PushRuleAndKind[] | undefined = ruleIds
+        ?.map((ruleId) => pushProcessor.getPushRuleAndKindById(ruleId))
+        .filter((n: PushRuleAndKind | null): n is PushRuleAndKind => Boolean(n));
+
+    if (!rules?.length) {
+        return;
+    }
+    for (const { kind, rule } of rules) {
+        await updatePushRuleActions(matrixClient, rule.rule_id, kind, actions);
+    }
+};