dev/core#147 Use location type machine name as table alias instead of label #12226
Conversation
CRM/Contact/BAO/Query.php
Outdated
| @@ -2374,7 +2374,7 @@ public static function getLocationTableName(&$where, &$locType) { | |||
| list($tbName, $fldName) = explode(".", $where); | |||
|
|
|||
| //get the location name | |||
| $locationType = CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id'); | |||
| $locationType = CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id', ['labelColumn' => 'name']); | |||
There was a problem hiding this comment.
So the comments on that function are
* Low-level option getter, rarely accessed directly.
* NOTE: Rather than calling this function directly use CRM_*_BAO_*::buildOptions()
* @see http://wiki.civicrm.org/confluence/display/CRMDOC/Pseudoconstant+%28option+list%29+Reference
I think getName makes more sense where we can - or buildOptions if we really need to get all - has a few different contexts
|
@monishdeb I'm confused about this patch. Will this result in the user-facing options on the search form using name instead of label? |
|
@colemanw no, the user-facing label will still be same (not name). But the backend query fragments will use name as table alias and this patch ensures the same across different places. |
CRM/Contact/Selector.php
Outdated
| @@ -492,7 +492,15 @@ public function &getColumnHeaders($action = NULL, $output = NULL) { | |||
| if (trim($phoneType) && !is_numeric($phoneType) && strtolower($phoneType) != $fld) { | |||
| $title .= "-{$phoneType}"; | |||
| } | |||
| $title .= " ($loc)"; | |||
| $title .= sprintf("(%s)", | |||
There was a problem hiding this comment.
Can we have a comment in the code as to why you are doing this?
| @@ -492,7 +492,16 @@ public function &getColumnHeaders($action = NULL, $output = NULL) { | |||
| if (trim($phoneType) && !is_numeric($phoneType) && strtolower($phoneType) != $fld) { | |||
| $title .= "-{$phoneType}"; | |||
| } | |||
| $title .= " ($loc)"; | |||
| // fetch Location type label from name as $loc, which will be later used in column header | |||
There was a problem hiding this comment.
@eileenmcnaughton added a comment here :)
There was a problem hiding this comment.
Is there a reason why you removed the whitespace at the beginning?
|
Ok I think this is safe given code review from multiple people and new unit tests. |
|
Merging as per the tag |
Overview
To reproduce, change Display Name of a location type to some Non-ASCII text (or just install civicrm on some Non-ASCII language, in my case Russian). Then go to Search Builder and try to search contacts with phone, or email, or address of that location type.
Before
After
Technical Details
Earlier we use non-ASCII character in location type labels, then this causes a validation issue (which is CORRECT). As per the fix, I have changed the logic to use machine name later used as a table alias + from UI end we already got a form validation in place which prevents a user to from entering a non-alphanumeric character for m/c name so there is no way we could use a non ASCII character in m/c name.