Questions about the `notifications` app

[ormsbee]

@saadyousafarbi, @asadazam93, @AhtishamShahid, @muhammadadeeltajamul: I came across the notifications app recently because of the ORM warning, and looked around for a bit. I had a number of questions/concerns:

1. Can it use openedx-events instead of pulling directly from models? It's currently listening for changes to the CourseEnrollment model directly, but it should probably be made to use the public event signals defined for enrollments instead. (Though in the case of that particular handler, it doesn't look like it's necessary at all? Because it's only creating the NotificationPreference that's auto-generated by API endpoint calls anyway?
2. What is the extensibility plan for this? It looks like this is being built in a way that the notifications app needs to be aware of all the things that we're going to send notifications about. So for instance, the app controlling enrollments isn't deciding to send a notification–the notifications app is listening for enrollment events and deciding to send that to the user. But won't we want to send notifications from apps that are in plugins, or other external repos like enterprise?
3. Does it make sense to put this into a separate repo, or maybe openedx-events? This largely depends on the answer to (2), but having it outside of edx-platform would make it easier to use from plugins.
4. *What is the purpose of the `get_` methods on the models?** This is much less important than the other questions, but why do the models have getter methods? It's not conventional for models, and it doesn't actually prevent mutation of the model fields since they're still publicly exposed.

I get that this is still in the early stages, and that much of this code might be proof-of-concept or scaffolding. But I'm not clear on the long term technical plan for this app, and the one wiki entry I'm aware of is on 2U's wiki and isn't public.

Thank you.

[asadazam93]

@ormsbee To answer your questions in order.

1. I think this is a good suggestion and we will work on it. Just one point the NotificationPreference is not auto generated. It is only generated when a user is enrolled. For all existing active enrolments we intend to run a management command.
2. Right now we only want to support notification for core platform features like discussions, grading etc. Currently there is no plan to extend this for plugins. We intend to create notification preferences for each user and want to be able to control those preferences. From an implementation point of view we intend to create a BaseClass for a notification that different apps can extend and generate notifications. The notification types and notification preferences will be saved in the notifications app itself.
3. We do not have any use case in mind that requires this to be implemented in a separate repo.
4. There is no purpose, we intend to remove those. I hope that answers your questions, let me know if there are any confusions. I would like to provide more context and I think that might help you understand the intent of the feature. I will share a public version of our spec doc when it is ready.

[ormsbee]

Just one point the NotificationPreference is not auto generated. It is only generated when a user is enrolled. For all existing active enrolments we intend to run a management command.

Ah, ok. So you're going to generate them during enrollment for new entries going forward, and backfill for older enrollments via management command. When i said "auto-generated", I was thinking about how a GET request to the UserNotificationPreferenceView will lazily get or create the NotificationPreference the notification preferences for that user.

Right now we only want to support notification for core platform features like discussions, grading etc. Currently there is no plan to extend this for plugins.

Please state that explicitly in an ADR inside this app, or in a README.rst file. At some point, someone is almost certainly going to want to add enterprise related notifications, and they should know that it's not a supported use case.

We intend to create notification preferences for each user and want to be able to control those preferences. From an implementation point of view we intend to create a BaseClass for a notification that different apps can extend and generate notifications. The notification types and notification preferences will be saved in the notifications app itself.

That sort of bidirectional relationship is what concerns me. I can see two straightforward paths for this sort of app:

1: notifications knows about other apps, other apps don't know about it.

This looks like what's being built today. The other apps are emitting events without knowing about the notifications app, and it's the responsibility of notifications to filter out the events a user would care enough about to receive notifications for.

With this approach, the big pain points are:

• Adding new notifications requires the maintainers of other apps to modify the notifications app. They'll develop a feature in whatever app they're working on, and then make a PR to notifications to add events for that new feature.
• We can't send any notifications for apps that are not part of the required installation. The notifications app is part of the core installation–it's not supposed to know about plugins and optionally added stuff that may get installed for any particular site.

This can make sense if there's a very curated list of events to display to the user, and longer term extensibility is not a strong factor.

2: notifications does not know about other apps, other apps know about it.

This would be consistent with making a base class that other apps have to subclass. They'd have to import something from the notifications app. But that means that third parties can't effectively add their own notifications–so for instance, open assessment problems can't send notifications about when a student's submission has been assessed.

My concern is that pursuing both approaches at the same time is going to end up in a tangle of dependencies that is going to make it difficult to extend and work with this app in the future.

I hope that answers your questions, let me know if there are any confusions. I would like to provide more context and I think that might help you understand the intent of the feature. I will share a public version of our spec doc when it is ready.

