Improve types around nodemailer (take 2)
#15638
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
…37-revert-15612-smtp-types
samwho
changed the title
nodemailer""nodemailer (take 2)
| email: string | ||
| userId: string | ||
| userId?: string |
There was a problem hiding this comment.
Automations don't send this, it has always been optional.
| cc?: string | ||
| bcc?: string |
There was a problem hiding this comment.
These were just flatly wrong, assuming an oversight in the original TS conversion.
| user = await db.tryGet<User>(userId) | ||
| if (!user) { | ||
| ctx.throw(404, "User not found.") | ||
| } |
There was a problem hiding this comment.
This is where the original bug was. I had this if (!user) check outside of the if (userId) block, meaning whenever a userId wasn't specified (e.g. in automations), it would always return a 404.
This was an oversight on my part that we had no tests to catch. I've added tests that catch this.
There was a problem hiding this comment.
I've folded all of the tests from realEmail.spec.ts into this file, and this file now uses maildev to send real emails to a local server.
I've wrapped most of the maildev functionality because it was callback based, not async/await. Also the @types/maildev package is quite sketchy and wrong in places, so I've done some manual typing of it.
…37-revert-15612-smtp-types
There was a problem hiding this comment.
LGTM - great to get an iCal invite test case together as well, this is a much better method of testing this, avoids the multitude of issues we've ran into with Ethereal being unavailable etc as well as giving us a lot more options in terms of how we test email.
Reverts the initial revert at #15637
I've pointed out in a comment where the bug from the first attempt was. I've substantially extended the testing around email sending in
packages/worker. Instead of doing it half with Jest mocks and half by firing off real emails toseamus99@ethereal.email, it now spins up an SMTP server in the tests using https://github.com/maildev/maildev and sends off real emails, then retrieves them throughmaildev's API. This results in much better test realism, and better assertions on the results of the tests.Feature branch env
Feature Branch Link