Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate events with executed push rule #3284

Merged
merged 6 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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