From ed0cb2fd3d385625bcd86d19f47931635732f08c Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Thu, 16 Nov 2017 10:06:25 +1300
Subject: [PATCH] [NFC] code cleanup: split out 2 functions that are mostly
 unrelated.

The handling of line items & financial items has been munged together in a way that gains little, adds confusion,
and blocks the financial items portion from being re-usable.

This is a code cleanup preparatory to the actual changes in pr #10962
---
 CRM/Price/BAO/LineItem.php | 46 +++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php
index f01ce8bba8b1..8926a5d1fdc0 100644
--- a/CRM/Price/BAO/LineItem.php
+++ b/CRM/Price/BAO/LineItem.php
@@ -760,7 +760,7 @@ public static function changeFeeSelections(
     if (!empty($trxn->id)) {
       $trxnId['id'] = $trxn->id;
     }
-    $lineItemObj->addLineItemOnChangeFeeSelection($requiredChanges['line_items_to_add'], $entityID, $entityTable, $contributionId, $trxnId, TRUE);
+    $lineItemObj->addFinancialItemsOnLineItemsChange($requiredChanges['line_items_to_add'], $entityID, $entityTable, $contributionId, $trxnId);
 
     // update participant fee_amount column
     $lineItemObj->updateEntityRecordOnChangeFeeSelection($params, $entityID, $entity);
@@ -945,6 +945,37 @@ protected function getLineItemsToAlter($submittedLineItems, $entityID, $entity)
     );
   }
 
+  /**
+   * Helper function to add lineitems or financial item related to it, to as result of fee change
+   *
+   * @param array $lineItemsToAdd
+   * @param int $entityID
+   * @param string $entityTable
+   * @param int $contributionID
+   */
+  protected function addLineItemOnChangeFeeSelection(
+    $lineItemsToAdd,
+    $entityID,
+    $entityTable,
+    $contributionID
+  ) {
+    // if there is no line item to add, do not proceed
+    if (empty($lineItemsToAdd)) {
+      return;
+    }
+
+    foreach ($lineItemsToAdd as $priceFieldValueID => $lineParams) {
+      $lineParams = array_merge($lineParams, array(
+        'entity_table' => $entityTable,
+        'entity_id' => $entityID,
+        'contribution_id' => $contributionID,
+      ));
+      if (!array_key_exists('skip', $lineParams)) {
+        self::create($lineParams);
+      }
+    }
+  }
+
   /**
    * Helper function to add lineitems or financial item related to it, to as result of fee change
    *
@@ -953,17 +984,15 @@ protected function getLineItemsToAlter($submittedLineItems, $entityID, $entity)
    * @param string $entityTable
    * @param int $contributionID
    * @param array $adjustedFinancialTrxnID
-   * @param bool $addFinancialItemOnly
    *
    * @return void
    */
-  protected function addLineItemOnChangeFeeSelection(
+  protected function addFinancialItemsOnLineItemsChange(
     $lineItemsToAdd,
     $entityID,
     $entityTable,
     $contributionID,
-    $adjustedFinancialTrxnID = NULL,
-    $addFinancialItemOnly = FALSE
+    $adjustedFinancialTrxnID = NULL
   ) {
     // if there is no line item to add, do not proceed
     if (empty($lineItemsToAdd)) {
@@ -981,12 +1010,7 @@ protected function addLineItemOnChangeFeeSelection(
         'entity_id' => $entityID,
         'contribution_id' => $contributionID,
       ));
-      if ($addFinancialItemOnly) {
-        $changedFinancialTypeID = $this->addFinancialItemsOnLineItemChange(empty($adjustedFinancialTrxnID), $lineParams, $updatedContribution, $tempFinancialTrxnID, $changedFinancialTypeID);
-      }
-      elseif (!array_key_exists('skip', $lineParams)) {
-        self::create($lineParams);
-      }
+      $changedFinancialTypeID = $this->addFinancialItemsOnLineItemChange(empty($adjustedFinancialTrxnID), $lineParams, $updatedContribution, $tempFinancialTrxnID, $changedFinancialTypeID);
     }
 
     if ($changedFinancialTypeID) {