From 484c7b03c9c992499676f720085320149413ca53 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 27 Nov 2023 09:07:37 +1300 Subject: [PATCH] Remove now-unused handling for amount, line items --- CRM/Contribute/Form/Contribution/Confirm.php | 6 ++ CRM/Contribute/Form/Contribution/Main.php | 71 +++----------------- CRM/Contribute/Form/ContributionBase.php | 11 +-- 3 files changed, 17 insertions(+), 71 deletions(-) diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php index cee6551f3c..7ae0b06378 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -1787,6 +1787,8 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr /** * This function sets the fields. * + * The results of it are likely unused. + * * - $this->_params['amount_level'] * - $this->_params['selectMembership'] * And under certain circumstances sets @@ -1811,10 +1813,12 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr // function to get correct amount level consistently. Remove setting of the amount level in // CRM_Price_BAO_PriceSet::processAmount. Extend the unit tests in CRM_Price_BAO_PriceSetTest // to cover all variants. + // We expect this to be ignored. $this->_params['amount_level'] = CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceFieldValue', $this->_params["price_{$priceField->id}"], 'label'); } if ($priceField->name == "membership_amount") { + // We expect this to be ignored. $this->_params['selectMembership'] = CRM_Core_DAO::getFieldValue('CRM_Price_DAO_PriceFieldValue', $this->_params["price_{$priceField->id}"], 'membership_type_id'); } @@ -1833,6 +1837,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr && ($this->_params["price_{$paramWeDoNotUnderstand}"] ?? NULL) < 1 && empty($this->_params["price_{$priceField->id}"]) ) { + // We expect this to be ignored. $this->_params['amount'] = NULL; } @@ -1846,6 +1851,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr && ($this->_values['fee'][$priceField->id]['name'] ?? NULL) == 'contribution_amount' && ($this->_params["price_{$priceField->id}"] ?? NULL) == '-1' ) { + // We expect this to be ignored. $this->_params['amount'] = NULL; } } diff --git a/CRM/Contribute/Form/Contribution/Main.php b/CRM/Contribute/Form/Contribution/Main.php index 42d34afe72..31407a467d 100644 --- a/CRM/Contribute/Form/Contribution/Main.php +++ b/CRM/Contribute/Form/Contribution/Main.php @@ -1012,7 +1012,8 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu $errors['_qf_default'] = ts('Please select at least one membership option.'); } } - + // @todo - processAmount is to be deprectated - can we use getTotalAmount or + // a function of self->order here? CRM_Price_BAO_PriceSet::processAmount($self->_values['fee'], $fields, $lineItem ); @@ -1214,6 +1215,7 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu $params['currencyID'] = CRM_Core_Config::singleton()->defaultCurrency; if ($this->isQuickConfig()) { + // @todo - this is silly cruft - we can likely remove it. $priceField = new CRM_Price_DAO_PriceField(); $priceField->price_set_id = $this->getPriceSetID(); $priceField->orderBy('weight'); @@ -1227,77 +1229,28 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu case 'membership_amount': $this->_params['selectMembership'] = $params['selectMembership'] = $priceOptions[$selectedPriceOptionID]['membership_type_id'] ?? NULL; $this->set('selectMembership', $params['selectMembership']); - - case 'contribution_amount': - $params['amount'] = $selectedPriceOptionID; - if ($priceField->name == 'contribution_amount' || - ($priceField->name == 'membership_amount' && - ($this->_membershipBlock['is_separate_payment'] ?? NULL) == 0) - ) { - $this->_values['amount'] = $priceOptions[$selectedPriceOptionID]['amount'] ?? NULL; - } - $this->_values[$selectedPriceOptionID]['value'] = $priceOptions[$selectedPriceOptionID]['amount'] ?? NULL; - $this->_values[$selectedPriceOptionID]['label'] = $priceOptions[$selectedPriceOptionID]['label'] ?? NULL; - $this->_values[$selectedPriceOptionID]['amount_id'] = $priceOptions[$selectedPriceOptionID]['id'] ?? NULL; - $this->_values[$selectedPriceOptionID]['weight'] = $priceOptions[$selectedPriceOptionID]['weight'] ?? NULL; break; case 'other_amount': + // Only used now when deciding whether to assign + // amount_level to the template in subsequent screens. $params['amount_other'] = $selectedPriceOptionID; break; } } } } - $balance = $this->getContributionBalance(); - if ($balance) { - $params['amount'] = $balance; - } - else { - // from here on down, $params['amount'] holds a monetary value (or null) rather than an option ID - $params['amount'] = $this->computeAmount($params, $this->_values); - } - $params['separate_amount'] = $params['amount']; - // @todo - stepping through the code indicates that amount is always set before this point so it never matters. - // Move more of the above into this function... $params['amount'] = $this->getMainContributionAmount(); - - if (!isset($params['amount_other'])) { - $this->set('amount_level', CRM_Utils_Array::value('amount_level', $params)); - } - - $priceSetID = $this->getPriceSetID(); + $this->set('amount_level', $this->order->getAmountLevel()); if (!empty($this->_ccid)) { // @todo - verify that this is the same as `$this->>getLineItems()` which it should be & consolidate $this->set('lineItem', [$this->getPriceSetID() => $this->getExistingContributionLineItems()]); } - elseif ($priceSetID) { - $lineItem = []; - if ($this->isQuickConfig()) { - foreach ($this->_values['fee'] as $key => & $val) { - if ($val['name'] == 'other_amount' && $val['html_type'] == 'Text' && !empty($params['price_' . $key])) { - // Clean out any currency symbols. - $params['price_' . $key] = CRM_Utils_Rule::cleanMoney($params['price_' . $key]); - if ($params['price_' . $key] != 0) { - foreach ($val['options'] as $optionKey => & $options) { - $options['amount'] = $params['price_' . $key] ?? NULL; - break; - } - } - $params['price_' . $key] = 1; - break; - } - } - } - + else { if ($this->_membershipBlock) { - $this->processAmountAndGetAutoRenew($this->_values['fee'], $params, $lineItem[$priceSetID]); + $this->processAmountAndGetAutoRenew($params); } - else { - CRM_Price_BAO_PriceSet::processAmount($this->_values['fee'], $params, $lineItem[$priceSetID], $priceSetID); - } - $this->set('lineItem', [$this->getPriceSetID() => $this->getLineItems()]); } @@ -1314,13 +1267,7 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu // Would be nice to someday understand the point of this set. $this->set('is_pay_later', $params['is_pay_later']); - - if ($this->_membershipBlock && $this->_membershipBlock['is_separate_payment'] && !empty($params['separate_amount'])) { - $this->set('amount', $params['separate_amount']); - } - else { - $this->set('amount', $params['amount']); - } + $this->set('amount', $this->getMainContributionAmount()); // generate and set an invoiceID for this transaction $invoiceID = md5(uniqid(rand(), TRUE)); diff --git a/CRM/Contribute/Form/ContributionBase.php b/CRM/Contribute/Form/ContributionBase.php index 70269527e0..90e7170a7c 100644 --- a/CRM/Contribute/Form/ContributionBase.php +++ b/CRM/Contribute/Form/ContributionBase.php @@ -1212,20 +1212,13 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form { /** * 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. */ - public function processAmountAndGetAutoRenew($fields, &$params, &$lineItems) { - CRM_Price_BAO_PriceSet::processAmount($fields, $params, $lineItems, $this->getPriceSetID()); + public function processAmountAndGetAutoRenew(&$params) { $autoRenew = []; $autoRenew[0] = $autoRenew[1] = $autoRenew[2] = 0; - foreach ($lineItems as $lineItem) { + foreach ($this->getLineItems() as $lineItem) { if (!empty($lineItem['auto_renew']) && is_numeric($lineItem['auto_renew']) ) { -- 2.25.1