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

Track e2e highlights better, particularly in 'Mentions Only' rooms #886

Merged
merged 1 commit into from
Apr 8, 2019

Conversation

turt2live
Copy link
Member

Fixes element-hq/element-web#9280

The server is unable to calculate encrypted highlights for us, so we calculate them. This also means the server always sends a zero for highlight_count, and therefore in sync.js we now trust our judgement over the server's. In future, this check will need to be altered to support server-side encrypted notifications if that happens. This fixes the part of 9280 where the badge count ends up disappearing unless the message received also happens to be a mention.

The changes in client.js are more to support rooms which are mentions only. Because the server doesn't send an unread_count for these rooms, the total notifications will always be zero. Therefore, we try and calculate that. In order to do that, we need to assume that our highlight count is also wrong and calculate it appropriately.

Fixes element-hq/element-web#9280

The server is unable to calculate encrypted highlights for us, so we calculate them. This also means the server always sends a zero for highlight_count, and therefore in sync.js we now trust our judgement over the server's. In future, this check will need to be altered to support server-side encrypted notifications if that happens. This fixes the part of 9280 where the badge count ends up disappearing unless the message received also happens to be a mention.

The changes in client.js are more to support rooms which are mentions only. Because the server doesn't send an unread_count for these rooms, the total notifications will always be zero. Therefore, we try and calculate that. In order to do that, we need to assume that our highlight count is also wrong and calculate it appropriately.
@turt2live turt2live requested a review from a team April 5, 2019 20:51
Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

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

otherwise lgtm!

src/client.js Show resolved Hide resolved
@turt2live turt2live merged commit f585c80 into develop Apr 8, 2019
@turt2live turt2live deleted the travis/e2e-notifs-2 branch April 8, 2019 13:40
turt2live added a commit that referenced this pull request Apr 8, 2019
A logic error introduced by #886 meant that all unencrypted rooms were not getting highlight notifications.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Encrypted ping count isn't a count and disappears after receiving a non-ping
2 participants