From 8426610258114e90eda46b849f7405ad8c875ed7 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Fri, 12 Jul 2024 18:00:49 +0100 Subject: [PATCH 1/2] MDL-82466 reportbuilder: new format helper for formatting time. --- .upgradenotes/MDL-82466-2024071510553022.yml | 7 ++++ .../reportbuilder/local/entities/task_log.php | 10 +----- .../reportbuilder/local/entities/enrol.php | 7 ++-- .../classes/local/helpers/format.php | 19 +++++++++++ .../tests/local/helpers/format_test.php | 33 +++++++++++++++++-- 5 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 .upgradenotes/MDL-82466-2024071510553022.yml diff --git a/.upgradenotes/MDL-82466-2024071510553022.yml b/.upgradenotes/MDL-82466-2024071510553022.yml new file mode 100644 index 00000000000..cd219f229ed --- /dev/null +++ b/.upgradenotes/MDL-82466-2024071510553022.yml @@ -0,0 +1,7 @@ +issueNumber: MDL-82466 +notes: + core_reportbuilder: + - message: > + New format helper `format_time` method, for use in column callbacks that + represent a duration of time (e.g. "3 days 4 hours") + type: improved diff --git a/admin/classes/reportbuilder/local/entities/task_log.php b/admin/classes/reportbuilder/local/entities/task_log.php index dc541c958b1..4922822744a 100644 --- a/admin/classes/reportbuilder/local/entities/task_log.php +++ b/admin/classes/reportbuilder/local/entities/task_log.php @@ -183,15 +183,7 @@ class task_log extends base { ->set_type(column::TYPE_FLOAT) ->add_field("{$tablealias}.timeend - {$tablealias}.timestart", 'duration') ->set_is_sortable(true) - ->add_callback(static function(float $value): string { - $duration = round($value, 2); - if (empty($duration)) { - // The format_time function returns 'now' when the difference is exactly 0. - // Note: format_time performs concatenation in exactly this fashion so we should do this for consistency. - return '0 ' . get_string('secs', 'moodle'); - } - return format_time($duration); - }); + ->add_callback([format::class, 'format_time'], 2); // Hostname column. $columns[] = (new column( diff --git a/enrol/classes/reportbuilder/local/entities/enrol.php b/enrol/classes/reportbuilder/local/entities/enrol.php index e3d8236029f..f34f16c5ba3 100644 --- a/enrol/classes/reportbuilder/local/entities/enrol.php +++ b/enrol/classes/reportbuilder/local/entities/enrol.php @@ -145,12 +145,11 @@ class enrol extends base { ->set_type(column::TYPE_TIMESTAMP) ->add_fields("{$enrolalias}.enrolperiod") ->set_is_sortable(true) - ->set_callback(static function(?int $enrolperiod): string { - if (!$enrolperiod) { + ->set_callback(static function(?int $enrolperiod, stdClass $row): string { + if ($enrolperiod === 0) { return ''; } - - return format_time($enrolperiod); + return format::format_time($enrolperiod, $row); }); // Start date column. diff --git a/reportbuilder/classes/local/helpers/format.php b/reportbuilder/classes/local/helpers/format.php index 644d7e76aa1..6b0d4520762 100644 --- a/reportbuilder/classes/local/helpers/format.php +++ b/reportbuilder/classes/local/helpers/format.php @@ -41,6 +41,25 @@ class format { return $value ? userdate($value, $format) : ''; } + /** + * Returns formatted time duration (e.g. "3 days 4 hours") + * + * @param float|null $value + * @param stdClass $row + * @param int|null $precision + * @return string + */ + public static function format_time(?float $value, stdClass $row, ?int $precision = 0): string { + if ($value === null) { + return ''; + } + $value = round($value, (int) $precision); + if ($value === 0.0) { + return '0 ' . get_string('secs', 'moodle'); + } + return format_time($value); + } + /** * Returns yes/no string depending on the given value * diff --git a/reportbuilder/tests/local/helpers/format_test.php b/reportbuilder/tests/local/helpers/format_test.php index e354b2f5531..dca11f858d9 100644 --- a/reportbuilder/tests/local/helpers/format_test.php +++ b/reportbuilder/tests/local/helpers/format_test.php @@ -29,7 +29,7 @@ use stdClass; * @copyright 2021 Paul Holden * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class format_test extends advanced_testcase { +final class format_test extends advanced_testcase { /** * Test userdate method @@ -41,12 +41,39 @@ class format_test extends advanced_testcase { $this->assertEquals(userdate($now), $userdate); } + /** + * Data provider for {@see test_format_time} + * + * @return array[] + */ + public static function format_time_provider(): array { + return [ + [null, 0, ''], + [0, 0, '0 secs'], + [2.456, 1, '2.5 secs'], + [3.2, null, '3 secs'], + ]; + } + + /** + * Test format time + * + * @param float|null $value + * @param int|null $precision + * @param string $expected + * + * @dataProvider format_time_provider + */ + public function test_format_time(?float $value, ?int $precision, string $expected): void { + $this->assertEquals($expected, format::format_time($value, (object) [], $precision)); + } + /** * Data provider for {@see test_boolean_as_text} * - * @return array + * @return array[] */ - public function boolean_as_text_provider(): array { + public static function boolean_as_text_provider(): array { return [ [false, get_string('no')], [true, get_string('yes')], From c10610d0352272d1a268bd53449765d91f07e8a5 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Mon, 15 Jul 2024 13:44:34 +0100 Subject: [PATCH 2/2] MDL-82466 completion: format time duration in report entity columns. Where the previous output was simple count of days, switch to using the new format helper to return a time duration. AMOS BEGIN CPY [daystakingcourse,core_course],[daystakingcourse,core_completion] AMOS END --- .../reportbuilder/local/entities/completion.php | 12 +++++++----- .../reportbuilder/datasource/participants_test.php | 6 +++--- lang/en/completion.php | 3 ++- lang/en/course.php | 4 +++- lang/en/deprecated.txt | 1 + 5 files changed, 16 insertions(+), 10 deletions(-) diff --git a/course/classes/reportbuilder/local/entities/completion.php b/course/classes/reportbuilder/local/entities/completion.php index 69b0db9edf5..e851c56380b 100644 --- a/course/classes/reportbuilder/local/entities/completion.php +++ b/course/classes/reportbuilder/local/entities/completion.php @@ -224,7 +224,7 @@ class completion extends base { $currenttime = time(); $columns[] = (new column( 'dayscourse', - new lang_string('daystakingcourse', 'course'), + new lang_string('daystakingcourse', 'completion'), $this->get_entity_name() )) ->add_joins($this->get_joins()) @@ -236,9 +236,10 @@ class completion extends base { {$coursecompletion}.timecompleted ELSE {$currenttime} - END - {$course}.startdate) / " . DAYSECS . " + END - {$course}.startdate) END)", 'dayscourse') - ->set_is_sortable(true); + ->set_is_sortable(true) + ->set_callback([format::class, 'format_time']); // Days since last completion (days since last enrolment date until completion or until current date if not completed). $columns[] = (new column( @@ -255,9 +256,10 @@ class completion extends base { {$coursecompletion}.timecompleted ELSE {$currenttime} - END - {$coursecompletion}.timeenrolled) / " . DAYSECS . " + END - {$coursecompletion}.timeenrolled) END)", 'daysuntilcompletion') - ->set_is_sortable(true); + ->set_is_sortable(true) + ->set_callback([format::class, 'format_time']); // Student course grade. $columns[] = (new column( diff --git a/course/tests/reportbuilder/datasource/participants_test.php b/course/tests/reportbuilder/datasource/participants_test.php index b4d4ff9eb39..9d63ab0093c 100644 --- a/course/tests/reportbuilder/datasource/participants_test.php +++ b/course/tests/reportbuilder/datasource/participants_test.php @@ -235,8 +235,8 @@ final class participants_test extends core_reportbuilder_testcase { '', // Time started. userdate($timecompleted), // Time completed. '', // Reagreggate. - 2, // Days taking course. - 2, // Days until completion. + '2 days', // Days taking course. + '2 days', // Days until completion. '42.50', // Grade. ], array_values($content[0])); } @@ -310,7 +310,7 @@ final class participants_test extends core_reportbuilder_testcase { $content = $this->get_custom_report_content($report->get('id')); $this->assertEquals([ - [$courseone->fullname, '2.5'], + [$courseone->fullname, '2 days 12 hours'], [$coursetwo->fullname, ''], ], array_map('array_values', $content)); } diff --git a/lang/en/completion.php b/lang/en/completion.php index f00fd478cc4..e7c74af1cbc 100644 --- a/lang/en/completion.php +++ b/lang/en/completion.php @@ -140,7 +140,8 @@ $string['csvdownload'] = 'Download in spreadsheet format (UTF-8 .csv)'; $string['datepassed'] = 'Date passed'; $string['days'] = 'Days'; $string['daysoftotal'] = '{$a->days} of {$a->total}'; -$string['daysuntilcompletion'] = 'Days until completion'; +$string['daystakingcourse'] = 'Time taking course'; +$string['daysuntilcompletion'] = 'Time until completion'; $string['defaultactivitycompletionsite'] = 'These are the default completion conditions for activities in all courses.'; $string['defaultactivitycompletioncourse'] = 'These are the default completion conditions for activities in this course.'; $string['defaultcompletion'] = 'Default activity completion'; diff --git a/lang/en/course.php b/lang/en/course.php index b923aaacaa4..b3da22e6ef2 100644 --- a/lang/en/course.php +++ b/lang/en/course.php @@ -85,7 +85,6 @@ $string['customfield_visibility_help'] = 'This setting determines who can view t $string['customfield_visibletoall'] = 'Everyone'; $string['customfield_visibletoteachers'] = 'Teachers'; $string['customfieldsettings'] = 'Common course custom fields settings'; -$string['daystakingcourse'] = 'Days taking course'; $string['defaultsettingscategory'] = 'Default settings'; $string['downloadcourseconfirmation'] = 'You are about to download a zip file of course content (excluding items which cannot be downloaded and any files larger than {$a}).'; $string['downloadcoursecontent'] = 'Download course content'; @@ -167,3 +166,6 @@ $string['gotosection'] = 'Go to section {$a}'; // Deprecated since Moodle 4.3. $string['aria:courseimage'] = 'Course image'; + +// Deprecated since Moodle 4.5. +$string['daystakingcourse'] = 'Days taking course'; diff --git a/lang/en/deprecated.txt b/lang/en/deprecated.txt index a9924fa2680..7313ba12e01 100644 --- a/lang/en/deprecated.txt +++ b/lang/en/deprecated.txt @@ -116,6 +116,7 @@ coursecalendar,core_calendar importcalendarexternal,core_calendar nocalendarsubscriptions,core_calendar datechanged,core +daystakingcourse,core_course siteregistrationcontact,core_hub siteregistrationcontact_help,core_hub registrationcontactno,core