Fix and write tests

Author: robintown

src/components/structures/TimelinePanel.tsx

@@ -1432,13 +1432,16 @@ class TimelinePanel extends React.Component<IProps, IState> {
         if (mainEvents.length > 0) {
             let paginationRequests: Promise<unknown>[];
 
+            // Keep paginating until the main window is covered
             do {
                 paginationRequests = [];
                 const overlayEvents = overlayWindow.getEvents();
 
                 if (
                     overlayWindow.canPaginate(EventTimeline.BACKWARDS) &&
-                    (overlayEvents.length === 0 || overlaysAfter(overlayEvents[0], mainEvents[0]))
+                    (overlayEvents.length === 0 ||
+                        overlaysAfter(overlayEvents[0], mainEvents[0]) ||
+                        !mainWindow.canPaginate(EventTimeline.BACKWARDS))
                 ) {
                     // Paginating backwards could reveal more events to be overlaid in the main window
                     paginationRequests.push(
@@ -1448,7 +1451,9 @@ class TimelinePanel extends React.Component<IProps, IState> {
 
                 if (
                     overlayWindow.canPaginate(EventTimeline.FORWARDS) &&
-                    (overlayEvents.length === 0 || overlaysBefore(overlayEvents.at(-1), mainEvents.at(-1)))
+                    (overlayEvents.length === 0 ||
+                        overlaysBefore(overlayEvents.at(-1), mainEvents.at(-1)) ||
+                        !mainWindow.canPaginate(EventTimeline.FORWARDS))
                 ) {
                     // Paginating forwards could reveal more events to be overlaid in the main window
                     paginationRequests.push(
@@ -1581,19 +1586,18 @@ class TimelinePanel extends React.Component<IProps, IState> {
         // This is a hot-path optimization by skipping a promise tick
         // by repeating a no-op sync branch in
         // TimelineSet.getTimelineForEvent & MatrixClient.getEventTimeline
-        if (this.props.timelineSet.getTimelineForEvent(eventId)) {
+        if (this.props.timelineSet.getTimelineForEvent(eventId) && !this.overlayTimelineWindow) {
             // if we've got an eventId, and the timeline exists, we can skip
             // the promise tick.
             this.timelineWindow.load(eventId, INITIAL_SIZE);
-            this.overlayTimelineWindow?.load(undefined, INITIAL_SIZE);
             // in this branch this method will happen in sync time
             onLoaded();
             return;
         }
 
         const prom = this.timelineWindow.load(eventId, INITIAL_SIZE).then(async (): Promise<void> => {
             if (this.overlayTimelineWindow) {
-                // @TODO(kerrya) use timestampToEvent to load the overlay timeline
+                // TODO: use timestampToEvent to load the overlay timeline
                 // with more correct position when main TL eventId is truthy
                 await this.overlayTimelineWindow.load(undefined, INITIAL_SIZE);
                 await this.extendOverlayWindowToCoverMainWindow();

test/components/structures/TimelinePanel-test.tsx

@@ -37,6 +37,8 @@ import {
     ThreadFilterType,
 } from "matrix-js-sdk/src/models/thread";
 import React, { createRef } from "react";
+import { mocked } from "jest-mock";
+import { forEachRight } from "lodash";
 
 import TimelinePanel from "../../../src/components/structures/TimelinePanel";
 import MatrixClientContext from "../../../src/contexts/MatrixClientContext";
@@ -60,10 +62,10 @@ const newReceipt = (eventId: string, userId: string, readTs: number, fullyReadTs
 const getProps = (room: Room, events: MatrixEvent[]): TimelinePanel["props"] => {
     const timelineSet = { room: room as Room } as EventTimelineSet;
     const timeline = new EventTimeline(timelineSet);
-    events.forEach((event) => timeline.addEvent(event, { toStartOfTimeline: true }));
+    events.forEach((event) => timeline.addEvent(event, { toStartOfTimeline: false }));
     timelineSet.getLiveTimeline = () => timeline;
     timelineSet.getTimelineForEvent = () => timeline;
-    timelineSet.getPendingEvents = () => events;
+    timelineSet.getPendingEvents = () => [];
     timelineSet.room!.getEventReadUpTo = () => events[1].getId() ?? null;
 
     return {
@@ -97,6 +99,12 @@ const setupTestData = (): [MatrixClient, Room, MatrixEvent[]] => {
     return [client, room, events];
 };
 
+const expectEvents = (container: HTMLElement, events: MatrixEvent[]): void => {
+    const eventTiles = container.querySelectorAll(".mx_EventTile");
+    const eventTileIds = [...eventTiles].map((tileElement) => tileElement.getAttribute("data-event-id"));
+    expect(eventTileIds).toEqual(events.map((ev) => ev.getId()));
+};
+
 describe("TimelinePanel", () => {
     beforeEach(() => {
         stubClient();
@@ -279,8 +287,7 @@ describe("TimelinePanel", () => {
     });
 
     describe("with overlayTimeline", () => {
-        // Trying to understand why this is not passing anymore
-        it.skip("renders merged timeline", () => {
+        it("renders merged timeline", () => {
             const [client, room, events] = setupTestData();
             const virtualRoom = mkRoom(client, "virtualRoomId");
             const virtualCallInvite = new MatrixEvent({
@@ -296,25 +303,87 @@ describe("TimelinePanel", () => {
             const virtualEvents = [virtualCallInvite, ...mockEvents(virtualRoom), virtualCallMetaEvent];
             const { timelineSet: overlayTimelineSet } = getProps(virtualRoom, virtualEvents);
 
-            const props = {
-                ...getProps(room, events),
-                overlayTimelineSet,
-                overlayTimelineSetFilter: isCallEvent,
-            };
-
-            const { container } = render(<TimelinePanel {...props} />);
+            const { container } = render(
+                <TimelinePanel
+                    {...getProps(room, events)}
+                    overlayTimelineSet={overlayTimelineSet}
+                    overlayTimelineSetFilter={isCallEvent}
+                />,
+            );
 
-            const eventTiles = container.querySelectorAll(".mx_EventTile");
-            const eventTileIds = [...eventTiles].map((tileElement) => tileElement.getAttribute("data-event-id"));
-            expect(eventTileIds).toEqual([
+            expectEvents(container, [
                 // main timeline events are included
-                events[1].getId(),
-                events[0].getId(),
+                events[0],
+                events[1],
                 // virtual timeline call event is included
-                virtualCallInvite.getId(),
+                virtualCallInvite,
                 // virtual call event has no tile renderer => not rendered
             ]);
         });
+
+        it("paginates to get enough overlay events", async () => {
+            const [client, room, events] = setupTestData();
+            const virtualRoom = mkRoom(client, "virtualRoomId");
+            const overlayEvents = mockEvents(virtualRoom, 5);
+            const overlayEventsPage1 = overlayEvents.slice(0, 2);
+            const overlayEventsPage2 = overlayEvents.slice(2, 3);
+            const overlayEventsPage3 = overlayEvents.slice(3, 5);
+
+            // Set the event order that we'll be looking for in the timeline
+            overlayEventsPage1[0].localTimestamp = 1000;
+            events[0].localTimestamp = 2000;
+            overlayEventsPage1[1].localTimestamp = 3000;
+            overlayEventsPage2[0].localTimestamp = 4000;
+            overlayEventsPage3[0].localTimestamp = 5000;
+            events[1].localTimestamp = 6000;
+            overlayEventsPage3[1].localTimestamp = 7000;
+
+            const overlayTimelineSet = {
+                room: room as Room,
+                getLiveTimeline: () => overlayTimeline,
+                getTimelineForEvent: () => overlayTimeline,
+                getPendingEvents: () => [],
+            } as EventTimelineSet;
+            const overlayTimeline = new EventTimeline(overlayTimelineSet);
+            // Start with only page 2 of the overlay events in the window
+            overlayEventsPage2.forEach((event) => overlayTimeline.addEvent(event, { toStartOfTimeline: false }));
+            // Enable the overlay timeline to be paginated
+            overlayTimeline.setPaginationToken("page2-start", EventTimeline.BACKWARDS);
+            overlayTimeline.setPaginationToken("page2-end", EventTimeline.FORWARDS);
+
+            mocked(client).paginateEventTimeline.mockImplementation(async (timeline, { backwards }) => {
+                if (timeline === overlayTimeline) {
+                    if (backwards) {
+                        forEachRight(overlayEventsPage1, (event) =>
+                            timeline.addEvent(event, { toStartOfTimeline: true }),
+                        );
+                    } else {
+                        overlayEventsPage3.forEach((event) => timeline.addEvent(event, { toStartOfTimeline: false }));
+                    }
+                    // Prevent any further pagination attempts in this direction
+                    timeline.setPaginationToken(null, backwards ? EventTimeline.BACKWARDS : EventTimeline.FORWARDS);
+                    return true;
+                } else {
+                    return false;
+                }
+            });
+
+            const { container } = render(
+                <TimelinePanel {...getProps(room, events)} overlayTimelineSet={overlayTimelineSet} />,
+            );
+
+            await waitFor(() =>
+                expectEvents(container, [
+                    overlayEventsPage1[0],
+                    events[0],
+                    overlayEventsPage1[1],
+                    overlayEventsPage2[0],
+                    overlayEventsPage3[0],
+                    events[1],
+                    overlayEventsPage3[1],
+                ]),
+            );
+        });
     });
 
     describe("when a thread updates", () => {

test/test-utils/test-utils.ts

@@ -537,7 +537,7 @@ export function mkStubRoom(
             on: jest.fn(),
             off: jest.fn(),
         } as unknown as RoomState,
-        eventShouldLiveIn: jest.fn().mockReturnValue({}),
+        eventShouldLiveIn: jest.fn().mockReturnValue({ shouldLiveInRoom: true, shouldLiveInThread: false }),
         fetchRoomThreads: jest.fn().mockReturnValue(Promise.resolve()),
         findEventById: jest.fn().mockReturnValue(undefined),
         findPredecessor: jest.fn().mockReturnValue({ roomId: "", eventId: null }),