Skip to content

Commit

Permalink
Annotate events with executed push rule (#3284)
Browse files Browse the repository at this point in the history
* unit test paginating /notifications

* add push rule to event

* 1% more test coverage
  • Loading branch information
Kerry authored Apr 17, 2023
1 parent d40d5c8 commit 4f67e59
Show file tree
Hide file tree
Showing 6 changed files with 234 additions and 24 deletions.
71 changes: 60 additions & 11 deletions spec/unit/matrix-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ import {
Method,
Room,
EventTimelineSet,
PushRuleActionName,
TweakName,
RuleId,
IPushRule,
ConditionKind,
} from "../../src";
import { supportsMatrixCall } from "../../src/webrtc/call";
import { makeBeaconEvent } from "../test-utils/beacon";
Expand Down Expand Up @@ -2751,7 +2756,7 @@ describe("MatrixClient", function () {
actions: ["notify"],
room_id: "__proto__",
event: testUtils.mkMessage({
user: userId,
user: "@villain:server.org",
room: "!roomId:server.org",
msg: "I am nefarious",
}),
Expand All @@ -2762,24 +2767,25 @@ describe("MatrixClient", function () {

const goodNotification = {
actions: ["notify"],
room_id: "!roomId:server.org",
event: testUtils.mkMessage({
user: userId,
room: "!roomId:server.org",
msg: "I am nice",
room_id: "!favouriteRoom:server.org",
event: new MatrixEvent({
sender: "@bob:server.org",
room_id: "!roomId:server.org",
type: "m.call.invite",
content: {},
}),
profile_tag: null,
read: true,
ts: 12345,
};

const highlightNotification = {
actions: ["notify", { set_tweak: "highlight" }],
actions: ["notify", { set_tweak: "highlight", value: true }],
room_id: "!roomId:server.org",
event: testUtils.mkMessage({
user: userId,
user: "@bob:server.org",
room: "!roomId:server.org",
msg: "I am highlighted",
msg: "I am highlighted banana",
}),
profile_tag: null,
read: true,
Expand All @@ -2795,6 +2801,41 @@ describe("MatrixClient", function () {
httpLookups = [response];
};

const callRule: IPushRule = {
actions: [PushRuleActionName.Notify],
conditions: [
{
kind: ConditionKind.EventMatch,
key: "type",
pattern: "m.call.invite",
},
],
default: true,
enabled: true,
rule_id: ".m.rule.call",
};
const masterRule: IPushRule = {
actions: [PushRuleActionName.DontNotify],
conditions: [],
default: true,
enabled: false,
rule_id: RuleId.Master,
};
const bananaRule = {
actions: [PushRuleActionName.Notify, { set_tweak: TweakName.Highlight, value: true }],
pattern: "banana",
rule_id: "banana",
default: false,
enabled: true,
} as IPushRule;
const pushRules = {
global: {
underride: [callRule],
override: [masterRule],
content: [bananaRule],
},
};

beforeEach(() => {
makeClient();

Expand All @@ -2807,6 +2848,8 @@ describe("MatrixClient", function () {
client.setNotifTimelineSet(notifTimelineSet);

setNotifsResponse();

client.setPushRules(pushRules);
});

it("should throw when trying to paginate forwards", async () => {
Expand Down Expand Up @@ -2839,7 +2882,7 @@ describe("MatrixClient", function () {
expect(timelineEvents.length).toEqual(2);
});

it("sets push actions on events and add to timeline", async () => {
it("sets push details on events and add to timeline", async () => {
setNotifsResponse([goodNotification, highlightNotification]);

const timelineSet = client.getNotifTimelineSet()!;
Expand All @@ -2853,9 +2896,15 @@ describe("MatrixClient", function () {
highlight: true,
},
});
expect(highlightEvent.getPushDetails().rule).toEqual({
...bananaRule,
kind: "content",
});
expect(goodEvent.getPushActions()).toEqual({
notify: true,
tweaks: {},
tweaks: {
highlight: false,
},
});
});
});
Expand Down
92 changes: 92 additions & 0 deletions spec/unit/models/event.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
import { MatrixEvent, MatrixEventEvent } from "../../../src/models/event";
import { emitPromise } from "../../test-utils/test-utils";
import { Crypto, IEventDecryptionResult } from "../../../src/crypto";
import { IAnnotatedPushRule, PushRuleActionName, TweakName } from "../../../src";

describe("MatrixEvent", () => {
it("should create copies of itself", () => {
Expand Down Expand Up @@ -216,4 +217,95 @@ describe("MatrixEvent", () => {
expect(encryptedEvent.replyEventId).toBeUndefined();
});
});

describe("push details", () => {
const pushRule = {
actions: [PushRuleActionName.Notify, { set_tweak: TweakName.Highlight, value: true }],
pattern: "banana",
rule_id: "banana",
kind: "override",
default: false,
enabled: true,
} as IAnnotatedPushRule;
describe("setPushActions()", () => {
it("sets actions on event", () => {
const actions = { notify: false, tweaks: {} };
const event = new MatrixEvent({
type: "com.example.test",
content: {
isTest: true,
},
});
event.setPushActions(actions);

expect(event.getPushActions()).toBe(actions);
});

it("sets actions to undefined", () => {
const event = new MatrixEvent({
type: "com.example.test",
content: {
isTest: true,
},
});
event.setPushActions(null);

// undefined is set on state
expect(event.getPushDetails().actions).toBe(undefined);
// but pushActions getter returns null when falsy
expect(event.getPushActions()).toBe(null);
});

it("clears existing push rule", () => {
const prevActions = { notify: true, tweaks: { highlight: true } };
const actions = { notify: false, tweaks: {} };
const event = new MatrixEvent({
type: "com.example.test",
content: {
isTest: true,
},
});
event.setPushDetails(prevActions, pushRule);

event.setPushActions(actions);

// rule is not in event push cache
expect(event.getPushDetails()).toEqual({ actions });
});
});

describe("setPushDetails()", () => {
it("sets actions and rule on event", () => {
const actions = { notify: false, tweaks: {} };
const event = new MatrixEvent({
type: "com.example.test",
content: {
isTest: true,
},
});
event.setPushDetails(actions, pushRule);

expect(event.getPushDetails()).toEqual({
actions,
rule: pushRule,
});
});
it("clears existing push rule", () => {
const prevActions = { notify: true, tweaks: { highlight: true } };
const actions = { notify: false, tweaks: {} };
const event = new MatrixEvent({
type: "com.example.test",
content: {
isTest: true,
},
});
event.setPushDetails(prevActions, pushRule);

event.setPushActions(actions);

// rule is not in event push cache
expect(event.getPushDetails()).toEqual({ actions });
});
});
});
});
5 changes: 2 additions & 3 deletions spec/unit/pushprocessor.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as utils from "../test-utils/test-utils";
import { IActionsObject, PushProcessor } from "../../src/pushprocessor";
import { ConditionKind, EventType, IContent, MatrixClient, MatrixEvent, PushRuleActionName, RuleId } from "../../src";
import { mockClientMethodsUser } from "../test-utils/client";

describe("NotificationService", function () {
const testUserId = "@ali:matrix.org";
Expand Down Expand Up @@ -45,9 +46,7 @@ describe("NotificationService", function () {
},
};
},
credentials: {
userId: testUserId,
},
...mockClientMethodsUser(testUserId),
supportsIntentionalMentions: () => true,
pushRules: {
device: {},
Expand Down
26 changes: 24 additions & 2 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
MatrixEvent,
MatrixEventEvent,
MatrixEventHandlerMap,
PushDetails,
} from "./models/event";
import { StubStore } from "./store/stub";
import { CallEvent, CallEventHandlerMap, createNewMatrixCall, MatrixCall, supportsMatrixCall } from "./webrtc/call";
Expand Down Expand Up @@ -5385,11 +5386,28 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
*/
public getPushActionsForEvent(event: MatrixEvent, forceRecalculate = false): IActionsObject | null {
if (!event.getPushActions() || forceRecalculate) {
event.setPushActions(this.pushProcessor.actionsForEvent(event));
const { actions, rule } = this.pushProcessor.actionsAndRuleForEvent(event);
event.setPushDetails(actions, rule);
}
return event.getPushActions();
}

/**
* Obtain a dict of actions which should be performed for this event according
* to the push rules for this user. Caches the dict on the event.
* @param event - The event to get push actions for.
* @param forceRecalculate - forces to recalculate actions for an event
* Useful when an event just got decrypted
* @returns A dict of actions to perform.
*/
public getPushDetailsForEvent(event: MatrixEvent, forceRecalculate = false): PushDetails | null {
if (!event.getPushDetails() || forceRecalculate) {
const { actions, rule } = this.pushProcessor.actionsAndRuleForEvent(event);
event.setPushDetails(actions, rule);
}
return event.getPushDetails();
}

/**
* @param info - The kind of info to set (e.g. 'avatar_url')
* @param data - The JSON object to set.
Expand Down Expand Up @@ -6064,7 +6082,11 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
for (let i = 0; i < res.notifications.length; i++) {
const notification = res.notifications[i];
const event = this.getEventMapper()(notification.event);
event.setPushActions(PushProcessor.actionListToActionsObject(notification.actions));

// @TODO(kerrya) reprocessing every notification is ugly
// remove if we get server MSC3994 support
this.getPushDetailsForEvent(event, true);

event.event.room_id = notification.room_id; // XXX: gutwrenching
matrixEvents[i] = event;
}
Expand Down
41 changes: 37 additions & 4 deletions src/models/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { EventStatus } from "./event-status";
import { DecryptionError } from "../crypto/algorithms";
import { CryptoBackend } from "../common-crypto/CryptoBackend";
import { WITHHELD_MESSAGES } from "../crypto/OlmDevice";
import { IAnnotatedPushRule } from "../@types/PushRules";

export { EventStatus } from "./event-status";

Expand Down Expand Up @@ -121,6 +122,11 @@ export interface IMentions {
room?: boolean;
}

export interface PushDetails {
rule?: IAnnotatedPushRule;
actions?: IActionsObject;
}

/**
* When an event is a visibility change event, as per MSC3531,
* the visibility change implied by the event.
Expand Down Expand Up @@ -220,7 +226,8 @@ export type MatrixEventHandlerMap = {
} & Pick<ThreadEventHandlerMap, ThreadEvent.Update>;

export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, MatrixEventHandlerMap> {
private pushActions: IActionsObject | null = null;
// applied push rule and action for this event
private pushDetails: PushDetails = {};
private _replacingEvent: MatrixEvent | null = null;
private _localRedactionEvent: MatrixEvent | null = null;
private _isCancelled = false;
Expand Down Expand Up @@ -888,7 +895,7 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
// highlighting when the user's name is mentioned rely on this happening. We also want
// to set the push actions before emitting so that any notification listeners don't
// pick up the wrong contents.
this.setPushActions(null);
this.setPushDetails();

if (options.emit !== false) {
this.emit(MatrixEventEvent.Decrypted, this, err);
Expand Down Expand Up @@ -1241,16 +1248,42 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
* @returns push actions
*/
public getPushActions(): IActionsObject | null {
return this.pushActions;
return this.pushDetails.actions || null;
}

/**
* Get the push details, if known, for this event
*
* @returns push actions
*/
public getPushDetails(): PushDetails {
return this.pushDetails;
}

/**
* Set the push actions for this event.
* Clears rule from push details if present
* @deprecated use `setPushDetails`
*
* @param pushActions - push actions
*/
public setPushActions(pushActions: IActionsObject | null): void {
this.pushActions = pushActions;
this.pushDetails = {
actions: pushActions || undefined,
};
}

/**
* Set the push details for this event.
*
* @param pushActions - push actions
* @param rule - the executed push rule
*/
public setPushDetails(pushActions?: IActionsObject, rule?: IAnnotatedPushRule): void {
this.pushDetails = {
actions: pushActions,
rule,
};
}

/**
Expand Down
Loading

0 comments on commit 4f67e59

Please sign in to comment.