Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve hasUserReadEvent and getUserReadUpTo realibility with threads #3031

Merged
merged 29 commits into from
Jan 11, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d1db4db
Add failing test scenario when multiple receipts are in the same event
germain-gg Jan 5, 2023
03968c3
Fix cached read receipts
germain-gg Jan 4, 2023
a776ff8
Improve hasUserReadEvent and getUserReadUpTo realibility with threads
germain-gg Jan 5, 2023
23cd620
Reword code comments and improve readibility
Jan 5, 2023
1f3760f
Optimise code paths
Jan 5, 2023
be7fb61
fix getEventReadUpTo logic with unthreaded receipts
germain-gg Jan 5, 2023
0d4da28
Re-introduce optional chaining
germain-gg Jan 5, 2023
1ccf954
fixes
germain-gg Jan 5, 2023
c24f027
mend
germain-gg Jan 6, 2023
435f485
Add tests for getEventReadUpTo and hasUserReadEvent
germain-gg Jan 6, 2023
3fe9d8a
Merge branch 'develop' into gsouquet/better-getreadupto
germain-gg Jan 6, 2023
2f8328d
Reword code comments and improve readibility
Jan 6, 2023
e634807
Add comments to methods
germain-gg Jan 6, 2023
2d147d4
Make properties private and provide accessors
germain-gg Jan 6, 2023
c9827de
Remove unwanted change
germain-gg Jan 6, 2023
d1ab2de
Improve thread spec
germain-gg Jan 6, 2023
02f5a6c
Explain the unthreaded receipt logic in comments
germain-gg Jan 6, 2023
420d0c3
Merge branch 'develop' into gsouquet/better-getreadupto
Jan 6, 2023
feb8f5e
Apply comments readibility suggestions
Jan 9, 2023
5c5cbbf
Clarify code comments based on PR feedback
germain-gg Jan 9, 2023
0ee16d3
Remove unneeded nullish coalescing check
germain-gg Jan 9, 2023
889c0b4
Merge branch 'develop' into gsouquet/better-getreadupto
Jan 9, 2023
83e0b71
Merge branch 'develop' into gsouquet/better-getreadupto
Jan 10, 2023
253427d
Improve comments wording
Jan 10, 2023
4efb717
Clarify comments
germain-gg Jan 10, 2023
d2935d8
fix tests
germain-gg Jan 10, 2023
1d88910
lint fix
germain-gg Jan 10, 2023
dae36b7
Final comment wording updates
Jan 11, 2023
64ab5a0
Merge branch 'develop' into gsouquet/better-getreadupto
Jan 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/models/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
// Useful to know at what point the current user has started using threads in this room
private oldestThreadedReceiptTs = Infinity;
/**
* Keeping a record of the lastest unthread receipts per user
* A record of the latest unthread receipts per user
* This is useful in determining whether a user has read a thread or not
*/
private unthreadedReceipts = new Map<string, Receipt>();
Expand Down Expand Up @@ -3306,7 +3306,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
}

/**
* Returns the most receipt unthreaded receipt for a given user
* Returns the most recent unthreaded receipt for a given user
* @param userId - the MxID of the User
* @returns an unthreaded Receipt
richvdh marked this conversation as resolved.
Show resolved Hide resolved
*/
Expand Down
12 changes: 8 additions & 4 deletions src/models/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,8 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
}

/**
* Get the ID of the event that a given user has read up to, or null if we
* have received no read receipts from them.
* Get the ID of the event that a given user has read up to within that thread,
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
* or null if we have received no read receipts from them.
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
* @param userId - The user ID to get read receipt event ID for
* @param ignoreSynthesized - If true, return only receipts that have been
* sent by the server, not implicit ones generated
Expand All @@ -521,7 +521,7 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
// sent in the room (suggesting that it was sent before the user started
// using a client that supported threaded read receipts), we want to
// consider this thread as read.
const beforeFirstThreadedReceipt = (lastReply.getTs() ?? 0) < this.room.getOldestThreadedReceiptTs();
const beforeFirstThreadedReceipt = lastReply.getTs() < this.room.getOldestThreadedReceiptTs();
if (beforeFirstThreadedReceipt) {
return this.timeline.at(-1)?.getId() ?? null;
richvdh marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -556,14 +556,18 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {

/**
* Determines if the given user has read a particular event ID with the known
* history of the room. This is not a definitive check as it relies only on
* history of the thread. This is not a definitive check as it relies only on
richvdh marked this conversation as resolved.
Show resolved Hide resolved
* what is available to the thread at the time of execution.
* @param userId - The user ID to check the read state of.
* @param eventId - The event ID to check if the user read.
* @returns True if the user has read the event, false otherwise.
*/
public hasUserReadEvent(userId: string, eventId: string): boolean {
if (userId === this.client.getUserId()) {
// Consider an event read if it's part of a thread that has no
// read receipts and has no notifications. It is likely that it is
richvdh marked this conversation as resolved.
Show resolved Hide resolved
// part of a thread that was created before read receipts for threads
// were supported (via MSC3771)
const beforeFirstThreadedReceipt =
(this.lastReply()?.getTs() ?? 0) < this.room.getOldestThreadedReceiptTs();
const unthreadedReceiptTs = this.room.getLastUnthreadedReceiptFor(userId)?.ts ?? 0;
Expand Down