Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Add getLatestTimeline test for latest event being thread
  • Loading branch information
MadLittleMods committed Jul 13, 2022
commit 50b5bb1d031ceb2df26d23291bfdd5b644f4e752
63 changes: 63 additions & 0 deletions spec/integ/matrix-client-event-timeline.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,11 @@ describe("MatrixClient event timelines", function() {
});

describe("getLatestTimeline", function() {
beforeEach(() => {
// @ts-ignore
client.clientOpts.experimentalThreadSupport = true;
});

it("should create a new timeline for new events", function() {
const room = client.getRoom(roomId);
const timelineSet = room.getTimelineSets()[0];
Expand Down Expand Up @@ -791,6 +796,64 @@ describe("MatrixClient event timelines", function() {
]);
});

it("should successfully create a new timeline even when the latest event is a threaded reply", function() {
const room = client.getRoom(roomId);
const timelineSet = room.getTimelineSets()[0];
expect(timelineSet.thread).toBeUndefined();

const latestMessageId = 'threadedEvent1:bar';

httpBackend.when("GET", "/rooms/!foo%3Abar/messages")
.respond(200, function() {
return {
chunk: [{
event_id: latestMessageId,
}],
};
});

httpBackend.when("GET", `/rooms/!foo%3Abar/context/${encodeURIComponent(latestMessageId)}`)
.respond(200, function() {
return {
start: "start_token",
events_before: [THREAD_ROOT, EVENTS[0]],
event: THREAD_REPLY,
events_after: [],
state: [
ROOM_NAME_EVENT,
USER_MEMBERSHIP_EVENT,
],
end: "end_token",
};
});

// Make it easy to debug when there is a mismatch of events. We care
// about the event ID for direct comparison and the content for a
// human readable description.
const eventPropertiesToCompare = (event) => {
return {
eventId: event.event_id || event.getId(),
contentBody: event.content?.body || event.getContent()?.body,
};
};
return Promise.all([
client.getLatestTimeline(timelineSet).then(function(tl) {
const events = tl.getEvents();
const expectedEvents = [EVENTS[0], THREAD_ROOT];
expect(events.map(event => eventPropertiesToCompare(event)))
.toEqual(expectedEvents.map(event => eventPropertiesToCompare(event)));
// Sanity check: The threaded reply should not be in the timeline
expect(events.find(e => e.getId() === THREAD_REPLY.event_id)).toBeFalsy();

expect(tl.getPaginationToken(EventTimeline.BACKWARDS))
.toEqual("start_token");
expect(tl.getPaginationToken(EventTimeline.FORWARDS))
.toEqual("end_token");
}),
httpBackend.flushAllExpected(),
]);
});

it("should throw error when /messages does not return a message", () => {
const room = client.getRoom(roomId);
const timelineSet = room.getTimelineSets()[0];
Expand Down
4 changes: 1 addition & 3 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5334,9 +5334,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
// There is no guarantee that the event ended up in "timeline" (we might have switched to a neighbouring
// timeline) - so check the room's index again. On the other hand, there's no guarantee the event ended up
// anywhere, if it was later redacted, so we just return the timeline we first thought of.
return timelineSet.getTimelineForEvent(eventId)
?? timelineSet.room.findThreadForEvent(event)?.liveTimeline // for Threads degraded support
Copy link
Contributor Author

Choose a reason for hiding this comment

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

?? timelineSet.room.findThreadForEvent(event)?.liveTimeline // for Threads degraded support was added in #2261. Is it necessary? I'm just trying to get the context behind why we have it.

What does Threads degraded support exactly map to?

If supportsExperimentalThreads is disabled all events will be added to the main timeline (eventShouldLiveIn always returns shouldLiveInRoom: true)

Copy link
Member

@t3chguy t3chguy Jul 13, 2022

Choose a reason for hiding this comment

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

Threads degraded support

Server has no APIs to help the client so the client has to build the threads objects without filters & relations support on the server based on /sync and /messages data alone, which is a mode we have to continue to support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the context!

I think it's safe to drop this case given getTimelineForEvent is always used in a fire and forget manner, see #2521 (comment)

?? timeline;
return timelineSet.getTimelineForEvent(eventId) ?? timeline;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/models/event-timeline-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,8 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
// Make sure events don't get mixed in timelines they shouldn't be in
// (e.g. a threaded message should not be in the main timeline).
if (!this.canContain(event)) {
logger.warn(`EventTimelineSet.addEventToTimeline: Ignoring event=${event.getId()} that does not belong in timeline=${timeline.toString()} timelineSet(threadId=${this.thread?.id})`);
logger.warn(`EventTimelineSet.addEventToTimeline: Ignoring event=${event.getId()} that does not belong " +
"in timeline=${timeline.toString()} timelineSet(threadId=${this.thread?.id})`);
return;
}
Comment on lines +698 to +709
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Split out to #2848


Expand Down