From 7670664eb520a15ed3322a8bd4be6758c47b331c Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 24 May 2021 20:29:08 +1200 Subject: [PATCH] [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 --- CRM/Batch/Form/Entry.php | 21 +++++++-------- CRM/Member/BAO/Membership.php | 36 ++++++++++++++++++------- api/v3/Membership.php | 22 +++++---------- tests/phpunit/api/v3/MembershipTest.php | 4 ++- 4 files changed, 44 insertions(+), 39 deletions(-) diff --git a/CRM/Batch/Form/Entry.php b/CRM/Batch/Form/Entry.php index 89938bb808..8465b27266 100644 --- 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 98f2f530e4..289d312c2d 100644 --- 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 c7a3b1d5e4..52e64b8b74 100644 --- 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 f8f8e026da..c23f5150d4 100644 --- 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); -- 2.25.1