From ebf7e65fce3391f3be40ed0eae0882cb533e2740 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Wed, 21 Oct 2020 23:51:51 +1300
Subject: [PATCH] Extract function to retrieve the membership labels.

Rather than construct an array (membershipTypes) early on with a db lookup
and pass it around we retrieve the labels when needed using a helper function
to retrieve the values from the already-available array on membership type
details
---
 CRM/Member/Form/Membership.php                | 30 +++++++++++--------
 .../CRM/Member/Form/MembershipTest.php        |  5 ++--
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php
index b5846d2177..84c5afd08a 100644
--- a/CRM/Member/Form/Membership.php
+++ b/CRM/Member/Form/Membership.php
@@ -610,9 +610,7 @@ DESC limit 1");
 
     // Retrieve the name and email of the contact - this will be the TO for receipt email
     if ($this->_contactID) {
-      list($this->_memberDisplayName,
-        $this->_memberEmail
-        ) = CRM_Contact_BAO_Contact_Location::getEmailDetails($this->_contactID);
+      [$this->_memberDisplayName, $this->_memberEmail] = CRM_Contact_BAO_Contact_Location::getEmailDetails($this->_contactID);
 
       $this->assign('emailExists', $this->_memberEmail);
       $this->assign('displayName', $this->_memberDisplayName);
@@ -1030,8 +1028,8 @@ DESC limit 1");
     $this->storeContactFields($this->_params);
     $this->beginPostProcess();
     $endDate = NULL;
-    $membershipTypes = $membership = $calcDate = [];
-    $membershipType = NULL;
+    $membership = $calcDate = [];
+
     $paymentInstrumentID = $this->_paymentProcessor['object']->getPaymentInstrumentID();
     $params = $softParams = $ids = [];
 
@@ -1184,13 +1182,8 @@ DESC limit 1");
         $this->_id,
         'Membership'
       );
-      $membershipTypes[$memType] = CRM_Core_DAO::getFieldValue('CRM_Member_DAO_MembershipType',
-        $memType
-      );
     }
 
-    $membershipType = implode(', ', $membershipTypes);
-
     // Retrieve the name and email of the current user - this will be the FROM for the receipt email
     list($userName) = CRM_Contact_BAO_Contact_Location::getEmailDetails(CRM_Core_Session::getLoggedInContactID());
 
@@ -1225,7 +1218,7 @@ DESC limit 1");
 
       if (empty($formValues['source'])) {
         $params['contribution_source'] = ts('%1 Membership: Offline signup (by %2)', [
-          1 => $membershipType,
+          1 => $this->getSelectedMembershipLabels(),
           2 => $userName,
         ]);
       }
@@ -1402,7 +1395,7 @@ DESC limit 1");
       $params['receive_date'] = date('Y-m-d H:i:s');
       $params['invoice_id'] = $formValues['invoiceID'];
       $params['contribution_source'] = ts('%1 Membership Signup: Credit card or direct debit (by %2)',
-        [1 => $membershipType, 2 => $userName]
+        [1 => $this->getSelectedMembershipLabels(), 2 => $userName]
       );
       $params['source'] = $formValues['source'] ?: $params['contribution_source'];
       $params['trxn_id'] = $result['trxn_id'] ?? NULL;
@@ -1844,4 +1837,17 @@ DESC limit 1");
     return $customValues;
   }
 
+  /**
+   * Get the selected memberships as a string of labels.
+   *
+   * @return string
+   */
+  protected function getSelectedMembershipLabels(): string {
+    $return = [];
+    foreach ($this->_memTypeSelected as $membershipTypeID) {
+      $return[] = $this->allMembershipTypeDetails[$membershipTypeID]['name'];
+    }
+    return implode(', ', $return);
+  }
+
 }
diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php
index 18ece33ba5..a70434be69 100644
--- a/tests/phpunit/CRM/Member/Form/MembershipTest.php
+++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php
@@ -676,12 +676,11 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
    * @param string $thousandSeparator
    *   punctuation used to refer to thousands.
    *
-   * @dataProvider getThousandSeparators
-   *
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
+   * @dataProvider getThousandSeparators
    */
-  public function testSubmitPartialPayment($thousandSeparator) {
+  public function testSubmitPartialPayment(string $thousandSeparator): void {
     $this->setCurrencySeparators($thousandSeparator);
     // Step 1: submit a partial payment for a membership via backoffice
     $form = $this->getForm();
-- 
2.25.1