From bcd4923ae04228eafdac4542c08316037f118074 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Mon, 3 Mar 2025 10:25:30 +0000 Subject: [PATCH] MDL-84795 reportbuilder: consistent/performant loading of context data. Ensure we select only context related fields as provided by the helper, and avoid empty joins by loading instances with the helper-specified ID field `{$tablealias}.ctxid`. --- .../reportbuilder/local/entities/role.php | 8 +-- .../reportbuilder/local/entities/badge.php | 17 +++--- .../reportbuilder/local/entities/cohort.php | 38 ++++++------- .../local/systemreports/cohorts.php | 4 +- .../reportbuilder/local/entities/comment.php | 19 +++---- .../local/entities/competency.php | 4 +- .../local/entities/framework.php | 4 +- .../local/entities/course_category.php | 47 +++++++-------- .../reportbuilder/local/entities/group.php | 26 ++++----- .../reportbuilder/local/entities/grouping.php | 26 ++++----- .../reportbuilder/local/entities/context.php | 22 ++++--- .../classes/local/entities/course.php | 57 ++++++++++--------- reportbuilder/classes/local/entities/user.php | 24 ++++---- 13 files changed, 138 insertions(+), 158 deletions(-) diff --git a/admin/roles/classes/reportbuilder/local/entities/role.php b/admin/roles/classes/reportbuilder/local/entities/role.php index b22cc57b604..c714a526c29 100644 --- a/admin/roles/classes/reportbuilder/local/entities/role.php +++ b/admin/roles/classes/reportbuilder/local/entities/role.php @@ -93,7 +93,7 @@ class role extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->add_fields("{$rolealias}.name, {$rolealias}.shortname, {$rolealias}.id, {$contextalias}.id AS contextid") + ->add_fields("{$rolealias}.name, {$rolealias}.shortname, {$rolealias}.id") ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) // The sorting is on name, unless empty then we use shortname. ->set_is_sortable(true, [ @@ -103,12 +103,12 @@ class role extends base { END", ]) ->add_callback(static function(?string $name, stdClass $role): string { - if ($name === null) { + if ($name === null || $role->ctxid === null) { return ''; } - context_helper::preload_from_record($role); - $context = context::instance_by_id($role->contextid); + context_helper::preload_from_record(clone $role); + $context = context::instance_by_id($role->ctxid); return role_get_name($role, $context, ROLENAME_BOTH); }); diff --git a/badges/classes/reportbuilder/local/entities/badge.php b/badges/classes/reportbuilder/local/entities/badge.php index e01831490d3..7d5937ec936 100644 --- a/badges/classes/reportbuilder/local/entities/badge.php +++ b/badges/classes/reportbuilder/local/entities/badge.php @@ -18,9 +18,8 @@ declare(strict_types=1); namespace core_badges\reportbuilder\local\entities; -use context_course; -use context_helper; -use context_system; +use core\{context, context_helper}; +use core\context\system; use html_writer; use lang_string; use moodle_url; @@ -158,6 +157,8 @@ class badge extends base { if (empty($badge->criteria)) { return ''; } + + /** @var \core_badges_renderer $renderer */ $renderer = $PAGE->get_renderer('core_badges'); return $renderer->print_badge_criteria($badge, 'short'); }); @@ -172,17 +173,17 @@ class badge extends base { ->add_join("LEFT JOIN {context} {$contextalias} ON {$contextalias}.contextlevel = " . CONTEXT_COURSE . " AND {$contextalias}.instanceid = {$badgealias}.courseid") - ->add_fields("{$badgealias}.id, {$badgealias}.type, {$badgealias}.courseid, {$badgealias}.imagecaption") + ->add_fields("{$badgealias}.id, {$badgealias}.type, {$badgealias}.imagecaption") ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) ->add_callback(static function($value, stdClass $badge): string { - if ($badge->id === null) { + if ($value === null) { return ''; } if ($badge->type == BADGE_TYPE_SITE) { - $context = context_system::instance(); + $context = system::instance(); } else { - context_helper::preload_from_record($badge); - $context = context_course::instance($badge->courseid); + context_helper::preload_from_record(clone $badge); + $context = context::instance_by_id($badge->ctxid); } $badgeimage = moodle_url::make_pluginfile_url($context->id, 'badges', 'badgeimage', $badge->id, '/', 'f2'); diff --git a/cohort/classes/reportbuilder/local/entities/cohort.php b/cohort/classes/reportbuilder/local/entities/cohort.php index 4fca506d7ad..b9973598208 100644 --- a/cohort/classes/reportbuilder/local/entities/cohort.php +++ b/cohort/classes/reportbuilder/local/entities/cohort.php @@ -18,11 +18,10 @@ declare(strict_types=1); namespace core_cohort\reportbuilder\local\entities; -use context; -use context_helper; use lang_string; use stdClass; use theme_config; +use core\{context, context_helper}; use core_reportbuilder\local\entities\base; use core_reportbuilder\local\filters\boolean_select; use core_reportbuilder\local\filters\cohort as cohort_filter; @@ -113,16 +112,17 @@ class cohort extends base { )) ->add_joins($this->get_joins()) ->add_join($this->get_context_join()) - ->set_type(column::TYPE_TEXT) - ->add_fields("{$tablealias}.contextid, " . context_helper::get_preload_record_columns_sql($contextalias)) + ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) ->set_is_sortable(true) ->add_callback(static function($contextid, stdClass $cohort): string { - if ($contextid === null) { + if ($cohort->ctxid === null) { return ''; } - context_helper::preload_from_record($cohort); - return context::instance_by_id($cohort->contextid)->get_context_name(false); + context_helper::preload_from_record(clone $cohort); + $context = context::instance_by_id($cohort->ctxid); + + return $context->get_context_name(false); }); // Name column. @@ -132,7 +132,6 @@ class cohort extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$tablealias}.name") ->set_is_sortable(true); @@ -143,7 +142,6 @@ class cohort extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$tablealias}.idnumber") ->set_is_sortable(true); @@ -156,24 +154,24 @@ class cohort extends base { ->add_joins($this->get_joins()) ->add_join($this->get_context_join()) ->set_type(column::TYPE_LONGTEXT) - ->add_fields("{$tablealias}.description, {$tablealias}.descriptionformat, {$tablealias}.id, {$tablealias}.contextid") + ->add_fields("{$tablealias}.description, {$tablealias}.descriptionformat, {$tablealias}.id") ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) ->set_is_sortable(true) ->add_callback(static function(?string $description, stdClass $cohort): string { global $CFG; require_once("{$CFG->libdir}/filelib.php"); - if ($description === null) { + if ($description === null || $cohort->ctxid === null) { return ''; } - context_helper::preload_from_record($cohort); - $context = context::instance_by_id($cohort->contextid); + context_helper::preload_from_record(clone $cohort); + $context = context::instance_by_id($cohort->ctxid); $description = file_rewrite_pluginfile_urls($description, 'pluginfile.php', $context->id, 'cohort', 'description', $cohort->id); - return format_text($description, $cohort->descriptionformat, ['context' => $context->id]); + return format_text($description, $cohort->descriptionformat, ['context' => $context]); }); // Visible column. @@ -219,7 +217,6 @@ class cohort extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$tablealias}.component") ->set_is_sortable(true) ->add_callback(static function(?string $component): string { @@ -239,7 +236,6 @@ class cohort extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$tablealias}.theme") ->set_is_sortable(true) ->add_callback(static function (?string $theme): string { @@ -286,16 +282,16 @@ class cohort extends base { // Load all contexts in which there are cohorts. $ctxfields = context_helper::get_preload_record_columns_sql('ctx'); $contexts = $DB->get_records_sql(" - SELECT DISTINCT {$ctxfields}, c.contextid + SELECT DISTINCT {$ctxfields} FROM {context} ctx JOIN {cohort} c ON c.contextid = ctx.id"); // Transform context record into it's name (used as the filter options). - return array_map(static function(stdClass $contextrecord): string { - context_helper::preload_from_record($contextrecord); + return array_map(static function(stdClass $context): string { + context_helper::preload_from_record(clone $context); + $context = context::instance_by_id($context->ctxid); - return context::instance_by_id($contextrecord->contextid) - ->get_context_name(false); + return $context->get_context_name(false); }, $contexts); }); diff --git a/cohort/classes/reportbuilder/local/systemreports/cohorts.php b/cohort/classes/reportbuilder/local/systemreports/cohorts.php index 668c1203022..d86106a26fa 100644 --- a/cohort/classes/reportbuilder/local/systemreports/cohorts.php +++ b/cohort/classes/reportbuilder/local/systemreports/cohorts.php @@ -112,10 +112,10 @@ class cohorts extends system_report { if ($this->get_context() instanceof context_system && $this->get_parameter('showall', false, PARAM_BOOL)) { $this->add_column_from_entity('cohort:context') ->add_callback(static function(string $value, stdClass $cohort): string { - $context = context::instance_by_id($cohort->contextid); + $context = context::instance_by_id($cohort->ctxid); if ($context instanceof context_coursecat) { return html_writer::link(new moodle_url('/cohort/index.php', - ['contextid' => $cohort->contextid]), $value); + ['contextid' => $context->id]), $value); } return $value; diff --git a/comment/classes/reportbuilder/local/entities/comment.php b/comment/classes/reportbuilder/local/entities/comment.php index a890e4bdf40..fddea34577f 100644 --- a/comment/classes/reportbuilder/local/entities/comment.php +++ b/comment/classes/reportbuilder/local/entities/comment.php @@ -18,14 +18,13 @@ declare(strict_types=1); namespace core_comment\reportbuilder\local\entities; -use context; -use context_helper; -use lang_string; -use stdClass; +use core\{context, context_helper}; +use core\lang_string; use core_reportbuilder\local\entities\base; use core_reportbuilder\local\filters\{date, text}; use core_reportbuilder\local\helpers\format; use core_reportbuilder\local\report\{column, filter}; +use stdClass; /** * Comment entity @@ -97,16 +96,16 @@ class comment extends base { ->add_joins($this->get_joins()) ->set_type(column::TYPE_LONGTEXT) ->add_join($this->get_context_join()) - ->add_fields("{$commentalias}.content, {$commentalias}.format, {$commentalias}.contextid") + ->add_fields("{$commentalias}.content, {$commentalias}.format") ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) ->set_is_sortable(true) - ->add_callback(static function($content, stdClass $comment): string { - if ($content === null) { + ->add_callback(static function(?string $content, stdClass $comment): string { + if ($content === null || $comment->ctxid === null) { return ''; } - context_helper::preload_from_record($comment); - $context = context::instance_by_id($comment->contextid); + context_helper::preload_from_record(clone $comment); + $context = context::instance_by_id($comment->ctxid); return format_text($content, $comment->format, ['context' => $context]); }); @@ -118,7 +117,6 @@ class comment extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$commentalias}.component") ->set_is_sortable(true); @@ -129,7 +127,6 @@ class comment extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$commentalias}.commentarea") ->set_is_sortable(true); diff --git a/competency/classes/reportbuilder/local/entities/competency.php b/competency/classes/reportbuilder/local/entities/competency.php index 3b70ce414ee..7932fb5ef49 100644 --- a/competency/classes/reportbuilder/local/entities/competency.php +++ b/competency/classes/reportbuilder/local/entities/competency.php @@ -110,14 +110,14 @@ class competency extends base { ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) ->set_is_sortable(true) ->add_callback(static function(?string $description, stdClass $competency): string { - if ($description === null) { + if ($description === null || $competency->ctxid === null) { return ''; } context_helper::preload_from_record(clone $competency); $context = context::instance_by_id($competency->ctxid); - return format_text($description, $competency->descriptionformat, ['context' => $context->id]); + return format_text($description, $competency->descriptionformat, ['context' => $context]); }); // ID number. diff --git a/competency/classes/reportbuilder/local/entities/framework.php b/competency/classes/reportbuilder/local/entities/framework.php index 322616f4140..d41c704ee6f 100644 --- a/competency/classes/reportbuilder/local/entities/framework.php +++ b/competency/classes/reportbuilder/local/entities/framework.php @@ -110,14 +110,14 @@ class framework extends base { ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) ->set_is_sortable(true) ->add_callback(static function(?string $description, stdClass $framework): string { - if ($description === null) { + if ($description === null || $framework->ctxid === null) { return ''; } context_helper::preload_from_record(clone $framework); $context = context::instance_by_id($framework->ctxid); - return format_text($description, $framework->descriptionformat, ['context' => $context->id]); + return format_text($description, $framework->descriptionformat, ['context' => $context]); }); // ID number. diff --git a/course/classes/reportbuilder/local/entities/course_category.php b/course/classes/reportbuilder/local/entities/course_category.php index a55d878e3b6..7c891e805da 100644 --- a/course/classes/reportbuilder/local/entities/course_category.php +++ b/course/classes/reportbuilder/local/entities/course_category.php @@ -18,11 +18,10 @@ declare(strict_types=1); namespace core_course\reportbuilder\local\entities; -use context_coursecat; -use context_helper; +use core\{context, context_helper}; +use core\url; use html_writer; use lang_string; -use moodle_url; use stdClass; use theme_config; use core_course_category; @@ -100,18 +99,17 @@ class course_category extends base { )) ->add_joins($this->get_joins()) ->add_join($this->get_context_join()) - ->set_type(column::TYPE_TEXT) - ->add_fields("{$tablealias}.name, {$tablealias}.id") + ->add_field("{$tablealias}.name") ->add_fields(context_helper::get_preload_record_columns_sql($tablealiascontext)) ->add_callback(static function(?string $name, stdClass $category): string { - if (empty($category->id)) { + if ($name === null || $category->ctxid === null) { return ''; } - context_helper::preload_from_record($category); - $context = context_coursecat::instance($category->id); + context_helper::preload_from_record(clone $category); + $context = context::instance_by_id($category->ctxid); - return format_string($category->name, true, ['context' => $context]); + return format_string($name, true, ['context' => $context]); }) ->set_is_sortable(true); @@ -123,18 +121,20 @@ class course_category extends base { )) ->add_joins($this->get_joins()) ->add_join($this->get_context_join()) - ->set_type(column::TYPE_TEXT) - ->add_fields("{$tablealias}.name, {$tablealias}.id") + ->add_field("{$tablealias}.name") ->add_fields(context_helper::get_preload_record_columns_sql($tablealiascontext)) ->add_callback(static function(?string $name, stdClass $category): string { - if (empty($category->id)) { + if ($name === null || $category->ctxid === null) { return ''; } - context_helper::preload_from_record($category); - $context = context_coursecat::instance($category->id); - $url = new moodle_url('/course/management.php', ['categoryid' => $category->id]); - return html_writer::link($url, - format_string($category->name, true, ['context' => $context])); + + context_helper::preload_from_record(clone $category); + $context = context::instance_by_id($category->ctxid); + + return html_writer::link( + new url('/course/management.php', ['categoryid' => $context->instanceid]), + format_string($name, true, ['context' => $context]), + ); }) ->set_is_sortable(true); @@ -145,7 +145,6 @@ class course_category extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$tablealias}.name, {$tablealias}.id") ->add_callback(static function(?string $name, stdClass $category): string { return empty($category->id) ? '' : @@ -161,7 +160,6 @@ class course_category extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$tablealias}.idnumber") ->set_is_sortable(true); @@ -174,24 +172,24 @@ class course_category extends base { ->add_joins($this->get_joins()) ->add_join($this->get_context_join()) ->set_type(column::TYPE_LONGTEXT) - ->add_fields("{$tablealias}.description, {$tablealias}.descriptionformat, {$tablealias}.id") + ->add_fields("{$tablealias}.description, {$tablealias}.descriptionformat") ->add_fields(context_helper::get_preload_record_columns_sql($tablealiascontext)) ->set_is_sortable(true) ->add_callback(static function(?string $description, stdClass $category): string { global $CFG; require_once("{$CFG->libdir}/filelib.php"); - if ($description === null) { + if ($description === null || $category->ctxid === null) { return ''; } - context_helper::preload_from_record($category); - $context = context_coursecat::instance($category->id); + context_helper::preload_from_record(clone $category); + $context = context::instance_by_id($category->ctxid); $description = file_rewrite_pluginfile_urls($description, 'pluginfile.php', $context->id, 'coursecat', 'description', null); - return format_text($description, $category->descriptionformat, ['context' => $context->id]); + return format_text($description, $category->descriptionformat, ['context' => $context]); }); // Theme column. @@ -201,7 +199,6 @@ class course_category extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$tablealias}.theme") ->set_is_sortable(true) ->add_callback(static function (?string $theme): string { diff --git a/group/classes/reportbuilder/local/entities/group.php b/group/classes/reportbuilder/local/entities/group.php index 8eaf901e9ee..3adfc37305c 100644 --- a/group/classes/reportbuilder/local/entities/group.php +++ b/group/classes/reportbuilder/local/entities/group.php @@ -18,8 +18,7 @@ declare(strict_types=1); namespace core_group\reportbuilder\local\entities; -use context_course; -use context_helper; +use core\{context, context_helper}; use html_writer; use lang_string; use moodle_url; @@ -112,19 +111,18 @@ class group extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) - ->add_fields("{$groupsalias}.name, {$groupsalias}.courseid") + ->add_field("{$groupsalias}.name") ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) ->set_is_sortable(true) - ->set_callback(static function($name, stdClass $group): string { - if ($name === null) { + ->set_callback(static function(?string $name, stdClass $group): string { + if ($name === null || $group->ctxid === null) { return ''; } - context_helper::preload_from_record($group); - $context = context_course::instance($group->courseid); + context_helper::preload_from_record(clone $group); + $context = context::instance_by_id($group->ctxid); - return format_string($group->name, true, ['context' => $context]); + return format_string($name, true, ['context' => $context]); }); // ID number column. @@ -134,7 +132,6 @@ class group extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$groupsalias}.idnumber") ->set_is_sortable(true); @@ -146,20 +143,20 @@ class group extends base { )) ->add_joins($this->get_joins()) ->set_type(column::TYPE_LONGTEXT) - ->add_fields("{$groupsalias}.description, {$groupsalias}.descriptionformat, {$groupsalias}.id, {$groupsalias}.courseid") + ->add_fields("{$groupsalias}.description, {$groupsalias}.descriptionformat, {$groupsalias}.id") ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) ->set_is_sortable(true) ->set_callback(static function(?string $description, stdClass $group): string { global $CFG; - if ($description === null) { + if ($description === null || $group->ctxid === null) { return ''; } require_once("{$CFG->libdir}/filelib.php"); - context_helper::preload_from_record($group); - $context = context_course::instance($group->courseid); + context_helper::preload_from_record(clone $group); + $context = context::instance_by_id($group->ctxid); $description = file_rewrite_pluginfile_urls($description, 'pluginfile.php', $context->id, 'group', 'description', $group->id); @@ -174,7 +171,6 @@ class group extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$groupsalias}.enrolmentkey") ->set_is_sortable(true); diff --git a/group/classes/reportbuilder/local/entities/grouping.php b/group/classes/reportbuilder/local/entities/grouping.php index edee4cd4cb4..f6e00ccca2d 100644 --- a/group/classes/reportbuilder/local/entities/grouping.php +++ b/group/classes/reportbuilder/local/entities/grouping.php @@ -18,8 +18,7 @@ declare(strict_types=1); namespace core_group\reportbuilder\local\entities; -use context_course; -use context_helper; +use core\{context, context_helper}; use lang_string; use stdClass; use core_reportbuilder\local\entities\base; @@ -105,19 +104,18 @@ class grouping extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) - ->add_fields("{$groupingsalias}.name, {$groupingsalias}.courseid") + ->add_field("{$groupingsalias}.name") ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) ->set_is_sortable(true) - ->set_callback(static function($name, stdClass $grouping): string { - if ($name === null) { + ->set_callback(static function(?string $name, stdClass $grouping): string { + if ($name === null || $grouping->ctxid === null) { return ''; } - context_helper::preload_from_record($grouping); - $context = context_course::instance($grouping->courseid); + context_helper::preload_from_record(clone $grouping); + $context = context::instance_by_id($grouping->ctxid); - return format_string($grouping->name, true, ['context' => $context]); + return format_string($name, true, ['context' => $context]); }); // ID number column. @@ -127,7 +125,6 @@ class grouping extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields("{$groupingsalias}.idnumber") ->set_is_sortable(true); @@ -139,21 +136,20 @@ class grouping extends base { )) ->add_joins($this->get_joins()) ->set_type(column::TYPE_LONGTEXT) - ->add_field("{$groupingsalias}.description") - ->add_fields("{$groupingsalias}.descriptionformat, {$groupingsalias}.id, {$groupingsalias}.courseid") + ->add_fields("{$groupingsalias}.description, {$groupingsalias}.descriptionformat, {$groupingsalias}.id") ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) ->set_is_sortable(true) ->set_callback(static function(?string $description, stdClass $grouping): string { global $CFG; - if ($description === null) { + if ($description === null || $grouping->ctxid === null) { return ''; } require_once("{$CFG->libdir}/filelib.php"); - context_helper::preload_from_record($grouping); - $context = context_course::instance($grouping->courseid); + context_helper::preload_from_record(clone $grouping); + $context = context::instance_by_id($grouping->ctxid); $description = file_rewrite_pluginfile_urls($description, 'pluginfile.php', $context->id, 'grouping', 'description', $grouping->id); diff --git a/lib/classes/reportbuilder/local/entities/context.php b/lib/classes/reportbuilder/local/entities/context.php index d18c184707b..aea22e08fb3 100644 --- a/lib/classes/reportbuilder/local/entities/context.php +++ b/lib/classes/reportbuilder/local/entities/context.php @@ -19,11 +19,11 @@ declare(strict_types=1); namespace core\reportbuilder\local\entities; use core\context_helper; +use core\lang_string; +use core\output\html_writer; use core_reportbuilder\local\entities\base; use core_reportbuilder\local\filters\{select, text}; use core_reportbuilder\local\report\{column, filter}; -use html_writer; -use lang_string; use stdClass; /** @@ -94,17 +94,18 @@ class context extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) // Sorting may not order alphabetically, but will at least group contexts together. ->set_is_sortable(true) ->add_callback(static function($contextid, stdClass $context): string { - if ($contextid === null) { + if ($context->ctxid === null) { return ''; } - context_helper::preload_from_record($context); - return context_helper::instance_by_id($contextid)->get_context_name(); + context_helper::preload_from_record(clone $context); + $context = context_helper::instance_by_id($context->ctxid); + + return $context->get_context_name(); }); // Link. @@ -114,17 +115,16 @@ class context extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_fields(context_helper::get_preload_record_columns_sql($contextalias)) // Sorting may not order alphabetically, but will at least group contexts together. ->set_is_sortable(true) ->add_callback(static function($contextid, stdClass $context): string { - if ($contextid === null) { + if ($context->ctxid === null) { return ''; } - context_helper::preload_from_record($context); - $context = context_helper::instance_by_id($contextid); + context_helper::preload_from_record(clone $context); + $context = context_helper::instance_by_id($context->ctxid); return html_writer::link($context->get_url(), $context->get_context_name()); }); @@ -153,7 +153,6 @@ class context extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) ->add_field("{$contextalias}.path") ->set_is_sortable(true); @@ -164,7 +163,6 @@ class context extends base { $this->get_entity_name() )) ->add_joins($this->get_joins()) - ->set_type(column::TYPE_TEXT) // The "path" column looks like "/1/2/3", for context ID 3. In order to select/group by the parent context, we // concatenate a trailing slash (to prevent partial matches, e.g. "/1/2/31"), then replace "/3/" with empty string. ->add_field(" diff --git a/reportbuilder/classes/local/entities/course.php b/reportbuilder/classes/local/entities/course.php index 2095d296b0b..ba91483177c 100644 --- a/reportbuilder/classes/local/entities/course.php +++ b/reportbuilder/classes/local/entities/course.php @@ -18,8 +18,7 @@ declare(strict_types=1); namespace core_reportbuilder\local\entities; -use context_course; -use context_helper; +use core\{context, context_helper}; use core_reportbuilder\local\filters\boolean_select; use core_reportbuilder\local\filters\course_selector; use core_reportbuilder\local\filters\date; @@ -215,33 +214,30 @@ class course extends base { 'courseidnumberewithlink' => 'idnumber', ]; foreach ($fields as $key => $field) { - $column = (new column( + $columns[] = (new column( $key, new lang_string($key, 'core_reportbuilder'), $this->get_entity_name() )) ->add_joins($this->get_joins()) + ->add_join($this->get_context_join()) ->set_type(column::TYPE_TEXT) - ->add_fields("{$tablealias}.{$field} as $key, {$tablealias}.id") + ->add_field("{$tablealias}.{$field}") + ->add_fields(context_helper::get_preload_record_columns_sql($contexttablealias)) ->set_is_sortable(true) - ->add_callback(static function(?string $value, stdClass $row): string { - if ($value === null) { + ->add_callback(static function(?string $value, stdClass $course): string { + if ($value === null || $course->ctxid === null) { return ''; } - context_helper::preload_from_record($row); + context_helper::preload_from_record(clone $course); + $context = context::instance_by_id($course->ctxid); - return html_writer::link(course_get_url($row->id), - format_string($value, true, ['context' => context_course::instance($row->id)])); + return html_writer::link( + course_get_url($context->instanceid), + format_string($value, true, ['context' => $context], + )); }); - - // Join on the context table so that we can use it for formatting these columns later. - if ($key === 'coursefullnamewithlink') { - $column->add_join($this->get_context_join()) - ->add_fields(context_helper::get_preload_record_columns_sql($contexttablealias)); - } - - $columns[] = $column; } foreach ($coursefields as $coursefield => $coursefieldlang) { @@ -261,10 +257,13 @@ class course extends base { // Join on the context table so that we can use it for formatting these columns later. if ($coursefield === 'summary' || $coursefield === 'shortname' || $coursefield === 'fullname') { $column->add_join($this->get_context_join()) - ->add_field("{$tablealias}.id", 'courseid') ->add_fields(context_helper::get_preload_record_columns_sql($contexttablealias)); } + if ($coursefield === 'summary') { + $column->add_field("{$tablealias}.summaryformat"); + } + $columns[] = $column; } @@ -428,22 +427,26 @@ class course extends base { } if (in_array($fieldname, ['fullname', 'shortname'])) { - if (!$row->courseid) { + if ($value === null || $row->ctxid === null) { return ''; } - context_helper::preload_from_record($row); - $context = context_course::instance($row->courseid); - return format_string($value, true, ['context' => $context->id, 'escape' => false]); + + context_helper::preload_from_record(clone $row); + $context = context::instance_by_id($row->ctxid); + + return format_string($value, true, ['context' => $context, 'escape' => false]); } if (in_array($fieldname, ['summary'])) { - if (!$row->courseid) { + if ($value === null || $row->ctxid === null) { return ''; } - context_helper::preload_from_record($row); - $context = context_course::instance($row->courseid); - $summary = file_rewrite_pluginfile_urls($row->summary, 'pluginfile.php', $context->id, 'course', 'summary', null); - return format_text($summary); + + context_helper::preload_from_record(clone $row); + $context = context::instance_by_id($row->ctxid); + + $summary = file_rewrite_pluginfile_urls($value, 'pluginfile.php', $context->id, 'course', 'summary', null); + return format_text($summary, $row->summaryformat, ['context' => $context]); } return s($value); diff --git a/reportbuilder/classes/local/entities/user.php b/reportbuilder/classes/local/entities/user.php index c1b412f4a30..104cd99e880 100644 --- a/reportbuilder/classes/local/entities/user.php +++ b/reportbuilder/classes/local/entities/user.php @@ -18,10 +18,8 @@ declare(strict_types=1); namespace core_reportbuilder\local\entities; -use context_helper; -use context_system; -use context_user; -use core\context; +use core\{context, context_helper}; +use core\context\system; use core_component; use core_date; use core_user; @@ -178,8 +176,6 @@ class user extends base { * @return column[] */ protected function get_all_columns(): array { - global $DB; - $usertablealias = $this->get_table_alias('user'); $contexttablealias = $this->get_table_alias('context'); @@ -187,7 +183,7 @@ class user extends base { $fullnamesort = explode(', ', $fullnameselect); $userpictureselect = fields::for_userpic()->get_sql($usertablealias, false, '', '', false)->selects; - $viewfullnames = has_capability('moodle/site:viewfullnames', context_system::instance()); + $viewfullnames = has_capability('moodle/site:viewfullnames', system::instance()); // Fullname column. $columns[] = (new column( @@ -305,7 +301,7 @@ class user extends base { ->add_join("LEFT JOIN {context} {$contexttablealias} ON {$contexttablealias}.contextlevel = " . CONTEXT_USER . " AND {$contexttablealias}.instanceid = {$usertablealias}.id") - ->add_fields("{$usertablealias}.descriptionformat, {$usertablealias}.id") + ->add_field("{$usertablealias}.descriptionformat") ->add_fields(context_helper::get_preload_record_columns_sql($contexttablealias)); } @@ -341,17 +337,17 @@ class user extends base { } if ($fieldname === 'description') { - if (empty($row->id)) { + if ($value === null || $row->ctxid === null) { return ''; } require_once("{$CFG->libdir}/filelib.php"); - context_helper::preload_from_record($row); - $context = context_user::instance($row->id); + context_helper::preload_from_record(clone $row); + $context = context::instance_by_id($row->ctxid); $description = file_rewrite_pluginfile_urls($value, 'pluginfile.php', $context->id, 'user', 'profile', null); - return format_text($description, $row->descriptionformat, ['context' => $context->id]); + return format_text($description, $row->descriptionformat, ['context' => $context]); } return s($value); @@ -369,7 +365,7 @@ class user extends base { $namefields = fields::get_name_fields(true); - $viewfullnames = has_capability('moodle/site:viewfullnames', context_system::instance()); + $viewfullnames = has_capability('moodle/site:viewfullnames', system::instance()); $dummyfullname = core_user::get_dummy_fullname(null, ['override' => $viewfullnames]); // Extract any name fields from the fullname format in the order that they appear. @@ -463,7 +459,7 @@ class user extends base { $tablealias = $this->get_table_alias('user'); // Fullname filter. - $canviewfullnames = has_capability('moodle/site:viewfullnames', context_system::instance()); + $canviewfullnames = has_capability('moodle/site:viewfullnames', system::instance()); [$fullnamesql, $fullnameparams] = fields::get_sql_fullname($tablealias, $canviewfullnames); $filters[] = (new filter( text::class,