From 745319383dcf017537be91c5d17b536b9372ef9e Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 15 Jun 2019 07:27:25 -0400 Subject: [PATCH] Fix creation of additional zero value line item when changing fee selection in edge case In working with the test written by Monish as part of multiline allocation I hit this bug whereby a zero value line item is created when altering one of 2 text fields in a price set --- CRM/Price/BAO/LineItem.php | 27 +-- CRM/Price/BAO/PriceField.php | 2 + CRM/Price/BAO/PriceFieldValue.php | 2 + .../CRM/Event/Form/ParticipantTest.php | 156 +++++++++++++++++- tests/phpunit/CiviTest/CiviUnitTestCase.php | 3 +- 5 files changed, 176 insertions(+), 14 deletions(-) diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php index 3859a974b8..78d8dc304c 100644 --- a/CRM/Price/BAO/LineItem.php +++ b/CRM/Price/BAO/LineItem.php @@ -832,19 +832,24 @@ WHERE li.contribution_id = %1"; $financialItemsArray[$updateFinancialItemInfoValues['entity_id']] = $updateFinancialItemInfoValues; } // INSERT a financial item to record surplus/lesser amount when a text price fee is changed - elseif (!empty($lineItemsToUpdate) && - $lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]['html_type'] == 'Text' && - $updateFinancialItemInfoValues['amount'] > 0 + elseif ( + !empty($lineItemsToUpdate) + && isset($lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]) + && $lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]['html_type'] == 'Text' + && $updateFinancialItemInfoValues['amount'] > 0 ) { - // calculate the amount difference, considered as financial item amount - $updateFinancialItemInfoValues['amount'] = $lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]['line_total'] - $totalFinancialAmount; - // add a flag, later used to link financial trxn and this new financial item - $updateFinancialItemInfoValues['link-financial-trxn'] = TRUE; - if ($previousLineItems[$updateFinancialItemInfoValues['entity_id']]['tax_amount']) { - $updateFinancialItemInfoValues['tax']['amount'] = $lineItemsToUpdate[$updateFinancialItemInfoValues['entity_id']]['tax_amount'] - $previousLineItems[$updateFinancialItemInfoValues['entity_id']]['tax_amount']; - $updateFinancialItemInfoValues['tax']['description'] = $this->getSalesTaxTerm(); + $amountChangeOnTextLineItem = $lineItemsToUpdate[$updateFinancialItemInfoValues['price_field_value_id']]['line_total'] - $totalFinancialAmount; + if ($amountChangeOnTextLineItem !== (float) 0) { + // calculate the amount difference, considered as financial item amount + $updateFinancialItemInfoValues['amount'] = $amountChangeOnTextLineItem; + // add a flag, later used to link financial trxn and this new financial item + $updateFinancialItemInfoValues['link-financial-trxn'] = TRUE; + if ($previousLineItems[$updateFinancialItemInfoValues['entity_id']]['tax_amount']) { + $updateFinancialItemInfoValues['tax']['amount'] = $lineItemsToUpdate[$updateFinancialItemInfoValues['entity_id']]['tax_amount'] - $previousLineItems[$updateFinancialItemInfoValues['entity_id']]['tax_amount']; + $updateFinancialItemInfoValues['tax']['description'] = $this->getSalesTaxTerm(); + } + $financialItemsArray[$updateFinancialItemInfoValues['entity_id']] = $updateFinancialItemInfoValues; } - $financialItemsArray[$updateFinancialItemInfoValues['entity_id']] = $updateFinancialItemInfoValues; } } diff --git a/CRM/Price/BAO/PriceField.php b/CRM/Price/BAO/PriceField.php index 0085d6ec89..67bcd99b65 100644 --- a/CRM/Price/BAO/PriceField.php +++ b/CRM/Price/BAO/PriceField.php @@ -82,7 +82,9 @@ class CRM_Price_BAO_PriceField extends CRM_Price_DAO_PriceField { * (reference) an assoc array of name/value pairs. * * @return CRM_Price_DAO_PriceField + * * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception */ public static function create(&$params) { if (empty($params['id']) && empty($params['name'])) { diff --git a/CRM/Price/BAO/PriceFieldValue.php b/CRM/Price/BAO/PriceFieldValue.php index 071d879aa3..dd590ca82f 100644 --- a/CRM/Price/BAO/PriceFieldValue.php +++ b/CRM/Price/BAO/PriceFieldValue.php @@ -85,6 +85,8 @@ class CRM_Price_BAO_PriceFieldValue extends CRM_Price_DAO_PriceFieldValue { * @param $ids * * @return CRM_Price_DAO_PriceFieldValue + * + * @throws \CRM_Core_Exception */ public static function create(&$params, $ids = []) { $id = CRM_Utils_Array::value('id', $params, CRM_Utils_Array::value('id', $ids)); diff --git a/tests/phpunit/CRM/Event/Form/ParticipantTest.php b/tests/phpunit/CRM/Event/Form/ParticipantTest.php index 77f0982f03..fb3d006123 100644 --- a/tests/phpunit/CRM/Event/Form/ParticipantTest.php +++ b/tests/phpunit/CRM/Event/Form/ParticipantTest.php @@ -114,6 +114,58 @@ class CRM_Event_Form_ParticipantTest extends CiviUnitTestCase { $this->assertEquals(100, $sum); } + /** + * (dev/core#310) : Test to ensure payments are correctly allocated, when a event fee is changed for a mult-line item event registration + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + public function testPaymentAllocationOnMultiLineItemEvent() { + // USE-CASE : + // 1. Create a Price set with two price fields + // 2. Register for a Event using both the price field A($55 - qty 1) and B($10 - qty 1) + // 3. Now after registration, edit the participant, change the fee of price B from $10 to $50 (i.e. change qty from 1 to 5) + // 4. After submission check that related contribution's status is changed to 'Partially Paid' + // 5. Record the additional amount which $40 ($50-$10) + // Expected : Check the amount of new Financial Item created is $40 + $this->createParticipantRecordsFromTwoFieldPriceSet(); + $priceSetBlock = CRM_Price_BAO_PriceSet::getSetDetail($this->_ids['price_set'], TRUE, FALSE)[$this->_ids['price_set']]['fields'];; + + $priceSetParams = [ + 'priceSetId' => $this->_ids['price_set'], + // The 1 & 5 refer to qty as they are text fields. + 'price_' . $this->_ids['price_field']['first_text_field'] => 5, + 'price_' . $this->_ids['price_field']['second_text_field'] => 1, + ]; + $participant = $this->callAPISuccess('Participant', 'get', []); + $lineItem = CRM_Price_BAO_LineItem::getLineItems($participant['id'], 'participant'); + $contribution = $this->callAPISuccessGetSingle('Contribution', []); + CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $participant['id'], 'participant', $contribution['id'], $priceSetBlock, $lineItem); + + $financialItems = $this->callAPISuccess('FinancialItem', 'get', [])['values']; + $sum = 0; + foreach ($financialItems as $financialItem) { + $sum += $financialItem['amount']; + } + $this->assertEquals(105, $sum); + $this->assertEquals(3, count($financialItems)); + + $contribution = $this->callAPISuccessGetSingle('Contribution', []); + $this->assertEquals('Partially paid', $contribution['contribution_status']); + + $this->callAPISuccess('Payment', 'create', [ + 'contribution_id' => $contribution['id'], + 'participant_id' => $participant['id'], + 'total_amount' => 40.00, + 'currency' => 'USD', + 'payment_instrument_id' => 'Check', + 'check_number' => '#123', + ]); + + $result = $this->callAPISuccess('EntityFinancialTrxn', 'get', ['entity_table' => 'civicrm_financial_item', 'sequential' => 1, 'return' => ['entity_table', 'amount']])['values']; + // @todo check the values assigned to these as part of fixing dev/financial#34 + } + /** * Initial test of submit function. * @@ -218,8 +270,9 @@ class CRM_Event_Form_ParticipantTest extends CiviUnitTestCase { * @param array $eventParams * * @return CRM_Event_Form_Participant + * @throws \CRM_Core_Exception */ - protected function getForm($eventParams = array()) { + protected function getForm($eventParams = []) { if (!empty($eventParams['is_monetary'])) { $event = $this->eventCreatePaid($eventParams); } @@ -228,6 +281,7 @@ class CRM_Event_Form_ParticipantTest extends CiviUnitTestCase { } $contactID = $this->individualCreate(); + /** @var CRM_Event_Form_Participant $form*/ $form = $this->getFormObject('CRM_Event_Form_Participant'); $form->_single = TRUE; $form->_contactID = $form->_contactId = $contactID; @@ -235,10 +289,108 @@ class CRM_Event_Form_ParticipantTest extends CiviUnitTestCase { $form->_eventId = $event['id']; if (!empty($eventParams['is_monetary'])) { $form->_bltID = 5; - $form->_values['fee'] = array(); + $form->_values['fee'] = []; $form->_isPaidEvent = TRUE; } return $form; } + /** + * Create a Price set with two price field of type Text. + * + * Financial Type: 'Event Fee' and 'Event Fee 2' respectively. + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ + protected function createParticipantRecordsFromTwoFieldPriceSet() { + // Create financial type - Event Fee 2 + $form = $this->getForm(['is_monetary' => 1, 'financial_type_id' => 1]); + + $textFieldsToCreate = [['amount' => 10, 'label' => 'First Text field'], ['amount' => 55, 'label' => 'Second Text field']]; + foreach ($textFieldsToCreate as $fieldToCreate) { + $fieldParams = [ + 'option_label' => ['1' => 'Price Field'], + 'option_value' => ['1' => $fieldToCreate['amount']], + 'option_name' => ['1' => $fieldToCreate['amount']], + 'option_amount' => ['1' => $fieldToCreate['amount']], + 'option_weight' => ['1' => $fieldToCreate['amount']], + 'is_display_amounts' => 1, + 'price_set_id' => $this->_ids['price_set'], + 'is_enter_qty' => 1, + 'html_type' => 'Text', + 'financial_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Campaign Contribution'), + ]; + $fieldParams['label'] = $fieldToCreate['label']; + $fieldParams['name'] = CRM_Utils_String::titleToVar($fieldToCreate['label']); + $fieldParams['price'] = $fieldToCreate['amount']; + $this->_ids['price_field'][strtolower(CRM_Utils_String::titleToVar($fieldToCreate['label']))] = $textPriceFieldID = $this->callAPISuccess('PriceField', 'create', $fieldParams)['id']; + $this->_ids['price_field_value'][strtolower(CRM_Utils_String::titleToVar($fieldToCreate['label']))] = (int) $this->callAPISuccess('PriceFieldValue', 'getsingle', ['price_field_id' => $textPriceFieldID])['id']; + } + + $form->_lineItem = [ + 0 => [ + 13 => [ + 'price_field_id' => $this->_ids['price_field']['second_text_field'], + 'price_field_value_id' => $this->_ids['price_field_value']['second_text_field'], + 'label' => 'Event Fee 1', + 'field_title' => 'Event Fee 1', + 'description' => NULL, + 'qty' => 1, + 'unit_price' => 55.00, + 'line_total' => 55., + 'participant_count' => 0, + 'max_value' => NULL, + 'membership_type_id' => NULL, + 'membership_num_terms' => NULL, + 'auto_renew' => NULL, + 'html_type' => 'Text', + 'financial_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Campaign Contribution'), + 'tax_amount' => NULL, + 'non_deductible_amount' => '0.00', + ], + 14 => [ + 'price_field_id' => $this->_ids['price_field']['first_text_field'], + 'price_field_value_id' => $this->_ids['price_field_value']['first_text_field'], + 'label' => 'Event Fee 2', + 'field_title' => 'Event Fee 2', + 'description' => NULL, + 'qty' => 1, + 'unit_price' => 10.00, + 'line_total' => 10, + 'participant_count' => 0, + 'max_value' => NULL, + 'membership_type_id' => NULL, + 'membership_num_terms' => NULL, + 'auto_renew' => NULL, + 'html_type' => 'Text', + 'financial_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Campaign Contribution'), + 'tax_amount' => NULL, + 'non_deductible_amount' => '0.00', + ], + ], + ]; + $form->setAction(CRM_Core_Action::ADD); + $form->_priceSetId = $this->_ids['price_set']; + + $form->submit([ + 'register_date' => date('Ymd'), + 'receive_date' => '2018-09-01', + 'status_id' => 5, + 'role_id' => 1, + 'event_id' => $form->_eventId, + 'priceSetId' => $this->_ids['price_set'], + 'price_' . $this->_ids['price_field']['first_text_field'] => [$this->_ids['price_field_value']['first_text_field'] => 1], + 'price_' . $this->_ids['price_field']['second_text_field'] => [$this->_ids['price_field_value']['second_text_field'] => 1], + 'amount_level' => 'Too much', + 'fee_amount' => 65, + 'total_amount' => 65, + 'payment_processor_id' => 0, + 'record_contribution' => TRUE, + 'financial_type_id' => 1, + 'contribution_status_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed'), + 'payment_instrument_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'payment_instrument_id', 'Check'), + ]); + } + } diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index f40a07df21..cba9742ea2 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -2544,6 +2544,7 @@ AND ( TABLE_NAME LIKE 'civicrm_value_%' ) * * @return int * Price Set ID. + * @throws \CRM_Core_Exception */ protected function eventPriceSetCreate($feeTotal, $minAmt = 0, $type = 'Text') { // creating price set, price field @@ -2579,7 +2580,7 @@ AND ( TABLE_NAME LIKE 'civicrm_value_%' ) $paramsField['option_value'][2] = $paramsField['option_weight'][2] = $paramsField['option_amount'][2] = 100; $paramsField['option_label'][2] = $paramsField['option_name'][2] = 'hundy'; } - CRM_Price_BAO_PriceField::create($paramsField); + $this->callAPISuccess('PriceField', 'create', $paramsField); $fields = $this->callAPISuccess('PriceField', 'get', array('price_set_id' => $this->_ids['price_set'])); $this->_ids['price_field'] = array_keys($fields['values']); $fieldValues = $this->callAPISuccess('PriceFieldValue', 'get', array('price_field_id' => $this->_ids['price_field'][0])); -- 2.25.1