Fix Contribution.create to not attempt to set contacts on activity update #19202

eileenmcnaughton · 2020-12-14T01:48:57Z

Overview

This fixes a bug whereby updating the status of a contribution where the related activity exists and has attached contacts results in the contacts being overwritten. This is relevant to the case where a pending activity is attached to more than one contact

Note this is in PR #19200 and covered in that test but I pulled it out for readability.

#19200

Before

The values passed to Activity->save() contain target & source contacts regardless of whether it is an update or a new activity. The ids used are less likely to be accurate during the update than in the original create as the create is likely to be in a form context

After

Original values are not overwritten

Technical Details

Comments

civibot · 2020-12-14T01:49:00Z

(Standard links)

If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

…date In udpate cases the original allocations were more correct

eileenmcnaughton · 2020-12-14T04:59:44Z

test this please

eileenmcnaughton · 2020-12-21T04:11:09Z

@demeritcowboy is this one you could review?

eileenmcnaughton · 2020-12-21T04:22:00Z

Note test cover in related PR (now stale but I will rebase once this is merged)

demeritcowboy · 2020-12-21T16:14:13Z

I'm having trouble seeing where this comes up and reproducing it? "pending activity is attached to more than one contact" - what does that mean? Multiple targets for a contribution?

eileenmcnaughton · 2020-12-21T19:51:03Z

@demeritcowboy yes sorry - that would be the case in an on-behalf of contribution. When you donate on behalf the source is the individual and the target is the organization (I think it's that way around). A pending contribution is created & a scheduled activity & then completeOrder is called. Currently the Contribution.create BAO messes with the activity and then the completeOrder function fixes it again - see https://github.com/civicrm/civicrm-core/pull/19200/files#diff-4c9d0b1abe07057a4eea2b47bc627eecb95face8ed8d86c1c005312a52cca811L4353

(I'll rebase that PR now too in case it's easier to look at that than this)

MegaphoneJon · 2020-12-21T21:03:06Z

I'm experiencing what I believe is a variant on this. It happened when I upgraded a site to 5.32 (from 5.31).

I have a CiviRule that sets a thank-you date on a contribution, which updates the contribution via API. On 5.31 this works; on 5.32, I get a SQL error because it's trying to create an ActivityContact record that already exists.

Dec 20 14:21:50  [error] $Fatal Error Details = Array
(
    [callback] => Array
        (
            [0] => CRM_Core_Error
            [1] => exceptionHandler
        )

    [code] => -5
    [message] => DB Error: already exists
    [mode] => 16
    [debug_info] => INSERT INTO `civicrm_activity_contact` (`activity_id` , `contact_id` , `record_type_id` ) VALUES ( 236352 ,  26686 ,  2 )  [nativecode=1062 ** Duplicate entry '26686-236352-2' for key 'UI_activity_contact']
    [type] => DB_Error
    [user_info] => INSERT INTO `civicrm_activity_contact` (`activity_id` , `contact_id` , `record_type_id` ) VALUES ( 236352 ,  26686 ,  2 )  [nativecode=1062 ** Duplicate entry '26686-236352-2' for key 'UI_activity_contact']
    [to_string] => [db_error: message="DB Error: already exists" code=-5 mode=callback callback=CRM_Core_Error::exceptionHandler prefix="" info="INSERT INTO `civicrm_activity_contact` (`activity_id` , `contact_id` , `record_type_id` ) VALUES ( 236352 ,  26686 ,  2 )  [nativecode=1062 ** Duplicate entry '26686-236352-2' for key 'UI_activity_contact']"]
)

eileenmcnaughton · 2020-12-21T22:45:21Z

@MegaphoneJon does this patch fix it for you in that scenario?

MegaphoneJon · 2020-12-21T23:05:15Z

@eileenmcnaughton I've been trying and failing to replicate this problem in a dev environment, but I've reviewed the code and feel comfortable deploying this in production. Because of the intermittent nature, I would need a day or two to collect sufficient data to report if it's working - but it looks good IMO.

That said - Activity::save should never choke on a duplicate activity contact. That IMO is a bug in the API4 Activity entity, but is out of scope here.

demeritcowboy · 2020-12-21T23:06:27Z

CRM/Contribute/BAO/Contribution.php

+        // correctly created and we risk overwriting them with
+        // 'best guess' params.
+        $activityParams['source_contact_id'] = (int) ($params['source_contact_id'] ?? (CRM_Core_Session::getLoggedInContactID() ?: $contribution->contact_id));
+        $activityParams['target_contact_id'] = ($activityParams['source_contact_id'] === (int) $contribution->contact_id) ? [] : [$contribution->contact_id];


Summarizing the logic here: $params['source_contact_id'] seems to be a new thing that means "the on-behalf-of individual", so the first part is "if we came from on-behalf-of, use the individual contributor as the activity source contact, otherwise do what we did before". Then the second part seems to be "if we have just set the activity source to the same thing as the contribution contact, then blank out the existing activity target (because otherwise it will duplicate source), otherwise overwrite with the contribution contact", so it's saying that the contribution contact is the correct one to use for the activity target, regardless of what was on the activity before or whether it's on-behalf-of or other. That might be right, it's just trying to convince myself it's true in the generic case.

Ok I misread the part above. So this block is only for new activities. So then my question would be is changing a contribution contact by calling api contribution.create an officially supported operation, and if so, what should happen with the activity's contacts. Before it would update the activity to match, now the activity target is still the old contact.

@demeritcowboy I don't think changing an activity via contribution create is supported.

The flow we need to work for repeattransactions is to be able to create a new contribution with the same activity contacts as the template activity. Since the contribution.create is taking responsibility for creating the activity I think it needs to do so with the correct activity contacts

Ok I don't have anything else then.

Thanks @demeritcowboy I'll see how @MegaphoneJon goes with this patch over the next few days

demeritcowboy · 2020-12-21T23:11:33Z

@MegaphoneJon was there a stack trace with that error - is it hitting these lines or a different set of lines?

MegaphoneJon · 2020-12-22T18:32:40Z

My client is reporting that this patch fixed their issue.

@demeritcowboy I have a backtrace, but to my surprise I found instances of this that predated my 5.32 update, so on the one hand it seems unrelated - on the other, it just as clear must be related.

Dec 21 15:00:56  [debug] $backTrace = #0 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Core/Error.php(942): CRM_Core_Error::backtrace("backTrace", TRUE)
#1 /var/www/crm.example.org/vendor/pear/pear-core-minimal/src/PEAR.php(944): CRM_Core_Error::exceptionHandler(Object(DB_Error))
#2 /var/www/crm.example.org/vendor/pear/db/DB.php(997): PEAR_Error->__construct("DB Error: already exists", -5, 16, (Array:2), "INSERT INTO `civicrm_activity_contact` (`activity_id` , `contact_id` , `recor...")
#3 /var/www/crm.example.org/vendor/pear/pear-core-minimal/src/PEAR.php(575): DB_Error->__construct(-5, 16, (Array:2), "INSERT INTO `civicrm_activity_contact` (`activity_id` , `contact_id` , `recor...")
#4 /var/www/crm.example.org/vendor/pear/pear-core-minimal/src/PEAR.php(223): PEAR->_raiseError(Object(DB_mysqli), NULL, -5, 16, (Array:2), "INSERT INTO `civicrm_activity_contact` (`activity_id` , `contact_id` , `recor...", "DB_Error", TRUE)
#5 /var/www/crm.example.org/vendor/pear/db/DB/common.php(1928): PEAR->__call("raiseError", (Array:7))
#6 /var/www/crm.example.org/vendor/pear/db/DB/mysqli.php(936): DB_common->raiseError(-5, NULL, NULL, "INSERT INTO `civicrm_activity_contact` (`activity_id` , `contact_id` , `recor...", "1062 ** Duplicate entry '55243-236379-2' for key 'UI_activity_contact'")
#7 /var/www/crm.example.org/vendor/pear/db/DB/mysqli.php(406): DB_mysqli->mysqliRaiseError()
#8 /var/www/crm.example.org/vendor/pear/db/DB/common.php(1234): DB_mysqli->simpleQuery("INSERT INTO `civicrm_activity_contact` (`activity_id` , `contact_id` , `recor...")
#9 /var/www/crm.example.org/vendor/civicrm/civicrm-packages/DB/DataObject.php(2696): DB_common->query("INSERT INTO `civicrm_activity_contact` (`activity_id` , `contact_id` , `recor...")
#10 /var/www/crm.example.org/vendor/civicrm/civicrm-packages/DB/DataObject.php(1245): DB_DataObject->_query("INSERT INTO `civicrm_activity_contact` (`activity_id` , `contact_id` , `recor...")
#11 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Core/DAO.php(639): DB_DataObject->insert()
#12 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Activity/BAO/ActivityContact.php(44): CRM_Core_DAO->save()
#13 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Api4/Generic/Traits/DAOActionTrait.php(140): CRM_Activity_BAO_ActivityContact::create((Array:4))
#14 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Api4/Generic/DAOSaveAction.php(41): Civi\Api4\Generic\DAOSaveAction->writeObjects((Array:1))
#15 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Api4/Provider/ActionObjectProvider.php(68): Civi\Api4\Generic\DAOSaveAction->_run(Object(Civi\Api4\Generic\Result))
#16 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Kernel.php(150): Civi\Api4\Provider\ActionObjectProvider->invoke(Object(Civi\Api4\Generic\DAOSaveAction))
#17 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Api4/Generic/AbstractAction.php(238): Civi\API\Kernel->runRequest(Object(Civi\Api4\Generic\DAOSaveAction))
#18 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Activity/BAO/Activity.php(411): Civi\Api4\Generic\AbstractAction->execute()
#19 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Api4/Generic/Traits/DAOActionTrait.php(140): CRM_Activity_BAO_Activity::create((Array:11))
#20 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Api4/Generic/DAOSaveAction.php(41): Civi\Api4\Generic\DAOSaveAction->writeObjects((Array:1))
#21 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Api4/Provider/ActionObjectProvider.php(68): Civi\Api4\Generic\DAOSaveAction->_run(Object(Civi\Api4\Generic\Result))
#22 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Kernel.php(150): Civi\Api4\Provider\ActionObjectProvider->invoke(Object(Civi\Api4\Generic\DAOSaveAction))
#23 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Api4/Generic/AbstractAction.php(238): Civi\API\Kernel->runRequest(Object(Civi\Api4\Generic\DAOSaveAction))
#24 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Contribute/BAO/Contribution.php(522): Civi\Api4\Generic\AbstractAction->execute()
#25 /var/www/crm.example.org/vendor/civicrm/civicrm-core/api/v3/utils.php(1297): CRM_Contribute_BAO_Contribution::create((Array:14), (Array:1))
#26 /var/www/crm.example.org/vendor/civicrm/civicrm-core/api/v3/Contribution.php(77): _civicrm_api3_basic_create("CRM_Contribute_BAO_Contribution", (Array:14), "Contribution")
#27 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Provider/MagicFunctionProvider.php(89): civicrm_api3_contribution_create((Array:14))
#28 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Kernel.php(150): Civi\API\Provider\MagicFunctionProvider->invoke((Array:8))
#29 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Kernel.php(81): Civi\API\Kernel->runRequest((Array:8))
#30 /var/www/crm.example.org/vendor/civicrm/civicrm-core/api/api.php(131): Civi\API\Kernel->runSafe("Contribution", "Create", (Array:3))
#31 /var/www/crm.example.org/web/sites/all/civicrm/extensions/org.civicoop.civirules/CRM/CivirulesActions/Contribution/ThankYouDate.php(46): civicrm_api3("Contribution", "Create", (Array:3))
#32 /var/www/crm.example.org/web/sites/all/civicrm/extensions/org.civicoop.civirules/CRM/Civirules/ActionEngine/RuleActionEngine.php(29): CRM_CivirulesActions_Contribution_ThankYouDate->processAction(Object(CRM_Civirules_TriggerData_Edit))
#33 /var/www/crm.example.org/web/sites/all/civicrm/extensions/org.civicoop.civirules/CRM/Civirules/Engine.php(83): CRM_Civirules_ActionEngine_RuleActionEngine->execute()
#34 /var/www/crm.example.org/web/sites/all/civicrm/extensions/org.civicoop.civirules/CRM/Civirules/Engine.php(58): CRM_Civirules_Engine::executeAction(Object(CRM_Civirules_TriggerData_Edit), (Array:6))
#35 /var/www/crm.example.org/web/sites/all/civicrm/extensions/org.civicoop.civirules/CRM/Civirules/Engine.php(31): CRM_Civirules_Engine::executeActions(Object(CRM_Civirules_TriggerData_Edit))
#36 /var/www/crm.example.org/web/sites/all/civicrm/extensions/org.civicoop.civirules/CRM/Civirules/Trigger/Post.php(108): CRM_Civirules_Engine::triggerRule(Object(CRM_CivirulesPostTrigger_Contribution), Object(CRM_Civirules_TriggerData_Edit))
#37 /var/www/crm.example.org/web/sites/all/civicrm/extensions/org.civicoop.civirules/CRM/Civirules/Trigger/Post.php(93): CRM_Civirules_Trigger_Post->triggerTrigger("edit", "Contribution", 176587, Object(CRM_Contribute_BAO_Contribution))
#38 /var/www/crm.example.org/web/sites/all/civicrm/extensions/org.civicoop.civirules/civirules.php(276): CRM_Civirules_Trigger_Post::post("edit", "Contribution", 176587, Object(CRM_Contribute_BAO_Contribution))
#39 [internal function](): civirules_civicrm_post_callback("edit", "Contribution", 176587, Object(CRM_Contribute_BAO_Contribution))
#40 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Core/Transaction/Frame.php(198): call_user_func_array("civirules_civicrm_post_callback", (Array:4))
#41 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Core/Transaction/Frame.php(148): Civi\Core\Transaction\Frame->invokeCallbacks(2)
#42 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/Core/Transaction/Manager.php(103): Civi\Core\Transaction\Frame->finish()
#43 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Core/Transaction.php(126): Civi\Core\Transaction\Manager->dec()
#44 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Contribute/BAO/Contribution.php(4476): CRM_Core_Transaction->commit()
#45 /var/www/crm.example.org/vendor/civicrm/civicrm-core/api/v3/Contribution.php(683): CRM_Contribute_BAO_Contribution::completeOrder((Array:10), NULL, Object(CRM_Contribute_BAO_Contribution), TRUE)
#46 /var/www/crm.example.org/vendor/civicrm/civicrm-core/api/v3/Contribution.php(497): _ipn_process_transaction((Array:7), Object(CRM_Contribute_BAO_Contribution), (Array:9), (Array:9))
#47 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Provider/MagicFunctionProvider.php(89): civicrm_api3_contribution_completetransaction((Array:7))
#48 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Kernel.php(150): Civi\API\Provider\MagicFunctionProvider->invoke((Array:8))
#49 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Kernel.php(81): Civi\API\Kernel->runRequest((Array:8))
#50 /var/www/crm.example.org/vendor/civicrm/civicrm-core/api/api.php(131): Civi\API\Kernel->runSafe("Contribution", "completetransaction", (Array:6))
#51 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Financial/BAO/Payment.php(161): civicrm_api3("Contribution", "completetransaction", (Array:6))
#52 /var/www/crm.example.org/web/sites/all/civicrm/extensions/mjwshared/api/v3/Mjwpayment.php(424): CRM_Financial_BAO_Payment::create((Array:10))
#53 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Provider/MagicFunctionProvider.php(89): civicrm_api3_mjwpayment_create_payment((Array:10))
#54 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Kernel.php(150): Civi\API\Provider\MagicFunctionProvider->invoke((Array:8))
#55 /var/www/crm.example.org/vendor/civicrm/civicrm-core/Civi/API/Kernel.php(81): Civi\API\Kernel->runRequest((Array:8))
#56 /var/www/crm.example.org/vendor/civicrm/civicrm-core/api/api.php(131): Civi\API\Kernel->runSafe("Mjwpayment", "create_payment", (Array:10))
#57 /var/www/crm.example.org/web/sites/all/civicrm/extensions/mjwshared/CRM/Core/Payment/MJWIPNTrait.php(332): civicrm_api3("Mjwpayment", "create_payment", (Array:10))
#58 /var/www/crm.example.org/web/sites/all/civicrm/extensions/stripe/CRM/Core/Payment/StripeIPN.php(357): CRM_Core_Payment_StripeIPN->updateContributionCompleted((Array:7))
#59 /var/www/crm.example.org/web/sites/all/civicrm/extensions/stripe/CRM/Core/Payment/Stripe.php(1148): CRM_Core_Payment_StripeIPN->main()
#60 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Core/Payment.php(1610): CRM_Core_Payment_Stripe->handlePaymentNotification()
#61 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Core/Payment.php(1503): CRM_Core_Payment::handlePaymentMethod("handlePaymentNotification", (Array:3))
#62 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Core/Invoke.php(278): CRM_Core_Payment::handleIPN()
#63 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Core/Invoke.php(68): CRM_Core_Invoke::runItem((Array:14))
#64 /var/www/crm.example.org/vendor/civicrm/civicrm-core/CRM/Core/Invoke.php(36): CRM_Core_Invoke::_invoke((Array:4))
#65 /var/www/crm.example.org/web/modules/contrib/civicrm/src/Civicrm.php(88): CRM_Core_Invoke::invoke((Array:4))
#66 /var/www/crm.example.org/web/modules/contrib/civicrm/src/Controller/CivicrmController.php(80): Drupal\civicrm\Civicrm->invoke((Array:4))
#67 [internal function](): Drupal\civicrm\Controller\CivicrmController->main((Array:4), "5")
#68 /var/www/crm.example.org/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array((Array:2), (Array:2))
#69 /var/www/crm.example.org/web/core/lib/Drupal/Core/Render/Renderer.php(573): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#70 /var/www/crm.example.org/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#71 /var/www/crm.example.org/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext((Array:2), (Array:2))
#72 /var/www/crm.example.org/vendor/symfony/http-kernel/HttpKernel.php(151): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#73 /var/www/crm.example.org/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#74 /var/www/crm.example.org/web/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, TRUE)
#75 /var/www/crm.example.org/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, TRUE)
#76 /var/www/crm.example.org/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, TRUE)
#77 /var/www/crm.example.org/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, TRUE)
#78 /var/www/crm.example.org/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, TRUE)
#79 /var/www/crm.example.org/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, TRUE)
#80 /var/www/crm.example.org/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, TRUE)
#81 /var/www/crm.example.org/web/core/lib/Drupal/Core/DrupalKernel.php(708): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, TRUE)
#82 /var/www/crm.example.org/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#83 {main}

demeritcowboy · 2020-12-22T18:49:56Z

CRM/Activity/BAO/Activity.php(411)

Maybe it's the logic just above that line, or a permissions thing, e.g. when someone without delete permission goes to update, if the DELETE above that line silently fails, then it would lead to a duplicate when it tries to insert. Except I can't seem to make that happen. But that would explain the pre-5.32 fails then.

eileenmcnaughton · 2020-12-22T19:30:02Z

@demeritcowboy @MegaphoneJon do we have an opinion on whether this should be merged?

demeritcowboy · 2020-12-22T19:33:22Z

No objection here.

MegaphoneJon · 2020-12-22T19:40:41Z

No objection here either.

eileenmcnaughton · 2020-12-22T19:44:25Z

@seamuslee001 @colemanw could one of you merge based on the (ahem soft) +1s above

MegaphoneJon · 2020-12-22T19:45:46Z

I don't know how to replicate the original set of circumstances, which is why it's "soft" - but it definitely fixes a bug I'm seeing, and my review of the code suggests that it shouldn't have negative side effects. I'm definitely in favor of merging it.

eileenmcnaughton · 2020-12-22T19:48:10Z

:-)

eileenmcnaughton · 2020-12-22T19:49:03Z

@MegaphoneJon in fact the original bug is more of a cleanup - ie there is extra code in completeOrder that should not be needed

seamuslee001 · 2020-12-22T21:41:23Z

merging as per reviews from @demeritcowboy @MegaphoneJon

eileenmcnaughton · 2020-12-22T22:13:43Z

Thanks @demeritcowboy @seamuslee001 @MegaphoneJon - I've rebased #19200 which adds the test where this was encountered

civibot bot added the master label Dec 14, 2020

Fix Contribution.create to not attempt to set contacts on activity up…

6fd7535

…date In udpate cases the original allocations were more correct

eileenmcnaughton force-pushed the act_cont2 branch from 3df7ea6 to 6fd7535 Compare December 14, 2020 04:55

demeritcowboy reviewed Dec 21, 2020

View reviewed changes

seamuslee001 merged commit 5c2a0a5 into civicrm:master Dec 22, 2020

seamuslee001 deleted the act_cont2 branch December 22, 2020 21:41

mflandorfer mentioned this pull request Mar 16, 2021

dev/core#2450 - Update source/target contacts on contribution updates #19820

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Contribution.create to not attempt to set contacts on activity update #19202

Fix Contribution.create to not attempt to set contacts on activity update #19202

