Send correct receipts when viewing a room (#10864)

* Send correct receipts when viewing a room

* Fix strict type issues

* Handle promises

* Handle more primises

* Add generic array type

* Replace existende check with type predicate

* Fix wrong variable check

* Improve comment about initial read marker

* Use read_markers API for fully read receipts

* Log public receipt fallback

* Rename variables in new code to be aligned to the spec

* Add end-2-end test for read markers and receipts

cypress/e2e/read-receipts/read-receipts.spec.ts

@@ -32,8 +32,8 @@ describe("Read receipts", () => {
     let selectedRoomId: string;
     let bot: MatrixClient | undefined;
 
-    const botSendMessage = (): Cypress.Chainable<ISendEventResponse> => {
-        return cy.botSendMessage(bot, otherRoomId, "Message");
+    const botSendMessage = (no = 1): Cypress.Chainable<ISendEventResponse> => {
+        return cy.botSendMessage(bot, otherRoomId, `Message ${no}`);
     };
 
     const botSendThreadMessage = (threadId: string): Cypress.Chainable<ISendEventResponse> => {
@@ -268,4 +268,87 @@ describe("Read receipts", () => {
             });
         });
     });
+
+    /**
+     * The idea of this test is to intercept the receipt / read read_markers requests and
+     * assert that the correct ones are sent.
+     * Prose playbook:
+     * - Another user sends enough messages that the timeline becomes scrollable
+     * - The current user looks at the room and jumps directly to the first unread message
+     * - At this point, a receipt for the last message in the room and
+     *   a fully read marker for the last visible message are expected to be sent
+     * - Then the user jumps to the end of the timeline
+     * - A fully read marker for the last message in the room is expected to be sent
+     */
+    it("Should send the correct receipts", () => {
+        const uriEncodedOtherRoomId = encodeURIComponent(otherRoomId);
+
+        cy.intercept({
+            method: "POST",
+            url: new RegExp(
+                `http://localhost:\\d+/_matrix/client/r0/rooms/${uriEncodedOtherRoomId}/receipt/m\\.read/.+`,
+            ),
+        }).as("receiptRequest");
+
+        const numberOfMessages = 20;
+        const sendMessagePromises = [];
+
+        for (let i = 1; i <= numberOfMessages; i++) {
+            sendMessagePromises.push(botSendMessage(i));
+        }
+
+        cy.all(sendMessagePromises).then((sendMessageResponses) => {
+            const lastMessageId = sendMessageResponses.at(-1).event_id;
+            const uriEncodedLastMessageId = encodeURIComponent(lastMessageId);
+
+            // wait until all messages have been received
+            cy.findByLabelText(`${otherRoomName} ${sendMessagePromises.length} unread messages.`).should("exist");
+
+            // switch to the room with the messages
+            cy.visit("/#/room/" + otherRoomId);
+
+            cy.wait("@receiptRequest").should((req) => {
+                // assert the read receipt for the last message in the room
+                expect(req.request.url).to.contain(uriEncodedLastMessageId);
+                expect(req.request.body).to.deep.equal({
+                    thread_id: "main",
+                });
+            });
+
+            // the following code tests the fully read marker somewhere in the middle of the room
+
+            cy.intercept({
+                method: "POST",
+                url: new RegExp(`http://localhost:\\d+/_matrix/client/r0/rooms/${uriEncodedOtherRoomId}/read_markers`),
+            }).as("readMarkersRequest");
+
+            cy.findByRole("button", { name: "Jump to first unread message." }).click();
+
+            cy.wait("@readMarkersRequest").should((req) => {
+                // since this is not pixel perfect,
+                // the fully read marker should be +/- 1 around the last visible message
+                expect(Array.from(Object.keys(req.request.body))).to.deep.equal(["m.fully_read"]);
+                expect(req.request.body["m.fully_read"]).to.be.oneOf([
+                    sendMessageResponses[11].event_id,
+                    sendMessageResponses[12].event_id,
+                    sendMessageResponses[13].event_id,
+                ]);
+            });
+
+            // the following code tests the fully read marker at the bottom of the room
+
+            cy.intercept({
+                method: "POST",
+                url: new RegExp(`http://localhost:\\d+/_matrix/client/r0/rooms/${uriEncodedOtherRoomId}/read_markers`),
+            }).as("readMarkersRequest");
+
+            cy.findByRole("button", { name: "Scroll to most recent messages" }).click();
+
+            cy.wait("@readMarkersRequest").should((req) => {
+                expect(req.request.body).to.deep.equal({
+                    ["m.fully_read"]: sendMessageResponses.at(-1).event_id,
+                });
+            });
+        });
+    });
 });

