From 71cf9bf932cc975157a9d18ea4b8c1d1c6784d76 Mon Sep 17 00:00:00 2001 From: Germain Date: Wed, 21 Sep 2022 10:13:33 +0100 Subject: [PATCH] Read receipts for threads (#9239) * Use EventType enum instead of hardcoded value * Enable read receipts on thread timelines * Strict null checks * Strict null checks * fix import group * strict checks * strict checks * null check * fix tests --- src/components/structures/MessagePanel.tsx | 11 +- src/components/structures/ThreadPanel.tsx | 8 +- src/components/structures/ThreadView.tsx | 3 +- src/components/structures/TimelinePanel.tsx | 119 ++++++++++-------- src/components/views/rooms/EventTile.tsx | 1 + .../views/settings/Notifications.tsx | 5 +- .../structures/TimelinePanel-test.tsx | 6 +- 7 files changed, 87 insertions(+), 66 deletions(-) diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index 8e41a599f1..0dbd10cb46 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -25,6 +25,8 @@ 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'; @@ -135,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; @@ -826,8 +828,13 @@ export default class MessagePanel extends React.Component { if (!room) { return null; } + + const receiptDestination: ReadReceipt> = 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 0a07393863..8ccb7fbdd4 100644 --- a/src/components/structures/ThreadPanel.tsx +++ b/src/components/structures/ThreadPanel.tsx @@ -282,10 +282,10 @@ const ThreadPanel: React.FC = ({ ? { { disableGrouping: false, }; - private lastRRSentEventId: string = undefined; - private lastRMSentEventId: string = undefined; + private lastRRSentEventId: string | null | undefined = undefined; + private lastRMSentEventId: string | null | 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) { @@ -942,13 +943,13 @@ class TimelinePanel extends React.Component { if (lastReadEventIndex === null) { shouldSendRR = false; } - let lastReadEvent = 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. 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; } @@ -974,48 +975,57 @@ 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) { - if ( - !sendRRs - && !MatrixClientPeg.get().doesServerSupportUnstableFeature("org.matrix.msc2285.stable") - ) return; - try { - return await MatrixClientPeg.get().sendReadReceipt( - lastReadEvent, - sendRRs ? ReceiptType.Read : ReceiptType.ReadPrivate, - ); - } catch (error) { + if (this.props.timelineSet.thread && sendRRs && lastReadEvent) { + // 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 ?? undefined) : undefined, // Public 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) { + if ( + !sendRRs + && !cli.doesServerSupportUnstableFeature("org.matrix.msc2285.stable") + ) return; + try { + return await cli.sendReadReceipt( + lastReadEvent, + sendRRs ? ReceiptType.Read : ReceiptType.ReadPrivate, + ); + } 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; - }); - - // do a quick-reset of our unreadNotificationCount to avoid having - // to wait from the remote echo from the homeserver. - // we only do this if we're right at the end, because we're just assuming - // that sending an RR for the latest message will set our notif counter - // to zero: it may not do this if we send an RR for somewhere before the end. - if (this.isAtEndOfLiveTimeline()) { - this.props.timelineSet.room.setUnreadNotificationCount(NotificationCountType.Total, 0); - this.props.timelineSet.room.setUnreadNotificationCount(NotificationCountType.Highlight, 0); - dis.dispatch({ - action: 'on_room_read', - roomId: this.props.timelineSet.room.roomId, + // 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. + // we only do this if we're right at the end, because we're just assuming + // that sending an RR for the latest message will set our notif counter + // to zero: it may not do this if we send an RR for somewhere before the end. + if (this.isAtEndOfLiveTimeline()) { + this.props.timelineSet.room.setUnreadNotificationCount(NotificationCountType.Total, 0); + this.props.timelineSet.room.setUnreadNotificationCount(NotificationCountType.Highlight, 0); + dis.dispatch({ + action: 'on_room_read', + roomId: this.props.timelineSet.room.roomId, + }); + } } } }; @@ -1149,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; }); @@ -1554,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 */ @@ -1655,7 +1666,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) { @@ -1663,21 +1674,23 @@ class TimelinePanel extends React.Component { } const myUserId = client.credentials.userId; - return this.props.timelineSet.room.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; } // 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; diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index b4d022cab4..9ae8aa7a45 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 c7a95b060a..77c02bc032 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]); } } }); diff --git a/test/components/structures/TimelinePanel-test.tsx b/test/components/structures/TimelinePanel-test.tsx index 5d133fb705..542f0c8887 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 => { @@ -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]); }); }); });