Fix bug whereby sorting by state province gives an error in search builder

diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
index 050beffcff6280ca5c107de1bffbbfc043701098..fc7d9a75e93be4c22ca35a045a6ada966d9e0a78 100644 (file)
--- a/CRM/Contact/BAO/Query.php
+++ b/CRM/Contact/BAO/Query.php
@@ -2680,6 +2680,18 @@ class CRM_Contact_BAO_Query {
         //CRM-14263 further handling of address joins further down...
         return " $side JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id {$limitToPrimaryClause} )";
 
+      case 'civicrm_state_province':
+        // This is encountered when doing an export after having applied a 'sort' - it pretty much implies primary
+        // but that will have been implied-in by the calling function.
+        // test cover in testContactIDQuery
+        return " $side JOIN civicrm_state_province ON ( civicrm_address.state_province_id = civicrm_state_province.id )";
+
+      case 'civicrm_country':
+        // This is encountered when doing an export after having applied a 'sort' - it pretty much implies primary
+        // but that will have been implied-in by the calling function.
+        // test cover in testContactIDQuery
+        return " $side JOIN civicrm_country ON ( civicrm_address.country_id = civicrm_country.id )";
+
       case 'civicrm_phone':
         return " $side JOIN civicrm_phone ON (contact_a.id = civicrm_phone.contact_id {$limitToPrimaryClause}) ";
 
@@ -2699,8 +2711,9 @@ class CRM_Contact_BAO_Query {
         return " $side JOIN civicrm_openid ON ( civicrm_openid.contact_id = contact_a.id {$limitToPrimaryClause} )";
 
       case 'civicrm_worldregion':
-        $from = " $side JOIN civicrm_country ON civicrm_address.country_id = civicrm_country.id ";
-        return "$from $side JOIN civicrm_worldregion ON civicrm_country.region_id = civicrm_worldregion.id ";
+        // We can be sure from the calling function that country will already be joined in.
+        // we really don't need world_region - we could use a pseudoconstant for it.
+        return "$side JOIN civicrm_worldregion ON civicrm_country.region_id = civicrm_worldregion.id ";
 
       case 'civicrm_location_type':
         return " $side JOIN civicrm_location_type ON civicrm_address.location_type_id = civicrm_location_type.id ";
@@ -6320,26 +6333,18 @@ AND   displayRelType.is_active = 1
       $orderByClauseParts = explode(' ', trim($orderByClause));
       $field = $orderByClauseParts[0];
       $direction = isset($orderByClauseParts[1]) ? $orderByClauseParts[1] : 'asc';
+      $fieldSpec = $this->getMetadataForRealField($field);
 
-      switch ($field) {
-        case 'city':
-        case 'postal_code':
-          $this->_tables["civicrm_address"] = $this->_whereTables["civicrm_address"] = 1;
-          $order = str_replace($field, "civicrm_address.{$field}", $order);
-          break;
+      if ($this->_returnProperties === []) {
+        if (!empty($fieldSpec['table_name']) && !isset($this->_tables[$fieldSpec['table_name']])) {
+          $this->_tables[$fieldSpec['table_name']] = 1;
+          $order = $fieldSpec['where'] . ' ' . $direction;
+        }
 
-        case 'country':
-        case 'state_province':
-          $this->_tables["civicrm_{$field}"] = $this->_whereTables["civicrm_{$field}"] = 1;
-          if (is_array($this->_returnProperties) && empty($this->_returnProperties)) {
-            $additionalFromClause .= " LEFT JOIN civicrm_{$field} ON civicrm_{$field}.id = civicrm_address.{$field}_id";
-          }
-          $order = str_replace($field, "civicrm_{$field}.name", $order);
-          break;
+      }
+      switch ($field) {
 
-        case 'email':
-          $this->_tables["civicrm_email"] = $this->_whereTables["civicrm_email"] = 1;
-          $order = str_replace($field, "civicrm_email.{$field}", $order);
+        case 'placeholder-will-remove-next-pr-but-jenkins-will-not-accept-without-and-removing-switch-will-make-hard-to-read':
           break;
 
         default:
@@ -6363,7 +6368,6 @@ AND   displayRelType.is_active = 1
           // is not declared for them.
           // @todo so far only integer fields are being handled. If we add string fields we need to look at
           // escaping.
-          $fieldSpec = $this->getMetadataForRealField($field);
           $pseudoConstantMetadata = CRM_Utils_Array::value('pseudoconstant', $fieldSpec, FALSE);
           if (!empty($pseudoConstantMetadata)
           ) {

diff --git a/CRM/Contact/Selector.php b/CRM/Contact/Selector.php
index 7f3952a1692caad4026095f4cf3e82de871fc60a..fc5bcc33170e882ac959a231f565c212f0fc49d0 100644 (file)
--- a/CRM/Contact/Selector.php
+++ b/CRM/Contact/Selector.php
@@ -500,8 +500,7 @@ class CRM_Contact_Selector extends CRM_Core_Selector_Base implements CRM_Core_Se
               CRM_Core_PseudoConstant::getKey('CRM_Core_DAO_Address', 'location_type_id', $loc)
             )
           );
-          // use field name instead of table alias
-          $prop = $fld;
+
         }
         elseif (isset($this->_query->_fields[$prop]) && isset($this->_query->_fields[$prop]['title'])) {
           $title = $this->_query->_fields[$prop]['title'];

diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php
index 1f94137f69f0f8e2804bad32ef36d3d60bee956f..a387a60777bda33eb9e48ce68c9d474a4de8a9fd 100644 (file)
--- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php
+++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php
@@ -696,6 +696,32 @@ civicrm_relationship.is_active = 1 AND
     $this->fail('Test failed for some reason which is not good');
   }
 
+  /**
+   * Test the sorting on the contact ID query works.
+   *
+   * Checking for lack of fatal.
+   *
+   * @param string $sortOrder
+   *   Param reflecting how sort is passed in.
+   *   - 1_d is column 1 descending.
+   *
+   * @dataProvider getSortOptions
+   */
+  public function testContactIDQuery($sortOrder) {
+    $selector = new CRM_Contact_Selector(NULL, ['radio_ts' => 'ts_all'], NULL, ['sort_name' => 1]);
+    $selector->contactIDQuery([], $sortOrder);
+  }
+
+  public function getSortOptions() {
+    return [
+      ['1_d'],
+      ['2_d'],
+      ['3_d'],
+      ['4_d'],
+      ['5_d'],
+      ['6_d'],
+    ];
+  }
 
   /**
    * Test the summary query does not add an acl clause when acls not enabled..

diff --git a/tests/phpunit/CRM/Contact/SelectorTest.php b/tests/phpunit/CRM/Contact/SelectorTest.php
index 2e3bc97fc8dc3eefb9ec08db26a56f16b341e9bf..1eb5337323e3bb036a26a8834bbff7345e674cff 100644 (file)
--- a/tests/phpunit/CRM/Contact/SelectorTest.php
+++ b/tests/phpunit/CRM/Contact/SelectorTest.php
@@ -550,10 +550,11 @@ class CRM_Contact_SelectorTest extends CiviUnitTestCase {
    */
   public function getDefaultFromString() {
     return ' FROM civicrm_contact contact_a LEFT JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id AND civicrm_address.is_primary = 1 )'
+    . ' LEFT JOIN civicrm_country ON ( civicrm_address.country_id = civicrm_country.id ) '
     . ' LEFT JOIN civicrm_email ON (contact_a.id = civicrm_email.contact_id AND civicrm_email.is_primary = 1)'
     . ' LEFT JOIN civicrm_phone ON (contact_a.id = civicrm_phone.contact_id AND civicrm_phone.is_primary = 1)'
     . ' LEFT JOIN civicrm_im ON (contact_a.id = civicrm_im.contact_id AND civicrm_im.is_primary = 1) '
-    . 'LEFT JOIN civicrm_country ON civicrm_address.country_id = civicrm_country.id LEFT JOIN civicrm_worldregion ON civicrm_country.region_id = civicrm_worldregion.id ';
+    . 'LEFT JOIN civicrm_worldregion ON civicrm_country.region_id = civicrm_worldregion.id ';
   }
 
   /**