From ae13d6479d15d45a211f26be2879c25e05453723 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 17 Jul 2023 13:20:56 +1200 Subject: [PATCH] [tests][php8.2] Use function rather than sometimes declared property for loggedInUserID --- tests/phpunit/CRM/Case/Form/CaseViewTest.php | 4 +-- .../CRM/Case/Form/ChangeStartDateTest.php | 4 +-- .../phpunit/CRM/Case/Form/CustomDataTest.php | 4 +-- tests/phpunit/CRM/Case/Form/EmailTest.php | 8 +++--- tests/phpunit/CRM/Case/Form/TaskTest.php | 6 ++--- .../CRM/Case/XMLProcessor/ProcessTest.php | 6 ++--- .../CRM/Case/XMLProcessor/ReportTest.php | 10 +++---- tests/phpunit/CiviTest/CiviCaseTestCase.php | 4 --- tests/phpunit/CiviTest/CiviUnitTestCase.php | 26 +++++++++++-------- tests/phpunit/api/v3/CaseTest.php | 4 +-- tests/phpunit/api/v3/CaseTypeTest.php | 4 +-- 11 files changed, 40 insertions(+), 40 deletions(-) diff --git a/tests/phpunit/CRM/Case/Form/CaseViewTest.php b/tests/phpunit/CRM/Case/Form/CaseViewTest.php index dff1949470..086a074d7b 100644 --- a/tests/phpunit/CRM/Case/Form/CaseViewTest.php +++ b/tests/phpunit/CRM/Case/Form/CaseViewTest.php @@ -8,9 +8,9 @@ class CRM_Case_Form_CaseViewTest extends CiviCaseTestCase { /** * Test that the search filter dropdown includes the desired activity types. */ - public function testSearchFilterDropdown() { + public function testSearchFilterDropdown(): void { $client_id = $this->individualCreate([], 0, TRUE); - $caseObj = $this->createCase($client_id, $this->_loggedInUser); + $caseObj = $this->createCase($client_id, $this->getLoggedInUser()); $form = $this->getFormObject('CRM_Case_Form_CaseView'); $form->set('cid', $client_id); diff --git a/tests/phpunit/CRM/Case/Form/ChangeStartDateTest.php b/tests/phpunit/CRM/Case/Form/ChangeStartDateTest.php index 4ed6bc9d66..c0132dee34 100644 --- a/tests/phpunit/CRM/Case/Form/ChangeStartDateTest.php +++ b/tests/phpunit/CRM/Case/Form/ChangeStartDateTest.php @@ -7,7 +7,7 @@ class CRM_Case_Form_ChangeStartDateTest extends CiviCaseTestCase { public function testChangeCaseStartDate(): void { $clientId = $this->individualCreate(); - $caseObj = $this->createCase($clientId, $this->_loggedInUser); + $caseObj = $this->createCase($clientId, $this->getLoggedInUser()); $result = $this->callAPISuccess('Case', 'getsingle', [ 'id' => $caseObj->id, @@ -28,7 +28,7 @@ class CRM_Case_Form_ChangeStartDateTest extends CiviCaseTestCase { $form = $this->getFormObject('CRM_Case_Form_Activity', [ 'activity_type_id' => $activity_type_id, 'caseid' => $caseObj->id, - 'source_contact_id' => $this->_loggedInUser, + 'source_contact_id' => $this->getLoggedInUser(), 'target_contact_id' => $clientId, 'cid' => $clientId, 'activity_date_time' => date('Y-m-d H:i:s'), diff --git a/tests/phpunit/CRM/Case/Form/CustomDataTest.php b/tests/phpunit/CRM/Case/Form/CustomDataTest.php index f4e9d0a8d9..93f5fe1799 100644 --- a/tests/phpunit/CRM/Case/Form/CustomDataTest.php +++ b/tests/phpunit/CRM/Case/Form/CustomDataTest.php @@ -38,7 +38,7 @@ class CRM_Case_Form_CustomDataTest extends CiviCaseTestCase { // set up case and set the custom field initial value $client_id = $this->individualCreate([], 0, TRUE); - $caseObj = $this->createCase($client_id, $this->_loggedInUser); + $caseObj = $this->createCase($client_id, $this->getLoggedInUser()); if (isset($input['custom_field_oldvalue'])) { $this->callAPISuccess('CustomValue', 'create', [ "custom_{$custom_field['id']}" => $input['custom_field_oldvalue'], @@ -349,7 +349,7 @@ class CRM_Case_Form_CustomDataTest extends CiviCaseTestCase { // Create a case and set the custom field to something $individual = $this->individualCreate(); - $caseObj = $this->createCase($individual, $this->_loggedInUser); + $caseObj = $this->createCase($individual, $this->getLoggedInUser()); $caseId = $caseObj->id; $this->callAPISuccess('CustomValue', 'create', [ "custom_{$customField['id']}" => 'immutable text', diff --git a/tests/phpunit/CRM/Case/Form/EmailTest.php b/tests/phpunit/CRM/Case/Form/EmailTest.php index e101658975..fa1f3e54a9 100644 --- a/tests/phpunit/CRM/Case/Form/EmailTest.php +++ b/tests/phpunit/CRM/Case/Form/EmailTest.php @@ -7,9 +7,9 @@ class CRM_Case_Form_EmailTest extends CiviCaseTestCase { public function testOpeningEmailForm(): void { $clientId = $this->individualCreate(); - $caseObj = $this->createCase($clientId, $this->_loggedInUser); + $caseObj = $this->createCase($clientId, $this->getLoggedInUser()); - $url = "civicrm/case/email/add?reset=1&action=add&atype=3&cid={$this->_loggedInUser}&caseid={$caseObj->id}"; + $url = "civicrm/case/email/add?reset=1&action=add&atype=3&cid={$this->getLoggedInUser()}&caseid={$caseObj->id}"; $_SERVER['REQUEST_URI'] = $url; $urlParts = explode('?', $url); @@ -42,12 +42,12 @@ class CRM_Case_Form_EmailTest extends CiviCaseTestCase { public function testCaseTokenForRecipientAddedAfterOpeningForm(): void { $clientId = $this->individualCreate(); - $caseObj = $this->createCase($clientId, $this->_loggedInUser); + $caseObj = $this->createCase($clientId, $this->getLoggedInUser()); $anotherPersonId = $this->individualCreate([], 1); $anotherPersonInfo = $this->callAPISuccess('Contact', 'getsingle', ['id' => $anotherPersonId]); - $senderEmail = $this->callAPISuccess('Email', 'getsingle', ['contact_id' => $this->_loggedInUser]); + $senderEmail = $this->callAPISuccess('Email', 'getsingle', ['contact_id' => $this->getLoggedInUser()]); $mut = new CiviMailUtils($this); diff --git a/tests/phpunit/CRM/Case/Form/TaskTest.php b/tests/phpunit/CRM/Case/Form/TaskTest.php index 47569b3f8e..618e2ea2f7 100644 --- a/tests/phpunit/CRM/Case/Form/TaskTest.php +++ b/tests/phpunit/CRM/Case/Form/TaskTest.php @@ -135,13 +135,13 @@ class CRM_Case_Form_TaskTest extends CiviCaseTestCase { public function testOpenFileOnCaseForm($input) { // Create a case and an activity to use $client_id = $this->individualCreate([], 0, TRUE); - $case = $this->createCase($client_id, $this->_loggedInUser); + $case = $this->createCase($client_id, $this->getLoggedInUser()); // create 2 cases since "move"/"copy" aren't available actions otherwise - $case = $this->createCase($this->individualCreate([], 1, TRUE), $this->_loggedInUser); + $case = $this->createCase($this->individualCreate([], 1, TRUE), $this->getLoggedInUser()); $activity_params = [ 'activity_type_id' => 'Inbound Email', 'source_contact_id' => $client_id, - 'target_id' => $this->_loggedInUser, + 'target_id' => $this->getLoggedInUser(), 'subject' => $input['subject'] ?? NULL, 'details' => 'test test test', 'activity_date_time' => date('Ymdhis'), diff --git a/tests/phpunit/CRM/Case/XMLProcessor/ProcessTest.php b/tests/phpunit/CRM/Case/XMLProcessor/ProcessTest.php index 8d6a890d56..4631684f66 100644 --- a/tests/phpunit/CRM/Case/XMLProcessor/ProcessTest.php +++ b/tests/phpunit/CRM/Case/XMLProcessor/ProcessTest.php @@ -214,7 +214,7 @@ class CRM_Case_XMLProcessor_ProcessTest extends CiviCaseTestCase { // @todo This seems wrong, it just happens to work out because both caseId and caseTypeId equal 1 in the stock setup here. 'caseID' => $this->caseTypeId, 'clientID' => $this->contacts['ana'], - 'creatorID' => $this->_loggedInUser, + 'creatorID' => $this->getLoggedInUser(), ]; } @@ -264,7 +264,7 @@ class CRM_Case_XMLProcessor_ProcessTest extends CiviCaseTestCase { // Make another case and add a case role with the same relationship we // want, but a different person. - $caseObj = $this->createCase($this->contacts['ana'], $this->_loggedInUser); + $caseObj = $this->createCase($this->contacts['ana'], $this->getLoggedInUser()); $this->callAPISuccess('Relationship', 'create', [ 'contact_id_a' => $this->contacts['ana'], 'contact_id_b' => $this->contacts['carlos'], @@ -393,7 +393,7 @@ class CRM_Case_XMLProcessor_ProcessTest extends CiviCaseTestCase { $this->activityTypeXml->default_assignee_type = $this->defaultAssigneeOptionsValues['USER_CREATING_THE_CASE']; $this->process->createActivity($this->activityTypeXml, $this->activityParams); - $this->assertActivityAssignedToContactExists($this->_loggedInUser); + $this->assertActivityAssignedToContactExists($this->getLoggedInUser()); } /** diff --git a/tests/phpunit/CRM/Case/XMLProcessor/ReportTest.php b/tests/phpunit/CRM/Case/XMLProcessor/ReportTest.php index 65a47c1d53..5ed613556f 100644 --- a/tests/phpunit/CRM/Case/XMLProcessor/ReportTest.php +++ b/tests/phpunit/CRM/Case/XMLProcessor/ReportTest.php @@ -40,7 +40,7 @@ class CRM_Case_XMLProcessor_ReportTest extends CiviCaseTestCase { 'prefix_id' => NULL, 'suffix_id' => NULL, ]); - $caseObj = $this->createCase($client_id, $this->_loggedInUser, ['start_date' => '2019-11-14', 'start_date_time' => '20191114000000']); + $caseObj = $this->createCase($client_id, $this->getLoggedInUser(), ['start_date' => '2019-11-14', 'start_date_time' => '20191114000000']); $case_id = $caseObj->id; // Add an additional meeting activity not in the timeline to the case. @@ -54,7 +54,7 @@ class CRM_Case_XMLProcessor_ReportTest extends CiviCaseTestCase { 'activity_type_id' => $meetingTypeId['value'], 'activity_date_time' => '20191114123456', 'subject' => 'Test Meeting', - 'source_contact_id' => $this->_loggedInUser, + 'source_contact_id' => $this->getLoggedInUser(), 'target_contact_id' => $client_id, ]); @@ -88,7 +88,7 @@ class CRM_Case_XMLProcessor_ReportTest extends CiviCaseTestCase { 'prefix_id' => NULL, 'suffix_id' => NULL, ]); - $caseObj = $this->createCase($client_id, $this->_loggedInUser, ['start_date' => '2019-11-14', 'start_date_time' => '20191114000000']); + $caseObj = $this->createCase($client_id, $this->getLoggedInUser(), ['start_date' => '2019-11-14', 'start_date_time' => '20191114000000']); $case_id = $caseObj->id; // Now update the timeline so it has Meeting in it. @@ -105,7 +105,7 @@ class CRM_Case_XMLProcessor_ReportTest extends CiviCaseTestCase { 'activity_type_id' => $meetingTypeId['value'], 'activity_date_time' => '20191114123456', 'subject' => 'Test Meeting', - 'source_contact_id' => $this->_loggedInUser, + 'source_contact_id' => $this->getLoggedInUser(), 'target_contact_id' => $client_id, ]); @@ -543,7 +543,7 @@ class CRM_Case_XMLProcessor_ReportTest extends CiviCaseTestCase { private function updateExpectedBecauseDataProviderEvaluatesBeforeEverything(&$expected, $client_id, $case_id) { $display_name = $this->callAPISuccess('Contact', 'getsingle', [ 'return' => ["display_name"], - 'id' => $this->_loggedInUser, + 'id' => $this->getLoggedInUser(), ]); foreach ($expected['activities'] as $idx => $activity) { diff --git a/tests/phpunit/CiviTest/CiviCaseTestCase.php b/tests/phpunit/CiviTest/CiviCaseTestCase.php index 6c58f70c4c..0d1f615d91 100644 --- a/tests/phpunit/CiviTest/CiviCaseTestCase.php +++ b/tests/phpunit/CiviTest/CiviCaseTestCase.php @@ -26,8 +26,6 @@ class CiviCaseTestCase extends CiviUnitTestCase { protected $optionValues; - protected $_loggedInUser; - /** * Tables to truncate as part of cleanup * @var array @@ -104,8 +102,6 @@ class CiviCaseTestCase extends CiviUnitTestCase { // create a logged in USER since the code references it for source_contact_id $this->createLoggedInUser(); - $session = CRM_Core_Session::singleton(); - $this->_loggedInUser = $session->get('userID'); /// note that activityType options are cached by the FULL set of options you pass in // ie. because Activity api includes campaign in it's call cache is not flushed unless // included in this call. Also note flush function doesn't work on this property as it sets to null not empty array diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index bf917de2e7..2240371b0d 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -2004,7 +2004,7 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase { * * @throws \Civi\Core\Exception\DBQueryException */ - public function setupACL($isProfile = FALSE) { + public function setupACL(bool $isProfile = FALSE): void { global $_REQUEST; $_REQUEST = $this->_params; @@ -2079,6 +2079,15 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase { } } + /** + * Get the logged in user record. + * + * @return int|null + */ + public function getLoggedInUser(): ?int { + return CRM_Core_Session::singleton()->get('userID') ?: NULL; + } + /** * Alter default price set so that the field numbers are not all 1 (hiding errors) */ @@ -3450,22 +3459,18 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase { /** * Create and return a case object for the given Client ID. * - * @param int $clientId - * @param int $loggedInUser + * @param int $clientID + * @param int|null $loggedInUser * Omit or pass NULL to use the same as clientId * @param array $extra * Optional specific parameters such as start_date * * @return CRM_Case_BAO_Case */ - public function createCase($clientId, $loggedInUser = NULL, $extra = []) { - if (empty($loggedInUser)) { - // backwards compatibility - but it's more typical that the creator is a different person than the client - $loggedInUser = $clientId; - } + public function createCase(int $clientID, ?int $loggedInUser = NULL, array $extra = []): CRM_Case_DAO_Case { $caseParams = array_merge([ 'activity_subject' => 'Case Subject', - 'client_id' => $clientId, + 'client_id' => $clientID, 'case_type_id' => 1, 'status_id' => 1, 'case_type' => 'housing_support', @@ -3475,8 +3480,7 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase { 'medium_id' => 2, 'activity_details' => '', ], $extra); - $form = new CRM_Case_Form_Case(); - return $form->testSubmit($caseParams, 'OpenCase', $loggedInUser, 'standalone'); + return (new CRM_Case_Form_Case())->testSubmit($caseParams, 'OpenCase', $loggedInUser ?: $clientID, 'standalone'); } /** diff --git a/tests/phpunit/api/v3/CaseTest.php b/tests/phpunit/api/v3/CaseTest.php index d44bd61dfe..9e23c44d8b 100644 --- a/tests/phpunit/api/v3/CaseTest.php +++ b/tests/phpunit/api/v3/CaseTest.php @@ -483,10 +483,10 @@ class api_v3_CaseTest extends CiviCaseTestCase { // follow up 'activity_type_id' => $this->followup_activity_type_value, 'subject' => 'Test followup 123', - 'source_contact_id' => $this->_loggedInUser, + 'source_contact_id' => $this->getLoggedInUser(), 'target_contact_id' => $this->_params['contact_id'], ]; - $result = $this->callAPISuccess('activity', 'create', $params); + $result = $this->callAPISuccess('Activity', 'create', $params); $this->assertEquals($result['values'][$result['id']]['activity_type_id'], $params['activity_type_id']); // might need this for other tests that piggyback on this one diff --git a/tests/phpunit/api/v3/CaseTypeTest.php b/tests/phpunit/api/v3/CaseTypeTest.php index 8a67826846..0a824c3940 100644 --- a/tests/phpunit/api/v3/CaseTypeTest.php +++ b/tests/phpunit/api/v3/CaseTypeTest.php @@ -198,13 +198,13 @@ class api_v3_CaseTypeTest extends CiviCaseTestCase { $createCase = $this->callAPISuccess('Case', 'create', [ 'case_type_id' => $createCaseType['id'], - 'contact_id' => $this->_loggedInUser, + 'contact_id' => $this->getLoggedInUser(), 'subject' => 'Example', ]); // Deletion fails while case-type is in-use $deleteCaseType = $this->callAPIFailure('CaseType', 'delete', ['id' => $createCaseType['id']]); - $this->assertEquals("You can not delete this case type -- it is assigned to 1 existing case record(s). If you do not want this case type to be used going forward, consider disabling it instead.", $deleteCaseType['error_message']); + $this->assertEquals('You can not delete this case type -- it is assigned to 1 existing case record(s). If you do not want this case type to be used going forward, consider disabling it instead.', $deleteCaseType['error_message']); $getCaseType = $this->callAPISuccess('CaseType', 'get', ['id' => $createCaseType['id']]); $this->assertEquals(1, $getCaseType['count']); -- 2.25.1