From e6d0c736b7133e8700514917bfb7eca94fc284f1 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 16 Apr 2020 18:23:26 +1200 Subject: [PATCH] [REF] get rid of variable variable structure Readability improvement --- CRM/Member/BAO/Membership.php | 19 ++++++----------- tests/phpunit/api/v3/ContactTest.php | 32 +++++++++++++++++----------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/CRM/Member/BAO/Membership.php b/CRM/Member/BAO/Membership.php index 52b15d6a3a..24b242371c 100644 --- a/CRM/Member/BAO/Membership.php +++ b/CRM/Member/BAO/Membership.php @@ -254,12 +254,11 @@ class CRM_Member_BAO_Membership extends CRM_Member_DAO_Membership { // eg pay later membership, membership update cron CRM-3984 if (empty($params['is_override']) && empty($params['skipStatusCal'])) { - $dates = ['start_date', 'end_date', 'join_date']; - // Declare these out of courtesy as IDEs don't pick up the setting of them below. - $start_date = $end_date = $join_date = NULL; - foreach ($dates as $date) { - $$date = $params[$date] = CRM_Utils_Date::processDate(CRM_Utils_Array::value($date, $params), NULL, TRUE, 'Ymd'); - } + // @todo - we should be able to count on dates being correctly formatted by they time they hit the BAO. + // Maybe do some tests & throw some deprecation warnings if they aren't? + $params['start_date'] = trim($params['start_date']) ? date('Ymd', strtotime(trim($params['start_date']))) : 'null'; + $params['end_date'] = trim($params['end_date']) ? date('Ymd', strtotime(trim($params['end_date']))) : 'null'; + $params['join_date'] = trim($params['join_date']) ? date('Ymd', strtotime(trim($params['join_date']))) : 'null'; //fix for CRM-3570, during import exclude the statuses those having is_admin = 1 $excludeIsAdmin = CRM_Utils_Array::value('exclude_is_admin', $params, FALSE); @@ -269,15 +268,11 @@ class CRM_Member_BAO_Membership extends CRM_Member_DAO_Membership { $excludeIsAdmin = TRUE; } - $calcStatus = CRM_Member_BAO_MembershipStatus::getMembershipStatusByDate($start_date, $end_date, $join_date, + $calcStatus = CRM_Member_BAO_MembershipStatus::getMembershipStatusByDate($params['start_date'], $params['end_date'], $params['join_date'], 'today', $excludeIsAdmin, CRM_Utils_Array::value('membership_type_id', $params), $params ); if (empty($calcStatus)) { - throw new CRM_Core_Exception(ts( - "The membership cannot be saved because the status cannot be calculated for start_date: $start_date end_date $end_date join_date $join_date as at " . date('Y-m-d H:i:s')), - 0, - $errorParams - ); + 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 " . date('Y-m-d H:i:s'))); } $params['status_id'] = $calcStatus['id']; } diff --git a/tests/phpunit/api/v3/ContactTest.php b/tests/phpunit/api/v3/ContactTest.php index ee23962f1b..7c21d4d0a2 100644 --- a/tests/phpunit/api/v3/ContactTest.php +++ b/tests/phpunit/api/v3/ContactTest.php @@ -1708,6 +1708,11 @@ class api_v3_ContactTest extends CiviUnitTestCase { * Test merging 2 organizations. * * CRM-20421: This test make sure that inherited memberships are deleted upon merging organization. + * + * @throws \API_Exception + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + * @throws \Civi\API\Exception\UnauthorizedException */ public function testMergeOrganizations() { $organizationID1 = $this->organizationCreate([], 0); @@ -1721,20 +1726,18 @@ class api_v3_ContactTest extends CiviUnitTestCase { $membershipParams = [ 'membership_type_id' => $membershipType["id"], 'contact_id' => $organizationID1, - 'start_date' => "01/01/2015", - 'join_date' => "01/01/2010", - 'end_date' => "12/31/2015", + 'start_date' => '01/01/2015', + 'join_date' => '01/01/2010', + 'end_date' => '12/31/2015', ]; - $ownermembershipid = $this->contactMembershipCreate($membershipParams); + $ownerMembershipID = $this->contactMembershipCreate($membershipParams); - $contactmembership = $this->callAPISuccess("membership", "getsingle", [ - "contact_id" => $contact["id"], - ]); + $contactMembership = $this->callAPISuccessGetSingle('membership', ['contact_id' => $contact['id']]); - $this->assertEquals($ownermembershipid, $contactmembership["owner_membership_id"], "Contact membership must be inherited from Organization"); + $this->assertEquals($ownerMembershipID, $contactMembership['owner_membership_id'], "Contact membership must be inherited from Organization"); CRM_Dedupe_Merger::moveAllBelongings($organizationID2, $organizationID1, [ - "move_rel_table_memberships" => "0", + 'move_rel_table_memberships' => "0", "move_rel_table_relationships" => "1", "main_details" => [ "contact_id" => $organizationID2, @@ -1746,11 +1749,11 @@ class api_v3_ContactTest extends CiviUnitTestCase { ], ]); - $contactmembership = $this->callAPISuccess("membership", "get", [ + $contactMembership = $this->callAPISuccess("membership", "get", [ "contact_id" => $contact["id"], ]); - $this->assertEquals(0, $contactmembership["count"], "Contact membership must be deleted after merging organization without memberships."); + $this->assertEquals(0, $contactMembership["count"], "Contact membership must be deleted after merging organization without memberships."); } /** @@ -4738,11 +4741,16 @@ class api_v3_ContactTest extends CiviUnitTestCase { return $tests; } - /** + /** * CRM-14743 - test api respects search operators. * * @param int $version * + * @param $query + * @param $field + * @param $expected + * + * @throws \CRM_Core_Exception * @dataProvider versionAndPrivacyOption */ public function testGetContactsByPrivacyFlag($version, $query, $field, $expected) { -- 2.25.1