diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index e0f140a7bde0..0e3a8b0e7b1b 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -2497,7 +2497,6 @@ public static function contributionCount($contactId, $includeSoftCredit = TRUE) * The count is out on how correct related entities wind up in this case. */ protected static function repeatTransaction(array $input, array $contributionParams) { - $templateContribution = CRM_Contribute_BAO_ContributionRecur::getTemplateContribution( (int) $contributionParams['contribution_recur_id'], array_filter([ diff --git a/CRM/Contribute/BAO/ContributionRecur.php b/CRM/Contribute/BAO/ContributionRecur.php index 0e90e0d76523..0fa36236573b 100644 --- a/CRM/Contribute/BAO/ContributionRecur.php +++ b/CRM/Contribute/BAO/ContributionRecur.php @@ -453,16 +453,25 @@ public static function getTemplateContribution(int $id, $overrides = []): array } if ($templateContributions->count()) { $templateContribution = $templateContributions->first(); - $lineItems = CRM_Price_BAO_LineItem::getLineItemsByContributionID($templateContribution['id']); + $order = new CRM_Financial_BAO_Order(); + $order->setTemplateContributionID($templateContribution['id']); + $order->setOverrideFinancialTypeID($overrides['financial_type_id'] ?? NULL); + $order->setOverridableFinancialTypeID($templateContribution['financial_type_id']); + $order->setOverrideTotalAmount($overrides['total_amount'] ?? NULL); + $order->setIsPermitOverrideFinancialTypeForMultipleLines(FALSE); + $lineItems = $order->getLineItems(); // We only permit the financial type to be overridden for single line items. // Otherwise we need to figure out a whole lot of extra complexity. // It's not UI-possible to alter financial_type_id for recurring contributions // with more than one line item. + // 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']); } $result = array_merge($templateContribution, $overrides); - $result['line_item'] = self::reformatLineItemsForRepeatContribution($result['total_amount'], $result['financial_type_id'], $lineItems, (array) $templateContribution); + $result['line_item'][$order->getPriceSetID()] = $lineItems; // If the template contribution was made on-behalf then add the // relevant values to ensure the activity reflects that. $relatedContact = CRM_Contribute_BAO_Contribution::getOnbehalfIds($result['id']); @@ -993,53 +1002,4 @@ public static function buildOptions($fieldName, $context = NULL, $props = []) { return CRM_Core_PseudoConstant::get(__CLASS__, $fieldName, $params, $context); } - /** - * Reformat line items for getTemplateContribution / repeat contribution. - * - * This is an extraction and may be subject to further cleanup. - * - * @param float $total_amount - * @param int $financial_type_id - * @param array $lineItems - * @param array $originalContribution - * - * @return array - */ - protected static function reformatLineItemsForRepeatContribution($total_amount, $financial_type_id, array $lineItems, array $originalContribution): array { - $lineSets = []; - if (count($lineItems) == 1) { - foreach ($lineItems as $index => $lineItem) { - if ($lineItem['financial_type_id'] != $originalContribution['financial_type_id']) { - // CRM-20685, Repeattransaction produces incorrect Financial Type ID (in specific circumstance) - if number of lineItems = 1, So this conditional will set the financial_type_id as the original if line_item and contribution comes with different data. - $financial_type_id = $lineItem['financial_type_id']; - } - if ($financial_type_id) { - // CRM-17718 allow for possibility of changed financial type ID having been set prior to calling this. - $lineItem['financial_type_id'] = $financial_type_id; - } - $taxAmountMatches = FALSE; - if ((!empty($lineItem['tax_amount']) && ($lineItem['line_total'] + $lineItem['tax_amount']) == $total_amount)) { - $taxAmountMatches = TRUE; - } - if ($lineItem['line_total'] != $total_amount && !$taxAmountMatches) { - // We are dealing with a changed amount! Per CRM-16397 we can work out what to do with these - // if there is only one line item, and the UI should prevent this situation for those with more than one. - $lineItem['line_total'] = $total_amount; - $lineItem['unit_price'] = round($total_amount / $lineItem['qty'], 2); - } - $priceField = new CRM_Price_DAO_PriceField(); - $priceField->id = $lineItem['price_field_id']; - $priceField->find(TRUE); - $lineSets[$priceField->price_set_id][$lineItem['price_field_id']] = $lineItem; - } - } - // CRM-19309 if more than one then just pass them through: - elseif (count($lineItems) > 1) { - foreach ($lineItems as $index => $lineItem) { - $lineSets[$index][$lineItem['price_field_id']] = $lineItem; - } - } - return $lineSets; - } - } diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php index 2388c445268c..2bbd19294c75 100644 --- a/CRM/Financial/BAO/Order.php +++ b/CRM/Financial/BAO/Order.php @@ -9,6 +9,7 @@ +--------------------------------------------------------------------+ */ +use Civi\Api4\LineItem; use Civi\Api4\PriceField; use Civi\Api4\PriceSet; @@ -58,6 +59,48 @@ class CRM_Financial_BAO_Order { */ protected $overrideFinancialTypeID; + /** + * Overridable financial type id. + * + * When this is set only this financial type will be overridden. + * + * This is relevant to repeat transactions where we want to + * override the type on the line items if it a different financial type has + * been saved against the recurring contribution. However, it the line item + * financial type differs from the contribution financial type then we + * treat this as deliberately uncoupled and don't flow through changes + * in financial type down to the line items. + * + * This is covered in testRepeatTransactionUpdatedFinancialTypeAndNotEquals. + * + * @var int + */ + protected $overridableFinancialTypeID; + + /** + * Get overridable financial type id. + * + * If only one financial type id can be overridden at the line item level + * then get it here, otherwise NULL. + * + * @return int|null + */ + public function getOverridableFinancialTypeID(): ?int { + return $this->overridableFinancialTypeID; + } + + /** + * Set overridable financial type id. + * + * If only one financial type id can be overridden at the line item level + * then set it here. + * + * @param int|null $overridableFinancialTypeID + */ + public function setOverridableFinancialTypeID(?int $overridableFinancialTypeID): void { + $this->overridableFinancialTypeID = $overridableFinancialTypeID; + } + /** * Financial type id to use for any lines where is is not provided. * @@ -65,6 +108,75 @@ class CRM_Financial_BAO_Order { */ protected $defaultFinancialTypeID; + /** + * ID of a contribution to be used as a template. + * + * @var int + */ + protected $templateContributionID; + + /** + * Should we permit the line item financial type to be overridden when there is more than one line. + * + * Historically the answer is 'yes' for v3 order api and 'no' for repeattransaction + * and backoffice forms. + * + * @var bool + */ + protected $isPermitOverrideFinancialTypeForMultipleLines = TRUE; + + /** + * @return bool + */ + public function isPermitOverrideFinancialTypeForMultipleLines(): bool { + return $this->isPermitOverrideFinancialTypeForMultipleLines; + } + + /** + * @param bool $isPermitOverrideFinancialTypeForMultipleLines + */ + public function setIsPermitOverrideFinancialTypeForMultipleLines(bool $isPermitOverrideFinancialTypeForMultipleLines): void { + $this->isPermitOverrideFinancialTypeForMultipleLines = $isPermitOverrideFinancialTypeForMultipleLines; + } + + /** + * Number of line items. + * + * @var int + */ + protected $lineItemCount; + + /** + * @return int + */ + public function getLineItemCount(): int { + if (!isset($this->lineItemCount)) { + $this->lineItemCount = count($this->getPriceOptions()) || count($this->lineItems); + } + return $this->lineItemCount; + } + + /** + * @param int $lineItemCount + */ + public function setLineItemCount(int $lineItemCount): void { + $this->lineItemCount = $lineItemCount; + } + + /** + * @return int|null + */ + public function getTemplateContributionID(): ?int { + return $this->templateContributionID; + } + + /** + * @param int $templateContributionID + */ + public function setTemplateContributionID(int $templateContributionID): void { + $this->templateContributionID = $templateContributionID; + } + /** * @return int */ @@ -179,20 +291,43 @@ public function _unserialize(array $data): void { public function getOverrideTotalAmount() { // The override amount is only valid for quick config price sets where more // than one field has not been selected. - if (!$this->overrideTotalAmount || !$this->supportsOverrideAmount() || count($this->getPriceOptions()) > 1) { + if (!$this->overrideTotalAmount || $this->getLineItemCount() > 1) { return FALSE; } return $this->overrideTotalAmount; } + /** + * Is the line item financial type to be overridden. + * + * We have a tested scenario for repeatcontribution where the line item + * does not match the top level financial type for the order. In this case + * any financial type override has been determined to NOT apply to the line items. + * + * This is locked in via testRepeatTransactionUpdatedFinancialTypeAndNotEquals. + * + * @param int $financialTypeID + * + * @return bool + */ + public function isOverrideLineItemFinancialType(int $financialTypeID) { + if (!$this->getOverrideFinancialTypeID()) { + return FALSE; + } + if (!$this->getOverridableFinancialTypeID()) { + return TRUE; + } + return $this->getOverrideFinancialTypeID() === $financialTypeID; + } + /** * Set override for total amount. * * @internal use in tested core code only. * - * @param float $overrideTotalAmount + * @param float|null $overrideTotalAmount */ - public function setOverrideTotalAmount(float $overrideTotalAmount): void { + public function setOverrideTotalAmount(?float $overrideTotalAmount): void { $this->overrideTotalAmount = $overrideTotalAmount; } @@ -204,7 +339,11 @@ public function setOverrideTotalAmount(float $overrideTotalAmount): void { * @return int| FALSE */ public function getOverrideFinancialTypeID() { - if (count($this->getPriceOptions()) !== 1) { + // We don't permit overrides if there is more than one line. + // The reason for this constraint may be more historical since + // the case could be made that if it is set it should be used and + // we have built out the tax calculations a lot now. + if (!$this->isPermitOverrideFinancialTypeForMultipleLines() && $this->getLineItemCount() > 1) { return FALSE; } return $this->overrideFinancialTypeID ?? FALSE; @@ -215,9 +354,9 @@ public function getOverrideFinancialTypeID() { * * @internal use in tested core code only. * - * @param int $overrideFinancialTypeID + * @param int|null $overrideFinancialTypeID */ - public function setOverrideFinancialTypeID(int $overrideFinancialTypeID): void { + public function setOverrideFinancialTypeID(?int $overrideFinancialTypeID): void { $this->overrideFinancialTypeID = $overrideFinancialTypeID; } @@ -265,17 +404,6 @@ public function setPriceSetToDefault(string $component): void { ->first()['id']; } - /** - * Is overriding the total amount valid for this price set. - * - * @internal tested. core code use only. - * - * @return bool - */ - public function supportsOverrideAmount(): bool { - return (bool) $this->getPriceSetMetadata()['is_quick_config']; - } - /** * Set price set ID based on the contribution page id. * @@ -283,7 +411,6 @@ public function supportsOverrideAmount(): bool { * * @param int $contributionPageID * - * @throws \CiviCRM_API3_Exception */ public function setPriceSetIDByContributionPageID(int $contributionPageID): void { $this->setPriceSetIDByEntity('contribution_page', $contributionPageID); @@ -550,7 +677,8 @@ public function getRenewableMembershipTypes(): array { /** * @return array - * @throws \CiviCRM_API3_Exception + * + * @throws \API_Exception */ protected function calculateLineItems(): array { $lineItems = []; @@ -564,20 +692,33 @@ protected function calculateLineItems(): array { // Dummy value to prevent e-notice in getLine. We calculate tax in this class. $params['financial_type_id'] = 0; - foreach ($this->getPriceOptions() as $fieldID => $valueID) { - $this->setPriceSetIDFromSelectedField($fieldID); - $throwAwayArray = []; - // @todo - still using getLine for now but better to bring it to this class & do a better job. - $newLines = CRM_Price_BAO_PriceSet::getLine($params, $throwAwayArray, $this->getPriceSetID(), $this->getPriceFieldSpec($fieldID), $fieldID)[1]; - foreach ($newLines as $newLine) { - $lineItems[$newLine['price_field_value_id']] = $newLine; + if ($this->getTemplateContributionID()) { + $lineItems = $this->getLinesFromTemplateContribution(); + } + else { + foreach ($this->getPriceOptions() as $fieldID => $valueID) { + $this->setPriceSetIDFromSelectedField($fieldID); + $throwAwayArray = []; + // @todo - still using getLine for now but better to bring it to this class & do a better job. + $newLines = CRM_Price_BAO_PriceSet::getLine($params, $throwAwayArray, $this->getPriceSetID(), $this->getPriceFieldSpec($fieldID), $fieldID)[1]; + foreach ($newLines as $newLine) { + $lineItems[$newLine['price_field_value_id']] = $newLine; + } } } + // Set the line item count here because it is needed to determine whether + // we can use overrides and would not be set yet if we have loaded them from + // a template contribution. + $this->setLineItemCount(count($lineItems)); foreach ($lineItems as &$lineItem) { + // Set the price set id if not set above. Note that the above + // requires it for line retrieval but we want to fix that as it + // should not be required at that point. + $this->setPriceSetIDFromSelectedField($lineItem['price_field_id']); // Set any pre-calculation to zero as we will calculate. $lineItem['tax_amount'] = 0; - if ($this->getOverrideFinancialTypeID() !== FALSE) { + if ($this->isOverrideLineItemFinancialType($lineItem['financial_type_id']) !== FALSE) { $lineItem['financial_type_id'] = $this->getOverrideFinancialTypeID(); } $taxRate = $this->getTaxRate((int) $lineItem['financial_type_id']); @@ -801,4 +942,48 @@ protected function addTotalsToLineBasedOnOverrideTotal(int $financialTypeID, arr } } + /** + * Get the line items from a template. + * + * @return \Civi\Api4\Generic\Result + * + * @throws \API_Exception + */ + protected function getLinesFromTemplateContribution(): array { + $lines = $this->getLinesForContribution(); + foreach ($lines as &$line) { + // The apiv4 insists on adding id - so let it get all the details + // and we will filter out those that are not part of a template here. + unset($line['id'], $line['contribution_id']); + } + return $lines; + } + + /** + * @return array + * @throws \API_Exception + * @throws \Civi\API\Exception\UnauthorizedException + */ + protected function getLinesForContribution(): array { + return (array) LineItem::get(FALSE) + ->addWhere('contribution_id', '=', $this->getTemplateContributionID()) + ->setSelect([ + 'contribution_id', + 'entity_id', + 'entity_table', + 'price_field_id', + 'price_field_value_id', + 'financial_type_id', + 'label', + 'qty', + 'unit_price', + 'line_total', + 'tax_amount', + 'non_deductible_amount', + 'participant_count', + 'membership_num_terms', + ]) + ->execute(); + } + } diff --git a/tests/phpunit/CRM/Contribute/BAO/ContributionRecurTest.php b/tests/phpunit/CRM/Contribute/BAO/ContributionRecurTest.php index c8904af7c04f..108a540892ef 100644 --- a/tests/phpunit/CRM/Contribute/BAO/ContributionRecurTest.php +++ b/tests/phpunit/CRM/Contribute/BAO/ContributionRecurTest.php @@ -218,7 +218,6 @@ public function testGetTemplateContributionMatchTest(): void { * Test that is_template contribution is used where available * * @throws \API_Exception - * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception * @throws \Civi\API\Exception\UnauthorizedException */ diff --git a/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php b/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php index 6541e9f5d164..e38822626ee2 100644 --- a/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php +++ b/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php @@ -129,7 +129,7 @@ public function testIPNPaymentRecurSuccess(): void { $this->assertEquals(1, $contribution1['contribution_status_id']); $this->assertEquals('8XA571746W2698126', $contribution1['trxn_id']); // source gets set by processor - $this->assertTrue(substr($contribution1['contribution_source'], 0, 20) === 'Online Contribution:'); + $this->assertEquals('Online Contribution:', substr($contribution1['contribution_source'], 0, 20)); $contributionRecur = $this->callAPISuccess('contribution_recur', 'getsingle', ['id' => $this->_contributionRecurID]); $this->assertEquals(5, $contributionRecur['contribution_status_id']); $paypalIPN = new CRM_Core_Payment_PayPalIPN($this->getPaypalRecurSubsequentTransaction()); @@ -207,7 +207,7 @@ public function testIPNPaymentMembershipRecurSuccess() { * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception */ - public function testIPNPaymentInputMembershipRecurSuccess() { + public function testIPNPaymentInputMembershipRecurSuccess(): void { $durationUnit = 'year'; $this->setupMembershipRecurringPaymentProcessorTransaction(['duration_unit' => $durationUnit, 'frequency_unit' => $durationUnit]); $membershipPayment = $this->callAPISuccessGetSingle('membership_payment', []); diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 1790e50ea537..324c08380c9c 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -2558,11 +2558,9 @@ public function setupRecurringPaymentProcessorTransaction($recurParams = [], $co 'total_amount' => '200', 'invoice_id' => $this->_invoiceID, 'financial_type_id' => 'Donation', - 'contribution_status_id' => 'Pending', 'contact_id' => $this->_contactID, 'contribution_page_id' => $this->_contributionPageID, 'payment_processor_id' => $this->_paymentProcessorID, - 'is_test' => 0, 'receive_date' => '2019-07-25 07:34:23', 'skipCleanMoney' => TRUE, 'amount_level' => 'expensive', @@ -2818,11 +2816,9 @@ protected function createPriceSet($component = 'contribution_page', $componentId $paramsSet['financial_type_id'] = 'Event Fee'; $paramsSet['extends'] = 1; $priceSet = $this->callAPISuccess('price_set', 'create', $paramsSet); - $priceSetId = $priceSet['id']; - //Checking for priceset added in the table. - $this->assertDBCompareValue('CRM_Price_BAO_PriceSet', $priceSetId, 'title', - 'id', $paramsSet['title'], 'Check DB for created priceset' - ); + if ($componentId) { + CRM_Price_BAO_PriceSet::addTo('civicrm_' . $component, $componentId, $priceSet['id']); + } $paramsField = array_merge([ 'label' => 'Price Field', 'name' => CRM_Utils_String::titleToVar('Price Field'), @@ -2842,9 +2838,6 @@ protected function createPriceSet($component = 'contribution_page', $componentId ], $priceFieldOptions); $priceField = CRM_Price_BAO_PriceField::create($paramsField); - if ($componentId) { - CRM_Price_BAO_PriceSet::addTo('civicrm_' . $component, $componentId, $priceSetId); - } return $this->callAPISuccess('PriceFieldValue', 'get', ['price_field_id' => $priceField->id]); } diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index d8bd66522f5e..49d1aa8507f8 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -2410,19 +2410,21 @@ public function testRepeatTransactionLineItems(): void { ]; $lineItem1 = $this->callAPISuccess('line_item', 'get', array_merge($lineItemParams, [ 'entity_id' => $originalContribution['id'], - ])); + 'options' => ['sort' => 'qty'], + ]))['values']; $lineItem2 = $this->callAPISuccess('line_item', 'get', array_merge($lineItemParams, [ 'entity_id' => $originalContribution['id'] + 1, - ])); + 'options' => ['sort' => 'qty'], + ]))['values']; // unset id and entity_id for all of them to be able to compare the lineItems: - unset($lineItem1['values'][0]['id'], $lineItem1['values'][0]['entity_id']); - unset($lineItem2['values'][0]['id'], $lineItem2['values'][0]['entity_id']); - $this->assertEquals($lineItem1['values'][0], $lineItem2['values'][0]); + unset($lineItem1[0]['id'], $lineItem1[0]['entity_id']); + unset($lineItem2[0]['id'], $lineItem2[0]['entity_id']); + $this->assertEquals($lineItem1[0], $lineItem2[0]); - unset($lineItem1['values'][1]['id'], $lineItem1['values'][1]['entity_id']); - unset($lineItem2['values'][1]['id'], $lineItem2['values'][1]['entity_id']); - $this->assertEquals($lineItem1['values'][1], $lineItem2['values'][1]); + unset($lineItem1[1]['id'], $lineItem1[1]['entity_id']); + unset($lineItem2[1]['id'], $lineItem2[1]['entity_id']); + $this->assertEquals($lineItem1[1], $lineItem2[1]); // CRM-19309 so in future we also want to: // check that financial_line_items have been created for entity_id 3 and 4; @@ -3001,7 +3003,7 @@ public function testRepeatTransactionUpdatedCampaign(): void { * * @throws \CRM_Core_Exception */ - public function testRepeatTransactionUpdatedFinancialTypeAndNotEquals() { + public function testRepeatTransactionUpdatedFinancialTypeAndNotEquals(): void { $originalContribution = $this->setUpRecurringContribution([], ['financial_type_id' => 2]); // This will made the trick to get the not equals behaviour. $this->callAPISuccess('line_item', 'create', ['id' => 1, 'financial_type_id' => 4]);