From 67313743fcfb71af3a0e2c8a7ce8fda8c830fa7b Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 26 May 2023 15:14:47 +0100 Subject: [PATCH] Fall back to receipt timestamp if we have no event (react-sdk part) (#10974) --- src/Unread.ts | 71 +++++++++++++++++++++++++++++++-------- test/Unread-test.ts | 82 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 14 deletions(-) diff --git a/src/Unread.ts b/src/Unread.ts index 7b86a013a5..25bf1c3529 100644 --- a/src/Unread.ts +++ b/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 | null, + 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 = readUpToId ? roomOrThread.findEventById(readUpToId) : null; + + 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; + } + } +} diff --git a/test/Unread-test.ts b/test/Unread-test.ts index 998448d886..0e59e9a8c0 100644 --- a/test/Unread-test.ts +++ b/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() ?? 0) - 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", () => {