Fix membership end date on confirming a pending contribution

https://github.com/civicrm/civicrm-core/pull/13706

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 31f424e14f6bb977a9a495f41453f0019fbece72..24e1d8ea11d60ce4218f140fb48d5a92f51bbbf5 100644 (file)
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -992,7 +992,7 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
     // as they would then me membership.contact_id, membership.is_test etc
     return civicrm_api3('Membership', 'get', [
       'id' => ['IN' => $membershipIDs],
-      'return' => ['id', 'contact_id', 'membership_type_id', 'is_test'],
+      'return' => ['id', 'contact_id', 'membership_type_id', 'is_test', 'status_id', 'end_date'],
     ])['values'];
   }
 
@@ -5412,11 +5412,16 @@ LIMIT 1;";
           $membershipParams['membership_type_id'] = $dao->membership_type_id;
         }
       }
-
-      $membershipParams['num_terms'] = $contribution->getNumTermsByContributionAndMembershipType(
-        $membershipParams['membership_type_id'],
-        $primaryContributionID
-      );
+      if (empty($membership['end_date']) || (int) $membership['status_id'] !== CRM_Core_PseudoConstant::getKey('CRM_Member_BAO_Membership', 'status_id', 'Pending')) {
+        // Passing num_terms to the api triggers date calculations, but for pending memberships these may be already calculated.
+        // sigh - they should  be  consistent but removing the end date check causes test failures & maybe UI too?
+        // The api assumes num_terms is a special sauce for 'is_renewal' so we need to not pass it when updating a pending to completed.
+        // @todo once apiv4 ships with core switch to that & find sanity.
+        $membershipParams['num_terms'] = $contribution->getNumTermsByContributionAndMembershipType(
+          $membershipParams['membership_type_id'],
+          $primaryContributionID
+        );
+      }
       // @todo remove all this stuff in favour of letting the api call further down handle in
       // (it is a duplication of what the api does).
       $dates = array_fill_keys([

diff --git a/api/v3/Membership.php b/api/v3/Membership.php
index af6ba7a0fcd37708e6846906bdc6b5a2ac0fe3a1..5b06b0820f0ae00dd6ee2b89d434e7615dddf40c 100644 (file)
--- a/api/v3/Membership.php
+++ b/api/v3/Membership.php
@@ -117,6 +117,8 @@ function civicrm_api3_membership_create($params) {
     }
     else {
       // This is an existing membership, calculate the membership dates after renewal
+      // num_terms is treated as a 'special sauce' for is_renewal but this
+      // isn't really helpful for completing pendings.
       $calcDates = CRM_Member_BAO_MembershipType::getRenewalDatesForMembershipType(
         $params['id'],
         NULL,

diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php
index ef3d5e05de8955918b3aadb7ee34e7593bb94e28..c42b52a06f5d536629a4e5f7b41b8ada5beb5f99 100644 (file)
--- a/tests/phpunit/CRM/Member/Form/MembershipTest.php
+++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php
@@ -1372,4 +1372,82 @@ Expires: ',
     $this->disableTaxAndInvoicing();
   }
 
+  /**
+   * Test that membership end_date is correct for multiple terms for pending contribution
+   *
+   * @throws CiviCRM_API3_Exception
+   * @throws \CRM_Core_Exception
+   */
+  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', [
+      'domain_id' => 1,
+      'name' => "AnnualRollingNew",
+      'member_of_contact_id' => 23,
+      'duration_unit' => "year",
+      'minimum_fee' => 50,
+      'duration_interval' => 1,
+      'period_type' => "rolling",
+      'relationship_type_id' => 20,
+      'relationship_direction' => 'b_a',
+      'financial_type_id' => 2,
+    ]);
+    $params = [
+      'cid' => $this->_individualId,
+      'join_date' => date('Y-m-d'),
+      'start_date' => '',
+      'end_date' => '',
+      'contribution_status_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Pending'),
+      'membership_type_id' => [23, $membershipTypeAnnualRolling['id']],
+      'max_related' => '',
+      'num_terms' => '3',
+      'record_contribution' => 1,
+      'source' => '',
+      'total_amount' => $this->formatMoneyInput(150.00),
+      'financial_type_id' => '2',
+      'soft_credit_type_id' => '11',
+      'soft_credit_contact_id' => '',
+      'from_email_address' => '"Demonstrators Anonymous" <info@example.org>',
+      'receipt_text' => '',
+    ];
+    $form->_contactID = $this->_individualId;
+    $form->testSubmit($params);
+    $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
+    $contribution = $this->callAPISuccess('Contribution', 'get', [
+      'contact_id' => $this->_individualId,
+    ]);
+    $endDate = (new DateTime(date('Y-m-d')))->modify("+3 years")->modify("-1 day");
+    $endDate = $endDate->format("Y-m-d");
+
+    $this->assertEquals($endDate, $membership['end_date'], 'Membership end date should be ' . $endDate);
+    $this->assertEquals(1, count($contribution['values']), 'Pending contribution should be created.');
+    $contribution = $contribution['values'][$contribution['id']];
+    $additionalPaymentForm = new CRM_Contribute_Form_AdditionalPayment();
+    $additionalPaymentForm->testSubmit([
+      'total_amount' => 150.00,
+      'trxn_date' => date("Y-m-d H:i:s"),
+      'payment_instrument_id' => array_search('Check', $this->paymentInstruments),
+      'check_number' => 'check-12345',
+      'trxn_id' => '',
+      'currency' => 'USD',
+      'fee_amount' => '',
+      'financial_type_id' => 1,
+      'net_amount' => '',
+      'payment_processor_id' => 0,
+      'contact_id' => $this->_individualId,
+      'contribution_id' => $contribution['id'],
+    ]);
+    $membership = $this->callAPISuccessGetSingle('Membership', ['contact_id' => $this->_individualId]);
+    $contribution = $this->callAPISuccess('Contribution', 'get', [
+      'contact_id' => $this->_individualId,
+      'contribution_status_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed'),
+    ]);
+    $this->assertEquals($endDate, $membership['end_date'], 'Membership end date should be same (' . $endDate . ') after payment');
+    $this->assertEquals(1, count($contribution['values']), 'Completed contribution should be fetched.');
+  }
+
 }

diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php
index 56a893409d02df14c8d1ebc935cd0e823cbec76b..28a12cd926a7ef0db94d813854b216f2b9372d0b 100644 (file)
--- a/tests/phpunit/api/v3/ContributionPageTest.php
+++ b/tests/phpunit/api/v3/ContributionPageTest.php
@@ -346,6 +346,8 @@ class api_v3_ContributionPageTest extends CiviUnitTestCase {
 
   /**
    * Test submit with a membership block in place works with renewal.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testSubmitMembershipBlockNotSeparatePaymentProcessorInstantRenew() {
     $this->setUpMembershipContributionPage();