Send unread notification counts #456

dbkr · 2015-12-22T17:57:25Z

Store each event that has push actions for a user & profile_tag in a table
Pull out of that table at sync time to give the number of unread notifications

…sn't store them yet. Not very efficient.

…pdu, otherwise it will be triggered whenever we backfill too.

…ion required: much as it would be nice to be able to tell between the event not having been processed and there being no notification for it, this isn't worth filling up the table with ['dont_notify'] I think. Consequently treat the empty actions array as dont_notify and filter dont_notify out of the result.

…object on sync (only actually corrrect in a full sync: hardcoded to 0 in incremental syncs).

… incremental syncs too, where necessary, and fix silly bugs like only select the event actions for that user...

… by one, but does far fewer db queries to fetch the rules

…file_tag

NegativeMjark · 2015-12-23T13:36:57Z

synapse/storage/event_actions.py

@@ -0,0 +1,98 @@
+# -*- coding: utf-8 -*-
+# Copyright 2014 OpenMarket Ltd


Copyright year should be 2015

dbkr · 2016-01-04T15:54:06Z

ptal

dbkr · 2016-01-04T16:22:55Z

Another thought: should I be running the event through _filter_events_for_client? If so, what would be an efficient way to do this given this function seems to do a db hit to fetch the state?

NegativeMjark · 2016-01-04T18:03:41Z

I tried running sytest using postgres and got an error:

File "/src/github/matrix-org/synapse/synapse/storage/event_push_actions.py", line 83, in _get_unread_event_push_actions_by_room

ProgrammingError: operator does not exist: bigint == integer
LINE 1: ...cal_ordering > 6       OR (e.topological_ordering == 6 AND e...

NegativeMjark · 2016-01-05T10:32:06Z

We probably do want to run the event through _filter_events_for_client. If we already have a copy of the state then we should probably use that rather than requesting a new copy for each user in the room.

… or not. Use this so we can run _filter_events_for_client when calculating event_push_actions.

…uid -> password hash

dbkr · 2016-01-06T18:31:46Z

ptal. This, I believe, correctly runs _filter... (and also processes redactions correctly). The former means more db queries since we need to fetch state and whether users are guests. These are ripe for optimisation but unsure whether doing so up front would be premature.

NegativeMjark · 2016-01-07T13:25:57Z

LGTM other than the potential fun for guest users since they can't set read receipts.

dbkr · 2016-01-07T15:00:25Z

Good point. With this code, guest users will just never be able to clear their notifications I think. If we plan to continue not allowing guest users to send read receipts, I'll have to work around this somehow.

dbkr · 2016-01-07T15:25:59Z

Conclusion is that we need to see read receipts from guests anyway, so no need for us to work around this. However this means we're probably blocked behind guest RRs being enabled.

NegativeMjark · 2016-01-08T13:58:28Z

I'm happy for this to land, since the guest access is orthogonal. It's not like the clients have to read the notification count fields.

dbkr · 2016-01-08T14:47:09Z

This is a very good point.

Send unread notification counts

dbkr added 18 commits December 10, 2015 16:26

Very first cut of calculating actions for events as they come in. Doe…

21f135b

…sn't store them yet. Not very efficient.

Having consulted The Erikle, this should go at the end of on_receive_…

a84a693

…pdu, otherwise it will be triggered whenever we backfill too.

Save event actions to the db

aa667ee

still very WIP, but now sends unread_notifications_count in the room …

42ad49f

…object on sync (only actually corrrect in a full sync: hardcoded to 0 in incremental syncs).

Make unread notification count sending work: put the correct count in…

413d0d6

… incremental syncs too, where necessary, and fix silly bugs like only select the event actions for that user...

add list of things I want to fix with this branch

b131fb1

pep8

091c545

Only run pushers for users on this hs!

f73f154

Merge remote-tracking branch 'origin/develop' into store_event_actions

c061b47

Add bulk push rule evaluator which actually still evaluates rules one…

65c451c

… by one, but does far fewer db queries to fetch the rules

clarify problems

77f0685

Insert push actions in a single db query rather than one per user/pro…

4c8f6a7

…file_tag

Remove the list of problems (moved to jira issues)

3fbb031

Add some comments to areas that could be optimised.

5645d97

Merge remote-tracking branch 'origin/develop' into store_event_actions

140a50f

pep8 & unused variable

9b4cd0c

Add mocks to make tests work again

d79e90f

dbkr assigned NegativeMjark Dec 22, 2015

This was referenced Dec 22, 2015

Propagate the unread count from sync onto rooms matrix-org/matrix-js-sdk#58

Merged

Set room highlight from unread_notification_count matrix-org/matrix-react-sdk#68

Merged

Fix merge fail with anon access stuff

d2a92c6

NegativeMjark reviewed Dec 23, 2015
View reviewed changes

dbkr added 5 commits January 4, 2016 14:05

Rename event-actions to event_push_actions as per PR request

c914d67

fix tests

92a1e74

Add unread_notif_count in incremental_sync_with_gap

f1b6773

comma

d74c6ac

Only joined rooms have unread_notif_count

c77e7e6

= not == in sql

4eb7b95

dbkr added 9 commits January 5, 2016 13:32

comment typo

85ca8cb

Merge remote-tracking branch 'origin/develop' into store_event_actions

eb03625

Add is_guest flag to users db to track whether a user is a guest user…

c79f221

… or not. Use this so we can run _filter_events_for_client when calculating event_push_actions.

Add schema change file for is_guest flag

ae1262a

Delete notifications for redacted events

9929283

fix tests

0e48f7f

Adding is_guest here won't work because it just constructs a dict of …

b6a5853

…uid -> password hash

Merge remote-tracking branch 'origin/develop' into store_event_actions

442fcc0

comma style

09dc985

dbkr added 2 commits January 7, 2016 10:02

more commas

823b679

This comma is actually important

daadcf3

dbkr added a commit that referenced this pull request Jan 8, 2016

Merge pull request #456 from matrix-org/store_event_actions

c232780

Send unread notification counts

dbkr merged commit c232780 into develop Jan 8, 2016

richvdh deleted the store_event_actions branch December 1, 2016 14:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send unread notification counts #456

Send unread notification counts #456

dbkr commented Dec 22, 2015

NegativeMjark Dec 23, 2015

dbkr commented Jan 4, 2016

dbkr commented Jan 4, 2016

NegativeMjark commented Jan 4, 2016

NegativeMjark commented Jan 5, 2016

dbkr commented Jan 6, 2016

NegativeMjark commented Jan 7, 2016

dbkr commented Jan 7, 2016

dbkr commented Jan 7, 2016

NegativeMjark commented Jan 8, 2016

dbkr commented Jan 8, 2016

		@@ -0,0 +1,98 @@
		# -- coding: utf-8 --
		# Copyright 2014 OpenMarket Ltd

Send unread notification counts #456

Send unread notification counts #456

Conversation

dbkr commented Dec 22, 2015

NegativeMjark Dec 23, 2015

Choose a reason for hiding this comment

dbkr commented Jan 4, 2016

dbkr commented Jan 4, 2016

NegativeMjark commented Jan 4, 2016

NegativeMjark commented Jan 5, 2016

dbkr commented Jan 6, 2016

NegativeMjark commented Jan 7, 2016

dbkr commented Jan 7, 2016

dbkr commented Jan 7, 2016

NegativeMjark commented Jan 8, 2016

dbkr commented Jan 8, 2016