[REF] Determine values where they are needed rather than passing them around

The membershipTypes array is constructed, but barely used. One place it is used
is in this function - which is explictly tested by CRM_Member_Form_MembershipTest::testSubmit

This fixes it to get the values when needed rather than construct a variable miles away from
where it is used - no more pass the parcel

diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php
index 23e59d1dda4bf2b6e7d33ed4cbb907a1626f2dcf..f8e50a2e972e72ed5a000e2d7490740c463f9df6 100644 (file)
--- a/CRM/Member/Form/Membership.php
+++ b/CRM/Member/Form/Membership.php
@@ -1599,7 +1599,7 @@ class CRM_Member_Form_Membership extends CRM_Member_Form {
       $this->addStatusMessage($this->getStatusMessageForUpdate($membership, $endDate));
     }
     elseif (($this->_action & CRM_Core_Action::ADD)) {
-      $this->addStatusMessage($this->getStatusMessageForCreate($endDate, $membershipTypes, $createdMemberships,
+      $this->addStatusMessage($this->getStatusMessageForCreate($endDate, $createdMemberships,
         $isRecur, $calcDates));
     }
 
@@ -1817,35 +1817,34 @@ class CRM_Member_Form_Membership extends CRM_Member_Form {
    * Get status message for create action.
    *
    * @param string $endDate
-   * @param array $membershipTypes
    * @param array $createdMemberships
    * @param bool $isRecur
    * @param array $calcDates
    *
    * @return array|string
    */
-  protected function getStatusMessageForCreate($endDate, $membershipTypes, $createdMemberships,
+  protected function getStatusMessageForCreate($endDate, $createdMemberships,
                                                $isRecur, $calcDates) {
     // FIX ME: fix status messages
 
     $statusMsg = [];
-    foreach ($membershipTypes as $memType => $membershipType) {
-      $statusMsg[$memType] = ts('%1 membership for %2 has been added.', [
-        1 => $membershipType,
+    foreach ($this->_memTypeSelected as $membershipTypeID) {
+      $statusMsg[$membershipTypeID] = ts('%1 membership for %2 has been added.', [
+        1 => $this->allMembershipTypeDetails[$membershipTypeID]['name'],
         2 => $this->_memberDisplayName,
       ]);
 
-      $membership = $createdMemberships[$memType];
+      $membership = $createdMemberships[$membershipTypeID];
       $memEndDate = $membership->end_date ?: $endDate;
 
       //get the end date from calculated dates.
       if (!$memEndDate && !$isRecur) {
-        $memEndDate = $calcDates[$memType]['end_date'] ?? NULL;
+        $memEndDate = $calcDates[$membershipTypeID]['end_date'] ?? NULL;
       }
 
       if ($memEndDate && $memEndDate !== 'null') {
         $memEndDate = CRM_Utils_Date::formatDateOnlyLong($memEndDate);
-        $statusMsg[$memType] .= ' ' . ts('The new membership End Date is %1.', [1 => $memEndDate]);
+        $statusMsg[$membershipTypeID] .= ' ' . ts('The new membership End Date is %1.', [1 => $memEndDate]);
       }
     }
     $statusMsg = implode('<br/>', $statusMsg);

diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php
index 4c4de605d575f08a1a8ede06c3ae7e6488c28072..9f61916847e013f77690f234c7b77b656bd6a661 100644 (file)
--- a/tests/phpunit/CRM/Member/Form/MembershipTest.php
+++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php
@@ -474,11 +474,12 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
    *
    * @param string $thousandSeparator
    *
-   * @dataProvider getThousandSeparators
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
+   *
+   * @dataProvider getThousandSeparators
    */
-  public function testSubmit($thousandSeparator) {
+  public function testSubmit(string $thousandSeparator) {
     CRM_Core_Session::singleton()->getStatus(TRUE);
     $this->setCurrencySeparators($thousandSeparator);
     $form = $this->getForm();
@@ -508,8 +509,7 @@ class CRM_Member_Form_MembershipTest extends CiviUnitTestCase {
       'cvv2' => '123',
       'credit_card_exp_date' => [
         'M' => '9',
-        // TODO: Future proof
-        'Y' => '2024',
+        'Y' => date('Y', strtotime('+ 2 years')),
       ],
       'credit_card_type' => 'Visa',
       'billing_first_name' => 'Test',
@@ -1488,8 +1488,6 @@ Expires: ',
    */
   public function testCreatePendingWithMultipleTerms() {
     CRM_Core_Session::singleton()->getStatus(TRUE);
-    $form = $this->getForm();
-    $form->preProcess();
     $this->mut = new CiviMailUtils($this, TRUE);
     $this->createLoggedInUser();
     $membershipTypeAnnualRolling = $this->callAPISuccess('membership_type', 'create', [
@@ -1522,6 +1520,8 @@ Expires: ',
       'from_email_address' => '"Demonstrators Anonymous" <info@example.org>',
       'receipt_text' => '',
     ];
+    $form = $this->getForm();
+    $form->preProcess();
     $form->_contactID = $this->_individualId;
     $form->testSubmit($params);
     $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);