From a844d885b25e186ac14e91fd2aa13127726fb2be Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Wed, 19 Jan 2022 08:22:03 +0000 Subject: [PATCH] MDL-73180 reportbuilder: improve relative date filter definitions. Changes since the original implementation in e55abd71 mean that relative date filtering (by last/next day/week, etc) is now based on the actual current time, rather than the start or end of the current time unit. --- lang/en/reportbuilder.php | 2 +- reportbuilder/classes/local/filters/date.php | 88 ++++++++----------- .../tests/local/filters/date_test.php | 24 +++-- 3 files changed, 55 insertions(+), 59 deletions(-) diff --git a/lang/en/reportbuilder.php b/lang/en/reportbuilder.php index fa095f57f78..5db0eaebb7f 100644 --- a/lang/en/reportbuilder.php +++ b/lang/en/reportbuilder.php @@ -113,10 +113,10 @@ $string['filterdatecurrent'] = 'Current'; $string['filterdatedays'] = 'day(s)'; $string['filterdatefrom'] = 'Date from'; $string['filterdatehours'] = 'hour(s)'; +$string['filterdatelast'] = 'Last'; $string['filterdateminutes'] = 'minute(s)'; $string['filterdatemonths'] = 'month(s)'; $string['filterdatenext'] = 'Next'; -$string['filterdateprevious'] = 'Previous'; $string['filterdateseconds'] = 'second(s)'; $string['filterdateto'] = 'Date to'; $string['filterdateweeks'] = 'week(s)'; diff --git a/reportbuilder/classes/local/filters/date.php b/reportbuilder/classes/local/filters/date.php index 517bbfaa7d8..93771eac2d0 100644 --- a/reportbuilder/classes/local/filters/date.php +++ b/reportbuilder/classes/local/filters/date.php @@ -46,8 +46,11 @@ class date extends base { /** @var int Date within defined range */ public const DATE_RANGE = 3; - /** @var int Date in the previous [X relative date unit(s)] */ - public const DATE_PREVIOUS = 4; + /** @var int Date in the last [X relative date unit(s)] */ + public const DATE_LAST = 4; + + /** @var int Date in the previous [X relative date unit(s)] Kept for backwards compatibility */ + public const DATE_PREVIOUS = self::DATE_LAST; /** @var int Date in current [relative date unit] */ public const DATE_CURRENT = 5; @@ -78,7 +81,7 @@ class date extends base { self::DATE_NOT_EMPTY => new lang_string('filterisnotempty', 'core_reportbuilder'), self::DATE_EMPTY => new lang_string('filterisempty', 'core_reportbuilder'), self::DATE_RANGE => new lang_string('filterrange', 'core_reportbuilder'), - self::DATE_PREVIOUS => new lang_string('filterdateprevious', 'core_reportbuilder'), + self::DATE_LAST => new lang_string('filterdatelast', 'core_reportbuilder'), self::DATE_CURRENT => new lang_string('filterdatecurrent', 'core_reportbuilder'), self::DATE_NEXT => new lang_string('filterdatenext', 'core_reportbuilder'), ]; @@ -99,7 +102,7 @@ class date extends base { $mform->setType("{$this->name}_operator", PARAM_INT); $mform->setDefault("{$this->name}_operator", self::DATE_ANY); - // Value selector for previous and next operators. + // Value selector for last and next operators. $valuelabel = get_string('filterfieldvalue', 'core_reportbuilder', $this->get_header()); $elements[] = $mform->createElement('text', "{$this->name}_value", $valuelabel, ['size' => 3]); @@ -111,7 +114,7 @@ class date extends base { $mform->hideIf("{$this->name}_value", "{$this->name}_operator", 'eq', self::DATE_RANGE); $mform->hideIf("{$this->name}_value", "{$this->name}_operator", 'eq', self::DATE_CURRENT); - // Unit selector for previous and next operators. + // Unit selector for last and next operators. $unitlabel = get_string('filterdurationunit', 'core_reportbuilder', $this->get_header()); $units = [ self::DATE_UNIT_DAY => get_string('filterdatedays', 'core_reportbuilder'), @@ -187,11 +190,11 @@ class date extends base { break; // Relative helper method can handle these three cases. - case self::DATE_PREVIOUS: + case self::DATE_LAST: case self::DATE_CURRENT: case self::DATE_NEXT: - // Previous and next operators require a unit value greater than zero. + // Last and next operators require a unit value greater than zero. if ($operator !== self::DATE_CURRENT && $dateunitvalue === 0) { return ['', []]; } @@ -217,80 +220,65 @@ class date extends base { /** * Return start and end time of given relative date period * - * @param int $operator - * @param int $dateunitvalue - * @param int $dateunit - * @return int[] + * @param int $operator One of the ::DATE_LAST/CURRENT/NEXT constants + * @param int $dateunitvalue Unit multiplier of the date unit + * @param int $dateunit One of the ::DATE_UNIT_DAY/WEEK/MONTH/YEAR constants + * @return int[] Timestamps representing the start/end of timeframe */ private static function get_relative_timeframe(int $operator, int $dateunitvalue, int $dateunit): array { - $datenow = new DateTimeImmutable(); + // Initialise start/end time to now. + $datestart = $dateend = new DateTimeImmutable(); switch ($dateunit) { case self::DATE_UNIT_DAY: - // Current day. - $datestart = $dateend = $datenow; - - if ($operator === self::DATE_PREVIOUS) { + if ($operator === self::DATE_CURRENT) { + $datestart = $datestart->setTime(0, 0); + $dateend = $dateend->setTime(23, 59, 59); + } else if ($operator === self::DATE_LAST) { $datestart = $datestart->modify("-{$dateunitvalue} day"); - $dateend = $dateend->modify('-1 day'); } else if ($operator === self::DATE_NEXT) { - $datestart = $datestart->modify('+1 day'); $dateend = $dateend->modify("+{$dateunitvalue} day"); } break; case self::DATE_UNIT_WEEK: - // Current week. - $datestart = $datenow->modify('monday this week'); - $dateend = $datenow->modify('sunday this week'); - - if ($operator === self::DATE_PREVIOUS) { + if ($operator === self::DATE_CURRENT) { + $datestart = $datestart->modify('monday this week')->setTime(0, 0); + $dateend = $dateend->modify('sunday this week')->setTime(23, 59, 59); + } else if ($operator === self::DATE_LAST) { $datestart = $datestart->modify("-{$dateunitvalue} week"); - $dateend = $dateend->modify('-1 week'); } else if ($operator === self::DATE_NEXT) { - $datestart = $datestart->modify('+1 week'); $dateend = $dateend->modify("+{$dateunitvalue} week"); } break; case self::DATE_UNIT_MONTH: - // Current month. - $datestart = $datenow->modify('first day of this month'); - $dateend = $datenow->modify('last day of this month'); - - [$dateyear, $datemonth] = explode('/', $datenow->format('Y/m')); - if ($operator === self::DATE_PREVIOUS) { - $datestart = $datestart->setDate((int) $dateyear, $datemonth - $dateunitvalue, 1); - $dateend = $dateend->modify('last day of last month'); + if ($operator === self::DATE_CURRENT) { + $datestart = $datestart->modify('first day of this month')->setTime(0, 0); + $dateend = $dateend->modify('last day of this month')->setTime(23, 59, 59); + } else if ($operator === self::DATE_LAST) { + $datestart = $datestart->modify("-{$dateunitvalue} month"); } else if ($operator === self::DATE_NEXT) { - $datestart = $datestart->modify('first day of next month'); - $dateend = $dateend->setDate((int) $dateyear, $datemonth + $dateunitvalue, 1) - ->modify('last day of this month'); + $dateend = $dateend->modify("+{$dateunitvalue} month"); } break; case self::DATE_UNIT_YEAR: - // Current year. - $datestart = $datenow->modify('first day of january this year'); - $dateend = $datenow->modify('last day of december this year'); - - $dateyear = (int) $datenow->format('Y'); - if ($operator === self::DATE_PREVIOUS) { - $datestart = $datestart->setDate($dateyear - $dateunitvalue, 1, 1); - $dateend = $dateend->modify('last day of december last year'); + if ($operator === self::DATE_CURRENT) { + $datestart = $datestart->modify('first day of january this year')->setTime(0, 0); + $dateend = $dateend->modify('last day of december this year')->setTime(23, 59, 59); + } else if ($operator === self::DATE_LAST) { + $datestart = $datestart->modify("-{$dateunitvalue} year"); } else if ($operator === self::DATE_NEXT) { - $datestart = $datestart->modify('first day of january next year'); - $dateend = $dateend->setDate($dateyear + $dateunitvalue, 12, 31); + $dateend = $dateend->modify("+{$dateunitvalue} year"); } break; - default: - return [0, 0]; } return [ - $datestart->setTime(0, 0)->getTimestamp(), - $dateend->setTime(23, 59, 59)->getTimestamp(), + $datestart->getTimestamp(), + $dateend->getTimestamp(), ]; } } diff --git a/reportbuilder/tests/local/filters/date_test.php b/reportbuilder/tests/local/filters/date_test.php index 40daae1f9ba..d35a3baa828 100644 --- a/reportbuilder/tests/local/filters/date_test.php +++ b/reportbuilder/tests/local/filters/date_test.php @@ -122,20 +122,28 @@ class date_test extends advanced_testcase { */ public function get_sql_filter_relative_provider(): array { return [ - 'Previous day' => [date::DATE_PREVIOUS, 1, date::DATE_UNIT_DAY, '-1 day'], - 'Previous week' => [date::DATE_PREVIOUS, 1, date::DATE_UNIT_WEEK, '-1 week'], - 'Previous month' => [date::DATE_PREVIOUS, 1, date::DATE_UNIT_MONTH, 'last day of last month'], - 'Previous year' => [date::DATE_PREVIOUS, 1, date::DATE_UNIT_YEAR, 'last day of december last year'], + 'Last day' => [date::DATE_LAST, 1, date::DATE_UNIT_DAY, '-6 hour'], + 'Last week' => [date::DATE_LAST, 1, date::DATE_UNIT_WEEK, '-3 day'], + 'Last month' => [date::DATE_LAST, 1, date::DATE_UNIT_MONTH, '-3 week'], + 'Last year' => [date::DATE_LAST, 1, date::DATE_UNIT_YEAR, '-6 month'], + 'Last two days' => [date::DATE_LAST, 2, date::DATE_UNIT_DAY, '-25 hour'], + 'Last two weeks' => [date::DATE_LAST, 2, date::DATE_UNIT_WEEK, '-10 day'], + 'Last two months' => [date::DATE_LAST, 2, date::DATE_UNIT_MONTH, '-7 week'], + 'Last two years' => [date::DATE_LAST, 2, date::DATE_UNIT_YEAR, '-15 month'], 'Current day' => [date::DATE_CURRENT, null, date::DATE_UNIT_DAY], 'Current week' => [date::DATE_CURRENT, null, date::DATE_UNIT_WEEK], 'Current month' => [date::DATE_CURRENT, null, date::DATE_UNIT_MONTH], 'Current year' => [date::DATE_CURRENT, null, date::DATE_UNIT_YEAR], - 'Next day' => [date::DATE_NEXT, 1, date::DATE_UNIT_DAY, '+1 day'], - 'Next week' => [date::DATE_NEXT, 1, date::DATE_UNIT_WEEK, '+1 week'], - 'Next month' => [date::DATE_NEXT, 1, date::DATE_UNIT_MONTH, 'first day of next month'], - 'Next year' => [date::DATE_NEXT, 1, date::DATE_UNIT_YEAR, 'first day of january next year'], + 'Next day' => [date::DATE_NEXT, 1, date::DATE_UNIT_DAY, '+6 hour'], + 'Next week' => [date::DATE_NEXT, 1, date::DATE_UNIT_WEEK, '+3 day'], + 'Next month' => [date::DATE_NEXT, 1, date::DATE_UNIT_MONTH, '+3 week'], + 'Next year' => [date::DATE_NEXT, 1, date::DATE_UNIT_YEAR, '+6 month'], + 'Next two days' => [date::DATE_NEXT, 2, date::DATE_UNIT_DAY, '+25 hour'], + 'Next two weeks' => [date::DATE_NEXT, 2, date::DATE_UNIT_WEEK, '+10 day'], + 'Next two months' => [date::DATE_NEXT, 2, date::DATE_UNIT_MONTH, '+7 week'], + 'Next two years' => [date::DATE_NEXT, 2, date::DATE_UNIT_YEAR, '+15 month'], ]; }