Add a 'reply to' email address to some more emails #13176
Conversation
drummer83
added
the
user facing changes
drummer83
force-pushed
the
reply-to-email-address_part2
branch
from
4652553 to
71fa0f0
Compare
app/mailers/payment_mailer.rb
Outdated
| shop_owner = @payment.order.distributor.owner | ||
| @order = @payment.order | ||
| shop_owner = @order.distributor.owner | ||
| subject = I18n.t('spree.payment_mailer.authorization_required.subject', | ||
| order: @payment.order) | ||
| order: @order) | ||
| I18n.with_locale valid_locale(shop_owner) do | ||
| mail(to: shop_owner.email, | ||
| subject:) | ||
| subject:, | ||
| reply_to: @order.email) |
There was a problem hiding this comment.
In Ruby, the @var notation refers to instance variables that are available beyond the method in which they are defined. So in controllers they are often defined to be used in the views. And here, @payment can be used in the email template.
The new variable @order however, is not used in the template. It's only used for translating the subject and the reply_to field. So you can turn this into a normal local variable. Other developers then know that it's not used beyond this scope.
I'll add a commit to show you what I mean.
| @payment = payment | ||
| @order = @payment.order | ||
| @order = payment.order |
There was a problem hiding this comment.
Now looking at the views, I found that they were only referring to the order, not the payment. So instead of making order a local variable, I removed @payment as variable and changed the views to access @order directly.
|
Hey @drummer83 , Thanks! I could verify in staging that the emails you've changed arrive and display the reply-to email address of the customer: Payment Authorization (Subscription)
Order Confirmation
Order Cancellation (Hub)
Merging! PS: One comment, I'm a bit surprised that these changes to emails trigger no failures on automated tests. One one side of things, it's a good sign 😅 on another perspective, might mean we're a bit short on coverage here. I guess the reason is we're adding reply_to addresses, and not changing these. |
What? Why?
This builds on top of #13139 and follows the discussion here.
The following should be changed by this PR:
What should we test?
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.
Dependencies
Documentation updates