novu
novu copied to clipboard
novuhq
fix: #1753 added switches for secure boolean UI
-
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...) This was a bug fix in response to issue #1753.
-
Why was this change needed? (You can also link to an open issue here) In that issue somebody brought up features that weren't being used according to the docs so they should be removed. I both removed the Sender name for nodemailer and added switches instead of input for all booleans, specifically the Secure field.
-
Other information:
I looked through the documentation and noticed many other documentation pages are either missing information, or the page is incorrect and needs to be fixed.
@CursedRock17 from the docs of nodemailer, seems like we can add the support to add the from name with the following syntax:
from - The email address of the sender. All email addresses can be plain ‘[email protected]’ or formatted '“Sender Name” [email protected]', see Address object for details
https://nodemailer.com/message/
Maybe we can create conditional the from string if a sendername exists? instead of removing this configuration option from the modal?
What do you think?
I definitely think that's a good idea, gives the users more options. Though, would this be better as an overall change, I haven't looked at the documentation for all of the providers in depth yet, but would this work for most of them as well. Just wondering if we should change the UI in general or if you just wanted a nodemailer provider fix here.
@CursedRock17 seems like we have here some unrelated commits regarding pushwoosh. Do you mind separating them to a different PR?
Regarding the addition of the sender name, I think this change is only missing in nodemaielr since other provider are familiar with the senderFrom property on the API level. I believe we can only fix nodemailer in this PR and no need for other changes. Wdyt?
@scopsy I agree we most likely don't need to make any other changes. As for pushwoosh I had just made a template for the new provider and it wasn't letting me create a new pull request, I'll get that cleared.
@scopsy I know it's very messy, but I should've gotten rid of the pushwoosher provider API for this Pull Request. If you can merge and close this one I will push that provider into a new Pull Request