From f6427651492ab9499995b519703121836f8ef82e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 29 Nov 2022 10:55:15 +0000 Subject: [PATCH] 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. --- src/RoomNotifs.ts | 7 +++--- src/hooks/useUnreadNotifications.ts | 2 +- src/stores/local-echo/RoomEchoChamber.ts | 7 ++++-- .../notifications/RoomNotificationState.ts | 4 ++- test/RoomNotifs-test.ts | 25 +++++++++++-------- 5 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/RoomNotifs.ts b/src/RoomNotifs.ts index 6c1e07e66b..04379c4683 100644 --- 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 index 3262137274..5510eae46a 100644 --- 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 index 284aada23e..85313bd573 100644 --- a/src/stores/local-echo/RoomEchoChamber.ts +++ b/src/stores/local-echo/RoomEchoChamber.ts @@ -50,7 +50,7 @@ export class RoomEchoChamber extends GenericEchoChamber { 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(); } @@ -58,7 +58,10 @@ export class RoomEchoChamber extends GenericEchoChamber { }); 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", () => {