Sort muted rooms to the bottom of their section of the room list (#10592)

* muted-to-the-bottom POC

* split muted rooms in natural algorithm

* add previous event to account data dispatch

* add muted to notification state

* sort muted rooms to the bottom

* only split muted rooms when sorting is RECENT

* remove debugs

* use RoomNotifState better

* add default notifications test util

* test getChangedOverrideRoomPushRules

* remove file

* test roomudpate in roomliststore

* unit test ImportanceAlgorithm

* strict fixes

* test recent x importance with muted rooms

* unit test NaturalAlgorithm

* test naturalalgorithm with muted rooms

* strict fixes

* comments

* add push rules test utility

* strict fixes

* more strict

* tidy comment

* document previousevent on account data dispatch event

* simplify (?) room mute rule utilities, comments

* remove debug
This commit is contained in:
Kerry 2023-05-05 13:53:26 +12:00 committed by GitHub
parent 3ca957b541
commit 44e0732144
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 765 additions and 27 deletions

View file

@ -182,19 +182,44 @@ function findOverrideMuteRule(roomId: string): IPushRule | null {
return null;
}
for (const rule of cli.pushRules.global.override) {
if (rule.enabled && isRuleForRoom(roomId, rule) && isMuteRule(rule)) {
if (rule.enabled && isRuleRoomMuteRuleForRoomId(roomId, rule)) {
return rule;
}
}
return null;
}
function isRuleForRoom(roomId: string, rule: IPushRule): boolean {
if (rule.conditions?.length !== 1) {
/**
* Checks if a given rule is a room mute rule as implemented by EW
* - matches every event in one room (one condition that is an event match on roomId)
* - silences notifications (one action that is `DontNotify`)
* @param rule - push rule
* @returns {boolean} - true when rule mutes a room
*/
export function isRuleMaybeRoomMuteRule(rule: IPushRule): boolean {
return (
// matches every event in one room
rule.conditions?.length === 1 &&
rule.conditions[0].kind === ConditionKind.EventMatch &&
rule.conditions[0].key === "room_id" &&
// silences notifications
isMuteRule(rule)
);
}
/**
* Checks if a given rule is a room mute rule as implemented by EW
* @param roomId - id of room to match
* @param rule - push rule
* @returns {boolean} true when rule mutes the given room
*/
function isRuleRoomMuteRuleForRoomId(roomId: string, rule: IPushRule): boolean {
if (!isRuleMaybeRoomMuteRule(rule)) {
return false;
}
const cond = rule.conditions[0];
return cond.kind === ConditionKind.EventMatch && cond.key === "room_id" && cond.pattern === roomId;
// isRuleMaybeRoomMuteRule checks this condition exists
const cond = rule.conditions![0]!;
return cond.pattern === roomId;
}
function isMuteRule(rule: IPushRule): boolean {

View file

@ -48,6 +48,7 @@ function createSyncAction(matrixClient: MatrixClient, state: string, prevState:
* @property {MatrixEvent} event the MatrixEvent that triggered the dispatch.
* @property {string} event_type the type of the MatrixEvent, e.g. "m.direct".
* @property {Object} event_content the content of the MatrixEvent.
* @property {MatrixEvent} previousEvent the previous account data event of the same type, if present
*/
/**
@ -56,14 +57,20 @@ function createSyncAction(matrixClient: MatrixClient, state: string, prevState:
*
* @param {MatrixClient} matrixClient the matrix client.
* @param {MatrixEvent} accountDataEvent the account data event.
* @param {MatrixEvent | undefined} previousAccountDataEvent the previous account data event of the same type, if present
* @returns {AccountDataAction} an action of type MatrixActions.accountData.
*/
function createAccountDataAction(matrixClient: MatrixClient, accountDataEvent: MatrixEvent): ActionPayload {
function createAccountDataAction(
matrixClient: MatrixClient,
accountDataEvent: MatrixEvent,
previousAccountDataEvent?: MatrixEvent,
): ActionPayload {
return {
action: "MatrixActions.accountData",
event: accountDataEvent,
event_type: accountDataEvent.getType(),
event_content: accountDataEvent.getContent(),
previousEvent: previousAccountDataEvent,
};
}

View file

@ -17,6 +17,7 @@ limitations under the License.
import { _t } from "../../languageHandler";
export enum NotificationColor {
Muted,
// Inverted (None -> Red) because we do integer comparisons on this
None, // nothing special
// TODO: Remove bold with notifications: https://github.com/vector-im/element-web/issues/14227

View file

@ -24,6 +24,7 @@ export interface INotificationStateSnapshotParams {
symbol: string | null;
count: number;
color: NotificationColor;
muted: boolean;
}
export enum NotificationStateEvents {
@ -42,6 +43,7 @@ export abstract class NotificationState
protected _symbol: string | null = null;
protected _count = 0;
protected _color: NotificationColor = NotificationColor.None;
protected _muted = false;
private watcherReferences: string[] = [];
@ -66,6 +68,10 @@ export abstract class NotificationState
return this._color;
}
public get muted(): boolean {
return this._muted;
}
public get isIdle(): boolean {
return this.color <= NotificationColor.None;
}
@ -110,16 +116,18 @@ export class NotificationStateSnapshot {
private readonly symbol: string | null;
private readonly count: number;
private readonly color: NotificationColor;
private readonly muted: boolean;
public constructor(state: INotificationStateSnapshotParams) {
this.symbol = state.symbol;
this.count = state.count;
this.color = state.color;
this.muted = state.muted;
}
public isDifferentFrom(other: INotificationStateSnapshotParams): boolean {
const before = { count: this.count, symbol: this.symbol, color: this.color };
const after = { count: other.count, symbol: other.symbol, color: other.color };
const before = { count: this.count, symbol: this.symbol, color: this.color, muted: this.muted };
const after = { count: other.count, symbol: other.symbol, color: other.color, muted: other.muted };
return JSON.stringify(before) !== JSON.stringify(after);
}
}

View file

@ -93,9 +93,12 @@ export class RoomNotificationState extends NotificationState implements IDestroy
const snapshot = this.snapshot();
const { color, symbol, count } = RoomNotifs.determineUnreadState(this.room);
const muted =
RoomNotifs.getRoomNotifsState(this.room.client, this.room.roomId) === RoomNotifs.RoomNotifState.Mute;
this._color = color;
this._symbol = symbol;
this._count = count;
this._muted = muted;
// finally, publish an update if needed
this.emitIfUpdated(snapshot);

View file

@ -40,6 +40,7 @@ import { RoomListStore as Interface, RoomListStoreEvent } from "./Interface";
import { SlidingRoomListStoreClass } from "./SlidingRoomListStore";
import { UPDATE_EVENT } from "../AsyncStore";
import { SdkContextClass } from "../../contexts/SDKContext";
import { getChangedOverrideRoomMutePushRules } from "./utils/roomMute";
interface IState {
// state is tracked in underlying classes
@ -289,6 +290,17 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> implements
this.onDispatchMyMembership(<any>payload);
return;
}
const possibleMuteChangeRoomIds = getChangedOverrideRoomMutePushRules(payload);
if (possibleMuteChangeRoomIds) {
for (const roomId of possibleMuteChangeRoomIds) {
const room = roomId && this.matrixClient.getRoom(roomId);
if (room) {
await this.handleRoomUpdate(room, RoomUpdateCause.PossibleMuteChange);
}
}
this.updateFn.trigger();
}
}
/**

View file

@ -42,6 +42,7 @@ const CATEGORY_ORDER = [
NotificationColor.Grey,
NotificationColor.Bold,
NotificationColor.None, // idle
NotificationColor.Muted,
];
/**
@ -81,6 +82,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
[NotificationColor.Grey]: [],
[NotificationColor.Bold]: [],
[NotificationColor.None]: [],
[NotificationColor.Muted]: [],
};
for (const room of rooms) {
const category = this.getRoomCategory(room);
@ -94,7 +96,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
// It's fine for us to call this a lot because it's cached, and we shouldn't be
// wasting anything by doing so as the store holds single references
const state = RoomNotificationStateStore.instance.getRoomState(room);
return state.color;
return this.isMutedToBottom && state.muted ? NotificationColor.Muted : state.color;
}
public setRooms(rooms: Room[]): void {
@ -164,15 +166,25 @@ export class ImportanceAlgorithm extends OrderingAlgorithm {
return this.handleSplice(room, cause);
}
if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) {
if (
cause !== RoomUpdateCause.Timeline &&
cause !== RoomUpdateCause.ReadReceipt &&
cause !== RoomUpdateCause.PossibleMuteChange
) {
throw new Error(`Unsupported update cause: ${cause}`);
}
const category = this.getRoomCategory(room);
// don't react to mute changes when we are not sorting by mute
if (cause === RoomUpdateCause.PossibleMuteChange && !this.isMutedToBottom) {
return false;
}
if (this.sortingAlgorithm === SortAlgorithm.Manual) {
return false; // Nothing to do here.
}
const category = this.getRoomCategory(room);
const roomIdx = this.getRoomIndex(room);
if (roomIdx === -1) {
throw new Error(`Room ${room.roomId} has no index in ${this.tagId}`);

View file

@ -21,42 +21,191 @@ import { SortAlgorithm } from "../models";
import { sortRoomsWithAlgorithm } from "../tag-sorting";
import { OrderingAlgorithm } from "./OrderingAlgorithm";
import { RoomUpdateCause, TagID } from "../../models";
import { RoomNotificationStateStore } from "../../../notifications/RoomNotificationStateStore";
type NaturalCategorizedRoomMap = {
defaultRooms: Room[];
mutedRooms: Room[];
};
/**
* Uses the natural tag sorting algorithm order to determine tag ordering. No
* additional behavioural changes are present.
*/
export class NaturalAlgorithm extends OrderingAlgorithm {
private cachedCategorizedOrderedRooms: NaturalCategorizedRoomMap = {
defaultRooms: [],
mutedRooms: [],
};
public constructor(tagId: TagID, initialSortingAlgorithm: SortAlgorithm) {
super(tagId, initialSortingAlgorithm);
}
public setRooms(rooms: Room[]): void {
this.cachedOrderedRooms = sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm);
const { defaultRooms, mutedRooms } = this.categorizeRooms(rooms);
this.cachedCategorizedOrderedRooms = {
defaultRooms: sortRoomsWithAlgorithm(defaultRooms, this.tagId, this.sortingAlgorithm),
mutedRooms: sortRoomsWithAlgorithm(mutedRooms, this.tagId, this.sortingAlgorithm),
};
this.buildCachedOrderedRooms();
}
public handleRoomUpdate(room: Room, cause: RoomUpdateCause): boolean {
const isSplice = cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved;
const isInPlace = cause === RoomUpdateCause.Timeline || cause === RoomUpdateCause.ReadReceipt;
const isInPlace =
cause === RoomUpdateCause.Timeline ||
cause === RoomUpdateCause.ReadReceipt ||
cause === RoomUpdateCause.PossibleMuteChange;
const isMuted = this.isMutedToBottom && this.getRoomIsMuted(room);
if (!isSplice && !isInPlace) {
throw new Error(`Unsupported update cause: ${cause}`);
}
if (cause === RoomUpdateCause.NewRoom) {
this.cachedOrderedRooms.push(room);
} else if (cause === RoomUpdateCause.RoomRemoved) {
const idx = this.getRoomIndex(room);
if (idx >= 0) {
this.cachedOrderedRooms.splice(idx, 1);
if (isMuted) {
this.cachedCategorizedOrderedRooms.mutedRooms = sortRoomsWithAlgorithm(
[...this.cachedCategorizedOrderedRooms.mutedRooms, room],
this.tagId,
this.sortingAlgorithm,
);
} else {
logger.warn(`Tried to remove unknown room from ${this.tagId}: ${room.roomId}`);
this.cachedCategorizedOrderedRooms.defaultRooms = sortRoomsWithAlgorithm(
[...this.cachedCategorizedOrderedRooms.defaultRooms, room],
this.tagId,
this.sortingAlgorithm,
);
}
this.buildCachedOrderedRooms();
return true;
} else if (cause === RoomUpdateCause.RoomRemoved) {
return this.removeRoom(room);
} else if (cause === RoomUpdateCause.PossibleMuteChange) {
if (this.isMutedToBottom) {
return this.onPossibleMuteChange(room);
} else {
return false;
}
}
// TODO: Optimize this to avoid useless operations: https://github.com/vector-im/element-web/issues/14457
// For example, we can skip updates to alphabetic (sometimes) and manually ordered tags
this.cachedOrderedRooms = sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm);
if (isMuted) {
this.cachedCategorizedOrderedRooms.mutedRooms = sortRoomsWithAlgorithm(
this.cachedCategorizedOrderedRooms.mutedRooms,
this.tagId,
this.sortingAlgorithm,
);
} else {
this.cachedCategorizedOrderedRooms.defaultRooms = sortRoomsWithAlgorithm(
this.cachedCategorizedOrderedRooms.defaultRooms,
this.tagId,
this.sortingAlgorithm,
);
}
this.buildCachedOrderedRooms();
return true;
}
/**
* Remove a room from the cached room list
* @param room Room to remove
* @returns {boolean} true when room list should update as result
*/
private removeRoom(room: Room): boolean {
const defaultIndex = this.cachedCategorizedOrderedRooms.defaultRooms.findIndex((r) => r.roomId === room.roomId);
if (defaultIndex > -1) {
this.cachedCategorizedOrderedRooms.defaultRooms.splice(defaultIndex, 1);
this.buildCachedOrderedRooms();
return true;
}
const mutedIndex = this.cachedCategorizedOrderedRooms.mutedRooms.findIndex((r) => r.roomId === room.roomId);
if (mutedIndex > -1) {
this.cachedCategorizedOrderedRooms.mutedRooms.splice(mutedIndex, 1);
this.buildCachedOrderedRooms();
return true;
}
logger.warn(`Tried to remove unknown room from ${this.tagId}: ${room.roomId}`);
// room was not in cached lists, no update
return false;
}
/**
* Sets cachedOrderedRooms from cachedCategorizedOrderedRooms
*/
private buildCachedOrderedRooms(): void {
this.cachedOrderedRooms = [
...this.cachedCategorizedOrderedRooms.defaultRooms,
...this.cachedCategorizedOrderedRooms.mutedRooms,
];
}
private getRoomIsMuted(room: Room): boolean {
// It's fine for us to call this a lot because it's cached, and we shouldn't be
// wasting anything by doing so as the store holds single references
const state = RoomNotificationStateStore.instance.getRoomState(room);
return state.muted;
}
private categorizeRooms(rooms: Room[]): NaturalCategorizedRoomMap {
if (!this.isMutedToBottom) {
return { defaultRooms: rooms, mutedRooms: [] };
}
return rooms.reduce<NaturalCategorizedRoomMap>(
(acc, room: Room) => {
if (this.getRoomIsMuted(room)) {
acc.mutedRooms.push(room);
} else {
acc.defaultRooms.push(room);
}
return acc;
},
{ defaultRooms: [], mutedRooms: [] } as NaturalCategorizedRoomMap,
);
}
private onPossibleMuteChange(room: Room): boolean {
const isMuted = this.getRoomIsMuted(room);
if (isMuted) {
const defaultIndex = this.cachedCategorizedOrderedRooms.defaultRooms.findIndex(
(r) => r.roomId === room.roomId,
);
// room has been muted
if (defaultIndex > -1) {
// remove from the default list
this.cachedCategorizedOrderedRooms.defaultRooms.splice(defaultIndex, 1);
// add to muted list and reorder
this.cachedCategorizedOrderedRooms.mutedRooms = sortRoomsWithAlgorithm(
[...this.cachedCategorizedOrderedRooms.mutedRooms, room],
this.tagId,
this.sortingAlgorithm,
);
// rebuild
this.buildCachedOrderedRooms();
return true;
}
} else {
const mutedIndex = this.cachedCategorizedOrderedRooms.mutedRooms.findIndex((r) => r.roomId === room.roomId);
// room has been unmuted
if (mutedIndex > -1) {
// remove from the muted list
this.cachedCategorizedOrderedRooms.mutedRooms.splice(mutedIndex, 1);
// add to default list and reorder
this.cachedCategorizedOrderedRooms.defaultRooms = sortRoomsWithAlgorithm(
[...this.cachedCategorizedOrderedRooms.defaultRooms, room],
this.tagId,
this.sortingAlgorithm,
);
// rebuild
this.buildCachedOrderedRooms();
return true;
}
}
return false;
}
}

View file

@ -42,6 +42,10 @@ export abstract class OrderingAlgorithm {
return this.cachedOrderedRooms;
}
public get isMutedToBottom(): boolean {
return this.sortingAlgorithm === SortAlgorithm.Recent;
}
/**
* Sets the sorting algorithm to use within the list.
* @param newAlgorithm The new algorithm. Must be defined.

View file

@ -43,6 +43,7 @@ export type TagID = string | DefaultTagID;
export enum RoomUpdateCause {
Timeline = "TIMELINE",
PossibleTagChange = "POSSIBLE_TAG_CHANGE",
PossibleMuteChange = "POSSIBLE_MUTE_CHANGE",
ReadReceipt = "READ_RECEIPT",
NewRoom = "NEW_ROOM",
RoomRemoved = "ROOM_REMOVED",

View file

@ -0,0 +1,54 @@
/*
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 { MatrixEvent, EventType, IPushRules } from "matrix-js-sdk/src/matrix";
import { ActionPayload } from "../../../dispatcher/payloads";
import { isRuleMaybeRoomMuteRule } from "../../../RoomNotifs";
import { arrayDiff } from "../../../utils/arrays";
/**
* Gets any changed push rules that are room specific overrides
* that mute notifications
* @param actionPayload
* @returns {string[]} ruleIds of added or removed rules
*/
export const getChangedOverrideRoomMutePushRules = (actionPayload: ActionPayload): string[] | undefined => {
if (
actionPayload.action !== "MatrixActions.accountData" ||
actionPayload.event?.getType() !== EventType.PushRules
) {
return undefined;
}
const event = actionPayload.event as MatrixEvent;
const prevEvent = actionPayload.previousEvent as MatrixEvent | undefined;
if (!event || !prevEvent) {
return undefined;
}
const roomPushRules = (event.getContent() as IPushRules)?.global?.override?.filter(isRuleMaybeRoomMuteRule);
const prevRoomPushRules = (prevEvent?.getContent() as IPushRules)?.global?.override?.filter(
isRuleMaybeRoomMuteRule,
);
const { added, removed } = arrayDiff(
prevRoomPushRules?.map((rule) => rule.rule_id) || [],
roomPushRules?.map((rule) => rule.rule_id) || [],
);
return [...added, ...removed];
};