Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
Sort muted rooms to the bottom of their section of the room list (#10592
Browse files Browse the repository at this point in the history
)

* 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
  • Loading branch information
Kerry committed May 5, 2023
1 parent 3ca957b commit 44e0732
Show file tree
Hide file tree
Showing 15 changed files with 765 additions and 27 deletions.
35 changes: 30 additions & 5 deletions src/RoomNotifs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 8 additions & 1 deletion src/actions/MatrixActionCreators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/

/**
Expand All @@ -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,
};
}

Expand Down
1 change: 1 addition & 0 deletions src/stores/notifications/NotificationColor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions src/stores/notifications/NotificationState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export interface INotificationStateSnapshotParams {
symbol: string | null;
count: number;
color: NotificationColor;
muted: boolean;
}

export enum NotificationStateEvents {
Expand All @@ -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[] = [];

Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
}
}
3 changes: 3 additions & 0 deletions src/stores/notifications/RoomNotificationState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions src/stores/room-list/RoomListStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const CATEGORY_ORDER = [
NotificationColor.Grey,
NotificationColor.Bold,
NotificationColor.None, // idle
NotificationColor.Muted,
];

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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 {
Expand Down Expand Up @@ -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}`);
Expand Down
Loading

0 comments on commit 44e0732

Please sign in to comment.