Skip to content

Commit

Permalink
Only add local receipt if it's after existing receipt
Browse files Browse the repository at this point in the history
  • Loading branch information
andybalaam committed May 25, 2023
1 parent 384a3c6 commit 37bb4c2
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 1 deletion.
40 changes: 40 additions & 0 deletions spec/unit/models/thread.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,46 @@ describe("Thread", () => {
expect(thread.getReadReceiptForUserId(userId, true)).toBeNull();
});

it("Doesn't create a local echo receipt for events before an existing receipt", async () => {
// Assumption: no server side support because if we have it, events
// can only be added to the timeline after the thread has been
// initialised, and we are not properly initialising it here.
expect(Thread.hasServerSideSupport).toBe(FeatureSupport.None);

const client = createClientWithEventMapper();
const userId = "user1";
const room = new Room("room1", client, userId);

// Given a thread
const { thread } = mkThread({
room,
client,
authorId: userId,
participantUserIds: [],
ts: 200, // Timestamp is after the event we add in a second
});
// Sanity: the current receipt is for the thread root
expect(thread.getReadReceiptForUserId(userId)?.eventId).toEqual(thread.rootEvent?.getId());

const awaitTimelineEvent = new Promise<void>((res) => thread.on(RoomEvent.Timeline, () => res()));

// When we add a message that is before the latest receipt
const message = makeThreadEvent({
event: true,
rootEventId: thread.id,
replyToEventId: thread.id,
user: userId,
room: room.roomId,
ts: 100,
});
await thread.addEvent(message, false, true);
await awaitTimelineEvent;

// Then no receipt was added to the thread (the receipt is still for
// the thread root).
expect(thread.getReadReceiptForUserId(userId)?.eventId).toEqual(thread.rootEvent?.getId());
});

function createClientWithEventMapper(): MatrixClient {
const client = mock(MatrixClient, "MatrixClient");
client.reEmitter = mock(ReEmitter, "ReEmitter");
Expand Down
24 changes: 23 additions & 1 deletion src/models/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,33 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
): void => {
// Add a synthesized receipt when paginating forward in the timeline
if (!toStartOfTimeline) {
room!.addLocalEchoReceipt(event.getSender()!, event, ReceiptType.Read);
const sender = event.getSender();
if (sender && room && this.shouldSendLocalEchoReceipt(sender, event)) {
room.addLocalEchoReceipt(sender, event, ReceiptType.Read);
}
}
this.onEcho(event, toStartOfTimeline ?? false);
};

private shouldSendLocalEchoReceipt(sender: string, event: MatrixEvent): boolean {
const recursionSupport = this.client.canSupport.get(Feature.RelationsRecursion) ?? ServerSupport.Unsupported;

if (recursionSupport === ServerSupport.Unsupported) {
// Normally we add a local receipt, but if we don't have
// recursion support, then events may arrive out of order, so we
// only create a receipt if it's after our existing receipt.
const oldReceiptEventId = this.getReadReceiptForUserId(sender)?.eventId;
if (oldReceiptEventId) {
const receiptEvent = this.findEventById(oldReceiptEventId);
if (receiptEvent && receiptEvent.getTs() > event.getTs()) {
return false;
}
}
}

return true;
}

private onLocalEcho = (event: MatrixEvent): void => {
this.onEcho(event, false);
};
Expand Down

0 comments on commit 37bb4c2

Please sign in to comment.