Skip to content

Fix getLatestTimeline not working when the latest event in the room is a threaded message #2521

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.

Sign up for GitHub

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

Closed
MadLittleMods wants to merge 12 commits into from
Closed

Fix getLatestTimeline not working when the latest event in the room is a threaded message #2521

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5f4d10d
Fix getLatestTimeline not working when the latest even in the room wa…
MadLittleMods Jul 13, 2022
e8dc590
Adjust tests to not include problem events instead of undefined timeline
MadLittleMods Jul 13, 2022
9882eb6
Add tests for not being able to add mixed events
MadLittleMods Jul 13, 2022
fdeb9e0
Correct JSDoc and types for getEventTimeline
MadLittleMods Jul 13, 2022
78f8019
No longer need check since it should always return a timeline
MadLittleMods Jul 13, 2022
50b5bb1
Add getLatestTimeline test for latest event being thread
MadLittleMods Jul 13, 2022
a3457e1
Add non-optional client to Room instances
MadLittleMods Jul 13, 2022
cdef5cb
Fix lint
MadLittleMods Jul 13, 2022
cd41d7e
We expect void so return void
MadLittleMods Jul 19, 2022
a3ea213
Merge branch 'develop' into madlittlemods/refresh-timeline-when-we-se…
MadLittleMods Aug 29, 2022
b97de19
Merge branch 'develop' into madlittlemods/refresh-timeline-when-we-se…
MadLittleMods Oct 14, 2022
f82ea0c
Fix some lints
MadLittleMods Oct 14, 2022
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
Adjust tests to not include problem events instead of undefined timeline
  • Loading branch information
@MadLittleMods
MadLittleMods committed Jul 13, 2022
commit e8dc5907ddca98cddc591535e479831e43cb6668
50 changes: 40 additions & 10 deletions spec/integ/matrix-client-event-timeline.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ describe("MatrixClient event timelines", function() {
expect(timeline.getEvents().find(e => e.getId() === THREAD_ROOT.event_id)).toBeTruthy();
});

it("should return undefined when event is not in the thread that the given timelineSet is representing", () => {
it("should not include main timeline event when timelineSet is representing a thread", async () => {
// @ts-ignore
client.clientOpts.experimentalThreadSupport = true;
Thread.setServerSideSupport(true, false);
Expand All @@ -649,13 +649,40 @@ describe("MatrixClient event timelines", function() {
};
});

return Promise.all([
expect(client.getEventTimeline(timelineSet, EVENTS[0].event_id)).resolves.toBeUndefined(),
httpBackend.flushAllExpected(),
]);
// getEventTimeline -> thread.fetchInitialEvents
httpBackend.when("GET", "/rooms/!foo%3Abar/relations/" +
encodeURIComponent(THREAD_ROOT.event_id) + "/" +
Copy link
Contributor Author

@MadLittleMods MadLittleMods Aug 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

There are some lints in CI: https://github.com/matrix-org/matrix-js-sdk/runs/8077711103?check_suite_focus=true

But I don't see them locally even after re-installing node_modules to ensure correct versions.

$ yarn lint
yarn run v1.22.18
$ yarn lint:types && yarn lint:js
$ tsc --noEmit
$ eslint --max-warnings 0 src spec
✨  Done in 21.60s.

(on the correct branch, madlittlemods/refresh-timeline-when-we-see-msc2716-marker-events-v2)

This is also the same pattern we use in the existing tests here but don't appear because they're not in the diff so will need to refactor this.

Copy link
Member

@t3chguy t3chguy Aug 31, 2022

Choose a reason for hiding this comment

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

Those are shown if you use tsc --strict - hence being reported by the Typescript Strict Error Checker

Copy link
Contributor Author

@MadLittleMods MadLittleMods Nov 3, 2022

Choose a reason for hiding this comment

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

Can probably merge develop to resolve all of these unrelated errors now that #2835 fixed them up

encodeURIComponent(THREAD_RELATION_TYPE.name) + "?limit=20&direction=b")
.respond(200, function() {
return {
original_event: THREAD_ROOT,
chunk: [THREAD_REPLY],
// no next batch as this is the oldest end of the timeline
};
});

// getEventTimeline -> thread.fetchEvents
httpBackend.when("GET", "/rooms/!foo%3Abar/relations/" +
encodeURIComponent(THREAD_ROOT.event_id) + "/" +
encodeURIComponent(THREAD_RELATION_TYPE.name) + "?direction=b&limit=50")
.respond(200, function() {
return {
original_event: THREAD_ROOT,
chunk: [THREAD_REPLY],
// no next batch as this is the oldest end of the timeline
};
});

const timelinePromise = client.getEventTimeline(timelineSet, EVENTS[0].event_id);
await httpBackend.flushAllExpected();

const timeline = await timelinePromise;

// The main timeline event should not be in the timelineSet representing a thread
expect(timeline.getEvents().find(e => e.getId() === EVENTS[0].event_id)).toBeFalsy();
});

it("should return undefined when event is within a thread but timelineSet is not", () => {
it("should not include threaded reply when timelineSet is representing the main room", async () => {
// @ts-ignore
client.clientOpts.experimentalThreadSupport = true;
Thread.setServerSideSupport(true, false);
Expand All @@ -675,10 +702,13 @@ describe("MatrixClient event timelines", function() {
};
});

return Promise.all([
expect(client.getEventTimeline(timelineSet, THREAD_REPLY.event_id)).resolves.toBeUndefined(),
httpBackend.flushAllExpected(),
]);
const timelinePromise = client.getEventTimeline(timelineSet, THREAD_REPLY.event_id);
await httpBackend.flushAllExpected();

const timeline = await timelinePromise;

// The threaded reply should not be in a main room timeline
expect(timeline.getEvents().find(e => e.getId() === THREAD_REPLY.event_id)).toBeFalsy();
});

it("should should add lazy loading filter when requested", async () => {
Expand Down