Fix refreshLiveTimeline(...) not working when the latest event in the room is a threaded message
#2852
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5400,65 +5400,116 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa | |
| * Get an EventTimeline for the latest events in the room. This will just | ||
| * call `/messages` to get the latest message in the room, then use | ||
| * `client.getEventTimeline(...)` to construct a new timeline from it. | ||
| * Always returns timeline in the given `timelineSet`. | ||
| * | ||
| * @param {EventTimelineSet} timelineSet The timelineSet to find or add the timeline to | ||
| * | ||
| * @return {Promise} Resolves: | ||
| * {@link module:models/event-timeline~EventTimeline} timeline with the latest events in the room | ||
| */ | ||
| public async getLatestLiveTimeline(timelineSet: EventTimelineSet): Promise<EventTimeline> { | ||
| // don't allow any timeline support unless it's been enabled. | ||
| if (!this.timelineSupport) { | ||
| throw new Error("timeline support is disabled. Set the 'timelineSupport'" + | ||
| " parameter to true when creating MatrixClient to enable it."); | ||
| } | ||
|
|
||
| if (!timelineSet.room) { | ||
| throw new Error("getLatestLiveTimeline only supports room timelines"); | ||
| } | ||
|
|
||
| if (timelineSet.threadListType !== null || timelineSet.thread && Thread.hasServerSideSupport) { | ||
| throw new Error("getLatestLiveTimeline only supports live timelines"); | ||
| } | ||
|
|
||
| const messagesPath = utils.encodeUri( | ||
| "/rooms/$roomId/messages", { | ||
| $roomId: timelineSet.room.roomId, | ||
| }, | ||
| ); | ||
| const messageRequestParams: Record<string, string | string[]> = { | ||
| dir: 'b', | ||
|
|
||
| // Since we only use the latest message in the response, we only need to | ||
| // fetch the one message here. | ||
| limit: "1", | ||
| }; | ||
| if (this.clientOpts?.lazyLoadMembers) { | ||
| messageRequestParams.filter = JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER); | ||
| } | ||
| const messagesRes = await this.http.authedRequest<IMessagesResponse>( | ||
| Method.Get, | ||
| messagesPath, | ||
| messageRequestParams, | ||
| ); | ||
|
Comment on lines
+5598
to
+5616
There was a problem hiding this comment. Can this reuse |
||
| const latestEventInTimeline = messagesRes.chunk?.[0]; | ||
| const latestEventIdInTimeline = latestEventInTimeline?.event_id; | ||
| if (!latestEventIdInTimeline) { | ||
| throw new Error("No message returned when trying to construct getLatestLiveTimeline"); | ||
| } | ||
|
|
||
| const contextPath = utils.encodeUri( | ||
| "/rooms/$roomId/context/$eventId", { | ||
| $roomId: timelineSet.room.roomId, | ||
| $eventId: latestEventIdInTimeline, | ||
| }, | ||
| ); | ||
| let contextRequestParams: Record<string, string | string[]> | undefined = undefined; | ||
| if (this.clientOpts?.lazyLoadMembers) { | ||
| contextRequestParams = { filter: JSON.stringify(Filter.LAZY_LOADING_MESSAGES_FILTER) }; | ||
| } | ||
| const contextRes = await this.http.authedRequest<IContextResponse>( | ||
| Method.Get, | ||
| contextPath, | ||
| contextRequestParams, | ||
| ); | ||
| if (!contextRes.event || contextRes.event.event_id !== latestEventIdInTimeline) { | ||
| throw new Error( | ||
| `getLatestLiveTimeline: \`/context\` response did not include latestEventIdInTimeline=` + | ||
| `${latestEventIdInTimeline} which we were asking about. This is probably a bug in the ` + | ||
| `homeserver since we just saw the event with the other request above and now the server ` + | ||
| `claims it does not exist.`, | ||
| ); | ||
| } | ||
|
|
||
| // By the time the request completes, the event might have ended up in the timeline. | ||
| const shortcutTimelineForEvent = timelineSet.getTimelineForEvent(latestEventIdInTimeline); | ||
| if (shortcutTimelineForEvent) { | ||
| return shortcutTimelineForEvent; | ||
| } | ||
|
|
||
| const mapper = this.getEventMapper(); | ||
| const latestMatrixEventInTimeline = mapper(contextRes.event); | ||
| const events = [ | ||
| // Order events from most recent to oldest (reverse-chronological). | ||
| // We start with the last event, since that's the point at which we have known state. | ||
| // events_after is already backwards; events_before is forwards. | ||
| ...contextRes.events_after.reverse().map(mapper), | ||
| latestMatrixEventInTimeline, | ||
| ...contextRes.events_before.map(mapper), | ||
| ]; | ||
|
|
||
| // This function handles non-thread timelines only, but we still process any | ||
| // thread events to populate thread summaries. | ||
| let timeline = timelineSet.getTimelineForEvent(events[0].getId()!); | ||
| if (timeline) { | ||
| timeline.getState(EventTimeline.BACKWARDS)!.setUnknownStateEvents(contextRes.state.map(mapper)); | ||
| } else { | ||
| // If the `latestEventIdInTimeline` does not belong to this `timelineSet` | ||
| // then it will be ignored and not added to the `timelineSet`. We'll instead | ||
| // just create a new blank timeline in the `timelineSet` with the proper | ||
| // pagination tokens setup to continue paginating. | ||
| timeline = timelineSet.addTimeline(); | ||
| timeline.initialiseState(contextRes.state.map(mapper)); | ||
| timeline.getState(EventTimeline.FORWARDS)!.paginationToken = contextRes.end; | ||
| } | ||
|
|
||
| const [timelineEvents, threadedEvents] = timelineSet.room.partitionThreadedEvents(events); | ||
| timelineSet.addEventsToTimeline(timelineEvents, true, timeline, contextRes.start); | ||
| // The target event is not in a thread but process the contextual events, so we can show any threads around it. | ||
| this.processThreadEvents(timelineSet.room, threadedEvents, true); | ||
| this.processBeaconEvents(timelineSet.room, timelineEvents); | ||
|
|
||
| return timelineSet.getTimelineForEvent(latestEventIdInTimeline) ?? timeline; | ||
|
There was a problem hiding this comment. Opted for a new function as concluded in #2521 (comment) This is very similar to the existing logic in
In |
||
| } | ||
|
|
||
| /** | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 looks like some non-refresh-timeline tests got infected with
getLatestTimelineusage in the months while these PR's have sat.But I don't really want to support
getLatestTimelinefor threads which these tests rely on.Either we have to maintain the old
getLatestTimeline(...)alongsidegetLatestLiveTimeline(...)forever or we can refactor these tests away from it.Do we care about any external consumers that also may have started using it? Maybe should have marked it experimental to match the sole usage when it was introduced.