Fall back to receipt timestamp if we have no event (react-sdk part)

Author: andybalaam

src/Unread.ts

@@ -72,13 +72,18 @@ export function doesRoomHaveUnreadMessages(room: Room): boolean {
 }
 
 export function doesRoomOrThreadHaveUnreadMessages(roomOrThread: Room | Thread): boolean {
+    // NOTE: this shares logic with hasUserReadEvent in
+    // matrix-js-sdk/src/models/read-receipt.ts. They are not combined (yet)
+    // because hasUserReadEvent is focussed on a single event, and this is
+    // focussed on the whole room/thread.
+
     // If there are no messages yet in the timeline then it isn't fully initialised
     // and cannot be unread.
     if (!roomOrThread || roomOrThread.timeline.length === 0) {
         return false;
     }
 
-    const myUserId = MatrixClientPeg.get().getUserId();
+    const myUserId = MatrixClientPeg.get().getUserId()!;
 
     // as we don't send RRs for our own messages, make sure we special case that
     // if *we* sent the last message into the room, we consider it not unread!
@@ -90,34 +95,26 @@ export function doesRoomOrThreadHaveUnreadMessages(roomOrThread: Room | Thread):
         return false;
     }
 
-    // get the most recent read receipt sent by our account.
-    // N.B. this is NOT a read marker (RM, aka "read up to marker"),
-    // despite the name of the method :((
-    const readUpToId = roomOrThread.getEventReadUpTo(myUserId!);
-
-    // this just looks at whatever history we have, which if we've only just started
-    // up probably won't be very much, so if the last couple of events are ones that
-    // don't count, we don't know if there are any events that do count between where
-    // we have and the read receipt. We could fetch more history to try & find out,
-    // but currently we just guess.
+    const readUpToId = roomOrThread.getEventReadUpTo(myUserId);
+    const hasReceipt = makeHasReceipt(roomOrThread, readUpToId, myUserId);
 
     // Loop through messages, starting with the most recent...
     for (let i = roomOrThread.timeline.length - 1; i >= 0; --i) {
         const ev = roomOrThread.timeline[i];
 
-        if (ev.getId() == readUpToId) {
+        if (hasReceipt(ev)) {
             // If we've read up to this event, there's nothing more recent
             // that counts and we can stop looking because the user's read
             // this and everything before.
             return false;
-        } else if (!shouldHideEvent(ev) && eventTriggersUnreadCount(ev)) {
+        } else if (isImportantEvent(ev)) {
             // We've found a message that counts before we hit
             // the user's read receipt, so this room is definitely unread.
             return true;
         }
     }
 
-    // If we got here, we didn't find a message that counted but didn't find
+    // If we got here, we didn't find a message was important but didn't find
     // the user's read receipt either, so we guess and say that the room is
     // unread on the theory that false positives are better than false
     // negatives here.
@@ -127,3 +124,49 @@ export function doesRoomOrThreadHaveUnreadMessages(roomOrThread: Room | Thread):
     });
     return true;
 }
+
+/**
+ * Given this event does not have a receipt, is it important enough to make
+ * this room unread?
+ */
+function isImportantEvent(event: MatrixEvent): boolean {
+    return !shouldHideEvent(event) && eventTriggersUnreadCount(event);
+}
+
+/**
+ * @returns a function that tells us whether a given event matches our read
+ *          receipt.
+ *
+ * We have the ID of an event based on a read receipt. If we can find the
+ * corresponding event, then it's easy - our returned function just decides
+ * whether the receipt refers to the event we are asking about.
+ *
+ * If we can't find the event, we guess by saying of the receipt's timestamp is
+ * after this event's timestamp, then it's probably saying this event is read.
+ */
+function makeHasReceipt(
+    roomOrThread: Room | Thread,
+    readUpToId: string,
+    myUserId: string,
+): (event: MatrixEvent) => boolean {
+    // get the most recent read receipt sent by our account.
+    // N.B. this is NOT a read marker (RM, aka "read up to marker"),
+    // despite the name of the method :((
+    const readEvent = roomOrThread.findEventById(readUpToId);
+
+    if (readEvent) {
+        // If we found an event matching our receipt, then it's easy: this event
+        // has a receipt if its ID is the same as the one in the receipt.
+        return (ev) => ev.getId() == readUpToId;
+    } else {
+        // If we didn't, we have to guess by saying if this event is before the
+        // receipt's ts, then it we pretend it has a receipt.
+        const receipt = roomOrThread.getReadReceiptForUserId(myUserId);
+        if (receipt) {
+            const receiptTimestamp = receipt.data.ts;
+            return (ev) => ev.getTs() < receiptTimestamp;
+        } else {
+            return (_ev) => false;
+        }
+    }
+}

