From 80b5c9f1662cbf567980fe96bc640819753b37e6 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 1 Nov 2019 19:15:03 +1300 Subject: [PATCH] Replace log_date with created_date & modified_date in advanced search This is almost the last step in getting rid of an awful lot of technical debt & hard to maintain code around jcalendar & datepicker. It does change the UI a bit - the code to create the current UI is pretty tough going and from seeing users try to work with it I don't think all that code is making it more usable. OTOH we can remove a maintainability blockerr by switching over --- CRM/Contact/BAO/Query.php | 57 +------- CRM/Contact/BAO/SavedSearch.php | 29 ----- CRM/Contact/Form/Search/Criteria.php | 12 +- CRM/Contact/Form/Task/SaveSearch.php | 1 - .../Form/Search/Criteria/ChangeLog.tpl | 43 +----- tests/phpunit/CRM/Contact/BAO/QueryTest.php | 42 +----- .../CRM/Contact/BAO/SavedSearchTest.php | 122 ------------------ 7 files changed, 19 insertions(+), 287 deletions(-) diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index ab90299b2d5a..287bd508018f 100644 --- a/CRM/Contact/BAO/Query.php +++ b/CRM/Contact/BAO/Query.php @@ -1600,41 +1600,6 @@ public static function convertFormValues(&$formValues, $wildcard = 0, $useEquals } self::filterCountryFromValuesIfStateExists($formValues); - // We shouldn't have to whitelist fields to not hack but here we are, for now. - $nonLegacyDateFields = [ - 'participant_register_date_relative', - 'receive_date_relative', - 'pledge_end_date_relative', - 'pledge_create_date_relative', - 'pledge_start_date_relative', - 'pledge_payment_scheduled_date_relative', - 'relationship_start_date_relative', - 'relationship_end_date_relative', - 'membership_join_date_relative', - 'membership_start_date_relative', - 'membership_end_date_relative', - 'case_start_date_relative', - 'case_end_date_relative', - 'mailing_job_start_date_relative', - 'birth_date_relative', - 'deceased_date_relative', - 'relation_active_period_date_relative', - ]; - // Handle relative dates first - foreach (array_keys($formValues) as $id) { - if ( - !in_array($id, $nonLegacyDateFields) && ( - preg_match('/_date_relative$/', $id)) - ) { - $dateComponent = explode('_date_relative', $id); - $fromRange = "{$dateComponent[0]}_date_low"; - $toRange = "{$dateComponent[0]}_date_high"; - - if (array_key_exists($fromRange, $formValues) && array_key_exists($toRange, $formValues)) { - CRM_Contact_BAO_Query::fixDateValues($formValues[$id], $formValues[$fromRange], $formValues[$toRange]); - } - } - } foreach ($formValues as $id => $values) { if (self::isAlreadyProcessedForQueryFormat($values)) { @@ -1692,13 +1657,6 @@ public static function convertFormValues(&$formValues, $wildcard = 0, $useEquals ) { self::convertCustomRelativeFields($formValues, $params, $values, $id); } - elseif ( - !in_array($id, $nonLegacyDateFields) && ( - preg_match('/_date_relative$/', $id)) - ) { - // Already handled in previous loop - continue; - } elseif (in_array($id, $entityReferenceFields) && !empty($values) && is_string($values) && (strpos($values, ',') != FALSE)) { $params[] = [$id, 'IN', explode(',', $values), 0, 0]; @@ -3999,15 +3957,8 @@ public function changeLog(&$values) { $name = $targetName[4] ? "%$name%" : $name; $this->_where[$grouping][] = "contact_b_log.sort_name LIKE '%$name%'"; $this->_tables['civicrm_log'] = $this->_whereTables['civicrm_log'] = 1; - $fieldTitle = ts('Added By'); - foreach ($this->_params as $params) { - if ($params[0] == 'log_date') { - if ($params[2] == 2) { - $fieldTitle = ts('Modified By'); - } - break; - } - } + $fieldTitle = ts('Altered By'); + list($qillop, $qillVal) = self::buildQillForFieldValue(NULL, 'changed_by', $name, 'LIKE'); $this->_qill[$grouping][] = ts("%1 %2 '%3'", [ 1 => $fieldTitle, @@ -5298,6 +5249,10 @@ public function dateQueryBuilder( // @todo - remove dateFormat - pretty sure it's never passed in... list($name, $op, $value, $grouping, $wildcard) = $values; + if ($tableName === 'civicrm_contact') { + // Special handling for contact table as it has a known alias in advanced search. + $tableName = 'contact_a'; + } if ($name == "{$fieldName}_low" || $name == "{$fieldName}_high" ) { diff --git a/CRM/Contact/BAO/SavedSearch.php b/CRM/Contact/BAO/SavedSearch.php index c929be0e911c..2819b8d04b92 100644 --- a/CRM/Contact/BAO/SavedSearch.php +++ b/CRM/Contact/BAO/SavedSearch.php @@ -386,35 +386,6 @@ protected function assignTestValue($fieldName, &$fieldDef, $counter) { } } - /** - * Store relative dates in separate array format - * - * @param array $queryParams - * @param array $formValues - * @deprecated - */ - 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 = [ - 'log_date_relative', - ]; - foreach ($formValues as $id => $value) { - if (in_array($id, $specialDateFields) && !empty($value)) { - $entityName = strstr($id, '_date', TRUE); - if (empty($entityName)) { - $entityName = strstr($id, '_relative', TRUE); - } - $relativeDates['relative_dates'][$entityName] = $value; - } - } - // merge with original queryParams if relative date value(s) found - if (count($relativeDates['relative_dates'])) { - $queryParams = array_merge($queryParams, $relativeDates); - } - } - /** * Store search variables in $queryParams which were skipped while processing query params, * precisely at CRM_Contact_BAO_Query::fixWhereValues(...). But these variable are required in diff --git a/CRM/Contact/Form/Search/Criteria.php b/CRM/Contact/Form/Search/Criteria.php index 243286cdfc05..8a5221bd3a32 100644 --- a/CRM/Contact/Form/Search/Criteria.php +++ b/CRM/Contact/Form/Search/Criteria.php @@ -263,6 +263,8 @@ public static function getSearchFieldMetadata() { 'sort_name' => ['title' => ts('Complete OR Partial Name'), 'template_grouping' => 'basic'], 'email' => ['title' => ts('Complete OR Partial Email'), 'entity' => 'Email', 'template_grouping' => 'basic'], 'contact_tags' => ['name' => 'contact_tags', 'type' => CRM_Utils_Type::T_INT, 'is_pseudofield' => TRUE, 'template_grouping' => 'basic'], + 'created_date' => ['name' => 'created_date', 'template_grouping' => 'changeLog'], + 'modified_date' => ['name' => 'modified_date', 'template_grouping' => 'changeLog'], 'birth_date' => ['name' => 'birth_date', 'template_grouping' => 'demographic'], 'deceased_date' => ['name' => 'deceased_date', 'template_grouping' => 'demographic'], 'is_deceased' => ['is_deceased', 'template_grouping' => 'demographic'], @@ -503,17 +505,15 @@ public static function activity(&$form) { /** * @param CRM_Core_Form $form + * + * @throws \CiviCRM_API3_Exception */ public static function changeLog(&$form) { $form->add('hidden', 'hidden_changeLog', 1); - + $form->addSearchFieldMetadata(['Contact' => self::getFilteredSearchFieldMetadata('changeLog')]); + $form->addFormFieldsFromMetadata(); // block for change log $form->addElement('text', 'changed_by', ts('Modified By'), NULL); - - $dates = [1 => ts('Added'), 2 => ts('Modified')]; - $form->addRadio('log_date', NULL, $dates, ['allowClear' => TRUE]); - - CRM_Core_Form_Date::buildDateRange($form, 'log_date', 1, '_low', '_high', ts('From:'), FALSE, FALSE); } /** diff --git a/CRM/Contact/Form/Task/SaveSearch.php b/CRM/Contact/Form/Task/SaveSearch.php index 1b888a6b53f8..758379ded531 100644 --- a/CRM/Contact/Form/Task/SaveSearch.php +++ b/CRM/Contact/Form/Task/SaveSearch.php @@ -191,7 +191,6 @@ public function postProcess() { // Ideally per CRM-17075 we will use entity reference fields heavily in the form layer & convert to the // sql operator syntax at the query layer. if (!$isSearchBuilder) { - CRM_Contact_BAO_SavedSearch::saveRelativeDates($queryParams, $formValues); CRM_Contact_BAO_SavedSearch::saveSkippedElement($queryParams, $formValues); $savedSearch->form_values = serialize($queryParams); } diff --git a/templates/CRM/Contact/Form/Search/Criteria/ChangeLog.tpl b/templates/CRM/Contact/Form/Search/Criteria/ChangeLog.tpl index 3d0621e29176..e8b8dfd67216 100644 --- a/templates/CRM/Contact/Form/Search/Criteria/ChangeLog.tpl +++ b/templates/CRM/Contact/Form/Search/Criteria/ChangeLog.tpl @@ -26,49 +26,16 @@
- - - - - - - {include file="CRM/Core/DateRange.tpl" fieldName="log_date" from='_low' to='_high'}
- {$form.log_date.html} -
- - + +
+ {$form.changed_by.html} 		- - + {include file="CRM/Core/DatePickerRangeWrapper.tpl" fieldName="created_date"}
- {$form.changed_by.html} + {include file="CRM/Core/DatePickerRangeWrapper.tpl" fieldName="modified_date"}
- -{literal} - -{/literal} diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php index b206e6f73caa..9d4f0a578b32 100644 --- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php +++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php @@ -1027,46 +1027,6 @@ public function testGetSummaryQueryWithFinancialACLEnabled() { $this->disableFinancialACLs(); } - /** - * When we have a relative date in search criteria, check that convertFormValues() sets _low & _high date fields and returns other criteria. - * CRM-21816 fix relative dates in search bug - */ - public function testConvertFormValuesCRM21816() { - $fv = [ - // next 60 days - "member_end_date_relative" => "starting_2.month", - "member_end_date_low" => "20180101000000", - "member_end_date_high" => "20180331235959", - "membership_is_current_member" => "1", - "member_is_primary" => "1", - ]; - // $fv is modified by convertFormValues() - $fv_orig = $fv; - $params = CRM_Contact_BAO_Query::convertFormValues($fv); - - // restructure for easier testing - $modparams = []; - foreach ($params as $p) { - $modparams[$p[0]] = $p; - } - - // Check member_end_date_low is in params - $this->assertTrue(is_array($modparams['member_end_date_low'])); - // ... fv and params should match - $this->assertEquals($modparams['member_end_date_low'][2], $fv['member_end_date_low']); - // ... fv & fv_orig should be different - $this->assertNotEquals($fv['member_end_date_low'], $fv_orig['member_end_date_low']); - - // same for member_end_date_high - $this->assertTrue(is_array($modparams['member_end_date_high'])); - $this->assertEquals($modparams['member_end_date_high'][2], $fv['member_end_date_high']); - $this->assertNotEquals($fv['member_end_date_high'], $fv_orig['member_end_date_high']); - - // Check other fv values are in params - $this->assertEquals($modparams['membership_is_current_member'][2], $fv_orig['membership_is_current_member']); - $this->assertEquals($modparams['member_is_primary'][2], $fv_orig['member_is_primary']); - } - /** * Create contributions to test summary calculations. * @@ -1078,6 +1038,8 @@ public function testConvertFormValuesCRM21816() { * Donation |NULL | 300.00 |SSF | Donation,Donation | 2 | 200.00,100.00 * Donation |2019-02-13 00:00:00 | 50.00 |SSF | Donation | 1 | 50.00 * Member Dues |2019-02-13 00:00:00 | 50.00 |SSF | Member Dues | 1 | 50.00 + * + * @throws \CRM_Core_Exception */ protected function createContributionsForSummaryQueryTests() { $contactID = $this->individualCreate(); diff --git a/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php b/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php index 4cbe6239cc73..b939d85bac89 100644 --- a/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php +++ b/tests/phpunit/CRM/Contact/BAO/SavedSearchTest.php @@ -97,53 +97,6 @@ public function testGetFormValues($formValues, $expectedResult, $searchDescripti } } - /** - * Test if dates ranges are stored correctly - * in civicrm_saved_search table and are - * extracted properly. - */ - public function testDateRange() { - $savedSearch = new CRM_Contact_BAO_SavedSearch(); - $formValues = [ - 'hidden_basic' => 1, - 'group_search_selected' => 'group', - 'component_mode' => 1, - 'operator' => 'AND', - 'privacy_operator' => 'OR', - 'privacy_toggle' => 1, - 'participant_register_date_low' => '01/01/2009', - 'participant_register_date_high' => '01/01/2018', - 'radio_ts' => 'ts_all', - 'title' => 'bah bah bah', - ]; - - $queryParams = [ - 0 => [ - 0 => 'participant_register_date_low', - 1 => '=', - 2 => '01/01/2009', - 3 => 0, - 4 => 0, - ], - 1 => [ - 0 => 'participant_register_date_high', - 1 => '=', - 2 => '01/01/2018', - 3 => 0, - 4 => 0, - ], - ]; - - CRM_Contact_BAO_SavedSearch::saveRelativeDates($queryParams, $formValues); - CRM_Contact_BAO_SavedSearch::saveSkippedElement($queryParams, $formValues); - $savedSearch->form_values = serialize($queryParams); - $savedSearch->save(); - - $result = CRM_Contact_BAO_SavedSearch::getFormValues(CRM_Core_DAO::singleValueQuery('SELECT LAST_INSERT_ID()')); - $this->assertEquals('01/01/2009', $result['participant_register_date_low']); - $this->assertEquals('01/01/2018', $result['participant_register_date_high']); - } - /** * Test if skipped elements are correctly * stored and retrieved as formvalues. @@ -174,81 +127,6 @@ public function testSkippedElements() { $this->checkArrayEquals($result, $expectedResult); } - /** - * Test if change log relative dates are stored correctly - * in civicrm_saved_search table. - */ - public function testRelativeDateChangeLog() { - $savedSearch = new CRM_Contact_BAO_SavedSearch(); - $formValues = [ - 'operator' => 'AND', - 'log_date_relative' => 'this.month', - 'radio_ts' => 'ts_all', - ]; - $queryParams = []; - CRM_Contact_BAO_SavedSearch::saveRelativeDates($queryParams, $formValues); - CRM_Contact_BAO_SavedSearch::saveSkippedElement($queryParams, $formValues); - $savedSearch->form_values = serialize($queryParams); - $savedSearch->save(); - - $result = CRM_Contact_BAO_SavedSearch::getFormValues(CRM_Core_DAO::singleValueQuery('SELECT LAST_INSERT_ID()')); - $expectedResult = [ - 'log' => 'this.month', - ]; - $this->checkArrayEquals($result['relative_dates'], $expectedResult); - } - - /** - * Test relative dates - * - * 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. - $customGroup = $this->customGroupCreate(['extends' => 'Individual', 'title' => 'relative_date_test_group']); - $params = [ - 'custom_group_id' => $customGroup['id'], - 'name' => 'test_datefield', - 'label' => 'Date Field for Testing', - 'html_type' => 'Select Date', - 'data_type' => 'Date', - 'default_value' => NULL, - 'weight' => 4, - 'is_required' => 1, - 'is_searchable' => 1, - 'date_format' => 'mm/dd/yy', - 'is_active' => 1, - ]; - $customField = $this->callAPIAndDocument('custom_field', 'create', $params, __FUNCTION__, __FILE__); - $id = $customField['id']; - - $queryParams = [ - 0 => [ - 0 => "custom_${id}_low", - 1 => '=', - 2 => '20170425000000', - ], - 1 => [ - 0 => "custom_${id}_high", - 1 => '=', - 2 => '20170501235959', - ], - ]; - $formValues = [ - "custom_${id}_relative" => 'ending.week', - ]; - 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. - $this->assertArrayNotHasKey('relative_dates', $queryParams, 'Relative date in custom field smart group creation failed.'); - $dropCustomValueTables = TRUE; - $this->quickCleanup(['civicrm_saved_search'], $dropCustomValueTables); - } - /** * Get variants of the fields we want to test. *