From 9e0798f36155bf5a8e88dc119ecb8779cb1185f6 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 2 Sep 2022 10:23:55 +0100 Subject: [PATCH 1/9] Use EventType enum instead of hardcoded value --- test/components/structures/TimelinePanel-test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/structures/TimelinePanel-test.tsx b/test/components/structures/TimelinePanel-test.tsx index 5d133fb7058..95c40a6db67 100644 --- a/test/components/structures/TimelinePanel-test.tsx +++ b/test/components/structures/TimelinePanel-test.tsx @@ -42,7 +42,7 @@ const newReceipt = (eventId: string, userId: string, readTs: number, fullyReadTs [ReceiptType.FullyRead]: { [userId]: { ts: fullyReadTs } }, }, }; - return new MatrixEvent({ content: receiptContent, type: "m.receipt" }); + return new MatrixEvent({ content: receiptContent, type: EventType.Receipt }); }; const renderPanel = (room: Room, events: MatrixEvent[]): RenderResult => { From 3134d4e6718332bac2f1815a1db7877275ec3104 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Fri, 2 Sep 2022 15:22:36 +0100 Subject: [PATCH 2/9] Enable read receipts on thread timelines --- src/components/structures/MessagePanel.tsx | 7 +- src/components/structures/ThreadPanel.tsx | 8 +-- src/components/structures/ThreadView.tsx | 3 +- src/components/structures/TimelinePanel.tsx | 69 +++++++++++-------- src/components/views/rooms/EventTile.tsx | 1 + .../views/settings/Notifications.tsx | 5 +- 6 files changed, 55 insertions(+), 38 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 8e41a599f11..3993dd8544a 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -826,8 +826,13 @@ export default class MessagePanel extends React.Component { if (!room) { return null; } + + const receiptDestination = this.context.threadId + ? room.getThread(this.context.threadId) + : room; + const receipts: IReadReceiptProps[] = []; - room.getReceiptsForEvent(event).forEach((r) => { + receiptDestination.getReceiptsForEvent(event).forEach((r) => { if ( !r.userId || !isSupportedReceiptType(r.type) || diff --git a/src/components/structures/ThreadPanel.tsx b/src/components/structures/ThreadPanel.tsx index 0a073938635..8ccb7fbdd46 100644 --- a/src/components/structures/ThreadPanel.tsx +++ b/src/components/structures/ThreadPanel.tsx @@ -282,10 +282,10 @@ const ThreadPanel: React.FC = ({ ? { { if (lastReadEventIndex === null) { shouldSendRR = false; } - let lastReadEvent = this.state.events[lastReadEventIndex]; + let lastReadEvent: MatrixEvent | null = this.state.events[lastReadEventIndex]; 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(); + this.lastRRSentEventId !== lastReadEvent.getId(); // Only send a RM if the last RM sent != the one we would send const shouldSendRM = @@ -975,33 +975,43 @@ class TimelinePanel extends React.Component { `prr=${lastReadEvent?.getId()}`, ); - MatrixClientPeg.get().setRoomReadMarkers( - roomId, - this.state.readMarkerEventId, - sendRRs ? lastReadEvent : null, // Public read receipt (could be null) - lastReadEvent, // Private read receipt (could be null) - ).catch(async (e) => { - // /read_markers API is not implemented on this HS, fallback to just RR - if (e.errcode === 'M_UNRECOGNIZED' && lastReadEvent) { - const privateField = await getPrivateReadReceiptField(MatrixClientPeg.get()); - if (!sendRRs && !privateField) return; - - try { - return await MatrixClientPeg.get().sendReadReceipt( - lastReadEvent, - sendRRs ? ReceiptType.Read : privateField, - ); - } catch (error) { + + if (this.props.timelineSet.thread && sendRRs) { + // 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 : null, // Public read receipt (could be null) + lastReadEvent, // Private read receipt (could be null) + ).catch(async (e) => { + // /read_markers API is not implemented on this HS, fallback to just RR + if (e.errcode === 'M_UNRECOGNIZED' && lastReadEvent) { + const privateField = await getPrivateReadReceiptField(cli); + if (!sendRRs && !privateField) return; + + try { + return await cli.sendReadReceipt( + lastReadEvent, + sendRRs ? ReceiptType.Read : privateField, + ); + } catch (error) { + logger.error(e); + this.lastRRSentEventId = undefined; + } + } else { 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; - }); + // 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. @@ -1654,7 +1664,7 @@ class TimelinePanel extends React.Component { * SDK. * @return {String} the event ID */ - private getCurrentReadReceipt(ignoreSynthesized = false): string { + private getCurrentReadReceipt(ignoreSynthesized = false): string | null { const client = MatrixClientPeg.get(); // the client can be null on logout if (client == null) { @@ -1662,7 +1672,8 @@ class TimelinePanel extends React.Component { } const myUserId = client.credentials.userId; - return this.props.timelineSet.room.getEventReadUpTo(myUserId, ignoreSynthesized); + const receiptStore = this.props.timelineSet.thread ?? this.props.timelineSet.room; + return receiptStore.getEventReadUpTo(myUserId, ignoreSynthesized); } private setReadMarker(eventId: string, eventTs: number, inhibitSetState = false): void { diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index b4d022cab4a..9ae8aa7a456 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -1333,6 +1333,7 @@ export class UnwrappedEventTile extends React.Component { { timestamp } + { msgOption } , reactionsRow, ]); diff --git a/src/components/views/settings/Notifications.tsx b/src/components/views/settings/Notifications.tsx index c7a95b060ae..77c02bc032e 100644 --- a/src/components/views/settings/Notifications.tsx +++ b/src/components/views/settings/Notifications.tsx @@ -398,12 +398,13 @@ export default class Notifications extends React.PureComponent { }; private onClearNotificationsClicked = () => { - MatrixClientPeg.get().getRooms().forEach(r => { + const client = MatrixClientPeg.get(); + client.getRooms().forEach(r => { if (r.getUnreadNotificationCount() > 0) { const events = r.getLiveTimeline().getEvents(); if (events.length) { // noinspection JSIgnoredPromiseFromCall - MatrixClientPeg.get().sendReadReceipt(events[events.length - 1]); + client.sendReadReceipt(events[events.length - 1]); } } }); From 2e4c619448383b693681679cdfebef65ca8de5de Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Wed, 14 Sep 2022 12:27:42 +0100 Subject: [PATCH 3/9] Strict null checks --- src/components/structures/TimelinePanel.tsx | 29 ++++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/components/structures/TimelinePanel.tsx b/src/components/structures/TimelinePanel.tsx index 88eebab9f24..a26eb9d11c2 100644 --- a/src/components/structures/TimelinePanel.tsx +++ b/src/components/structures/TimelinePanel.tsx @@ -30,6 +30,7 @@ import { ClientEvent } from "matrix-js-sdk/src/client"; import { Thread } 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'; +import { ReadReceipt } from 'matrix-js-sdk/src/models/read-receipt'; import SettingsStore from "../../settings/SettingsStore"; import { Layout } from "../../settings/enums/Layout"; @@ -229,8 +230,8 @@ class TimelinePanel extends React.Component { disableGrouping: false, }; - private lastRRSentEventId: string = undefined; - private lastRMSentEventId: string = undefined; + private lastRRSentEventId: string | undefined = undefined; + private lastRMSentEventId: string | undefined = undefined; private readonly messagePanel = createRef(); private readonly dispatcherRef: string; @@ -250,7 +251,7 @@ class TimelinePanel extends React.Component { // XXX: we could track RM per TimelineSet rather than per Room. // but for now we just do it per room for simplicity. - let initialReadMarker = null; + let initialReadMarker: string | null = null; if (this.props.manageReadMarkers) { const readmarker = this.props.timelineSet.room.getAccountData('m.fully_read'); if (readmarker) { @@ -948,7 +949,7 @@ class TimelinePanel extends React.Component { // the current RR event. lastReadEventIndex > currentRREventIndex && // Only send a RR if the last RR set != the one we would send - this.lastRRSentEventId !== lastReadEvent.getId(); + this.lastRRSentEventId !== lastReadEvent?.getId(); // Only send a RM if the last RM sent != the one we would send const shouldSendRM = @@ -958,7 +959,7 @@ class TimelinePanel extends React.Component { // same one at the server repeatedly if (shouldSendRR || shouldSendRM) { if (shouldSendRR) { - this.lastRRSentEventId = lastReadEvent.getId(); + this.lastRRSentEventId = lastReadEvent?.getId(); } else { lastReadEvent = null; } @@ -975,7 +976,7 @@ class TimelinePanel extends React.Component { ); - if (this.props.timelineSet.thread && sendRRs) { + if (this.props.timelineSet.thread && sendRRs && lastReadEvent) { // There's no support for fully read markers on threads // as defined by MSC3771 cli.sendReadReceipt( @@ -986,7 +987,7 @@ class TimelinePanel extends React.Component { cli.setRoomReadMarkers( roomId, this.state.readMarkerEventId, - sendRRs ? lastReadEvent : null, // Public read receipt (could be null) + sendRRs ? lastReadEvent : undefined, // Public read receipt (could be null) lastReadEvent, // Private read receipt (could be null) ).catch(async (e) => { // /read_markers API is not implemented on this HS, fallback to just RR @@ -1563,7 +1564,8 @@ class TimelinePanel extends React.Component { return 0; } - private indexForEventId(evId: string): number | null { + private indexForEventId(evId: string | null): number | null { + if (evId === null) { return null; } /* Threads do not have server side support for read receipts and the concept is very tied to the main room timeline, we are forcing the timeline to send read receipts for threaded events */ @@ -1672,16 +1674,17 @@ class TimelinePanel extends React.Component { } const myUserId = client.credentials.userId; - const receiptStore = this.props.timelineSet.thread ?? this.props.timelineSet.room; - return receiptStore.getEventReadUpTo(myUserId, ignoreSynthesized); + const receiptStore: ReadReceipt = + this.props.timelineSet.thread ?? this.props.timelineSet.room; + return receiptStore?.getEventReadUpTo(myUserId, ignoreSynthesized); } - private setReadMarker(eventId: string, eventTs: number, inhibitSetState = false): void { - const roomId = this.props.timelineSet.room.roomId; + private setReadMarker(eventId: string | null, eventTs: number, inhibitSetState = false): void { + const roomId = this.props.timelineSet.room?.roomId; // don't update the state (and cause a re-render) if there is // no change to the RM. - if (eventId === this.state.readMarkerEventId) { + if (eventId === this.state.readMarkerEventId || eventId === null) { return; } From e68e4630419696efc1549a4df3ca4d7b50c989b4 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Wed, 14 Sep 2022 12:29:22 +0100 Subject: [PATCH 4/9] Strict null checks --- src/components/structures/MessagePanel.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 3993dd8544a..2e417de7191 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -25,7 +25,9 @@ import { logger } from 'matrix-js-sdk/src/logger'; import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; import { M_BEACON_INFO } from 'matrix-js-sdk/src/@types/beacon'; import { isSupportedReceiptType } from "matrix-js-sdk/src/utils"; +import { ReadReceipt } from 'matrix-js-sdk/src/models/read-receipt'; +import { ListenerMap } from 'matrix-js-sdk/src/models/typed-event-emitter'; import shouldHideEvent from '../../shouldHideEvent'; import { wantsDateSeparator } from '../../DateUtils'; import { MatrixClientPeg } from '../../MatrixClientPeg'; @@ -827,7 +829,7 @@ export default class MessagePanel extends React.Component { return null; } - const receiptDestination = this.context.threadId + const receiptDestination: ReadReceipt> = this.context.threadId ? room.getThread(this.context.threadId) : room; From 053b68d6b422ea2d8c7560318a66fc15e65bccb7 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Wed, 14 Sep 2022 12:33:28 +0100 Subject: [PATCH 5/9] fix import group --- src/components/structures/MessagePanel.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 2e417de7191..a0b608a9a8f 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -26,8 +26,8 @@ import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; import { M_BEACON_INFO } from 'matrix-js-sdk/src/@types/beacon'; import { isSupportedReceiptType } from "matrix-js-sdk/src/utils"; import { ReadReceipt } from 'matrix-js-sdk/src/models/read-receipt'; - import { ListenerMap } from 'matrix-js-sdk/src/models/typed-event-emitter'; + import shouldHideEvent from '../../shouldHideEvent'; import { wantsDateSeparator } from '../../DateUtils'; import { MatrixClientPeg } from '../../MatrixClientPeg'; From 36459aead9ff2b58e47775b0cf7eb2bd9a57770e Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Wed, 14 Sep 2022 12:39:22 +0100 Subject: [PATCH 6/9] strict checks --- src/components/structures/TimelinePanel.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/structures/TimelinePanel.tsx b/src/components/structures/TimelinePanel.tsx index a26eb9d11c2..546edd5305b 100644 --- a/src/components/structures/TimelinePanel.tsx +++ b/src/components/structures/TimelinePanel.tsx @@ -180,7 +180,7 @@ interface IState { // disappearance when switching into the room. readMarkerVisible: boolean; - readMarkerEventId: string; + readMarkerEventId: string | null; backPaginating: boolean; forwardPaginating: boolean; @@ -943,7 +943,7 @@ class TimelinePanel extends React.Component { if (lastReadEventIndex === null) { shouldSendRR = false; } - let lastReadEvent: MatrixEvent | null = this.state.events[lastReadEventIndex]; + 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. @@ -987,7 +987,7 @@ class TimelinePanel extends React.Component { cli.setRoomReadMarkers( roomId, this.state.readMarkerEventId, - sendRRs ? lastReadEvent : undefined, // Public read receipt (could be null) + sendRRs ? (lastReadEvent ?? undefined) : undefined, // Public read receipt (could be null) lastReadEvent, // Private read receipt (could be null) ).catch(async (e) => { // /read_markers API is not implemented on this HS, fallback to just RR @@ -1690,7 +1690,7 @@ class TimelinePanel extends React.Component { // in order to later figure out if the read marker is // above or below the visible timeline, we stash the timestamp. - TimelinePanel.roomReadMarkerTsMap[roomId] = eventTs; + TimelinePanel.roomReadMarkerTsMap[roomId ?? ""] = eventTs; if (inhibitSetState) { return; From 77a1fb5aa7e1f57f726e46526b72ae748ca5c60f Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Wed, 14 Sep 2022 12:49:07 +0100 Subject: [PATCH 7/9] strict checks --- src/components/structures/MessagePanel.tsx | 2 +- src/components/structures/TimelinePanel.tsx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index a0b608a9a8f..0dbd10cb467 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -137,7 +137,7 @@ interface IProps { showUrlPreview?: boolean; // event after which we should show a read marker - readMarkerEventId?: string; + readMarkerEventId?: string | null; // whether the read marker should be visible readMarkerVisible?: boolean; diff --git a/src/components/structures/TimelinePanel.tsx b/src/components/structures/TimelinePanel.tsx index 546edd5305b..9ba4be27b86 100644 --- a/src/components/structures/TimelinePanel.tsx +++ b/src/components/structures/TimelinePanel.tsx @@ -230,8 +230,8 @@ class TimelinePanel extends React.Component { disableGrouping: false, }; - private lastRRSentEventId: string | undefined = undefined; - private lastRMSentEventId: string | undefined = undefined; + private lastRRSentEventId: string | null | undefined = undefined; + private lastRMSentEventId: string | null | undefined = undefined; private readonly messagePanel = createRef(); private readonly dispatcherRef: string; @@ -986,7 +986,7 @@ class TimelinePanel extends React.Component { } else { cli.setRoomReadMarkers( roomId, - this.state.readMarkerEventId, + this.state.readMarkerEventId ?? "", sendRRs ? (lastReadEvent ?? undefined) : undefined, // Public read receipt (could be null) lastReadEvent, // Private read receipt (could be null) ).catch(async (e) => { From a2609533d6da14011d00826d04f505376d42bb52 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Wed, 14 Sep 2022 15:13:24 +0100 Subject: [PATCH 8/9] null check --- src/components/structures/TimelinePanel.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/structures/TimelinePanel.tsx b/src/components/structures/TimelinePanel.tsx index 9ba4be27b86..b14016d0be8 100644 --- a/src/components/structures/TimelinePanel.tsx +++ b/src/components/structures/TimelinePanel.tsx @@ -988,7 +988,7 @@ class TimelinePanel extends React.Component { roomId, this.state.readMarkerEventId ?? "", sendRRs ? (lastReadEvent ?? undefined) : undefined, // Public read receipt (could be null) - lastReadEvent, // Private read receipt (could be null) + lastReadEvent ?? undefined, // Private read receipt (could be null) ).catch(async (e) => { // /read_markers API is not implemented on this HS, fallback to just RR if (e.errcode === 'M_UNRECOGNIZED' && lastReadEvent) { @@ -1159,7 +1159,7 @@ class TimelinePanel extends React.Component { const rmId = this.getCurrentReadReceipt(); // Look up the timestamp if we can find it - const tl = this.props.timelineSet.getTimelineForEvent(rmId); + const tl = this.props.timelineSet.getTimelineForEvent(rmId ?? ""); let rmTs: number; if (tl) { const event = tl.getEvents().find((e) => { return e.getId() == rmId; }); From 89ea873493cf3cde4aa16e24c8ab8b4b6eabc049 Mon Sep 17 00:00:00 2001 From: Germain Souquet Date: Wed, 21 Sep 2022 08:57:59 +0100 Subject: [PATCH 9/9] fix tests --- test/components/structures/TimelinePanel-test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/components/structures/TimelinePanel-test.tsx b/test/components/structures/TimelinePanel-test.tsx index 95c40a6db67..542f0c88878 100644 --- a/test/components/structures/TimelinePanel-test.tsx +++ b/test/components/structures/TimelinePanel-test.tsx @@ -154,7 +154,7 @@ describe('TimelinePanel', () => { }); renderPanel(room, events); - expect(client.setRoomReadMarkers).toHaveBeenCalledWith(room.roomId, null, events[0], events[0]); + expect(client.setRoomReadMarkers).toHaveBeenCalledWith(room.roomId, "", events[0], events[0]); }); it("does not send public read receipt when enabled", () => { @@ -169,7 +169,7 @@ describe('TimelinePanel', () => { }); renderPanel(room, events); - expect(client.setRoomReadMarkers).toHaveBeenCalledWith(room.roomId, null, null, events[0]); + expect(client.setRoomReadMarkers).toHaveBeenCalledWith(room.roomId, "", undefined, events[0]); }); }); });