From fab405c86398fc3c3e075d55fdd7578d8cad1105 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Wed, 30 Oct 2019 23:49:23 +1300
Subject: [PATCH] dev/financial#94 fix eroneous payment creation when editing
 line items

Per https://lab.civicrm.org/dev/financial/issues/94 an eroneous payment transaction is being created when an extra line is
added to an order. The code comments indicate that during the last refactoring the chunk of code
doing this was identified as highly suspect and on re-review of it I'm sure it should go
as we are altering the items purchases, not recording payment in this routine.

I think it dates back to an earlier concept of the flow where altering what was purchased implictly
altered the payments towards that purchase
---
 CRM/Price/BAO/LineItem.php                    | 42 +------------------
 .../CRM/Event/Form/ParticipantTest.php        |  4 ++
 2 files changed, 5 insertions(+), 41 deletions(-)

diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php
index cd9145d6cc..042e2dbc2f 100644
--- a/CRM/Price/BAO/LineItem.php
+++ b/CRM/Price/BAO/LineItem.php
@@ -669,6 +669,7 @@ WHERE li.contribution_id = %1";
    * @param $feeBlock
    * @param array $lineItems
    *
+   * @throws \CiviCRM_API3_Exception
    */
   public static function changeFeeSelections(
     $params,
@@ -1046,7 +1047,6 @@ WHERE li.contribution_id = %1";
         'entity_id' => $entityID,
         'contribution_id' => $contributionID,
       ]);
-      $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.
@@ -1248,46 +1248,6 @@ WHERE li.contribution_id = %1";
     return $adjustedTrxn;
   }
 
-  /**
-   * Add financial items to reflect line item change.
-   *
-   * @param bool $isCreateAdditionalFinancialTrxn
-   * @param array $lineParams
-   * @param \CRM_Contribute_BAO_Contribution $updatedContribution
-   */
-  protected function addFinancialItemsOnLineItemChange($isCreateAdditionalFinancialTrxn, $lineParams, $updatedContribution) {
-    $tempFinancialTrxnID = NULL;
-    // don't add financial item for cancelled line item
-    if ($lineParams['qty'] == 0) {
-      return NULL;
-    }
-    elseif ($isCreateAdditionalFinancialTrxn) {
-      // This routine & the return below is super uncomfortable.
-      // 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'];
-        $adjustedTrxnValues = [
-          'from_financial_account_id' => NULL,
-          'to_financial_account_id' => CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($updatedContribution->payment_instrument_id),
-          'total_amount' => $lineParams['line_total'],
-          'net_amount' => $lineParams['line_total'],
-          'status_id' => $updatedContribution->contribution_status_id,
-          'payment_instrument_id' => $updatedContribution->payment_instrument_id,
-          'contribution_id' => $updatedContribution->id,
-          'is_payment' => TRUE,
-          // since balance is 0, which means contribution is completed
-          'trxn_date' => date('YmdHis'),
-          'currency' => $updatedContribution->currency,
-        ];
-        $adjustedTrxn = CRM_Core_BAO_FinancialTrxn::create($adjustedTrxnValues);
-        return $adjustedTrxn->id;
-      }
-    }
-  }
-
   /**
    * Get Financial items, culling out any that have already been reversed.
    *
diff --git a/tests/phpunit/CRM/Event/Form/ParticipantTest.php b/tests/phpunit/CRM/Event/Form/ParticipantTest.php
index 5a86cfcbb0..851481de22 100644
--- a/tests/phpunit/CRM/Event/Form/ParticipantTest.php
+++ b/tests/phpunit/CRM/Event/Form/ParticipantTest.php
@@ -102,6 +102,10 @@ class CRM_Event_Form_ParticipantTest extends CiviUnitTestCase {
     $this->assertEquals(55, $sum);
 
     CRM_Price_BAO_LineItem::changeFeeSelections($priceSetParams, $participants['id'], 'participant', $contribution['id'], $this->eventFeeBlock, $lineItem);
+    // Check that no payment records have been created.
+    // In https://lab.civicrm.org/dev/financial/issues/94 we had an issue where payments were created when none happend.
+    $payments = $this->callAPISuccess('Payment', 'get', [])['values'];
+    $this->assertCount(0, $payments);
     $lineItem = CRM_Price_BAO_LineItem::getLineItems($participants['id'], 'participant');
     // Participants is updated to 0 but line remains.
     $this->assertEquals(0, $lineItem[1]['subTotal']);
-- 
2.25.1