Deliver mail asynchronously via deliver_later

[zachmargolis]

• Also adds a Rubocop linter to help keep us consistent

For all we know, there's some mail permission that's missing on the worker hosts so we need to confirm this works before merging

[zachmargolis]

alphabetizing

[mitchellhenke]

This looks good to me. Does it do retries, and do we want to limit that, e.g. so we don't send password reset emails after the code has expired?

Would we want to feature flag this?

[zachmargolis]

Does it do retries, and do we want to limit that, e.g. so we don't send password reset emails after the code has expired?

I wasn't sure so I checked the the ActionMailer docs

deliver_later(options = {})
Enqueues the email to be delivered through Active Job. When the job runs it will send the email using deliver_now.

and then the ActiveJob docs:

10.1 Retrying or Discarding failed jobs

A failed job will not be retried, unless configured otherwise.

So my current believe is: no these will not be automatically retried.

Would we want to feature flag this?

Yeah I am open to feature flagging, do you have an approach? I'm thinking we could monkey patch on a method like deliver_now_or_later that checks the flag and routes it? intead of updating each callsite with a ternary?

[mitchellhenke]

So my current believe is: no these will not be automatically retried.

I think that's the behavior we'd want for now, so sounds good.

Yeah I am open to feature flagging, do you have an approach? I'm thinking we could monkey patch on a method like deliver_now_or_later that checks the flag and routes it? intead of updating each callsite with a ternary?

That makes sense to me, if we can subclass the mailer or something, that'd be great.

[zachmargolis]

That makes sense to me, if we can subclass the mailer or something, that'd be great.

🤦 UserMailer is already our subclass --- I will use that

[zachmargolis]

feature flag added in 11e64d6

[zachmargolis]

For all we know, there's some mail permission that's missing on the worker hosts so we need to confirm this works before merging

Confirmed the worker hosts have correct SES permsision

[zachmargolis]

Admin overriding the diff coverage