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

Delayed events (Futures) / MSC4140 for call widget #12714

Merged
merged 1 commit into from
Aug 7, 2024
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@
"matrix-encrypt-attachment": "^1.0.3",
"matrix-events-sdk": "0.0.1",
"matrix-js-sdk": "github:matrix-org/matrix-js-sdk#develop",
"matrix-widget-api": "^1.5.0",
"matrix-widget-api": "^1.8.2",
"memoize-one": "^6.0.0",
"minimist": "^1.2.5",
"oidc-client-ts": "^3.0.1",
Expand Down
105 changes: 99 additions & 6 deletions src/stores/widgets/StopGapWidgetDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
EventDirection,
IOpenIDCredentials,
IOpenIDUpdate,
ISendDelayedEventDetails,
ISendEventDetails,
ITurnServer,
IReadEventRelationsResult,
Expand All @@ -33,6 +34,7 @@ import {
WidgetKind,
ISearchUserDirectoryResult,
IGetMediaConfigResult,
UpdateDelayedEventAction,
} from "matrix-widget-api";
import {
ClientEvent,
Expand All @@ -43,6 +45,7 @@ import {
Room,
Direction,
THREAD_RELATION_TYPE,
SendDelayedEventResponse,
StateEvents,
TimelineEvents,
} from "matrix-js-sdk/src/matrix";
Expand Down Expand Up @@ -128,6 +131,8 @@ export class StopGapWidgetDriver extends WidgetDriver {
this.allowedCapabilities.add(MatrixCapabilities.AlwaysOnScreen);
this.allowedCapabilities.add(MatrixCapabilities.MSC3846TurnServers);
this.allowedCapabilities.add(`org.matrix.msc2762.timeline:${inRoomId}`);
this.allowedCapabilities.add(MatrixCapabilities.MSC4157SendDelayedEvent);
this.allowedCapabilities.add(MatrixCapabilities.MSC4157UpdateDelayedEvent);

this.allowedCapabilities.add(
WidgetEventCapability.forRoomEvent(EventDirection.Send, "org.matrix.rageshake_request").raw,
Expand Down Expand Up @@ -160,7 +165,7 @@ export class StopGapWidgetDriver extends WidgetDriver {
`_${clientUserId}_${clientDeviceId}`,
).raw,
);
// MSC3779 version, with no leading underscore
// Version with no leading underscore, for room versions whose auth rules allow it
this.allowedCapabilities.add(
WidgetEventCapability.forStateEvent(
EventDirection.Send,
Expand Down Expand Up @@ -271,20 +276,20 @@ export class StopGapWidgetDriver extends WidgetDriver {
public async sendEvent<K extends keyof StateEvents>(
eventType: K,
content: StateEvents[K],
stateKey?: string,
targetRoomId?: string,
stateKey: string | null,
targetRoomId: string | null,
): Promise<ISendEventDetails>;
public async sendEvent<K extends keyof TimelineEvents>(
eventType: K,
content: TimelineEvents[K],
stateKey: null,
targetRoomId?: string,
targetRoomId: string | null,
): Promise<ISendEventDetails>;
public async sendEvent(
eventType: string,
content: IContent,
stateKey?: string | null,
targetRoomId?: string,
stateKey: string | null = null,
targetRoomId: string | null = null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the changes from undefined to null? I think in general intend want to move in the other direction.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's to match the signature of WidgetApi#sendEvent, which uses string | null = null for these arguments.

It also struck me as dangerous to allow stateKey to be undefined or null when there was only a !== null check on it, and felt that restricting both parameters to just null made things simpler.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, it would probably be good to change the widget api at some point but that needn't be today

): Promise<ISendEventDetails> {
const client = MatrixClientPeg.get();
const roomId = targetRoomId || SdkContextClass.instance.roomViewStore.getRoomId();
Expand Down Expand Up @@ -328,6 +333,94 @@ export class StopGapWidgetDriver extends WidgetDriver {
return { roomId, eventId: r.event_id };
}

/**
* @experimental Part of MSC4140 & MSC4157
* @see {@link WidgetDriver#sendDelayedEvent}
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this have some doc on the params please? For instance, I'm at a loss as to what parentDelayId might be.

Also, what would it mean to pass null to delay here?

Copy link
Member

@dbkr dbkr Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this is quite a lot of params for a function. May want to consider making it an object (although equally I guess we want it consistent with the rest of the API).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implements WidgetDriver#sendDelayedEvent, which is present as of v1.8.0 & is where the documentation is. I'll add a @see here to link to it.

Also, making this take an object would require updating the widget-api. It doesn't take an object for the sake of avoiding adding a new type in case the whole "parent" idea gets dropped.

public async sendDelayedEvent<K extends keyof StateEvents>(
delay: number | null,
parentDelayId: string | null,
eventType: K,
content: StateEvents[K],
stateKey: string | null,
targetRoomId: string | null,
): Promise<ISendDelayedEventDetails>;
/**
* @experimental Part of MSC4140 & MSC4157
*/
public async sendDelayedEvent<K extends keyof TimelineEvents>(
delay: number | null,
parentDelayId: string | null,
eventType: K,
content: TimelineEvents[K],
stateKey: null,
targetRoomId: string | null,
): Promise<ISendDelayedEventDetails>;
public async sendDelayedEvent(
delay: number | null,
parentDelayId: string | null,
eventType: string,
content: IContent,
stateKey: string | null = null,
targetRoomId: string | null = null,
): Promise<ISendDelayedEventDetails> {
const client = MatrixClientPeg.get();
const roomId = targetRoomId || SdkContextClass.instance.roomViewStore.getRoomId();

if (!client || !roomId) throw new Error("Not in a room or not attached to a client");

let delayOpts;
if (delay !== null) {
delayOpts = {
delay,
...(parentDelayId !== null && { parent_delay_id: parentDelayId }),
};
} else if (parentDelayId !== null) {
delayOpts = {
parent_delay_id: parentDelayId,
};
} else {
throw new Error("Must provide at least one of delay or parentDelayId");
toger5 marked this conversation as resolved.
Show resolved Hide resolved
}

let r: SendDelayedEventResponse | null;
if (stateKey !== null) {
// state event
r = await client._unstable_sendDelayedStateEvent(
roomId,
delayOpts,
eventType as keyof StateEvents,
content as StateEvents[keyof StateEvents],
stateKey,
);
} else {
// message event
r = await client._unstable_sendDelayedEvent(
roomId,
delayOpts,
null,
eventType as keyof TimelineEvents,
content as TimelineEvents[keyof TimelineEvents],
);
}

return {
roomId,
delayId: r.delay_id,
};
}

/**
* @experimental Part of MSC4140 & MSC4157
*/
public async updateDelayedEvent(delayId: string, action: UpdateDelayedEventAction): Promise<void> {
const client = MatrixClientPeg.get();

if (!client) throw new Error("Not in a room or not attached to a client");

await client._unstable_updateDelayedEvent(delayId, action);
}

public async sendToDevice(
eventType: string,
encrypted: boolean,
Expand Down
122 changes: 122 additions & 0 deletions test/stores/widgets/StopGapWidgetDriver-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
SimpleObservable,
OpenIDRequestState,
IOpenIDUpdate,
UpdateDelayedEventAction,
} from "matrix-widget-api";
import {
ApprovalOpts,
Expand Down Expand Up @@ -122,6 +123,8 @@ describe("StopGapWidgetDriver", () => {
"org.matrix.msc3819.receive.to_device:org.matrix.call.sdp_stream_metadata_changed",
"org.matrix.msc3819.send.to_device:m.call.replaces",
"org.matrix.msc3819.receive.to_device:m.call.replaces",
"org.matrix.msc4157.send.delayed_event",
"org.matrix.msc4157.update_delayed_event",
]);

// As long as this resolves, we'll know that it didn't try to pop up a modal
Expand Down Expand Up @@ -388,6 +391,125 @@ describe("StopGapWidgetDriver", () => {
});
});

describe("sendDelayedEvent", () => {
let driver: WidgetDriver;
const roomId = "!this-room-id";

beforeEach(() => {
driver = mkDefaultDriver();
});

it("cannot send delayed events with missing arguments", async () => {
await expect(driver.sendDelayedEvent(null, null, EventType.RoomMessage, {})).rejects.toThrow(
"Must provide at least one of",
);
});

it("sends delayed message events", async () => {
client._unstable_sendDelayedEvent.mockResolvedValue({
delay_id: "id",
});

await expect(driver.sendDelayedEvent(2000, null, EventType.RoomMessage, {})).resolves.toEqual({
roomId,
delayId: "id",
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do with an assertion that the client function is actually called in the way you think it should be called, otherwise this isn't testing what it claims to (same for the rest of these).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!


expect(client._unstable_sendDelayedEvent).toHaveBeenCalledWith(
roomId,
{ delay: 2000 },
null,
EventType.RoomMessage,
{},
);
});

it("sends child action delayed message events", async () => {
client._unstable_sendDelayedEvent.mockResolvedValue({
delay_id: "id-child",
});

await expect(driver.sendDelayedEvent(null, "id-parent", EventType.RoomMessage, {})).resolves.toEqual({
roomId,
delayId: "id-child",
});

expect(client._unstable_sendDelayedEvent).toHaveBeenCalledWith(
roomId,
{ parent_delay_id: "id-parent" },
null,
EventType.RoomMessage,
{},
);
});

it("sends delayed state events", async () => {
client._unstable_sendDelayedStateEvent.mockResolvedValue({
delay_id: "id",
});

await expect(driver.sendDelayedEvent(2000, null, EventType.RoomTopic, {}, "")).resolves.toEqual({
roomId,
delayId: "id",
});

expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalledWith(
roomId,
{ delay: 2000 },
EventType.RoomTopic,
{},
"",
);
});

it("sends child action delayed state events", async () => {
client._unstable_sendDelayedStateEvent.mockResolvedValue({
delay_id: "id-child",
});

await expect(driver.sendDelayedEvent(null, "id-parent", EventType.RoomTopic, {}, "")).resolves.toEqual({
roomId,
delayId: "id-child",
});

expect(client._unstable_sendDelayedStateEvent).toHaveBeenCalledWith(
roomId,
{ parent_delay_id: "id-parent" },
EventType.RoomTopic,
{},
"",
);
});
});

describe("updateDelayedEvent", () => {
let driver: WidgetDriver;

beforeEach(() => {
driver = mkDefaultDriver();
});

it("updates delayed events", async () => {
client._unstable_updateDelayedEvent.mockResolvedValue({});
for (const action of [
UpdateDelayedEventAction.Cancel,
UpdateDelayedEventAction.Restart,
UpdateDelayedEventAction.Send,
]) {
await expect(driver.updateDelayedEvent("id", action)).resolves.toBeUndefined();
expect(client._unstable_updateDelayedEvent).toHaveBeenCalledWith("id", action);
}
});

it("fails to update delayed events", async () => {
const errorMessage = "Cannot restart this delayed event";
client._unstable_updateDelayedEvent.mockRejectedValue(new Error(errorMessage));
await expect(driver.updateDelayedEvent("id", UpdateDelayedEventAction.Restart)).rejects.toThrow(
errorMessage,
);
});
});

describe("If the feature_dynamic_room_predecessors feature is not enabled", () => {
beforeEach(() => {
jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
Expand Down
4 changes: 4 additions & 0 deletions test/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ export function createTestClient(): MatrixClient {
});
}),

_unstable_sendDelayedEvent: jest.fn(),
_unstable_sendDelayedStateEvent: jest.fn(),
_unstable_updateDelayedEvent: jest.fn(),

searchUserDirectory: jest.fn().mockResolvedValue({ limited: false, results: [] }),
setDeviceVerified: jest.fn(),
joinRoom: jest.fn(),
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6938,10 +6938,10 @@ matrix-web-i18n@^3.2.1:
minimist "^1.2.8"
walk "^2.3.15"

matrix-widget-api@^1.5.0:
version "1.7.0"
resolved "https://registry.yarnpkg.com/matrix-widget-api/-/matrix-widget-api-1.7.0.tgz#ae3b44380f11bb03519d0bf0373dfc3341634667"
integrity sha512-dzSnA5Va6CeIkyWs89xZty/uv38HLyfjOrHGbbEikCa2ZV0HTkUNtrBMKlrn4CRYyDJ6yoO/3ssRwiR0jJvOkQ==
matrix-widget-api@^1.8.2:
version "1.8.2"
resolved "https://registry.yarnpkg.com/matrix-widget-api/-/matrix-widget-api-1.8.2.tgz#28d344502a85593740f560b0f8120e474a054505"
integrity sha512-kdmks3CvFNPIYN669Y4rO13KrazDvX8KHC7i6jOzJs8uZ8s54FNkuRVVyiQHeVCSZG5ixUqW9UuCj9lf03qxTQ==
dependencies:
"@types/events" "^3.0.0"
events "^3.2.0"
Expand Down
Loading