Simplify / Cleanup Contribution BAO create #17115
Conversation
|
(Standard links)
|
|
". In some cases this will result in one extra database query per request, but in multiple cases it will reduce the number of queries later on."
|
| ]; | ||
| if (!$noteParams['contact_id']) { | ||
| $noteParams['contact_id'] = $params['contact_id']; | ||
| } | ||
| CRM_Core_BAO_Note::add($noteParams); | ||
| } | ||
|
|
||
| // make entry in batch entity batch table |
There was a problem hiding this comment.
So all of this is to do a load if we don't have all the contribution fields - maybe a helper function that identifies if there IS missing required info
c4b414b to
65ec8f1
Compare
| if ($retrieveRequired == 1) { | ||
| $contribution->find(TRUE); | ||
| } | ||
| } |
There was a problem hiding this comment.
There was a problem hiding this comment.
Looks like it does get hit at least in one place https://test.civicrm.org/job/CiviCRM-Core-PR/33520/testReport/junit/(root)/CRM_Contribute_Form_SearchTest/testBatchFilter/
There was a problem hiding this comment.
Yep - it's not clear the loading that is happening adds any value though!
| CRM_Activity_BAO_Activity::addActivity($contribution, 'Contribution'); | ||
|
|
||
| // do not add to recent items for import, CRM-4399 | ||
| if (empty($params['skipRecentView'])) { | ||
| $url = CRM_Utils_System::url('civicrm/contact/view/contribution', | ||
| "action=view&reset=1&id={$contribution->id}&cid={$contribution->contact_id}&context=home" | ||
| ); | ||
| // in some update cases we need to get extra fields - ie an update that doesn't pass in all these params | ||
| $titleFields = [ | ||
| 'contact_id', |
There was a problem hiding this comment.
Ah - so we need these for addRecent. Maybe we just need a function to addContributionRecent that loads extra if need-be
…ing if bits are set. Fix bug where retrieve fails if you had passed in an unrounded fee_amount which led to addActivity failing with missing contact ID
…:testSendconfirmationPayLater
65ec8f1 to
e74f01c
Compare
Overview
Retrieve full contribution object once so we don't have to keep checking if bits are set. Fix bug where DAO retrieve fails if you had passed in an unrounded
fee_amountwhich causesaddActivity ()to fail with missing contact ID and triggering an obscure message "Failed to update contribution in database" but only if there is no logged in user (because otherwise it will default to the logged in contact ID and not fail).Before
CRM_Contribute_BAO_Contribution::createfunction complex and does not always retrieve the params needed. Also has various bits of code just to deal with retrieving missing params.After
Call
$contribution->find(TRUE)once from add. In some cases this will result in one extra database query per request, but in multiple cases it will reduce the number of queries later on. And it significantly simplifies the callingcreatefunction which will make maintenance and improvements to this function much easier.Technical Details
Once the contribution has been created in the database we retrieve it using standard DAO functions.
We only use the contribution ID to retrieve it because we always have this value after saving. This prevents unrounded amounts causing the database select to fail (eg. fee_amount=0.3142424 gets saved as 0.31 in the database but then the contribution cannot be retrieved with the same params (because 0.3142424 != 0.31). The
$paramsarray that is passed in is copied to the contribution before saving so we won't lose any data.Comments
There are not many payment processors I'm aware of that actually record the fees. But if we do then the anonymous user is likely to see an error page despite the payment going through successfully.