626958 - Notifications: Design de-duplication

Status: VERIFIED FIXED

(support.mozilla.org :: General, defect, P1)

Description

[Erik Rose [:erik][:erikrose]]

Think about, decide whether to implement, and (if yes) implement de-duplication on email addresses at some level. Perhaps we will have to put all notifications into a big per-request bucket and de-dupe and send them only when the request ends. Perhaps de-duping on the level of a single Event type is enough. Figure it out, and do it.

Comment 1

[James Socol [:jsocol, :james]]

We should de-dupe messages whenever they'd be the same content (is that the single Event type level?)

Maybe we could batch events into a request-level queue and have some process_request middleware that actually fired them off together into a celery task to work all of that out?

Comment 2

[Erik Rose [:erik][:erikrose]]

Yes, content will typically be per event type. Because of this and because we decided to go with explicit firing of events, de-duping per call to __users_watching_by_filter() should suffice. We can always do something crazier like the per-request approach if we ever find implicit firing a win.

Comment 3

[Paul Craciunoiu [:paulc]]

(In reply to comment #2)
> if we ever find implicit firing a win.
... which looks like a no since we may have double-firing going on* and dealing with that seems painful.

* Example:
A forum's last_post is changed when new post is added, but someone watching both the thread and the forum to which the post belongs would probably get 2 notifications, one per fire.

Just wanted to write that down so it's there if we ever consider implicit firing. PEP 20 prefers explicit anyway :)

Comment 4

[Erik Rose [:erik][:erikrose]]

Hit the integration branch in https://github.com/jsocol/kitsune/commit/5bf840f.

Once the "notifications" integration branch is merged, this can be QA'd thus: watch the same thing as a logged in user and anonymously with the same email address. Then cause the emails to be sent. You should receive only one.

Comment 5

[Erik Rose [:erik][:erikrose]]

Brain dump of a possible "union" operator for firing multiple events together and de-duplicating the results:

union(ThreadEvent(post), ForumEvent(post)).fire()  # Fire the first arg, passing to it the union of .users_watching() on all args. Use this when the content of each arg's template is essentially the same, e.g. "This new post was made. Enjoy."—when the receipt of a mail from the 2nd template as well would add no value. (The fact that there's an unsubscribe link to only the first event's watch is acceptable. It's a convenience only, and they can always visit their user dashboard.) If the 2nd one would add value, you should probably fire both events independently and let both mails be delivered. {Or is fire_union(...) better?) (To implement, have union return an instance of a special class which has state which can hold the args, which can get pickled.)

Comment 6

[Erik Rose [:erik][:erikrose]]

nm; moved that to bug 629515

Comment 7

[Rebecca Billings [:rbillings]]

Verified received single notification for an email address used by a registered user and also as an anonymous sign up