Replace CRM_Utils_Array::value in contribution/confirm and bao/membership #18157
Conversation
|
(Standard links)
|
| $pending = $this->getIsPending(); | ||
| // The concept of contributeMode is deprecated. | ||
| // the is_monetary concept probably should be too as it can be calculated from | ||
| // the existence of 'amount' & seems fragile. | ||
| if (((isset($this->_contributeMode)) || !empty($this->_params['is_pay_later']) | ||
| ) && | ||
| (($this->_values['is_monetary'] && $this->_amount > 0.0)) | ||
| ) { | ||
| $pending = TRUE; | ||
| } | ||
| $pending = FALSE; |
There was a problem hiding this comment.
This function is only used here. We want to move it to BAO/membership but first step is remove function and put inline so you can see what it is doing.
This is the only bit that is not replacing CRM_Utils_Array::value
| list($membership, $renewalMode, $dates) = CRM_Member_BAO_Membership::processMembership( | ||
| $contactID, $memType, $isTest, | ||
| date('YmdHis'), CRM_Utils_Array::value('cms_contactID', $membershipParams), | ||
| date('YmdHis'), $membershipParams['cms_contactID'] ?? NULL, | ||
| $customFieldsFormatted, |
There was a problem hiding this comment.
@eileenmcnaughton cms_contactID would be a good variable to eliminate in a future PR because it is only used in very few circumstances and is almost certainly available via other parameters.
|
I've reviewed the code & am happy with this if tests pass. |
556c8a3 to
b92c6ce
Compare
| @@ -155,19 +155,19 @@ public static function getContributionParams( | |||
| $paymentProcessorOutcome, $receiptDate, $recurringContributionID) { | |||
| $contributionParams = [ | |||
| 'financial_type_id' => $financialTypeID, | |||
| 'receive_date' => (CRM_Utils_Array::value('receive_date', $params)) ? CRM_Utils_Date::processDate($params['receive_date']) : date('YmdHis'), | |||
| 'receive_date' => isset($params['receive_date']) ? CRM_Utils_Date::processDate($params['receive_date']) : date('YmdHis'), | |||
There was a problem hiding this comment.
Is it possible for $params['receive_date'] to be the empty string? If so then this now evaluates to null instead of today's date.
There was a problem hiding this comment.
@demeritcowboy My understanding of CRM_Utils_Array::value was that it would return the value of receive_date if set otherwise NULL. So I don't think this line changes anything?
There was a problem hiding this comment.
Yep that's what it does but consider when $params['receive_date'] = "";
It's also checkable with cv:
cv ev "$params['receive_date'] = ''; $result = (CRM_Utils_Array::value('receive_date', $params)) ? CRM_Utils_Date::processDate($params['receive_date']) : date('YmdHis'); var_dump($result);"
vs
cv ev "$params['receive_date'] = ''; $result = isset($params['receive_date']) ? CRM_Utils_Date::processDate($params['receive_date']) : date('YmdHis'); var_dump($result);"
The difference would also come up if $params['receive_date'] = 0; or $params['receive_date'] = FALSE;, but that seems much less likely to happen.
There was a problem hiding this comment.
Oh, I see now. I'll update the PR :-)
| @@ -887,7 +887,7 @@ public static function processFormContribution( | |||
|
|
|||
| $isEmailReceipt = !empty($form->_values['is_email_receipt']); | |||
| $isSeparateMembershipPayment = !empty($params['separate_membership_payment']); | |||
| $pledgeID = !empty($params['pledge_id']) ? $params['pledge_id'] : CRM_Utils_Array::value('pledge_id', $form->_values); | |||
| $pledgeID = !empty($params['pledge_id']) ? $params['pledge_id'] : $form->_values['pledge_id'] ?? NULL; | |||
There was a problem hiding this comment.
This turns out to be ok no matter which has precedence, just might be slightly clearer with brackets around the last clause, i.e. ($form->_values['pledge_id'] ?? NULL)
| @@ -2398,7 +2394,7 @@ public static function recordMembershipContribution(&$params) { | |||
| $contributionParams = []; | |||
| $config = CRM_Core_Config::singleton(); | |||
| $contributionParams['currency'] = $config->defaultCurrency; | |||
| $contributionParams['receipt_date'] = (CRM_Utils_Array::value('receipt_date', $params)) ? $params['receipt_date'] : 'null'; | |||
| $contributionParams['receipt_date'] = isset($params['receipt_date']) ? $params['receipt_date'] : 'null'; | |||
There was a problem hiding this comment.
Same comment as earlier if it's possible to be the empty string.
|
related fail |
|
(style) |
b92c6ce to
699e088
Compare
Overview
Simple replace of deprecated function - helps make code more readable for further changes.
Before
Lot's of uses of
CRM_Utils_Array::valueAfter
No uses
Technical Details
There are almost certainly further simplifications in some cases. In most cases I've just replaced with
?? NULLwhere there was no default.Comments