This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Implement unread counter (MSC2625) #7673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ations This is a potential solution to https://github.com/vector-im/riot-web/issues/3374 and element-hq/element-web#5953 as raised by Mozilla at element-hq/element-web#10868. This lets you define a push rule action which increases the badge count (unread notification) count on a given room, but doesn't actually send a push for that notification via email or HTTP. We might want to define this as the default behaviour for group chats in future to solve https://github.com/vector-im/riot-web/issues/3268 at last. This is implemented as a string action rather than a tweak because: * Other pushers don't care about the tweak, given they won't ever get pushed * The DB can store the tweak more efficiently using the existing `notify` table. * It avoids breaking the default_notif/highlight_action optimisations. Clients which generate their own notifs (e.g. desktop notifs from Riot/Web would need to be aware of the new push action) to uphold it. An alternative way to do this would be to maintain a `msg_count` alongside `highlight_count` and `notification_count` in `unread_notifications` in sync responses. However, doing this by counting the rows in `events` since the `stream_position` of the user's last read receipt turns out to be painfully slow (~200ms), perhaps due to the size of the events table. So instead, we use the highly optimised existing event_push_actions (and event_push_actions_staging) table to maintain the counts - using the code paths which already exist for tracking unread notification counts efficiently. These queries are typically ~3ms or so. The biggest issues I see here are: * We're slightly repurposing the `notif` field on `event_push_actions` to track whether a given action actually sent a `push` or not. This doesn't seem unreasonable, but it's slightly naughty given that previously the field explicitly tracked whether `notify` was true for the action (and as a result, it was uselessly always set to 1 in the DB). * We're going to put more load on the `event_push_actions` table for all the random group chats which people had previously muted. In practice i don't think there are many of these though. * There isn't an MSC for this yet (although this comment could become one).
deepbluev7
reviewed
Jun 10, 2020
richvdh
reviewed
Jun 11, 2020
richvdh
previously requested changes
Jun 12, 2020
synapse/storage/data_stores/main/schema/delta/59/00push_summary_unread_count.sql
Outdated
Show resolved
Hide resolved
richvdh
reviewed
Jun 12, 2020
richvdh
approved these changes
Jun 15, 2020
|
I've resolved the conflict, I'm looking into adding support in Sytest. |
|
Sytest PR: matrix-org/sytest#890 |
babolivier
added a commit
to matrix-org/sytest
that referenced
this pull request
Jun 17, 2020
Test for matrix-org/synapse#7673 This is basically hacking `Messages that notify from another user increment unread notification count` so it also runs for the action specified in MSC2625.
babolivier
added a commit
that referenced
this pull request
Jun 23, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #7667
Implements matrix-org/matrix-spec-proposals#2625
TODO: change the following tests to test that amark_unreadaction increments correctly the unread count:tests.storage.test_event_push_actions.EventPushActionsStoreTestCase.test_count_aggregationtests.replication.slave.storage.test_events.SlavedEventStoreTestCase.test_push_actions_for_user