dev/financial#141 - Update return parameters on all payment processors to match expected results #22683
Conversation
|
(Standard links)
|
mattwire
force-pushed
the
paymentstatushelpersall
branch
from
3e4ca72
to
f38c76e
Compare
mattwire
force-pushed
the
paymentstatushelpersall
branch
from
f38c76e
to
ac8226f
Compare
mattwire
changed the title
|
mattwire
force-pushed
the
paymentstatushelpersall
branch
from
ac8226f
to
8e81988
Compare
| } | ||
|
|
||
| $params['trxn_result_code'] = $processorResponse['ssl_approval_code'] . "-Cvv2:" . $processorResponse['ssl_cvv2_response'] . "-avs:" . $processorResponse['ssl_avs_response']; | ||
| $params['payment_status_id'] = array_search('Completed', $statuses); |
There was a problem hiding this comment.
A couple of these processors set $params['trxn_result_code'] (line 236 here) but then it's not being set in $result, so the return value of the function won't contain it anymore.
Also the signature for doPayment is &$params, so by changing all the $params to $result, it is no longer altering $params like it did before. Maybe that's ok and no callers are relying on that - I'm not familiar with how doPayment() is used in practice. It might even be a good thing.
There was a problem hiding this comment.
@demeritcowboy I realised this PR didn't have the linked lab issue and previous PR - now added to the description. We can't actually change the pass-by-reference signature because that will cause PHP errors but basically no calling code should be using the pass-by-reference result (none is in core and I'm not aware of third-party code that is).
trxn_result_code was set by various core processors but is not used anywhere - maybe it was once upon a time.
| $params['trxn_result_code'] = serialize($extras); | ||
| $params['currencyID'] = $this->_getParam('currency'); | ||
| $params['fee_amount'] = 0; |
There was a problem hiding this comment.
Also currencyID here same as trxn_result_code.
There was a problem hiding this comment.
See comment above - have now added link to lab issue where we agreed return parameters.
|
Ok got it. Jenkins retest this please since it's been a while. |
demeritcowboy
changed the title
|
Thanks @demeritcowboy |
Overview
Switch to using helper functions
setStatusPaymentPending()andsetStatusPaymentCompleted()for all core payment processors. This will help with transitioning frompayment_status_idtopayment_status.See https://lab.civicrm.org/dev/financial/-/issues/141. Follow on from #22680
Before
Each payment processor sets the payment return status manually.
After
Each payment processor sets the payment return status by calling a standard helper function. This means that we have a path to deprecate
payment_status_idin the future by "switching it off" in the helper functions if we wanted to.Technical Details
Comments