Skip delivery notification when perform_deliveries is false.

[yskkin]

Summary

If perform_deliveries is false, current master logs delivery of mail even if it is not delivered actually.
I think delivery notification should be skipped for this case.

[rails-bot]

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @schneems (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review.

Please see the contribution instructions for more information.

[y-yagi]

This seems the expected behavior. Ref: #32754 (comment)

[eugeneius]

How about logging a different message when delivery is disabled, e.g.

Skipped sending mail to test@email.com as perform_deliveries is false

[y-yagi]

Or how about including perform_deliveries in payload?
This will allow the user to deal with notificaiton when perform_deliveries is false.

[yskkin]

I think both of changing message and adding payload is required since user cannot detach LogScbscriber and LogScbscriber cannot know perform_deriveries without adding it to payload.
I'll update PR later unless I missed something.

[yskkin]

@y-yagi @eugeneius Thank you for review!
I updated my PR.

[yskkin]

Thanks ❤️

By the way, as I added to the guide, if perform_deliveries is false, AM::Base.deliveries array will not be populated even if delivery_method is :test.

To me, it does not align well with #32754 (comment). Is this bug?

the action of delivering the mail message is still performed and logged so people can for example see what is going to be delivered

[rafaelfranca]

No. The delivery is not made so we don't want to add the the deliveries array. But we wan to log it specially because in development delivery is disabled by default.

[yskkin]

I got it. Thank you for answer!

[LilyReile]

@yskkin @rafaelfranca

In Rails 5 ActionMailer::Base.deliveries was populated by deliver_later, right? Do you know which commit made that change?