The processAmount function is really problematic because it's trying to do several disparate things & is called from
all over the place with unclear intent. From looking at it I see it does a few things. It reformats line items,
it does some obscure & likely flawed filtering, it generates a total cost & a tax cost and it generates a very
specifically formatted array of autoreneal properties,
The way I see this going is
1) Move the formatting of the autorenewal back to the calling form (this PR) & simplify it
2) Split out the foreach so it goes through once & formats - this can be shoved out to a separate function
- and then it goes through the formatted array & calculates total_amount & tax_amount - we should
have a wrapper function that just returns these & we might see that is most of what is needed
3) Move all that awful partial_payment_total stuff back to the event form. Note that we are working
to entirely remove that from here are it makes so much less than no sense.
4) Calculates amount_level text - that should have it's own function.
It worth noting all of this does very little intensive work - a DB lookup or 2 that could be cached & an iteration
through a very small array so it would be fine to have 3 functions -
- getAmountTotal
- getTaxTotal
- getAmountText
that each go through the same process of generating a formatted array from price_x => 5 etc rather than trying
to pass the array around for 'performance' or to 'save work'.
From previous refactorings I would suggest we add an Order class where by you set the price fields
& then you can call 'getLineItems' - but that is a few steps after this....
// as a point of fragility rather than a logical 'if' clause.
if ($priceSetId) {
CRM_Price_BAO_PriceSet::processAmount($this->_priceSet['fields'],
- $submittedValues, $lineItem[$priceSetId], NULL, $priceSetId);
+ $submittedValues, $lineItem[$priceSetId], $priceSetId);
// Unset tax amount for offline 'is_quick_config' contribution.
// @todo WHY - quick config was conceived as a quick way to configure contribution forms.
// this is an example of 'other' functionality being hung off it.
}
$priceFields = $priceFields[$priceSetID]['fields'];
$lineItems = [];
- CRM_Price_BAO_PriceSet::processAmount($priceFields, $paramsProcessedForForm, $lineItems, 'civicrm_contribution', $priceSetID);
+ $form->processAmountAndGetAutoRenew($priceFields, $paramsProcessedForForm, $lineItems, $priceSetID);
$form->_lineItem = [$priceSetID => $lineItems];
$membershipPriceFieldIDs = [];
foreach ((array) $lineItems as $lineItem) {
}
}
}
- $component = '';
+
if ($this->_membershipBlock) {
- $component = 'membership';
+ $this->processAmountAndGetAutoRenew($this->_values['fee'], $params, $lineItem[$priceSetId], $priceSetId);
+ }
+ else {
+ CRM_Price_BAO_PriceSet::processAmount($this->_values['fee'], $params, $lineItem[$priceSetId], $priceSetId);
}
- CRM_Price_BAO_PriceSet::processAmount($this->_values['fee'], $params, $lineItem[$priceSetId], $component, $priceSetId);
if ($params['tax_amount']) {
$this->set('tax_amount', $params['tax_amount']);
}
return $params['amount'] ?? 0;
}
+ /**
+ * Wrapper for processAmount that also sets autorenew.
+ *
+ * @param $fields
+ * This is the output of the function CRM_Price_BAO_PriceSet::getSetDetail($priceSetID, FALSE, FALSE);
+ * And, it would make sense to introduce caching into that function and call it from here rather than
+ * require the $fields array which is passed from pillar to post around the form in order to pass it in here.
+ * @param array $params
+ * Params reflecting form input e.g with fields 'price_5' => 7, 'price_8' => array(7, 8)
+ * @param $lineItems
+ * Line item array to be altered.
+ * @param int $priceSetID
+ */
+ public function processAmountAndGetAutoRenew($fields, &$params, &$lineItems, $priceSetID = NULL) {
+ CRM_Price_BAO_PriceSet::processAmount($fields, $params, $lineItems, $priceSetID);
+ $autoRenew = [];
+ $autoRenew[0] = $autoRenew[1] = $autoRenew[2] = 0;
+ foreach ($lineItems as $lineItem) {
+ if (!empty($lineItem['auto_renew']) &&
+ is_numeric($lineItem['auto_renew'])
+ ) {
+ $autoRenew[$lineItem['auto_renew']] += $lineItem['line_total'];
+ }
+ }
+ if (count($autoRenew) > 1) {
+ $params['autoRenew'] = $autoRenew;
+ }
+ }
+
}
/**
* Process the form submission.
+ *
+ * @throws \CiviCRM_API3_Exception
+ * @throws \CRM_Core_Exception
*/
public function postProcess() {
$now = date('YmdHis');
// END Fix for dev/core/issues/860
CRM_Price_BAO_PriceSet::processAmount($this->_priceSet['fields'],
- $formValues, $lineItem[$this->_priceSetId], NULL, $this->_priceSetId);
+ $formValues, $lineItem[$this->_priceSetId], $this->_priceSetId);
if (!empty($formValues['tax_amount'])) {
$params['tax_amount'] = $formValues['tax_amount'];
$lineItem = [];
$this->_params = $this->setPriceSetParameters($this->_params);
CRM_Price_BAO_PriceSet::processAmount($this->_priceSet['fields'],
- $this->_params, $lineItem[$this->_priceSetId], NULL, $this->_priceSetId
+ $this->_params, $lineItem[$this->_priceSetId], $this->_priceSetId
);
//CRM-11529 for quick config backoffice transactions
//when financial_type_id is passed in form, update the
* Params reflecting form input e.g with fields 'price_5' => 7, 'price_8' => array(7, 8)
* @param $lineItem
* Line item array to be altered.
- * @param string $component
- * This parameter appears to only be relevant to determining whether memberships should be auto-renewed.
- * (and is effectively a boolean for 'is_membership' which could be calculated from the line items.)
* @param int $priceSetID
*/
- public static function processAmount($fields, &$params, &$lineItem, $component = '', $priceSetID = NULL) {
+ 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.
// set up (which allows a free form field).
$amount_override = NULL;
- if ($component) {
- $autoRenew = [];
- $autoRenew[0] = $autoRenew[1] = $autoRenew[2] = 0;
- }
if ($priceSetID) {
$priceFields = self::filterPriceFieldsFromParams($priceSetID, $params);
if (count($priceFields) == 1) {
}
}
$totalPrice += $lineItem[$optionValueId]['line_total'] + CRM_Utils_Array::value('tax_amount', $lineItem[$optionValueId]);
- if (
- $component &&
- // auto_renew exists and is empty in some workflows, which php treat as a 0
- // and hence we explicitly check to see if auto_renew is numeric
- isset($lineItem[$optionValueId]['auto_renew']) &&
- is_numeric($lineItem[$optionValueId]['auto_renew'])
- ) {
- $autoRenew[$lineItem[$optionValueId]['auto_renew']] += $lineItem[$optionValueId]['line_total'];
- }
break;
case 'Select':
$lineItem = self::setLineItem($field, $lineItem, $optionValueId, $totalTax);
}
$totalPrice += $lineItem[$optionValueId]['line_total'] + CRM_Utils_Array::value('tax_amount', $lineItem[$optionValueId]);
- if (
- $component &&
- isset($lineItem[$optionValueId]['auto_renew']) &&
- is_numeric($lineItem[$optionValueId]['auto_renew'])
- ) {
- $autoRenew[$lineItem[$optionValueId]['auto_renew']] += $lineItem[$optionValueId]['line_total'];
- }
break;
case 'CheckBox':
$lineItem = self::setLineItem($field, $lineItem, $optionId, $totalTax);
}
$totalPrice += $lineItem[$optionId]['line_total'] + CRM_Utils_Array::value('tax_amount', $lineItem[$optionId]);
- if (
- $component &&
- isset($lineItem[$optionId]['auto_renew']) &&
- is_numeric($lineItem[$optionId]['auto_renew'])
- ) {
- $autoRenew[$lineItem[$optionId]['auto_renew']] += $lineItem[$optionId]['line_total'];
- }
}
break;
}
$params['amount'] = $totalPrice;
$params['tax_amount'] = $totalTax;
- if ($component) {
- foreach ($autoRenew as $dontCare => $eachAmount) {
- if (!$eachAmount) {
- unset($autoRenew[$dontCare]);
- }
- }
- if (count($autoRenew) > 1) {
- $params['autoRenew'] = $autoRenew;
- }
- }
}
/**
projects / civicrm-core.git / commitdiff