openwisp / openwisp-notifications

Notifications module of OpenWISP
https://openwisp.io/docs/dev/notifications/
GNU General Public License v3.0
41 stars 42 forks source link

[feature] Add notification types #1

Closed pandafy closed 4 years ago

pandafy commented 4 years ago

Goals:

Project Idea

Add the concept of notification type, it may be a list with some default values which can then be extended by each module. Each notification type shall have a message which is generated from a configurable text template, the text shall be markdown formatted, the markdown text shall be expanded into HTML when the notification is sent via email or viewed via the JS widget.

Discussion Project idea says to have some kind of list, in my opinion having a model will be more appropriate for this use case. .

NoumbissiValere commented 4 years ago

@pandafy please how do you intend to achieve the requirements of this feature with a model? i was thinking of dict :smile:

pandafy commented 4 years ago

I was thinking to have something like templates for device configuration as implemented in openwisp-controller. We can provide migration script to provied default notification types. This way we make it easy for users to customize notifications from admin dashboard.

nemesifier commented 4 years ago

@pandafy will you implement the HTML formatting now or later?

Remember we have to decouple the notifications module in openwisp-monitoring as soon as possible, so the priority has to be to prepare the minimum changes required to do the decoupling.

I would leave the HTML formatting for the next step after we have decoupled the module, because it should be an internal change which will not affect openwisp-monitoring.

nemesifier commented 4 years ago

I was thinking to have something like templates for device configuration as implemented in openwisp-controller. We can provide migration script to provied default notification types. This way we make it easy for users to customize notifications from admin dashboard.

The problem is that the notification types have to be used by code and the code won't know what's in the database, which means we have to define the notification types in code. The code will be extensible, so if anyone needs can add a new notification type and use it in some custom code (eg a custom django app or some code which connects to some signal and fires the custom notification type).

For a recent example, see how chart configurations are implemented in openwisp-monitoring, I changed this just the other week and I am so far satisfied of the result: https://github.com/openwisp/openwisp-monitoring/blob/master/openwisp_monitoring/monitoring/models.py#L257-L259 We can adopt a similar approach.

nemesifier commented 4 years ago

Done, thanks @pandafy.