Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REF] Minor code clean up #17974

Merged
merged 1 commit into from
Jul 27, 2020
Merged

[REF] Minor code clean up #17974

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions CRM/Contribute/BAO/Contribution.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4489,9 +4489,6 @@ public static function completeOrder($input, &$ids, $objects, $transaction = NUL
$contributionParams['financial_type_id'] = $contribution->financial_type_id;

$values = [];
if (isset($input['is_email_receipt'])) {
$values['is_email_receipt'] = $input['is_email_receipt'];
}

if ($input['component'] == 'contribute') {
if ($contribution->contribution_page_id) {
Expand All @@ -4505,11 +4502,7 @@ public static function completeOrder($input, &$ids, $objects, $transaction = NUL
$values['title'] = $source = ts('Offline Recurring Contribution');
}

if (isset($input['is_email_receipt'])) {
// CRM-19601 - we may have overwritten this above.
$values['is_email_receipt'] = $input['is_email_receipt'];
}
elseif ($recurContrib && $recurringContributionID) {
if ($recurContrib && $recurringContributionID) {
//CRM-13273 - is_email_receipt setting on recurring contribution should take precedence over contribution page setting
// but CRM-16124 if $input['is_email_receipt'] is set then that should not be overridden.
// dev/core#1245 this maybe not the desired effect because the default value for is_email_receipt is set to 0 rather than 1 in
Expand Down Expand Up @@ -4577,11 +4570,13 @@ public static function completeOrder($input, &$ids, $objects, $transaction = NUL
CRM_Activity_BAO_Activity::addActivity($contribution, NULL, $targetContactID);
}

$isEmailReceipt = !array_key_exists('is_email_receipt', $values) || $values['is_email_receipt'] == 1;
if (isset($input['is_email_receipt'])) {
$isEmailReceipt = $input['is_email_receipt'];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Contributor Author

@eileenmcnaughton eileenmcnaughton Jul 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ie. $input always overrides $values - if set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, my bad, sorry. I'd missed that the 2nd if{} was about $input!

// CRM-9132 legacy behaviour was that receipts were sent out in all instances. Still sending
// when array_key 'is_email_receipt doesn't exist in case some instances where is needs setting haven't been set
if (!array_key_exists('is_email_receipt', $values) ||
$values['is_email_receipt'] == 1
) {
if ($isEmailReceipt) {
civicrm_api3('Contribution', 'sendconfirmation', [
'id' => $contribution->id,
'payment_processor_id' => $paymentProcessorId,
Expand Down