[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
civicrm · Nov 15, 2017 · 76109b1 · 76109b1
1 parent 73162db
commit 76109b1
Showing 1 changed file with 38 additions and 9 deletions.
diff --git 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);
@@ -958,12 +958,46 @@ protected function getLineItemsToAlter($submittedLineItems, $entityID, $entity)
    * @return void
    */
   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
+   *
+   * @param array $lineItemsToAdd
+   * @param int $entityID
+   * @param string $entityTable
+   * @param int $contributionID
+   * @param array $adjustedFinancialTrxnID
+   * @param bool $addFinancialItemOnly
+   *
+   * @return void
+   */
+  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 +1015,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) {