From 9392f61487ade8ccaed8c65f77672c81c75a5551 Mon Sep 17 00:00:00 2001 From: Matthew Wire Date: Fri, 19 Jan 2018 15:44:19 +0700 Subject: [PATCH 1/3] CRM-21682 Support multiple memberships for recurring --- CRM/Contribute/BAO/Contribution.php | 122 ++++++++++++---------------- CRM/Member/BAO/Membership.php | 6 +- 2 files changed, 56 insertions(+), 72 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index bd1d1d452ef5..6ad63b33da2a 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -3930,6 +3930,7 @@ public static function recordAdditionalPayment($contributionId, $trxnsData, $pay } } + // update membership details self::updateMembershipBasedOnCompletionOfContribution( $contributionDAO, $contributionId, @@ -5406,60 +5407,63 @@ protected static function isPaymentInstrumentChange(&$params, $pendingStatuses) * @param int $primaryContributionID * @param string $changeDate * - * @throws \CRM_Core_Exception - * @throws \CiviCRM_API3_Exception */ protected static function updateMembershipBasedOnCompletionOfContribution($contribution, $primaryContributionID, $changeDate) { - $contribution->loadRelatedMembershipObjects(); - if (empty($contribution->_relatedObjects['membership'])) { - return; + // Load memberships from contribution info. There may be multiple memberships (or none) + $membershipFields = array('id', 'contact_id', 'is_test', 'membership_type_id', 'contribution_recur_id', 'status_id.is_current_member', + 'start_date', 'join_date', 'end_date', 'status_id'); + $memberships = array(); + if (!empty($contribution->contribution_recur_id)) { + // Load memberships associated with recurring contribution + $membershipResult = civicrm_api3('Membership', 'get', array( + 'contribution_recur_id' => $contribution->contribution_recur_id, + 'return' => $membershipFields, + 'options' => ['limit' => 0], + )); + } + elseif (!empty($primaryContributionID)) { + // Load membership associated with original contribution + $membershipPaymentResult = civicrm_api3('MembershipPayment', 'get', array( + 'contribution_id' => $primaryContributionID, + 'options' => ['limit' => 0], + )); + if (!empty($membershipPaymentResult['count'])) { + foreach ($membershipPaymentResult['values'] as $payment) { + $membershipIDs[] = $payment['membership_id']; + } + $membershipResult = civicrm_api3('Membership', 'get', array( + 'id' => array('IN' => $membershipIDs), + 'return' => $membershipFields, + 'options' => ['limit' => 0], + )); + } + } + if (isset($membershipResult) && !empty($membershipResult['count'])) { + $memberships = $membershipResult['values']; } - $memberships = $contribution->_relatedObjects['membership']; - foreach ($memberships as $membershipTypeIdKey => $membership) { + + // Loop through all found memberships and update status/renew + foreach ($memberships as $membershipId => $membership) { if ($membership) { $membershipParams = array( - 'id' => $membership->id, - 'contact_id' => $membership->contact_id, - 'is_test' => $membership->is_test, - 'membership_type_id' => $membership->membership_type_id, + 'id' => $membership['id'], + 'contact_id' => $membership['contact_id'], + 'is_test' => $membership['is_test'], + 'membership_type_id' => $membership['membership_type_id'], 'membership_activity_status' => 'Completed', ); - $currentMembership = CRM_Member_BAO_Membership::getContactMembership($membershipParams['contact_id'], + $membershipParams['num_terms'] = $contribution->getNumTermsByContributionAndMembershipType( $membershipParams['membership_type_id'], - $membershipParams['is_test'], - $membershipParams['id'] + $primaryContributionID ); - // CRM-8141 update the membership type with the value recorded in log when membership created/renewed - // this picks up membership type changes during renewals - // @todo this is almost certainly an obsolete sql call, the pre-change - // membership is accessible via $this->_relatedObjects - $sql = " -SELECT membership_type_id -FROM civicrm_membership_log -WHERE membership_id={$membershipParams['id']} -ORDER BY id DESC -LIMIT 1;"; - $dao = CRM_Core_DAO::executeQuery($sql); - if ($dao->fetch()) { - if (!empty($dao->membership_type_id)) { - $membershipParams['membership_type_id'] = $dao->membership_type_id; - } - } - $dao->free(); - - $membershipParams['num_terms'] = $contribution->getNumTermsByContributionAndMembershipType( + // Does the contact have a "current" membership (ie. not expired/pending etc). + $currentMembership = CRM_Member_BAO_Membership::getContactMembership($membershipParams['contact_id'], $membershipParams['membership_type_id'], - $primaryContributionID + $membershipParams['is_test'], + $membershipParams['id'] ); - // @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(array( - 'join_date', - 'start_date', - 'end_date', - ), NULL); if ($currentMembership) { /* * Fixed FOR CRM-4433 @@ -5467,38 +5471,20 @@ protected static function updateMembershipBasedOnCompletionOfContribution($contr * when Contribution mode is notify and membership is for renewal ) */ CRM_Member_BAO_Membership::fixMembershipStatusBeforeRenew($currentMembership, $changeDate); - - // @todo - we should pass membership_type_id instead of null here but not - // adding as not sure of testing - $dates = CRM_Member_BAO_MembershipType::getRenewalDatesForMembershipType($membershipParams['id'], - $changeDate, NULL, $membershipParams['num_terms'] - ); - $dates['join_date'] = $currentMembership['join_date']; } - //get the status for membership. - $calcStatus = CRM_Member_BAO_MembershipStatus::getMembershipStatusByDate($dates['start_date'], - $dates['end_date'], - $dates['join_date'], - 'today', - TRUE, - $membershipParams['membership_type_id'], - $membershipParams - ); - - unset($dates['end_date']); - $membershipParams['status_id'] = CRM_Utils_Array::value('id', $calcStatus, 'New'); - //we might be renewing membership, - //so make status override false. + // Tell the Membership BAO to calculate membership status. + $membershipParams['skipStatusCal'] = 0; + $membershipParams['exclude_is_admin'] = TRUE; $membershipParams['is_override'] = FALSE; $membershipParams['status_override_end_date'] = 'null'; - - //CRM-17723 - reset static $relatedContactIds array() - // @todo move it to Civi Statics. - $var = TRUE; - CRM_Member_BAO_Membership::createRelatedMemberships($var, $var, TRUE); - civicrm_api3('Membership', 'create', $membershipParams); } + + //CRM-17723 - reset static $relatedContactIds array() + // @todo move it to Civi Statics. + $var = TRUE; + CRM_Member_BAO_Membership::createRelatedMemberships($var, $var, TRUE); + civicrm_api3('Membership', 'create', $membershipParams); } } diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php index 1826901429c0..6c22e2888fd6 100644 --- a/CRM/Member/BAO/Membership.php +++ b/CRM/Member/BAO/Membership.php @@ -1164,10 +1164,8 @@ public static function fixMembershipStatusBeforeRenew(&$currentMembership, $chan $currentMembership ); - if (empty($status) || - empty($status['id']) - ) { - CRM_Core_Error::fatal(ts('Oops, it looks like there is no valid membership status corresponding to the membership start and end dates for this membership. Contact the site administrator for assistance.')); + if (empty($status) || empty($status['id'])) { + throw new CRM_Core_Exception(ts('Oops, it looks like there is no valid membership status corresponding to the membership start and end dates for this membership. Contact the site administrator for assistance.')); } $currentMembership['today_date'] = $today; From fe39f1a6f04722f4d8f8be272d4f38c782eb4125 Mon Sep 17 00:00:00 2001 From: "Matthew Wire (MJW Consulting)" Date: Mon, 23 Jul 2018 20:58:17 +0100 Subject: [PATCH 2/3] CRM-21682 Don't allow auto-renew membership if frequencies do not match --- CRM/Contribute/BAO/Contribution.php | 6 ++++ CRM/Member/BAO/Membership.php | 28 +++++++++++++++++++ tests/phpunit/api/v3/ContributionPageTest.php | 6 ++-- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 6ad63b33da2a..afd33a8ed6aa 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -5445,6 +5445,12 @@ protected static function updateMembershipBasedOnCompletionOfContribution($contr // Loop through all found memberships and update status/renew foreach ($memberships as $membershipId => $membership) { if ($membership) { + if ((!empty($contribution->contribution_recur_id)) + && (!CRM_Member_BAO_Membership::isRecurFrequencyEqualToMembershipType($membership['membership_type_id'], $membership['contribution_recur_id']))) { + Civi::log()->warning('You have enabled auto-renew on membership (id=' . $membership['id'] . ') but the frequencies do not match! The membership will not be auto-renewed.'); + continue; + } + $membershipParams = array( 'id' => $membership['id'], 'contact_id' => $membership['contact_id'], diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php index 6c22e2888fd6..4c2d99966658 100644 --- a/CRM/Member/BAO/Membership.php +++ b/CRM/Member/BAO/Membership.php @@ -1705,6 +1705,34 @@ public static function isSubscriptionCancelled($mid) { return FALSE; } + /** + * Check if the membership type has the same frequency as the recurring contribution + * @param $membershipTypeId + * @param $contributionRecurId + * + * @return bool + */ + public static function isRecurFrequencyEqualToMembershipType($membershipTypeId, $contributionRecurId) { + try { + $membershipType = civicrm_api3('MembershipType', 'getsingle', array( + 'return' => array("duration_unit", "duration_interval"), + 'id' => $membershipTypeId, + )); + $contributionRecur = civicrm_api3('ContributionRecur', 'getsingle', array( + 'return' => array("frequency_unit", "frequency_interval"), + 'id' => $contributionRecurId, + )); + if (($membershipType['duration_unit'] === $contributionRecur['frequency_unit']) + && ($membershipType['duration_interval'] === $contributionRecur['frequency_interval'])) { + return TRUE; + } + } + catch (CiviCRM_API3_Exception $e) { + return FALSE; + } + return FALSE; + } + /** * Get membership joins for a specified membership type. * diff --git a/tests/phpunit/api/v3/ContributionPageTest.php b/tests/phpunit/api/v3/ContributionPageTest.php index 2d908fb363bb..001ff37e5850 100644 --- a/tests/phpunit/api/v3/ContributionPageTest.php +++ b/tests/phpunit/api/v3/ContributionPageTest.php @@ -838,9 +838,9 @@ public function testSubmitMembershipPriceSetPaymentPaymentProcessorRecurInstantP * - the first creates a new membership, completed contribution, in progress recurring. Check these * - create another - end date should be extended */ - //public function testSubmitMembershipPriceSetPaymentPaymentProcessorRecurInstantPaymentDifferentFrequency() { - // $this->doSubmitMembershipPriceSetPaymentPaymentProcessorRecurInstantPayment(array('duration_unit' => 'year', 'recur_frequency_unit' => 'month')); - //} + public function testSubmitMembershipPriceSetPaymentPaymentProcessorRecurInstantPaymentDifferentFrequency() { + $this->doSubmitMembershipPriceSetPaymentPaymentProcessorRecurInstantPayment(array('duration_unit' => 'year', 'recur_frequency_unit' => 'month')); + } /** * Helper function for testSubmitMembershipPriceSetPaymentProcessorRecurInstantPayment* From cde3f382981e6e908f168948ddfb3e46a8c0dbba Mon Sep 17 00:00:00 2001 From: "Matthew Wire (MJW Consulting)" Date: Mon, 23 Jul 2018 20:44:24 +0100 Subject: [PATCH 3/3] Drop unnecessary call to getContactMembership --- CRM/Contribute/BAO/Contribution.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index afd33a8ed6aa..987d2f2e7a80 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -5465,18 +5465,14 @@ protected static function updateMembershipBasedOnCompletionOfContribution($contr ); // Does the contact have a "current" membership (ie. not expired/pending etc). - $currentMembership = CRM_Member_BAO_Membership::getContactMembership($membershipParams['contact_id'], - $membershipParams['membership_type_id'], - $membershipParams['is_test'], - $membershipParams['id'] - ); - if ($currentMembership) { + if (!empty($membership['status_id.is_current_member'])) { /* * Fixed FOR CRM-4433 * In BAO/Membership.php(renewMembership function), we skip the extend membership date and status * when Contribution mode is notify and membership is for renewal ) + * FIXME: Is this still required? */ - CRM_Member_BAO_Membership::fixMembershipStatusBeforeRenew($currentMembership, $changeDate); + CRM_Member_BAO_Membership::fixMembershipStatusBeforeRenew($membership, $changeDate); } // Tell the Membership BAO to calculate membership status.