REF Filter params in completetransaction #18321
Conversation
|
(Standard links)
|
| if (isset($params['payment_processor_id'])) { | ||
| $input['payment_processor_id'] = $params['payment_processor_id']; | ||
| } |
There was a problem hiding this comment.
Moving this here prepares for a follow-up PR by making it clear that payment_processor_id is only used by loadRelatedObjects
|
The next follow-up will allow us to get rid of |
|
I dropped the sig-refactor label since you added our standard one on & it's a duplicate |
| if (!$contribution->loadRelatedObjects($input, $ids, TRUE)) { | ||
| throw new API_Exception('failed to load related objects'); | ||
| } | ||
| elseif ($contribution->contribution_status_id == CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed')) { | ||
| throw new API_Exception(ts('Contribution already completed'), 'contribution_completed'); | ||
| } | ||
| $input['trxn_id'] = $params['trxn_id'] ?? $contribution->trxn_id; | ||
| $params['trxn_id'] = $params['trxn_id'] ?? $contribution->trxn_id; |
There was a problem hiding this comment.
This is setting the same value twice?
|
I'm doing a triage pass & I think think this is ok-without-test. In general the first goal of refactoring is to identify gaps in test cover as they relate to the code being touched but I can't think of one here |
|
Despite the mistake I pointed out I'm going to merge this. I agree we should pass payment_processor_id into completeOrder as 'input' I have doubts about our current practice of loading trxn_id if one is not supplied - I guess we are probably stuck with that until we switch the code over to call this via Payment.create rather than directly |
Overview
Partial from #18313. This makes it clear which
$inputparams are being set from completetransaction and makes the implementation consistent with repeattransaction.Technical Details
Comments