[SIP-96] Proposal for adding a notification feature

[mattitoo]

Motivation As maintainer of a Superset Instance, I want to be able to message my users, so I can inform them about possible downtimes, data troubles or other causes. This can of course be achieved in various channels (Mail, Slack, Teams, etc) but it also makes sense to do this in the actual tool used, in this case right inside Superset.

Proposed Change Luckily, Superset already has a messaging capability built in: so-called Toasts, which are displayed after certain actions like saving, refreshing dashboards and so on. We have created a “management interface” for such toasts, in which new messages (we call them notifications) can be configured:

Following parameters can be set: ·       Active ·       Name or Summary ·       Category (currently 3 categories, these manage the color of the notification: Info=blue, Warning=yellow, Alert=red) ·       Message: The message to be displayed. ·       Time Range: The range in which the notification should be displayed ·       Use daily timeframe: If active, the notification is only shown when the current time is inside the start and end time ·       Re-trigger: Minimum of 5 Minutes to reduce the load on the database due to querying for new messages for each user ·       Duration: How long until the notification disappears (it can always be closed via the ‘x’) ·       Roles: Which roles should see that notification

For each user, the internal database is queried at login and at a certain interval to see if she/he has new notifications pending.

Example notification in action:

NOTE: This should be behind a feature flag like “ENABLE_UI_NOTIFICATIONS”

New or Changed Public Interfaces Extends internal database to include two additional schemas: public.notification and public.notification.roles

Extends API to include endpoints for management of Notifications

Adds an entry in the Settings menu

Adds new permission for notification management

Migration Plan and Compatibility Database migration necessary to include new tables

Rejected Alternatives Using another method of displaying notifications, e.g. as a banner on top of Superset or below the menu bar. Rejected in favor of using existing and proven methods already built in.

[mattitoo]

We will also be implementing a "Preview" button for the notifications in the configuration overlay.

[villebro]

I LOVE this proposal! I routinely need something similar, but hadn't even thought of having something like this. +1 for the proposal and an even bigger +1 for the well thought out UI + API.

Some thoughts:

• I know we aren't super consistent in always applying or not applying training slashes on REST endpoints, but I believe it's commonly accepted that leaving out the trailing slash is best practice. So maybe replace /api/v1/notification/ with /api/v1/notification etc. @dpgaspar any thoughts here?
• The "Manage" menu item is maybe unnecessarily ambiguous. I've been thinking about starting a SIP to introduce a similar UI for cache warmup jobs, and if we do end up adding more scheduling functionality, maybe we could consider breaking them out under a new menu "Scheduled tasks". Maybe not now, but perhaps for Superset 4.0
• Instead of toasts, maybe important notifications should be displayed more prominently, e.g. on the subnav (the one with "Home" in the pic below), with a button to explicitly ack that the message has been read? Typical cases for such notifications could be scheduled downtimes, upgrade notifications etc. Also being able to add links to the messages would be a nice addition.

[mattitoo]

Talking about links i realize i forgot to mention the elephant in the room: We explicitly disabled HTML sanitization for these toasts to be able to include links (and styling). The reason behind this is that in a toast you only have limit space and you e.g. want to link to Release notes or similar. I understand this can be seen as a security risk, but for us this is not a big deal. So, in our current implementation, HTML is possible within a notification.

As for other forms of notifications: That was my initial idea (to use banners or similar), but we went with the built-in way of using toasts because it was easier and within the known User Experience. We can still think about enhancing the functionality to include other notification types.

[pankajsoni22]

I agree with the thought process, it's definitely a good step make this tool better and more useful.

[rusackas]

Thanks for the SIP! Numbering this as SIP-96. Please make sure to submit it on a [DISCUSS] thread on the Superset @dev mailing list, so we can start progressing it toward a vote. Thanks! Apologies, you already did this! Shocking discovery: I'm terrible about email.

[michael-s-molina]

Thanks for the SIP! Some thoughts:

