Skip to content

Conversation

@germain-gg
Copy link
Contributor

@germain-gg germain-gg commented Jan 4, 2023

Needed for matrix-org/matrix-react-sdk#9763
Will help element-hq/element-web#23907

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

  • Always mark legacy threaded event as read (#3020).

Comment on lines 517 to 521

// We consider all threaded events read if they are part of a thread
// that has no activity since the first ever threaded event recorded in that room
// This prevents rooms to generated unwanted notifications for threads
// created before MSC3771
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm finding this comment very difficult to read, maybe the following is a bit clearer:

Suggested change
// We consider all threaded events read if they are part of a thread
// that has no activity since the first ever threaded event recorded in that room
// This prevents rooms to generated unwanted notifications for threads
// created before MSC3771
// We consider all events in a thread as read if the thread has not had
// activity since the first ever threaded receipt recorded in that room.
// This prevents rooms generating unwanted notifications for threads
// created before MSC3771.

Comment on lines +2728 to +2731

// Some threads were created before MSC3771 landed. Those threads
// do not have read receipts, and this will be problematic in encrypted
// rooms where clients rely on receipts to compute highlight notifications
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually necessary for more than the "bold" case of thread activity?

@germain-gg
Copy link
Contributor Author

Replaced by #3031

@germain-gg germain-gg closed this Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants