From 87a51ee63e16d266f19c066206feef1b9e851114 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Sat, 2 Jul 2022 09:13:07 +1200 Subject: [PATCH] Upgrade script for contribution_recur amount, fix loading --- CRM/Contribute/BAO/Contribution.php | 5 ++-- CRM/Contribute/BAO/ContributionRecur.php | 28 +++++++++---------- .../Incremental/sql/5.52.alpha1.mysql.tpl | 8 ++++++ tests/phpunit/api/v3/ContributionTest.php | 10 +++++++ 4 files changed, 34 insertions(+), 17 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index e90d9eb220..0ad9215b24 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -2162,12 +2162,11 @@ LEFT JOIN civicrm_contribution contribution ON ( componentPayment.contribution_ protected static function repeatTransaction(array $input, array $contributionParams) { $templateContribution = CRM_Contribute_BAO_ContributionRecur::getTemplateContribution( (int) $contributionParams['contribution_recur_id'], - array_filter([ + [ 'total_amount' => $input['total_amount'] ?? NULL, 'financial_type_id' => $input['financial_type_id'] ?? NULL, 'campaign_id' => $input['campaign_id'] ?? NULL, - // array_filter with strlen filters out NULL, '' and FALSE but not 0. - ], 'strlen') + ] ); $contributionParams['line_item'] = $templateContribution['line_item']; $contributionParams['status_id'] = 'Pending'; diff --git a/CRM/Contribute/BAO/ContributionRecur.php b/CRM/Contribute/BAO/ContributionRecur.php index 7ede12463a..a8827cfd5f 100644 --- a/CRM/Contribute/BAO/ContributionRecur.php +++ b/CRM/Contribute/BAO/ContributionRecur.php @@ -488,30 +488,30 @@ INNER JOIN civicrm_contribution con ON ( con.id = mp.contribution_id ) * Later we might merge in data stored against the contribution recur record rather than just return the contribution. * * @param int $id - * @param array $overrides + * @param array $inputOverrides * Parameters that should be overridden. Add unit tests if using parameters other than total_amount & financial_type_id. * * @return array * * @throws \API_Exception */ - public static function getTemplateContribution(int $id, $overrides = []): array { - $recurFields = ['is_test', 'financial_type_id', 'total_amount', 'campaign_id']; + public static function getTemplateContribution(int $id, array $inputOverrides = []): array { + $recurFields = ['is_test', 'financial_type_id', 'amount', 'campaign_id']; $recurringContribution = ContributionRecur::get(FALSE) ->addWhere('id', '=', $id) ->setSelect($recurFields) ->execute() ->first(); - // If financial_type_id or total_amount are set on the - // recurring they are overrides, but of lower precedence - // than input parameters. + + // Parameters passed into the function take precedences, falling back to those loaded from + // the recurring contribution. // we filter out null, '' and FALSE but not zero - I'm on the fence about zero. - $overrides = array_filter(array_merge( - // We filter recurringContribution as we only want the fields we asked for - // and specifically don't want 'id' added to overrides. - array_intersect_key($recurringContribution, array_fill_keys($recurFields, 1)), - $overrides - ), 'strlen'); + $overrides = array_filter([ + 'is_test' => $inputOverrides['is_test'] ?? $recurringContribution['is_test'], + 'financial_type_id' => $inputOverrides['financial_type_id'] ?? $recurringContribution['financial_type_id'], + 'campaign_id' => $inputOverrides['campaign_id'] ?? $recurringContribution['campaign_id'], + 'total_amount' => $inputOverrides['total_amount'] ?? $recurringContribution['amount'], + ], 'strlen'); // First look for new-style template contribution with is_template=1 $templateContributions = Contribution::get(FALSE) @@ -546,8 +546,8 @@ INNER JOIN civicrm_contribution con ON ( con.id = mp.contribution_id ) // The handling of the line items is managed in BAO_Order so this // is whether we should override on the contribution. Arguably the 2 should // be decoupled. - if (count($lineItems) > 1 && isset($overrides['financial_type_id'])) { - unset($overrides['financial_type_id']); + if (count($lineItems) > 1) { + unset($overrides['financial_type_id'], $overrides['total_amount']); } $result = array_merge($templateContribution, $overrides); // Line items aren't always written to a contribution, for mystery reasons. diff --git a/CRM/Upgrade/Incremental/sql/5.52.alpha1.mysql.tpl b/CRM/Upgrade/Incremental/sql/5.52.alpha1.mysql.tpl index e98ea9c421..6c918dce14 100644 --- a/CRM/Upgrade/Incremental/sql/5.52.alpha1.mysql.tpl +++ b/CRM/Upgrade/Incremental/sql/5.52.alpha1.mysql.tpl @@ -1 +1,9 @@ {* file to handle db changes in 5.52.alpha1 during upgrade *} +-- Update any recurring contributions to have the same amount +-- as the recurring template contribution if it exists. +-- Some of these got out of sync over recent changes. +UPDATE civicrm_contribution_recur r +INNER JOIN civicrm_contribution c ON contribution_recur_id = r.id +AND c.is_template = 1 +SET amount = total_amount +WHERE total_amount IS NOT NULL; diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index e8dfd90d19..39fcdf5304 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -2769,6 +2769,16 @@ class api_v3_ContributionTest extends CiviUnitTestCase { unset($expectedLineItem['id'], $expectedLineItem['entity_id'], $lineItem2['values'][0]['id'], $lineItem2['values'][0]['entity_id']); $this->assertEquals($expectedLineItem, $lineItem2['values'][0]); + + $this->callAPISuccess('contribution', 'repeattransaction', [ + 'original_contribution_id' => $originalContribution['id'], + 'contribution_status_id' => 'Completed', + 'trxn_id' => 789, + ]); + $this->callAPISuccessGetSingle('contribution', [ + 'total_amount' => 400, + 'trxn_id' => 789, + ]); } /** -- 2.25.1