CRM-19710 - Preserve is_email_receipt parameter through to email sent #9487
Conversation
agileware
commented
Dec 2, 2016
•
agileware
commented
- CRM-19710: Preserve is_email_receipt parameter through to email sent
…se in _gatherMessageValues()
| //not really sure what params might be passed in but lets merge em into values | ||
| $values = array_merge($this->_gatherMessageValues($input, $values, $ids), $values); | ||
| $values = array_merge($this->_gatherMessageValues($input, $values, $ids), $tmp_values); | ||
| if (!empty($input['receipt_date'])) { | ||
| $values['receipt_date'] = $input['receipt_date']; | ||
| } |
There was a problem hiding this comment.
@agileware Can we assign $values after _gatherMessageValues() is done similar to receipt_date? Since we could avoid defining $tmp_values in that case.
There was a problem hiding this comment.
Yes we can, but if any extra options are passed through in $values that differ from any options after _gatherMessageValues(), they'll be lost anyway.
e.g. If receipt_from_email is passed along in $values, but differs from whatever is retrieved after _gatherMessageValues().
Not sure if/when this would ever be a problem. If you think it's not an issue, let me know and we'll adjust the code.
There was a problem hiding this comment.
Let's ditch this bit because I feel it's blocking the PR. That function is only called from here & ideally it would be dismantled into the calling function in bits e.g
extract this
if ($this->address_id) {
$addressParams = array('id' => $this->address_id);
$addressDetails = CRM_Core_BAO_Address::getValues($addressParams, FALSE, 'id');
$addressDetails = array_values($addressDetails);
}
// Else we assign the billing address of the contribution contact.
else {
$addressParams = array('contact_id' => $this->contact_id, 'is_billing' => 1);
$addressDetails = (array) CRM_Core_BAO_Address::getValues($addressParams);
$addressDetails = array_values($addressDetails);
}
if (!empty($addressDetails[0]['display'])) {
$values['address'] = $addressDetails[0]['display'];
}
into a function & call from composeMessageArray, ideally with tests, and possibly some review of how consistently that address token is used.
slow & painful, yes.
| @@ -4684,6 +4694,7 @@ public static function completeOrder(&$input, &$ids, $objects, $transaction, $re | |||
| civicrm_api3('Contribution', 'sendconfirmation', array( | |||
| 'id' => $contribution->id, | |||
| 'payment_processor_id' => $paymentProcessorId, | |||
| 'is_email_receipt' => $values['is_email_receipt'], | |||
There was a problem hiding this comment.
I think the test failures are related to undefined index is_email_receipt here. Can you use CRM_Utils_Array::value('is_email_receipt', $values ) and check ?
There was a problem hiding this comment.
Looks like is_email_receipt wasn't included as part of the $allowedParams in sendconfirmation. Committed and will be included in the updated PR.
|
Test failures seems related - https://test.civicrm.org/job/CiviCRM-Core-PR/12878/testReport/ |
… key does not exist
---------------------------------------- * CRM-19710: Preserve is_email_receipt parameter through to email sent https://issues.civicrm.org/jira/browse/CRM-19710
| @@ -4684,6 +4695,7 @@ public static function completeOrder(&$input, &$ids, $objects, $transaction, $re | |||
| civicrm_api3('Contribution', 'sendconfirmation', array( | |||
| 'id' => $contribution->id, | |||
| 'payment_processor_id' => $paymentProcessorId, | |||
| 'is_email_receipt' => (array_key_exists('is_email_receipt', $values) ? $values['is_email_receipt'] : 0), | |||
There was a problem hiding this comment.
@agileware Instead of using ternary operator, can we use existing function which does this ?
'is_email_receipt' => CRM_Utils_Array::value('is_email_receipt', $values);
There was a problem hiding this comment.
Can do. Updating now.
|
Latest test failed due to not being able to find the string ' |
|
Last failing test was for a test we didn't write. Is ours okay to go? |
|
Jenkins re test this please |
|
Any movement on this one? |
|
jenkins, retest this please |
|
@agileware The test failure https://test.civicrm.org/job/CiviCRM-Core-PR/13338/ is related to this PR. |
|
jenkins, retest this please |
|
(Couldn't get at results) |
|
test this please |
|
test this please |
|
@agileware I am proposing a replacement PR = #10000 - can you take a look? & spot the lucky number! |
|
Closing in favour of #10000 |
|
Ohh I got a heart emoji! Not sure I've ever had one of them before :-) |
