[REF] Move membership date calc from v3 api to BAO

This moves code to calculate membership dates for new memberships from the
v3 api Membership.create and the batch entry membership form to the membership bao.

I think more cleanup could follow this - in the api most obviously and
the BAO is a bit insane still

This has test cover in the v3 api membership test and in
CRM_Batch_Form_EntryTest

diff --git a/CRM/Batch/Form/Entry.php b/CRM/Batch/Form/Entry.php
index 89938bb808686b022aebc19645d4645ba040fd70..8465b27266b6d61801e93f6babbf951d52ffe034 100644 (file)
--- a/CRM/Batch/Form/Entry.php
+++ b/CRM/Batch/Form/Entry.php
@@ -846,7 +846,12 @@ class CRM_Batch_Form_Entry extends CRM_Core_Form {
         // end of contribution related section
 
         unset($value['membership_type']);
-
+        $membershipParams = [
+          'start_date' => $value['membership_start_date'] ?? NULL,
+          'end_date' => $value['membership_end_date'] ?? NULL,
+          'join_date' => $value['membership_join_date'] ?? NULL,
+          'campaign_id' => $value['member_campaign_id'] ?? NULL,
+        ];
         $value['is_renew'] = FALSE;
         if (!empty($params['member_option']) && CRM_Utils_Array::value($key, $params['member_option']) == 2) {
 
@@ -873,17 +878,9 @@ class CRM_Batch_Form_Entry extends CRM_Core_Form {
           $this->setCurrentRowMembershipID($membership->id);
         }
         else {
-          $calcDates = CRM_Member_BAO_MembershipType::getDatesForMembershipType($membershipTypeId,
-            $value['membership_join_date'] ?? NULL, $value['membership_start_date'] ?? NULL, $value['membership_end_date'] ?? NULL
-          );
-          $value['join_date'] = $value['membership_join_date'] ?? $calcDates['join_date'];
-          $value['start_date'] = $value['membership_start_date'] ?? $calcDates['start_date'];
-          $value['end_date'] = $value['membership_end_date'] ?? $calcDates['end_date'];
-
-          unset($value['membership_start_date']);
-          unset($value['membership_end_date']);
-          $membership = CRM_Member_BAO_Membership::create($value);
-          $this->setCurrentRowMembershipID($membership->id);
+          // @todo - specify the relevant fields - don't copy all over
+          $membershipParams = array_merge($value, $membershipParams);
+          $membership = CRM_Member_BAO_Membership::create($membershipParams);
         }
 
         //process premiums

diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php
index 98f2f530e44cc656ebc32ce842cb70541053b01d..289d312c2d9ffac4acb5cb4ca1b0041e907e0352 100644 (file)
--- a/CRM/Member/BAO/Membership.php
+++ b/CRM/Member/BAO/Membership.php
@@ -257,7 +257,7 @@ class CRM_Member_BAO_Membership extends CRM_Member_DAO_Membership {
     // To skip status calculation we should use 'skipStatusCal'.
     // eg pay later membership, membership update cron CRM-3984
 
-    if (empty($params['is_override']) && empty($params['skipStatusCal'])) {
+    if (empty($params['skipStatusCal'])) {
       $fieldsToLoad = [];
       foreach (['start_date', 'end_date', 'join_date'] as $dateField) {
         if (!empty($params[$dateField]) && $params[$dateField] !== 'null' && strpos($params[$dateField], date('Ymd', CRM_Utils_Time::strtotime(trim($params[$dateField])))) !== 0) {
@@ -275,9 +275,23 @@ class CRM_Member_BAO_Membership extends CRM_Member_DAO_Membership {
           $params[$fieldToLoad] = $membership[$fieldToLoad];
         }
       }
-      $params['start_date'] = $params['start_date'] ?: 'null';
-      $params['end_date'] = $params['end_date'] ?: 'null';
-      $params['join_date'] = $params['join_date'] ?: 'null';
+      if (empty($params['id'])
+        && (empty($params['start_date']) || empty($params['join_date']) || (empty($params['end_date']) && !$isLifeTime))) {
+        // This is a new membership, calculate the membership dates.
+        $calcDates = CRM_Member_BAO_MembershipType::getDatesForMembershipType(
+          $params['membership_type_id'],
+          $params['join_date'] ?? NULL,
+          $params['start_date'] ?? NULL,
+          $params['end_date'] ?? NULL,
+          $params['num_terms'] ?? 1
+        );
+      }
+      else {
+        $calcDates = [];
+      }
+      $params['start_date'] = $params['start_date'] ?? ($calcDates['start_date'] ?? 'null');
+      $params['end_date'] = $params['end_date'] ?? ($calcDates['end_date'] ?? 'null');
+      $params['join_date'] = $params['join_date'] ?? ($calcDates['join_date'] ?? 'null');
 
       //fix for CRM-3570, during import exclude the statuses those having is_admin = 1
       $excludeIsAdmin = $params['exclude_is_admin'] ?? FALSE;
@@ -287,13 +301,15 @@ class CRM_Member_BAO_Membership extends CRM_Member_DAO_Membership {
         $excludeIsAdmin = TRUE;
       }
 
-      $calcStatus = CRM_Member_BAO_MembershipStatus::getMembershipStatusByDate($params['start_date'], $params['end_date'], $params['join_date'],
-        'now', $excludeIsAdmin, $params['membership_type_id'] ?? NULL, $params
-      );
-      if (empty($calcStatus)) {
-        throw new CRM_Core_Exception(ts("The membership cannot be saved because the status cannot be calculated for start_date: {$params['start_date']} end_date {$params['end_date']} join_date {$params['join_date']} as at " . CRM_Utils_Time::date('Y-m-d H:i:s')));
+      if (empty($params['is_override'])) {
+        $calcStatus = CRM_Member_BAO_MembershipStatus::getMembershipStatusByDate($params['start_date'], $params['end_date'], $params['join_date'],
+          'now', $excludeIsAdmin, $params['membership_type_id'] ?? NULL, $params
+        );
+        if (empty($calcStatus)) {
+          throw new CRM_Core_Exception(ts("The membership cannot be saved because the status cannot be calculated for start_date: {$params['start_date']} end_date {$params['end_date']} join_date {$params['join_date']} as at " . CRM_Utils_Time::date('Y-m-d H:i:s')));
+        }
+        $params['status_id'] = $calcStatus['id'];
       }
-      $params['status_id'] = $calcStatus['id'];
     }
 
     // data cleanup only: all verifications on number of related memberships are done upstream in:

diff --git a/api/v3/Membership.php b/api/v3/Membership.php
index c7a3b1d5e478cba39c8a0fc6575f83f75a370d92..52e64b8b741bf653cf755544d81991aa97c191c9 100644 (file)
--- a/api/v3/Membership.php
+++ b/api/v3/Membership.php
@@ -90,19 +90,9 @@ function civicrm_api3_membership_create($params) {
 
   // Calculate membership dates
   // Fixme: This code belongs in the BAO
-  if (empty($params['id']) || !empty($params['num_terms'])) {
+  if (!empty($params['num_terms'])) {
     // If this is a new membership or we have a specified number of terms calculate membership dates.
-    if (empty($params['id'])) {
-      // This is a new membership, calculate the membership dates.
-      $calcDates = CRM_Member_BAO_MembershipType::getDatesForMembershipType(
-        $params['membership_type_id'],
-        CRM_Utils_Array::value('join_date', $params),
-        CRM_Utils_Array::value('start_date', $params),
-        CRM_Utils_Array::value('end_date', $params),
-        CRM_Utils_Array::value('num_terms', $params, 1)
-      );
-    }
-    else {
+    if (!empty($params['id'])) {
       // 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.
@@ -112,10 +102,10 @@ function civicrm_api3_membership_create($params) {
         CRM_Utils_Array::value('membership_type_id', $params),
         $params['num_terms']
       );
-    }
-    foreach (['join_date', 'start_date', 'end_date'] as $date) {
-      if (empty($params[$date]) && isset($calcDates[$date])) {
-        $params[$date] = $calcDates[$date];
+      foreach (['join_date', 'start_date', 'end_date'] as $date) {
+        if (empty($params[$date]) && isset($calcDates[$date])) {
+          $params[$date] = $calcDates[$date];
+        }
       }
     }
   }

diff --git a/tests/phpunit/api/v3/MembershipTest.php b/tests/phpunit/api/v3/MembershipTest.php
index f8f8e026dae36c6d47f3e7a9948593915cd413dd..c23f5150d4b6eb538577757b487554cfdcfe9f76 100644 (file)
--- a/tests/phpunit/api/v3/MembershipTest.php
+++ b/tests/phpunit/api/v3/MembershipTest.php
@@ -315,8 +315,10 @@ class api_v3_MembershipTest extends CiviUnitTestCase {
    * Test civicrm_membership_get with params not array.
    *
    * Gets treated as contact_id, memberships expected.
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testGetInSyntax() {
+  public function testGetInSyntax(): void {
     $this->_membershipID = $this->contactMembershipCreate($this->_params);
     $this->_membershipID2 = $this->contactMembershipCreate($this->_params);
     $this->_membershipID3 = $this->contactMembershipCreate($this->_params);