search

reactioncommerce / api-plugin-notifications

Notifications plugin for the Reaction API
Apache License 2.0
1 stars 5 forks source link

Rewrite notifications to be more flexible/configurable #3

Closed brent-hoover closed 2 years ago

brent-hoover commented 7 years ago

Opening this because @spencern has mentioned several times that we need to do this and he's very right so I wanted to get this spec'd out so it could be worked on when appropriate.

The current notification system does a good job sending Order notifications, but needs to be refactored to be more general and allow configuring a notification for almost any system event, plus allow which events get SMS and in-app notifications. The current events are essentially hard-coded.

My initial proposal for how this should work would be:

  1. Settings panel searches for all available system events (Hooks) and allows you select which events should receive either an in-app notification and/or an SMS (maybe also Slack could go here also?). This should be dynamic so as system events are added they are automatically appear in this settings panel. Probably with a default set for basic Order notification similar to what we have now.
  2. You should also be able to configure the content of the message similar to how you can do now with emails (via templates probably?).
  3. Should be extendable with other system notification types. (Hopefully Slack could be an included example of this?)
  4. Should listen for all system events, check configuration, and then send appropriate notification.
  5. Optionally could listen for Errors and allow you to send notification when certain types of errors appear.

Sort of a "sub-task" of reactioncommerce/reaction#429 ( as was the original notifications system I suppose)

spencern commented 7 years ago

I think this is a good start.

I believe that you should also be able to trigger an In-App Notification for any user with arbitrary content from the server.

Perhaps this should be integrated with our Alerts and Email notifications work. I can see a system where the three levels of notification could all be triggered by a single call.

E.g. Reaction.Notify("Message", "success", {Alert: toast, email: true, inApp: true, users: [arrayOfUserIds]}) Or something along those lines.

Additionally I wrote a long blog post about this (how it should work and some of how it does work that might be helpful for whoever ends up implementing.

https://blog.reactioncommerce.com/reaction-architecture-alerts-notifications-and-emails/

brent-hoover commented 7 years ago

@spencern Yes, agreed that it makes sense to fold Email into this as just another type of notification

spencern commented 7 years ago

We've had suggestion that Browser notifications be considered as part of our notification framework as well.

spencern commented 6 years ago

Another issue that may not be directly resolvable by this issue is that order notifications are currently hardcoded to use the ShopId associated with the order. Order notifications (and probably other types of notifications as well) need to be flexible enough to notify users of Shops with order items rather than just the shopId associated with the order.

brent-hoover commented 2 years ago

Closing due to restructuring of priorities. Will revisit if there is customer demand.