From: eileen <emcnaughton@wikimedia.org>
Date: Sun, 14 Mar 2021 03:29:50 +0000 (+1300)
Subject: [REF] Simplify code calculating the number of membership terms
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=e84c5dc202a37cf0d99af7e02ba472fa2da5388a;p=civicrm-core.git

[REF] Simplify code calculating the number of membership terms

We have 2 possibilities
1) there is a price set in use - the number of line items comes from the calculated line items
2) default priceset in use - it is a submitted value

The option to submit is only present in 2 so we know that if it is
submitted it should take precedence, otherwise it comes from the line item

This simplifies the code to do that clearly
---

diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php
index f1d0c771db..47085372b7 100644
--- a/CRM/Financial/BAO/Order.php
+++ b/CRM/Financial/BAO/Order.php
@@ -319,6 +319,27 @@ class CRM_Financial_BAO_Order {
     return $this->lineItems;
   }
 
+  /**
+   * Get line items that specifically relate to memberships.
+   *
+   * return array
+   *
+   * @throws \CiviCRM_API3_Exception
+   */
+  public function getMembershipLineItems():array {
+    $lines = $this->getLineItems();
+    foreach ($lines as $index => $line) {
+      if (empty($line['membership_type_id'])) {
+        unset($lines[$index]);
+        continue;
+      }
+      if (empty($line['membership_num_terms'])) {
+        $lines[$index]['membership_num_terms'] = 1;
+      }
+    }
+    return $lines;
+  }
+
   /**
    * @return array
    * @throws \CiviCRM_API3_Exception
diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php
index 778f2bc690..a734d8e23a 100644
--- a/CRM/Member/Form/Membership.php
+++ b/CRM/Member/Form/Membership.php
@@ -1055,16 +1055,6 @@ DESC limit 1");
 
     $params['tax_amount'] = $this->order->getTotalTaxAmount();
     $params['total_amount'] = $this->order->getTotalAmount();
-    if (!empty($lineItem[$this->_priceSetId])) {
-      foreach ($lineItem[$this->_priceSetId] as &$li) {
-        if (!empty($li['membership_type_id'])) {
-          if (!empty($li['membership_num_terms'])) {
-            $termsByType[$li['membership_type_id']] = $li['membership_num_terms'];
-          }
-        }
-      }
-    }
-
     $params['contact_id'] = $this->_contactID;
 
     $params = array_merge($params, $this->getFormMembershipParams());
@@ -1073,14 +1063,10 @@ DESC limit 1");
     $startDate = $formValues['start_date'];
     $endDate = $formValues['end_date'];
 
-    $memTypeNumTerms = empty($termsByType) ? CRM_Utils_Array::value('num_terms', $formValues) : NULL;
-
     $calcDates = [];
-    foreach ($this->_memTypeSelected as $memType) {
-      if (empty($memTypeNumTerms)) {
-        $memTypeNumTerms = CRM_Utils_Array::value($memType, $termsByType, 1);
-      }
-      $calcDates[$memType] = CRM_Member_BAO_MembershipType::getDatesForMembershipType($memType,
+    foreach ($this->order->getMembershipLineItems() as $membershipLineItem) {
+      $memTypeNumTerms = $this->getSubmittedValue('num_terms') ?: $membershipLineItem['membership_num_terms'];
+      $calcDates[$membershipLineItem['membership_type_id']] = CRM_Member_BAO_MembershipType::getDatesForMembershipType($membershipLineItem['membership_type_id'],
         $joinDate, $startDate, $endDate, $memTypeNumTerms
       );
     }
diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php
index d5e964e588..7f396984bb 100644
--- a/tests/phpunit/CRM/Member/Form/MembershipTest.php
+++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php
@@ -491,7 +491,6 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
     CRM_Core_Session::singleton()->getStatus(TRUE);
     $this->setCurrencySeparators($thousandSeparator);
     $form = $this->getForm();
-    $form->preProcess();
     $this->mut = new CiviMailUtils($this, TRUE);
     $form->_mode = 'test';
     $this->createLoggedInUser();
@@ -591,7 +590,6 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
   public function testContributionUpdateOnMembershipTypeChange(): void {
     // Step 1: Create a Membership via backoffice whose with 50.00 payment
     $form = $this->getForm();
-    $form->preProcess();
     $this->mut = new CiviMailUtils($this, TRUE);
     $this->createLoggedInUser();
     $priceSet = $this->callAPISuccess('PriceSet', 'Get', ["extends" => "CiviMember"]);