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
This commit is contained in:
Germain 2022-09-21 10:13:33 +01:00 committed by GitHub
parent fa2ec7f6c9
commit 71cf9bf932
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 87 additions and 66 deletions

View file

@ -25,6 +25,8 @@ import { logger } from 'matrix-js-sdk/src/logger';
import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state"; import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state";
import { M_BEACON_INFO } from 'matrix-js-sdk/src/@types/beacon'; import { M_BEACON_INFO } from 'matrix-js-sdk/src/@types/beacon';
import { isSupportedReceiptType } from "matrix-js-sdk/src/utils"; 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 shouldHideEvent from '../../shouldHideEvent';
import { wantsDateSeparator } from '../../DateUtils'; import { wantsDateSeparator } from '../../DateUtils';
@ -135,7 +137,7 @@ interface IProps {
showUrlPreview?: boolean; showUrlPreview?: boolean;
// event after which we should show a read marker // event after which we should show a read marker
readMarkerEventId?: string; readMarkerEventId?: string | null;
// whether the read marker should be visible // whether the read marker should be visible
readMarkerVisible?: boolean; readMarkerVisible?: boolean;
@ -826,8 +828,13 @@ export default class MessagePanel extends React.Component<IProps, IState> {
if (!room) { if (!room) {
return null; return null;
} }
const receiptDestination: ReadReceipt<string, ListenerMap<string>> = this.context.threadId
? room.getThread(this.context.threadId)
: room;
const receipts: IReadReceiptProps[] = []; const receipts: IReadReceiptProps[] = [];
room.getReceiptsForEvent(event).forEach((r) => { receiptDestination.getReceiptsForEvent(event).forEach((r) => {
if ( if (
!r.userId || !r.userId ||
!isSupportedReceiptType(r.type) || !isSupportedReceiptType(r.type) ||

View file

@ -282,10 +282,10 @@ const ThreadPanel: React.FC<IProps> = ({
? <TimelinePanel ? <TimelinePanel
key={timelineSet.getFilter()?.filterId ?? (roomId + ":" + filterOption)} key={timelineSet.getFilter()?.filterId ?? (roomId + ":" + filterOption)}
ref={timelinePanel} ref={timelinePanel}
showReadReceipts={false} // No RR support in thread's MVP showReadReceipts={false} // No RR support in thread's list
manageReadReceipts={false} // No RR support in thread's MVP manageReadReceipts={false} // No RR support in thread's list
manageReadMarkers={false} // No RM support in thread's MVP manageReadMarkers={false} // No RM support in thread's list
sendReadReceiptOnLoad={false} // No RR support in thread's MVP sendReadReceiptOnLoad={false} // No RR support in thread's list
timelineSet={timelineSet} timelineSet={timelineSet}
showUrlPreview={false} // No URL previews at the threads list level showUrlPreview={false} // No URL previews at the threads list level
empty={<EmptyThread empty={<EmptyThread

View file

@ -329,8 +329,7 @@ export default class ThreadView extends React.Component<IProps, IState> {
<TimelinePanel <TimelinePanel
key={this.state.thread.id} key={this.state.thread.id}
ref={this.timelinePanel} ref={this.timelinePanel}
showReadReceipts={false} // Hide the read receipts showReadReceipts={true}
// until homeservers speak threads language
manageReadReceipts={true} manageReadReceipts={true}
manageReadMarkers={true} manageReadMarkers={true}
sendReadReceiptOnLoad={true} sendReadReceiptOnLoad={true}

View file

@ -30,6 +30,7 @@ import { ClientEvent } from "matrix-js-sdk/src/client";
import { Thread } from 'matrix-js-sdk/src/models/thread'; import { Thread } from 'matrix-js-sdk/src/models/thread';
import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts"; import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts";
import { MatrixError } from 'matrix-js-sdk/src/http-api'; 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 SettingsStore from "../../settings/SettingsStore";
import { Layout } from "../../settings/enums/Layout"; import { Layout } from "../../settings/enums/Layout";
@ -179,7 +180,7 @@ interface IState {
// disappearance when switching into the room. // disappearance when switching into the room.
readMarkerVisible: boolean; readMarkerVisible: boolean;
readMarkerEventId: string; readMarkerEventId: string | null;
backPaginating: boolean; backPaginating: boolean;
forwardPaginating: boolean; forwardPaginating: boolean;
@ -229,8 +230,8 @@ class TimelinePanel extends React.Component<IProps, IState> {
disableGrouping: false, disableGrouping: false,
}; };
private lastRRSentEventId: string = undefined; private lastRRSentEventId: string | null | undefined = undefined;
private lastRMSentEventId: string = undefined; private lastRMSentEventId: string | null | undefined = undefined;
private readonly messagePanel = createRef<MessagePanel>(); private readonly messagePanel = createRef<MessagePanel>();
private readonly dispatcherRef: string; private readonly dispatcherRef: string;
@ -250,7 +251,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
// XXX: we could track RM per TimelineSet rather than per Room. // XXX: we could track RM per TimelineSet rather than per Room.
// but for now we just do it per room for simplicity. // but for now we just do it per room for simplicity.
let initialReadMarker = null; let initialReadMarker: string | null = null;
if (this.props.manageReadMarkers) { if (this.props.manageReadMarkers) {
const readmarker = this.props.timelineSet.room.getAccountData('m.fully_read'); const readmarker = this.props.timelineSet.room.getAccountData('m.fully_read');
if (readmarker) { if (readmarker) {
@ -942,13 +943,13 @@ class TimelinePanel extends React.Component<IProps, IState> {
if (lastReadEventIndex === null) { if (lastReadEventIndex === null) {
shouldSendRR = false; shouldSendRR = false;
} }
let lastReadEvent = this.state.events[lastReadEventIndex]; let lastReadEvent: MatrixEvent | null = this.state.events[lastReadEventIndex ?? 0];
shouldSendRR = shouldSendRR && shouldSendRR = shouldSendRR &&
// Only send a RR if the last read event is ahead in the timeline relative to // Only send a RR if the last read event is ahead in the timeline relative to
// the current RR event. // the current RR event.
lastReadEventIndex > currentRREventIndex && lastReadEventIndex > currentRREventIndex &&
// Only send a RR if the last RR set != the one we would send // 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 // Only send a RM if the last RM sent != the one we would send
const shouldSendRM = const shouldSendRM =
@ -958,7 +959,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
// same one at the server repeatedly // same one at the server repeatedly
if (shouldSendRR || shouldSendRM) { if (shouldSendRR || shouldSendRM) {
if (shouldSendRR) { if (shouldSendRR) {
this.lastRRSentEventId = lastReadEvent.getId(); this.lastRRSentEventId = lastReadEvent?.getId();
} else { } else {
lastReadEvent = null; lastReadEvent = null;
} }
@ -974,21 +975,29 @@ class TimelinePanel extends React.Component<IProps, IState> {
`prr=${lastReadEvent?.getId()}`, `prr=${lastReadEvent?.getId()}`,
); );
MatrixClientPeg.get().setRoomReadMarkers(
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, roomId,
this.state.readMarkerEventId, this.state.readMarkerEventId ?? "",
sendRRs ? lastReadEvent : null, // Public read receipt (could be null) 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) => { ).catch(async (e) => {
// /read_markers API is not implemented on this HS, fallback to just RR // /read_markers API is not implemented on this HS, fallback to just RR
if (e.errcode === 'M_UNRECOGNIZED' && lastReadEvent) { if (e.errcode === 'M_UNRECOGNIZED' && lastReadEvent) {
if ( if (
!sendRRs !sendRRs
&& !MatrixClientPeg.get().doesServerSupportUnstableFeature("org.matrix.msc2285.stable") && !cli.doesServerSupportUnstableFeature("org.matrix.msc2285.stable")
) return; ) return;
try { try {
return await MatrixClientPeg.get().sendReadReceipt( return await cli.sendReadReceipt(
lastReadEvent, lastReadEvent,
sendRRs ? ReceiptType.Read : ReceiptType.ReadPrivate, sendRRs ? ReceiptType.Read : ReceiptType.ReadPrivate,
); );
@ -1018,6 +1027,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
}); });
} }
} }
}
}; };
// if the read marker is on the screen, we can now assume we've caught up to the end // if the read marker is on the screen, we can now assume we've caught up to the end
@ -1149,7 +1159,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
const rmId = this.getCurrentReadReceipt(); const rmId = this.getCurrentReadReceipt();
// Look up the timestamp if we can find it // 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; let rmTs: number;
if (tl) { if (tl) {
const event = tl.getEvents().find((e) => { return e.getId() == rmId; }); const event = tl.getEvents().find((e) => { return e.getId() == rmId; });
@ -1554,7 +1564,8 @@ class TimelinePanel extends React.Component<IProps, IState> {
return 0; 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 /* 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 is very tied to the main room timeline, we are forcing the timeline to
send read receipts for threaded events */ send read receipts for threaded events */
@ -1655,7 +1666,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
* SDK. * SDK.
* @return {String} the event ID * @return {String} the event ID
*/ */
private getCurrentReadReceipt(ignoreSynthesized = false): string { private getCurrentReadReceipt(ignoreSynthesized = false): string | null {
const client = MatrixClientPeg.get(); const client = MatrixClientPeg.get();
// the client can be null on logout // the client can be null on logout
if (client == null) { if (client == null) {
@ -1663,21 +1674,23 @@ class TimelinePanel extends React.Component<IProps, IState> {
} }
const myUserId = client.credentials.userId; const myUserId = client.credentials.userId;
return this.props.timelineSet.room.getEventReadUpTo(myUserId, ignoreSynthesized); const receiptStore: ReadReceipt<any, any> =
this.props.timelineSet.thread ?? this.props.timelineSet.room;
return receiptStore?.getEventReadUpTo(myUserId, ignoreSynthesized);
} }
private setReadMarker(eventId: string, eventTs: number, inhibitSetState = false): void { private setReadMarker(eventId: string | null, eventTs: number, inhibitSetState = false): void {
const roomId = this.props.timelineSet.room.roomId; const roomId = this.props.timelineSet.room?.roomId;
// don't update the state (and cause a re-render) if there is // don't update the state (and cause a re-render) if there is
// no change to the RM. // no change to the RM.
if (eventId === this.state.readMarkerEventId) { if (eventId === this.state.readMarkerEventId || eventId === null) {
return; return;
} }
// in order to later figure out if the read marker is // in order to later figure out if the read marker is
// above or below the visible timeline, we stash the timestamp. // above or below the visible timeline, we stash the timestamp.
TimelinePanel.roomReadMarkerTsMap[roomId] = eventTs; TimelinePanel.roomReadMarkerTsMap[roomId ?? ""] = eventTs;
if (inhibitSetState) { if (inhibitSetState) {
return; return;

View file

@ -1333,6 +1333,7 @@ export class UnwrappedEventTile extends React.Component<IProps, IState> {
<a href={permalink} onClick={this.onPermalinkClicked}> <a href={permalink} onClick={this.onPermalinkClicked}>
{ timestamp } { timestamp }
</a> </a>
{ msgOption }
</div>, </div>,
reactionsRow, reactionsRow,
]); ]);

View file

@ -398,12 +398,13 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
}; };
private onClearNotificationsClicked = () => { private onClearNotificationsClicked = () => {
MatrixClientPeg.get().getRooms().forEach(r => { const client = MatrixClientPeg.get();
client.getRooms().forEach(r => {
if (r.getUnreadNotificationCount() > 0) { if (r.getUnreadNotificationCount() > 0) {
const events = r.getLiveTimeline().getEvents(); const events = r.getLiveTimeline().getEvents();
if (events.length) { if (events.length) {
// noinspection JSIgnoredPromiseFromCall // noinspection JSIgnoredPromiseFromCall
MatrixClientPeg.get().sendReadReceipt(events[events.length - 1]); client.sendReadReceipt(events[events.length - 1]);
} }
} }
}); });

View file

@ -42,7 +42,7 @@ const newReceipt = (eventId: string, userId: string, readTs: number, fullyReadTs
[ReceiptType.FullyRead]: { [userId]: { ts: 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 => { const renderPanel = (room: Room, events: MatrixEvent[]): RenderResult => {
@ -154,7 +154,7 @@ describe('TimelinePanel', () => {
}); });
renderPanel(room, events); 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", () => { it("does not send public read receipt when enabled", () => {
@ -169,7 +169,7 @@ describe('TimelinePanel', () => {
}); });
renderPanel(room, events); renderPanel(room, events);
expect(client.setRoomReadMarkers).toHaveBeenCalledWith(room.roomId, null, null, events[0]); expect(client.setRoomReadMarkers).toHaveBeenCalledWith(room.roomId, "", undefined, events[0]);
}); });
}); });
}); });