-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added MailAdapter and support for Mailgun API #191
Conversation
A more generic implementation of service providers I think will help, not hardcoding the provider into ParseServer .. see #197 .. |
Great idea, it would certainly make things more extensible in the future without the need to constantly hack the ParseServer constructor, I'll take a look st refactoring MailAdapted soon based on #197 if everyone agrees that it's the best approach 👍. |
I guess we would have to also consider what would happen if multiple providers are configured simultaneously too — i.e. which service would take precedence if |
For now I think we should stick with the Adapter approach we've got and see how far that takes us... I do like #197 too, but I agree it can open a different set of issues too. |
You still may have a default adapter defined, as it is in DatabaseAdapter, for fallback. But yes, maybe this should be added as infrastructure a little bit latter, as the code will be more stable. |
Also, this pluggable infrastructure should keep server clean of pull request for other adapters for been hardcoded, as for example S3 cloud file, which of course is great, but what if I want to have Azure ? Send a pull request and hardcode in server Azure init code ?? |
Perhaps rather than throwing everything we've got at the constructor, maybe we should extend the API to to support configuring other services?
|
It makes sense to have abstractions built around file storage, database interaction, session caching. These are integral components of the Parse server. However, transactional mail, SMS, and other services are auxiliary and belong in Cloud Code in my mind, as they don't have basic implementations that will apply to all Parse Servers. In service to this approach, we could expand the transaction lifecycle by adding more trigger definitions. |
True to an extent @maysale01, I would argue that email is definitely a integral component though as its used for user account verification and password reset links (https://parse.com/docs/js/api/classes/Parse.User.html#methods_requestPasswordReset, etc) |
@jamiechapman Ah fair enough. My only concern is that it inherently relies on an external service and is only ever invoked by user action (registration and password reset request). Seems like a good candidate for moving to some system reserved triggers, |
Pushing things to Cloud Code is a good idea for another adapter, but a default implementation should be whatever is easiest, like tossing in a mailgun id and being done. Mail is a core feature and forcing new cloud functions to get it back wouldn't be the best experience. What I'd like to do is merge this adapter method, and then have #187 updated to use it. If you disagree, then let's do something else. |
Here's an example for how we can expose this with triggers without baking in and having to maintain any interop with 3rd party vendor libraries. https://github.com/maysale01/parse-server/tree/add-triggers-for-mail-events In the users handler we could query the user before hand and add some basic validation, before getting passed to the cloud code trigger. This would also support a default implementation with the MailgunAdapter e.g. |
Looks good @maysale01, I definitely see the value in users being able to switch to whatever mail provider they like using Cloud Code —your implementation looks pretty neat for most developers! I do however know some Parse developers that don't even use Cloud Code or write JavaScript at all (just purely mobile devs using the mobile SDK's) — so I guess for developers like this; they would have to learn how to write Cloud Code functions, understand how npm/modules work, be able to write JavaScript etc etc to get something as simple as password resets email to work. For the most part I think a |
@jamiechapman Well, we can add a default implementation to the example. I think a good compromise would be to have a MailService that is exposed globally, i.e. |
@maysale01 sounds like a good idea, I think there needs to be some default behaviour for sure. I guess it would boil down to which 3rd party mail provider you would choose as Heroku doesn't support system mail (they have add-ons like Mailgun etc) and I believe Azure doesn't either (they usually plug things like SendGrid). |
Lets consolidate discussion in #275 |
Closing this, looks like the eventual solution will come from issue 275. |
I am trying to enable reset password and email verification for my parse-server-example installed locally. I could see we have https://github.com/parse-server-modules/parse-mailgun. But I am not clear how to use parse-mailgun in parse-server-example, I am completely lost with it. Regards |
Stack Overflow is the best place to ask for implementation issues. |
I've added initial support for sending transactional e-mail via Parse Server. The MailAdapter can eventually be used for sending verification e-mails after a user signs up, or for sending password reset instructions #99, etc.
At the moment it only supports Mailgun — which can easily be added as an add-on with Heroku for free (300 e-mails per day), or users can sign up manually for an API key at mailgun.com for 10,000 emails free per month.
To configure, simply add
mailConfig
into theParseServer
constructor as follows:Once configured, e-mails can be sent like this:-
I haven't updated the main readme file for now as no part of Parse Server currently sends transactional e-mails and I didn't want to cause any confusion for new adopters! Also if
mailConfig
isn't configured inside theParseServer
constructor, nothing should happen.Support for Mandrill / Postmark etc should be fairly trivial as they just need hooking up to MailAdapter once implemented.