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.
This commit is contained in:
Richard van der Hoff 2022-11-29 10:55:15 +00:00 committed by GitHub
parent 8bd60d09dc
commit f642765149
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 28 additions and 17 deletions

View file

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

View file

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

View file

@ -50,7 +50,7 @@ export class RoomEchoChamber extends GenericEchoChamber<RoomEchoContext, CachedR
private onAccountData = (event: MatrixEvent) => { private onAccountData = (event: MatrixEvent) => {
if (event.getType() === EventType.PushRules) { if (event.getType() === EventType.PushRules) {
const currentVolume = this.properties.get(CachedRoomKey.NotificationVolume); 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) { if (currentVolume !== newVolume) {
this.updateNotificationVolume(); this.updateNotificationVolume();
} }
@ -58,7 +58,10 @@ export class RoomEchoChamber extends GenericEchoChamber<RoomEchoContext, CachedR
}; };
private 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.markEchoReceived(CachedRoomKey.NotificationVolume);
this.emit(PROPERTY_UPDATED, CachedRoomKey.NotificationVolume); this.emit(PROPERTY_UPDATED, CachedRoomKey.NotificationVolume);
} }

View file

@ -117,7 +117,9 @@ export class RoomNotificationState extends NotificationState implements IDestroy
this._color = NotificationColor.Unsent; this._color = NotificationColor.Unsent;
this._symbol = "!"; this._symbol = "!";
this._count = 1; // not used, technically 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. // When muted we suppress all notification states, even if we have context on them.
this._color = NotificationColor.None; this._color = NotificationColor.None;
this._symbol = null; this._symbol = null;

View file

@ -32,7 +32,8 @@ describe("RoomNotifs test", () => {
}); });
it("getRoomNotifsState handles rules with no conditions", () => { it("getRoomNotifsState handles rules with no conditions", () => {
mocked(MatrixClientPeg.get()).pushRules = { const cli = MatrixClientPeg.get();
mocked(cli).pushRules = {
global: { global: {
override: [{ override: [{
rule_id: "!roomId:server", 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", () => { it("getRoomNotifsState handles guest users", () => {
mocked(MatrixClientPeg.get()).isGuest.mockReturnValue(true); const cli = MatrixClientPeg.get();
expect(getRoomNotifsState("!roomId:server")).toBe(RoomNotifState.AllMessages); mocked(cli).isGuest.mockReturnValue(true);
expect(getRoomNotifsState(cli, "!roomId:server")).toBe(RoomNotifState.AllMessages);
}); });
it("getRoomNotifsState handles mute state", () => { it("getRoomNotifsState handles mute state", () => {
MatrixClientPeg.get().pushRules = { const cli = MatrixClientPeg.get();
cli.pushRules = {
global: { global: {
override: [{ override: [{
rule_id: "!roomId:server", 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", () => { it("getRoomNotifsState handles mentions only", () => {
MatrixClientPeg.get().getRoomPushRule = () => ({ const cli = MatrixClientPeg.get();
cli.getRoomPushRule = () => ({
rule_id: "!roomId:server", rule_id: "!roomId:server",
enabled: true, enabled: true,
default: false, default: false,
actions: [PushRuleActionName.DontNotify], actions: [PushRuleActionName.DontNotify],
}); });
expect(getRoomNotifsState("!roomId:server")).toBe(RoomNotifState.MentionsOnly); expect(getRoomNotifsState(cli, "!roomId:server")).toBe(RoomNotifState.MentionsOnly);
}); });
it("getRoomNotifsState handles noisy", () => { it("getRoomNotifsState handles noisy", () => {
MatrixClientPeg.get().getRoomPushRule = () => ({ const cli = MatrixClientPeg.get();
cli.getRoomPushRule = () => ({
rule_id: "!roomId:server", rule_id: "!roomId:server",
enabled: true, enabled: true,
default: false, default: false,
actions: [{ set_tweak: TweakName.Sound, value: "default" }], actions: [{ set_tweak: TweakName.Sound, value: "default" }],
}); });
expect(getRoomNotifsState("!roomId:server")).toBe(RoomNotifState.AllMessagesLoud); expect(getRoomNotifsState(cli, "!roomId:server")).toBe(RoomNotifState.AllMessagesLoud);
}); });
describe("getUnreadNotificationCount", () => { describe("getUnreadNotificationCount", () => {