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

notificationCatcher: unable to send attachements via email #52

Closed vmarchaud closed 3 years ago

vmarchaud commented 6 years ago

It seems that the library never attachements to the notification catcher : https://github.com/notifme/notifme-sdk/blob/master/src/providers/email/notificationCatcher.js#L8 Is this a expected behavior even if its defined as available in the request : https://github.com/notifme/notifme-sdk/blob/master/src/models/notification-request.js#L17

Regards

BDav24 commented 6 years ago

You're right, it should be easy to send attachments (https://github.com/notifme/catcher/blob/master/scripts/send.js#L30-L32). Would you like to make a pull request?

vmarchaud commented 6 years ago

Yes of course, since i'm integrating your sdk with our product i might have more modifications to do, i'll send a PR when i'm happy with the changes !

mrfrase3 commented 3 years ago

I just noticed that I had patched this years ago in a project, and apparently never made a PR, pretty simple fix.

NB: I've noticed another bug with the catcher where only the latest email attachments will be visible, but it's better than nothing and I think most people want to make sure the files are actually sending through correctly anyway.