diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index 009b99186675..f803f567153a 100644 --- a/CRM/Contact/BAO/Query.php +++ b/CRM/Contact/BAO/Query.php @@ -653,8 +653,7 @@ public function selectClause($apiEntity = NULL) { if ( (substr($name, 0, 12) == 'participant_') || (substr($name, 0, 7) == 'pledge_') || - (substr($name, 0, 5) == 'case_') || - (substr($name, 0, 8) == 'payment_') + (substr($name, 0, 5) == 'case_') ) { continue; } @@ -6203,10 +6202,17 @@ protected function prepareOrderBy($sort, $sortByChar, $sortOrder, $additionalFro $order = trim(str_replace('ORDER BY', '', $order)); // hack for order clause - $fieldOrder = explode(' ', $order); - $field = $fieldOrder[0]; + if (!empty($orderByArray)) { + $order = implode(', ', $orderByArray); + } + else { + $orderByArray = explode(',', $order); + } + foreach ($orderByArray as $orderByClause) { + $orderByClauseParts = explode(' ', trim($orderByClause)); + $field = $orderByClauseParts[0]; + $direction = isset($orderByClauseParts[1]) ? $orderByClauseParts[1] : 'asc'; - if ($field) { switch ($field) { case 'city': case 'postal_code': @@ -6229,25 +6235,47 @@ protected function prepareOrderBy($sort, $sortByChar, $sortOrder, $additionalFro break; default: - //CRM-12565 add "`" around $field if it is a pseudo constant - foreach ($this->_pseudoConstantsSelect as $key => $value) { - if (!empty($value['element']) && $value['element'] == $field) { + foreach ($this->_pseudoConstantsSelect as $key => $pseudoConstantMetadata) { + // By replacing the join to the option value table with the mysql construct + // ORDER BY field('contribution_status_id', 2,1,4) + // we can remove a join. In the case of the option value join it is + /// a join known to cause slow queries. + // @todo cover other pseudoconstant types. Limited to option group ones in the + // first instance for scope reasons. They require slightly different handling as the column (label) + // is not declared for them. + // @todo so far only integer fields are being handled. If we add string fields we need to look at + // escaping. + if (isset($pseudoConstantMetadata['pseudoconstant']) + && isset($pseudoConstantMetadata['pseudoconstant']['optionGroupName']) + && $field === CRM_Utils_Array::value('optionGroupName', $pseudoConstantMetadata['pseudoconstant']) + ) { + $sortedOptions = $pseudoConstantMetadata['bao']::buildOptions($pseudoConstantMetadata['pseudoField'], NULL, array( + 'orderColumn' => 'label', + )); + $order = str_replace("$field $direction", "field({$pseudoConstantMetadata['pseudoField']}," . implode(',', array_keys($sortedOptions)) . ") $direction", $order); + } + //CRM-12565 add "`" around $field if it is a pseudo constant + // This appears to be for 'special' fields like locations with appended numbers or hyphens .. maybe. + if (!empty($pseudoConstantMetadata['element']) && $pseudoConstantMetadata['element'] == $field) { $order = str_replace($field, "`{$field}`", $order); } } } - $this->_fromClause = self::fromClause($this->_tables, NULL, NULL, $this->_primaryLocation, $this->_mode); - $this->_simpleFromClause = self::fromClause($this->_whereTables, NULL, NULL, $this->_primaryLocation, $this->_mode); } + $this->_fromClause = self::fromClause($this->_tables, NULL, NULL, $this->_primaryLocation, $this->_mode); + $this->_simpleFromClause = self::fromClause($this->_whereTables, NULL, NULL, $this->_primaryLocation, $this->_mode); + // The above code relies on crazy brittle string manipulation of a peculiarly-encoded ORDER BY // clause. But this magic helper which forgivingly reescapes ORDER BY. // Note: $sortByChar implies that $order was hard-coded/trusted, so it can do funky things. - if ($order && !$sortByChar) { + if ($sortByChar) { + return array(' ORDER BY ' . $order, $additionalFromClause); + } + if ($order) { $order = CRM_Utils_Type::escape($order, 'MysqlOrderBy'); return array(' ORDER BY ' . $order, $additionalFromClause); } - return array($order, $additionalFromClause); } /** @@ -6312,10 +6340,6 @@ 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; diff --git a/CRM/Contact/Selector.php b/CRM/Contact/Selector.php index 8cb3ec84e6cb..9d1ceb8f69c0 100644 --- a/CRM/Contact/Selector.php +++ b/CRM/Contact/Selector.php @@ -488,6 +488,9 @@ public function &getColumnHeaders($action = NULL, $output = NULL) { elseif (isset($this->_query->_fields[$prop]) && isset($this->_query->_fields[$prop]['title'])) { $title = $this->_query->_fields[$prop]['title']; } + elseif (isset($this->_query->_pseudoConstantsSelect[$prop]) && isset($this->_query->_pseudoConstantsSelect[$prop]['pseudoconstant']['optionGroupName'])) { + $title = CRM_Core_BAO_OptionGroup::getTitleByName($this->_query->_pseudoConstantsSelect[$prop]['pseudoconstant']['optionGroupName']); + } else { $title = ''; } diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index ab6c479f012d..d76de521415a 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -780,6 +780,15 @@ public static function &importableFields($contactType = 'Individual', $status = return self::$_importableFields; } + /** + * Get exportable fields with pseudoconstants rendered as an extra field. + */ + public static function getExportableFieldsWithPseudoConstants() { + $fields = self::exportableFields(); + CRM_Core_DAO::appendPseudoConstantsToFields($fields); + return $fields; + } + /** * Combine all the exportable fields from the lower level objects. * @@ -799,14 +808,6 @@ public static function &exportableFields($checkPermission = TRUE) { $expFieldsContrib = CRM_Contribute_DAO_ContributionProduct::export(); $typeField = CRM_Financial_DAO_FinancialType::export(); $financialAccount = CRM_Financial_DAO_FinancialAccount::export(); - $optionField = CRM_Core_OptionValue::getFields($mode = 'contribute'); - $contributionStatus = array( - 'contribution_status' => array( - 'title' => ts('Contribution Status'), - 'name' => 'contribution_status', - 'data_type' => CRM_Utils_Type::T_STRING, - ), - ); $contributionPage = array( 'contribution_page' => array( @@ -883,7 +884,7 @@ public static function &exportableFields($checkPermission = TRUE) { ), ); - $fields = array_merge($impFields, $typeField, $contributionStatus, $contributionPage, $optionField, $expFieldProduct, + $fields = array_merge($impFields, $typeField, $contributionPage, $expFieldProduct, $expFieldsContrib, $contributionNote, $extraFields, $softCreditFields, $financialAccount, $premiums, $campaignTitle, CRM_Core_BAO_CustomField::getFieldsForImport('Contribution', FALSE, FALSE, FALSE, $checkPermission) ); @@ -1137,6 +1138,8 @@ public static function addPremium(&$params) { */ public static function getContributionFields($addExtraFields = TRUE) { $contributionFields = CRM_Contribute_DAO_Contribution::export(); + // @todo remove this - this line was added because payment_instrument_id was not + // set to exportable - but now it is. $contributionFields = array_merge($contributionFields, CRM_Core_OptionValue::getFields($mode = 'contribute')); if ($addExtraFields) { @@ -3702,6 +3705,9 @@ public static function deleteContactContribution($contactId) { public static function buildOptions($fieldName, $context = NULL, $props = array()) { $className = __CLASS__; $params = array(); + if (isset($props['orderColumn'])) { + $params['orderColumn'] = $props['orderColumn']; + } switch ($fieldName) { // This field is not part of this object but the api supports it case 'payment_processor': diff --git a/CRM/Contribute/BAO/Query.php b/CRM/Contribute/BAO/Query.php index f7e4f5d06c8b..e31a87f67da9 100644 --- a/CRM/Contribute/BAO/Query.php +++ b/CRM/Contribute/BAO/Query.php @@ -97,39 +97,6 @@ public static function select(&$query) { $query->_tables['contribution_batch'] = 1; } - // get contribution_status - if (!empty($query->_returnProperties['contribution_status_id'])) { - $query->_select['contribution_status_id'] = "contribution_status.value as contribution_status_id"; - $query->_element['contribution_status_id'] = 1; - $query->_tables['civicrm_contribution'] = 1; - $query->_tables['contribution_status'] = 1; - } - - // get contribution_status label - if (!empty($query->_returnProperties['contribution_status'])) { - $query->_select['contribution_status'] = "contribution_status.label as contribution_status"; - $query->_element['contribution_status'] = 1; - $query->_tables['civicrm_contribution'] = 1; - $query->_tables['contribution_status'] = 1; - } - - // get payment instrument - if (!empty($query->_returnProperties['payment_instrument'])) { - $query->_select['payment_instrument'] = "contribution_payment_instrument.label as payment_instrument"; - $query->_element['payment_instrument'] = 1; - $query->_tables['civicrm_contribution'] = 1; - $query->_tables['contribution_payment_instrument'] = 1; - } - - // get payment instrument id - if (!empty($query->_returnProperties['payment_instrument_id'])) { - $query->_select['instrument_id'] = "contribution_payment_instrument.value as instrument_id"; - $query->_select['payment_instrument_id'] = "contribution_payment_instrument.value as payment_instrument_id"; - $query->_element['instrument_id'] = $query->_element['payment_instrument_id'] = 1; - $query->_tables['civicrm_contribution'] = 1; - $query->_tables['contribution_payment_instrument'] = 1; - } - if (!empty($query->_returnProperties['contribution_campaign_title'])) { $query->_select['contribution_campaign_title'] = "civicrm_campaign.title as contribution_campaign_title"; $query->_element['contribution_campaign_title'] = $query->_tables['civicrm_campaign'] = 1; @@ -181,7 +148,7 @@ public static function whereClauseSingle(&$values, &$query) { list($name, $op, $value, $grouping, $wildcard) = $values; $quoteValue = NULL; - $fields = array_merge(CRM_Contribute_BAO_Contribution::fields(), self::getFields()); + $fields = self::getFields(); if (!empty($value) && !is_array($value)) { $quoteValue = "\"$value\""; @@ -578,18 +545,6 @@ public static function from($name, $mode, $side) { $from .= " $side JOIN civicrm_product ON civicrm_contribution_product.product_id =civicrm_product.id "; break; - case 'contribution_payment_instrument': - $from = " $side JOIN civicrm_option_group option_group_payment_instrument ON ( option_group_payment_instrument.name = 'payment_instrument')"; - $from .= " $side JOIN civicrm_option_value contribution_payment_instrument ON (civicrm_contribution.payment_instrument_id = contribution_payment_instrument.value - AND option_group_payment_instrument.id = contribution_payment_instrument.option_group_id ) "; - break; - - case 'contribution_status': - $from = " $side JOIN civicrm_option_group option_group_contribution_status ON (option_group_contribution_status.name = 'contribution_status')"; - $from .= " $side JOIN civicrm_option_value contribution_status ON (civicrm_contribution.contribution_status_id = contribution_status.value - AND option_group_contribution_status.id = contribution_status.option_group_id ) "; - break; - case 'contribution_softcredit_type': $from = " $side JOIN civicrm_option_group option_group_contribution_softcredit_type ON (option_group_contribution_softcredit_type.name = 'soft_credit_type')"; diff --git a/CRM/Contribute/DAO/Contribution.php b/CRM/Contribute/DAO/Contribution.php index 0a1a2f3eb3e2..13d752e08122 100644 --- a/CRM/Contribute/DAO/Contribution.php +++ b/CRM/Contribute/DAO/Contribution.php @@ -30,7 +30,7 @@ * * Generated from xml/schema/CRM/Contribute/Contribution.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:b25399b3b6cf17e97809102666de8ee7) + * (GenCodeChecksum:3e387ab5c998da1bbb367ed526e84289) */ require_once 'CRM/Core/DAO.php'; require_once 'CRM/Utils/Type.php'; @@ -334,8 +334,12 @@ static function &fields() { 'payment_instrument_id' => array( 'name' => 'payment_instrument_id', 'type' => CRM_Utils_Type::T_INT, - 'title' => ts('Payment Method') , + 'title' => ts('Payment Method ID') , 'description' => 'FK to Payment Instrument', + 'export' => true, + 'where' => 'civicrm_contribution.payment_instrument_id', + 'headerPattern' => '/^payment|(p(ayment\s)?instrument)$/i', + 'dataPattern' => '', 'table_name' => 'civicrm_contribution', 'entity' => 'Contribution', 'bao' => 'CRM_Contribute_BAO_Contribution', diff --git a/CRM/Contribute/Selector/Search.php b/CRM/Contribute/Selector/Search.php index 23181c272cef..81beacae2857 100644 --- a/CRM/Contribute/Selector/Search.php +++ b/CRM/Contribute/Selector/Search.php @@ -366,6 +366,7 @@ public function &getRows($action, $offset, $rowCount, $sort, $output = NULL) { $allCampaigns = CRM_Campaign_BAO_Campaign::getCampaigns(NULL, NULL, FALSE, FALSE, FALSE, TRUE); while ($result->fetch()) { + $this->_query->convertToPseudoNames($result); $links = self::links($componentId, $componentAction, $qfKey, diff --git a/CRM/Core/BAO/Mapping.php b/CRM/Core/BAO/Mapping.php index 4b0d9106ed24..d45ef17529c9 100644 --- a/CRM/Core/BAO/Mapping.php +++ b/CRM/Core/BAO/Mapping.php @@ -381,7 +381,7 @@ public static function buildMappingForm(&$form, $mappingType, $mappingId, $colum if (($mappingType == 'Search Builder') || ($exportMode == CRM_Export_Form_Select::CONTRIBUTE_EXPORT)) { if (CRM_Core_Permission::access('CiviContribute')) { - $fields['Contribution'] = CRM_Contribute_BAO_Contribution::exportableFields(); + $fields['Contribution'] = CRM_Contribute_BAO_Contribution::getExportableFieldsWithPseudoConstants(); unset($fields['Contribution']['contribution_contact_id']); $compArray['Contribution'] = ts('Contribution'); } diff --git a/CRM/Core/BAO/OptionGroup.php b/CRM/Core/BAO/OptionGroup.php index 7a6931400c53..d0f0afc71b45 100644 --- a/CRM/Core/BAO/OptionGroup.php +++ b/CRM/Core/BAO/OptionGroup.php @@ -184,4 +184,39 @@ public static function ensureOptionGroupExists($params) { } } + /** + * Get the title of an option group by name. + * + * @param string $name + * The name value for the option group table. + * + * @return string + * The relevant title. + */ + public static function getTitleByName($name) { + $groups = self::getTitlesByNames(); + return $groups[$name]; + } + + /** + * Get a cached mapping of all group titles indexed by their unique name. + * + * We tend to only have a limited number of option groups so memory caching + * makes more sense than multiple look-ups. + * + * @return array + * Array of all group titles by name. + * e.g + * array('activity_status' => 'Activity Status', 'msg_mode' => 'Message Mode'....) + */ + public static function getTitlesByNames() { + if (!isset(\Civi::$statics[__CLASS__]) || !isset(\Civi::$statics[__CLASS__]['titles_by_name'])) { + $dao = CRM_Core_DAO::executeQuery("SELECT name, title FROM civicrm_option_group"); + while ($dao->fetch()) { + \Civi::$statics[__CLASS__]['titles_by_name'][$dao->name] = $dao->title; + } + } + return \Civi::$statics[__CLASS__]['titles_by_name']; + } + } diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php index 2c4d8992b7a1..5917cd11be24 100644 --- a/CRM/Core/DAO.php +++ b/CRM/Core/DAO.php @@ -2154,6 +2154,30 @@ public static function getGlobalSetting($name, $default = NULL) { } } + + /** + * Update the fields array to also hold keys for pseudoconstant fields that relate to contained fields. + * + * This is relevant where we want to offer both the ID field and the label field + * as an option, e.g. search builder. + * + * It is currently limited for optionGroupName for purposes keeping the scope of the + * change small, but is appropriate for other sorts of pseudoconstants. + * + * @param array $fields + */ + protected static function appendPseudoConstantsToFields(&$fields) { + foreach ($fields as $field) { + if (!empty($field['pseudoconstant']) && !empty($field['pseudoconstant']['optionGroupName'])) { + $fields[$field['pseudoconstant']['optionGroupName']] = array( + 'title' => CRM_Core_BAO_OptionGroup::getTitleByName($field['pseudoconstant']['optionGroupName']), + 'name' => $field['pseudoconstant']['optionGroupName'], + 'data_type' => CRM_Utils_Type::T_STRING, + ); + } + } + } + /** * Get options for the called BAO object's field. * diff --git a/CRM/Core/DAO/AllCoreTables.data.php b/CRM/Core/DAO/AllCoreTables.data.php index 26571f62eec6..8282834d468c 100644 --- a/CRM/Core/DAO/AllCoreTables.data.php +++ b/CRM/Core/DAO/AllCoreTables.data.php @@ -24,7 +24,7 @@ | see the CiviCRM license FAQ at http://civicrm.org/licensing | +--------------------------------------------------------------------+ */ -// (GenCodeChecksum:5a41993a5c97a2544f3cc2e4df8879e2) +// (GenCodeChecksum:b66a5c6ab79bec53c7781738c148e525) return array( 'CRM_Core_DAO_AddressFormat' => array( 'name' => 'AddressFormat', diff --git a/CRM/Utils/Rule.php b/CRM/Utils/Rule.php index 034bec2116d0..a40286d4b5c1 100644 --- a/CRM/Utils/Rule.php +++ b/CRM/Utils/Rule.php @@ -138,6 +138,21 @@ public static function mysqlOrderByDirection($str) { * @return bool */ public static function mysqlOrderBy($str) { + $matches = array(); + // Using the field function in order by is valid. + // Look for a string like field(contribution_status_id,3,4,6). + // or field(civicrm_contribution.contribution_status_id,3,4,6) + if (preg_match('/field\([a-z_.]+,[0-9,]+\)/', $str, $matches)) { + // We have checked these. Remove them as they will fail the next lot. + // Our check currently only permits numbers & no back ticks. If we get a + // need for strings or backticks we can add. + $str = str_replace($matches, '', $str); + } + $str = trim($str); + if (!empty($matches) && empty($str)) { + // nothing left to check after the field check. + return TRUE; + } // Making a regex for a comma separated list is quite hard and not readable // at all, so we split and loop over. $parts = explode(',', $str); diff --git a/CRM/Utils/Type.php b/CRM/Utils/Type.php index 65b726c98f04..43b920f8be4b 100644 --- a/CRM/Utils/Type.php +++ b/CRM/Utils/Type.php @@ -312,7 +312,33 @@ public static function escape($data, $type, $abort = TRUE) { case 'MysqlOrderBy': if (CRM_Utils_Rule::mysqlOrderBy($data)) { $parts = explode(',', $data); - foreach ($parts as &$part) { + + // The field() syntax is tricky here because it uses commas & when + // we separate by them we break it up. But we want to keep the clauses in order. + // so we just clumsily re-assemble it. Test cover exists. + $fieldClauseStart = NULL; + foreach ($parts as $index => &$part) { + if (substr($part, 0, 6) === 'field(') { + // Looking to escape a string like 'field(contribution_status_id,3,4,5) asc' + // to 'field(`contribution_status_id`,3,4,5) asc' + $fieldClauseStart = $index; + continue; + } + if ($fieldClauseStart !== NULL) { + // this is part of the list of field options. Concatenate it back on. + $parts[$fieldClauseStart] .= ',' . $part; + unset($parts[$index]); + if (!strstr($parts[$fieldClauseStart], ')')) { + // we have not reached the end of the list. + continue; + } + // We have the last piece of the field() clause, time to escape it. + $parts[$fieldClauseStart] = self::mysqlOrderByFieldFunctionCallback($parts[$fieldClauseStart]); + $fieldClauseStart = NULL; + continue; + + } + // Normal clause. $part = preg_replace_callback('/^(?:(?:((?:`[\w-]{1,64}`|[\w-]{1,64}))(?:\.))?(`[\w-]{1,64}`|[\w-]{1,64})(?: (asc|desc))?)$/i', array('CRM_Utils_Type', 'mysqlOrderByCallback'), trim($part)); } return implode(', ', $parts); @@ -453,6 +479,19 @@ public static function validate($data, $type, $abort = TRUE, $name = 'One of par return NULL; } + /** + * Preg_replace_callback for mysqlOrderByFieldFunction escape. + * + * Add backticks around the field name. + * + * @param string $clause + * + * @return string + */ + public static function mysqlOrderByFieldFunctionCallback($clause) { + return preg_replace('/field\((\w*)/', 'field(`${1}`', $clause); + } + /** * preg_replace_callback for MysqlOrderBy escape. */ diff --git a/tests/phpunit/CRM/Utils/TypeTest.php b/tests/phpunit/CRM/Utils/TypeTest.php index fcdaa9ad0f8d..ad9ff03738fe 100644 --- a/tests/phpunit/CRM/Utils/TypeTest.php +++ b/tests/phpunit/CRM/Utils/TypeTest.php @@ -69,6 +69,8 @@ public function validateDataProvider() { array('DESC', 'MysqlOrderByDirection', 'desc'), array('DESCc', 'MysqlOrderByDirection', NULL), array('table.civicrm_column_name desc', 'MysqlOrderBy', 'table.civicrm_column_name desc'), + array('field(civicrm_column_name,4,5,6)', 'MysqlOrderBy', 'field(civicrm_column_name,4,5,6)'), + array('field(table.civicrm_column_name,4,5,6)', 'MysqlOrderBy', 'field(table.civicrm_column_name,4,5,6)'), array('table.civicrm_column_name desc,other_column, another_column desc', 'MysqlOrderBy', 'table.civicrm_column_name desc,other_column, another_column desc'), array('table.`Home-street_address` asc, `table-alias`.`Home-street_address` desc,`table-alias`.column', 'MysqlOrderBy', 'table.`Home-street_address` asc, `table-alias`.`Home-street_address` desc,`table-alias`.column'), array('a string', 'String', 'a string'), @@ -121,6 +123,8 @@ public function escapeDataProvider() { array('DESC', 'MysqlOrderByDirection', 'desc'), array('DESCc', 'MysqlOrderByDirection', NULL), array('table.civicrm_column_name desc', 'MysqlOrderBy', '`table`.`civicrm_column_name` desc'), + array('field(contribution_status_id,4,5,6) asc', 'MysqlOrderBy', 'field(`contribution_status_id`,4,5,6) asc'), + array('field(contribution_status_id,4,5,6) asc, contact_id asc', 'MysqlOrderBy', 'field(`contribution_status_id`,4,5,6) asc, `contact_id` asc'), array('table.civicrm_column_name desc,other_column,another_column desc', 'MysqlOrderBy', '`table`.`civicrm_column_name` desc, `other_column`, `another_column` desc'), array('table.`Home-street_address` asc, `table-alias`.`Home-street_address` desc,`table-alias`.column', 'MysqlOrderBy', '`table`.`Home-street_address` asc, `table-alias`.`Home-street_address` desc, `table-alias`.`column`'), ); diff --git a/xml/schema/Contribute/Contribution.xml b/xml/schema/Contribute/Contribution.xml index 7b9148732ab1..ad9260293beb 100644 --- a/xml/schema/Contribute/Contribution.xml +++ b/xml/schema/Contribute/Contribution.xml @@ -122,9 +122,11 @@ payment_instrument_id payment_instrument_id - Payment Method + Payment Method ID int unsigned FK to Payment Instrument + true + /^payment|(p(ayment\s)?instrument)$/i payment_instrument