search

notifme / notifme-sdk

A Node.js library to send all kinds of transactional notifications.
https://notifme.github.io/www/
MIT License
1.94k stars 149 forks source link

feat: add support for the full slack webhook payload #51

Closed terebentina closed 6 years ago

terebentina commented 6 years ago

The initial slack provider had 2 limitations:

  1. it allowed sending only the message, without all the customization slack allows.
  2. it allowed sending to a unique/global webhook. There are 2 scenarios for notifme-sdk:
    • used to send global notifications (like application errors, logs, events). In this case a single webhook in the config is enough.
    • used by the users of an app - they integrate the app with slack and the app sends them messages. In this case the webhook urls are more like the to emails in the email provider - there should be a single slack provider set up but able to send to various recipients. This case wasn't covered by the existing slack provider but it is with these changes.

I made the webhookUrl in the slack config optional. Either the config, or the send payload must specify a webhookUrl. To be honest, I would like webhookUrl in the payload to be either a string or an array eventually, so we can send to more hooks at once, just like we can send to more cc or bcc recipients in emails. The difference would be that notifme-sdk would have to fire multiple fetch requests in this case.

BDav24 commented 6 years ago

Thanks for the PR! You're right webhookUrl can be seen as the to field.

About webhookUrl being an array: it can't be the case by design (email.to is also a string) to make templating easier (https://github.com/notifme/notifme-template) and other (maybe future) functionalities.