Skip to content

Commit

Permalink
MDL-78117 reportbuilder: use context entity in report sources.
Browse files Browse the repository at this point in the history
Deprecate existing columns, to be replaced by corresponding elements
from the new entity.
  • Loading branch information
paulholden committed Aug 2, 2023
1 parent beea104 commit d8df4ad
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 21 deletions.
11 changes: 9 additions & 2 deletions comment/classes/reportbuilder/datasource/comments.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

namespace core_comment\reportbuilder\datasource;

use core\reportbuilder\local\entities\context;
use core_reportbuilder\datasource;
use core_reportbuilder\local\entities\user;
use core_comment\reportbuilder\local\entities\comment;
Expand Down Expand Up @@ -48,9 +49,15 @@ protected function initialise(): void {
$commentalias = $commententity->get_table_alias('comments');

$this->set_main_table('comments', $commentalias);

$this->add_entity($commententity);

// Join the context entity.
$contextentity = (new context())
->set_table_alias('context', $commententity->get_table_alias('context'));
$this->add_entity($contextentity
->add_join($commententity->get_context_join())
);

// Join the user entity to the comment userid (author).
$userentity = new user();
$useralias = $userentity->get_table_alias('user');
Expand All @@ -68,7 +75,7 @@ protected function initialise(): void {
*/
public function get_default_columns(): array {
return [
'comment:context',
'context:name',
'comment:content',
'user:fullname',
'comment:timecreated',
Expand Down
20 changes: 17 additions & 3 deletions comment/classes/reportbuilder/local/entities/comment.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ protected function get_all_columns(): array {
))
->add_joins($this->get_joins())
->set_type(column::TYPE_LONGTEXT)
->add_join("LEFT JOIN {context} {$contextalias} ON {$contextalias}.id = {$commentalias}.contextid")
->add_join($this->get_context_join())
->add_field($contentfieldsql, 'content')
->add_fields("{$commentalias}.format, {$commentalias}.contextid, " .
context_helper::get_preload_record_columns_sql($contextalias))
Expand All @@ -126,10 +126,11 @@ protected function get_all_columns(): array {
))
->add_joins($this->get_joins())
->set_type(column::TYPE_TEXT)
->add_join("LEFT JOIN {context} {$contextalias} ON {$contextalias}.id = {$commentalias}.contextid")
->add_join($this->get_context_join())
->add_fields("{$commentalias}.contextid, " . context_helper::get_preload_record_columns_sql($contextalias))
// Sorting may not order alphabetically, but will at least group contexts together.
->set_is_sortable(true)
->set_is_deprecated('See \'context:name\' for replacement')
->add_callback(static function($contextid, stdClass $context): string {
if ($contextid === null) {
return '';
Expand All @@ -147,10 +148,11 @@ protected function get_all_columns(): array {
))
->add_joins($this->get_joins())
->set_type(column::TYPE_TEXT)
->add_join("LEFT JOIN {context} {$contextalias} ON {$contextalias}.id = {$commentalias}.contextid")
->add_join($this->get_context_join())
->add_fields("{$commentalias}.contextid, " . context_helper::get_preload_record_columns_sql($contextalias))
// Sorting may not order alphabetically, but will at least group contexts together.
->set_is_sortable(true)
->set_is_deprecated('See \'context:link\' for replacement')
->add_callback(static function($contextid, stdClass $context): string {
if ($contextid === null) {
return '';
Expand Down Expand Up @@ -246,4 +248,16 @@ protected function get_all_filters(): array {

return $filters;
}

/**
* Return syntax for joining on the context table
*
* @return string
*/
public function get_context_join(): string {
$commentalias = $this->get_table_alias('comments');
$contextalias = $this->get_table_alias('context');

return "LEFT JOIN {context} {$contextalias} ON {$contextalias}.id = {$commentalias}.contextid";
}
}
16 changes: 13 additions & 3 deletions comment/tests/reportbuilder/datasource/comments_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use core_comment_generator;
use core_reportbuilder_generator;
use core_reportbuilder_testcase;
use core_reportbuilder\local\filters\{date, text};
use core_reportbuilder\local\filters\{date, select, text};

defined('MOODLE_INTERNAL') || die();

Expand Down Expand Up @@ -98,7 +98,7 @@ public function test_datasource_non_default_columns(): void {
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'Blogs', 'source' => comments::class, 'default' => 0]);

$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'comment:contexturl']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'context:link']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'comment:component']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'comment:area']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'comment:itemid']);
Expand Down Expand Up @@ -139,7 +139,17 @@ public function datasource_filters_provider(): array {
'comment:timecreated_to' => 1622502000,
], false],

