Skip to content

Commit

Permalink
Merge pull request #9885 from eileenmcnaughton/search
Browse files Browse the repository at this point in the history 
CRM-19815, CRM-19830 make pseudoconstant handling more generic in order to improve performance
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton authored Feb 24, 2017
2 parents 826e8a7 + 86ab13b commit bf3a316
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 88 deletions.
149 changes: 101 additions & 48 deletions CRM/Contact/BAO/Query.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,15 +698,12 @@ public function selectClause($apiEntity = NULL) {
}
}

if (in_array($name, array('prefix_id', 'suffix_id', 'gender_id', 'communication_style_id'))) {
if (CRM_Utils_Array::value($field['pseudoconstant']['optionGroupName'], $this->_returnProperties)) {
$makeException = TRUE;
}
}

$cfID = CRM_Core_BAO_CustomField::getKeyID($name);
if (!empty($this->_paramLookup[$name]) || !empty($this->_returnProperties[$name]) ||
$makeException
if (
!empty($this->_paramLookup[$name])
|| !empty($this->_returnProperties[$name])
|| $this->pseudoConstantNameIsInReturnProperties($field)
|| $makeException
) {
if ($cfID) {
// add to cfIDs array if not present
Expand Down Expand Up @@ -826,42 +823,12 @@ public function selectClause($apiEntity = NULL) {
$this->_element['provider_id'] = 1;
}

if ($tName == 'contact') {
if ($tName == 'contact' && $fieldName == 'organization_name') {
// special case, when current employer is set for Individual contact
if ($fieldName == 'organization_name') {
$this->_select[$name] = "IF ( contact_a.contact_type = 'Individual', NULL, contact_a.organization_name ) as organization_name";
}
elseif ($fieldName != 'id') {
if ($fieldName == 'prefix_id') {
$this->_pseudoConstantsSelect['individual_prefix'] = array(
'pseudoField' => 'prefix_id',
'idCol' => "prefix_id",
'bao' => 'CRM_Contact_BAO_Contact',
);
}
if ($fieldName == 'suffix_id') {
$this->_pseudoConstantsSelect['individual_suffix'] = array(
'pseudoField' => 'suffix_id',
'idCol' => "suffix_id",
'bao' => 'CRM_Contact_BAO_Contact',
);
}
if ($fieldName == 'gender_id') {
$this->_pseudoConstantsSelect['gender'] = array(
'pseudoField' => 'gender_id',
'idCol' => "gender_id",
'bao' => 'CRM_Contact_BAO_Contact',
);
}
if ($name == 'communication_style_id') {
$this->_pseudoConstantsSelect['communication_style'] = array(
'pseudoField' => 'communication_style_id',
'idCol' => "communication_style_id",
'bao' => 'CRM_Contact_BAO_Contact',
);
}
$this->_select[$name] = "contact_a.{$fieldName} as `$name`";
}
$this->_select[$name] = "IF ( contact_a.contact_type = 'Individual', NULL, contact_a.organization_name ) as organization_name";
}
elseif ($tName == 'contact' && $fieldName === 'id') {
// Handled elsewhere, explicitly ignore. Possibly for all tables...
}
elseif (in_array($tName, array('country', 'county'))) {
$this->_pseudoConstantsSelect[$name]['select'] = "{$field['where']} as `$name`";
Expand All @@ -877,7 +844,22 @@ public function selectClause($apiEntity = NULL) {
}
}
else {
$this->_select[$name] = "{$field['where']} as `$name`";
// If we have an option group defined then rather than joining the option value table in
// (which is an unindexed join) we render the option value on output.
// @todo - extend this to other pseudoconstants.
if ($this->pseudoConstantNameIsInReturnProperties($field)) {
$pseudoFieldName = $field['pseudoconstant']['optionGroupName'];
$this->_pseudoConstantsSelect[$pseudoFieldName] = array(
'pseudoField' => $field['name'],
'idCol' => $field['name'],
'field_name' => $field['name'],
'bao' => $field['bao'],
'pseudoconstant' => $field['pseudoconstant'],
);
$this->_tables[$tableName] = 1;
$this->_element[$pseudoFieldName] = 1;
}
$this->_select[$name] = str_replace('civicrm_contact.', 'contact_a.', "{$field['where']} as `$name`");
}
if (!in_array($tName, array('state_province', 'country', 'county'))) {
$this->_element[$name] = 1;
Expand Down Expand Up @@ -4400,7 +4382,7 @@ public static function apiQuery(
return array($noRows, NULL);
}
$val = $query->store($dao);
$convertedVals = $query->convertToPseudoNames($dao, TRUE);
$convertedVals = $query->convertToPseudoNames($dao, TRUE, TRUE);

if (!empty($convertedVals)) {
$val = array_replace_recursive($val, $convertedVals);
Expand Down Expand Up @@ -5829,12 +5811,40 @@ public function convertToPseudoNames(&$dao, $return = FALSE, $usedForAPI = FALSE
$val = $dao->{$value['idCol']};
if ($key == 'groups') {
$dao->groups = $this->convertGroupIDStringToLabelString($dao, $val);
return;
continue;
}

if (CRM_Utils_System::isNull($val)) {
$dao->$key = NULL;
}
elseif (!empty($value['pseudoconstant'])) {
// If pseudoconstant is set that is kind of defacto for 'we have a bit more info about this'
// and we can use the metadata to figure it out.
// ideally this bit of IF will absorb & replace all the rest in time as we move to
// more metadata based choices.
if (strpos($val, CRM_Core_DAO::VALUE_SEPARATOR) !== FALSE) {
$dbValues = explode(CRM_Core_DAO::VALUE_SEPARATOR, trim($val, CRM_Core_DAO::VALUE_SEPARATOR));
foreach ($dbValues as $pseudoValue) {
$convertedValues[] = CRM_Core_PseudoConstant::getLabel($value['bao'], $value['idCol'], $pseudoValue);
}

$dao->$key = ($usedForAPI) ? $convertedValues : implode(', ', $convertedValues);
$realFieldName = CRM_Utils_Array::value('field_name', $this->_pseudoConstantsSelect[$key]);
if ($usedForAPI && $realFieldName) {
// normally we would see 2 fields returned for pseudoConstants. An exception is
// preferred_communication_method where there is no id-variant.
// For the api we prioritise getting the real data returned.
// over the resolved version
$dao->$realFieldName = $dbValues;
}

}
else {
// This is basically the same as the default but since we have the bao we can use
// a cached function.
$dao->$key = CRM_Core_PseudoConstant::getLabel($value['bao'], $value['idCol'], $val);
}
}
elseif ($baoName = CRM_Utils_Array::value('bao', $value, NULL)) {
//preserve id value
$idColumn = "{$key}_id";
Expand All @@ -5850,8 +5860,7 @@ public function convertToPseudoNames(&$dao, $return = FALSE, $usedForAPI = FALSE
elseif ($value['pseudoField'] == 'state_province_abbreviation') {
$dao->$key = CRM_Core_PseudoConstant::stateProvinceAbbreviation($val);
}
// FIX ME: we should potentially move this to component Query and write a wrapper function that
// handles pseudoconstant fixes for all component
// @todo handle this in the section above for pseudoconstants.
elseif (in_array($value['pseudoField'], array('participant_role_id', 'participant_role'))) {
// @todo define bao on this & merge into the above condition.
$viewValues = explode(CRM_Core_DAO::VALUE_SEPARATOR, $val);
Expand Down Expand Up @@ -5889,6 +5898,21 @@ public function convertToPseudoNames(&$dao, $return = FALSE, $usedForAPI = FALSE
}
}
}
if (!$usedForAPI) {
foreach (array(
'gender_id' => 'gender',
'prefix_id' => 'individual_prefix',
'suffix_id' => 'individual_suffix',
'communication_style_id' => 'communication_style',
) as $realField => $labelField) {
// This is a temporary routine for handling these fields while
// we figure out how to handled them based on metadata in
/// export and search builder. CRM-19815, CRM-19830.
if (isset($dao->$realField) && is_numeric($dao->$realField)) {
$dao->$realField = $dao->$labelField;
}
}
}
return $values;
}

Expand Down Expand Up @@ -6273,4 +6297,33 @@ public function setQillAndWhere($name, $op, $value, $grouping, $field) {
));
}

/**
* Has the pseudoconstant of the field been requested.
*
* For example if the field is payment_instrument_id then it
* has been requested if either payment_instrument_id or payment_instrument
* have been requested. Payment_instrument is the option groun name field value.
*
* @param array $field
*
* @return bool
*/
private function pseudoConstantNameIsInReturnProperties($field) {
if (!isset($field['pseudoconstant']['optionGroupName'])) {
return FALSE;
}
if (empty($field['bao']) || $field['bao'] != 'CRM_Contact_BAO_Contact') {
// For now....
return FALSE;
}

if (CRM_Utils_Array::value($field['pseudoconstant']['optionGroupName'], $this->_returnProperties)) {
return TRUE;
}
if (CRM_Utils_Array::value($field['name'], $this->_returnProperties)) {
return TRUE;
}
return FALSE;
}

}
14 changes: 0 additions & 14 deletions CRM/Contact/Selector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,6 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
$names = self::$_properties;
}

$multipleSelectFields = array('preferred_communication_method' => 1);

$links = self::links($this->_context, $this->_contextMenu, $this->_key);

//check explicitly added contact to a Smart Group.
Expand All @@ -656,7 +654,6 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
while ($result->fetch()) {
$row = array();
$this->_query->convertToPseudoNames($result);

// the columns we are interested in
foreach ($names as $property) {
if ($property == 'status') {
Expand All @@ -669,17 +666,6 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) {
$result->contact_id
);
}
elseif (
$multipleSelectFields &&
array_key_exists($property, $multipleSelectFields)
) {
$key = $property;
$paramsNew = array($key => $result->$property);
$name = array($key => array('newName' => $key, 'groupName' => $key));

CRM_Core_OptionGroup::lookupValues($paramsNew, $name, FALSE);
$row[$key] = $paramsNew[$key];
}
elseif (strpos($property, '-im')) {
$row[$property] = $result->$property;
if (!empty($result->$property)) {
Expand Down
25 changes: 0 additions & 25 deletions CRM/Export/BAO/Export.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,8 +721,6 @@ public static function exportComponents(
}
}

$multipleSelectFields = array('preferred_communication_method' => 1);

$addPaymentHeader = FALSE;

$paymentDetails = array();
Expand Down Expand Up @@ -790,7 +788,6 @@ public static function exportComponents(
$query->convertToPseudoNames($iterationDAO);

//first loop through output columns so that we return what is required, and in same order.
$relationshipField = 0;
foreach ($outputColumns as $field => $value) {

// add im_provider to $dao object
Expand Down Expand Up @@ -868,12 +865,6 @@ public static function exportComponents(
}
$field = $field . '_';

if (array_key_exists($relationField, $multipleSelectFields)) {
$param = array($relationField => $fieldValue);
$names = array($relationField => array('newName' => $relationField, 'groupName' => $relationField));
CRM_Core_OptionGroup::lookupValues($param, $names, FALSE);
$fieldValue = $param[$relationField];
}
if (is_object($relDAO) && $relationField == 'id') {
$row[$field . $relationField] = $relDAO->contact_id;
}
Expand Down Expand Up @@ -949,22 +940,6 @@ public static function exportComponents(
if ($cfID = CRM_Core_BAO_CustomField::getKeyID($field)) {
$row[$field] = CRM_Core_BAO_CustomField::displayValue($fieldValue, $cfID);
}
elseif (array_key_exists($field, $multipleSelectFields)) {
//option group fixes
$paramsNew = array($field => $fieldValue);
if ($field == 'test_tutoring') {
$name = array($field => array('newName' => $field, 'groupName' => 'test'));
// for readers group
}
elseif (substr($field, 0, 4) == 'cmr_') {
$name = array($field => array('newName' => $field, 'groupName' => substr($field, 0, -3)));
}
else {
$name = array($field => array('newName' => $field, 'groupName' => $field));
}
CRM_Core_OptionGroup::lookupValues($paramsNew, $name, FALSE);
$row[$field] = $paramsNew[$field];
}

elseif (in_array($field, array(
'email_greeting',
Expand Down
2 changes: 1 addition & 1 deletion tests/phpunit/CRM/Contact/BAO/QueryTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public function testSearchProfilePrimaryCityCRM14263() {
'contact_sub_type' => 1,
'sort_name' => 1,
);
$expectedSQL = "SELECT contact_a.id as contact_id, contact_a.contact_type as `contact_type`, contact_a.contact_sub_type as `contact_sub_type`, contact_a.sort_name as `sort_name`, civicrm_address.id as address_id, civicrm_address.city as `city` FROM civicrm_contact contact_a LEFT JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id AND civicrm_address.is_primary = 1 ) WHERE ( ( LOWER(civicrm_address.city) = 'cool city' ) ) AND (contact_a.is_deleted = 0) ORDER BY `contact_a`.`sort_name` asc, `contact_a`.`id` ";
$expectedSQL = "SELECT contact_a.id as contact_id, contact_a.contact_type as `contact_type`, contact_a.contact_sub_type as `contact_sub_type`, contact_a.sort_name as `sort_name`, civicrm_address.id as address_id, civicrm_address.city as `city` FROM civicrm_contact contact_a LEFT JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id AND civicrm_address.is_primary = 1 ) WHERE ( ( LOWER(civicrm_address.city) = 'cool city' ) ) AND (contact_a.is_deleted = 0) ORDER BY `contact_a`.`sort_name` asc, `contact_a`.`id` ";
$queryObj = new CRM_Contact_BAO_Query($params, $returnProperties);
try {
$this->assertEquals($expectedSQL, $queryObj->searchQuery(0, 0, NULL,
Expand Down
34 changes: 34 additions & 0 deletions tests/phpunit/api/v3/ContactTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1646,6 +1646,40 @@ public function testContactGetEmail() {
$this->callAPISuccess('contact', 'delete', $contact);
}

/**
* Ensure consistent return format for option group fields.
*/
public function testPseudoFields() {
$params = array(
'preferred_communication_method' => array('Phone', 'SMS'),
'preferred_language' => 'en_US',
'gender_id' => 'Female',
'prefix_id' => 'Mrs.',
'suffix_id' => 'II',
'communication_style_id' => 'Formal',
);

$contact = $this->callAPISuccess('contact', 'create', array_merge($this->_params, $params));

$result = $this->callAPISuccess('contact', 'getsingle', array('id' => $contact['id']));
$this->assertEquals('Both', $result['preferred_mail_format']);

$this->assertEquals('en_US', $result['preferred_language']);
$this->assertEquals(1, $result['communication_style_id']);
$this->assertEquals(1, $result['gender_id']);
$this->assertEquals('Female', $result['gender']);
$this->assertEquals('Mrs.', $result['individual_prefix']);
$this->assertEquals(1, $result['prefix_id']);
$this->assertEquals('II', $result['individual_suffix']);
$this->assertEquals(CRM_Core_PseudoConstant::getKey("CRM_Contact_BAO_Contact", 'suffix_id', 'II'), $result['suffix_id']);
$this->callAPISuccess('contact', 'delete', $contact);
$this->assertEquals(array(
CRM_Core_PseudoConstant::getKey("CRM_Contact_BAO_Contact", 'preferred_communication_method', 'Phone'),
CRM_Core_PseudoConstant::getKey("CRM_Contact_BAO_Contact", 'preferred_communication_method', 'SMS'),
), $result['preferred_communication_method']);
}


/**
* Test birth date parameters.
*
Expand Down

0 comments on commit bf3a316

Please sign in to comment.