Move use of priceSetID & amount_override to where they are used

The field name is misleading since it implies it is useful in general to pass in a priceSetID - however, it has
a very specific usage & meaning & ideally we would deprecate it in favour of something more readable. For now
group the code more logically & comment it

diff --git a/CRM/Price/BAO/PriceSet.php b/CRM/Price/BAO/PriceSet.php
index 5ac93721fd9a9d804309a47935c587fa969d8b68..4f34f2243db69de9b42bfbf84233e270d53860a2 100644 (file)
--- a/CRM/Price/BAO/PriceSet.php
+++ b/CRM/Price/BAO/PriceSet.php
@@ -661,23 +661,13 @@ WHERE  id = %1";
    * @param $lineItem
    *   Line item array to be altered.
    * @param int $priceSetID
+   *
+   * @todo $priceSetID is a pseudoparam for permit override - we should stop passing it where we
+   * don't specifically need it & find a better way where we do.
    */
   public static function processAmount($fields, &$params, &$lineItem, $priceSetID = NULL) {
     // using price set
     $totalPrice = $totalTax = 0;
-    // CRM-18701 Sometimes the amount in the price set is overridden by the amount on the form.
-    // This is notably the case with memberships and we need to put this amount
-    // on the line item rather than the calculated amount.
-    // This seems to only affect radio link items as that is the use case for the 'quick config'
-    // set up (which allows a free form field).
-    $amount_override = NULL;
-
-    if ($priceSetID) {
-      $priceFields = self::filterPriceFieldsFromParams($priceSetID, $params);
-      if (count($priceFields) == 1) {
-        $amount_override = CRM_Utils_Array::value('partial_payment_total', $params, CRM_Utils_Array::value('total_amount', $params));
-      }
-    }
     foreach ($fields as $id => $field) {
       if (empty($params["price_{$id}"]) ||
         (empty($params["price_{$id}"]) && $params["price_{$id}"] == NULL)
@@ -693,7 +683,7 @@ WHERE  id = %1";
           CRM_Price_BAO_LineItem::format($id, $params, $field, $lineItem, CRM_Utils_Array::value('partial_payment_total', $params));
           $optionValueId = key($field['options']);
 
-          if (CRM_Utils_Array::value('name', $field['options'][$optionValueId]) == 'contribution_amount') {
+          if (CRM_Utils_Array::value('name', $field['options'][$optionValueId]) === 'contribution_amount') {
             $taxRates = CRM_Core_PseudoConstant::getTaxRates();
             if (array_key_exists($params['financial_type_id'], $taxRates)) {
               $field['options'][key($field['options'])]['tax_rate'] = $taxRates[$params['financial_type_id']];
@@ -715,6 +705,18 @@ WHERE  id = %1";
           $params["price_{$id}"] = [$params["price_{$id}"] => 1];
           $optionValueId = CRM_Utils_Array::key(1, $params["price_{$id}"]);
 
+          // CRM-18701 Sometimes the amount in the price set is overridden by the amount on the form.
+          // This is notably the case with memberships and we need to put this amount
+          // on the line item rather than the calculated amount.
+          // This seems to only affect radio link items as that is the use case for the 'quick config'
+          // set up (which allows a free form field).
+          // @todo $priceSetID is a pseudoparam for permit override - we should stop passing it where we
+          // don't specifically need it & find a better way where we do.
+          $amount_override = NULL;
+
+          if ($priceSetID && count(self::filterPriceFieldsFromParams($priceSetID, $params)) === 1) {
+            $amount_override = CRM_Utils_Array::value('partial_payment_total', $params, CRM_Utils_Array::value('total_amount', $params));
+          }
           CRM_Price_BAO_LineItem::format($id, $params, $field, $lineItem, $amount_override);
           if (!empty($field['options'][$optionValueId]['tax_rate'])) {
             $lineItem = self::setLineItem($field, $lineItem, $optionValueId, $totalTax);
@@ -769,7 +771,7 @@ WHERE  id = %1";
         // will get us by.
         // Crucially a test has been added so a better solution can be implemented later with some comfort.
         // @todo - stop setting amount level in this function & call the getAmountLevel function to retrieve it.
-        if ($values['label'] != ts('Contribution Amount')) {
+        if ($values['label'] !== ts('Contribution Amount')) {
           $amount_level[] = $values['label'] . ' - ' . (float) $values['qty'];
         }
       }
@@ -819,7 +821,7 @@ WHERE  id = %1";
         // We deliberately & specifically exclude contribution amount as it has a specific meaning.
         // ie. it represents the default price field for a contribution. Another approach would be not
         // to give it a label if we don't want it to show.
-        if ($field['label'] != ts('Contribution Amount')) {
+        if ($field['label'] !== ts('Contribution Amount')) {
           $amount_level[] = $field['label'] . $qtyString;
         }
       }