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

fix(email-nodemailer): Fix skipVerify behaviour being reversed #6790

Open
wants to merge 3 commits into
base: beta
Choose a base branch
from

Conversation

JasperBeaurain
Copy link

Description

Fixes #6789

The skipVerify field in NodemailerAdapterArgs worked in reverse of what it was supposed to do:

  • With skipVerify = true -> Verified transport
  • With skipVerify = false -> Did not verify transport

This PR makes the property work in the intended way:

  • With skipVerify = true -> DO NOT verify transport

  • With skipVerify = false -> DO verify transport

  • I have read and understand the CONTRIBUTING.md document in this repository.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have added tests that prove my fix is effective or that my feature works
  • Existing test suite passes locally with my changes
  • I have made corresponding changes to the documentation

Copy link
Member

@denolfe denolfe left a comment

Choose a reason for hiding this comment

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

@JasperBeaurain Thanks for the PR! Just one additional scenario that needs to be handled.

@@ -66,7 +66,7 @@ async function buildEmail(emailConfig?: NodemailerAdapterArgs): Promise<{
transport = await createMockAccount(emailConfig)
}

if (emailConfig.skipVerify !== false) {
if (emailConfig.skipVerify === false) {
Copy link
Member

@denolfe denolfe Jun 17, 2024

Choose a reason for hiding this comment

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

This also needs to handle when skipVerify is undefined. It should verify when undefined.

So this could either be if (!emailConfig.skipVerify) or if (emailConfig.skipVerify !== true).

@JasperBeaurain
Copy link
Author

@denolfe Whoops, I missed that indeed. Made changes as requested.
I also added a few tests that validate the flag's behaviour.

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.

None yet

2 participants