From 7365dd7f2e7bb3f3a5df42f224513a185d4feee4 Mon Sep 17 00:00:00 2001 From: Alok Patel Date: Mon, 29 Jul 2019 12:06:33 +1200 Subject: [PATCH] Fix membership end date on confirming a pending contribution https://github.com/civicrm/civicrm-core/pull/13706 --- CRM/Contribute/BAO/Contribution.php | 17 ++-- api/v3/Membership.php | 2 + .../CRM/Member/Form/MembershipTest.php | 78 +++++++++++++++++++ tests/phpunit/api/v3/ContributionPageTest.php | 2 + 4 files changed, 93 insertions(+), 6 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 31f424e14f..24e1d8ea11 100644 --- 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 af6ba7a0fc..5b06b0820f 100644 --- 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 ef3d5e05de..c42b52a06f 100644 --- 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" ', + '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 56a893409d..28a12cd926 100644 --- 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(); -- 2.25.1