-
-
Notifications
You must be signed in to change notification settings - Fork 829
Delayed events (Futures) / MSC4140 for call widget #12714
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ import { | |
EventDirection, | ||
IOpenIDCredentials, | ||
IOpenIDUpdate, | ||
ISendDelayedEventDetails, | ||
ISendEventDetails, | ||
ITurnServer, | ||
IReadEventRelationsResult, | ||
|
@@ -33,6 +34,7 @@ import { | |
WidgetKind, | ||
ISearchUserDirectoryResult, | ||
IGetMediaConfigResult, | ||
UpdateDelayedEventAction, | ||
} from "matrix-widget-api"; | ||
import { | ||
ClientEvent, | ||
|
@@ -43,6 +45,7 @@ import { | |
Room, | ||
Direction, | ||
THREAD_RELATION_TYPE, | ||
SendDelayedEventResponse, | ||
StateEvents, | ||
TimelineEvents, | ||
} from "matrix-js-sdk/src/matrix"; | ||
|
@@ -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, | ||
|
@@ -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, | ||
|
@@ -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, | ||
): Promise<ISendEventDetails> { | ||
const client = MatrixClientPeg.get(); | ||
const roomId = targetRoomId || SdkContextClass.instance.roomViewStore.getRoomId(); | ||
|
@@ -328,6 +333,94 @@ export class StopGapWidgetDriver extends WidgetDriver { | |
return { roomId, eventId: r.event_id }; | ||
} | ||
|
||
/** | ||
* @experimental Part of MSC4140 & MSC4157 | ||
* @see {@link WidgetDriver#sendDelayedEvent} | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Also, what would it mean to pass There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This implements 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, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ import { | |
SimpleObservable, | ||
OpenIDRequestState, | ||
IOpenIDUpdate, | ||
UpdateDelayedEventAction, | ||
} from "matrix-widget-api"; | ||
import { | ||
ApprovalOpts, | ||
|
@@ -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 | ||
|
@@ -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", | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 usesstring | null = null
for these arguments.It also struck me as dangerous to allow
stateKey
to beundefined
ornull
when there was only a!== null
check on it, and felt that restricting both parameters to justnull
made things simpler.There was a problem hiding this comment.
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