src/components/structures/TimelinePanel.tsx

@@ -16,7 +16,7 @@ limitations under the License.
 
 import React, { createRef, ReactNode } from "react";
 import ReactDOM from "react-dom";
-import { NotificationCountType, Room, RoomEvent } from "matrix-js-sdk/src/models/room";
+import { Room, RoomEvent } from "matrix-js-sdk/src/models/room";
 import { MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/models/event";
 import { EventTimelineSet, IRoomTimelineData } from "matrix-js-sdk/src/models/event-timeline-set";
 import { Direction, EventTimeline } from "matrix-js-sdk/src/models/event-timeline";
@@ -26,7 +26,7 @@ import { SyncState } from "matrix-js-sdk/src/sync";
 import { RoomMember, RoomMemberEvent } from "matrix-js-sdk/src/models/room-member";
 import { debounce, findLastIndex, throttle } from "lodash";
 import { logger } from "matrix-js-sdk/src/logger";
-import { ClientEvent } from "matrix-js-sdk/src/client";
+import { ClientEvent, MatrixClient } from "matrix-js-sdk/src/client";
 import { Thread, ThreadEvent } from "matrix-js-sdk/src/models/thread";
 import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts";
 import { MatrixError } from "matrix-js-sdk/src/http-api";
@@ -261,6 +261,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
 
     // A map of <callId, LegacyCallEventGrouper>
     private callEventGroupers = new Map<string, LegacyCallEventGrouper>();
+    private initialReadMarkerId: string | null = null;
 
     public constructor(props: IProps, context: React.ContextType<typeof RoomContext>) {
         super(props, context);
@@ -270,13 +271,12 @@ class TimelinePanel extends React.Component<IProps, IState> {
 
         // XXX: we could track RM per TimelineSet rather than per Room.
         // but for now we just do it per room for simplicity.
-        let initialReadMarker: string | null = null;
         if (this.props.manageReadMarkers) {
             const readmarker = this.props.timelineSet.room?.getAccountData("m.fully_read");
             if (readmarker) {
-                initialReadMarker = readmarker.getContent().event_id;
+                this.initialReadMarkerId = readmarker.getContent().event_id;
             } else {
-                initialReadMarker = this.getCurrentReadReceipt();
+                this.initialReadMarkerId = this.getCurrentReadReceipt();
             }
         }
 
@@ -288,7 +288,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
             canBackPaginate: false,
             canForwardPaginate: false,
             readMarkerVisible: true,
-            readMarkerEventId: initialReadMarker,
+            readMarkerEventId: this.initialReadMarkerId,
             backPaginating: false,
             forwardPaginating: false,
             clientSyncState: MatrixClientPeg.get().getSyncState(),
@@ -1000,7 +1000,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
                 continue; /* aborted */
             }
             // outside of try/catch to not swallow errors
-            this.updateReadMarker();
+            await this.updateReadMarker();
         }
     }
 
@@ -1015,26 +1015,74 @@ class TimelinePanel extends React.Component<IProps, IState> {
                 continue; /* aborted */
             }
             // outside of try/catch to not swallow errors
-            this.sendReadReceipt();
+            await this.sendReadReceipts();
         }
     }
 
