Skip to content

Draft: Refresh room timeline when we see a MSC2716 marker event #2282

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
wants to merge 21 commits into from
Closed

Draft: Refresh room timeline when we see a MSC2716 marker event #2282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2b029c9
Refresh room timeline when we see a MSC2716 marker event
MadLittleMods Apr 8, 2022
ffbcbff
Don't refresh timeline when first sync or from cache
MadLittleMods Apr 8, 2022
39cd6e9
Fix docstring typo
MadLittleMods Apr 8, 2022
e5f34db
Only notify client that timeline should be refreshed instead of doing…
MadLittleMods Apr 13, 2022
e070b3c
Remove console.log
MadLittleMods Apr 13, 2022
21f6aa2
Fixup docstrings
MadLittleMods Apr 13, 2022
9bfcf4b
Fix some lints
MadLittleMods Apr 13, 2022
1c2b955
Merge branch 'develop' into madlittlemods/refresh-timeline-when-we-se…
MadLittleMods Apr 13, 2022
05b9800
Fix some lints
MadLittleMods Apr 13, 2022
8b82bb5
Rename to roomState to align with other options objects
MadLittleMods Apr 13, 2022
35f3f04
Rename to toStartOfTimeline to align with other options objects
MadLittleMods Apr 13, 2022
b2636c3
Revert arg rename change for unused
MadLittleMods Apr 13, 2022
da57a74
Fix up tests
MadLittleMods Apr 13, 2022
c539b64
Fix timeline-window specs
MadLittleMods Apr 13, 2022
11040e5
Stop prompting to refresh timeline when syncing from cache
MadLittleMods Apr 13, 2022
07fcf27
Fix lints and move to logger
MadLittleMods Apr 13, 2022
701bf34
Remove todo
MadLittleMods Apr 13, 2022
c080c6c
Remove fixme in favor of aspirational explanation of what we could do…
MadLittleMods Apr 13, 2022
0694b84
Add some explanation comments
MadLittleMods Apr 13, 2022
26fb2b6
Add some tests
MadLittleMods Apr 13, 2022
8ca2645
WIP: messy timelineWasEmpty
MadLittleMods Apr 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
206 changes: 206 additions & 0 deletions spec/integ/matrix-client-syncing.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { MatrixEvent } from "../../src/models/event";
import { EventTimeline } from "../../src/models/event-timeline";
import { EventType } from "../../src/@types/event";
import * as utils from "../test-utils/test-utils";
import { TestClient } from "../TestClient";

