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

Use nodemailer for sending email (with local email dev solution) #433

Merged
merged 26 commits into from
Aug 22, 2020

Conversation

patcon
Copy link
Contributor

@patcon patcon commented Jul 18, 2020

Addresses #231

Should be working, but need to write tests before I seek full review. But I'm still looking for feedback on the approach I've taken :)

Screen Shot 2020-07-18 at 2 28 53 AM

To Do

other e2e tests to consider adding in future:

@patcon patcon added 🔩 p:server ⚒️ infrastructure Re: automation, continuous integration. labels Jul 18, 2020
@patcon patcon requested review from metasoarous and a team as code owners July 18, 2020 05:27
@patcon patcon self-assigned this Jul 18, 2020
@patcon patcon added this to To do in Polis development via automation Jul 18, 2020
@patcon patcon moved this from To do to Feedback Required in Polis development Jul 18, 2020
@patcon patcon marked this pull request as draft July 18, 2020 05:31
@patcon patcon changed the title Feature/231 dev email Use nodemailer for sending email (with local email dev solution) Jul 18, 2020
@patcon
Copy link
Contributor Author

patcon commented Jul 19, 2020

Hm. Worked locally, but something happening when running on GitHub Actions, and cy.visit is getting confused... Will investigate later.

@patcon patcon marked this pull request as ready for review July 20, 2020 03:07
@patcon patcon marked this pull request as draft July 20, 2020 03:16
@patcon patcon moved this from Feedback Required to In progress in Polis development Aug 13, 2020
@patcon
Copy link
Contributor Author

patcon commented Aug 14, 2020

For later, apparently maildev has a restful API, so we can be hitting that instead of parsing an html page, as recommended by cypress best-practices.

https://github.com/maildev/maildev/blob/master/docs/rest.md
https://docs.cypress.io/guides/references/best-practices.html#Visiting-external-sites

Copy link
Contributor

@ballPointPenguin ballPointPenguin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks great. I have one concern only, in changing the table name.

server/postgres/db_setup_draft.sql Outdated Show resolved Hide resolved
@patcon
Copy link
Contributor Author

patcon commented Aug 16, 2020

Thanks so much for the review @ballPointPenguin! Good call on figuring out migrations. I'll do that in this PR then bring it out of draft

@patcon
Copy link
Contributor Author

patcon commented Aug 18, 2020

This is now blocked on merging of #542

@patcon patcon marked this pull request as ready for review August 21, 2020 17:48
@patcon
Copy link
Contributor Author

patcon commented Aug 21, 2020

Hm. Not sure why e2e tests aren't running... seems to have stopped in cca71ac

server/docker-dev.env Outdated Show resolved Hide resolved
server/email/senders.js Outdated Show resolved Hide resolved
Copy link
Member

@metasoarous metasoarous left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks!

@patcon patcon merged commit 59900fa into compdemocracy:dev Aug 22, 2020
Polis development automation moved this from In progress to Done Aug 22, 2020
@patcon patcon deleted the feature/231-dev-email branch August 22, 2020 20:00
@metasoarous metasoarous removed this from Done in Polis development Feb 25, 2021
raulsperoni pushed a commit to proyectourgente/polis that referenced this pull request Sep 1, 2021
…pdemocracy#433)

* Updated database schema to bugfix password reset process. [Fixes compdemocracy#273]

* Added maildev docker container for inspecting emails during dev.

* Ensured the proxied services are seeing the origin host.

* Small fixup from rebase.

* Added SMTP port exposure to maildev container.

* Migrated AWS_REGION config into envvar.

* Added mailgun nodemailer transport. Added fallback through multiple transport mechanisms.

* Added ability for cypress to check maildev inbox on another port.

* e2e: Fixed create_user test.

* e2e: Added checks of password reset flow.

* e2e: Added test stubs for types of emails sent.

* e2e: Run through whole password reset flow, and confirm new password.

* e2e: Added plugin to output more details to stdout.

* e2e: Make more clear when reporter prints to terminal.

* Added log command for troubleshooting GitHub Actions issue.

* e2e: Fixed issue with matching password reset token.

* Added testing of email transport failover.

* Adding docs for email transport configuration. [skip ci]

* Check maildev via API instead of UI.

* Improved documentation of cypress workflow for email transports.

* e2e: Added note about cypress-terminal-report in README.

* Removed straggling TODO.

* Set email transport defaults to match current production.
raulsperoni pushed a commit to proyectourgente/polis that referenced this pull request Oct 26, 2021
…pdemocracy#433)

* Updated database schema to bugfix password reset process. [Fixes compdemocracy#273]

* Added maildev docker container for inspecting emails during dev.

* Ensured the proxied services are seeing the origin host.

* Small fixup from rebase.

* Added SMTP port exposure to maildev container.

* Migrated AWS_REGION config into envvar.

* Added mailgun nodemailer transport. Added fallback through multiple transport mechanisms.

* Added ability for cypress to check maildev inbox on another port.

* e2e: Fixed create_user test.

* e2e: Added checks of password reset flow.

* e2e: Added test stubs for types of emails sent.

* e2e: Run through whole password reset flow, and confirm new password.

* e2e: Added plugin to output more details to stdout.

* e2e: Make more clear when reporter prints to terminal.

* Added log command for troubleshooting GitHub Actions issue.

* e2e: Fixed issue with matching password reset token.

* Added testing of email transport failover.

* Adding docs for email transport configuration. [skip ci]

* Check maildev via API instead of UI.

* Improved documentation of cypress workflow for email transports.

* e2e: Added note about cypress-terminal-report in README.

* Removed straggling TODO.

* Set email transport defaults to match current production.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚒️ infrastructure Re: automation, continuous integration. 🔩 p:server
Projects
None yet
3 participants