Thank you, I really appreciate it. I'm particularly interested in understanding why edx-ace wasn't used. (I don't know much about it, but it seems like it was built with this sort of thing in mind).

---

Can we use openedx-events for the user notification API interface?

Instead of having a base class for notifications that other apps import, would you consider creating a new event signal in openedx-events? Probably a new UserNotification signal in a new openedx_events.user package?

This would give you a number of advantages, like:

• It has well established conventions for specifying what the event data should look like (example).
• It has support for sending events over the message bus, so you could potentially centralize notifications and delivery preferences across services.
• The openedx-events repo is already built to be a small place for integrations, so plugins and other repos can take easy advantage of it.
• The fact that you'd be defining these signals in a place outside the notifications app would simplify dependencies.
• It would also allow for others to implement alternative notification systems more easily.

The notifications app would still have a centralized list of apps and types that it will display preferences for, but that could be moved to a setting pretty easily, and that could be made more pluggable over time.

The docs for openedx-events are available here: https://openedx-events.readthedocs.io/en/latest/

FYI: @mariajgrimaldi, @felipemontoya

[ormsbee]

FYI to @robrap and @rgraber: Please see the last part of my last comment (the "Can we use openedx-events for the user notification API interface?" section). I'm curious to get your thoughts on whether it would be feasible to use the event bus for this kind of thing in the future someday.

[robrap]

@ormsbee:

1. The future is now. If there is a need for an event, openedx-events is a great place to define it. If that event is needed across services, it can easily be published to the event bus.
2. I'm not clear on the process by which underlying platform events get transformed into notification events, and whether or not those too would be sent to the event bus. That said, as you point out, these patterns all exist to solve common problems and where notifications can take advantage of these patterns, it certainly should.

[ormsbee]

@robrap:

2. I'm not clear on the process by which underlying platform events get transformed into notification events, and whether or not those too would be sent to the event bus. That said, as you point out, these patterns all exist to solve common problems and where notifications can take advantage of these patterns, it certainly should.

I don't have access to the technical design doc for this, so I'm just speculating based on what's been said in this thread, particularly:

From an implementation point of view we intend to create a BaseClass for a notification that different apps can extend and generate notifications.

That implies that individual apps are given control over when to queue up user notifications, though it's ultimately the notifications app and its preferences that determines what to do with this queued notifications (ignore them, send immediately, aggregate into an email, etc.). That seems reasonable to me. So the way I'm imagining that this would work is that there would be no centralized auto-translation of other events into USER_NOTIFICATION signals. It would be the responsibility of each app to decide what events are worth sending notifications for and explicitly create and emit a USER_NOTIFICATION at that time.

The data for the USER_NOTIFICATION signal would include the originating app name and message type. The notifications app then receives these, checks against the user's saved notification preferences, and decides what to do with it.

We might even get to situations where apps are receiving signals from other apps and deciding to send user notifications about that. Like say you had a plugin that did some kind of course recommendation. I could imagine it catching the CERTIFICATE_CREATED signal and sending a USER_NOTIFICATION that basically says, "Congratulations on completing this course! Try one of these three others next..."

[felipemontoya]

Thanks for calling my attention to this Dave. I'd like to weight in some of the initial topics you raised.

Can it use openedx-events instead of pulling directly from models?

Absolutely. As maintainers of openedx-events we would be happy to collaborate on this to make the event be available sooner.

What is the extensibility plan for this?

We have had many requests over the years to make a better notifications system for openedx. Most recently as part of the Spanish Universities project. I just wrote an issue in the campus project roadmap that covers the use cases that they would like to implement. I think having possibilities to extend the notification rules would be a great enabler for many of our customers.

Does it make sense to put this into a separate repo, or maybe openedx-events?

Given the discussion that unfolded in this thread I think as a potential developer of extensions for this I would like to have options to modify:

• the events that users can be notified about
• the way in which those notifications are actually sent.

I'm assuming a lot here, but I would thing that being able to import the base classes from a different repo would make it easier to extend.

[ormsbee]

In order to maintain the focus of this issue on the notifications app, I've moved the discussion that @robrap and I were having over topic/event mapping and 1:M/M:1 producer:consumer relationships to a new issue in the openedx-events repo.

[ormsbee]

Given the discussion that unfolded in this thread I think as a potential developer of extensions for this I would like to have options to modify:

• the events that users can be notified about
• the way in which those notifications are actually sent.

The way this is currently implemented is via a set of per-user preferences, the defaults for which is hardcoded for the moment, but I imagine it could be configurable (maybe via Django settings) in the future?

[ayub02]

Hi everyone! Thanks for participating in this thread. I'm the product manager for the Infinity squad. Here's the specification document of web notifications for forums: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/3768123519/Spec+Web+notifications+for+forums

[ormsbee]

@ayub02: Thank you!

[ormsbee]

@ayub02: Is there a corresponding technical document by any chance?

[ayub02]

Yes. @asadazam93 is working on that. I know that he plans to share a draft soon.

[ormsbee]

Fantastic, thank you!

[asadazam93]

@ormsbee Thank you for your suggestions. I think creating a signal in open edX events for a notification is a great approach and we certainly want to use that. I am creating an architecture document that should display the flow in detail. I am already including the open edX events architecture in that document as I feel that is a must have. Would like to hear your thoughts on the document.

[ormsbee]

@asadazam93: Thank you! Your arch writeup looks like it's in a reviewable state, so I'll leave inline comments there.