-    private sendReadReceipt = (): void => {
-        if (SettingsStore.getValue("lowBandwidth")) return;
+    /**
+     * Whether to send public or private receipts.
+     */
+    private async determineReceiptType(client: MatrixClient): Promise<ReceiptType> {
+        const roomId = this.props.timelineSet.room?.roomId ?? null;
+        const shouldSendPublicReadReceipts = SettingsStore.getValue("sendReadReceipts", roomId);
 
-        if (!this.messagePanel.current) return;
-        if (!this.props.manageReadReceipts) return;
-        // This happens on user_activity_end which is delayed, and it's
-        // very possible have logged out within that timeframe, so check
-        // we still have a client.
-        const cli = MatrixClientPeg.get();
-        // if no client or client is guest don't send RR or RM
-        if (!cli || cli.isGuest()) return;
+        if (shouldSendPublicReadReceipts) {
+            return ReceiptType.Read;
+        }
 
-        let shouldSendRR = true;
+        if (
+            !(await client.doesServerSupportUnstableFeature("org.matrix.msc2285.stable")) ||
+            !(await client.isVersionSupported("v1.4"))
+        ) {
+            logger.warn(
+                "Falling back to public instead of private receipts because the homeserver does not support them",
+            );
 
-        const currentRREventId = this.getCurrentReadReceipt(true);
-        const currentRREventIndex = this.indexForEventId(currentRREventId);
+            // The server does not support private read receipt. Fall back to public ones.
+            return ReceiptType.Read;
+        }
+
+        return ReceiptType.ReadPrivate;
+    }
+
+    /**
+     * Whether a fully_read marker should be send.
+     */
+    private shouldSendFullyReadMarker(fullyReadMarkerEventId: string | null): fullyReadMarkerEventId is string {
+        if (!this.state.readMarkerEventId) {
+            // Nothing that can be send.
+            return false;
+        }
+
+        if (this.lastRMSentEventId && this.lastRMSentEventId === this.state.readMarkerEventId) {
+            // Prevent sending the same receipt twice.
+            return false;
+        }
+
+        if (this.state.readMarkerEventId && this.state.readMarkerEventId === this.initialReadMarkerId) {
+            // The initial read marker is the one stored in the room account data.
+            // It makes no sense to send a read marker for it,
+            // because if it is in the room account data, a read marker must have been sent before.
+            return false;
+        }
+
+        if (this.props.timelineSet.thread) {
+            // Read marker for threads are not supported per spec.
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * Whether a read receipt should be send.
+     */
+    private shouldSendReadReceipt(
+        currentReadReceiptEventId: string | null,
+        currentReadReceiptEventIndex: number | null,
+        lastReadEvent: MatrixEvent | null,
+        lastReadEventIndex: number | null,
+    ): boolean {
         // We want to avoid sending out read receipts when we are looking at
         // events in the past which are before the latest RR.
         //
@@ -1047,110 +1095,133 @@ class TimelinePanel extends React.Component<IProps, IState> {
         // timeline which is *after* the latest RR (so we should actually send
         // RRs) - but that is a bit of a niche case. It will sort itself out when
         // the user eventually hits the live timeline.
-        //
+
         if (
-            currentRREventId &&
-            currentRREventIndex === null &&
+            currentReadReceiptEventId &&
+            currentReadReceiptEventIndex === null &&
             this.timelineWindow?.canPaginate(EventTimeline.FORWARDS)
         ) {
-            shouldSendRR = false;
+            return false;
         }
+        // Only send a RR if the last read event is ahead in the timeline relative to the current RR event.
+        // Only send a RR if the last RR set != the one we would send
+        return (
+            (lastReadEventIndex === null ||
+                currentReadReceiptEventIndex === null ||
+                lastReadEventIndex > currentReadReceiptEventIndex) &&
+            (!this.lastRRSentEventId || this.lastRRSentEventId !== lastReadEvent?.getId())
+        );
+    }
 
+    private sendReadReceipts = async (): Promise<void> => {
+        if (SettingsStore.getValue("lowBandwidth")) return;
+        if (!this.messagePanel.current) return;
+        if (!this.props.manageReadReceipts) return;
+
+        // This happens on user_activity_end which is delayed, and it's
+        // very possible have logged out within that timeframe, so check
+        // we still have a client.
+        const client = MatrixClientPeg.get();
+        // if no client or client is guest don't send RR or RM
+        if (!client || client.isGuest()) return;
+
+        // "current" here means the receipts that have already been sent
+        const currentReadReceiptEventId = this.getCurrentReadReceipt(true);
+        const currentReadReceiptEventIndex = this.indexForEventId(currentReadReceiptEventId);
+
+        // "last" here means the last displayed event
         const lastReadEventIndex = this.getLastDisplayedEventIndex({
             ignoreOwn: true,
         });
-        if (lastReadEventIndex === null) {
-            shouldSendRR = false;
+        const lastReadEvent: MatrixEvent | null = this.state.events[lastReadEventIndex ?? 0];
+
+        const shouldSendReadReceipt = this.shouldSendReadReceipt(
+            currentReadReceiptEventId,
+            currentReadReceiptEventIndex,
+            lastReadEvent,
+            lastReadEventIndex,
+        );
+        const fullyReadMarkerEventId = this.state.readMarkerEventId;
+        const shouldSendFullyReadMarker = this.shouldSendFullyReadMarker(fullyReadMarkerEventId);
+        const roomId = this.props.timelineSet.room?.roomId;
+
+        debuglog(`Sending Read Markers for ${roomId}: `, {
+            shouldSendReadReceipt,
+            shouldSendFullyReadMarker,
+            currentReadReceiptEventId,
+            currentReadReceiptEventIndex,
+            lastReadEventId: lastReadEvent?.getId(),
+            lastReadEventIndex,
+            readMarkerEventId: this.state.readMarkerEventId,
+        });
+
+        const proms: Array<Promise<void>> = [];
+
+        if (shouldSendReadReceipt) {
+            proms.push(this.sendReadReceipt(client, lastReadEvent));
         }
-        let lastReadEvent: MatrixEvent | null = this.state.events[lastReadEventIndex ?? 0];
-        shouldSendRR =
-            shouldSendRR &&
-            // Only send a RR if the last read event is ahead in the timeline relative to
-            // the current RR event.
-            lastReadEventIndex! > currentRREventIndex! &&
-            // Only send a RR if the last RR set != the one we would send
-            this.lastRRSentEventId !== lastReadEvent?.getId();
 
-        // Only send a RM if the last RM sent != the one we would send
-        const shouldSendRM = this.lastRMSentEventId != this.state.readMarkerEventId;
+        if (shouldSendFullyReadMarker) {
+            const readMarkerEvent = this.props.timelineSet.findEventById(fullyReadMarkerEventId);
 
-        // we also remember the last read receipt we sent to avoid spamming the
-        // same one at the server repeatedly
-        if (shouldSendRR || shouldSendRM) {
-            if (shouldSendRR) {
-                this.lastRRSentEventId = lastReadEvent?.getId();
-            } else {
-                lastReadEvent = null;
-            }
-            this.lastRMSentEventId = this.state.readMarkerEventId;
-
-            const roomId = this.props.timelineSet.room.roomId;
-            const sendRRs = SettingsStore.getValue("sendReadReceipts", roomId);
-
-            debuglog(
-                `Sending Read Markers for ${roomId}: `,
-                `rm=${this.state.readMarkerEventId} `,
-                `rr=${sendRRs ? lastReadEvent?.getId() : null} `,
-                `prr=${lastReadEvent?.getId()}`,
-            );
-
-            if (this.props.timelineSet.thread && sendRRs && lastReadEvent) {
-                // There's no support for fully read markers on threads
-                // as defined by MSC3771
-                cli.sendReadReceipt(lastReadEvent, sendRRs ? ReceiptType.Read : ReceiptType.ReadPrivate);
-            } else {
-                cli.setRoomReadMarkers(
-                    roomId,
-                    this.state.readMarkerEventId ?? "",
-                    sendRRs ? lastReadEvent ?? undefined : undefined, // Public read receipt (could be null)
-                    lastReadEvent ?? undefined, // Private read receipt (could be null)
-                ).catch(async (e): Promise<void> => {
-                    // /read_markers API is not implemented on this HS, fallback to just RR
-                    if (e.errcode === "M_UNRECOGNIZED" && lastReadEvent) {
-                        if (
-                            !sendRRs &&
-                            !(await cli.doesServerSupportUnstableFeature("org.matrix.msc2285.stable")) &&
-                            !(await cli.isVersionSupported("v1.4"))
-                        )
-                            return;
-                        try {
-                            await cli.sendReadReceipt(
-                                lastReadEvent,
-                                sendRRs ? ReceiptType.Read : ReceiptType.ReadPrivate,
-                            );
-                            return;
-                        } catch (error) {
-                            logger.error(e);
-                            this.lastRRSentEventId = undefined;
-                        }
-                    } else {
-                        logger.error(e);
-                    }
-                    // it failed, so allow retries next time the user is active
-                    this.lastRRSentEventId = undefined;
-                    this.lastRMSentEventId = undefined;
-                });
-
-                // do a quick-reset of our unreadNotificationCount to avoid having
-                // to wait from the remote echo from the homeserver.
-                // we only do this if we're right at the end, because we're just assuming
-                // that sending an RR for the latest message will set our notif counter
-                // to zero: it may not do this if we send an RR for somewhere before the end.
-                if (this.isAtEndOfLiveTimeline() && this.props.timelineSet.room) {
-                    this.props.timelineSet.room.setUnreadNotificationCount(NotificationCountType.Total, 0);
-                    this.props.timelineSet.room.setUnreadNotificationCount(NotificationCountType.Highlight, 0);
-                    dis.dispatch({
-                        action: "on_room_read",
-                        roomId: this.props.timelineSet.room.roomId,
-                    });
-                }
+            if (readMarkerEvent) {
+                // Empty room Id should not happen here.
+                // Either way fall back to empty string and let further functions handle it.
+                proms.push(this.sendFullyReadMarker(client, roomId ?? "", fullyReadMarkerEventId));
             }
         }
+
+        await Promise.all(proms);
     };
 
+    /**
+     * Sends a read receipt for event.
+     * Resets the last sent event Id in case of an error, so that it will be retried next time.
+     */
+    private async sendReadReceipt(client: MatrixClient, event: MatrixEvent): Promise<void> {
+        this.lastRRSentEventId = event.getId();
+        const receiptType = await this.determineReceiptType(client);
+
+        try {
+            await client.sendReadReceipt(event, receiptType);
+        } catch (err) {
+            // it failed, so allow retries next time the user is active
+            this.lastRRSentEventId = undefined;
+
+            logger.error("Error sending receipt", {
+                room: this.props.timelineSet.room?.roomId,
+                error: err,
+            });
+        }
+    }
+
+    /**
+     * Sends a fully_read marker for readMarkerEvent.
+     * Resets the last sent event Id in case of an error, so that it will be retried next time.
+     */
+    private async sendFullyReadMarker(
+        client: MatrixClient,
+        roomId: string,
+        fullyReadMarkerEventId: string,
+    ): Promise<void> {
+        this.lastRMSentEventId = this.state.readMarkerEventId;
+
+        try {
+            await client.setRoomReadMarkers(roomId, fullyReadMarkerEventId);
+        } catch (error) {
+            // it failed, so allow retries next time the user is active
+            this.lastRMSentEventId = undefined;
+
+            logger.error("Error sending fully_read", {
+                roomId,
+                error,
+            });
+        }
+    }
+
     // if the read marker is on the screen, we can now assume we've caught up to the end
     // of the screen, so move the marker down to the bottom of the screen.
-    private updateReadMarker = (): void => {
+    private updateReadMarker = async (): Promise<void> => {
         if (!this.props.manageReadMarkers) return;
         if (this.getReadMarkerPosition() === 1) {
             // the read marker is at an event below the viewport,
@@ -1179,7 +1250,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
         }
 
         // Send the updated read marker (along with read receipt) to the server
-        this.sendReadReceipt();
+        await this.sendReadReceipts();
     };
 
     // advance the read marker past any events we sent ourselves.
@@ -1264,9 +1335,10 @@ class TimelinePanel extends React.Component<IProps, IState> {
         this.loadTimeline(this.state.readMarkerEventId, 0, 1 / 3);
     };
 
-    /* update the read-up-to marker to match the read receipt
+    /**
+     * update the read-up-to marker to match the read receipt
      */
-    public forgetReadMarker = (): void => {
+    public forgetReadMarker = async (): Promise<void> => {
         if (!this.props.manageReadMarkers) return;
 
         // Find the read receipt - we will set the read marker to this
@@ -1288,7 +1360,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
         this.setReadMarker(rmId, rmTs);
 
         // Send the receipts to the server immediately (don't wait for activity)
-        this.sendReadReceipt();
+        await this.sendReadReceipts();
     };
 
     /* return true if the content is fully scrolled down and we are
@@ -1529,7 +1601,9 @@ class TimelinePanel extends React.Component<IProps, IState> {
                     }
 
                     if (this.props.sendReadReceiptOnLoad) {
-                        this.sendReadReceipt();
+                        this.sendReadReceipts().catch((err) => {
+                            logger.warn("Error sending receipts on load", err);
+                        });
                     }
                 },
             );

test/components/structures/TimelinePanel-test.tsx

@@ -22,6 +22,7 @@ import {
     MatrixClient,
     MatrixEvent,
     PendingEventOrdering,
+    RelationType,
     Room,
     RoomEvent,
     RoomMember,
@@ -37,16 +38,17 @@ import {
     ThreadFilterType,
 } from "matrix-js-sdk/src/models/thread";
 import React, { createRef } from "react";
-import { mocked } from "jest-mock";
+import { Mocked, mocked } from "jest-mock";
 import { forEachRight } from "lodash";
 
 import TimelinePanel from "../../../src/components/structures/TimelinePanel";
 import MatrixClientContext from "../../../src/contexts/MatrixClientContext";
 import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
 import { isCallEvent } from "../../../src/components/structures/LegacyCallEventGrouper";
-import { flushPromises, mkMembership, mkRoom, stubClient } from "../../test-utils";
+import { filterConsole, flushPromises, mkMembership, mkRoom, stubClient } from "../../test-utils";
 import { mkThread } from "../../test-utils/threads";
 import { createMessageEventContent } from "../../test-utils/events";
+import SettingsStore from "../../../src/settings/SettingsStore";
 import ScrollPanel from "../../../src/components/structures/ScrollPanel";
 
 // ScrollPanel calls this, but jsdom doesn't mock it for us
@@ -168,68 +170,197 @@ const setupPagination = (
 };
 
 describe("TimelinePanel", () => {
+    let client: Mocked<MatrixClient>;
+    let userId: string;
+
+    filterConsole("checkForPreJoinUISI: showing all messages, skipping check");
+
     beforeEach(() => {
-        stubClient();
+        client = mocked(stubClient());
+        userId = client.getSafeUserId();
     });
 
     describe("read receipts and markers", () => {
-        it("should forget the read marker when asked to", () => {
-            const cli = MatrixClientPeg.get();
-            const readMarkersSent: string[] = [];
+        const roomId = "#room:example.com";
+        let room: Room;
+        let timelineSet: EventTimelineSet;
+        let timelinePanel: TimelinePanel;
 
-            // Track calls to setRoomReadMarkers
-            cli.setRoomReadMarkers = (_roomId, rmEventId, _a, _b) => {
-                readMarkersSent.push(rmEventId);
-                return Promise.resolve({});
-            };
+        const ev1 = new MatrixEvent({
+            event_id: "ev1",
+            sender: "@u2:m.org",
+            origin_server_ts: 111,
+            type: EventType.RoomMessage,
+            content: createMessageEventContent("hello 1"),
+        });
 
-            const ev0 = new MatrixEvent({
-                event_id: "ev0",
-                sender: "@u2:m.org",
-                origin_server_ts: 111,
-                type: EventType.RoomMessage,
-                content: createMessageEventContent("hello 1"),
-            });
-            const ev1 = new MatrixEvent({
-                event_id: "ev1",
-                sender: "@u2:m.org",
-                origin_server_ts: 222,
-                type: EventType.RoomMessage,
-                content: createMessageEventContent("hello 2"),
-            });
+        const ev2 = new MatrixEvent({
+            event_id: "ev2",
+            sender: "@u2:m.org",
+            origin_server_ts: 222,
+            type: EventType.RoomMessage,
+            content: createMessageEventContent("hello 2"),
+        });
 
-            const roomId = "#room:example.com";
-            const userId = cli.credentials.userId!;
-            const room = new Room(roomId, cli, userId, { pendingEventOrdering: PendingEventOrdering.Detached });
-
-            // Create a TimelinePanel with ev0 already present
-            const timelineSet = new EventTimelineSet(room, {});
-            timelineSet.addLiveEvent(ev0);
+        const renderTimelinePanel = async (): Promise<void> => {
             const ref = createRef<TimelinePanel>();
             render(
                 <TimelinePanel
                     timelineSet={timelineSet}
                     manageReadMarkers={true}
                     manageReadReceipts={true}
-                    eventId={ev0.getId()}
                     ref={ref}
                 />,
             );
-            const timelinePanel = ref.current!;
+            await flushPromises();
+            timelinePanel = ref.current!;
+        };
 
-            // An event arrived, and we read it
-            timelineSet.addLiveEvent(ev1);
-            room.addEphemeralEvents([newReceipt("ev1", userId, 222, 220)]);
+        const setUpTimelineSet = (threadRoot?: MatrixEvent) => {
+            let thread: Thread | undefined = undefined;
 
-            // Sanity: We have not sent any read marker yet
-            expect(readMarkersSent).toEqual([]);
+            if (threadRoot) {
+                thread = new Thread(threadRoot.getId()!, threadRoot, {
+                    client: client,
+                    room,
+                });
+            }
 
-            // This is what we are testing: forget the read marker - this should
-            // update our read marker to match the latest receipt we sent
-            timelinePanel.forgetReadMarker();
+            timelineSet = new EventTimelineSet(room, {}, client, thread);
+            timelineSet.on(RoomEvent.Timeline, (...args) => {
+                // TimelinePanel listens for live events on the client.
+                // → Re-emit on the client.
+                client.emit(RoomEvent.Timeline, ...args);
+            });
+        };
 
-            // We sent off a read marker for the new event
-            expect(readMarkersSent).toEqual(["ev1"]);
+        beforeEach(() => {
+            room = new Room(roomId, client, userId, { pendingEventOrdering: PendingEventOrdering.Detached });
+        });
+
+        afterEach(() => {
+            TimelinePanel.roomReadMarkerTsMap = {};
+        });
+
+        describe("when there is a non-threaded timeline", () => {
+            beforeEach(() => {
+                setUpTimelineSet();
+            });
+
+            describe("and reading the timeline", () => {
+                beforeEach(async () => {
+                    await renderTimelinePanel();
+                    timelineSet.addLiveEvent(ev1, {});
+                    await flushPromises();
+
+                    // @ts-ignore
+                    await timelinePanel.sendReadReceipts();
+                    // @ts-ignore Simulate user activity by calling updateReadMarker on the TimelinePanel.
+                    await timelinePanel.updateReadMarker();
+                });
+
+                it("should send a fully read marker and a public receipt", async () => {
+                    expect(client.setRoomReadMarkers).toHaveBeenCalledWith(roomId, ev1.getId());
+                    expect(client.sendReadReceipt).toHaveBeenCalledWith(ev1, ReceiptType.Read);
+                });
+
+                describe("and reading the timeline again", () => {
+                    beforeEach(async () => {
+                        client.sendReadReceipt.mockClear();
+                        client.setRoomReadMarkers.mockClear();
+
+                        // @ts-ignore Simulate user activity by calling updateReadMarker on the TimelinePanel.
+                        await timelinePanel.updateReadMarker();
+                    });
+
+                    it("should not send receipts again", () => {
+                        expect(client.sendReadReceipt).not.toHaveBeenCalled();
+                        expect(client.setRoomReadMarkers).not.toHaveBeenCalled();
+                    });
+
+                    it("and forgetting the read markers, should send the stored marker again", async () => {
+                        timelineSet.addLiveEvent(ev2, {});
+                        room.addEphemeralEvents([newReceipt(ev2.getId()!, userId, 222, 200)]);
+                        await timelinePanel.forgetReadMarker();
+                        expect(client.setRoomReadMarkers).toHaveBeenCalledWith(roomId, ev2.getId());
+                    });
+                });
+            });
+
+            describe("and sending receipts is disabled", () => {
+                beforeEach(async () => {
+                    client.isVersionSupported.mockResolvedValue(true);
+                    client.doesServerSupportUnstableFeature.mockResolvedValue(true);
+
+                    jest.spyOn(SettingsStore, "getValue").mockImplementation((setting: string) => {
+                        if (setting === "sendReadReceipt") return false;
+
+                        return undefined;
+                    });
+                });
+
+                afterEach(() => {
+                    mocked(SettingsStore.getValue).mockReset();
+                });
+
+                it("should send a fully read marker and a private receipt", async () => {
+                    await renderTimelinePanel();
+                    timelineSet.addLiveEvent(ev1, {});
+                    await flushPromises();
+
+                    // @ts-ignore
+                    await timelinePanel.sendReadReceipts();
+
+                    // Expect the private reception to be sent directly
+                    expect(client.sendReadReceipt).toHaveBeenCalledWith(ev1, ReceiptType.ReadPrivate);
+                    // Expect the fully_read marker not to be send yet
+                    expect(client.setRoomReadMarkers).not.toHaveBeenCalled();
+
+                    client.sendReadReceipt.mockClear();
+
+                    // @ts-ignore simulate user activity
+                    await timelinePanel.updateReadMarker();
+
+                    // It should not send the receipt again.
+                    expect(client.sendReadReceipt).not.toHaveBeenCalledWith(ev1, ReceiptType.ReadPrivate);
+                    // Expect the fully_read marker to be sent after user activity.
+                    expect(client.setRoomReadMarkers).toHaveBeenCalledWith(roomId, ev1.getId());
+                });
+            });
+        });
+
+        describe("and there is a thread timeline", () => {
+            const threadEv1 = new MatrixEvent({
+                event_id: "thread_ev1",
+                sender: "@u2:m.org",
+                origin_server_ts: 222,
+                type: EventType.RoomMessage,
+                content: {
+                    ...createMessageEventContent("hello 2"),
+                    "m.relates_to": {
+                        event_id: ev1.getId(),
+                        rel_type: RelationType.Thread,
+                    },
+                },
+            });
+
+            beforeEach(() => {
+                client.supportsThreads.mockReturnValue(true);
+                setUpTimelineSet(ev1);
+            });
+
+            it("should send receipts but no fully_read when reading the thread timeline", async () => {
+                await renderTimelinePanel();
+                timelineSet.addLiveEvent(threadEv1, {});
+                await flushPromises();
+
+                // @ts-ignore
+                await timelinePanel.sendReadReceipts();
+
+                // fully_read is not supported for threads per spec
+                expect(client.setRoomReadMarkers).not.toHaveBeenCalled();
+                expect(client.sendReadReceipt).toHaveBeenCalledWith(threadEv1, ReceiptType.Read);
+            });
         });
     });
 

test/test-utils/test-utils.ts

@@ -175,7 +175,7 @@ export function createTestClient(): MatrixClient {
         decryptEventIfNeeded: () => Promise.resolve(),
         isUserIgnored: jest.fn().mockReturnValue(false),
         getCapabilities: jest.fn().mockResolvedValue({}),
-        supportsThreads: () => false,
+        supportsThreads: jest.fn().mockReturnValue(false),
         supportsIntentionalMentions: () => false,
         getRoomUpgradeHistory: jest.fn().mockReturnValue([]),
         getOpenIdToken: jest.fn().mockResolvedValue(undefined),