Merge pull request #14892 from eileenmcnaughton/lines

dev/financial#40 add missing financial item when altering a radio amount
civicrm · Aug 28, 2019 · 2430907 · 2430907
2 parents b3bc4bd + 0f62ef9
commit 2430907
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 24 deletions.
diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php
@@ -769,21 +769,18 @@ public static function changeFeeSelections(
           ]);
           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 @@ protected function getAdjustedFinancialItemsToRecord($entityID, $entityTable, $c
         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,21 +1028,29 @@ protected function addLineItemOnChangeFeeSelection(
    * @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, [
         'entity_table' => $entityTable,
         '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 @@ protected function addFinancialItemsOnLineItemChange($isCreateAdditionalFinancia
     $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 @@ protected function addFinancialItemsOnLineItemChange($isCreateAdditionalFinancia
           '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
@@ -538,4 +538,75 @@ public function testRefundWithFeeAmount0() {
     }
   }
 
+  /**
+   * dev-financial-40: Test that partial payment entries in entity-financial-trxn table to ensure that reverse transaction is entered
+   */
+  public function testPartialPaymentEntries() {
+    $this->registerParticipantAndPay($this->_expensiveFee);
+    $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'])['values'];
+    $this->assertCount(3, $actualResults);
+    $expectedResults = [
+      [
+        'id' => 2,
+        'amount' => 100.0,
+        'entity_id' => 1,
+        'financial_trxn_id' => 1,
+        'entity_table' => 'civicrm_financial_item',
+      ],
+      [
+        'id' => 4,
+        // 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
+   */
+  public function testRefundPaymentEntries() {
+    $this->registerParticipantAndPay($this->_expensiveFee);
+    $priceSetParams['price_' . $this->priceSetFieldID] = $this->cheapFeeValueID;
+    $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'];
+    $expectedResults = [
+      [
+        'id' => 2,
+        'amount' => 100.00,
+        'entity_id' => 1,
+      ],
+      [
+        'id' => 4,
+        // ensure that reverse entry is entered in the EntityFinancialTrxn table
+        'amount' => -100.00,
+        'entity_id' => 2,
+      ],
+      [
+        'id' => 5,
+        'amount' => 80.00,
+        'entity_id' => 3,
+      ],
+    ];
+    foreach ($expectedResults as $key => $expectedResult) {
+      $this->checkArrayEquals($expectedResult, $actualResults[$key]);
+    }
+  }
+
 }
diff --git a/tests/phpunit/CRM/Event/Form/ParticipantTest.php b/tests/phpunit/CRM/Event/Form/ParticipantTest.php
@@ -82,6 +82,7 @@ public function testSubmitUnpaidPriceChangeWhileStillPending() {
       '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']);