From: Matthew Wire <devel@mrwire.co.uk>
Date: Fri, 15 Jun 2018 08:07:52 +0000 (+0100)
Subject: Only auto-renew membership when contribution status is completed
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=185a4fe8c0ef7b64eb6fc739428ba8f9c8ebaf1e;p=civicrm-core.git

Only auto-renew membership when contribution status is completed
---

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 6d8f3de6dd..f428d685e3 100644
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -4618,12 +4618,13 @@ WHERE eft.financial_trxn_id IN ({$trxnId}, {$baseTrxnId['financialTrxnId']})
         $values['is_email_receipt'] = $recurContrib->is_email_receipt;
       }
 
-      self::updateMembershipBasedOnCompletionOfContribution(
-        $contribution,
-        $primaryContributionID,
-        $changeDate,
-        CRM_Core_PseudoConstant::getLabel('CRM_Contribute_BAO_Contribution', 'contribution_status_id', CRM_Utils_Array::value('contribution_status_id', $input))
-      );
+      if ($contributionParams['contribution_status_id'] === $completedContributionStatusID) {
+        self::updateMembershipBasedOnCompletionOfContribution(
+          $contribution,
+          $primaryContributionID,
+          $changeDate
+        );
+      }
     }
     else {
       if (empty($input['IAmAHorribleNastyBeyondExcusableHackInTheCRMEventFORMTaskClassThatNeedsToBERemoved'])) {
@@ -5388,12 +5389,11 @@ LEFT JOIN  civicrm_contribution on (civicrm_contribution.contact_id = civicrm_co
    * @param \CRM_Contribute_BAO_Contribution $contribution
    * @param int $primaryContributionID
    * @param string $changeDate
-   * @param string $contributionStatus
-   *   This shouldn't be required but historical function overload by repeattransaction probably requires it.
    *
-   * @todo investigate completely bypassing this function if $contributionStatus != Completed.
+   * @throws \CRM_Core_Exception
+   * @throws \CiviCRM_API3_Exception
    */
-  protected static function updateMembershipBasedOnCompletionOfContribution($contribution, $primaryContributionID, $changeDate, $contributionStatus = 'Completed') {
+  protected static function updateMembershipBasedOnCompletionOfContribution($contribution, $primaryContributionID, $changeDate) {
     $contribution->loadRelatedMembershipObjects();
     if (empty($contribution->_relatedObjects['membership'])) {
       return;
@@ -5433,55 +5433,50 @@ LIMIT 1;";
         }
         $dao->free();
 
-        // Unclear why this is here but this function is overloaded by repeattransaction.
-        if ($contributionStatus === 'Pending') {
-          $membershipParams['num_terms'] = 0;
-        }
-        else {
-          $membershipParams['num_terms'] = $contribution->getNumTermsByContributionAndMembershipType(
-            $membershipParams['membership_type_id'],
-            $primaryContributionID
+        $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(array(
+          'join_date',
+          'start_date',
+          'end_date',
+        ), NULL);
+        if ($currentMembership) {
+          /*
+           * 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 )
+           */
+          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']
           );
-          // @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
-             * In BAO/Membership.php(renewMembership function), we skip the extend membership date and status
-             * 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'];
-          }
+          $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
-          );
+        //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.
+        $membershipParams['is_override'] = FALSE;
+        $membershipParams['status_override_end_date'] = 'null';
 
-          unset($dates['end_date']);
-          $membershipParams['status_id'] = CRM_Utils_Array::value('id', $calcStatus, 'New');
-          //we might be renewing membership,
-          //so make status override false.
-          $membershipParams['is_override'] = FALSE;
-          $membershipParams['status_override_end_date'] = 'null';
-        }
         //CRM-17723 - reset static $relatedContactIds array()
         // @todo move it to Civi Statics.
         $var = TRUE;