[REF] Code cleanup on membership renewal & test

This wound up being mainly a clean up of the test class & the addition of a test on changing
membership type.

However, there is a minor code consolidation which makes 3 actual changes
1) if we are doing a test renewal we shouldn't make the membership test
2) even when renewing a Pending membership we should save any submitted custom data
3) modified id is also set for pending (although future test work might indicate this can go

diff --git a/CRM/Member/Form/MembershipRenewal.php b/CRM/Member/Form/MembershipRenewal.php
index 162ca1cc613f7d303814d6f77f9aa5c6aad2eb11..e7ea29422939abe4653f287f6e2c04da75a7f24f 100644 (file)
--- a/CRM/Member/Form/MembershipRenewal.php
+++ b/CRM/Member/Form/MembershipRenewal.php
@@ -750,7 +750,19 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form {
     $allStatus = CRM_Member_PseudoConstant::membershipStatus();
     $ids = [];
     $currentMembership = civicrm_api3('Membership', 'getsingle', ['id' => $membershipID]);
-
+    $memParams = [
+      'id' => $currentMembership['id'],
+      'membership_type_id' => $membershipTypeID,
+      'join_date' => $currentMembership['join_date'],
+      'modified_id' => $contactID,
+      'custom' => $customFieldsFormatted,
+      'membership_activity_status' => ($pending || $isPayLater) ? 'Scheduled' : 'Completed',
+      // Since we are renewing, make status override false.
+      'is_override' => FALSE,
+    ];
+    if ($contributionRecurID) {
+      $memParams['contribution_recur_id'] = $contributionRecurID;
+    }
     // Do NOT do anything.
     //1. membership with status : PENDING/CANCELLED (CRM-2395)
     //2. Paylater/IPN renew. CRM-4556.
@@ -759,18 +771,11 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form {
       // CRM-15475
       array_search('Cancelled', CRM_Member_PseudoConstant::membershipStatus(NULL, " name = 'Cancelled' ", 'name', FALSE, TRUE)),
     ])) {
-      $memParams = [
-        'id' => $currentMembership['id'],
+      $memParams = array_merge($memParams, [
         'status_id' => $currentMembership['status_id'],
         'start_date' => $currentMembership['start_date'],
         'end_date' => $currentMembership['end_date'],
-        'join_date' => $currentMembership['join_date'],
-        'membership_type_id' => $membershipTypeID,
-        'membership_activity_status' => ($pending || $isPayLater) ? 'Scheduled' : 'Completed',
-      ];
-      if ($contributionRecurID) {
-        $memParams['contribution_recur_id'] = $contributionRecurID;
-      }
+      ]);
       return CRM_Member_BAO_Membership::create($memParams);
     }
 
@@ -785,18 +790,11 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form {
       $membershipTypeID,
       $numRenewTerms
     );
