From ac431196eb807073b694d101fde452c4ae18d0f9 Mon Sep 17 00:00:00 2001
From: Germain <germain@souquet.com>
Date: Tue, 13 Dec 2022 12:47:20 +0100
Subject: [PATCH 1/2] Revert "Check each thread for unread messages. (#9723)"
This reverts commit 9de5654353a9209ff85cdaea5832043822af5e4c.
---
src/Unread.ts | 45 +++---
test/Unread-test.ts | 354 +++++++++-----------------------------------
2 files changed, 96 insertions(+), 303 deletions(-)
diff --git a/src/Unread.ts b/src/Unread.ts
index 4f38c8e76ab..727090a810b 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,33 +59,33 @@ 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!);
+ 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
+ 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
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();
});
});
From dc18fc824bf7244f97e11ebcd8f35b4f419cae0e Mon Sep 17 00:00:00 2001
From: Germain <germain@souquet.com>
Date: Tue, 13 Dec 2022 15:03:30 +0100
Subject: [PATCH 2/2] ts strict
---
src/Unread.ts | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/Unread.ts b/src/Unread.ts
index 727090a810b..1a39c6f212b 100644
--- a/src/Unread.ts
+++ b/src/Unread.ts
@@ -64,7 +64,7 @@ export function doesRoomHaveUnreadMessages(room: Room): 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 readUpToId = room.getEventReadUpTo(myUserId);
+ 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
@@ -82,9 +82,11 @@ export function doesRoomHaveUnreadMessages(room: Room): boolean {
// 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
- const event = room.findEventById(readUpToId);
- if (event?.getThread()) {
- return false;
+ 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