diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index e4c08ed9b71e..5874379f8a96 100644 --- a/CRM/Contact/BAO/Query.php +++ b/CRM/Contact/BAO/Query.php @@ -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 @@ -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`"; @@ -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; @@ -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); @@ -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"; @@ -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); @@ -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; } @@ -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; + } + } diff --git a/CRM/Contact/Selector.php b/CRM/Contact/Selector.php index 8a92ce1a099b..8cb3ec84e6cb 100644 --- a/CRM/Contact/Selector.php +++ b/CRM/Contact/Selector.php @@ -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. @@ -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') { @@ -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)) { diff --git a/CRM/Export/BAO/Export.php b/CRM/Export/BAO/Export.php index cb0c0c762c4b..22fa793331c2 100644 --- a/CRM/Export/BAO/Export.php +++ b/CRM/Export/BAO/Export.php @@ -721,8 +721,6 @@ public static function exportComponents( } } - $multipleSelectFields = array('preferred_communication_method' => 1); - $addPaymentHeader = FALSE; $paymentDetails = array(); @@ -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 @@ -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; } @@ -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', diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php index 8111339be4b1..8ed611f1cb8a 100644 --- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php +++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php @@ -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, diff --git a/tests/phpunit/api/v3/ContactTest.php b/tests/phpunit/api/v3/ContactTest.php index 15e3be87dbe3..8e9c020395fe 100644 --- a/tests/phpunit/api/v3/ContactTest.php +++ b/tests/phpunit/api/v3/ContactTest.php @@ -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. *