From 94e0d40a280702827c42074483a12a77a5791db2 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Tue, 14 Feb 2023 15:00:48 +1300
Subject: [PATCH] Preliminary cleanup, comments, type hints

---
 CRM/Activity/Selector/Search.php              | 10 +++----
 .../CRM/Activity/Selector/SearchTest.php      | 28 +++++++++++++------
 tests/phpunit/CiviTest/CiviUnitTestCase.php   |  8 ++----
 3 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/CRM/Activity/Selector/Search.php b/CRM/Activity/Selector/Search.php
index 14215f6adc..04a68f94be 100644
--- a/CRM/Activity/Selector/Search.php
+++ b/CRM/Activity/Selector/Search.php
@@ -205,7 +205,7 @@ class CRM_Activity_Selector_Search extends CRM_Core_Selector_Base implements CRM
    *   The row number to start from.
    * @param int $rowCount
    *   The number of rows to return.
-   * @param string $sort
+   * @param string|CRM_Utils_Sort $sort
    *   The sql string that describes the sort order.
    * @param string $output
    *   What should the result set include (web/email/csv).
@@ -265,7 +265,7 @@ class CRM_Activity_Selector_Search extends CRM_Core_Selector_Base implements CRM
       $row['source_contact_name'] = implode(',', array_values($row['source_contact_name']));
       $row['source_contact_id'] = implode(',', $row['source_contact_id']);
 
-      if ($this->_context == 'search') {
+      if ($this->_context === 'search') {
         $row['checkbox'] = CRM_Core_Form::CB_PREFIX . $result->activity_id;
       }
       $row['contact_type'] = CRM_Contact_BAO_Contact_Utils::getImage($result->contact_sub_type ? $result->contact_sub_type : $result->contact_type, FALSE, $result->contact_id
@@ -291,10 +291,10 @@ class CRM_Activity_Selector_Search extends CRM_Core_Selector_Base implements CRM
         $accessMailingReport = TRUE;
       }
       $activityActions = new CRM_Activity_Selector_Activity($result->contact_id, NULL);
-      $actionLinks = $activityActions->actionLinks($activityTypeId,
-        CRM_Utils_Array::value('source_record_id', $row),
+      $actionLinks = $activityActions::actionLinks($activityTypeId,
+        $row['source_record_id'] ?? NULL,
         $accessMailingReport,
-        CRM_Utils_Array::value('activity_id', $row),
+        $row['activity_id'] ?? NULL,
         $this->_key,
         $this->_compContext
       );
diff --git a/tests/phpunit/CRM/Activity/Selector/SearchTest.php b/tests/phpunit/CRM/Activity/Selector/SearchTest.php
index 44728353e8..bb0c848d8d 100644
--- a/tests/phpunit/CRM/Activity/Selector/SearchTest.php
+++ b/tests/phpunit/CRM/Activity/Selector/SearchTest.php
@@ -18,10 +18,8 @@ class CRM_Activity_Selector_SearchTest extends CiviUnitTestCase {
 
   /**
    * Test activity search applies a permission based component filter.
-   *
-   * @throws \CRM_Core_Exception
    */
-  public function testActivitySearchComponentPermission() {
+  public function testActivitySearchComponentPermission(): void {
     $this->activityCreate(['activity_type_id' => 'Contribution']);
     $this->activityCreate(['activity_type_id' => 'Pledge Reminder']);
     $this->activityCreate(['activity_type_id' => 'Meeting']);
@@ -33,8 +31,11 @@ class CRM_Activity_Selector_SearchTest extends CiviUnitTestCase {
     $this->assertEquals("civicrm_activity.location = 'Baker Street'", $queryObject->_where[''][0]);
   }
 
-  public function testActivityOrderBy() {
-    $sortVars = [
+  /**
+   * Test for absence of fatal error on sort.
+   */
+  public function testActivityOrderBy(): void {
+    $sort = new CRM_Utils_Sort([
       1 => [
         'name' => 'activity_date_time',
         'sort' => 'activity_date_time',
@@ -65,10 +66,21 @@ class CRM_Activity_Selector_SearchTest extends CiviUnitTestCase {
         'direction' => 1,
         'title' => 'Status',
       ],
-    ];
-    $sort = new CRM_Utils_Sort($sortVars, '5_u');
+    ], '5_u');
+    $this->getSearchRows([], $sort);
+  }
+
+  /**
+   * Get the result of the search.
+   *
+   * @param array $queryParams
+   * @param \CRM_Utils_Sort|NULL $sort
+   *
+   * @return array
+   */
+  protected function getSearchRows(array $queryParams, ?CRM_Utils_Sort $sort): array {
     $searchSelector = new CRM_Activity_Selector_Search($queryParams, CRM_Core_Action::VIEW);
-    $searchSelector->getRows(4, 0, 50, $sort);
+    return $searchSelector->getRows(4, 0, 50, $sort);
   }
 
 }
diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
index 03fe148698..f4cf1bdb35 100644
--- a/tests/phpunit/CiviTest/CiviUnitTestCase.php
+++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -1414,11 +1414,9 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase {
    *
    * @param array $params
    *
-   * @return array|int
-   *
-   * @throws \CRM_Core_Exception
+   * @return array
    */
-  public function activityCreate($params = []) {
+  public function activityCreate(array $params = []): array {
     $params = array_merge([
       'subject' => 'Discussion on warm beer',
       'activity_date_time' => date('Ymd'),
@@ -1444,7 +1442,7 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase {
       $params['assignee_contact_id'] = $params['target_contact_id'];
     }
 
-    $result = civicrm_api3('Activity', 'create', $params);
+    $result = $this->callAPISuccess('Activity', 'create', $params);
 
     $result['target_contact_id'] = $params['target_contact_id'];
     $result['assignee_contact_id'] = $params['assignee_contact_id'];
-- 
2.25.1