Improve hasUserReadEvent and getUserReadUpTo realibility with threads by germain-gg · Pull Request #3031 · matrix-org/matrix-js-sdk

germain-gg · 2023-01-05T16:08:46Z

Fixes element-hq/element-web#24164
Helps solving element-hq/element-web#23907

Marks all threads before the first threaded read receipts in a room as read. deals with threads that have no read receipts by checking the room unthreaded receipt for that user.

Checklist

Tests written for new code (and old code if feasible)
Linter and other CI checks pass
Sign-off given on the changes (see CONTRIBUTING.md)

Here's what your changelog entry will look like:

✨ Features

Improve hasUserReadEvent and getUserReadUpTo realibility with threads (#3031). Fixes Unread count increments when opening the threads panel element-hq/element-web#24164.

src/models/room.ts

src/models/thread.ts

src/client.ts

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>

src/models/room.ts

src/models/thread.ts

richvdh

Seems somewhat clearer now - thanks for the updates. Still a few questions though.

src/models/thread.ts

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

germain-gg · 2023-01-09T12:41:57Z

I was struggling to understand how the read receipt information we now track in Room related to the information we already track in ReadReceipt (and, by inheritance, Thread). I still am a bit confused about that, to be honest. Do the receipts in ReadReceipt only include those related to either a particular thread, or no thread at all?

ReadReceipt (which is very ill named, and should be changed, but that won't be part of this PR) is a structure that host all the logic to have a timeline that you want to track read receipts in.
So the main room timeline, or each individual thread.

It's not related to a single read receipt, but it's more the structure to host them, and check the MatrixEvent in a timeline

src/models/thread.ts

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

src/models/room.ts

src/models/thread.ts

richvdh

LGTM if you agree with my suggestions. Thanks for your perseverance!

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Remove extensible events v1 field population on legacy events ([\matrix-org#3040](matrix-org#3040)). * Improve hasUserReadEvent and getUserReadUpTo realibility with threads ([\matrix-org#3031](matrix-org#3031)). Fixes element-hq/element-web#24164. * Remove video track when muting video ([\matrix-org#3028](matrix-org#3028)). Fixes element-hq/element-call#209. * Make poll start event type available (PSG-962) ([\matrix-org#3034](matrix-org#3034)). * Add alt event type matching in Relations model ([\matrix-org#3018](matrix-org#3018)). * Remove usage of v1 Identity Server API ([\matrix-org#3003](matrix-org#3003)). * Add `device_id` to `/account/whoami` types ([\matrix-org#3005](matrix-org#3005)). * Implement MSC3912: Relation-based redactions ([\matrix-org#2954](matrix-org#2954)). * Introduce a mechanism for using the rust-crypto-sdk ([\matrix-org#2969](matrix-org#2969)). * Support MSC3391: Account data deletion ([\matrix-org#2967](matrix-org#2967)). * Fix threaded cache receipt when event holds multiple receipts ([\matrix-org#3026](matrix-org#3026)). * Fix false key requests after verifying new device ([\matrix-org#3029](matrix-org#3029)). Fixes element-hq/element-web#24167 and element-hq/element-web#23333. * Avoid triggering decryption errors when decrypting redacted events ([\matrix-org#3004](matrix-org#3004)). Fixes element-hq/element-web#24084. * bugfix: upload OTKs in sliding sync mode ([\matrix-org#3008](matrix-org#3008)). * Apply edits discovered from sync after thread is initialised ([\matrix-org#3002](matrix-org#3002)). Fixes element-hq/element-web#23921. * Sliding sync: Fix issue where no unsubs are sent when switching rooms ([\matrix-org#2991](matrix-org#2991)). * Threads are missing from the timeline ([\matrix-org#2996](matrix-org#2996)). Fixes element-hq/element-web#24036. * Close all streams when a call ends ([\matrix-org#2992](matrix-org#2992)). Fixes element-hq/element-call#742. * Resume to-device message queue after resumed sync ([\matrix-org#2920](matrix-org#2920)). Fixes matrix-org/element-web-rageshakes#17170. * Fix browser entrypoint ([\matrix-org#3051](matrix-org#3051)). Fixes matrix-org#3013. * Fix failure to start in firefox private browser ([\matrix-org#3058](matrix-org#3058)). Fixes element-hq/element-web#24216. * Correctly handle limited sync responses by resetting the thread timeline ([\matrix-org#3056](matrix-org#3056)). Fixes element-hq/element-web#23952.

germain-gg added 3 commits January 5, 2023 09:25

Add failing test scenario when multiple receipts are in the same event

d1db4db

Fix cached read receipts

03968c3

Improve hasUserReadEvent and getUserReadUpTo realibility with threads

a776ff8

germain-gg added the T-Enhancement label Jan 5, 2023

github-actions bot deployed to PR Documentation Preview January 5, 2023 16:11 View deployment

clokep reviewed Jan 5, 2023

View reviewed changes

Germain and others added 4 commits January 5, 2023 17:24

Reword code comments and improve readibility

23cd620

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>

Optimise code paths

1f3760f

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>

fix getEventReadUpTo logic with unthreaded receipts

be7fb61

Re-introduce optional chaining

0d4da28

github-actions bot deployed to PR Documentation Preview January 5, 2023 17:36 View deployment

fixes

1ccf954

github-actions bot deployed to PR Documentation Preview January 5, 2023 17:43 View deployment

germain-gg added 2 commits January 6, 2023 09:31

mend

c24f027

Add tests for getEventReadUpTo and hasUserReadEvent

435f485

github-actions bot deployed to PR Documentation Preview January 6, 2023 10:12 View deployment

germain-gg marked this pull request as ready for review January 6, 2023 10:22

germain-gg requested a review from a team as a code owner January 6, 2023 10:22

germain-gg requested review from kerryarchibald and richvdh and removed request for a team January 6, 2023 10:22

germain-gg changed the base branch from gsouquet/fix-cached-rr to develop January 6, 2023 10:24

This was referenced Jan 6, 2023

Always mark legacy threaded event as read #3020

Closed

Display rooms & threads as unread (bold) if threads have unread messages. matrix-org/matrix-react-sdk#9763

Merged

github-actions bot deployed to PR Documentation Preview January 6, 2023 10:25 View deployment

germain-gg marked this pull request as draft January 6, 2023 10:31

germain-gg removed request for kerryarchibald and richvdh January 6, 2023 10:32

germain-gg force-pushed the gsouquet/better-getreadupto branch from cf86e72 to 1779fbb Compare January 6, 2023 10:34

github-actions bot deployed to PR Documentation Preview January 6, 2023 10:36 View deployment

richvdh reviewed Jan 9, 2023

View reviewed changes

richvdh requested changes Jan 9, 2023

View reviewed changes

src/models/thread.ts Outdated Show resolved Hide resolved

Apply comments readibility suggestions

feb8f5e

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

github-actions bot deployed to PR Documentation Preview January 9, 2023 12:28 View deployment

germain-gg and others added 3 commits January 9, 2023 12:30

Clarify code comments based on PR feedback

5c5cbbf

Remove unneeded nullish coalescing check

0ee16d3

Merge branch 'develop' into gsouquet/better-getreadupto

889c0b4

github-actions bot deployed to PR Documentation Preview January 9, 2023 12:33 View deployment

germain-gg requested a review from richvdh January 9, 2023 12:39

richvdh requested changes Jan 10, 2023

View reviewed changes

src/models/thread.ts Outdated Show resolved Hide resolved

src/models/thread.ts Outdated Show resolved Hide resolved

src/models/thread.ts Outdated Show resolved Hide resolved

src/models/thread.ts Outdated Show resolved Hide resolved

Germain and others added 2 commits January 10, 2023 13:50

Merge branch 'develop' into gsouquet/better-getreadupto

83e0b71

Improve comments wording

253427d

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

github-actions bot deployed to PR Documentation Preview January 10, 2023 13:53 View deployment

Clarify comments

4efb717

germain-gg requested a review from richvdh January 10, 2023 14:03

github-actions bot deployed to PR Documentation Preview January 10, 2023 14:05 View deployment

germain-gg added 2 commits January 10, 2023 14:31

fix tests

d2935d8

lint fix

1d88910

github-actions bot deployed to PR Documentation Preview January 10, 2023 14:50 View deployment

richvdh reviewed Jan 11, 2023

View reviewed changes

src/models/room.ts Show resolved Hide resolved

src/models/room.ts Show resolved Hide resolved

src/models/thread.ts Outdated Show resolved Hide resolved

src/models/thread.ts Outdated Show resolved Hide resolved

richvdh approved these changes Jan 11, 2023

View reviewed changes

Germain and others added 2 commits January 11, 2023 09:42

Final comment wording updates

dae36b7

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

Merge branch 'develop' into gsouquet/better-getreadupto

64ab5a0

github-actions bot deployed to PR Documentation Preview January 11, 2023 09:44 View deployment

germain-gg merged commit 8e29f8e into develop Jan 11, 2023

germain-gg deleted the gsouquet/better-getreadupto branch January 11, 2023 09:53

This was referenced Jan 11, 2023

Fix mock client missing user functions matrix-org/matrix-react-sdk#9887

Merged

Stuck notifications: DM stuck as unread element-hq/element-web#23991

Closed

Uh oh!

Conversation

germain-gg commented Jan 5, 2023 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

✨ Features

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

richvdh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

germain-gg commented Jan 9, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

richvdh left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

germain-gg commented Jan 5, 2023 •

edited by github-actions bot

Loading