From 5730e4e5737d190aa5817e610e012c1e1c14915a Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Thu, 23 Nov 2023 13:22:35 +1300 Subject: [PATCH] Fix test to have correct input params The test was exploiting a code gremlin which did not require the right submitted values to be set to trigger a secondary contribution - this fixed. other_amount is not a real field as the presented field is always price_x --- CRM/Contribute/Form/Contribution/Confirm.php | 48 ++++++++++++------- .../Form/Contribution/ConfirmTest.php | 4 +- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php index 3ea55fbdae..06a0837567 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -1443,7 +1443,6 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr if ($this->isSeparatePaymentSelected()) { try { - $this->_lineItem = $unprocessedLineItems; if (empty($this->_params['auto_renew']) && !empty($membershipParams['is_recur'])) { unset($membershipParams['is_recur']); } @@ -1562,7 +1561,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr // If this is a single membership-related contribution, it won't have // be performed yet, so do it now. - if ($isPaidMembership && !$isProcessSeparateMembershipTransaction) { + if ($isPaidMembership && !$this->isSeparatePaymentSelected()) { $paymentActionResult = $payment->doPayment($paymentParams); $paymentResults[] = ['contribution_id' => $paymentResult['contribution']->id, 'result' => $paymentActionResult]; } @@ -1852,7 +1851,6 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr $form->_id = $params['id']; CRM_Contribute_BAO_ContributionPage::setValues($form->_id, $form->_values); - $form->_separateMembershipPayment = $form->isSeparateMembershipPayment(); //this way the mocked up controller ignores the session stuff $_SERVER['REQUEST_METHOD'] = 'GET'; $form->controller = new CRM_Contribute_Controller_Contribution(); @@ -1907,8 +1905,6 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr $form->_useForMember = 1; } $priceFields = $priceFields[$priceSetID]['fields']; - - $form->_lineItem = [$priceSetID => $form->order->getLineItems()]; $membershipPriceFieldIDs = []; foreach ($form->order->getLineItems() as $lineItem) { if (!empty($lineItem['membership_type_id'])) { @@ -2372,20 +2368,10 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr $membershipParams = $this->getMembershipParamsFromPriceSet($membershipParams); if ($this->isMembershipSelected()) { - // CRM-12233 - $membershipLineItems = [$this->getPriceSetID() => $this->getLineItems()];; - if ($this->isSeparatePaymentSelected()) { - $membershipLineItems = []; - foreach ($this->_values['fee'] as $key => $feeValues) { - if ($feeValues['name'] == 'membership_amount') { - $fieldId = $this->_params['price_' . $key]; - unset($this->_lineItem[$this->_priceSetId][$fieldId]); - $membershipLineItems[$this->_priceSetId][$fieldId] = $this->getLineItems()[$fieldId]; - break; - } - } - } + // CRM-12233. try { + $membershipLineItems = [$this->getPriceSetID() => $this->getMainContributionLineItems()]; + $membershipParams['amount'] = $this->getMainContributionAmount(); $this->processMembership($membershipParams, $contactID, $customFieldsFormatted, $premiumParams, $membershipLineItems); } catch (\Civi\Payment\Exception\PaymentProcessorException $e) { @@ -2411,6 +2397,32 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr } } + /** + * Get the amount for the main contribution. + * + * The goal is to expand this function so that all the argy-bargy of figuring out the amount + * winds up here as the main spaghetti shrinks. + * + * If there is a separate membership contribution this is the 'other one'. Otherwise there + * is only one. + * + * @todo - move this to the parent, replace existing (some tests to fight). + * + * @param $params + * + * @return float + * + * @throws \CRM_Core_Exception + */ + protected function getMainContributionAmount($params = []) { + $amount = 0; + foreach ($this->getMainContributionLineItems() as $lineItem) { + // Line total inclusive should really be always set but this is a safe fall back. + $amount += $lineItem['line_total_inclusive'] ?? ($lineItem['line_total'] + $lineItem['tax_amount']); + } + return $amount; + } + /** * Complete transaction if payment has been processed. * diff --git a/tests/phpunit/CRM/Contribute/Form/Contribution/ConfirmTest.php b/tests/phpunit/CRM/Contribute/Form/Contribution/ConfirmTest.php index 97c3aca606..461163b46d 100644 --- a/tests/phpunit/CRM/Contribute/Form/Contribution/ConfirmTest.php +++ b/tests/phpunit/CRM/Contribute/Form/Contribution/ConfirmTest.php @@ -216,7 +216,7 @@ class CRM_Contribute_Form_Contribution_ConfirmTest extends CiviUnitTestCase { ' Amount - $1,000.00 + $352.00 ', '************1111', ]); @@ -314,7 +314,7 @@ class CRM_Contribute_Form_Contribution_ConfirmTest extends CiviUnitTestCase { 'Y' => (int) (CRM_Utils_Time::date('Y')) + 1, ], 'price_' . $this->ids['PriceField']['membership'] => $this->ids['PriceFieldValue']['membership_general'], - 'other_amount' => 100, + 'price_' . $this->ids['PriceField']['contribution'] => $this->ids['PriceFieldValue']['contribution'], 'priceSetId' => $this->ids['PriceSet']['membership_block'], 'credit_card_type' => 'Visa', 'email-5' => 'test@test.com', -- 2.25.1