eileenmcnaughton commented Dec 14, 2020

civibot bot commented Dec 14, 2020

eileenmcnaughton commented Dec 14, 2020

eileenmcnaughton commented Dec 21, 2020

eileenmcnaughton commented Dec 21, 2020

demeritcowboy commented Dec 21, 2020

eileenmcnaughton commented Dec 21, 2020

MegaphoneJon commented Dec 21, 2020

eileenmcnaughton commented Dec 21, 2020

MegaphoneJon commented Dec 21, 2020

demeritcowboy Dec 21, 2020

demeritcowboy Dec 22, 2020

eileenmcnaughton Dec 22, 2020

demeritcowboy Dec 22, 2020

eileenmcnaughton Dec 22, 2020

demeritcowboy commented Dec 21, 2020

MegaphoneJon commented Dec 22, 2020

demeritcowboy commented Dec 22, 2020

eileenmcnaughton commented Dec 22, 2020

demeritcowboy commented Dec 22, 2020

MegaphoneJon commented Dec 22, 2020

eileenmcnaughton commented Dec 22, 2020

MegaphoneJon commented Dec 22, 2020

eileenmcnaughton commented Dec 22, 2020

eileenmcnaughton commented Dec 22, 2020

seamuslee001 commented Dec 22, 2020

eileenmcnaughton commented Dec 22, 2020

