Notifications: Design de-duplication

VERIFIED FIXED in 2.5

Status

support.mozilla.org
General
P1
normal
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: erik, Assigned: erik)

Tracking

unspecified

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

8 years ago
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.
(Assignee)

Updated

8 years ago
Blocks: 623641
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?
(Assignee)

Updated

7 years ago
Assignee: nobody → erik
(Assignee)

Comment 2

7 years ago
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.
(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 :)
(Assignee)

Comment 4

7 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Comment 5

7 years ago
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.)
(Assignee)

Comment 6

7 years ago
nm; moved that to bug 629515
Verified received single notification for an email address used by a registered user and also as an anonymous sign up
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.