// User (just to check the join).
// Context.
'Context level' => ['context:level', [
'context:level_operator' => select::EQUAL_TO,
'context:level_value' => CONTEXT_COURSE,
], true],
'Context level (no match)' => ['context:level', [
'context:level_operator' => select::EQUAL_TO,
'context:level_value' => CONTEXT_BLOCK,
], false],

// User.
'Filter user' => ['user:username', [
'user:username_operator' => text::IS_EQUAL_TO,
'user:username_value' => 'admin',
Expand Down
10 changes: 9 additions & 1 deletion files/classes/reportbuilder/datasource/files.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

namespace core_files\reportbuilder\datasource;

use core\reportbuilder\local\entities\context;
use core_files\reportbuilder\local\entities\file;
use core_reportbuilder\datasource;
use core_reportbuilder\local\entities\user;
Expand Down Expand Up @@ -51,6 +52,13 @@ protected function initialise(): void {
$this->set_main_table('files', $filesalias);
$this->add_entity($fileentity);

// Join the context entity.
$contextentity = new context();
$contextalias = $contextentity->get_table_alias('context');
$this->add_entity($contextentity
->add_join("LEFT JOIN {context} {$contextalias} ON {$contextalias}.id = {$filesalias}.contextid")
);

// Join the user entity.
$userentity = new user();
$useralias = $userentity->get_table_alias('user');
Expand All @@ -69,7 +77,7 @@ protected function initialise(): void {
*/
public function get_default_columns(): array {
return [
'file:context',
'context:name',
'user:fullname',
'file:name',
'file:type',
Expand Down
2 changes: 2 additions & 0 deletions files/classes/reportbuilder/local/entities/file.php
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ protected function get_all_columns(): array {
->add_fields("{$filesalias}.contextid, " . context_helper::get_preload_record_columns_sql($contextalias))
// Sorting may not order alphabetically, but will at least group contexts together.
->set_is_sortable(true)
->set_is_deprecated('See \'context:name\' for replacement')
->add_callback(static function($contextid, stdClass $context): string {
if ($contextid === null) {
return '';
Expand All @@ -221,6 +222,7 @@ protected function get_all_columns(): array {
->add_fields("{$filesalias}.contextid, " . context_helper::get_preload_record_columns_sql($contextalias))
// Sorting may not order alphabetically, but will at least group contexts together.
->set_is_sortable(true)
->set_is_deprecated('See \'context:link\' for replacement')
->add_callback(static function($contextid, stdClass $context): string {
if ($contextid === null) {
return '';
Expand Down
34 changes: 27 additions & 7 deletions files/tests/reportbuilder/datasource/files_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ public function test_datasource_default(): void {

$content = $this->get_custom_report_content($report->get('id'));
$content = $this->filter_custom_report_content($content, static function(array $row): bool {
return $row['c0_contextid'] !== 'System';
return $row['c0_ctxid'] !== 'System';
});

$this->assertCount(2, $content);

// Consistent order (course, user), just in case.
core_collator::asort_array_of_arrays_by_key($content, 'c0_contextid');
core_collator::asort_array_of_arrays_by_key($content, 'c0_ctxid');
$content = array_values($content);

// First row (course summary file).
Expand Down Expand Up @@ -114,8 +114,10 @@ public function test_datasource_non_default_columns(): void {
$report = $generator->create_report(['name' => 'Files', 'source' => files::class, 'default' => 0]);

// Consistent order, sorted by context and content hash.
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'file:contexturl',
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'context:link',
'sortenabled' => 1, 'sortorder' => 1]);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'context:name']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'context:level']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'file:path']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'file:author']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'file:license']);
Expand All @@ -127,13 +129,15 @@ public function test_datasource_non_default_columns(): void {

$content = $this->get_custom_report_content($report->get('id'));
$content = $this->filter_custom_report_content($content, static function(array $row): bool {
return stripos($row['c0_contextid'], 'System') === false;
return stripos($row['c0_ctxid'], 'System') === false;
});

// There should be two entries (directory & file) for each context.
$this->assertEquals([
[
"<a href=\"{$coursecontext->get_url()}\">{$coursecontext->get_context_name()}</a>",
$coursecontext->get_context_name(),
'Course',
'/',
null,
'',
Expand All @@ -144,6 +148,8 @@ public function test_datasource_non_default_columns(): void {
],
[
"<a href=\"{$coursecontext->get_url()}\">{$coursecontext->get_context_name()}</a>",
$coursecontext->get_context_name(),
'Course',
'/',
null,
'',
Expand All @@ -154,6 +160,8 @@ public function test_datasource_non_default_columns(): void {
],
[
"<a href=\"{$usercontext->get_url()}\">{$usercontext->get_context_name()}</a>",
$usercontext->get_context_name(),
'User',
'/',
null,
'',
Expand All @@ -164,6 +172,8 @@ public function test_datasource_non_default_columns(): void {
],
[
"<a href=\"{$usercontext->get_url()}\">{$usercontext->get_context_name()}</a>",
$usercontext->get_context_name(),
'User',
'/',
null,
'',
Expand Down Expand Up @@ -222,7 +232,17 @@ public function datasource_filters_provider(): array {
'file:timecreated_to' => 1622502000,
], 0],

// User (just to check the join).
// Context.
'Context level' => ['context:level', [
'context:level_operator' => select::EQUAL_TO,
'context:level_value' => CONTEXT_COURSE,
], 2],
'Context level (no match)' => ['context:level', [
'context:level_operator' => select::EQUAL_TO,
'context:level_value' => CONTEXT_BLOCK,
], 0],

// User.
'Filter user' => ['user:username', [
'user:username_operator' => text::IS_EQUAL_TO,
'user:username_value' => 'alfie',
Expand Down Expand Up @@ -264,13 +284,13 @@ public function test_datasource_filters(

// Create report containing single column, and given filter.
$report = $generator->create_report(['name' => 'Files', 'source' => files::class, 'default' => 0]);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'file:context']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'context:name']);

// Add filter, set it's values.
$generator->create_filter(['reportid' => $report->get('id'), 'uniqueidentifier' => $filtername]);
$content = $this->get_custom_report_content($report->get('id'), 0, $filtervalues);
$content = $this->filter_custom_report_content($content, static function(array $row): bool {
return stripos($row['c0_contextid'], 'System') === false;
return stripos($row['c0_ctxid'], 'System') === false;
});

$this->assertCount($expectmatchcount, $content);
Expand Down
6 changes: 6 additions & 0 deletions reportbuilder/upgrade.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@ Information provided here is intended especially for developers.
* New `get_tag_joins_for_entity` helper in base entity class, for returning SQL joins necessary for retrieving tags
* New `set_is_deprecated` method in base `local\report\[column|filter]` classes to deprecate report entity columns and filters
* The following report entity columns have been deprecated, with replacements as follows:
- `comment:context` => `context:name`
- `comment:contexturl` => `context:link`
- `enrolment:method` => `enrol:name` (plus enrolment formatter `enrolment_name` method)
- 'enrolment:role` => `role:name`
- `file:context` => `context:name`
- `file:contexturl` => `context:link`
- `instance:context` (tag) => `context:name`
- `instance:contexturl` (tag) => `context:link`
* The following report entity filters/conditions have been deprecated, with replacements as follows:
- `enrolment:method` => `enrol:plugin`
* Trying to add/annotate duplicate entity names to a report will now throw a coding exception
Expand Down
14 changes: 11 additions & 3 deletions tag/classes/reportbuilder/datasource/tags.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
namespace core_tag\reportbuilder\datasource;

use lang_string;
use core\reportbuilder\local\entities\context;
use core_reportbuilder\datasource;
use core_reportbuilder\local\entities\user;
use core_tag\reportbuilder\local\entities\{collection, tag, instance};
Expand Down Expand Up @@ -46,10 +47,9 @@ public static function get_name(): string {
*/
protected function initialise(): void {
$collectionentity = new collection();

$collectionalias = $collectionentity->get_table_alias('tag_coll');
$this->set_main_table('tag_coll', $collectionalias);

$this->set_main_table('tag_coll', $collectionalias);
$this->add_entity($collectionentity);

// Join tag entity to collection.
Expand All @@ -67,6 +67,14 @@ protected function initialise(): void {
->add_join("LEFT JOIN {tag_instance} {$instancealias} ON {$instancealias}.tagid = {$tagalias}.id")
);

// Join the context entity.
$contextentity = new context();
$contextalias = $contextentity->get_table_alias('context');
$this->add_entity($contextentity
->add_joins($instanceentity->get_joins())
->add_join("LEFT JOIN {context} {$contextalias} ON {$contextalias}.id = {$instancealias}.contextid")
);

// Join user entity to represent the tag author.
$userentity = (new user())
->set_entity_title(new lang_string('tagauthor', 'core_tag'));
Expand All @@ -90,7 +98,7 @@ public function get_default_columns(): array {
'collection:name',
'tag:namewithlink',
'tag:standard',
'instance:context',
'context:name',
];
}

Expand Down
2 changes: 2 additions & 0 deletions tag/classes/reportbuilder/local/entities/instance.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ protected function get_all_columns(): array {
->add_fields("{$instancealias}.contextid, " . context_helper::get_preload_record_columns_sql($contextalias))
// Sorting may not order alphabetically, but will at least group contexts together.
->set_is_sortable(true)
->set_is_deprecated('See \'context:name\' for replacement')
->add_callback(static function($contextid, stdClass $context): string {
if ($contextid === null) {
return '';
Expand All @@ -142,6 +143,7 @@ protected function get_all_columns(): array {
->add_fields("{$instancealias}.contextid, " . context_helper::get_preload_record_columns_sql($contextalias))
// Sorting may not order alphabetically, but will at least group contexts together.
->set_is_sortable(true)
->set_is_deprecated('See \'context:link\' for replacement')
->add_callback(static function($contextid, stdClass $context): string {
if ($contextid === null) {
return '';
Expand Down
6 changes: 4 additions & 2 deletions tag/tests/reportbuilder/datasource/tags_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function test_datasource_default(): void {
$this->assertCount(2, $content);

// Consistent order (course, user), just in case.
core_collator::asort_array_of_arrays_by_key($content, 'c3_contextid');
core_collator::asort_array_of_arrays_by_key($content, 'c3_ctxid');
$content = array_values($content);

// Default columns are collection, tag name, tag standard, instance context.
Expand Down Expand Up @@ -103,8 +103,10 @@ public function test_datasource_non_default_columns(): void {
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'tag:flagged']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'tag:timemodified']);

// Context.
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'context:link']);

// Instance.
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'instance:contexturl']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'instance:area']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'instance:component']);
$generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'instance:itemtype']);
Expand Down

0 comments on commit d8df4ad

Please sign in to comment.