[REF] - Clean up recentItems functions and add test

Use array_filter() instead of brittle for() loops.

diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php
index 7e2ff0b74e25311a96eb76211f1385aa560afb42..cb855c0b46db49c3d32442f2ecc45e61d06f656b 100644 (file)
--- a/CRM/Contact/BAO/Contact.php
+++ b/CRM/Contact/BAO/Contact.php
@@ -1063,7 +1063,7 @@ WHERE     civicrm_contact.id = " . CRM_Utils_Type::escape($id, 'Integer');
     }
 
     //delete the contact id from recently view
-    CRM_Utils_Recent::delContact($id);
+    CRM_Utils_Recent::del(['contact_id' => $id]);
     self::updateContactCache($id, empty($restore));
 
     // delete any prevnext/dupe cache entry

diff --git a/CRM/Utils/Recent.php b/CRM/Utils/Recent.php
index 231f4af39540be574e3bf4cc1f2a7b856abbcbda..64636385199d5823f834027febf1c17d49d119fe 100644 (file)
--- a/CRM/Utils/Recent.php
+++ b/CRM/Utils/Recent.php
@@ -90,22 +90,13 @@ class CRM_Utils_Recent {
     $contactName,
     $others = []
   ) {
-    self::initialize();
-
+    // Abort if this entity type is not supported
     if (!self::isProviderEnabled($type)) {
       return;
     }
 
-    $session = CRM_Core_Session::singleton();
-
-    // make sure item is not already present in list
-    for ($i = 0; $i < count(self::$_recent); $i++) {
-      if (self::$_recent[$i]['type'] === $type && self::$_recent[$i]['id'] == $id) {
-        // delete item from array
-        array_splice(self::$_recent, $i, 1);
-        break;
-      }
-    }
+    // Ensure item is not already present in list
+    self::removeItems(['id' => $id, 'type' => $type]);
 
     if (!is_array($others)) {
       $others = [];
@@ -127,12 +118,14 @@ class CRM_Utils_Recent {
       ]
     );
 
-    if (count(self::$_recent) > self::$_maxItems) {
+    // Keep the list trimmed to max length
+    while (count(self::$_recent) > self::$_maxItems) {
       array_pop(self::$_recent);
     }
 
     CRM_Utils_Hook::recent(self::$_recent);
 
+    $session = CRM_Core_Session::singleton();
     $session->set(self::STORE_NAME, self::$_recent);
   }
 
@@ -151,27 +144,30 @@ class CRM_Utils_Recent {
   }
 
   /**
-   * Delete an item from the recent stack.
-   *
-   * @param array $recentItem
-   *   Array of the recent Item to be removed.
+   * Remove items from the array that match given props
+   * @param array $props
    */
-  public static function del($recentItem) {
+  private static function removeItems(array $props) {
     self::initialize();
-    $tempRecent = self::$_recent;
-
-    self::$_recent = [];
 
-    // make sure item is not already present in list
-    for ($i = 0; $i < count($tempRecent); $i++) {
-      if (!($tempRecent[$i]['id'] == $recentItem['id'] &&
-        $tempRecent[$i]['type'] == $recentItem['type']
-      )
-      ) {
-        self::$_recent[] = $tempRecent[$i];
+    self::$_recent = array_filter(self::$_recent, function($item) use ($props) {
+      foreach ($props as $key => $val) {
+        if (isset($item[$key]) && $item[$key] == $val) {
+          return FALSE;
+        }
       }
-    }
+      return TRUE;
+    });
+  }
 
+  /**
+   * Delete item(s) from the recently-viewed list.
+   *
+   * @param array $removeItem
+   *   Item to be removed.
+   */
+  public static function del($removeItem) {
+    self::removeItems($removeItem);
     CRM_Utils_Hook::recent(self::$_recent);
     $session = CRM_Core_Session::singleton();
     $session->set(self::STORE_NAME, self::$_recent);
@@ -181,27 +177,11 @@ class CRM_Utils_Recent {
    * Delete an item from the recent stack.
    *
    * @param string $id
-   *   Contact id that had to be removed.
+   * @deprecated
    */
   public static function delContact($id) {
-    self::initialize();
-
-    $tempRecent = self::$_recent;
-
-    self::$_recent = [];
-
-    // rebuild recent.
-    for ($i = 0; $i < count($tempRecent); $i++) {
-      // don't include deleted contact in recent.
-      if (CRM_Utils_Array::value('contact_id', $tempRecent[$i]) == $id) {
-        continue;
-      }
-      self::$_recent[] = $tempRecent[$i];
-    }
-
-    CRM_Utils_Hook::recent(self::$_recent);
-    $session = CRM_Core_Session::singleton();
-    $session->set(self::STORE_NAME, self::$_recent);
+    CRM_Core_Error::deprecatedFunctionWarning('del');
+    self::del(['contact_id' => $id]);
   }
 
   /**

diff --git a/tests/phpunit/api/v4/Action/RecentItemsTest.php b/tests/phpunit/api/v4/Action/RecentItemsTest.php
new file mode 100644 (file)
index 0000000..967e37a
--- /dev/null
+++ b/tests/phpunit/api/v4/Action/RecentItemsTest.php
@@ -0,0 +1,62 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *
+ * @package CRM
+ * @copyright CiviCRM LLC https://civicrm.org/licensing
+ */
+
+
+namespace api\v4\Action;
+
+use api\v4\UnitTestCase;
+use Civi\Api4\Activity;
+
+/**
+ * @group headless
+ */
+class RecentItemsTest extends UnitTestCase {
+
+  /**
+   * This locks in a fix to ensure that if a user doesn't have permission to view the is_deleted field that doesn't hard fail if that field happens to be in an APIv4 call.
+   */
+  public function testIsDeletedPermission(): void {
+    $cid = $this->createLoggedInUser();
+
+    $aid = Activity::create(FALSE)
+      ->addValue('activity_type_id:name', 'Meeting')
+      ->addValue('source_contact_id', $cid)
+      ->addValue('subject', 'Hello recent!')
+      ->execute()->first()['id'];
+
+    $this->assertEquals(1, $this->getRecentItemCount(['type' => 'Activity', 'id' => $aid]));
+
+    Activity::delete(FALSE)->addWhere('id', '=', $aid)->execute();
+
+    $this->assertEquals(0, $this->getRecentItemCount(['type' => 'Activity', 'id' => $aid]));
+  }
+
+  private function getRecentItemCount($props) {
+    $count = 0;
+    foreach (\CRM_Utils_Recent::get() as $item) {
+      foreach ($props as $key => $val) {
+        if (($item[$key] ?? NULL) != $val) {
+          continue 2;
+        }
+      }
+      ++$count;
+    }
+    return $count;
+  }
+
+}