diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 791ebe27a5b0..5e884b930ea9 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -2611,9 +2611,9 @@ protected static function repeatTransaction(&$contribution, &$input, $contributi ]) ); $input['line_item'] = $contributionParams['line_item'] = $templateContribution['line_item']; - $contributionParams['status_id'] = 'Pending'; - if (isset($contributionParams['financial_type_id'])) { + + if (isset($contributionParams['financial_type_id']) && count($input['line_item']) === 1) { // Give precedence to passed in type. $contribution->financial_type_id = $contributionParams['financial_type_id']; } @@ -4440,10 +4440,8 @@ public static function completeOrder($input, &$ids, $objects, $transaction = NUL 'contribution_status_id', 'card_type_id', 'pan_truncation', + 'financial_type_id', ]; - if (self::isSingleLineItem($primaryContributionID)) { - $inputContributionWhiteList[] = 'financial_type_id'; - } $participant = $objects['participant'] ?? NULL; $recurContrib = $objects['contributionRecur'] ?? NULL; @@ -4487,6 +4485,12 @@ public static function completeOrder($input, &$ids, $objects, $transaction = NUL $changeDate = CRM_Utils_Array::value('trxn_date', $input, date('YmdHis')); self::repeatTransaction($contribution, $input, $contributionParams, $paymentProcessorId); + // @todo this line only exists because after we save the contribution in repeatTransaction + // with the correct params we resave it shortly after this with ahem params. + // if we stop doing that per + // https://github.com/civicrm/civicrm-core/pull/17972 + // this line can go. Test cover in testRepeatTransactionPassedInFinancialTypeTwoLineItems + // and testRepeatTransactionPassedInFinancialType $contributionParams['financial_type_id'] = $contribution->financial_type_id; $values = []; @@ -4533,6 +4537,7 @@ public static function completeOrder($input, &$ids, $objects, $transaction = NUL // CRM-19309 - if you update the contribution here with financial_type_id it can/will mess with $lineItem // unsetting it here does NOT cause any other contribution test to fail! + // this will no longer be required once we stop setting it above/ stop double saving. unset($contributionParams['financial_type_id']); $contributionResult = civicrm_api3('Contribution', 'create', $contributionParams); diff --git a/CRM/Contribute/BAO/ContributionRecur.php b/CRM/Contribute/BAO/ContributionRecur.php index 72d5d4eaf943..4bae1b03db1d 100644 --- a/CRM/Contribute/BAO/ContributionRecur.php +++ b/CRM/Contribute/BAO/ContributionRecur.php @@ -442,8 +442,11 @@ public static function getTemplateContribution($id, $overrides = []) { } if ($templateContributions->count()) { $templateContribution = $templateContributions->first(); - $result = array_merge($templateContribution, $overrides); $lineItems = CRM_Price_BAO_LineItem::getLineItemsByContributionID($templateContribution['id']); + if (count($lineItems) > 1 && isset($overrides['financial_type_id'])) { + unset($overrides['financial_type_id']); + } + $result = array_merge($templateContribution, $overrides); $result['line_item'] = self::reformatLineItemsForRepeatContribution($result['total_amount'], $result['financial_type_id'], $lineItems, (array) $templateContribution); return $result; } diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index 70e9db324823..8de1b2272f97 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -2684,7 +2684,10 @@ public function testRepeatTransactionAlteredAmount() { } /** - * CRM-17718 test appropriate action if financial type has changed for single line items. + * Test financial_type_id override behaviour with a single line item. + * + * CRM-17718 a passed in financial_type_id is allowed to override the original contribution where there + * is only one line item. */ public function testRepeatTransactionPassedInFinancialType() { $originalContribution = $this->setUpRecurringContribution(); @@ -2711,7 +2714,7 @@ public function testRepeatTransactionPassedInFinancialType() { ], ]; - $this->callAPISuccessGetSingle('contribution', [ + $this->callAPISuccessGetSingle('Contribution', [ 'total_amount' => 100, 'financial_type_id' => 2, ]); @@ -2734,6 +2737,37 @@ public function testRepeatTransactionPassedInFinancialType() { $this->assertEquals($expectedLineItem, $lineItem2['values'][0]); } + /** + * Test financial_type_id override behaviour with a single line item. + * + * CRM-17718 a passed in financial_type_id is not allowed to override the original contribution where there + * is more than one line item. + */ + public function testRepeatTransactionPassedInFinancialTypeTwoLineItems() { + $this->_params = $this->getParticipantOrderParams(); + $originalContribution = $this->setUpRecurringContribution(); + + $this->callAPISuccess('Contribution', 'repeattransaction', [ + 'original_contribution_id' => $originalContribution['id'], + 'contribution_status_id' => 'Completed', + 'trxn_id' => 'repeat', + 'financial_type_id' => 2, + ]); + + // Retrieve the new contribution and note the financial type passed in has been ignored. + $contribution = $this->callAPISuccessGetSingle('Contribution', [ + 'trxn_id' => 'repeat', + ]); + $this->assertEquals(4, $contribution['financial_type_id']); + + $lineItems = $this->callAPISuccess('line_item', 'get', [ + 'entity_id' => $contribution['id'], + ])['values']; + foreach ($lineItems as $lineItem) { + $this->assertNotEquals(2, $lineItem['financial_type_id']); + } + } + /** * CRM-17718 test appropriate action if financial type has changed for single line items. */ @@ -4064,6 +4098,7 @@ public function _deletedAddedPaymentInstrument() { * Parameters to merge into the recur only. * * @return array|int + * @throws \CRM_Core_Exception */ protected function setUpRecurringContribution($generalParams = [], $recurParams = []) { $contributionRecur = $this->callAPISuccess('contribution_recur', 'create', array_merge([ @@ -4077,12 +4112,14 @@ protected function setUpRecurringContribution($generalParams = [], $recurParams 'frequency_unit' => 'month', 'payment_processor_id' => $this->paymentProcessorID, ], $generalParams, $recurParams)); - $originalContribution = $this->callAPISuccess('contribution', 'create', array_merge( + $contributionParams = array_merge( $this->_params, [ 'contribution_recur_id' => $contributionRecur['id'], - ], $generalParams) - ); + 'contribution_status_id' => 'Pending', + ], $generalParams); + $contributionParams['api.Payment.creaate'] = $contributionParams['total_amount']; + $originalContribution = $this->callAPISuccess('Order', 'create', $contributionParams); return $originalContribution; }