Fix Contribution.create to not attempt to set contacts on activity update #19202

Fix Contribution.create to not attempt to set contacts on activity update #19202

Conversation

eileenmcnaughton commented Dec 14, 2020

Overview

Before

After

Technical Details

Comments

civibot bot commented Dec 14, 2020

eileenmcnaughton commented Dec 14, 2020

eileenmcnaughton commented Dec 21, 2020

eileenmcnaughton commented Dec 21, 2020

demeritcowboy commented Dec 21, 2020

eileenmcnaughton commented Dec 21, 2020

MegaphoneJon commented Dec 21, 2020

eileenmcnaughton commented Dec 21, 2020

MegaphoneJon commented Dec 21, 2020

demeritcowboy Dec 21, 2020

Choose a reason for hiding this comment

demeritcowboy Dec 22, 2020

Choose a reason for hiding this comment

eileenmcnaughton Dec 22, 2020

Choose a reason for hiding this comment

demeritcowboy Dec 22, 2020

Choose a reason for hiding this comment

eileenmcnaughton Dec 22, 2020

Choose a reason for hiding this comment

demeritcowboy commented Dec 21, 2020

MegaphoneJon commented Dec 22, 2020

demeritcowboy commented Dec 22, 2020

eileenmcnaughton commented Dec 22, 2020

demeritcowboy commented Dec 22, 2020

MegaphoneJon commented Dec 22, 2020

eileenmcnaughton commented Dec 22, 2020

MegaphoneJon commented Dec 22, 2020

eileenmcnaughton commented Dec 22, 2020

eileenmcnaughton commented Dec 22, 2020

seamuslee001 commented Dec 22, 2020

eileenmcnaughton commented Dec 22, 2020