From 8b72ecff84a0c55d59c848d10463e814499d7d90 Mon Sep 17 00:00:00 2001
From: colemanw <coleman@civicrm.org>
Date: Sat, 27 May 2023 14:08:34 -0400
Subject: [PATCH] APIv4 - Fix CONTAINS operator to work with more types of
 serialized fields

---
 Civi/Api4/Query/Api4SelectQuery.php           | 23 +++++++++++-
 tests/phpunit/api/v4/Api4TestBase.php         |  2 +-
 .../api/v4/Custom/CustomContactRefTest.php    |  4 +-
 tests/phpunit/api/v4/Entity/GroupTest.php     | 37 +++++++++++++++++++
 4 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php
index 0ba463c334..7c93362e28 100644
--- a/Civi/Api4/Query/Api4SelectQuery.php
+++ b/Civi/Api4/Query/Api4SelectQuery.php
@@ -592,8 +592,13 @@ class Api4SelectQuery {
       }
       return $sql ? implode(' AND ', $sql) : NULL;
     }
+
+    // The CONTAINS operator matches a substring for strings. For arrays & serialized fields,
+    // it only matches a complete (not partial) string within the array.
     if ($operator === 'CONTAINS') {
+      $sep = \CRM_Core_DAO::VALUE_SEPARATOR;
       switch ($field['serialize'] ?? NULL) {
+
         case \CRM_Core_DAO::SERIALIZE_JSON:
           $operator = 'LIKE';
           $value = '%"' . $value . '"%';
@@ -603,7 +608,23 @@ class Api4SelectQuery {
 
         case \CRM_Core_DAO::SERIALIZE_SEPARATOR_BOOKEND:
           $operator = 'LIKE';
-          $value = '%' . \CRM_Core_DAO::VALUE_SEPARATOR . $value . \CRM_Core_DAO::VALUE_SEPARATOR . '%';
+          // This is easy to query because the string is always bookended by separators.
+          $value = '%' . $sep . $value . $sep . '%';
+          break;
+
+        case \CRM_Core_DAO::SERIALIZE_SEPARATOR_TRIMMED:
+          $operator = 'REGEXP';
+          // This is harder to query because there's no bookend.
+          // Use regex to match string within separators or content boundary
+          // Escaping regex per https://stackoverflow.com/questions/3782379/whats-the-best-way-to-escape-user-input-for-regular-expressions-in-mysql
+          $value = "(^|$sep)" . preg_quote($value, '&') . "($sep|$)";
+          break;
+
+        case \CRM_Core_DAO::SERIALIZE_COMMA:
+          $operator = 'REGEXP';
+          // Match string within commas or content boundary
+          // Escaping regex per https://stackoverflow.com/questions/3782379/whats-the-best-way-to-escape-user-input-for-regular-expressions-in-mysql
+          $value = '(^|,)' . preg_quote($value, '&') . '(,|$)';
           break;
 
         default:
diff --git a/tests/phpunit/api/v4/Api4TestBase.php b/tests/phpunit/api/v4/Api4TestBase.php
index 8ae16a8600..44f9a7449d 100644
--- a/tests/phpunit/api/v4/Api4TestBase.php
+++ b/tests/phpunit/api/v4/Api4TestBase.php
@@ -64,7 +64,7 @@ class Api4TestBase extends TestCase implements HeadlessInterface {
   /**
    * Quick clean by emptying tables created for the test.
    *
-   * @param array $params
+   * @param array{tablesToTruncate: array} $params
    */
   public function cleanup(array $params): void {
     $params += [
diff --git a/tests/phpunit/api/v4/Custom/CustomContactRefTest.php b/tests/phpunit/api/v4/Custom/CustomContactRefTest.php
index 1e9473ee79..2d80b306df 100644
--- a/tests/phpunit/api/v4/Custom/CustomContactRefTest.php
+++ b/tests/phpunit/api/v4/Custom/CustomContactRefTest.php
@@ -104,7 +104,7 @@ class CustomContactRefTest extends CustomTestBase {
 
     $result = Contact::get(FALSE)
       ->addSelect('id')
-      ->addWhere('MyContactRef.FavPeople.first_name', 'CONTAINS', 'First')
+      ->addWhere('MyContactRef.FavPeople.first_name', 'CONTAINS', 'FirstFav')
       ->execute()
       ->single();
 
@@ -112,7 +112,7 @@ class CustomContactRefTest extends CustomTestBase {
 
     $result = Contact::get(FALSE)
       ->addSelect('id')
-      ->addWhere('MyContactRef.FavPeople.first_name', 'CONTAINS', 'Second')
+      ->addWhere('MyContactRef.FavPeople.first_name', 'CONTAINS', 'SecondFav')
       ->execute();
 
     $this->assertCount(2, $result);
diff --git a/tests/phpunit/api/v4/Entity/GroupTest.php b/tests/phpunit/api/v4/Entity/GroupTest.php
index bfe4ab1c49..9a7730bbcc 100644
--- a/tests/phpunit/api/v4/Entity/GroupTest.php
+++ b/tests/phpunit/api/v4/Entity/GroupTest.php
@@ -84,6 +84,43 @@ class GroupTest extends Api4TestBase {
       ->execute();
   }
 
+  public function testParentsInWhereClause() {
+    // Create 10 groups - at least 1 id will be 2-digit and contain the number 1
+    $groups = $this->saveTestRecords('Group', [
+      'records' => array_fill(0, 10, []),
+    ]);
+
+    $child1 = $this->createTestRecord('Group', [
+      'parents' => [$groups[1]['id'], $groups[2]['id']],
+    ]);
+    $child2 = $this->createTestRecord('Group', [
+      'parents' => [$groups[8]['id']],
+    ]);
+    $child3 = $this->createTestRecord('Group', [
+      'parents' => [$groups[8]['id'], $groups[9]['id']],
+    ]);
+
+    // Check that a digit of e.g. "1" doesn't match a value of e.g. "10"
+    $firstDigit = substr($groups[9]['id'], 0, 1);
+    $found = Group::get(FALSE)
+      ->addWhere('parents', 'CONTAINS', $firstDigit)
+      ->selectRowCount()
+      ->execute();
+    $this->assertCount(0, $found);
+
+    $found = Group::get(FALSE)
+      ->addWhere('parents', 'CONTAINS', $groups[8]['id'])
+      ->selectRowCount()
+      ->execute();
+    $this->assertCount(2, $found);
+
+    $found = Group::get(FALSE)
+      ->addWhere('parents', 'CONTAINS', $groups[9]['id'])
+      ->execute();
+    $this->assertCount(1, $found);
+    $this->assertEquals($child3['id'], $found[0]['id']);
+  }
+
   public function testGetParents() {
     $parent1 = Group::create(FALSE)
       ->addValue('title', uniqid())
-- 
2.25.1