MDL-82467 completion: simplify field SQL for completion columns.

For columns whose values can be represented purely in SQL, we don't
need to/shouldn't select extra data because it confuses the column
output during aggregation for numeric/boolean columns.
This commit is contained in:
Paul Holden 2024-07-15 11:58:55 +01:00
parent 4197e50fec
commit aa550ddea6
No known key found for this signature in database
GPG Key ID: A81A96D6045F6164
2 changed files with 58 additions and 22 deletions

View File

@ -107,15 +107,14 @@ class completion extends base {
))
->add_joins($this->get_joins())
->set_type(column::TYPE_BOOLEAN)
->add_field("CASE WHEN {$coursecompletion}.timecompleted > 0 THEN 1 ELSE 0 END", 'completed')
->add_field("{$user}.id", 'userid')
->add_field("
CASE
WHEN {$coursecompletion}.id IS NULL THEN NULL
WHEN {$coursecompletion}.timecompleted > 0 THEN 1
ELSE 0
END", 'completed')
->set_is_sortable(true)
->add_callback(static function(bool $value, stdClass $row): string {
if (!$row->userid) {
return '';
}
return format::boolean_as_text($value);
});
->add_callback([format::class, 'boolean_as_text']);
// Completion criteria column.
$criterias = database::generate_alias();
@ -232,14 +231,14 @@ class completion extends base {
->set_type(column::TYPE_INTEGER)
->add_field("(
CASE
WHEN {$coursecompletion}.timecompleted > 0 THEN
WHEN {$coursecompletion}.id IS NULL THEN NULL
ELSE (CASE WHEN {$coursecompletion}.timecompleted > 0 THEN
{$coursecompletion}.timecompleted
ELSE
ELSE
{$currenttime}
END - {$course}.startdate) / " . DAYSECS, 'dayscourse')
->add_field("{$user}.id", 'userid')
->set_is_sortable(true)
->add_callback([completion_formatter::class, 'get_days']);
END - {$course}.startdate) / " . DAYSECS . "
END)", 'dayscourse')
->set_is_sortable(true);
// Days since last completion (days since last enrolment date until completion or until current date if not completed).
$columns[] = (new column(
@ -251,14 +250,14 @@ class completion extends base {
->set_type(column::TYPE_INTEGER)
->add_field("(
CASE
WHEN {$coursecompletion}.timecompleted > 0 THEN
WHEN {$coursecompletion}.id IS NULL THEN NULL
ELSE (CASE WHEN {$coursecompletion}.timecompleted > 0 THEN
{$coursecompletion}.timecompleted
ELSE
ELSE
{$currenttime}
END - {$coursecompletion}.timeenrolled) / " . DAYSECS, 'daysuntilcompletion')
->add_field("{$user}.id", 'userid')
->set_is_sortable(true)
->add_callback([completion_formatter::class, 'get_days']);
END - {$coursecompletion}.timeenrolled) / " . DAYSECS . "
END)", 'daysuntilcompletion')
->set_is_sortable(true);
// Student course grade.
$columns[] = (new column(

View File

@ -239,7 +239,7 @@ class participants_test extends core_reportbuilder_testcase {
/**
* Test creating course report, with aggregated last access date (minimum and maximum)
* Test creating participants report, with aggregated last access date (minimum and maximum)
*/
public function test_course_last_access_aggregation(): void {
$this->resetAfterTest();
@ -255,7 +255,7 @@ class participants_test extends core_reportbuilder_testcase {
/** @var core_reportbuilder_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'Courses', 'source' => participants::class, 'default' => 0]);
$report = $generator->create_report(['name' => 'Participants', 'source' => participants::class, 'default' => 0]);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:fullname']);
$column = $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'access:timeaccess']);
@ -274,6 +274,43 @@ class participants_test extends core_reportbuilder_testcase {
], array_map('array_values', $content));
}
/**
* Test creating participants report, with aggregated days taking course column
*/
public function test_completion_days_taking_course_aggregation(): void {
$this->resetAfterTest();
$courseone = $this->getDataGenerator()->create_course(['fullname' => 'Course 1', 'startdate' => 1622502000]);
$coursetwo = $this->getDataGenerator()->create_course(['fullname' => 'Course 2']);
// User one completed the course in two days.
$userone = $this->getDataGenerator()->create_and_enrol($courseone);
$completion = new completion_completion(['course' => $courseone->id, 'userid' => $userone->id]);
$completion->mark_complete(1622502000 + (2 * DAYSECS));
// User two completed the course in three days (lazy bum).
$usertwo = $this->getDataGenerator()->create_and_enrol($courseone);
$completion = new completion_completion(['course' => $courseone->id, 'userid' => $usertwo->id]);
$completion->mark_complete(1622502000 + (3 * DAYSECS));
/** @var core_reportbuilder_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'Participants', 'source' => participants::class, 'default' => 0]);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'course:fullname', 'sortenabled' => 1]);
$generator->create_column([
'reportid' => $report->get('id'),
'uniqueidentifier' => 'completion:dayscourse',
'aggregation' => 'avg',
]);
$content = $this->get_custom_report_content($report->get('id'));
$this->assertEquals([
[$courseone->fullname, '2.5'],
[$coursetwo->fullname, ''],
], array_map('array_values', $content));
}
/**
* Data provider for {@see test_datasource_filters}
*