From 9668a24ca705a3d38c240174e04e0d66abe8953d Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 13 Dec 2022 14:59:52 +0000 Subject: [PATCH] Revert "Check each thread for unread messages. (#9723)" (#9745) * Revert "Check each thread for unread messages. (#9723)" This reverts commit 9de5654353a9209ff85cdaea5832043822af5e4c. * ts strict --- src/Unread.ts | 45 +++--- test/Unread-test.ts | 354 +++++++++----------------------------------- 2 files changed, 97 insertions(+), 302 deletions(-) diff --git a/src/Unread.ts b/src/Unread.ts index 4f38c8e76ab..1a39c6f212b 100644 --- a/src/Unread.ts +++ b/src/Unread.ts @@ -15,7 +15,6 @@ limitations under the License. */ import { Room } from "matrix-js-sdk/src/models/room"; -import { Thread } from "matrix-js-sdk/src/models/thread"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { EventType } from "matrix-js-sdk/src/@types/event"; import { M_BEACON } from "matrix-js-sdk/src/@types/beacon"; @@ -60,34 +59,36 @@ export function doesRoomHaveUnreadMessages(room: Room): boolean { return false; } - for (const timeline of [room, ...room.getThreads()]) { - // If the current timeline has unread messages, we're done. - if (doesRoomOrThreadHaveUnreadMessages(timeline)) { - return true; - } - } - // If we got here then no timelines were found with unread messages. - return false; -} - -function doesRoomOrThreadHaveUnreadMessages(room: Room | Thread): boolean { 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! - // Should fix: https://github.com/vector-im/element-web/issues/3263 - // https://github.com/vector-im/element-web/issues/2427 - // ...and possibly some of the others at - // https://github.com/vector-im/element-web/issues/3363 - if (room.timeline.at(-1)?.getSender() === myUserId) { - 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 = room.getEventReadUpTo(myUserId!); + if (!SettingsStore.getValue("feature_thread")) { + // 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! + // Should fix: https://github.com/vector-im/element-web/issues/3263 + // https://github.com/vector-im/element-web/issues/2427 + // ...and possibly some of the others at + // https://github.com/vector-im/element-web/issues/3363 + if (room.timeline.length && room.timeline[room.timeline.length - 1].getSender() === myUserId) { + return false; + } + } + + // if the read receipt relates to an event is that part of a thread + // we consider that there are no unread messages + // This might be a false negative, but probably the best we can do until + // the read receipts have evolved to cater for threads + if (readUpToId) { + const event = room.findEventById(readUpToId); + if (event?.getThread()) { + return false; + } + } + // 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 diff --git a/test/Unread-test.ts b/test/Unread-test.ts index 8ff759b142b..7a271354de1 100644 --- a/test/Unread-test.ts +++ b/test/Unread-test.ts @@ -15,306 +15,100 @@ limitations under the License. */ import { mocked } from "jest-mock"; -import { MatrixEvent, EventType, MsgType, Room } from "matrix-js-sdk/src/matrix"; -import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts"; +import { MatrixEvent, EventType, MsgType } from "matrix-js-sdk/src/matrix"; import { haveRendererForEvent } from "../src/events/EventTileFactory"; -import { makeBeaconEvent, mkEvent, stubClient } from "./test-utils"; -import { mkThread } from "./test-utils/threads"; -import { doesRoomHaveUnreadMessages, eventTriggersUnreadCount } from "../src/Unread"; -import { MatrixClientPeg } from "../src/MatrixClientPeg"; +import { getMockClientWithEventEmitter, makeBeaconEvent, mockClientMethodsUser } from "./test-utils"; +import { eventTriggersUnreadCount } from "../src/Unread"; jest.mock("../src/events/EventTileFactory", () => ({ haveRendererForEvent: jest.fn(), })); -describe("Unread", () => { - // A different user. +describe("eventTriggersUnreadCount()", () => { const aliceId = "@alice:server.org"; - stubClient(); - const client = MatrixClientPeg.get(); + const bobId = "@bob:server.org"; - describe("eventTriggersUnreadCount()", () => { - // setup events - const alicesMessage = new MatrixEvent({ - type: EventType.RoomMessage, - sender: aliceId, - content: { - msgtype: MsgType.Text, - body: "Hello from Alice", - }, - }); - - const ourMessage = new MatrixEvent({ - type: EventType.RoomMessage, - sender: client.getUserId()!, - content: { - msgtype: MsgType.Text, - body: "Hello from Bob", - }, - }); - - const redactedEvent = new MatrixEvent({ - type: EventType.RoomMessage, - sender: aliceId, - }); - redactedEvent.makeRedacted(redactedEvent); - - beforeEach(() => { - jest.clearAllMocks(); - mocked(haveRendererForEvent).mockClear().mockReturnValue(false); - }); - - it("returns false when the event was sent by the current user", () => { - expect(eventTriggersUnreadCount(ourMessage)).toBe(false); - // returned early before checking renderer - expect(haveRendererForEvent).not.toHaveBeenCalled(); - }); - - it("returns false for a redacted event", () => { - expect(eventTriggersUnreadCount(redactedEvent)).toBe(false); - // returned early before checking renderer - expect(haveRendererForEvent).not.toHaveBeenCalled(); - }); - - it("returns false for an event without a renderer", () => { - mocked(haveRendererForEvent).mockReturnValue(false); - expect(eventTriggersUnreadCount(alicesMessage)).toBe(false); - expect(haveRendererForEvent).toHaveBeenCalledWith(alicesMessage, false); - }); - - it("returns true for an event with a renderer", () => { - mocked(haveRendererForEvent).mockReturnValue(true); - expect(eventTriggersUnreadCount(alicesMessage)).toBe(true); - expect(haveRendererForEvent).toHaveBeenCalledWith(alicesMessage, false); - }); - - it("returns false for beacon locations", () => { - const beaconLocationEvent = makeBeaconEvent(aliceId); - expect(eventTriggersUnreadCount(beaconLocationEvent)).toBe(false); - expect(haveRendererForEvent).not.toHaveBeenCalled(); - }); - - const noUnreadEventTypes = [ - EventType.RoomMember, - EventType.RoomThirdPartyInvite, - EventType.CallAnswer, - EventType.CallHangup, - EventType.RoomCanonicalAlias, - EventType.RoomServerAcl, - ]; - - it.each(noUnreadEventTypes)( - "returns false without checking for renderer for events with type %s", - (eventType) => { - const event = new MatrixEvent({ - type: eventType, - sender: aliceId, - }); - expect(eventTriggersUnreadCount(event)).toBe(false); - expect(haveRendererForEvent).not.toHaveBeenCalled(); - }, - ); + // mock user credentials + getMockClientWithEventEmitter({ + ...mockClientMethodsUser(bobId), }); - describe("doesRoomHaveUnreadMessages()", () => { - let room: Room; - let event: MatrixEvent; - const roomId = "!abc:server.org"; - const myId = client.getUserId()!; - - beforeAll(() => { - client.supportsExperimentalThreads = () => true; - }); - - beforeEach(() => { - // Create a room and initial event in it. - room = new Room(roomId, client, myId); - event = mkEvent({ - event: true, - type: "m.room.message", - user: aliceId, - room: roomId, - content: {}, - }); - room.addLiveEvents([event]); - - // Don't care about the code path of hidden events. - mocked(haveRendererForEvent).mockClear().mockReturnValue(true); - }); - - it("returns true for a room with no receipts", () => { - expect(doesRoomHaveUnreadMessages(room)).toBe(true); - }); - - it("returns false for a room when the latest event was sent by the current user", () => { - event = mkEvent({ - event: true, - type: "m.room.message", - user: myId, - room: roomId, - content: {}, - }); - // Only for timeline events. - room.addLiveEvents([event]); - - expect(doesRoomHaveUnreadMessages(room)).toBe(false); - }); - - it("returns false for a room when the read receipt is at the latest event", () => { - const receipt = new MatrixEvent({ - type: "m.receipt", - room_id: "!foo:bar", - content: { - [event.getId()!]: { - [ReceiptType.Read]: { - [myId]: { ts: 1 }, - }, - }, - }, - }); - room.addReceipt(receipt); - - expect(doesRoomHaveUnreadMessages(room)).toBe(false); - }); - - it("returns true for a room when the read receipt is earlier than the latest event", () => { - const receipt = new MatrixEvent({ - type: "m.receipt", - room_id: "!foo:bar", - content: { - [event.getId()!]: { - [ReceiptType.Read]: { - [myId]: { ts: 1 }, - }, - }, - }, - }); - room.addReceipt(receipt); - - const event2 = mkEvent({ - event: true, - type: "m.room.message", - user: aliceId, - room: roomId, - content: {}, - }); - // Only for timeline events. - room.addLiveEvents([event2]); - - expect(doesRoomHaveUnreadMessages(room)).toBe(true); - }); - - it("returns true for a room with an unread message in a thread", () => { - // Mark the main timeline as read. - const receipt = new MatrixEvent({ - type: "m.receipt", - room_id: "!foo:bar", - content: { - [event.getId()!]: { - [ReceiptType.Read]: { - [myId]: { ts: 1 }, - }, - }, - }, - }); - room.addReceipt(receipt); - - // Create a thread as a different user. - mkThread({ room, client, authorId: myId, participantUserIds: [aliceId] }); - - expect(doesRoomHaveUnreadMessages(room)).toBe(true); - }); - - it("returns false for a room when the latest thread event was sent by the current user", () => { - // Mark the main timeline as read. - const receipt = new MatrixEvent({ - type: "m.receipt", - room_id: "!foo:bar", - content: { - [event.getId()!]: { - [ReceiptType.Read]: { - [myId]: { ts: 1 }, - }, - }, - }, - }); - room.addReceipt(receipt); - - // Create a thread as the current user. - mkThread({ room, client, authorId: myId, participantUserIds: [myId] }); + // setup events + const alicesMessage = new MatrixEvent({ + type: EventType.RoomMessage, + sender: aliceId, + content: { + msgtype: MsgType.Text, + body: "Hello from Alice", + }, + }); - expect(doesRoomHaveUnreadMessages(room)).toBe(false); - }); + const bobsMessage = new MatrixEvent({ + type: EventType.RoomMessage, + sender: bobId, + content: { + msgtype: MsgType.Text, + body: "Hello from Bob", + }, + }); - it("returns false for a room with read thread messages", () => { - // Mark the main timeline as read. - let receipt = new MatrixEvent({ - type: "m.receipt", - room_id: "!foo:bar", - content: { - [event.getId()!]: { - [ReceiptType.Read]: { - [myId]: { ts: 1 }, - }, - }, - }, - }); - room.addReceipt(receipt); + const redactedEvent = new MatrixEvent({ + type: EventType.RoomMessage, + sender: aliceId, + }); + redactedEvent.makeRedacted(redactedEvent); - // Create threads. - const { rootEvent, events } = mkThread({ room, client, authorId: myId, participantUserIds: [aliceId] }); + beforeEach(() => { + jest.clearAllMocks(); + mocked(haveRendererForEvent).mockClear().mockReturnValue(false); + }); - // Mark the thread as read. - receipt = new MatrixEvent({ - type: "m.receipt", - room_id: "!foo:bar", - content: { - [events[events.length - 1].getId()!]: { - [ReceiptType.Read]: { - [myId]: { ts: 1, thread_id: rootEvent.getId()! }, - }, - }, - }, - }); - room.addReceipt(receipt); + it("returns false when the event was sent by the current user", () => { + expect(eventTriggersUnreadCount(bobsMessage)).toBe(false); + // returned early before checking renderer + expect(haveRendererForEvent).not.toHaveBeenCalled(); + }); - expect(doesRoomHaveUnreadMessages(room)).toBe(false); - }); + it("returns false for a redacted event", () => { + expect(eventTriggersUnreadCount(redactedEvent)).toBe(false); + // returned early before checking renderer + expect(haveRendererForEvent).not.toHaveBeenCalled(); + }); - it("returns true for a room when read receipt is not on the latest thread messages", () => { - // Mark the main timeline as read. - let receipt = new MatrixEvent({ - type: "m.receipt", - room_id: "!foo:bar", - content: { - [event.getId()!]: { - [ReceiptType.Read]: { - [myId]: { ts: 1 }, - }, - }, - }, - }); - room.addReceipt(receipt); + it("returns false for an event without a renderer", () => { + mocked(haveRendererForEvent).mockReturnValue(false); + expect(eventTriggersUnreadCount(alicesMessage)).toBe(false); + expect(haveRendererForEvent).toHaveBeenCalledWith(alicesMessage, false); + }); - // Create threads. - const { rootEvent, events } = mkThread({ room, client, authorId: myId, participantUserIds: [aliceId] }); + it("returns true for an event with a renderer", () => { + mocked(haveRendererForEvent).mockReturnValue(true); + expect(eventTriggersUnreadCount(alicesMessage)).toBe(true); + expect(haveRendererForEvent).toHaveBeenCalledWith(alicesMessage, false); + }); - // Mark the thread as read. - receipt = new MatrixEvent({ - type: "m.receipt", - room_id: "!foo:bar", - content: { - [events[0].getId()!]: { - [ReceiptType.Read]: { - [myId]: { ts: 1, threadId: rootEvent.getId()! }, - }, - }, - }, - }); - room.addReceipt(receipt); + it("returns false for beacon locations", () => { + const beaconLocationEvent = makeBeaconEvent(aliceId); + expect(eventTriggersUnreadCount(beaconLocationEvent)).toBe(false); + expect(haveRendererForEvent).not.toHaveBeenCalled(); + }); - expect(doesRoomHaveUnreadMessages(room)).toBe(true); + const noUnreadEventTypes = [ + EventType.RoomMember, + EventType.RoomThirdPartyInvite, + EventType.CallAnswer, + EventType.CallHangup, + EventType.RoomCanonicalAlias, + EventType.RoomServerAcl, + ]; + + it.each(noUnreadEventTypes)("returns false without checking for renderer for events with type %s", (eventType) => { + const event = new MatrixEvent({ + type: eventType, + sender: aliceId, }); + expect(eventTriggersUnreadCount(event)).toBe(false); + expect(haveRendererForEvent).not.toHaveBeenCalled(); }); });