From 6d6630cf6d9a5057e8d620644ccf5abbbe7ae461 Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Sun, 3 Nov 2019 08:40:09 +1100 Subject: [PATCH] Convert Reports from jcalender to datepicker format Add in unit test of report upgrade process Fix time handling as per review from Eileen and fix query for time handling --- CRM/Core/Form.php | 16 +- CRM/Report/Form.php | 52 +++---- CRM/Report/Form/Contact/Relationship.php | 7 +- .../Incremental/php/FiveTwentyFive.php | 61 ++++++++ templates/CRM/Core/DatePickerRangeWrapper.tpl | 4 +- templates/CRM/Report/Form/Tabs/Filters.tpl | 2 +- .../CRM/Upgrade/Incremental/BaseTest.php | 137 ++++++++++++++++++ 7 files changed, 239 insertions(+), 40 deletions(-) diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index d5fdbb7f0f..0f6cf2020a 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -1335,14 +1335,24 @@ class CRM_Core_Form extends HTML_QuickForm_Page { * @param bool $required * @param string $fromLabel * @param string $toLabel + * @param array $additionalOptions + * @param string $to string to append to the to field. + * @param string $from string to append to the from field. */ - public function addDatePickerRange($fieldName, $label, $isDateTime = FALSE, $required = FALSE, $fromLabel = 'From', $toLabel = 'To') { + public function addDatePickerRange($fieldName, $label, $isDateTime = FALSE, $required = FALSE, $fromLabel = 'From', $toLabel = 'To', $additionalOptions = [], + $to = '_high', $from = '_low') { $options = [ '' => ts('- any -'), 0 => ts('Choose Date Range'), ] + CRM_Core_OptionGroup::values('relative_date_filters'); + if ($additionalOptions) { + foreach ($additionalOptions as $key => $optionLabel) { + $options[$key] = $optionLabel; + } + } + $this->add('select', "{$fieldName}_relative", $label, @@ -1352,8 +1362,8 @@ class CRM_Core_Form extends HTML_QuickForm_Page { ); $attributes = ['format' => 'searchDate']; $extra = ['time' => $isDateTime]; - $this->add('datepicker', $fieldName . '_low', ts($fromLabel), $attributes, $required, $extra); - $this->add('datepicker', $fieldName . '_high', ts($toLabel), $attributes, $required, $extra); + $this->add('datepicker', $fieldName . $from, ts($fromLabel), $attributes, $required, $extra); + $this->add('datepicker', $fieldName . $to, ts($toLabel), $attributes, $required, $extra); } /** diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php index 8d51b1b1fe..479c73fb4f 100644 --- a/CRM/Report/Form.php +++ b/CRM/Report/Form.php @@ -1345,14 +1345,12 @@ class CRM_Report_Form extends CRM_Core_Form { case CRM_Report_Form::OP_DATE: // build datetime fields - CRM_Core_Form_Date::buildDateRange($this, $fieldName, $count, '_from', '_to', ts('From:'), FALSE, $operations); - $count++; + $this->addDatePickerRange($fieldName, $field['title'], FALSE, FALSE, 'From', 'To', $operations, '_to', '_from'); break; case CRM_Report_Form::OP_DATETIME: // build datetime fields - CRM_Core_Form_Date::buildDateRange($this, $fieldName, $count, '_from', '_to', ts('From:'), FALSE, $operations, 'searchDate', TRUE); - $count++; + $this->addDatePickerRange($fieldName, $field['title'], TRUE, FALSE, 'From', 'To', $operations, '_to', '_from'); break; case CRM_Report_Form::OP_INT: @@ -2168,16 +2166,17 @@ class CRM_Report_Form extends CRM_Core_Form { return "( {$fieldName} {$sqlOP} )"; } - list($from, $to) = $this->getFromTo($relative, $from, $to, $fromTime, $toTime); + if ($relative) { + list($from, $to) = $this->getFromTo($relative, $from, $to, $fromTime, $toTime); + } if ($from) { - $clauses[] = "( {$fieldName} >= $from )"; + $clauses[] = "( {$fieldName} >= " . date('YmdHis', strtotime($from)) . ')'; } if ($to) { - $clauses[] = "( {$fieldName} <= {$to} )"; + $clauses[] = "( {$fieldName} <= " . date('YmdHis', strtotime($to)) . ')'; } - if (!empty($clauses)) { return implode(' AND ', $clauses); } @@ -2199,13 +2198,10 @@ class CRM_Report_Form extends CRM_Core_Form { * @return array */ public function getFromTo($relative, $from, $to, $fromTime = NULL, $toTime = NULL) { - if (empty($toTime)) { - // odd legacy behaviour to treat NULL as 'end of the day' - // recommend updating reports to call CRM_Utils_Date::getFromTo - //directly (default on the function is the actual default there). - $toTime = '235959'; + if (!empty($fromTime) || !empty($toTime)) { + throw new CRM_Core_Exception('Report template needs to be updated as getFromTo no longer supports fromTime or ToTime Parameters'); } - return CRM_Utils_Date::getFromTo($relative, $from, $to, $fromTime, $toTime); + return CRM_Utils_Date::getFromTo($relative, $from, $to); } /** @@ -3295,21 +3291,19 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND CRM_Utils_Array::value('operatorType', $field) != CRM_Report_Form::OP_MONTH ) { - list($from, $to) - = $this->getFromTo( - CRM_Utils_Array::value("{$fieldName}_relative", $this->_params), - CRM_Utils_Array::value("{$fieldName}_from", $this->_params), - CRM_Utils_Array::value("{$fieldName}_to", $this->_params), - CRM_Utils_Array::value("{$fieldName}_from_time", $this->_params), - CRM_Utils_Array::value("{$fieldName}_to_time", $this->_params) - ); - $from_time_format = !empty($this->_params["{$fieldName}_from_time"]) ? 'h' : 'd'; - $from = CRM_Utils_Date::customFormat($from, NULL, [$from_time_format]); - - $to_time_format = !empty($this->_params["{$fieldName}_to_time"]) ? 'h' : 'd'; - $to = CRM_Utils_Date::customFormat($to, NULL, [$to_time_format]); + $from = $this->_params["{$fieldName}_from"] ?? NULL; + $to = $this->_params["{$fieldName}_to"] ?? NULL; + if (!empty($this->_params["{$fieldName}_relative"])) { + list($from, $to) = CRM_Utils_Date::getFromTo($this->_params["{$fieldName}_relative"], NULL, NULL); + } if ($from || $to) { + if ($from) { + $from = date('l j F Y, g:iA', strtotime($from)); + } + if ($to) { + $to = date('l j F Y, g:iA', strtotime($to)); + } $statistics['filters'][] = [ 'title' => $field['title'], 'value' => ts("Between %1 and %2", [1 => $from, 2 => $to]), @@ -5954,9 +5948,7 @@ LEFT JOIN civicrm_contact {$field['alias']} ON {$field['alias']}.id = {$this->_a $relative = $this->_params["{$fieldName}_relative"] ?? NULL; $from = $this->_params["{$fieldName}_from"] ?? NULL; $to = $this->_params["{$fieldName}_to"] ?? NULL; - $fromTime = $this->_params["{$fieldName}_from_time"] ?? NULL; - $toTime = $this->_params["{$fieldName}_to_time"] ?? NULL; - return $this->dateClause($field['dbAlias'], $relative, $from, $to, $field['type'], $fromTime, $toTime); + return $this->dateClause($field['dbAlias'], $relative, $from, $to, $field['type']); } } else { diff --git a/CRM/Report/Form/Contact/Relationship.php b/CRM/Report/Form/Contact/Relationship.php index 1b122fe7e2..d6873e3408 100644 --- a/CRM/Report/Form/Contact/Relationship.php +++ b/CRM/Report/Form/Contact/Relationship.php @@ -821,21 +821,18 @@ class CRM_Report_Form_Contact_Relationship extends CRM_Report_Form { * @param string $from * @param string $to * @param string $type - * @param string $fromTime - * @param string $toTime * * @return null|string */ public function activeClause( $fieldName, - $relative, $from, $to, $type = NULL, $fromTime = NULL, $toTime = NULL - ) { + $relative, $from, $to, $type = NULL) { $clauses = []; if (in_array($relative, array_keys($this->getOperationPair(CRM_Report_Form::OP_DATE)))) { return NULL; } - list($from, $to) = $this->getFromTo($relative, $from, $to, $fromTime, $toTime); + list($from, $to) = $this->getFromTo($relative, $from, $to); if ($from) { $from = ($type == CRM_Utils_Type::T_DATE) ? substr($from, 0, 8) : $from; diff --git a/CRM/Upgrade/Incremental/php/FiveTwentyFive.php b/CRM/Upgrade/Incremental/php/FiveTwentyFive.php index 86b3622734..892053f1a6 100644 --- a/CRM/Upgrade/Incremental/php/FiveTwentyFive.php +++ b/CRM/Upgrade/Incremental/php/FiveTwentyFive.php @@ -69,4 +69,65 @@ class CRM_Upgrade_Incremental_php_FiveTwentyFive extends CRM_Upgrade_Incremental // return TRUE; // } + /** + * Upgrade function. + * + * @param string $rev + */ + public function upgrade_5_25_alpha1($rev) { + $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); + $this->addTask('Convert Report Form dates from jcalander to datepicker', 'convertReportsJcalendarToDatePicker'); + } + + /** + * Convert date fields stored in civicrm_report_instance to that format for datepicker + */ + public static function convertReportsJcalendarToDatePicker() { + $date_fields = []; + $reports = CRM_Core_DAO::executeQuery("SELECT id FROM civicrm_report_instance WHERE form_values like '%relative%'"); + while ($reports->fetch()) { + $report = civicrm_api3('ReportInstance', 'getsingle', ['id' => $reports->id]); + $reportFormValues = unserialize($report['form_values']); + foreach ($reportFormValues as $index => $value) { + if (strpos($index, '_relative') !== FALSE) { + $date_fields[] = str_replace('_relative', '', $index); + } + } + foreach ($date_fields as $date_field) { + foreach ($reportFormValues as $index => $value) { + if ($index === $date_field . '_to' || $index === $date_field . '_from') { + $isEndOfDay = strpos($index, '_to') !== FALSE ? TRUE : FALSE; + // If We have stored in the database hours minutes seconds use them + if (!empty($reportFormValues[$index . '_time'])) { + $time = $reportFormValues[$index . '_time']; + } + else { + $time = NULL; + } + $dateValue = $value; + if (date('Y-m-d', strtotime($dateValue)) !== $dateValue + && date('Y-m-d H:i:s', strtotime($dateValue)) !== $dateValue + && !empty($dateValue) + ) { + $dateValue = date('Y-m-d H:i:s', strtotime(CRM_Utils_Date::processDate($value, $time))); + if ($isEndOfDay) { + $dateValue = str_replace('00:00:00', '23:59:59', $dateValue); + } + } + $reportFormValues[$index] = $dateValue; + // Now remove the time keys as no longer needed. + if (!empty($reportFormValues[$index . '_time'])) { + unset($reportFormValues[$index . '_time']); + } + } + } + if (serialize($reportFormValues) !== $report['form_values']) { + civicrm_api3('ReportInstance', 'create', ['id' => $report['id'], 'form_values' => serialize($reportFormValues)]); + } + $date_fields = []; + } + } + return TRUE; + } + } diff --git a/templates/CRM/Core/DatePickerRangeWrapper.tpl b/templates/CRM/Core/DatePickerRangeWrapper.tpl index 8af6545aef..2e6fcb54ad 100644 --- a/templates/CRM/Core/DatePickerRangeWrapper.tpl +++ b/templates/CRM/Core/DatePickerRangeWrapper.tpl @@ -10,5 +10,7 @@ {* Wrapper around DatePickerRange TPL file *} {assign var='hideRelativeLabel' value=$hideRelativeLabel|default:0} - {include file="CRM/Core/DatePickerRange.tpl" fieldName=$fieldName hideRelativeLabel=$hideRelativeLabel} + {assign var='from' value=$from|default:'_low'} + {assign var='to' value=$to|default:'_high'} + {include file="CRM/Core/DatePickerRange.tpl" fieldName=$fieldName hideRelativeLabel=$hideRelativeLabel to=$to from=$from} diff --git a/templates/CRM/Report/Form/Tabs/Filters.tpl b/templates/CRM/Report/Form/Tabs/Filters.tpl index baa006e4ac..04c6ca30e1 100644 --- a/templates/CRM/Report/Form/Tabs/Filters.tpl +++ b/templates/CRM/Report/Form/Tabs/Filters.tpl @@ -36,7 +36,7 @@ {if $field.operatorType & 4} {$field.title} - {include file="CRM/Core/DateRange.tpl" fieldName=$fieldName from='_from' to='_to'} + {include file="CRM/Core/DatePickerRangeWrapper.tpl" fieldName=$fieldName hideRelativeLabel=1, from='_from' to='_to'} {elseif $form.$fieldOp.html} diff --git a/tests/phpunit/CRM/Upgrade/Incremental/BaseTest.php b/tests/phpunit/CRM/Upgrade/Incremental/BaseTest.php index a08f02950f..e9e0536764 100644 --- a/tests/phpunit/CRM/Upgrade/Incremental/BaseTest.php +++ b/tests/phpunit/CRM/Upgrade/Incremental/BaseTest.php @@ -536,4 +536,141 @@ class CRM_Upgrade_Incremental_BaseTest extends CiviUnitTestCase { $this->callAPISuccess('OptionGroup', 'delete', ['id' => $optionGroup2['id']]); } + /** + * Test conversion between jcalendar and datepicker in reports + */ + public function testReportFormConvertDatePicker() { + $report = $this->callAPISuccess('ReportInstance', 'create', [ + 'report_id' => 'contribute/detail', + 'form_values' => [ + 'fields' => [ + 'sort_name' => 1, + 'email' => 1, + 'phone' => 1, + 'financial_type_id' => 1, + 'receive_date' => 1, + 'total_amount' => 1, + 'country_id' => 1, + ], + 'sort_name_op' => 'has', + 'sort_name_value' => '', + 'id_min' => '', + 'id_max' => '', + 'id_op' => 'lte', + 'id_value' => '', + 'contact_type_op' => 'in', + 'contact_type_value' => [], + 'contact_sub_type_op' => 'in', + 'contact_sub_type_value' => [], + 'is_deleted_op' => 'eq', + 'is_deleted_value' => 0, + 'preferred_language_op' => 'in', + 'preferred_language_value' => [], + 'do_not_email_op' => 'eq', + 'do_not_email_value' => '', + 'do_not_phone_op' => 'eq', + 'do_not_phone_value' => '', + 'do_not_mail_op' => 'eq', + 'do_not_mail_value' => '', + 'do_not_sms_op' => 'eq', + 'do_not_sms_value' => '', + 'is_opt_out_op' => 'eq', + 'is_opt_out_value' => '', + 'first_name_op' => 'has', + 'first_name_value' => '', + 'prefix_id_op' => 'in', + 'prefix_id_value' => [], + 'suffix_id_op' => 'in', + 'suffix_id_value' => [], + 'gender_id_op' => 'in', + 'gender_id_value' => [], + 'birth_date_relative' => '', + 'birth_date_from' => '', + 'birth_date_to' => '', + 'is_deceased_op' => 'eq', + 'is_deceased_value' => '', + 'contribution_or_soft_op' => 'eq', + 'contribution_or_soft_value' => 'contributions_only', + 'receive_date_relative' => 0, + 'receive_date_from' => '11/01/1991', + 'receive_date_to' => '', + 'thankyou_date_relative' => '', + 'thankyou_date_from' => '', + 'thankyou_date_to' => '', + 'contribution_source_op' => 'has', + 'contribution_source_value' => '', + 'currency_op' => 'in', + 'currency_value' => [], + 'non_deductible_amount_min' => '', + 'non_deductible_amount_max' => '', + 'non_deductible_amount_op' => 'lte', + 'non_deductible_amount_value' => '', + 'financial_type_id_op' => 'in', + 'financial_type_id_value' => [], + 'contribution_page_id_op' => 'in', + 'contribution_page_id_value' => [], + 'payment_instrument_id_op' => 'in', + 'payment_instrument_id_value' => [], + 'contribution_status_id_op' => 'in', + 'contribution_status_id_value' => [0 => 1], + 'total_amount_min' => '', + 'total_amount_max' => '', + 'total_amount_op' => 'lte', + 'total_amount_value' => '', + 'cancel_date_relative' => '', + 'cancel_date_from' => '', + 'cancel_date_to' => '', + 'cancel_reason_op' => 'has', + 'cancel_reason_value' => '', + 'soft_credit_type_id_op' => 'in', + 'soft_credit_type_id_value' => [], + 'card_type_id_op' => 'in', + 'card_type_id_value' => [], + 'ordinality_op' => 'in', + 'ordinality_value' => [], + 'note_value' => '', + 'street_address_op' => 'has', + 'street_address_value' => '', + 'postal_code_op' => 'has', + 'postal_code_value' => '', + 'city_op' => 'has', + 'city_value' => '', + 'country_id_op' => 'in', + 'country_id_value' => [], + 'state_province_id_op' => 'in', + 'state_province_id_value' => [], + 'county_id_op' => 'in', + 'county_id_value' => [], + 'tagid_op' => 'in', + 'tagid_value' => [], + 'gid_op' => 'in', + 'gid_value' => [], + 'group_bys' => ['contribution_id' => 1], + 'order_bys' => [ + 1 => [ + 'column' => 'sort_name', + 'order' => 'ASC', + ], + ], + 'description' => 'Lists specific contributions by criteria including contact, time period, contribution type, contributor location, etc. Contribution summary report points to this report for contribution details.', + 'email_subject' => '', + 'email_to' => '', + 'email_cc' => '', + 'row_count' => '', + 'view_mode' => 'criteria', + 'cache_minutes' => 60, + 'permission' => 'access CiviContribute', + 'parent_id' => '', + 'radio_ts' => '', + 'groups' => '', + 'report_id' => 'contribute/detail', + ], + 'title' => 'test Report', + ]); + CRM_Upgrade_Incremental_php_FiveTwentyFive::convertReportsJcalendarToDatePicker(); + $reportGet = $this->callAPISuccess('ReportInstance', 'getsingle', ['id' => $report['id']]); + $formValues = unserialize($reportGet['form_values']); + $this->assertEquals('1991-11-01 00:00:00', $formValues['receive_date_from']); + } + } -- 2.25.1