Search Builder fails with an error when searching for State if the location type differs from the display name.

[kainuk]

Overview

Search Builder fails with an error when searching for State if the location type differs from the display name.

To reproduce

• Goto https://dmaster.demo.civicrm.org/civicrm/admin/locationType?reset=1
• Change the name of Other (nr 4) to other.
• Start the search builder.
• Create the following search
  
  The search ends with a DB Error - Syntax Error

Before

To reproduce

• Goto https://dmaster.demo.civicrm.org/civicrm/admin/locationType?reset=1
• Change the name of Other (nr 4) to other.
• Start the search builder.
• Create the following search
  
  The search ends with a DB Error - Syntax Error

After

The same as above except that the search now gives a result.

Technical Details

The cause of the problem is that locationtypes can be retrieved on two different ways in CiviCRM e.g.
CRM_Core_DAO_Address::buildOptions('location_type_id', 'validate');
and
CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id')

When name and display_name have a different case these functions return different results.

My solution is to replace the second with the first.

A test is added to the PR. It fails with unknown variable. I do not know how to catch it

Comments

Also on gitlab (https://lab.civicrm.org/dev/core/issues/607).
In the test database name and display are the same. The effect is that this kind of bug is not catched. Tip for implementers. Take care that name and display name are always the same.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[mattwire]

The same thing could be achieved with:
CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id', [], 'validate');

I'm not sure which of CRM_Core_PseudoConstant::get and CRM_Core_DAO_Address::buildOptions` is preferred - @eileenmcnaughton ??

In this case, I think you could further simplify this to something like:
CRM_Core_PseudoConstant::getName('CRM_Core_DAO_Address', 'location_type_id', $locType[1]);

[kainuk]

Maybe buildOptions is not the best solution, but I chose it to be the same as the code used on line 2790.

[monishdeb]

@mattwire I think its ok to use buildOptions in here as we are doing in other places too:

CRM//Contact/Selector.php:419:        $locationTypes = CRM_Core_DAO_Address::buildOptions('location_type_id', 'validate');
CRM//Contact/BAO/Query.php:1016:    $locationTypes = CRM_Core_DAO_Address::buildOptions('location_type_id', 'validate');
CRM//Contact/BAO/Query.php:2371:      $locationType = CRM_Core_DAO_Address::buildOptions('location_type_id', 'validate');
CRM//Contact/BAO/Query.php:2790:          $locationTypes = CRM_Core_DAO_Address::buildOptions('location_type_id', 'validate');
CRM//Core/BAO/Mapping.php:1132:    $locationTypes = CRM_Core_DAO_Address::buildOptions('location_type_id', 'validate');

[monishdeb]

The patch successfully fixes the issue. Here's the screencasts:

BEFORE

AFTER

[monishdeb]

Merging on basis of:

• (r-explain) PASS
• (r-user) PASS
• (r-doc) PASS
• (r-run) PASS
• (r-tech) PASS
• (r-code) PASS
• (r-maint) PASS
• (r-test) PASS