From ba2bd8753f48a7b9057f5528fd07dee9bc2e07bb Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 9 Jan 2020 20:01:04 +1300 Subject: [PATCH] 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 --- CRM/Price/BAO/PriceSet.php | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/CRM/Price/BAO/PriceSet.php b/CRM/Price/BAO/PriceSet.php index 5ac93721fd..4f34f2243d 100644 --- 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; } } -- 2.25.1