Rework notifications system to be more flexible
This is a fairly technical brainstorming issue and will talk about the internals meant to make a rework of notifications easier. After looking at the code, I'd strongly recommend doing something like this before the plethora of notification-related issues (#107 #282 #420 #550 #788 - these are the ones I could find quickly).
My personal recommendation would be for the following changes:
- Rename
CommentNotification
model toNotification
(and all related types) - this gives a single point of entry when creating a notification and allows us to rely purely on thetype
column to determine what type of notification this is - Rename
TopicIgnore
model toTopicSubscription
, add atype
column withSUBSCRIBED
orIGNORED
as possible states - essentially make some subscriptions explicit rather than implicit. A missingTopicSubscription
would only allow certain types of notifications (like mentions and replies to your comments) - Replace
CommentNotification.should_create_reply_notification
with a method to return all users which should be notified as well as if it was an explicit mention (like mentions) or implicit (like replying to a comment/topic). When sending a notification, we would get the list of users, filter out any users with a subscription state ofIGNORED
, additionally include allSUBSCRIBED
users, and send the notifications.
Part of this still needs to be figured out:
- I don't know how this would work in the UI - normally you have 2 states for toggles, but this has 3. It doesn't make sense to ignore and subscribe to the same topic, so 2 separate toggles wouldn't make sense and having a dropdown for "Default Notifications", "All Notifications" and "No Notifications" in every topic would probably be strange.
What do you think? Does this make sense? Is it worth doing?