test/Unread-test.ts

@@ -321,6 +321,88 @@ describe("Unread", () => {
 
                 expect(doesRoomHaveUnreadMessages(room)).toBe(true);
             });
+
+            it("returns false when the event for a thread receipt can't be found, but the receipt ts is late", () => {
+                // Given a room that is read
+                let receipt = new MatrixEvent({
+                    type: "m.receipt",
+                    room_id: "!foo:bar",
+                    content: {
+                        [event.getId()!]: {
+                            [ReceiptType.Read]: {
+                                [myId]: { ts: 1 },
+                            },
+                        },
+                    },
+                });
+                room.addReceipt(receipt);
+
+                // And a thread
+                const { rootEvent, events } = mkThread({ room, client, authorId: myId, participantUserIds: [aliceId] });
+
+                // When we provide a receipt that points at an unknown event,
+                // but its timestamp is after all events in the thread
+                //
+                // (This could happen if we mis-filed a reaction into the main
+                // thread when it should actually have gone into this thread, or
+                // maybe the event is just not loaded for some reason.)
+                const receiptTs = Math.max(...events.map((e) => e.getTs())) + 100;
+                receipt = new MatrixEvent({
+                    type: "m.receipt",
+                    room_id: "!foo:bar",
+                    content: {
+                        ["UNKNOWN_EVENT_ID"]: {
+                            [ReceiptType.Read]: {
+                                [myId]: { ts: receiptTs, threadId: rootEvent.getId()! },
+                            },
+                        },
+                    },
+                });
+                room.addReceipt(receipt);
+
+                expect(doesRoomHaveUnreadMessages(room)).toBe(false);
+            });
+
+            it("returns true when the event for a thread receipt can't be found, and the receipt ts is early", () => {
+                // Given a room that is read
+                let receipt = new MatrixEvent({
+                    type: "m.receipt",
+                    room_id: "!foo:bar",
+                    content: {
+                        [event.getId()!]: {
+                            [ReceiptType.Read]: {
+                                [myId]: { ts: 1 },
+                            },
+                        },
+                    },
+                });
+                room.addReceipt(receipt);
+
+                // And a thread
+                const { rootEvent, events } = mkThread({ room, client, authorId: myId, participantUserIds: [aliceId] });
+
+                // When we provide a receipt that points at an unknown event,
+                // but its timestamp is before some of the events in the thread
+                //
+                // (This could happen if we mis-filed a reaction into the main
+                // thread when it should actually have gone into this thread, or
+                // maybe the event is just not loaded for some reason.)
+                const receiptTs = events.at(-1).getTs() - 100;
+                receipt = new MatrixEvent({
+                    type: "m.receipt",
+                    room_id: "!foo:bar",
+                    content: {
+                        ["UNKNOWN_EVENT_ID"]: {
+                            [ReceiptType.Read]: {
+                                [myId]: { ts: receiptTs, threadId: rootEvent.getId()! },
+                            },
+                        },
+                    },
+                });
+                room.addReceipt(receipt);
+
+                expect(doesRoomHaveUnreadMessages(room)).toBe(true);
+            });
         });
 
         it("returns true for a room that only contains a hidden event", () => {