Fix fatal error when sorting by status in activity search

diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php
index a18061ae966734e4bfad4e900a6f9825568063b1..11e8a49c5a2ccb0d72d753f15689fa4b827a9697 100644 (file)
--- a/CRM/Activity/BAO/Activity.php
+++ b/CRM/Activity/BAO/Activity.php
@@ -2119,6 +2119,9 @@ AND cl.modified_id  = c.id
         'type' => CRM_Utils_Type::T_STRING,
       ];
 
+      // @todo - remove these - they are added by CRM_Core_DAO::appendPseudoConstantsToFields
+      // below. That search label stuff is referenced in search builder but is likely just
+      // a hack that duplicates, maybe differently, other functionality.
       $Activityfields = [
         'activity_type' => [
           'title' => ts('Activity Type'),
@@ -2189,7 +2192,7 @@ AND cl.modified_id  = c.id
 
     // add custom data for case activities
     $fields = array_merge($fields, CRM_Core_BAO_CustomField::getFieldsForImport('Activity'));
-
+    CRM_Core_DAO::appendPseudoConstantsToFields($fields);
     self::$_exportableFields[$name] = $fields;
     return self::$_exportableFields[$name];
   }

diff --git a/CRM/Activity/BAO/Query.php b/CRM/Activity/BAO/Query.php
index 6cea3e0f908c3bb0eab14ba899e00c653345f135..548d6cd96c89c66f3e654f84ac4444bebd652eb3 100644 (file)
--- a/CRM/Activity/BAO/Query.php
+++ b/CRM/Activity/BAO/Query.php
@@ -82,13 +82,10 @@ class CRM_Activity_BAO_Query {
     }
 
     if (!empty($query->_returnProperties['activity_status'])) {
-      $query->_select['activity_status'] = 'activity_status.label as activity_status,
-      civicrm_activity.status_id as status_id';
+      $query->_select['activity_status_id'] = 1;
       $query->_element['activity_status'] = 1;
       $query->_tables['civicrm_activity'] = 1;
-      $query->_tables['activity_status'] = 1;
       $query->_whereTables['civicrm_activity'] = 1;
-      $query->_whereTables['activity_status'] = 1;
     }
 
     if (!empty($query->_returnProperties['activity_duration'])) {
@@ -189,15 +186,24 @@ class CRM_Activity_BAO_Query {
    *
    * @param array $values
    * @param CRM_Contact_BAO_Query $query
+   *
+   * @throws \CRM_Core_Exception
    */
   public static function whereClauseSingle(&$values, &$query) {
     list($name, $op, $value, $grouping) = $values;
 
     $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_id':
@@ -215,7 +221,6 @@ class CRM_Activity_BAO_Query {
           $name = $qillName = str_replace('activity_', '', $name);
         }
         if (in_array($name, [
-          'activity_status_id',
           'activity_subject',
           'activity_priority_id',
         ])) {
@@ -228,7 +233,11 @@ class CRM_Activity_BAO_Query {
 
         $dataType = !empty($fields[$qillName]['type']) ? CRM_Utils_Type::typeToString($fields[$qillName]['type']) : 'String';
 
-        $query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause("civicrm_activity.$name", $op, $value, $dataType);
+        $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'],
@@ -242,7 +251,6 @@ class CRM_Activity_BAO_Query {
         break;
 
       case 'activity_type':
-      case 'activity_status':
       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);
@@ -413,12 +421,6 @@ class CRM_Activity_BAO_Query {
                       ON ( civicrm_activity_contact.contact_id = civicrm_contact.id and civicrm_contact.is_deleted != 1 )";
         break;
 
-      case 'activity_status':
-        $from .= " $side JOIN civicrm_option_group option_group_activity_status ON (option_group_activity_status.name = 'activity_status')";
-        $from .= " $side JOIN civicrm_option_value activity_status ON (civicrm_activity.status_id = activity_status.value
-                               AND option_group_activity_status.id = activity_status.option_group_id ) ";
-        break;
-
       case 'activity_type':
         $from .= " $side JOIN civicrm_option_group option_group_activity_type ON (option_group_activity_type.name = 'activity_type')";
         $from .= " $side JOIN civicrm_option_value activity_type ON (civicrm_activity.activity_type_id = activity_type.value

diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
index 71b5dfbb79ae9294ff9d445f5d3a047cae3bd46b..4cfc7b2e84ed7566f656880b5dded591a6cb6637 100644 (file)
--- a/CRM/Contact/BAO/Query.php
+++ b/CRM/Contact/BAO/Query.php
@@ -6934,7 +6934,7 @@ AND   displayRelType.is_active = 1
    *
    * @return array
    */
-  protected function getMetadataForRealField($fieldName) {
+  public function getMetadataForRealField($fieldName) {
     $field = $this->getMetadataForField($fieldName);
     if (!empty($field['is_pseudofield_for'])) {
       $field = $this->getMetadataForField($field['is_pseudofield_for']);
@@ -7031,7 +7031,11 @@ AND   displayRelType.is_active = 1
    */
   public function getFieldSpec($fieldName) {
     if (isset($this->_fields[$fieldName])) {
-      return $this->_fields[$fieldName];
+      $fieldSpec = $this->_fields[$fieldName];
+      if (!empty($fieldSpec['is_pseudofield_for'])) {
+        $fieldSpec = array_merge($this->_fields[$fieldSpec['is_pseudofield_for']], $this->_fields[$fieldName]);
+      }
+      return $fieldSpec;
     }
     $lowFieldName = str_replace('_low', '', $fieldName);
     if (isset($this->_fields[$lowFieldName])) {

diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index c2f024face8a673a2a286b99a5f50a6dc843e786..4f1bad0a36a7c662f4e00ec3b2d0c0bcdb6074eb 100644 (file)
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -2475,7 +2475,7 @@ SELECT contact_id
    * @param array $fields
    */
   public static function appendPseudoConstantsToFields(&$fields) {
-    foreach ($fields as $field) {
+    foreach ($fields as $fieldUniqueName => $field) {
       if (!empty($field['pseudoconstant'])) {
         $pseudoConstant = $field['pseudoconstant'];
         if (!empty($pseudoConstant['optionGroupName'])) {
@@ -2483,7 +2483,7 @@ SELECT contact_id
             'title' => CRM_Core_BAO_OptionGroup::getTitleByName($pseudoConstant['optionGroupName']),
             'name' => $pseudoConstant['optionGroupName'],
             'data_type' => CRM_Utils_Type::T_STRING,
-            'is_pseudofield_for' => $field['name'],
+            'is_pseudofield_for' => $fieldUniqueName,
           ];
         }
         // We restrict to id + name + FK as we are extending this a bit, but cautiously.

diff --git a/tests/phpunit/CRM/Activity/Selector/SearchTest.php b/tests/phpunit/CRM/Activity/Selector/SearchTest.php
index 9370cd56c0073ec5a838a49c7c7a64ba7228949e..c6eec4180d8669a616c3065ac43cbf1e4986cf28 100644 (file)
--- a/tests/phpunit/CRM/Activity/Selector/SearchTest.php
+++ b/tests/phpunit/CRM/Activity/Selector/SearchTest.php
@@ -47,4 +47,42 @@ class CRM_Activity_Selector_SearchTest extends CiviUnitTestCase {
     $this->assertEquals("civicrm_activity.location = 'Baker Street'", $queryObject->_where[''][0]);
   }
 
+  public function testActivityOrderBy() {
+    $sortVars = [
+      1 => [
+        'name' => 'activity_date_time',
+        'sort' => 'activity_date_time',
+        'direction' => 2,
+        'title' => 'Date',
+      ],
+      2 => [
+        'name' => 'activity_type_id',
+        'sort' => 'activity_type_id',
+        'direction' => 4,
+        'title' => 'Type',
+      ],
+      3 => [
+        'name' => 'activity_subject',
+        'sort' => 'activity_subject',
+        'direction' => 4,
+        'title' => 'Subject',
+      ],
+      4 => [
+        'name' => 'source_contact',
+        'sort' => 'source_contact',
+        'direction' => 4,
+        'title' => 'Added By',
+      ],
+      5 => [
+        'name' => 'activity_status',
+        'sort' => 'activity_status',
+        'direction' => 1,
+        'title' => 'Status',
+      ],
+    ];
+    $sort = new CRM_Utils_Sort($sortVars, '5_u');
+    $searchSelector = new CRM_Activity_Selector_Search($queryParams, CRM_Core_Action::VIEW);
+    $searchSelector->getRows(4, 0, 50, $sort);
+  }
+
 }

diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
index fae2b981b9b32665c9e6e3c65ae2e638813d0f9d..505aa775d9e4ec950925a53a6d7144f8e5607e76 100644 (file)
--- a/tests/phpunit/CiviTest/CiviUnitTestCase.php
+++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -1726,7 +1726,11 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase {
     if ($dropCustomValueTables) {
       $optionGroupResult = CRM_Core_DAO::executeQuery('SELECT option_group_id FROM civicrm_custom_field');
       while ($optionGroupResult->fetch()) {
-        if (!empty($optionGroupResult->option_group_id)) {
+        // We have a test that sets the option_group_id for a custom group to that of 'activity_type'.
+        // Then test tearDown deletes it. This is all mildly terrifying but for the context here we can be pretty
+        // sure the low-numbered (50 is arbitrary) option groups are not ones to 'just delete' in a
+        // generic cleanup routine.
+        if (!empty($optionGroupResult->option_group_id) && $optionGroupResult->option_group_id > 50) {
           CRM_Core_DAO::executeQuery('DELETE FROM civicrm_option_group WHERE id = ' . $optionGroupResult->option_group_id);
         }
       }