From ca1b238b07c3001ad5c748f009d6bda45a62435d Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 12 Jul 2021 14:51:10 +1200 Subject: [PATCH] [REF] Further order api cleanup --- CRM/Contribute/BAO/Contribution.php | 31 ----- CRM/Financial/BAO/Order.php | 130 ++++++++++++++++-- api/v3/Order.php | 104 ++++++++------ .../CRM/Contribute/BAO/ContributionTest.php | 112 --------------- 4 files changed, 184 insertions(+), 193 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 0e3a8b0e7b..7dab1582be 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -4385,37 +4385,6 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac } } - /** - * Checks if line items total amounts - * match the contribution total amount. - * - * @param array $params - * array of order params. - * - * @throws \API_Exception - */ - public static function checkLineItems(&$params) { - $totalAmount = $params['total_amount'] ?? NULL; - $lineItemAmount = 0; - - foreach ($params['line_items'] as &$lineItems) { - foreach ($lineItems['line_item'] as &$item) { - $lineItemAmount += $item['line_total'] + ($item['tax_amount'] ?? 0.00); - } - } - - if (!isset($totalAmount)) { - $params['total_amount'] = $lineItemAmount; - } - else { - $currency = $params['currency'] ?? CRM_Core_Config::singleton()->defaultCurrency; - - if (!CRM_Utils_Money::equals($totalAmount, $lineItemAmount, $currency)) { - throw new CRM_Contribute_Exception_CheckLineItemsException(); - } - } - } - /** * Get the financial account for the item associated with the new transaction. * diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php index 5dfdc7d2dd..542eb85136 100644 --- a/CRM/Financial/BAO/Order.php +++ b/CRM/Financial/BAO/Order.php @@ -209,6 +209,75 @@ class CRM_Financial_BAO_Order { */ protected $lineItems = []; + /** + * Array of entities ordered. + * + * @var array + */ + protected $entityParameters = []; + + /** + * Default price sets for component. + * + * @var array + */ + protected $defaultPriceSets = []; + + /** + * Cache of the default price field. + * + * @var array + */ + protected $defaultPriceField; + + /** + * Get parameters for the entities bought as part of this order. + * + * @return array + * + * @internal core tested code only. + * + */ + public function getEntitiesToCreate(): array { + $entities = []; + foreach ($this->entityParameters as $entityToCreate) { + if (in_array($entityToCreate['entity'], ['participant', 'membership'], TRUE)) { + $entities[] = $entityToCreate; + } + } + return $entities; + } + + /** + * Set parameters for the entities bought as part of this order. + * + * @param array $entityParameters + * @param int|string $key indexing reference + * + * @internal core tested code only. + * + */ + public function setEntityParameters(array $entityParameters, $key): void { + $this->entityParameters[$key] = $entityParameters; + } + + /** + * Add a line item to an entity. + * + * The v3 api supports more than on line item being stored against a given + * set of entity parameters. There is some doubt as to whether this is a + * good thing that should be supported in v4 or something that 'seemed + * like a good idea at the time' - but this allows the lines to be added from the + * v3 api. + * + * @param string $lineIndex + * @param string $entityKey + */ + public function addLineItemToEntityParameters(string $lineIndex, string $entityKey): void { + $this->entityParameters[$entityKey]['entity'] = $this->getLineItemEntity($lineIndex); + $this->entityParameters[$entityKey]['line_references'][] = $lineIndex; + } + /** * Metadata for price fields. * @@ -398,10 +467,7 @@ class CRM_Financial_BAO_Order { * @internal use in tested core code only. */ public function setPriceSetToDefault(string $component): void { - $this->priceSetID = PriceSet::get(FALSE) - ->addWhere('name', '=', ($component === 'membership' ? 'default_membership_type_amount' : 'default_contribution_amount')) - ->execute() - ->first()['id']; + $this->priceSetID = $this->getDefaultPriceSetForComponent($component); } /** @@ -575,7 +641,7 @@ class CRM_Financial_BAO_Order { } if (empty($this->priceSelection) && isset($input['total_amount']) && is_numeric($input['total_amount']) && !empty($input['financial_type_id'])) { - $this->priceSelection['price_' . $this->getDefaultPriceField()] = $input['total_amount']; + $this->priceSelection['price_' . $this->getDefaultPriceFieldID()] = $input['total_amount']; $this->setOverrideFinancialTypeID($input['financial_type_id']); } } @@ -584,12 +650,17 @@ class CRM_Financial_BAO_Order { * Get the id of the price field to use when just an amount is provided. * * @throws \API_Exception + * + * @return int */ - public function getDefaultPriceField() { - return PriceField::get(FALSE) - ->addWhere('name', '=', 'contribution_amount') - ->addWhere('price_set_id.name', '=', 'default_contribution_amount') - ->execute()->first()['id']; + public function getDefaultPriceFieldID():int { + if (!$this->defaultPriceField) { + $this->defaultPriceField = PriceField::get(FALSE) + ->addWhere('name', '=', 'contribution_amount') + ->addWhere('price_set_id.name', '=', 'default_contribution_amount') + ->execute()->first(); + } + return $this->defaultPriceField['id']; } /** @@ -853,6 +924,9 @@ class CRM_Financial_BAO_Order { $lineItem = $this->fillMembershipLine($lineItem); } } + if ($this->getPriceSetID() === $this->getDefaultPriceSetForComponent('contribution')) { + $this->fillDefaultContributionLine($lineItem); + } $this->lineItems[$index] = $lineItem; } @@ -986,4 +1060,40 @@ class CRM_Financial_BAO_Order { ->execute(); } + /** + * Get the default price set id for the given component. + * + * @param string $component + * + * @return int + * @throws \API_Exception + */ + protected function getDefaultPriceSetForComponent(string $component): int { + if (!isset($this->defaultPriceSets[$component])) { + $this->defaultPriceSets[$component] = PriceSet::get(FALSE) + ->addWhere('name', '=', ($component === 'membership' ? 'default_membership_type_amount' : 'default_contribution_amount')) + ->execute() + ->first()['id']; + } + return $this->defaultPriceSets[$component]; + } + + /** + * Fill in values for a default contribution line item. + * + * @param array $lineItem + * + * @throws \API_Exception + */ + protected function fillDefaultContributionLine(array &$lineItem): void { + $defaults = [ + 'qty' => 1, + 'price_field_id' => $this->getDefaultPriceFieldID(), + 'entity_table' => 'civicrm_contribution', + 'unit_price' => $lineItem['line_total'], + 'label' => ts('Contribution Amount'), + ]; + $lineItem = array_merge($defaults, $lineItem); + } + } diff --git a/api/v3/Order.php b/api/v3/Order.php index fdb86c1253..5f4ef7ed73 100644 --- a/api/v3/Order.php +++ b/api/v3/Order.php @@ -73,61 +73,85 @@ function _civicrm_api3_order_get_spec(array &$params) { */ function civicrm_api3_order_create(array $params): array { civicrm_api3_verify_one_mandatory($params, NULL, ['line_items', 'total_amount']); - + if (empty($params['skipCleanMoney'])) { + // We have to do this for v3 api - sadly. For v4 it will be no more. + foreach (['total_amount', 'net_amount', 'fee_amount', 'non_deductible_amount'] as $field) { + if (isset($params[$field])) { + $params[$field] = CRM_Utils_Rule::cleanMoney($params[$field]); + } + } + $params['skipCleanMoney'] = TRUE; + } $params['contribution_status_id'] = 'Pending'; $order = new CRM_Financial_BAO_Order(); $order->setDefaultFinancialTypeID($params['financial_type_id'] ?? NULL); if (!empty($params['line_items']) && is_array($params['line_items'])) { - CRM_Contribute_BAO_Contribution::checkLineItems($params); foreach ($params['line_items'] as $index => $lineItems) { + if (!empty($lineItems['params'])) { + $order->setEntityParameters($lineItems['params'], $index); + } foreach ($lineItems['line_item'] as $innerIndex => $lineItem) { $lineIndex = $index . '+' . $innerIndex; $order->setLineItem($lineItem, $lineIndex); - } - - $entityParams = $lineItems['params'] ?? NULL; - - if ($entityParams && $order->getLineItemEntity($lineIndex) !== 'contribution') { - switch ($order->getLineItemEntity($lineIndex)) { - case 'participant': - if (isset($entityParams['participant_status_id']) - && (!CRM_Event_BAO_ParticipantStatusType::getIsValidStatusForClass($entityParams['participant_status_id'], 'Pending'))) { - throw new CiviCRM_API3_Exception('Creating a participant via the Order API with a non "pending" status is not supported'); - } - $entityParams['participant_status_id'] = $entityParams['participant_status_id'] ?? 'Pending from incomplete transaction'; - $entityParams['status_id'] = $entityParams['participant_status_id']; - $entityParams['skipLineItem'] = TRUE; - $entityResult = civicrm_api3('Participant', 'create', $entityParams); - // @todo - once membership is cleaned up & financial validation tests are extended - // we can look at removing this - some weird handling in removeFinancialAccounts - $params['contribution_mode'] = 'participant'; - $params['participant_id'] = $entityResult['id']; - break; - - case 'membership': - $entityParams['status_id'] = 'Pending'; - if (!empty($params['contribution_recur_id'])) { - $entityParams['contribution_recur_id'] = $params['contribution_recur_id']; - } - $entityParams['skipLineItem'] = TRUE; - $entityResult = civicrm_api3('Membership', 'create', $entityParams); - break; - - } - - foreach ($lineItems['line_item'] as $innerIndex => $lineItem) { - $lineIndex = $index . '+' . $innerIndex; - $order->setLineItemValue('entity_id', $entityResult['id'], $lineIndex); - } + $order->addLineItemToEntityParameters($lineIndex, $index); } } - $priceSetID = $order->getPriceSetID(); - $params['line_item'][$priceSetID] = $order->getLineItems(); } else { $order->setPriceSetToDefault('contribution'); + $order->setLineItem([ + // Historically total_amount in this case could be tax + // inclusive if tax is also supplied. + // This is inconsistent with the contribution api.... + 'line_total' => ((float) $params['total_amount'] - (float) ($params['tax_amount'] ?? 0)), + 'financial_type_id' => (int) $params['financial_type_id'], + ], 0); } + // Only check the amount if line items are set because that is what we have historically + // done and total amount is historically only inclusive of tax_amount IF + // tax amount is also passed in it seems + if (isset($params['total_amount']) && !empty($params['line_items'])) { + $currency = $params['currency'] ?? CRM_Core_Config::singleton()->defaultCurrency; + if (!CRM_Utils_Money::equals($params['total_amount'], $order->getTotalAmount(), $currency)) { + throw new CRM_Contribute_Exception_CheckLineItemsException(); + } + } + $params['total_amount'] = $order->getTotalAmount(); + + foreach ($order->getEntitiesToCreate() as $entityParams) { + if ($entityParams['entity'] === 'participant') { + if (isset($entityParams['participant_status_id']) + && (!CRM_Event_BAO_ParticipantStatusType::getIsValidStatusForClass($entityParams['participant_status_id'], 'Pending'))) { + throw new CiviCRM_API3_Exception('Creating a participant via the Order API with a non "pending" status is not supported'); + } + $entityParams['participant_status_id'] = $entityParams['participant_status_id'] ?? 'Pending from incomplete transaction'; + $entityParams['status_id'] = $entityParams['participant_status_id']; + $entityParams['skipLineItem'] = TRUE; + $entityResult = civicrm_api3('Participant', 'create', $entityParams); + // @todo - once membership is cleaned up & financial validation tests are extended + // we can look at removing this - some weird handling in removeFinancialAccounts + $params['contribution_mode'] = 'participant'; + $params['participant_id'] = $entityResult['id']; + foreach ($entityParams['line_references'] as $lineIndex) { + $order->setLineItemValue('entity_id', $entityResult['id'], $lineIndex); + } + } + + if ($entityParams['entity'] === 'membership') { + $entityParams['status_id'] = 'Pending'; + if (!empty($params['contribution_recur_id'])) { + $entityParams['contribution_recur_id'] = $params['contribution_recur_id']; + } + $entityParams['skipLineItem'] = TRUE; + $entityResult = civicrm_api3('Membership', 'create', $entityParams); + foreach ($entityParams['line_references'] as $lineIndex) { + $order->setLineItemValue('entity_id', $entityResult['id'], $lineIndex); + } + } + } + + $params['line_item'][$order->getPriceSetID()] = $order->getLineItems(); $contributionParams = $params; // If this is nested we need to set sequential to 0 as sequential handling is done diff --git a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php index 7141f17f89..a152941b13 100644 --- a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php +++ b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php @@ -773,118 +773,6 @@ WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2"; return $contributionObject; } - /** - * checkLineItems() check if total amount matches the sum of line total - */ - public function testcheckLineItems() { - $params = [ - 'contact_id' => 202, - 'receive_date' => '2010-01-20', - 'total_amount' => 100, - 'financial_type_id' => 3, - 'line_items' => [ - [ - 'line_item' => [ - [ - 'entity_table' => 'civicrm_contribution', - 'price_field_id' => 8, - 'price_field_value_id' => 16, - 'label' => 'test 1', - 'qty' => 1, - 'unit_price' => 100, - 'line_total' => 100, - ], - [ - 'entity_table' => 'civicrm_contribution', - 'price_field_id' => 8, - 'price_field_value_id' => 17, - 'label' => 'Test 2', - 'qty' => 1, - 'unit_price' => 200, - 'line_total' => 200, - 'financial_type_id' => 1, - ], - ], - 'params' => [], - ], - ], - ]; - - try { - CRM_Contribute_BAO_Contribution::checkLineItems($params); - $this->fail("Missed expected exception"); - } - catch (CRM_Contribute_Exception_CheckLineItemsException $e) { - $this->assertEquals( - CRM_Contribute_Exception_CheckLineItemsException::LINE_ITEM_DIFFERRING_TOTAL_EXCEPTON_MSG, - $e->getMessage() - ); - } - $params['total_amount'] = 300; - - CRM_Contribute_BAO_Contribution::checkLineItems($params); - } - - /** - * Tests CRM_Contribute_BAO_Contribution::checkLineItems() method works with - * floating point values. - */ - public function testCheckLineItemsWithFloatingPointValues() { - $params = [ - 'contact_id' => 202, - 'receive_date' => date('Y-m-d'), - 'total_amount' => 16.67, - 'financial_type_id' => 3, - 'line_items' => [ - [ - 'line_item' => [ - [ - 'entity_table' => 'civicrm_contribution', - 'price_field_id' => 8, - 'price_field_value_id' => 16, - 'label' => 'test 1', - 'qty' => 1, - 'unit_price' => 14.85, - 'line_total' => 14.85, - ], - [ - 'entity_table' => 'civicrm_contribution', - 'price_field_id' => 8, - 'price_field_value_id' => 17, - 'label' => 'Test 2', - 'qty' => 1, - 'unit_price' => 1.66, - 'line_total' => 1.66, - 'financial_type_id' => 1, - ], - [ - 'entity_table' => 'civicrm_contribution', - 'price_field_id' => 8, - 'price_field_value_id' => 17, - 'label' => 'Test 2', - 'qty' => 1, - 'unit_price' => 0.16, - 'line_total' => 0.16, - 'financial_type_id' => 1, - ], - ], - 'params' => [], - ], - ], - ]; - - $foundException = FALSE; - - try { - CRM_Contribute_BAO_Contribution::checkLineItems($params); - } - catch (CRM_Contribute_Exception_CheckLineItemsException $e) { - $foundException = TRUE; - } - - $this->assertFalse($foundException); - } - /** * Test activity amount updates activity subject. */ -- 2.25.1