[REF] Minor code clean up #17974
[REF] Minor code clean up #17974
Conversation
This just consolidates a little of the code used to determin if an email receipt should be sent. It's clear that there is more code above that could be pulled into this & combined with the isEmailReceipt function but I wanted to just offer up a small reviewable change unless there is reviewer enthusiasm for more
|
(Standard links)
|
| $isEmailReceipt = !array_key_exists('is_email_receipt', $values) || $values['is_email_receipt'] == 1; | ||
| if (isset($input['is_email_receipt'])) { | ||
| $isEmailReceipt = $input['is_email_receipt']; | ||
| } |
There was a problem hiding this comment.
This is a bit confusing - surely the first line is all that's needed?
isset will always be true if the array key exists, but if the array key exists then $isEmailReceipt will already be correctly set, with the exception of possible different values of truthy. As in it comes down to what values == 1.
I think this is equivalent to these 4 lines?
$isEmailReceipt = $values['is_email_receipt'] ?? 1;There was a problem hiding this comment.
@artfulrobot yes - probably - I was focussed on moving existing lines to a new order so we don't do
- set this
- set it to something else possibly
- set it again in case we changed it & didn't mean to (because input overrides $values)
& instead do
- set it to something else possibly
- set it to $input['is_email_receipt'] if that has been passed in as it clobbers
There was a problem hiding this comment.
ie. $input always overrides $values - if set.
There was a problem hiding this comment.
Oh, my bad, sorry. I'd missed that the 2nd if{} was about $input!
|
This looks fine as a cleanup |
Overview
This just consolidates a little of the code used to determine if an email receipt should be sent.
Before
$input['is_email_receipt'] always 'wins' if set but it does so by being handled in 2 places
After
$input['is_email_receipt'] is handled onnce.
Technical Details
It's clear that there is more code above that could be pulled into this & combined
with the isEmailReceipt function but I wanted to just offer up a small reviewable change unless
there is reviewer enthusiasm for more
Comments