-    $memParams = [
-      'membership_type_id' => $membershipTypeID,
+    $memParams = array_merge($memParams, [
       'end_date' => $dates['end_date'] ?? NULL,
-      'join_date' => $currentMembership['join_date'],
       'start_date' => $isMembershipCurrent ? $currentMembership['start_date'] : ($dates['start_date'] ?? NULL),
-      'id' => $currentMembership['id'],
-      'is_test' => $is_test,
-      // Since we are renewing, make status override false.
-      'is_override' => FALSE,
-      'modified_id' => $contactID,
       'log_start_date' => $dates['log_start_date'],
-    ];
+    ]);
 
     // Now Renew the membership
     if ($isMembershipCurrent) {
@@ -804,16 +802,8 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form {
       if (!empty($currentMembership['id'])) {
         $ids['membership'] = $currentMembership['id'];
       }
-      $memParams['membership_activity_status'] = ($pending || $isPayLater) ? 'Scheduled' : 'Completed';
-    }
-
-    // Putting this in an IF is precautionary as it seems likely that it would be ignored if empty, but
-    // perhaps shouldn't be?
-    if ($contributionRecurID) {
-      $memParams['contribution_recur_id'] = $contributionRecurID;
     }
 
-    $memParams['custom'] = $customFieldsFormatted;
     // @todo stop passing $ids (membership and userId may be set by this point)
     $membership = CRM_Member_BAO_Membership::create($memParams, $ids);
 

diff --git a/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php b/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php
index 1b8fe6b3ce3e33d071f2f5f9cac52ce958364fa1..f13fdf4c52916636332802b1288e9a381fc6f3e4 100644 (file)
--- a/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php
+++ b/tests/phpunit/CRM/Member/Form/MembershipRenewalTest.php
@@ -77,12 +77,11 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
     $this->_individualId = $this->individualCreate();
     $this->_paymentProcessorID = $this->processorCreate();
     $this->financialTypeID = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Member Dues');
-
-    $this->loadXMLDataSet(dirname(__FILE__) . '/dataset/data.xml');
+    $this->ids['contact']['organization'] = $this->organizationCreate();
     $this->membershipTypeAnnualFixedID = $this->callAPISuccess('membership_type', 'create', [
       'domain_id' => 1,
       'name' => 'AnnualFixed',
-      'member_of_contact_id' => 23,
+      'member_of_contact_id' => $this->ids['contact']['organization'],
       'duration_unit' => 'year',
       'duration_interval' => 1,
       'period_type' => 'fixed',
@@ -116,16 +115,13 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
     $this->quickCleanup(
       [
         'civicrm_relationship',
-        'civicrm_membership_type',
-        'civicrm_membership',
         'civicrm_uf_match',
         'civicrm_address',
       ]
     );
-    foreach ([17, 18, 23, 32] as $contactID) {
+    foreach ($this->ids['contact'] as $contactID) {
       $this->callAPISuccess('contact', 'delete', ['id' => $contactID, 'skip_undelete' => TRUE]);
     }
-    $this->callAPISuccess('relationship_type', 'delete', ['id' => 20]);
   }
 
   /**
@@ -137,43 +133,10 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
   public function testSubmit() {
     $form = $this->getForm();
     $this->createLoggedInUser();
-    $params = [
-      'cid' => $this->_individualId,
-      'join_date' => date('m/d/Y', time()),
-      'start_date' => '',
-      'end_date' => '',
-      // This format reflects the 23 being the organisation & the 25 being the type.
-      'membership_type_id' => [23, $this->membershipTypeAnnualFixedID],
-      'auto_renew' => '0',
-      'num_terms' => '1',
-      'source' => '',
-      'total_amount' => '50.00',
-      //Member dues, see data.xml
-      'financial_type_id' => '2',
-      'soft_credit_type_id' => '',
-      'soft_credit_contact_id' => '',
-      'from_email_address' => '"Demonstrators Anonymous" <info@example.org>',
-      'receipt_text_signup' => 'Thank you text',
-      'payment_processor_id' => $this->_paymentProcessorID,
-      'credit_card_number' => '4111111111111111',
-      'cvv2' => '123',
-      'credit_card_exp_date' => [
-        'M' => '9',
-        // TODO: Future proof
-        'Y' => '2024',
-      ],
-      'credit_card_type' => 'Visa',
-      'billing_first_name' => 'Test',
-      'billing_middlename' => 'Last',
-      'billing_street_address-5' => '10 Test St',
-      'billing_city-5' => 'Test',
-      'billing_state_province_id-5' => '1003',
-      'billing_postal_code-5' => '90210',
-      'billing_country_id-5' => '1228',
-    ];
+    $params = $this->getBaseSubmitParams();
     $form->_contactID = $this->_individualId;
 
-    $form->testSubmit($params);
+    $form->testSubmit(array_merge($params, ['total_amount' => 50]));
     $form->setRenewalMessage();
     $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
     $this->callAPISuccessGetCount('ContributionRecur', ['contact_id' => $this->_individualId], 0);
@@ -216,42 +179,41 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
     $this->enableTaxAndInvoicing();
     $this->relationForFinancialTypeWithFinancialAccount($this->financialTypeID);
     $form = $this->getForm();
-
-    $form->testSubmit([
-      'cid' => $this->_individualId,
-      'join_date' => date('Y-m-d'),
-      'start_date' => '',
-      'end_date' => '',
-      // This format reflects the 23 being the organisation & the 25 being the type.
-      'membership_type_id' => [23, $this->membershipTypeAnnualFixedID],
-      'auto_renew' => '0',
-      'num_terms' => '1',
-      'source' => '',
+    $form->testSubmit(array_merge($this->getBaseSubmitParams(), [
       'total_amount' => '50.00',
-      'financial_type_id' => $this->financialTypeID,
-      'from_email_address' => '"Demonstrators Anonymous" <info@example.org>',
-      'receipt_text_signup' => 'Thank you text',
-      'payment_processor_id' => $this->_paymentProcessorID,
-      'credit_card_number' => '4111111111111111',
-      'cvv2' => '123',
-      'credit_card_exp_date' => [
-        'M' => '9',
-        'Y' => date('Y') + 2,
-      ],
-      'credit_card_type' => 'Visa',
-      'billing_first_name' => 'Test',
-      'billing_middle_name' => 'Last',
-      'billing_street_address-5' => '10 Test St',
-      'billing_city-5' => 'Test',
-      'billing_state_province_id-5' => '1003',
-      'billing_postal_code-5' => '90210',
-      'billing_country_id-5' => '1228',
-    ]);
+    ]));
     $contribution = $this->callAPISuccessGetSingle('Contribution', ['contact_id' => $this->_individualId, 'is_test' => TRUE, 'return' => ['total_amount', 'tax_amount']]);
     $this->assertEquals(50, $contribution['total_amount']);
     $this->assertEquals(4.55, $contribution['tax_amount']);
   }
 
+  /**
+   * Test the submit function of the membership form.
+   *
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
+   */
+  public function testSubmitChangeType() {
+    $form = $this->getForm();
+    $this->createLoggedInUser();
+    $membershipBefore = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
+    $newMembershipTypeID = $this->callAPISuccess('MembershipType', 'create', [
+      'name' => 'Monthly',
+      'member_of_contact_id' => $this->ids['contact']['organization'],
+      'financial_type_id' => $this->financialTypeID,
+      'duration_unit' => 'month',
+      'duration_interval' => 2,
+      'period_type' => 'rolling',
+    ])['id'];
+    $form->_contactID = $this->_individualId;
+
+    $form->testSubmit(array_merge($this->getBaseSubmitParams(), ['membership_type_id' => [$this->ids['contact']['organization'], $newMembershipTypeID]]));
+    $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
+    $this->assertEquals($newMembershipTypeID, $membership['membership_type_id']);
+    // The date (31 Dec this year) should be progressed by 2 months to 28 Dec next year.
+    $this->assertEquals(date('Y', strtotime($membershipBefore['end_date'])) + 1 . '-02-28', $membership['end_date']);
+  }
+
   /**
    * Test the submit function of the membership form.
    *
@@ -277,7 +239,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'end_date' => '',
       'campaign_id' => '',
       // This format reflects the 23 being the organisation & the 25 being the type.
-      'membership_type_id' => [23, $this->membershipTypeAnnualFixedID],
+      'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID],
       'auto_renew' => '1',
       'is_recur' => 1,
       'num_terms' => '1',
@@ -285,8 +247,6 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'total_amount' => '77.00',
       //Member dues, see data.xml
       'financial_type_id' => '2',
-      'soft_credit_type_id' => 11,
-      'soft_credit_contact_id' => '',
       'from_email_address' => '"Demonstrators Anonymous" <info@example.org>',
       'receipt_text' => 'Thank you text',
       'payment_processor_id' => $this->_paymentProcessorID,
@@ -373,7 +333,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
     $form->preProcess();
 
     $form->_contactID = $this->_individualId;
-    $params = $this->getBaseSubmitParams();
+    $params = array_merge($this->getBaseSubmitParams(), ['is_recur' => 1, 'auto_renew' => '1']);
     $form->_mode = 'test';
 
     $form->testSubmit($params);
@@ -459,11 +419,9 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
     $form->preProcess();
 
     $form->_contactID = $this->_individualId;
-    $params = $this->getBaseSubmitParams();
-    $params['send_receipt'] = 1;
     $form->_mode = 'test';
 
-    $form->testSubmit($params);
+    $form->testSubmit(array_merge($this->getBaseSubmitParams(), ['is_recur' => 1, 'send_receipt' => 1, 'auto_renew' => 1]));
     $contributionRecur = $this->callAPISuccessGetSingle('ContributionRecur', ['contact_id' => $this->_individualId]);
     $this->assertEquals(1, $contributionRecur['is_email_receipt']);
     $this->mut->checkMailLog([
@@ -489,14 +447,12 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'start_date' => '',
       'end_date' => '',
       // This format reflects the 23 being the organisation & the 25 being the type.
-      'membership_type_id' => [23, $this->membershipTypeAnnualFixedID],
+      'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID],
       'auto_renew' => '0',
       'num_terms' => '2',
       'total_amount' => '50.00',
       //Member dues, see data.xml
       'financial_type_id' => '2',
-      'soft_credit_type_id' => '',
-      'soft_credit_contact_id' => '',
       'payment_instrument_id' => 4,
       'from_email_address' => '"Demonstrators Anonymous" <info@example.org>',
       'receipt_text_signup' => 'Thank you text',
@@ -537,18 +493,15 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
     $originalMembership = $this->callAPISuccessGetSingle('membership', []);
     $params = [
       'cid' => $this->_individualId,
-      'join_date' => date('m/d/Y', time()),
       'start_date' => '',
       'end_date' => '',
-      // This format reflects the 23 being the organisation & the 25 being the type.
-      'membership_type_id' => [23, $this->membershipTypeAnnualFixedID],
+      // This format reflects the first value being the organisation & the second being the type.
+      'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID],
       'auto_renew' => '0',
       'num_terms' => '2',
       'total_amount' => '50.00',
       //Member dues, see data.xml
       'financial_type_id' => '2',
-      'soft_credit_type_id' => '',
-      'soft_credit_contact_id' => '',
       'payment_instrument_id' => 4,
       'from_email_address' => '"Demonstrators Anonymous" <info@example.org>',
       'receipt_text_signup' => 'Thank you text',
@@ -605,14 +558,12 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
       'start_date' => '',
       'end_date' => '',
       // This format reflects the 23 being the organisation & the 25 being the type.
-      'membership_type_id' => [23, $this->membershipTypeAnnualFixedID],
+      'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID],
       'auto_renew' => '0',
       'num_terms' => '2',
       'total_amount' => '50.00',
       //Member dues, see data.xml
       'financial_type_id' => '2',
-      'soft_credit_type_id' => '',
-      'soft_credit_contact_id' => '',
       'payment_instrument_id' => 4,
       'from_email_address' => '"Demonstrators Anonymous" <info@example.org>',
       'receipt_text_signup' => 'Thank you text',
@@ -673,21 +624,12 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
   protected function getBaseSubmitParams() {
     return [
       'cid' => $this->_individualId,
-      'price_set_id' => 0,
-      'join_date' => date('Y-m-d'),
-      'start_date' => '',
-      'end_date' => '',
-      'campaign_id' => '',
-      // This format reflects the 23 being the organisation & the 25 being the type.
-      'membership_type_id' => [23, $this->membershipTypeAnnualFixedID],
-      'auto_renew' => '1',
-      'is_recur' => 1,
+      // This format reflects the key being the organisation & the value being the type.
+      'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID],
       'num_terms' => '1',
       'total_amount' => $this->formatMoneyInput('7800.90'),
       //Member dues, see data.xml
       'financial_type_id' => '2',
-      'soft_credit_type_id' => 11,
-      'soft_credit_contact_id' => '',
       'from_email_address' => '"Demonstrators Anonymous" <info@example.org>',
       'receipt_text' => 'Thank you text',
       'payment_processor_id' => $this->_paymentProcessorID,
@@ -729,7 +671,7 @@ class CRM_Member_Form_MembershipRenewalTest extends CiviUnitTestCase {
 
     $params = [
       'contact_id' => $this->_individualId,
-      'membership_type_id' => [23, $this->membershipTypeAnnualFixedID],
+      'membership_type_id' => [$this->ids['contact']['organization'], $this->membershipTypeAnnualFixedID],
       'renewal_date' => '2020-06-10',
       'financial_type_id' => '2',
       'num_terms' => '1',