From 7f175707d67597717d72661c55878d6432f87736 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 31 May 2019 12:47:30 +1200 Subject: [PATCH] dev/core#389 Fix custom data relative date searching This addresses a fairly old regression where smart groups based on relative dates stopped being relative --- CRM/Contact/BAO/Query.php | 9 ++++--- CRM/Contact/BAO/SavedSearch.php | 4 ++- CRM/Core/BAO/CustomQuery.php | 26 +++++++++++++++---- .../CRM/Contact/BAO/SavedSearchTest.php | 11 ++++---- .../phpunit/CRM/Core/BAO/CustomQueryTest.php | 11 ++++---- 5 files changed, 41 insertions(+), 20 deletions(-) diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index c2f6190e357d..ab5e7f69d06f 100644 --- a/CRM/Contact/BAO/Query.php +++ b/CRM/Contact/BAO/Query.php @@ -607,7 +607,7 @@ public function buildParamsLookup() { if (empty($value[0])) { continue; } - $cfID = CRM_Core_BAO_CustomField::getKeyID($value[0]); + $cfID = CRM_Core_BAO_CustomField::getKeyID(str_replace('_relative', '', $value[0])); if ($cfID) { if (!array_key_exists($cfID, $this->_cfIDs)) { $this->_cfIDs[$cfID] = []; @@ -1638,8 +1638,7 @@ public static function convertFormValues(&$formValues, $wildcard = 0, $useEquals } elseif (substr($id, 0, 7) == 'custom_' && ( - substr($id, -9, 9) == '_relative' - || substr($id, -5, 5) == '_from' + substr($id, -5, 5) == '_from' || substr($id, -3, 3) == '_to' ) ) { @@ -6932,7 +6931,9 @@ protected function buildRelativeDateQuery(&$values) { $tableName = $fieldSpec['table_name']; $filters = CRM_Core_OptionGroup::values('relative_date_filters'); $grouping = CRM_Utils_Array::value(3, $values); - $this->_tables[$tableName] = $this->_whereTables[$tableName] = 1; + // If the table value is already set for a custom field it will be more nuanced than just '1'. + $this->_tables[$tableName] = $this->_tables[$tableName] ?? 1; + $this->_whereTables[$tableName] = $this->_whereTables[$tableName] ?? 1; $dates = CRM_Utils_Date::getFromTo($value, NULL, NULL); if (empty($dates[0])) { diff --git a/CRM/Contact/BAO/SavedSearch.php b/CRM/Contact/BAO/SavedSearch.php index 46886c641360..b3561a72fed2 100644 --- a/CRM/Contact/BAO/SavedSearch.php +++ b/CRM/Contact/BAO/SavedSearch.php @@ -434,10 +434,12 @@ protected function assignTestValue($fieldName, &$fieldDef, $counter) { * @param array $formValues */ public static function saveRelativeDates(&$queryParams, $formValues) { + // This is required only until all fields are converted to datepicker fields as the new format is truer to the + // form format and simply saves (e.g) custom_3_relative => "this.year" $relativeDates = ['relative_dates' => []]; $specialDateFields = ['event_relative', 'case_from_relative', 'case_to_relative', 'participant_relative']; foreach ($formValues as $id => $value) { - if ((preg_match('/(_date|custom_[0-9]+)_relative$/', $id) || in_array($id, $specialDateFields)) && !empty($value)) { + if ((preg_match('/_date$/', $id) || in_array($id, $specialDateFields)) && !empty($value)) { $entityName = strstr($id, '_date', TRUE); if (empty($entityName)) { $entityName = strstr($id, '_relative', TRUE); diff --git a/CRM/Core/BAO/CustomQuery.php b/CRM/Core/BAO/CustomQuery.php index eb1b645c744a..a3e2cf273677 100644 --- a/CRM/Core/BAO/CustomQuery.php +++ b/CRM/Core/BAO/CustomQuery.php @@ -227,8 +227,7 @@ public function select() { return; } - $this->_tables[$name] = "\nLEFT JOIN $name ON $name.entity_id = $joinTable.id"; - $this->_whereTables[$name] = $this->_tables[$name]; + $this->joinCustomTableForField($field); if ($joinTable) { $joinClause = 1; @@ -292,6 +291,8 @@ public function where() { $qillOp = CRM_Utils_Array::value($op, CRM_Core_SelectValues::getSearchBuilderOperators(), $op); + // Ensure the table is joined in (eg if in where but not select). + $this->joinCustomTableForField($field); switch ($field['data_type']) { case 'String': case 'StateProvince': @@ -414,9 +415,12 @@ public function where() { break; case 'Date': - $this->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause($fieldName, $op, $value, 'Date'); - list($qillOp, $qillVal) = CRM_Contact_BAO_Query::buildQillForFieldValue(NULL, $field['label'], $value, $op, [], CRM_Utils_Type::T_DATE); - $this->_qill[$grouping][] = "{$field['label']} $qillOp '$qillVal'"; + if (substr($name, -9, 9) !== '_relative') { + // Relative dates are handled in the buildRelativeDateQuery function. + $this->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause($fieldName, $op, $value, 'Date'); + list($qillOp, $qillVal) = CRM_Contact_BAO_Query::buildQillForFieldValue(NULL, $field['label'], $value, $op, [], CRM_Utils_Type::T_DATE); + $this->_qill[$grouping][] = "{$field['label']} $qillOp '$qillVal'"; + } break; case 'File': @@ -471,4 +475,16 @@ public function query() { ]; } + /** + * Join the custom table for the field in (if not already in the query). + * + * @param array $field + */ + protected function joinCustomTableForField($field) { + $name = $field['table_name']; + $join = "\nLEFT JOIN $name ON $name.entity_id = {$field['search_table']}.id"; + $this->_tables[$name] = $this->_tables[$name] ?? $join; + $this->_whereTables[$name] = $this->_whereTables[$name] ?? $join; + } + } diff --git a/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php b/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php index 43bcf0eb3692..9289e8fddb88 100644 --- a/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php +++ b/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php @@ -205,9 +205,11 @@ public function testRelativeDateValues() { /** * Test relative dates * - * The function saveRelativeDates should detect whether a field is using - * a relative date range and include in the fromValues a relative_date - * index so it is properly detects when executed. + * This is a slightly odd test because it was originally created to test that we DO create a + * special 'relative_dates' key but the new favoured format is not to do that and to + * save (eg) custom_1_relative = this.day. + * + * It still presumably provides useful 'this does not fatal or give enotice' coverage. */ public function testCustomFieldRelativeDates() { // Create a custom field. @@ -246,8 +248,7 @@ public function testCustomFieldRelativeDates() { CRM_Contact_BAO_SavedSearch::saveRelativeDates($queryParams, $formValues); // Since custom_13 doesn't have the word 'date' in it, the key is // set to 0, rather than the field name. - $err = 'Relative date in custom field smart group creation failed.'; - $this->assertArrayHasKey('relative_dates', $queryParams, $err); + $this->assertArrayNotHasKey('relative_dates', $queryParams, 'Relative date in custom field smart group creation failed.'); $dropCustomValueTables = TRUE; $this->quickCleanup(array('civicrm_saved_search'), $dropCustomValueTables); } diff --git a/tests/phpunit/CRM/Core/BAO/CustomQueryTest.php b/tests/phpunit/CRM/Core/BAO/CustomQueryTest.php index ce496704d841..a5fd1cc1152e 100644 --- a/tests/phpunit/CRM/Core/BAO/CustomQueryTest.php +++ b/tests/phpunit/CRM/Core/BAO/CustomQueryTest.php @@ -42,14 +42,15 @@ public function testSearchCustomDataDateRelative() { // Assigning the relevant form value to be within a custom key is normally done in // build field params. It would be better if it were all done in convertFormValues // but for now we just imitate it. - $params[$dateCustomField['id']] = CRM_Contact_BAO_Query::convertFormValues($formValues); - $queryObj = new CRM_Core_BAO_CustomQuery($params); - $queryObj->Query(); + + $params = CRM_Contact_BAO_Query::convertFormValues($formValues); + $queryObj = new CRM_Contact_BAO_Query($params); $this->assertEquals( - 'civicrm_value_testsearchcus_1.date_field_2 BETWEEN "' . date('Y') . '0101000000" AND "' . date('Y') . '1231235959"', + "civicrm_value_testsearchcus_1.date_field_2 BETWEEN '" . date('Y') . "0101000000' AND '" . date('Y') . "1231235959'", $queryObj->_where[0][0] ); - $this->assertEquals($queryObj->_qill[0][0], "date field BETWEEN 'January 1st, " . date('Y') . " 12:00 AM AND December 31st, " . date('Y') . " 11:59 PM'"); + $this->assertEquals("date field is This calendar year (between January 1st, " . date('Y') . " 12:00 AM and December 31st, " . date('Y') . " 11:59 PM)", $queryObj->_qill[0][0]); + $queryObj = new CRM_Core_BAO_CustomQuery($params); $this->assertEquals([ 'id' => $dateCustomField['id'], 'label' => 'date field',