From 6723d96fccf66775c543e9a33a87017f8c44d2c0 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Thu, 23 Nov 2023 11:49:13 +1300
Subject: [PATCH] Fixes to functions to determine if isMembership,
 isSeparatePayment

In the case of isSeparatePayment we have a function to determine if the form
supports separate payments - but are using this interchangeably with whether the
user has selected more than one option (if they only selected 1 there is only 1 payment)
---
 CRM/Contribute/Form/Contribution/Confirm.php  | 23 ++++++-------------
 CRM/Contribute/Form/ContributionBase.php      | 12 +++++++++-
 tests/phpunit/api/v3/ContributionPageTest.php |  8 ++++---
 3 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php
index b84a3e6b27..3ea55fbdae 100644
--- a/CRM/Contribute/Form/Contribution/Confirm.php
+++ b/CRM/Contribute/Form/Contribution/Confirm.php
@@ -1407,7 +1407,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     $totalAmount = $membershipParams['amount'];
 
     if ($isPaidMembership) {
-      if ($isProcessSeparateMembershipTransaction) {
+      if ($this->isSeparatePaymentSelected()) {
         // If we have 2 transactions only one can use the invoice id.
         $membershipParams['invoiceID'] .= '-2';
         if (!empty($membershipParams['auto_renew'])) {
@@ -1441,7 +1441,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
       }
     }
 
-    if ($isProcessSeparateMembershipTransaction) {
+    if ($this->isSeparatePaymentSelected()) {
       try {
         $this->_lineItem = $unprocessedLineItems;
         if (empty($this->_params['auto_renew']) && !empty($membershipParams['is_recur'])) {
@@ -2201,7 +2201,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     $this->_useForMember = $this->get('useForMember');
 
     // store the fact that this is a membership and membership type is selected
-    if ($this->isMembershipSelected($membershipParams)) {
+    if ($this->isMembershipSelected()) {
       $this->doMembershipProcessing($contactID, $membershipParams, $premiumParams);
     }
     else {
@@ -2266,20 +2266,11 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
 
   /**
    * Return True/False if we have a membership selected on the contribution page
-   * @param array $membershipParams
    *
    * @return bool
    */
-  private function isMembershipSelected($membershipParams) {
-    $priceFieldIds = $this->get('memberPriceFieldIDS');
-    if ((!empty($membershipParams['selectMembership']) && $membershipParams['selectMembership'] != 'no_thanks')
-        && empty($priceFieldIds)) {
-      return TRUE;
-    }
-    else {
-      $membershipParams = $this->getMembershipParamsFromPriceSet($membershipParams);
-    }
-    return !empty($membershipParams['selectMembership']);
+  private function isMembershipSelected(): bool {
+    return !empty($this->getMembershipLineItems());
   }
 
   /**
@@ -2380,10 +2371,10 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
     }
 
     $membershipParams = $this->getMembershipParamsFromPriceSet($membershipParams);
-    if (!empty($membershipParams['selectMembership'])) {
+    if ($this->isMembershipSelected()) {
       // CRM-12233
       $membershipLineItems = [$this->getPriceSetID() => $this->getLineItems()];;
-      if ($this->_separateMembershipPayment && $this->isFormSupportsNonMembershipContributions()) {
+      if ($this->isSeparatePaymentSelected()) {
         $membershipLineItems = [];
         foreach ($this->_values['fee'] as $key => $feeValues) {
           if ($feeValues['name'] == 'membership_amount') {
diff --git a/CRM/Contribute/Form/ContributionBase.php b/CRM/Contribute/Form/ContributionBase.php
index cef3b2fe24..236cfde397 100644
--- a/CRM/Contribute/Form/ContributionBase.php
+++ b/CRM/Contribute/Form/ContributionBase.php
@@ -580,6 +580,16 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form {
     return $mainContributionLineItems;
   }
 
+  /**
+   * Is the form separate payment AND has the user selected 2 options,
+   * resulting in 2 payments.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  protected function isSeparatePaymentSelected(): bool {
+    return (bool) $this->getSecondaryMembershipContributionLineItems();
+  }
+
   /**
    * Set the line items for the secondary membership contribution.
    *
@@ -602,7 +612,7 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form {
         $lineItems[$index] = $lineItem;
       }
     }
-    if (empty($lineItems) || count($lineItems) === $this->getLineItems()) {
+    if (empty($lineItems) || count($lineItems) === count($this->getLineItems())) {
       return FALSE;
     }
     return $lineItems;
diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php
index c05b9f0774..c992a3a3b0 100644
--- a/tests/phpunit/api/v3/ContributionPageTest.php
+++ b/tests/phpunit/api/v3/ContributionPageTest.php
@@ -976,12 +976,14 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
   }
 
   /**
-   * Test non-recur contribution with membership payment
+   * Test non-recur contribution with membership payment selected.
+   *
+   * In this scenario the contribution option was not selected so only
+   * one contribution is actually created.
    *
    * @throws \CRM_Core_Exception
-   * @throws \Civi\API\Exception\UnauthorizedException
    */
-  public function testSubmitMembershipIsSeparatePaymentNotRecur(): void {
+  public function testSubmitMembershipIsSeparatePaymentNotRecurMembershipOnly(): void {
     $this->setUpMembershipContributionPage(TRUE, TRUE);
     $dummyPP = Civi\Payment\System::singleton()->getById($this->ids['PaymentProcessor']['dummy']);
     $dummyPP->setDoDirectPaymentResult(['payment_status_id' => 1, 'trxn_id' => 'create_first_success']);
-- 
2.25.1