Better unread statistics #521
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
Room::isEventNotable has been moved out from Room::Private and made
compliant with MSC2654.
The concept of Room::checkForNotifications is taken from Quaternion
where a method with the same name has been in QuaternionRoom for a long
time - however, actual body is a stub for now, always returning
{ Notification::None } (Quaternion's implementation is too crude to be
taken to the library). Now we really need a pushrules processor to fill
this method with something reasonably good. Internally the library now
calls checkForNotifications() on every event added to the timeline,
filling up the events-to-notifications map because it is anticipated
that calculation of notifications can be rather resource-intensive and
should only be done once for a given event.
Finally, Room::notificationsFor is an accessor into the mentioned map,
standing next to isEventNotable (but unlike isEventNotable, it's not
virtual; checkForNotifications is).
Since MSC2654's unread count is counted from the m.read receipt, and the course is to follow the spec's terminology and use "unread count" for the number of notable events since m.read, this required to move the existing number of notable events since m.fully_read to another field, henceforth called partiallyReadCount. At the same time, SyncData::notificationCount is dropped completely since MSC2654 claims to supersede it. Also: Room::resetNotificationCount() and Room::resetHighlightCount() are deprecated, as these never worked properly overwriting values that can be calculated or sourced from the server, only for these values to be set back again the next time the room is updated from /sync.
This makes updating display name and emission of necessary signals including Room::changed() more systematic when it has to occur outside of updateData() flow - e.g. when loading all members.
This introduces a new API to count unread events that would allow to obtain those unread and highlight counts since either the fully read marker (Room::partiallyReadStats) or the last read receipt (Room::unreadStats). Element uses the read receipt as the anchor to count unread numbers, while Quaternion historically used the fully read marker for that (with the pre-0.7 library sticking the two markers to each other). From now on the meaning of "unread" in Quotient is aligned with that of the spec and Element, and "partially read" means events between the fully read marker and the local read receipt; the design allows client authors to use either or both counting strategies as they see fit. Respectively, Room::P::setFullyReadMarker() updates partially-read statistics, while Room::P::setLastReadReceipt(), when called on a local user, updates unread statistics. Room::notificationCount() and Room::highlightCount() maintain their previous meaning as the counters since the last read receipt; Room::notificationCount() counts unread events locally, falling back to the value from the above-mentioned key defined by MSC2654, and if that is not there, further to `unread_notifications/notification_count` defined in the current spec. Room::highlightCount(), however, is still taken from the homeserver, not from Room::unreadStats().highlightCount.
KitsuneRal
force-pushed
the
kitsune-unread-statistics
branch
from
dd4b266
to
59ab0d5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This makes it possible to count unread events and lays down the framework to do the same for highlights - both since
m.read(along the lines of MSC2654) and sincem.fully_read.Room::isEventNotable()andRoom::checkForNotifications()are virtual, allowing to extend and even override the behaviour.Room::partiallyReadStats()andRoom::unreadStats()return values of a newly introducedEventStatsclass that is supposed to be suitable for consumption in QML.A nice side effect of this work is making
Room::changed()signal more systematic, with the intention to provide client room list models with a single point to connect to in order to refresh on room changes. Previously one had to connect to the whole array of signals, ultimately to onlyemit dataChanged()to refresh a line item.Closes #204. Closes #516.