Skip to content

Commit

Permalink
Read receipts for threads (#9239)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
Germain committed Sep 21, 2022
1 parent fa2ec7f commit 71cf9bf
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 67 deletions.
11 changes: 9 additions & 2 deletions src/components/structures/MessagePanel.tsx
View file
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -826,8 +828,13 @@ export default class MessagePanel extends React.Component<IProps, IState> {
if (!room) {
return null;
}

const receiptDestination: ReadReceipt<string, ListenerMap<string>> = 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) ||
Expand Down
8 changes: 4 additions & 4 deletions src/components/structures/ThreadPanel.tsx
View file
Expand Up @@ -282,10 +282,10 @@ const ThreadPanel: React.FC<IProps> = ({
? <TimelinePanel
key={timelineSet.getFilter()?.filterId ?? (roomId + ":" + filterOption)}
ref={timelinePanel}
showReadReceipts={false} // No RR support in thread's MVP
manageReadReceipts={false} // No RR support in thread's MVP
manageReadMarkers={false} // No RM support in thread's MVP
sendReadReceiptOnLoad={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 list
manageReadMarkers={false} // No RM support in thread's list
sendReadReceiptOnLoad={false} // No RR support in thread's list
timelineSet={timelineSet}
showUrlPreview={false} // No URL previews at the threads list level
empty={<EmptyThread
Expand Down
3 changes: 1 addition & 2 deletions src/components/structures/ThreadView.tsx
View file
Expand Up @@ -329,8 +329,7 @@ export default class ThreadView extends React.Component<IProps, IState> {
<TimelinePanel
key={this.state.thread.id}
ref={this.timelinePanel}
showReadReceipts={false} // Hide the read receipts
// until homeservers speak threads language
showReadReceipts={true}
manageReadReceipts={true}
manageReadMarkers={true}
sendReadReceiptOnLoad={true}
Expand Down
121 changes: 67 additions & 54 deletions src/components/structures/TimelinePanel.tsx
View file
Expand Up @@ -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";
Expand Down Expand Up @@ -179,7 +180,7 @@ interface IState {
// disappearance when switching into the room.
readMarkerVisible: boolean;

readMarkerEventId: string;
readMarkerEventId: string | null;

backPaginating: boolean;
forwardPaginating: boolean;
Expand Down Expand Up @@ -229,8 +230,8 @@ class TimelinePanel extends React.Component<IProps, IState> {
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<MessagePanel>();
private readonly dispatcherRef: string;
Expand All @@ -250,7 +251,7 @@ class TimelinePanel extends React.Component<IProps, IState> {

// 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) {
Expand Down Expand Up @@ -942,13 +943,13 @@ class TimelinePanel extends React.Component<IProps, IState> {
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 =
Expand All @@ -958,7 +959,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
// same one at the server repeatedly
if (shouldSendRR || shouldSendRM) {
if (shouldSendRR) {
this.lastRRSentEventId = lastReadEvent.getId();
this.lastRRSentEventId = lastReadEvent?.getId();
} else {
lastReadEvent = null;
}
Expand All @@ -974,48 +975,57 @@ class TimelinePanel extends React.Component<IProps, IState> {
`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,
});
}
}
}
};
Expand Down Expand Up @@ -1149,7 +1159,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
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; });
Expand Down Expand Up @@ -1554,7 +1564,8 @@ class TimelinePanel extends React.Component<IProps, IState> {
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 */
Expand Down Expand Up @@ -1655,29 +1666,31 @@ class TimelinePanel extends React.Component<IProps, IState> {
* 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) {
return null;
}

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 {
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;
Expand Down
1 change: 1 addition & 0 deletions src/components/views/rooms/EventTile.tsx
View file
Expand Up @@ -1333,6 +1333,7 @@ export class UnwrappedEventTile extends React.Component<IProps, IState> {
<a href={permalink} onClick={this.onPermalinkClicked}>
{ timestamp }
</a>
{ msgOption }
</div>,
reactionsRow,
]);
Expand Down
5 changes: 3 additions & 2 deletions src/components/views/settings/Notifications.tsx
View file
Expand Up @@ -398,12 +398,13 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
};

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]);
}
}
});
Expand Down
6 changes: 3 additions & 3 deletions test/components/structures/TimelinePanel-test.tsx
View file
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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", () => {
Expand All @@ -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]);
});
});
});

0 comments on commit 71cf9bf

Please sign in to comment.