• It would be a good idea to get @kasiazjc's input on the screens.
• I really like @villebro's idea of different types of notifications. A banner would be helpful for critical messages that need to persist while an event is happening.
• Regarding the HTML sanitization, you can always extend the default schema with HTML_SANITIZATION_OVERRIDES and keep it safe with a well defined Content Security Policy (CSP). The important part here is that notifications should respect these settings.

[mattitoo]

How we could continue on this:

• We need to do some refactoring of the existing code to make it nicer and more accessible
• While doing that, we would also check the HTML sanitization
• We create a PR to have the code checked etc
• After merging, whoever wants can work on expanding the functionality (like different types of notifications) The reasoning is that the topic is already big in my opinion and maybe we should add additional complexity step by step. Of course, I am open to other way of going forward as well, but the first 3 points are pretty clear ;)

[villebro]

I agree with @michael-s-molina 's proposal to use HTML_SANITIZATION and HTML_SANITIZATION_SCHEMA_EXTENSIONS (I believe that's what they're called?). I feel this needs to be part of the initial PR, as it's already an established pattern in the codebase. Regarding toasts vs banners, I'm fine with @mattitoo 's proposal to keep the scope limited to toasts for now. Let's just make sure the initial body of work can stand on its own (=so the project isn't stuck with more dead wood if development interest simmers) and we don't paint ourselves into a corner in a way which makes it difficult to extend notifications beyond toasts. And to distribute the work, I'm quite happy to pick up the task of creating banner notifications after the initial feature is ready if nobody else is volunteering for it.

One more thing that came to mind - should we support Jinja templating in these notifications? This will make it possible to make dynamic messages, e.g. "You only have 2 days left to migrate X to Y" instead of "Please migrate X to Y by Z". This may also be something worth considering for the other fields, like Category: It's an Info when there's more than 7 days left, becomes and Alert at 3 days and Danger when there's less than 2 days. Also, I brought this up in my initial message, but I think we should also consider adding support for snoozing and acknowledging messages.

[mattitoo]

Very good points. I like the JINJA templating idea to automate messaging more. Regarding snoozing and acknowledging: This is also a great idea. We were thinking about it, but at first abandoned it to get a working solution out of the way and not increase initial complexity too much. But once we have the PRs out, we should be thinking about this, i feel this is a very good thing to have.

[villebro]

I agree, let's not overload the initial work, but let's make sure we keep these in mind early on so the initial design can accommodate iterative improvements later. To summarize; let's leave as much room for extensibility as possible.

[michael-s-molina]

I agree, let's not overload the initial work, but let's make sure we keep these in mind early on so the initial design can accommodate iterative improvements later. To summarize; let's leave as much room for extensibility as possible.

@mattitoo @villebro Exactly. The important part is that the foundation considers these future points of development. As we can see by the comments, the proposed feature seems to have great potential. Thanks again for the proposal @mattitoo and for driving its development. I'll gladly help reviewing the work and maybe contributing as well.

[rusackas]

Hi @mattitoo (and everyone) - I just wanted to check in on the status here. If we intend to move forward with this effort, someone will need to start a "DISCUSS" thread on the Apache Superset dev mailing list. After that, we can carry it forward to a vote. Ping me on Slack if help is needed.

[mattitoo]

Sorry, we have been bogged down with other issues. Next up for us is switching to 3.0.1, where we will also make sure that this functionality works. I will start the DISCUSS thread shortly.

[soniagtm]

Hi @mattitoo,

I wanted to inquire if there have been any updates regarding this proposal or its implementation, as it's a feature we're eagerly anticipating.

Thanks!

[mattitoo]

I am sorry there has not been much progress, we have been busy with other features, but will get started to make it "contributable" soon

[rusackas]

Hi @mattitoo - just wondering if there's any hope of moving this forward. I can kick off the [DISCUSS] thread for you on the mailing list if you'd like.

[MialLewis]

Just commenting to say that this feature would be much appreciated!

[vmbaraiya]

+1, Adding this feature would be highly beneficial for running Superset workloads in production environments.

[rusackas]

@mattitoo any ideas of timing here? Would love to see this move forward through the process (starting with a Discuss thread)

[sudhakar-datta]

It would be amazing if there are plans for making into a particular release and timeframe. This feature will be one of the most useful feature in a production workflow.