APIv4 - Fix CONTAINS operator to work with more types of serialized f…

…ields
civicrm · May 27, 2023 · 8b72ecf · 8b72ecf
1 parent e44931e
commit 8b72ecf
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 4 deletions.
diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php
@@ -592,8 +592,13 @@ protected function createSQLClause($fieldAlias, $operator, $value, $field, int $
       }
       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 @@ protected function createSQLClause($fieldAlias, $operator, $value, $field, int $
 
         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
@@ -64,7 +64,7 @@ public function tearDown(): void {
   /**
    * 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
@@ -104,15 +104,15 @@ public function testGetWithJoin() {
 
     $result = Contact::get(FALSE)
       ->addSelect('id')
-      ->addWhere('MyContactRef.FavPeople.first_name', 'CONTAINS', 'First')
+      ->addWhere('MyContactRef.FavPeople.first_name', 'CONTAINS', 'FirstFav')
       ->execute()
       ->single();
 
     $this->assertEquals($contactId1, $result['id']);
 
     $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
@@ -84,6 +84,43 @@ public function testCreate() {
       ->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())