Expand Down Expand Up @@ -461,6 +462,211 @@ describe("MatrixClient syncing", function() {
xit("should update the room topic", function() {

});

fdescribe("onMarkerStateEvent", () => {
it('no marker means the timeline does not need a refresh (check a sane default)', async () => {
const syncData = {
next_batch: "batch_token",
rooms: {
join: {},
},
};
syncData.rooms.join[roomOne] = {
timeline: {
events: [
utils.mkMessage({
room: roomOne, user: otherUserId, msg: "hello",
}),
],
prev_batch: "pagTok",
},
state: {
events: [
utils.mkEvent({
type: "m.room.create", room: roomOne, user: otherUserId,
content: {
creator: otherUserId,
},
}),
],
},
};

httpBackend.when("GET", "/sync").respond(200, syncData);

client.startClient();
await Promise.all([
httpBackend.flushAllExpected(),
awaitSyncEvent(),
]);

const room = client.getRoom(roomOne);
expect(room.getTimelineNeedsRefresh()).toEqual(false);
});

fit('when this is our first sync for the room, there is no timeline to refresh', async () => {
const syncData = {
next_batch: "batch_token",
rooms: {
join: {},
},
};
syncData.rooms.join[roomOne] = {
timeline: {
events: [
utils.mkEvent({
type: EventType.Marker, room: roomOne, user: otherUserId,
skey: "",
content: {
"m.insertion_id": "$abc",
},
}),
],
prev_batch: "pagTok",
},
state: {
events: [
utils.mkEvent({
type: "m.room.create", room: roomOne, user: otherUserId,
content: {
creator: otherUserId,
},
}),
],
},
};

httpBackend.when("GET", "/sync").respond(200, syncData);

client.startClient();
await Promise.all([
httpBackend.flushAllExpected(),
awaitSyncEvent(),
]);

const room = client.getRoom(roomOne);
expect(room.getTimelineNeedsRefresh()).toEqual(false);
});

it('a new marker event should mark the timeline as needing a refresh', async () => {
const syncData = {
next_batch: "batch_token",
rooms: {
join: {},
},
};
syncData.rooms.join[roomOne] = {
timeline: {
events: [
utils.mkMessage({
room: roomOne, user: otherUserId, msg: "hello",
}),
],
prev_batch: "pagTok",
},
state: {
events: [
utils.mkEvent({
type: "m.room.create", room: roomOne, user: otherUserId,
content: {
creator: otherUserId,
},
}),
],
},
};

const nextSyncData = {
next_batch: "batch_token",
rooms: {
join: {},
},
};
nextSyncData.rooms.join[roomOne] = {
timeline: {
events: [
utils.mkEvent({
type: EventType.Marker, room: roomOne, user: otherUserId,
skey: "",
content: {
"m.insertion_id": "$abc",
},
}),
],
prev_batch: "pagTok",
},
};

const markerEventId = nextSyncData.rooms.join[roomOne].timeline.events[0].event_id;

let emitCount = 0;
client.on("Room.historyImportedWithinTimeline", function(markerEvent, room) {
expect(markerEvent.getId()).toEqual(markerEventId);
expect(room.roomId).toEqual(roomOne);
emitCount += 1;
});

httpBackend.when("GET", "/sync").respond(200, syncData);
httpBackend.when("GET", "/sync").respond(200, nextSyncData);

client.startClient();
await Promise.all([
httpBackend.flushAllExpected(),
awaitSyncEvent(2),
]);

const room = client.getRoom(roomOne);
expect(room.getTimelineNeedsRefresh()).toEqual(true);
// Make sure "Room.historyImportedWithinTimeline" was emitted
expect(emitCount).toEqual(1);
expect(room.getLastMarkerEventIdProcessed()).toEqual(markerEventId);
});

it('marker event sent far back in the scroll back but since our last sync will cause the timeline to refresh', async () => {
const syncData = {
next_batch: "batch_token",
rooms: {
join: {},
},
};
syncData.rooms.join[roomOne] = {
timeline: {
events: [
// TODO: Update this scenario to match test title
utils.mkEvent({
type: EventType.Marker, room: roomOne, user: otherUserId,
skey: "",
content: {
"m.insertion_id": "$abc",
},
}),
],
prev_batch: "pagTok",
},
state: {
events: [
utils.mkEvent({
type: "m.room.create", room: roomOne, user: otherUserId,
content: {
creator: otherUserId,
},
}),
],
},
};

httpBackend.when("GET", "/sync").respond(200, syncData);

client.startClient();
await Promise.all([
httpBackend.flushAllExpected(),
awaitSyncEvent(),
]);

const room = client.getRoom(roomOne);
expect(room.getTimelineNeedsRefresh()).toEqual(true);
});
});
});

