Add *_deliver callbacks for Action Mailer

[bensheldon]

Motivation / Background

This Pull Request has been created to add deliver callbacks (e.g. before_deliver, after_deliver, around_deliver) to Action Mailer. The benefit is that it allows delivery observer/interceptor-like behaviors within the context of the instance of ActionMailer::Base rather than operating only on the Mail object.

Use cases for this:

• Accurately updating an AR model's delivered_at value, especially when using deliver_later.
• Doing something with the delivery provider's message_id (e.g. storing it with a reference to the user or message so that provider webhooks for opened/clicked/spammed can be associated)
• Handling the weird delivery errors like invalid email addresses that are not visible until the mailer is attempted to be delivered... while having the full context of the Mailer (e.g. adding a note onto the User record that the email was undeliverable).

Additional information

I also found #42139, which is similar but doesn't go all the way of providing the context of the Mailer.

Something to note: Action Mailer's rescue_from wraps both the mailer's action processing/rendering step, and the deliver step (effectively twice). I only wanted the deliver callback to wrap deliver, because the existing other *_action callback wraps the action processing/rendering step. In pseudocode:

processed_mailer = rescue_from do
  around_action do
    render_mail
  end
end

rescue_from do
  around_deliver do
    deliver(processed_mailer)
  end 
end

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

[rafaelfranca]

LGTM, but can you added documentation for this new feature?

[bensheldon]

✅ I added documentation and changelog. I also changed the callback name from _delivery to _deliver because that matches the method being wrapped and avoids the implication that the destination smtp server (or wherever down the multi-hop network chain) actually received it which is beyond the scope of the Mail gem.

[Bhacaz]

@bensheldon Hi, nice addition.
Quick question can those new callbacks could completely make obsolete Interceptor and Observor from mail gem?

[bensheldon]

@Bhacaz thanks for the question! I wouldn't say "obsolete" because the Mail gem is usable outside of Rails, so interceptors/observers are still necessary from Mail's perspective.

From the perspective of Rails, I think interceptors/observers are less necessary. I'm not confident to say un-necessary. I mentioned the use-cases that I find myself frequently writing (updating a record at delivery, getting a provider id, responding to delivery errors), but there are likely uses that I'm unaware of.

From looking at the documentation, I could imagine that a more significant documentation change might be:

• Emphasizing the available callbacks and what they can be used for
• De-emphasizing Mail's observer/interceptors as like "btw, these exist too"

[alkesh26]

@bensheldon, Thanks for the fantastic feature.

I only wanted the deliver callback to wrap deliver, because the existing other *_action callback wraps the action processing/rendering step.

To verify the above sentence,
I changed the CallbackMailer class to include before_action and after_action. Please find the gist here.

I ran a single test of rails/actionmailer/test/callbacks_test.rb and found the below order -

in before_action
in after_action
in before_deliver
in after_deliver

Expected order of callbacks -

1. before_action
2. before_deliver
3. after_deliver
4. after_action

To get this expected order of callbacks, many more significant changes would be required to the ActionMailer.

[zzak]

@alkesh26 Just going based on the documentation but my interpretation is that *_action is intended to occur before delivery takes place, for things like setup and configuration.

You can specify callbacks using before_action and after_action for configuring your messages,
and using before_deliver and after_deliver for wrapping the delivery process.

In that case, I think the behavior you're experiencing is intended.