dev/core#502 fix bug on sorting by address fields when viewing search results by profile #13884
Conversation
|
(Standard links)
|
| $this->_element["{$tName}_id"] = 1; | ||
| $addressJoin = "\nLEFT JOIN civicrm_address $aName ON ($aName.contact_id = contact_a.id AND $aName.$lCond)"; | ||
| $this->_tables[$tName] = $addressJoin; | ||
| list($aName, $addressJoin) = $this->addAddressTable($name, $lCond); |
There was a problem hiding this comment.
extract this so we can re-use it from orderBy
| FALSE, FALSE | ||
| ); | ||
| return $value; | ||
| return $query->searchQuery(0, 0, $sort); |
There was a problem hiding this comment.
just removing passing of default values in this flow
… results by profile Fixes a bug whereby a fatal error occurs if you attempt to export contacts after sorting by postal_code from an search profile
eileenmcnaughton
force-pushed
the
search_lastish
branch
from
350818b to
708ce91
Compare
| @@ -1014,9 +1014,9 @@ public function addHierarchicalElements() { | |||
|
|
|||
| foreach ($this->_returnProperties['location'] as $name => $elements) { | |||
| $lCond = self::getPrimaryCondition($name); | |||
| $locationTypeId = is_numeric($name) ? NULL : array_search($name, $locationTypes); | |||
There was a problem hiding this comment.
This just moves the line & makes it clearer how it is calculated. I didn't change usage of this var elsewhere in this PR n the end
| if (empty($fieldSpec) && substr($field, 0, 2) === '1-') { | ||
| $fieldSpec = $this->getMetadataForField(substr($field, 2)); | ||
| $this->addAddressTable('1-' . str_replace('civicrm_', '', $fieldSpec['table_name']), 'is_primary = 1'); | ||
| } |
There was a problem hiding this comment.
Have tested the complete changes and confirm that it works fine on profile search through advanced search.
Maybe, if we extend the above condition to also support for primary location type "names" eg Home-, this will also fix the same problem in search builder? @eileenmcnaughton
There was a problem hiding this comment.
@jitendrapurohit yeah - do we still have that problem in search builder or is it fixed now?
I'd rather not fix anything in this area without adding test cover so if we do still have a problem I think that would be better as a follow up
There was a problem hiding this comment.
yeah - do we still have that problem in search builder or is it fixed now?
We do have them, and it exists after this PR too. But can be fixed if the solution for Home- type is extended. There also seems to be other issues with address field search, eg
Advanced search -> select search profile -> select loc type = non primary -> search -> Error.
But I concur it also errors before this PR so these changes are better in than out as it fixes the main issue with profile search sorting 👍
|
thanks @jitendrapurohit I might do one more PR to rip out that redundant switch if you are ok to review it? |
Overview
Fixes a bug whereby a fatal error occurs if you attempt to export contacts after sorting by postal_code from a search profile
Before
Steps to reproduce:
Create a profile with e.g. the postal code field of the primary address
Perform a contact search using this profile for displaying the results
Sort the result by the postal code field
Tick the "all # records" radio button
Choose any action to perform on the result (e.g. "Print PDF document")
After
works
Technical Details
@mattwire @jitendrapurohit I figured I'd fix this related bug before closing out this cleanup / fix up round. I note that there is still some obvious cleanup in the function to be done (consolidate orderBy & orderByArray, get rid of the switch) but I think those are also soooo obvious they could be left until next time someone touches that code
Comments
https://lab.civicrm.org/dev/core/issues/502