dev/core#1714 Fix mis-filtering on activity type

This fixes a bug where the activity_type_id was being compared against the label of the option value

diff --git a/CRM/Activity/BAO/Query.php b/CRM/Activity/BAO/Query.php
index e965b43d48eb82765105800071f9ea7bf6cf3d62..808b672e451a56b3edf9a78b2f831c904479b6db 100644 (file)
--- a/CRM/Activity/BAO/Query.php
+++ b/CRM/Activity/BAO/Query.php
@@ -180,19 +180,16 @@ class CRM_Activity_BAO_Query {
 
     $fields = CRM_Activity_BAO_Activity::exportableFields();
     $fieldSpec = $query->getFieldSpec($name);
+
     $query->_tables['civicrm_activity'] = $query->_whereTables['civicrm_activity'] = 1;
     if ($query->_mode & CRM_Contact_BAO_Query::MODE_ACTIVITY) {
       $query->_skipDeleteClause = TRUE;
     }
-    // @todo we want to do this in a more metadata driven way, and also in contribute.
-    // But for the rc...
-    $namesToConvert = [
-      'activity_status' => 'activity_status_id',
-    ];
-    $name = $namesToConvert[$name] ?? $name;
 
     switch ($name) {
+      case 'activity_type':
       case 'activity_type_id':
+      case 'activity_status':
       case 'activity_status_id':
       case 'activity_engagement_level':
       case 'activity_id':
@@ -201,42 +198,14 @@ class CRM_Activity_BAO_Query {
         // We no longer expect "subject" as a specific criteria (as of CRM-19447),
         // but we still use activity_subject in Activity.Get API
       case 'activity_subject':
-
-        $qillName = $name;
-        if (in_array($name, ['activity_engagement_level', 'activity_id'])) {
-          $name = $qillName = str_replace('activity_', '', $name);
-        }
-        if (in_array($name, [
-          'activity_subject',
-          'activity_priority_id',
-        ])) {
-          $name = str_replace('activity_', '', $name);
-          $qillName = str_replace('_id', '', $qillName);
-        }
-        if ($name == 'activity_campaign_id') {
-          $name = 'campaign_id';
-        }
-
-        $dataType = !empty($fields[$qillName]['type']) ? CRM_Utils_Type::typeToString($fields[$qillName]['type']) : 'String';
-
-        $where = $fieldSpec['where'];
-        if (!$where) {
-          $where = 'civicrm_activity.' . $name;
-        }
-        $query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause($where, $op, $value, $dataType);
-        list($op, $value) = CRM_Contact_BAO_Query::buildQillForFieldValue('CRM_Activity_DAO_Activity', $name, $value, $op);
-        $query->_qill[$grouping][] = ts('%1 %2 %3', [
-          1 => $fields[$qillName]['title'],
-          2 => $op,
-          3 => $value,
-        ]);
+        $query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause($fieldSpec['where'], $op, $value, CRM_Utils_Type::typeToString($fieldSpec['type']));
+        $query->_qill[$grouping][]  = $query->getQillForField($fieldSpec['is_pseudofield_for'] ?? $fieldSpec['name'], $value, $op, $fieldSpec);
         break;
 
       case 'activity_text':
         self::whereClauseSingleActivityText($values, $query);
         break;
 
-      case 'activity_type':
       case 'activity_priority':
         $query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause("$name.label", $op, $value, 'String');
         list($op, $value) = CRM_Contact_BAO_Query::buildQillForFieldValue('CRM_Activity_DAO_Activity', $name, $value, $op);

diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
index 727e5fc2eaa7d80527befc89ece5486a5b76c892..d41bfea47a40b194bc59eb8785489a6b8afe5988 100644 (file)
--- a/CRM/Contact/BAO/Query.php
+++ b/CRM/Contact/BAO/Query.php
@@ -516,8 +516,9 @@ class CRM_Contact_BAO_Query {
       $this->_fields = array_merge($this->_fields, $fields);
 
       // add activity fields
-      $fields = CRM_Activity_BAO_Activity::exportableFields();
-      $this->_fields = array_merge($this->_fields, $fields);
+      $this->_fields = array_merge($this->_fields, CRM_Activity_BAO_Activity::exportableFields());
+      // Add hack as no unique name is defined for the field but the search form is in denial.
+      $this->_fields['activity_priority_id'] = $this->_fields['priority_id'];
 
       // add any fields provided by hook implementers
       $extFields = CRM_Contact_BAO_Query_Hook::singleton()->getFields();

diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php
index ad0e3f54caf29451708760d10282be000ade1378..063074db14aeae257d5243c89b616d9d6c99df9e 100644 (file)
--- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php
+++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php
@@ -437,11 +437,56 @@ class CRM_Contact_BAO_QueryTest extends CiviUnitTestCase {
     ];
   }
 
+  /**
+   * Test similarly handled activity fields qill and where clauses.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testSearchBuilderActivityType() {
+    $queryObj = new CRM_Contact_BAO_Query([['activity_type', '=', '3', 1, 0]]);
+    $this->assertContains('WHERE  (  ( civicrm_activity.activity_type_id = 3 )', $queryObj->getSearchSQL());
+    $this->assertEquals('Activity Type = Email', $queryObj->_qill[1][0]);
+
+    $queryObj = new CRM_Contact_BAO_Query([['activity_type_id', '=', '3', 1, 0]]);
+    $this->assertContains('WHERE  (  ( civicrm_activity.activity_type_id = 3 )', $queryObj->getSearchSQL());
+    $this->assertEquals('Activity Type ID = Email', $queryObj->_qill[1][0]);
+
+    $queryObj = new CRM_Contact_BAO_Query([['activity_status', '=', '3', 1, 0]]);
+    $this->assertContains('WHERE  (  ( civicrm_activity.status_id = 3 )', $queryObj->getSearchSQL());
+    $this->assertEquals('Activity Status = Cancelled', $queryObj->_qill[1][0]);
+
+    $queryObj = new CRM_Contact_BAO_Query([['activity_status_id', '=', '3', 1, 0]]);
+    $this->assertContains('WHERE  (  ( civicrm_activity.status_id = 3 )', $queryObj->getSearchSQL());
+    $this->assertEquals('Activity Status = Cancelled', $queryObj->_qill[1][0]);
+
+    $queryObj = new CRM_Contact_BAO_Query([['activity_engagement_level', '=', '3', 1, 0]]);
+    $this->assertContains('WHERE  (  ( civicrm_activity.engagement_level = 3 )', $queryObj->getSearchSQL());
+    $this->assertEquals('Engagement Index = 3', $queryObj->_qill[1][0]);
+
+    $queryObj = new CRM_Contact_BAO_Query([['activity_id', '=', '3', 1, 0]]);
+    $this->assertContains('WHERE  (  ( civicrm_activity.id = 3 )', $queryObj->getSearchSQL());
+    $this->assertEquals('Activity ID = 3', $queryObj->_qill[1][0]);
+
+    $queryObj = new CRM_Contact_BAO_Query([['activity_campaign_id', '=', '3', 1, 0]]);
+    $this->assertContains('WHERE  (  ( civicrm_activity.campaign_id = 3 )', $queryObj->getSearchSQL());
+    $this->assertEquals('Campaign = 3', $queryObj->_qill[1][0]);
+
+    $queryObj = new CRM_Contact_BAO_Query([['activity_priority_id', '=', '3', 1, 0]]);
+    $this->assertContains('WHERE  (  ( civicrm_activity.priority_id = 3 )', $queryObj->getSearchSQL());
+    $this->assertEquals('Priority = Low', $queryObj->_qill[1][0]);
+
+    $queryObj = new CRM_Contact_BAO_Query([['activity_subject', '=', '3', 1, 0]]);
+    $this->assertContains("WHERE  (  ( civicrm_activity.subject = '3' )", $queryObj->getSearchSQL());
+    $this->assertEquals("Subject = '3'", $queryObj->_qill[1][0]);
+  }
+
   /**
    * Test set up to test calling the query object per GroupContactCache BAO usage.
    *
    * CRM-17254 ensure that if only the contact_id is required other fields should
    * not be appended.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testGroupContactCacheAddSearch() {
     $returnProperties = ['contact_id'];

diff --git a/tests/phpunit/CRM/Contact/Form/Search/SearchContactTest.php b/tests/phpunit/CRM/Contact/Form/Search/SearchContactTest.php
index 6c7a3d2efdf01e3e40430e6db163c3c3cdce8ba7..fcbb20f3adce0bfb5c5922ce61e732470e29582e 100644 (file)
--- a/tests/phpunit/CRM/Contact/Form/Search/SearchContactTest.php
+++ b/tests/phpunit/CRM/Contact/Form/Search/SearchContactTest.php
@@ -19,6 +19,8 @@ class CRM_Contact_Form_Search_SearchContactTest extends CiviUnitTestCase {
 
   /**
    * Test contact sub type search.
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testContactSubtype() {
     foreach (['Contact_sub_type', 'Contact2__sub__type'] as $contactSubType) {
@@ -26,7 +28,7 @@ class CRM_Contact_Form_Search_SearchContactTest extends CiviUnitTestCase {
         'name' => $contactSubType,
         'label' => $contactSubType,
         'is_active' => 1,
-        'parent_id' => "Individual",
+        'parent_id' => 'Individual',
       ]);
       // Contact Type api munge name in create mode
       // Therefore updating the name in update mode
@@ -39,6 +41,11 @@ class CRM_Contact_Form_Search_SearchContactTest extends CiviUnitTestCase {
     $this->searchContacts('Contact2__sub__type');
   }
 
+  /**
+   * @param string $contactSubType
+   *
+   * @throws \CRM_Core_Exception
+   */
   protected function searchContacts($contactSubType) {
     // create contact
     $params = [
@@ -78,11 +85,13 @@ class CRM_Contact_Form_Search_SearchContactTest extends CiviUnitTestCase {
   /**
    * Test to search based on Group type.
    * https://lab.civicrm.org/dev/core/issues/726
+   *
+   * @throws \CRM_Core_Exception
    */
   public function testContactSearchOnGroupType() {
     $groupTypes = $this->callAPISuccess('OptionValue', 'get', [
-      'return' => ["id", "name"],
-      'option_group_id' => "group_type",
+      'return' => ['id', 'name'],
+      'option_group_id' => 'group_type',
     ])['values'];
     $groupTypes = array_column($groupTypes, 'id', 'name');