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

feat: use 2 nodemailer clients #5369

Merged
merged 9 commits into from
Nov 15, 2022
Merged

feat: use 2 nodemailer clients #5369

merged 9 commits into from
Nov 15, 2022

Conversation

wanlingt
Copy link
Contributor

@wanlingt wanlingt commented Nov 10, 2022

Problem

Closes #126

Solution

  • Create 2 nodemailer clients - 1 using SG env vars, another using US env vars
  • Conditionally send mail from SG client depending on a rollout threshold NODEMAILER_THRESHOLD_SG

Breaking Changes

  • No - this PR is backwards compatible

Tests

  • Before sending emails: check the SES dashboard (US and SG) to see the count of emails sent. Trigger the sending of multiple emails (e.g. OTPs). These emails should be sent successfully. Check the cloudwatch logs to determine which nodemailer client is being used. Check the SES dashboard (US and SG) to see if the new count of emails tallies with the cloudwatch logs.
fields @timestamp, @message
| filter label like 'mail'
| sort @timestamp desc

Deploy Notes

New environment variables:

  • SES_PORT_US
  • SES_HOST_US
  • SES_USER_US
  • SES_PASS_US
  • SES_PORT_SG
  • SES_HOST_SG
  • SES_USER_SG
  • SES_PASS_SG
  • NODEMAILER_SG_WARMUP_START_DATE: Date to start rollout of emails from SG nodemailer client. Should be in the format YYYY-MM-DDTHH:MM:SS+08:00

The permissions for the SES sending policy should also be modified accordingly to allow both US and SG nodemailer clients to send emails

@wanlingt wanlingt marked this pull request as ready for review November 10, 2022 04:23
__tests__/e2e/setup/.test-env Outdated Show resolved Hide resolved
src/app/services/mail/mail.service.ts Outdated Show resolved Hide resolved
src/app/services/mail/mail.service.ts Outdated Show resolved Hide resolved
@justynoh justynoh temporarily deployed to staging-al2 November 11, 2022 03:19 Inactive
src/app/config/schema.ts Outdated Show resolved Hide resolved
@wanlingt wanlingt temporarily deployed to staging-al2 November 14, 2022 09:16 Inactive
@wanlingt wanlingt temporarily deployed to staging-al2 November 14, 2022 10:04 Inactive
src/app/services/mail/mail.service.ts Outdated Show resolved Hide resolved
src/app/config/schema.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@tshuli tshuli left a comment

Choose a reason for hiding this comment

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

lgtm, other than Tim's comments and a minor nit

src/app/config/config.ts Outdated Show resolved Hide resolved
src/app/config/schema.ts Outdated Show resolved Hide resolved
@wanlingt wanlingt temporarily deployed to staging-al2 November 15, 2022 01:46 Inactive
Copy link
Contributor

@timotheeg timotheeg left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@wanlingt wanlingt merged commit 38f688b into develop Nov 15, 2022
@wanlingt wanlingt deleted the feat/nodemailer-clients branch November 15, 2022 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants