Merge pull request #19801 from eileenmcnaughton/mem_terms

[REF] Simplify code calculating the number of membership terms
civicrm · Mar 24, 2021 · 218f91c · 218f91c
2 parents f056f83 + e84c5dc
commit 218f91c
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 19 deletions.
diff --git a/CRM/Financial/BAO/Order.php b/CRM/Financial/BAO/Order.php
@@ -319,6 +319,27 @@ public function getLineItems():array {
     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
@@ -1051,16 +1051,6 @@ public function submit(): void {
 
     $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());
@@ -1069,14 +1059,10 @@ public function submit(): void {
     $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
@@ -491,7 +491,6 @@ public function testSubmit(string $thousandSeparator) {
     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 @@ public function testSubmit(string $thousandSeparator) {
   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"]);