Pass a client into RoomNotifs.getRoomNotifsState (#9631)

Pass an explicit client into `RoomNotifs.getRoomNotifsState`, rather than relying on `MatrixClientPeg`. This resolves a race condition where we have a component which thinks it is using a particular component, but `MatrixClientPeg` has been updated.
matrix-org · Nov 29, 2022 · f642765 · f642765
1 parent 8bd60d0
commit f642765
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 17 deletions.
diff --git a/src/RoomNotifs.ts b/src/RoomNotifs.ts
@@ -25,6 +25,7 @@ import {
     TweakName,
 } from "matrix-js-sdk/src/@types/PushRules";
 import { EventType } from 'matrix-js-sdk/src/@types/event';
+import { MatrixClient } from "matrix-js-sdk/src/matrix";
 
 import { MatrixClientPeg } from './MatrixClientPeg';
 
@@ -35,8 +36,8 @@ export enum RoomNotifState {
     Mute = 'mute',
 }
 
-export function getRoomNotifsState(roomId: string): RoomNotifState {
-    if (MatrixClientPeg.get().isGuest()) return RoomNotifState.AllMessages;
+export function getRoomNotifsState(client: MatrixClient, roomId: string): RoomNotifState {
+    if (client.isGuest()) return RoomNotifState.AllMessages;
 
     // look through the override rules for a rule affecting this room:
     // if one exists, it will take precedence.
@@ -48,7 +49,7 @@ export function getRoomNotifsState(roomId: string): RoomNotifState {
     // for everything else, look at the room rule.
     let roomRule = null;
     try {
-        roomRule = MatrixClientPeg.get().getRoomPushRule('global', roomId);
+        roomRule = client.getRoomPushRule('global', roomId);
     } catch (err) {
         // Possible that the client doesn't have pushRules yet. If so, it
         // hasn't started either, so indicate that this room is not notifying.

diff --git a/src/hooks/useUnreadNotifications.ts b/src/hooks/useUnreadNotifications.ts
@@ -55,7 +55,7 @@ export const useUnreadNotifications = (room: Room, threadId?: string): {
             setSymbol("!");
             setCount(1);
             setColor(NotificationColor.Red);
-        } else if (getRoomNotifsState(room.roomId) === RoomNotifState.Mute) {
+        } else if (getRoomNotifsState(room.client, room.roomId) === RoomNotifState.Mute) {
             setSymbol(null);
             setCount(0);
             setColor(NotificationColor.None);

diff --git a/src/stores/local-echo/RoomEchoChamber.ts b/src/stores/local-echo/RoomEchoChamber.ts
@@ -50,15 +50,18 @@ export class RoomEchoChamber extends GenericEchoChamber<RoomEchoContext, CachedR
     private onAccountData = (event: MatrixEvent) => {
         if (event.getType() === EventType.PushRules) {
             const currentVolume = this.properties.get(CachedRoomKey.NotificationVolume);
-            const newVolume = getRoomNotifsState(this.context.room.roomId);
+            const newVolume = getRoomNotifsState(this.matrixClient, this.context.room.roomId);
             if (currentVolume !== newVolume) {
                 this.updateNotificationVolume();
             }
         }
     };
 
     private updateNotificationVolume() {
-        this.properties.set(CachedRoomKey.NotificationVolume, getRoomNotifsState(this.context.room.roomId));
+        this.properties.set(
+            CachedRoomKey.NotificationVolume,
+            getRoomNotifsState(this.matrixClient, this.context.room.roomId),
+        );
         this.markEchoReceived(CachedRoomKey.NotificationVolume);
         this.emit(PROPERTY_UPDATED, CachedRoomKey.NotificationVolume);
     }

diff --git a/src/stores/notifications/RoomNotificationState.ts b/src/stores/notifications/RoomNotificationState.ts
@@ -117,7 +117,9 @@ export class RoomNotificationState extends NotificationState implements IDestroy
             this._color = NotificationColor.Unsent;
             this._symbol = "!";
             this._count = 1; // not used, technically
-        } else if (RoomNotifs.getRoomNotifsState(this.room.roomId) === RoomNotifs.RoomNotifState.Mute) {
+        } else if (RoomNotifs.getRoomNotifsState(
+            this.room.client, this.room.roomId,
+        ) === RoomNotifs.RoomNotifState.Mute) {
             // When muted we suppress all notification states, even if we have context on them.
             this._color = NotificationColor.None;
             this._symbol = null;

diff --git a/test/RoomNotifs-test.ts b/test/RoomNotifs-test.ts
@@ -32,7 +32,8 @@ describe("RoomNotifs test", () => {
     });
 
     it("getRoomNotifsState handles rules with no conditions", () => {
-        mocked(MatrixClientPeg.get()).pushRules = {
+        const cli = MatrixClientPeg.get();
+        mocked(cli).pushRules = {
             global: {
                 override: [{
                     rule_id: "!roomId:server",
@@ -42,16 +43,18 @@ describe("RoomNotifs test", () => {
                 }],
             },
         };
-        expect(getRoomNotifsState("!roomId:server")).toBe(null);
+        expect(getRoomNotifsState(cli, "!roomId:server")).toBe(null);
     });
 
     it("getRoomNotifsState handles guest users", () => {
-        mocked(MatrixClientPeg.get()).isGuest.mockReturnValue(true);
-        expect(getRoomNotifsState("!roomId:server")).toBe(RoomNotifState.AllMessages);
+        const cli = MatrixClientPeg.get();
+        mocked(cli).isGuest.mockReturnValue(true);
+        expect(getRoomNotifsState(cli, "!roomId:server")).toBe(RoomNotifState.AllMessages);
     });
 
     it("getRoomNotifsState handles mute state", () => {
-        MatrixClientPeg.get().pushRules = {
+        const cli = MatrixClientPeg.get();
+        cli.pushRules = {
             global: {
                 override: [{
                     rule_id: "!roomId:server",
@@ -66,27 +69,29 @@ describe("RoomNotifs test", () => {
                 }],
             },
         };
-        expect(getRoomNotifsState("!roomId:server")).toBe(RoomNotifState.Mute);
+        expect(getRoomNotifsState(cli, "!roomId:server")).toBe(RoomNotifState.Mute);
     });
 
     it("getRoomNotifsState handles mentions only", () => {
-        MatrixClientPeg.get().getRoomPushRule = () => ({
+        const cli = MatrixClientPeg.get();
+        cli.getRoomPushRule = () => ({
             rule_id: "!roomId:server",
             enabled: true,
             default: false,
             actions: [PushRuleActionName.DontNotify],
         });
-        expect(getRoomNotifsState("!roomId:server")).toBe(RoomNotifState.MentionsOnly);
+        expect(getRoomNotifsState(cli, "!roomId:server")).toBe(RoomNotifState.MentionsOnly);
     });
 
     it("getRoomNotifsState handles noisy", () => {
-        MatrixClientPeg.get().getRoomPushRule = () => ({
+        const cli = MatrixClientPeg.get();
+        cli.getRoomPushRule = () => ({
             rule_id: "!roomId:server",
             enabled: true,
             default: false,
             actions: [{ set_tweak: TweakName.Sound, value: "default" }],
         });
-        expect(getRoomNotifsState("!roomId:server")).toBe(RoomNotifState.AllMessagesLoud);
+        expect(getRoomNotifsState(cli, "!roomId:server")).toBe(RoomNotifState.AllMessagesLoud);
     });
 
     describe("getUnreadNotificationCount", () => {