Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#389 Fix custom data relative date searching #14625

Merged
merged 1 commit into from
Jun 28, 2019
Merged

dev/core#389 Fix custom data relative date searching #14625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions CRM/Contact/BAO/Query.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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] = [];
Expand Down Expand Up @@ -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'
)
) {
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the ??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seamuslee001 that is php 7.0 syntax - see https://stackoverflow.com/questions/34571330/php-ternary-operator-vs-null-coalescing-operator - it avoids enotices without lots of isset...

$this->_whereTables[$tableName] = $this->_whereTables[$tableName] ?? 1;

$dates = CRM_Utils_Date::getFromTo($value, NULL, NULL);
if (empty($dates[0])) {
Expand Down
4 changes: 3 additions & 1 deletion CRM/Contact/BAO/SavedSearch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
26 changes: 21 additions & 5 deletions CRM/Core/BAO/CustomQuery.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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':
Expand Down Expand Up @@ -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':
Expand Down Expand Up @@ -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;
}

}
11 changes: 6 additions & 5 deletions tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
Expand Down
11 changes: 6 additions & 5 deletions tests/phpunit/CRM/Core/BAO/CustomQueryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down