CRM-16417 fully remove renewMembershipFormWrapper

This function was a temporary refactoring function as part of moving the interaction
with the form layer back to the form layer & is not fully gone

diff --git a/CRM/Batch/Form/Entry.php b/CRM/Batch/Form/Entry.php
index a9e24bcd82bf66292c7c7dad24fea0d177bd0d53..9baf89a4fd65cfbf1fbbb738fb050d2782c01507 100755 (executable)
--- a/CRM/Batch/Form/Entry.php
+++ b/CRM/Batch/Form/Entry.php
@@ -64,7 +64,6 @@ class CRM_Batch_Form_Entry extends CRM_Core_Form {
 
   public $_params;
 
-  public $_membershipId = NULL;
   /**
    * When not to reset sort_name.
    */
@@ -787,19 +786,23 @@ class CRM_Batch_Form_Entry extends CRM_Core_Form {
 
         $value['is_renew'] = FALSE;
         if (!empty($params['member_option']) && CRM_Utils_Array::value($key, $params['member_option']) == 2) {
+
+          // The following parameter setting may be obsolete.
           $this->_params = $params;
           $value['is_renew'] = TRUE;
-          $membership = CRM_Member_BAO_Membership::renewMembershipFormWrapper(
-            $value['contact_id'],
-            $value['membership_type_id'],
-            FALSE,
-            $this,
-            NULL,
-            NULL,
-            $value['custom'],
-            1,
-            NULL,
-            FALSE
+          $isPayLater = CRM_Utils_Array::value('is_pay_later', $params);
+          $campaignId = NULL;
+          if (isset($this->_values) && is_array($this->_values) && !empty($this->_values)) {
+            $campaignId = CRM_Utils_Array::value('campaign_id', $this->_params);
+            if (!array_key_exists('campaign_id', $this->_params)) {
+              $campaignId = CRM_Utils_Array::value('campaign_id', $this->_values);
+            }
+          }
+
+          list($membership) = CRM_Member_BAO_Membership::renewMembership(
+            $value['contact_id'], $value['membership_type_id'], FALSE,
+            NULL, NULL, $value['custom'], NULL, NULL, FALSE,
+            NULL, NULL, $isPayLater, $campaignId
           );
 
           // make contribution entry

diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php
index c8ac51b7d8ac7b7fb29d2d39a57d05ae113e42b5..82a44216e2bdfbe82437223bf7a24ed9c0ad7b5f 100644 (file)
--- a/CRM/Member/BAO/Membership.php
+++ b/CRM/Member/BAO/Membership.php
@@ -1420,13 +1420,42 @@ AND civicrm_membership.is_test = %2";
         else {
           $pending = $isPending;
         }
-        $membership = self::renewMembershipFormWrapper($contactID, $memType,
-          $isTest, $form, date('YmdHis'),
-          CRM_Utils_Array::value('cms_contactID', $membershipParams),
-          $customFieldsFormatted, $numTerms,
-          $membershipID,
-          $pending
+        $contributionRecurID = isset($form->_params['contributionRecurID']) ? $form->_params['contributionRecurID'] : NULL;
+
+        $membershipSource = NULL;
+        if (!empty($form->_params['membership_source'])) {
+          $membershipSource = $form->_params['membership_source'];
+        }
+        elseif (isset($form->_values['title']) && !empty($form->_values['title'])) {
+          $membershipSource = ts('Online Contribution:') . ' ' . $form->_values['title'];
+        }
+        $isPayLater = NULL;
+        if (isset($form->_params)) {
+          $isPayLater = CRM_Utils_Array::value('is_pay_later', $form->_params);
+        }
+        $campaignId = NULL;
+        if (isset($form->_values) && is_array($form->_values) && !empty($form->_values)) {
+          $campaignId = CRM_Utils_Array::value('campaign_id', $form->_params);
+          if (!array_key_exists('campaign_id', $form->_params)) {
+            $campaignId = CRM_Utils_Array::value('campaign_id', $form->_values);
+          }
+        }
+
+        list($membership, $renewalMode, $dates) =  self::renewMembership(
+          $contactID, $memType, $isTest,
+          date('YmdHis'), CRM_Utils_Array::value('cms_contactID', $membershipParams), $customFieldsFormatted,
+          $numTerms, $membershipID, $pending,
+          $contributionRecurID, $membershipSource, $isPayLater, $campaignId
         );
+        $form->set('renewal_mode', $renewalMode);
+        if (!empty($dates)) {
+          $form->assign('mem_start_date',
+            CRM_Utils_Date::customFormat($dates['start_date'], '%Y%m%d')
+          );
+          $form->assign('mem_end_date',
+            CRM_Utils_Date::customFormat($dates['end_date'], '%Y%m%d')
+          );
+        }
 
         if (!empty($membershipContribution)) {
           // update recurring id for membership record
@@ -1540,90 +1569,6 @@ AND civicrm_membership.is_test = %2";
     CRM_Core_DAO::executeQuery($sql, $params);
   }
 
-  /**
-   * A wrapper for renewing memberships from a form.
-   *
-   * @deprecated
-   *  - including the form in the membership processing adds complexity
-   * as the forms are being forced to pretend similarity
-   * Try to call the renewMembership directly
-   * @todo - this form method needs to have the interaction with the form layer removed from it
-   * as a BAO function. Note that the api now supports membership renewals & it is not clear this function does anything
-   * not done by the membership.create api (with a lot less unit tests)
-   *
-   * This method will renew / create the membership depending on
-   * whether the given contact has a membership or not. And will add
-   * the modified dates for membership and in the log table.
-   *
-   * @param int $contactID
-   *   Id of the contact.
-   * @param int $membershipTypeID
-   *   Id of the new membership type.
-   * @param bool $is_test
-   *   If this is test contribution or live contribution.
-   * @param CRM_Core_Form $form
-   *   Form object.
-   * @param null $changeToday
-   * @param int $modifiedID
-   *   Individual contact id in case of On Behalf signup (CRM-4027 ).
-   * @param null $customFieldsFormatted
-   * @param int $numRenewTerms
-   *   How many membership terms are being added to end date (default is 1).
-   * @param int $membershipID
-   *   Membership ID, this should always be passed in & optionality should be removed.
-   *
-   * @param bool $isPending
-   *   Is the transaction pending. We are working towards this ALWAYS being true and completion being done
-   *   in the complete transaction function, called by all types of payment processor (removing assumptions
-   *   about what they do & always doing a pending + a complete at the appropriate time).
-   *
-   * @return CRM_Member_BAO_Membership|CRM_Core_Error
-   */
-  public static function renewMembershipFormWrapper(
-    $contactID,
-    $membershipTypeID,
-    $is_test,
-    &$form,
-    $changeToday,
-    $modifiedID,
-    $customFieldsFormatted,
-    $numRenewTerms,
-    $membershipID,
-    $isPending
-  ) {
-    $statusFormat = '%Y-%m-%d';
-    $format = '%Y%m%d';
-    $ids = array();
-    //@todo would be better to make $membershipID a compulsory function param & make form layer responsible for extracting it
-    if (!$membershipID && isset($form->_membershipId)) {
-      $membershipID = $form->_membershipId;
-    }
-
-    //get all active statuses of membership.
-    $allStatus = CRM_Member_PseudoConstant::membershipStatus();
-
-    $membershipTypeDetails = CRM_Member_BAO_MembershipType::getMembershipTypeDetails($membershipTypeID);
-
-    // check is it pending. - CRM-4555
-    list($contributionRecurID, $membershipSource, $isPayLater, $campaignId) = self::extractFormValues($form);
-    list($membership, $renewalMode, $dates) = self::renewMembership(
-      $contactID, $membershipTypeID, $is_test,
-      $changeToday, $modifiedID, $customFieldsFormatted, $numRenewTerms, $membershipID, $isPending, $allStatus,
-      $membershipTypeDetails, $contributionRecurID, $format, $membershipSource, $ids, $statusFormat, $isPayLater, $campaignId
-    );
-    $form->set('renewal_mode', $renewalMode);
-    if (!empty($dates)) {
-      $form->assign('mem_start_date',
-        CRM_Utils_Date::customFormat($dates['start_date'], $format)
-      );
-      $form->assign('mem_end_date',
-        CRM_Utils_Date::customFormat($dates['end_date'], $format)
-      );
-    }
-    return $membership;
-
-  }
-
   /**
    * Method to fix membership status of stale membership.
    *
@@ -2366,37 +2311,6 @@ INNER JOIN  civicrm_contact contact ON ( contact.id = membership.contact_id AND
     return ts('Payment Processor Error message') . ': ' . implode('<br/>', $message);
   }
 
-  /**
-   * Extract relevant values from the form so we can separate form logic from BAO logcis.
-   *
-   * @param CRM_Core_Form $form
-   *
-   * @return array
-   */
-  public static function extractFormValues($form) {
-    $contributionRecurID = isset($form->_params['contributionRecurID']) ? $form->_params['contributionRecurID'] : NULL;
-
-    $membershipSource = NULL;
-    if (!empty($form->_params['membership_source'])) {
-      $membershipSource = $form->_params['membership_source'];
-    }
-    elseif (isset($form->_values['title']) && !empty($form->_values['title'])) {
-      $membershipSource = ts('Online Contribution:') . ' ' . $form->_values['title'];
-    }
-    $isPayLater = NULL;
-    if (isset($form->_params)) {
-      $isPayLater = CRM_Utils_Array::value('is_pay_later', $form->_params);
-    }
-    $campaignId = NULL;
-    if (isset($form->_values) && is_array($form->_values) && !empty($form->_values)) {
-      $campaignId = CRM_Utils_Array::value('campaign_id', $form->_params);
-      if (!array_key_exists('campaign_id', $form->_params)) {
-        $campaignId = CRM_Utils_Array::value('campaign_id', $form->_values);
-      }
-    }
-    return array($contributionRecurID, $membershipSource, $isPayLater, $campaignId);
-  }
-
   /**
    * @param int $contactID
    * @param int $membershipTypeID
@@ -2407,21 +2321,20 @@ INNER JOIN  civicrm_contact contact ON ( contact.id = membership.contact_id AND
    * @param $numRenewTerms
    * @param int $membershipID
    * @param $pending
-   * @param $allStatus
-   * @param array $membershipTypeDetails
    * @param int $contributionRecurID
-   * @param $format
    * @param $membershipSource
-   * @param $ids
-   * @param $statusFormat
    * @param $isPayLater
    * @param int $campaignId
    *
    * @throws CRM_Core_Exception
    * @return array
    */
-  public static function renewMembership($contactID, $membershipTypeID, $is_test, $changeToday, $modifiedID, $customFieldsFormatted, $numRenewTerms, $membershipID, $pending, $allStatus, $membershipTypeDetails, $contributionRecurID, $format, $membershipSource, $ids, $statusFormat, $isPayLater, $campaignId) {
+  public static function renewMembership($contactID, $membershipTypeID, $is_test, $changeToday, $modifiedID, $customFieldsFormatted, $numRenewTerms, $membershipID, $pending, $contributionRecurID, $membershipSource, $statusFormat, $isPayLater, $campaignId) {
     $renewalMode = $updateStatusId = FALSE;
+    $allStatus = CRM_Member_PseudoConstant::membershipStatus();
+    $format = '%Y%m%d';
+    $statusFormat = '%Y-%m-%d';
+    $membershipTypeDetails = CRM_Member_BAO_MembershipType::getMembershipTypeDetails($membershipTypeID);
     $dates = array();
     // CRM-7297 - allow membership type to be be changed during renewal so long as the parent org of new membershipType
     // is the same as the parent org of an existing membership of the contact
@@ -2475,7 +2388,7 @@ INNER JOIN  civicrm_contact contact ON ( contact.id = membership.contact_id AND
         else {
           $logParams['modified_id'] = $membership->contact_id;
         }
-        CRM_Member_BAO_MembershipLog::add($logParams, CRM_Core_DAO::$_nullArray);
+        CRM_Member_BAO_MembershipLog::add($logParams);
 
         if (!empty($contributionRecurID)) {
           CRM_Core_DAO::setFieldValue('CRM_Member_DAO_Membership', $membership->id,

diff --git a/CRM/Member/BAO/MembershipLog.php b/CRM/Member/BAO/MembershipLog.php
index d993fce25dc3743b29407a5cd7decc57a833def8..327bbb8b1e2fdfd487125d66694a819b08675a0e 100644 (file)
--- a/CRM/Member/BAO/MembershipLog.php
+++ b/CRM/Member/BAO/MembershipLog.php
@@ -45,7 +45,7 @@ class CRM_Member_BAO_MembershipLog extends CRM_Member_DAO_MembershipLog {
    *
    * @return object
    */
-  public static function add(&$params, &$ids) {
+  public static function add(&$params) {
     $membershipLog = new CRM_Member_DAO_MembershipLog();
     $membershipLog->copyValues($params);
 

diff --git a/CRM/Member/Form/MembershipRenewal.php b/CRM/Member/Form/MembershipRenewal.php
index 395ba5126742a0961944befce75b6aa98fe594c6..9291ac6831dee09a967b3f2ec1f5bac01e909c8b 100644 (file)
--- a/CRM/Member/Form/MembershipRenewal.php
+++ b/CRM/Member/Form/MembershipRenewal.php
@@ -611,14 +611,30 @@ class CRM_Member_Form_MembershipRenewal extends CRM_Member_Form {
       $this->_params['is_pay_later'] = 1;
     }
 
-    $renewMembership = CRM_Member_BAO_Membership::renewMembershipFormWrapper($this->_contactID,
-      $formValues['membership_type_id'][1],
-      $isTestMembership, $this,
-      $renewalDate,
-      NULL,
-      $customFieldsFormatted, $numRenewTerms,
-      $this->_membershipId,
-      self::extractPendingFormValue($this, $formValues['membership_type_id'][1])
+    $contributionRecurID = isset($form->_params['contributionRecurID']) ? $form->_params['contributionRecurID'] : NULL;
+
+    $membershipSource = NULL;
+    if (!empty($form->_params['membership_source'])) {
+      $membershipSource = $form->_params['membership_source'];
+    }
+    elseif (isset($form->_values['title']) && !empty($form->_values['title'])) {
+      $membershipSource = ts('Online Contribution:') . ' ' . $form->_values['title'];
+    }
+    $isPayLater = NULL;
+    if (isset($form->_params)) {
+      $isPayLater = CRM_Utils_Array::value('is_pay_later', $form->_params);
+    }
+    $campaignId = NULL;
+    if (isset($form->_values) && is_array($form->_values) && !empty($form->_values)) {
+      $campaignId = CRM_Utils_Array::value('campaign_id', $form->_params);
+      if (!array_key_exists('campaign_id', $form->_params)) {
+        $campaignId = CRM_Utils_Array::value('campaign_id', $form->_values);
+      }
+    }
+    $renewMembership = CRM_Member_BAO_Membership::renewMembership(
+      $this->_contactID, $formValues['membership_type_id'][1], $isTestMembership,
+      $renewalDate, NULL, $customFieldsFormatted, $numRenewTerms, $this->_membershipId, self::extractPendingFormValue($this, $formValues['membership_type_id'][1],
+      $contributionRecurID, $membershipSource, $isPayLater, $campaignId
     );
 
     $endDate = CRM_Utils_Date::processDate($renewMembership->end_date);

diff --git a/tests/phpunit/CRM/Member/BAO/MembershipTest.php b/tests/phpunit/CRM/Member/BAO/MembershipTest.php
index 2b2ba6c61ad0a40b3f25674f496cfeb80e91d80d..0b81f4f9e6eeea37258382158f118776cc6487bb 100644 (file)
--- a/tests/phpunit/CRM/Member/BAO/MembershipTest.php
+++ b/tests/phpunit/CRM/Member/BAO/MembershipTest.php
@@ -522,7 +522,7 @@ class CRM_Member_BAO_MembershipTest extends CiviUnitTestCase {
     $membershipRenewal = new CRM_Core_Form();
     $membershipRenewal->controller = new CRM_Core_Controller();
     $isTestMembership = 0;
-    $MembershipRenew = CRM_Member_BAO_Membership::renewMembershipFormWrapper(
+    $MembershipRenew = CRM_Member_BAO_Membership::renewMembership(
       $contactId,
       $this->_membershipTypeID,
       $isTestMembership,
@@ -606,10 +606,10 @@ class CRM_Member_BAO_MembershipTest extends CiviUnitTestCase {
 
     $membershipRenewal = new CRM_Core_Form();
     $membershipRenewal->controller = new CRM_Core_Controller();
-    $MembershipRenew = CRM_Member_BAO_Membership::renewMembershipFormWrapper(
+    $MembershipRenew = CRM_Member_BAO_Membership::renewMembership(
       $contactId,
       $this->_membershipTypeID,
-      $isTestMembership = 0,
+      FALSE,
       $membershipRenewal,
       NULL,
       NULL,