describe("timeline", function() {
Expand Down
66 changes: 39 additions & 27 deletions spec/unit/event-timeline.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,14 @@ describe("EventTimeline", function() {
];
timeline.initialiseState(events);
expect(timeline.startState.setStateEvents).toHaveBeenCalledWith(
events,
events, {
fromInitialState: true,
},
);
expect(timeline.endState.setStateEvents).toHaveBeenCalledWith(
events,
events, {
fromInitialState: true,
},
);
});

Expand All @@ -73,7 +77,7 @@ describe("EventTimeline", function() {
expect(function() {
timeline.initialiseState(state);
}).not.toThrow();
timeline.addEvent(event, false);
timeline.addEvent(event, { toStartOfTimeline: false });
expect(function() {
timeline.initialiseState(state);
}).toThrow();
Expand Down Expand Up @@ -149,19 +153,19 @@ describe("EventTimeline", function() {
];

it("should be able to add events to the end", function() {
timeline.addEvent(events[0], false);
timeline.addEvent(events[0], { toStartOfTimeline: false });
const initialIndex = timeline.getBaseIndex();
timeline.addEvent(events[1], false);
timeline.addEvent(events[1], { toStartOfTimeline: false });
expect(timeline.getBaseIndex()).toEqual(initialIndex);
expect(timeline.getEvents().length).toEqual(2);
expect(timeline.getEvents()[0]).toEqual(events[0]);
expect(timeline.getEvents()[1]).toEqual(events[1]);
});

it("should be able to add events to the start", function() {
timeline.addEvent(events[0], true);
timeline.addEvent(events[0], { toStartOfTimeline: true });
const initialIndex = timeline.getBaseIndex();
timeline.addEvent(events[1], true);
timeline.addEvent(events[1], { toStartOfTimeline: true });
expect(timeline.getBaseIndex()).toEqual(initialIndex + 1);
expect(timeline.getEvents().length).toEqual(2);
expect(timeline.getEvents()[0]).toEqual(events[1]);
Expand Down Expand Up @@ -203,9 +207,9 @@ describe("EventTimeline", function() {
content: { name: "Old Room Name" },
});

timeline.addEvent(newEv, false);
timeline.addEvent(newEv, { toStartOfTimeline: false });
expect(newEv.sender).toEqual(sentinel);
timeline.addEvent(oldEv, true);
timeline.addEvent(oldEv, { toStartOfTimeline: true });
expect(oldEv.sender).toEqual(oldSentinel);
});

Expand Down Expand Up @@ -242,9 +246,9 @@ describe("EventTimeline", function() {
const oldEv = utils.mkMembership({
room: roomId, mship: "ban", user: userB, skey: userA, event: true,
});
timeline.addEvent(newEv, false);
timeline.addEvent(newEv, { toStartOfTimeline: false });
expect(newEv.target).toEqual(sentinel);
timeline.addEvent(oldEv, true);
timeline.addEvent(oldEv, { toStartOfTimeline: true });
expect(oldEv.target).toEqual(oldSentinel);
});

Expand All @@ -262,13 +266,17 @@ describe("EventTimeline", function() {
}),
];

timeline.addEvent(events[0], false);
timeline.addEvent(events[1], false);
timeline.addEvent(events[0], { toStartOfTimeline: false });
timeline.addEvent(events[1], { toStartOfTimeline: false });

expect(timeline.getState(EventTimeline.FORWARDS).setStateEvents).
toHaveBeenCalledWith([events[0]]);
toHaveBeenCalledWith([events[0]], {
fromInitialState: undefined,
});
expect(timeline.getState(EventTimeline.FORWARDS).setStateEvents).
toHaveBeenCalledWith([events[1]]);
toHaveBeenCalledWith([events[1]], {
fromInitialState: undefined,
});

expect(events[0].forwardLooking).toBe(true);
expect(events[1].forwardLooking).toBe(true);
Expand All @@ -291,13 +299,17 @@ describe("EventTimeline", function() {
}),
];

timeline.addEvent(events[0], true);
timeline.addEvent(events[1], true);
timeline.addEvent(events[0], { toStartOfTimeline: true });
timeline.addEvent(events[1], { toStartOfTimeline: true });

expect(timeline.getState(EventTimeline.BACKWARDS).setStateEvents).
toHaveBeenCalledWith([events[0]]);
toHaveBeenCalledWith([events[0]], {
fromInitialState: undefined,
});
expect(timeline.getState(EventTimeline.BACKWARDS).setStateEvents).
toHaveBeenCalledWith([events[1]]);
toHaveBeenCalledWith([events[1]], {
fromInitialState: undefined,
});

expect(events[0].forwardLooking).toBe(false);
expect(events[1].forwardLooking).toBe(false);
Expand All @@ -324,8 +336,8 @@ describe("EventTimeline", function() {
];

it("should remove events", function() {
timeline.addEvent(events[0], false);
timeline.addEvent(events[1], false);
timeline.addEvent(events[0], { toStartOfTimeline: false });
timeline.addEvent(events[1], { toStartOfTimeline: false });
expect(timeline.getEvents().length).toEqual(2);

let ev = timeline.removeEvent(events[0].getId());
Expand All @@ -338,9 +350,9 @@ describe("EventTimeline", function() {
});

it("should update baseIndex", function() {
timeline.addEvent(events[0], false);
timeline.addEvent(events[1], true);
timeline.addEvent(events[2], false);
timeline.addEvent(events[0], { toStartOfTimeline: false });
timeline.addEvent(events[1], { toStartOfTimeline: true });
timeline.addEvent(events[2], { toStartOfTimeline: false });
expect(timeline.getEvents().length).toEqual(3);
expect(timeline.getBaseIndex()).toEqual(1);

Expand All @@ -358,11 +370,11 @@ describe("EventTimeline", function() {
// further addEvent(ev, false) calls made the index increase.
it("should not make baseIndex assplode when removing the last event",
function() {
timeline.addEvent(events[0], true);
timeline.addEvent(events[0], { toStartOfTimeline: true });
timeline.removeEvent(events[0].getId());
const initialIndex = timeline.getBaseIndex();
timeline.addEvent(events[1], false);
timeline.addEvent(events[2], false);
timeline.addEvent(events[1], { toStartOfTimeline: false });
timeline.addEvent(events[2], { toStartOfTimeline: false });
expect(timeline.getBaseIndex()).toEqual(initialIndex);
expect(timeline.getEvents().length).toEqual(2);
});
Expand Down
Loading