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

Feature: Email Notifications #9

Open
NunoDasNeves opened this issue Feb 27, 2019 · 4 comments
Open

Feature: Email Notifications #9

NunoDasNeves opened this issue Feb 27, 2019 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@NunoDasNeves
Copy link
Contributor

Feature
A basic MVP email notification system would provide groundwork for a richer site experience and higher user engagement.
Email notifications draw existing users back to the site.
Infrastructure required for email notifications can be used for on-site popup notifications, notification feed, notification options including opt-in/opt-outs, subscribe (or 'watch') to a thread etc...

For MVP I propose default email notifications for the following events:

  • User answered your question
  • User commented on your review
  • User posted a comment/answer in a thread you commented/answered in

With a single option in the Profile view to turn off all email notifications (and probably a link in each email to unsubscribe which has the same functionality.)

Implementation
This design is primarily based off this quora answer.

We need several new tables:

  • A subscriptions table that associates user IDs with objects that can be subscribed to - for MVP this would be questions and reviews.
  • A queue table whose ORM implements a notification queue with 'push' and 'pop' operations (possibly a bit more granular than this...)
  • A settings table for storing notification preferences and other settings for each user. Pros and cons of different schemas for this are discussed here. I think the 'property bag' method is the way to go - aka a row for each setting for each user (max_num_rows = num_users * num_settings). We'll probably be changing the notification settings a lot, so using this method keeps the system flexible. In order to make best use of this we'll need well-defined defaults for each setting, so a missing association between a user and a given setting means 'use the default'.

On posting a new comment, the subscriptions table is queried for all users subscribing to the relevant question/review, then notifications for each user (including all information needed to send the notification) would be pushed to the queue table.

An email daemon (a separate nodejs process) periodically 'pop's notifications off the queue table and sends out emails. (The daemon would first check the settings table for the user)

I'm not sure what the best way to implement the in-email 'unsubscribe' button is - maybe there should be an 'unsubscribe' token in the database somewhere (user table?) which is part of the 'unsubscribe' link and clicking it sends the token, which gets checked by the backend for verification...?

@NunoDasNeves NunoDasNeves added the enhancement New feature or request label Feb 27, 2019
@NunoDasNeves NunoDasNeves self-assigned this Feb 27, 2019
@DarkPurple141
Copy link
Contributor

Will discuss on this in a bit, but mostly sounds good.

@DarkPurple141
Copy link
Contributor

I'm not sure what the best way to implement the in-email 'unsubscribe' button is - maybe there should be an 'unsubscribe' token in the database somewhere (user table?) which is part of the 'unsubscribe' link and clicking it sends the token, which gets checked by the backend for verification...?

Not critical but we could generate something like that for each user. Given it would only unsubscribe them it doesn't need to be crazy secure.

Q. Would settings apply to all notifications or just a single thread? Eg. muting a thread doesn't mean I would necessarily not want to get any notifications. Would that user then be popped off the relevant subscription?

Q. Wondering whether the queue table is directly relevant; surely certain events would trigger a notification, and with these events any subscribers would be notified -- or is ur feeling here that if it's dealt with in the POST call it adds additional work to the server thread. Eg. posting a comment takes longer because it also deals with setting up another HTTP request.

Not sure if I see the value of the settings table, except for extensibility.

@DarkPurple141 DarkPurple141 self-assigned this Mar 10, 2019
@NunoDasNeves
Copy link
Contributor Author

NunoDasNeves commented Mar 12, 2019

I'm not sure what the best way to implement the in-email 'unsubscribe' button is - maybe there should be an 'unsubscribe' token in the database somewhere (user table?) which is part of the 'unsubscribe' link and clicking it sends the token, which gets checked by the backend for verification...?

Not critical but we could generate something like that for each user. Given it would only unsubscribe them it doesn't need to be crazy secure.

Agreed

Q. Would settings apply to all notifications or just a single thread? Eg. muting a thread doesn't mean I would necessarily not want to get any notifications. Would that user then be popped off the relevant subscription?

A row in subscriptions is just an association between a user and thread. Muting a thread would just delete that row.
A row in settings describes (for example) whether to receive any (email) notifications at all - this might mean that notifications for that user never get added to the queue, but more likely we'll queue them anyway because on-site notifications will be a thing (that you can't turn off, probably).

Q. Wondering whether the queue table is directly relevant; surely certain events would trigger a notification, and with these events any subscribers would be notified -- or is ur feeling here that if it's dealt with in the POST call it adds additional work to the server thread. Eg. posting a comment takes longer because it also deals with setting up another HTTP request.

Yes, I think it's too much work for the server thread. Think about what happens when you as a user have to wait for 100 emails to fire off because you posted a comment in a popular thread.
We should do something similar to refactor likes (or even use the notification queue for likes).

Not sure if I see the value of the settings table, except for extensibility.

Weeelll, the strength of this approach is that we don't need to do database migrations/schema modifications in order to add new settings to the table - just update the code and the same table handles any new settings.

@DarkPurple141
Copy link
Contributor

Cool yeah that all sounds fair enough. Github being meta at the same time.

Screen Shot 2019-03-13 at 9 57 00 am

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants