From fde1821d1d163162cfe8950b0cf0b8316be35020 Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 27 Jul 2019 15:59:30 +1200 Subject: [PATCH] dev/financial#40 add missing financial item when altering a radio amount --- CRM/Price/BAO/LineItem.php | 42 ++++++++----------- .../CRM/Event/BAO/ChangeFeeSelectionTest.php | 17 ++++++-- .../CRM/Event/Form/ParticipantTest.php | 1 + 3 files changed, 32 insertions(+), 28 deletions(-) diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php index 04c5a5d3e2..0c88ab14d9 100644 --- a/CRM/Price/BAO/LineItem.php +++ b/CRM/Price/BAO/LineItem.php @@ -769,21 +769,18 @@ WHERE li.contribution_id = %1"; ]); unset($updateFinancialItemInfoValues['financialTrxn']); } - elseif (!empty($updateFinancialItemInfoValues['link-financial-trxn']) && $newFinancialItem->amount != 0) { + elseif ($trxn && $newFinancialItem->amount != 0) { civicrm_api3('EntityFinancialTrxn', 'create', [ 'entity_id' => $newFinancialItem->id, 'entity_table' => 'civicrm_financial_item', 'financial_trxn_id' => $trxn->id, 'amount' => $newFinancialItem->amount, ]); - unset($updateFinancialItemInfoValues['link-financial-trxn']); } } } - // @todo - it may be that trxn_id is always empty - flush out scenarios. Add tests. - $trxnId = !empty($trxn->id) ? ['id' => $trxn->id] : []; - $lineItemObj->addFinancialItemsOnLineItemsChange(array_merge($requiredChanges['line_items_to_add'], $requiredChanges['line_items_to_resurrect']), $entityID, $entityTable, $contributionId, $trxnId); + $lineItemObj->addFinancialItemsOnLineItemsChange(array_merge($requiredChanges['line_items_to_add'], $requiredChanges['line_items_to_resurrect']), $entityID, $entityTable, $contributionId, $trxn->id ?? NULL); // update participant fee_amount column $lineItemObj->updateEntityRecordOnChangeFeeSelection($params, $entityID, $entity); @@ -842,8 +839,6 @@ WHERE li.contribution_id = %1"; 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(); @@ -1033,13 +1028,14 @@ WHERE li.contribution_id = %1"; * @param int $entityID * @param string $entityTable * @param int $contributionID - * @param bool $isCreateAdditionalFinancialTrxn + * @param bool $trxnID * Is there a change to the total balance requiring additional transactions to be created. */ - protected function addFinancialItemsOnLineItemsChange($lineItemsToAdd, $entityID, $entityTable, $contributionID, $isCreateAdditionalFinancialTrxn) { + protected function addFinancialItemsOnLineItemsChange($lineItemsToAdd, $entityID, $entityTable, $contributionID, $trxnID) { $updatedContribution = new CRM_Contribute_BAO_Contribution(); $updatedContribution->id = $contributionID; $updatedContribution->find(TRUE); + $trxnArray = $trxnID ? ['id' => $trxnID] : NULL; foreach ($lineItemsToAdd as $priceFieldValueID => $lineParams) { $lineParams = array_merge($lineParams, [ @@ -1047,7 +1043,14 @@ WHERE li.contribution_id = %1"; 'entity_id' => $entityID, 'contribution_id' => $contributionID, ]); - $this->addFinancialItemsOnLineItemChange($isCreateAdditionalFinancialTrxn, $lineParams, $updatedContribution); + $financialTypeChangeTrxnID = $this->addFinancialItemsOnLineItemChange($trxnID, $lineParams, $updatedContribution); + $lineObj = CRM_Price_BAO_LineItem::retrieve($lineParams); + // insert financial items + // ensure entity_financial_trxn table has a linking of it. + CRM_Financial_BAO_FinancialItem::add($lineObj, $updatedContribution, NULL, $trxnArray); + if (isset($lineObj->tax_amount)) { + CRM_Financial_BAO_FinancialItem::add($lineObj, $updatedContribution, TRUE, $trxnArray); + } } } @@ -1253,15 +1256,13 @@ WHERE li.contribution_id = %1"; $tempFinancialTrxnID = NULL; // don't add financial item for cancelled line item if ($lineParams['qty'] == 0) { - return; + return NULL; } elseif ($isCreateAdditionalFinancialTrxn) { // This routine & the return below is super uncomfortable. - // I have refactored to here but don't understand how this would be hit - // and it is how it would be a good thing, given the odd return below which - // does not seem consistent with what is going on. - // I'm tempted to add an e-deprecated into it to confirm my suspicion it only exists to - // cause mental anguish. + // I have refactored to here and it is hit from + // testSubmitUnpaidPriceChangeWhileStillPending + // but I'm still skeptical it's not covered elsewhere. // original comment : add financial item if ONLY financial type is changed if ($lineParams['financial_type_id'] != $updatedContribution->financial_type_id) { $changedFinancialTypeID = (int) $lineParams['financial_type_id']; @@ -1279,16 +1280,9 @@ WHERE li.contribution_id = %1"; 'currency' => $updatedContribution->currency, ]; $adjustedTrxn = CRM_Core_BAO_FinancialTrxn::create($adjustedTrxnValues); - $tempFinancialTrxnID = ['id' => $adjustedTrxn->id]; + return $adjustedTrxn->id; } } - $lineObj = CRM_Price_BAO_LineItem::retrieve($lineParams); - // insert financial items - // ensure entity_financial_trxn table has a linking of it. - CRM_Financial_BAO_FinancialItem::add($lineObj, $updatedContribution, NULL, $tempFinancialTrxnID); - if (isset($lineObj->tax_amount)) { - CRM_Financial_BAO_FinancialItem::add($lineObj, $updatedContribution, TRUE, $tempFinancialTrxnID); - } } /** diff --git a/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php b/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php index 792e9344b5..21113b149f 100644 --- a/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php +++ b/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php @@ -546,28 +546,36 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase { $priceSetParams['price_' . $this->priceSetFieldID] = $this->veryExpensiveFeeValueID; $lineItem = CRM_Price_BAO_LineItem::getLineItems($this->participantID, 'participant'); CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $this->_participantId, 'participant', $this->_contributionId, $this->_feeBlock, $lineItem); - $actualResults = $this->callAPISuccess('EntityFinancialTrxn', 'get', ['sequential' => 1, 'entity_table' => 'civicrm_financial_item', 'return' => ['amount', 'entity_id']])['values']; + $actualResults = $this->callAPISuccess('EntityFinancialTrxn', 'get', ['sequential' => 1, 'entity_table' => 'civicrm_financial_item'])['values']; $expectedResults = [ [ 'id' => 2, - 'amount' => 100.00, + 'amount' => 100.0, 'entity_id' => 1, + 'financial_trxn_id' => 1, + 'entity_table' => 'civicrm_financial_item', ], [ 'id' => 4, - 'amount' => -100.00, // ensure that reverse entry is entered in the EntityFinancialTrxn table on fee change to greater amount + // ensure that reverse entry is entered in the EntityFinancialTrxn table on fee change to greater amount + 'amount' => -100.0, 'entity_id' => 2, + 'financial_trxn_id' => 2, + 'entity_table' => 'civicrm_financial_item', ], [ 'id' => 5, 'amount' => 120.00, 'entity_id' => 3, + 'financial_trxn_id' => 2, + 'entity_table' => 'civicrm_financial_item', ], ]; foreach ($expectedResults as $key => $expectedResult) { $this->checkArrayEquals($expectedResult, $actualResults[$key]); } } + /** * dev-financial-40: Test that refund payment entries in entity-financial-trxn table to ensure that reverse transaction is entered on fee change to lesser amount */ @@ -585,7 +593,8 @@ class CRM_Event_BAO_ChangeFeeSelectionTest extends CiviUnitTestCase { ], [ 'id' => 4, - 'amount' => -100.00, // ensure that reverse entry is entered in the EntityFinancialTrxn table + // ensure that reverse entry is entered in the EntityFinancialTrxn table + 'amount' => -100.00, 'entity_id' => 2, ], [ diff --git a/tests/phpunit/CRM/Event/Form/ParticipantTest.php b/tests/phpunit/CRM/Event/Form/ParticipantTest.php index cd57dbd88e..4cb83a0935 100644 --- a/tests/phpunit/CRM/Event/Form/ParticipantTest.php +++ b/tests/phpunit/CRM/Event/Form/ParticipantTest.php @@ -82,6 +82,7 @@ class CRM_Event_Form_ParticipantTest extends CiviUnitTestCase { 'financial_type_id' => 1, 'contribution_status_id' => 2, 'payment_instrument_id' => 1, + 'receive_date' => date('Y-m-d'), ]); $participants = $this->callAPISuccess('Participant', 'get', []); $this->assertEquals(1, $participants['count']); -- 2.25.1