Fix test to have correct input params
authorEileen McNaughton <emcnaughton@wikimedia.org>
Thu, 23 Nov 2023 00:22:35 +0000 (13:22 +1300)
committerEileen McNaughton <emcnaughton@wikimedia.org>
Thu, 23 Nov 2023 07:12:26 +0000 (20:12 +1300)
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
tests/phpunit/CRM/Contribute/Form/Contribution/ConfirmTest.php

index 3ea55fbdae5371ed95f656ad27c1e61011efd215..06a08375675b8812ea259dfeb2973b5976223d78 100644 (file)
@@ -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.
    *
index 97c3aca6065c15291262ed0466bc4759275cb28a..461163b46d74c9be776bc7298a8926261ac48254 100644 (file)
@@ -216,7 +216,7 @@ class CRM_Contribute_Form_Contribution_ConfirmTest extends CiviUnitTestCase {
       '<td style="padding: 4px; border-bottom: 1px solid #999; background-color: #f7f7f7;">
          Amount        </td>
         <td style="padding: 4px; border-bottom: 1px solid #999;">
-         $1,000.00         </td>
+         $352.00         </td>
        </tr>',
       '************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',