Skip to content
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

Make emails easily accessible in dev environment #231

Closed
patcon opened this issue Apr 26, 2020 · 10 comments · Fixed by #433
Closed

Make emails easily accessible in dev environment #231

patcon opened this issue Apr 26, 2020 · 10 comments · Fixed by #433
Assignees
Labels
🎉 enhancement ⚒️ infrastructure Re: automation, continuous integration. 🔩 p:server

Comments

@patcon
Copy link
Contributor

patcon commented Apr 26, 2020

Right now, emails get sent out for password reset, and for data exports, and perhaps others things. For the latter, it's after some processing in the math component, which calls back to server to send email afterwards.

It would be great if email bodies/metadata could be inspected during development without trying to send a real email via Mailgun SaaS (currently the only way to send).

An alternative approach would be to try to send it via postfix, but printing to logs seems simplest.

I think this is where this would need to be added:
https://github.com/pol-is/polisServer/blob/806e77e508cd8c27a8c372502c111359fdd8b28e/email/sendEmailSesMailgun.js#L63-L73

Off the top of my head, this could be enabled in one of two ways:

  1. setting devMode to true, and always printing to logs when in this mode, or
  2. added a config like emailServer which could be one of logs or mailgun (or some future SaaS)
@joshsmith2
Copy link
Contributor

+1! This would be super useful

@patcon
Copy link
Contributor Author

patcon commented May 5, 2020

Potential other option, perhaps in a separate docker container: https://github.com/maildev/maildev

@colinmegill
Copy link
Member

colinmegill commented May 5, 2020

Agree, this needs to be handled for local and non mailgun scenarios

@patcon
Copy link
Contributor Author

patcon commented May 6, 2020

@joshsmith2 Heads up -- Data export might not be working rn anyhow: #39 (comment)

@patcon
Copy link
Contributor Author

patcon commented May 6, 2020

From gitter chat: https://gitter.im/pol-is/polisDeployment?at=5eb2cacb7975db7ebfe99d9f

@crkrenn is +1 to suggestion on using nodemailer and transport plugins

@colinmegill
Copy link
Member

colinmegill commented May 6, 2020 via email

@patcon patcon transferred this issue from pol-is/polis-issues May 10, 2020
@patcon patcon changed the title Allow emails to print to logs in dev environment Make emails easily accessible in dev environment May 21, 2020
@patcon
Copy link
Contributor Author

patcon commented May 21, 2020

I've got this working on a feature branch:
Screen Shot 2020-05-21 at 3 10 55 AM

Currently, we fall through catches from AWS SES (first choice) and mailgun as a backup. Should we just fall through to trying to send to maildev if no AWS or Mailgun is configured? Alternatively, we could ditch the fall-through approach that's currently in there, and just set a config toggle between aws-ses, mailgun or maildev (or something)

@patcon patcon moved this from To do to In progress in Polis development May 21, 2020
@patcon patcon self-assigned this May 21, 2020
@patcon
Copy link
Contributor Author

patcon commented Jun 6, 2020

Here's the pending branch for spinning up with a dev mailserver (from above screenshot):
dev...patcon:feature/231-dev-email

Still needs some work. The way it was set up before (pre-monorepo), SES was first priority and fallback was mailgun. This was hardcoded. Thinking it's be nicer to pass it a config that sets the mailer, including order of attempts. So, for example, in envvars:

EMAIL_TRANSPORT_TYPE=maildev would need no config, as just sends to the development mailbox in another docker container. This is what my branch does rn, which obviously isn't good enough for anything except dev :)

EMAIL_TRANSPORT_TYPE=mailgun would expect other config to be set, like MAILGUN_API_KEY and MAILGUN_DOMAIN. If that fails, then mail doesn't go out.

EMAIL_TRANSPORT_TYPE=aws-ses,mailgun would expect additional config as well, and would try SES first, but if it fails, it will then try mailgun. (this is what the old code did, but without ability to choose from amongst mail providers)

Thoughts?

@ballPointPenguin
Copy link
Contributor

Thoughts?

I like this whole approach. I'd really like to leave the door open for other configurations, as much as possible, e.g. sendgrid. Maybe default to maildev, and for now allow for the others to be user-specified "mailgun", "aws-ses", or combo.

I wonder how necessary a fallback mail service really is. Does it ever get used? I've never set up an api to fall back to a second service personally. But hey, if it was working already, I'm not going to make a fuss.

@patcon
Copy link
Contributor Author

patcon commented Jun 6, 2020

I wonder how necessary a fallback mail service really is. Does it ever get used?

good call. would you know, @metasoarous? it does add more logic, and better to have less :)

Polis development automation moved this from In progress to Done Jun 27, 2020
Polis development automation moved this from Done to In progress Jun 27, 2020
@patcon patcon linked a pull request Jul 30, 2020 that will close this issue
15 tasks
@patcon patcon added ⚒️ infrastructure Re: automation, continuous integration. 🎉 enhancement labels Jul 30, 2020
Polis development automation moved this from In progress to Done Aug 22, 2020
@metasoarous metasoarous removed this from Done in Polis development Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement ⚒️ infrastructure Re: automation, continuous integration. 🔩 p:server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants