From 78f56cffde3124de9a8a63a73a503681786172e8 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Fri, 7 Jun 2024 00:14:12 +0100 Subject: [PATCH] MDL-76392 reportbuilder: migrate manual to native report aggregation. Now that system reports support aggregation natively, we can replace a whole bunch of similar code containing hand-rolled versions with much simpler/consistent use of API. --- .../local/systemreports/badges.php | 42 ++++++++--------- badges/tests/behat/manage_badges.feature | 22 +++++++++ .../local/systemreports/cohorts.php | 35 +++++++-------- .../local/systemreports/reports_list.php | 45 +++++-------------- .../tests/behat/customreports.feature | 4 ++ .../reportbuilder/local/entities/tag.php | 13 ++++++ .../local/systemreports/tags.php | 39 ++++++++-------- 7 files changed, 105 insertions(+), 95 deletions(-) diff --git a/badges/classes/reportbuilder/local/systemreports/badges.php b/badges/classes/reportbuilder/local/systemreports/badges.php index 305cf840da2..5a8da89262f 100644 --- a/badges/classes/reportbuilder/local/systemreports/badges.php +++ b/badges/classes/reportbuilder/local/systemreports/badges.php @@ -20,6 +20,7 @@ namespace core_badges\reportbuilder\local\systemreports; use core\context\{course, system}; use core_badges\reportbuilder\local\entities\badge; +use core_reportbuilder\local\entities\user; use core_reportbuilder\local\helpers\database; use core_reportbuilder\local\report\{action, column}; use core_reportbuilder\system_report; @@ -53,6 +54,15 @@ class badges extends system_report { $this->set_main_table('badge', $entityalias); $this->add_entity($badgeentity); + // Join user entity. + $userentity = new user(); + $useralias = $userentity->get_table_alias('user'); + $badgeissuedalias = database::generate_alias(); + $this->add_entity($userentity->add_joins([ + "LEFT JOIN {badge_issued} {$badgeissuedalias} ON {$badgeissuedalias}.badgeid = {$entityalias}.id", + "LEFT JOIN {user} {$useralias} ON {$useralias}.id = {$badgeissuedalias}.userid AND {$useralias}.deleted = 0", + ])); + $paramtype = database::generate_param_name(); $context = $this->get_context(); if ($context instanceof system) { @@ -69,7 +79,7 @@ class badges extends system_report { $this->add_base_fields("{$entityalias}.id, {$entityalias}.type, {$entityalias}.courseid, {$entityalias}.status"); // Now we can call our helper methods to add the content we want to include in the report. - $this->add_columns($badgeentity); + $this->add_columns(); $this->add_filters(); $this->add_actions(); @@ -101,37 +111,21 @@ class badges extends system_report { * * They are provided by the entities we previously added in the {@see initialise} method, referencing each by their * unique identifier. If custom columns are needed just for this report, they can be defined here. - * - * @param badge $badgeentity */ - public function add_columns(badge $badgeentity): void { - $columns = [ + protected function add_columns(): void { + $this->add_columns_from_entities([ 'badge:image', 'badge:namewithlink', 'badge:version', 'badge:status', 'badge:criteria', - ]; - - $this->add_columns_from_entities($columns); + 'user:username', + ]); // Issued badges column. - // TODO: Move this column to the entity when MDL-76392 is integrated. - $tempbadgealias = database::generate_alias(); - $badgeentityalias = $badgeentity->get_table_alias('badge'); - $this->add_column((new column( - 'issued', - new lang_string('awards', 'core_badges'), - $badgeentity->get_entity_name() - )) - ->add_joins($this->get_joins()) - ->set_type(column::TYPE_INTEGER) - ->add_field("(SELECT COUNT({$tempbadgealias}.userid) - FROM {badge_issued} {$tempbadgealias} - INNER JOIN {user} u - ON {$tempbadgealias}.userid = u.id - WHERE {$tempbadgealias}.badgeid = {$badgeentityalias}.id AND u.deleted = 0)", 'issued') - ->set_is_sortable(true)); + $this->get_column('user:username') + ->set_title(new lang_string('awards', 'core_badges')) + ->set_aggregation('count'); // Remove title from image column. $this->get_column('badge:image')->set_title(null); diff --git a/badges/tests/behat/manage_badges.feature b/badges/tests/behat/manage_badges.feature index 73433fbd1ed..61648dc4525 100644 --- a/badges/tests/behat/manage_badges.feature +++ b/badges/tests/behat/manage_badges.feature @@ -107,3 +107,25 @@ Feature: Manage badges Then the following should exist in the "reportbuilder-table" table: | Name | Badge status | Recipients | | Badge #1 | Available | 1 | + + Scenario: View list of badges with recipients + Given the following "users" exist: + | username | firstname | lastname | + | user1 | User | One | + | user2 | User | Two | + And the following "core_badges > Badges" exist: + | name | status | + | Badge #2 | 1 | + | Badge #3 | 1 | + And the following "core_badges > Issued badges" exist: + | badge | user | + | Badge #1 | user1 | + | Badge #1 | user2 | + | Badge #2 | user1 | + When I log in as "admin" + And I navigate to "Badges > Manage badges" in site administration + Then the following should exist in the "Badges" table: + | Name | Badge status | Recipients | + | Badge #1 | Not available | 2 | + | Badge #2 | Available | 1 | + | Badge #3 | Available | 0 | diff --git a/cohort/classes/reportbuilder/local/systemreports/cohorts.php b/cohort/classes/reportbuilder/local/systemreports/cohorts.php index 305e1a86466..91825c73e22 100644 --- a/cohort/classes/reportbuilder/local/systemreports/cohorts.php +++ b/cohort/classes/reportbuilder/local/systemreports/cohorts.php @@ -19,7 +19,7 @@ namespace core_cohort\reportbuilder\local\systemreports; use context; use context_coursecat; use context_system; -use core_cohort\reportbuilder\local\entities\cohort; +use core_cohort\reportbuilder\local\entities\{cohort, cohort_member}; use core_reportbuilder\local\helpers\database; use core_reportbuilder\local\report\action; use core_reportbuilder\local\report\column; @@ -50,6 +50,13 @@ class cohorts extends system_report { $this->set_main_table('cohort', $entitymainalias); $this->add_entity($cohortentity); + // Join cohort member entity. + $cohortmemberentity = new cohort_member(); + $cohortmemberalias = $cohortmemberentity->get_table_alias('cohort_members'); + $this->add_entity($cohortmemberentity + ->add_join("LEFT JOIN {cohort_members} {$cohortmemberalias} ON {$cohortmemberalias}.cohortid = {$entitymainalias}.id") + ); + // Any columns required by actions should be defined here to ensure they're always available. $this->add_base_fields("{$entitymainalias}.id, {$entitymainalias}.contextid, {$entitymainalias}.visible, " . "{$entitymainalias}.component"); @@ -63,7 +70,7 @@ class cohorts extends system_report { } // Now we can call our helper methods to add the content we want to include in the report. - $this->add_columns($cohortentity); + $this->add_columns(); $this->add_filters(); $this->add_actions(); @@ -92,12 +99,11 @@ class cohorts extends system_report { * * They are provided by the entities we previously added in the {@see initialise} method, referencing each by their * unique identifier. If custom columns are needed just for this report, they can be defined here. - * - * @param cohort $cohortentity */ - public function add_columns(cohort $cohortentity): void { - + protected function add_columns(): void { + $cohortentity = $this->get_entity('cohort'); $entitymainalias = $cohortentity->get_table_alias('cohort'); + $showall = $this->get_parameter('showall', false, PARAM_BOOL); // Category column. An extra callback is appended in order to extend the current column formatting. @@ -151,19 +157,10 @@ class cohorts extends system_report { // Description column. $this->add_column_from_entity('cohort:description'); - // Cohort size column using a custom SQL query to count cohort members. - $cm = database::generate_param_name(); - $sql = "(SELECT count($cm.id) as memberscount - FROM {cohort_members} $cm - WHERE $cm.cohortid = {$entitymainalias}.id)"; - $this->add_column(new column( - 'memberscount', - new lang_string('memberscount', 'cohort'), - $cohortentity->get_entity_name() - )) - ->set_type(column::TYPE_INTEGER) - ->set_is_sortable(true) - ->add_field($sql, 'memberscount'); + // Member count. + $this->add_column_from_entity('cohort_member:timeadded') + ->set_title(new lang_string('memberscount', 'cohort')) + ->set_aggregation('count'); // Component column. Override the display name of a column. $this->add_column_from_entity('cohort:component') diff --git a/reportbuilder/classes/local/systemreports/reports_list.php b/reportbuilder/classes/local/systemreports/reports_list.php index da80e6f62b5..37daf09e702 100644 --- a/reportbuilder/classes/local/systemreports/reports_list.php +++ b/reportbuilder/classes/local/systemreports/reports_list.php @@ -39,6 +39,7 @@ use core_reportbuilder\local\report\filter; use core_reportbuilder\output\report_name_editable; use core_reportbuilder\local\models\report; use core_reportbuilder\permission; +use core_tag\reportbuilder\local\entities\tag; use core_tag_tag; /** @@ -76,11 +77,16 @@ class reports_list extends system_report { // Join user entity for "User modified" column. $entityuser = new user(); $entityuseralias = $entityuser->get_table_alias('user'); - $this->add_entity($entityuser ->add_join("JOIN {user} {$entityuseralias} ON {$entityuseralias}.id = rb.usermodified") ); + // Join tag entity. + $entitytag = new tag(); + $this->add_entity($entitytag + ->add_joins($entitytag->get_tag_joins('core_reportbuilder', 'reportbuilder_report', 'rb.id')) + ); + // Define our internal entity for report elements. $this->annotate_entity($this->get_report_entity_name(), new lang_string('customreports', 'core_reportbuilder')); @@ -115,8 +121,6 @@ class reports_list extends system_report { * Add columns to report */ protected function add_columns(): void { - global $DB; - $tablealias = $this->get_main_table_alias(); // Report name column. @@ -162,36 +166,11 @@ class reports_list extends system_report { }) ); - // Tags column. TODO: Reuse tag entity column when MDL-76392 is integrated. - $tagfieldconcatsql = $DB->sql_group_concat( - field: $DB->sql_concat_join("'|'", ['t.name', 't.rawname']), - sort: 't.name', - ); - $this->add_column((new column( - 'tags', - new lang_string('tags'), - $this->get_report_entity_name(), - )) - ->set_type(column::TYPE_TEXT) - ->add_field("( - SELECT {$tagfieldconcatsql} - FROM {tag_instance} ti - JOIN {tag} t ON t.id = ti.tagid - WHERE ti.component = 'core_reportbuilder' AND ti.itemtype = 'reportbuilder_report' - AND ti.itemid = {$tablealias}.id - )", 'tags') - ->set_is_sortable(true) - ->set_is_available(core_tag_tag::is_enabled('core_reportbuilder', 'reportbuilder_report') === true) - ->add_callback(static function(?string $tags): string { - return implode(', ', array_map(static function(string $tag): string { - [$name, $rawname] = explode('|', $tag); - return core_tag_tag::make_display_name((object) [ - 'name' => $name, - 'rawname' => $rawname, - ]); - }, preg_split('/, /', (string) $tags, -1, PREG_SPLIT_NO_EMPTY))); - }) - ); + // Tags column. + $this->add_column_from_entity('tag:name') + ->set_title(new lang_string('tags')) + ->set_aggregation('groupconcat') + ->set_is_available(core_tag_tag::is_enabled('core_reportbuilder', 'reportbuilder_report') === true); // Time created column. $this->add_column((new column( diff --git a/reportbuilder/tests/behat/customreports.feature b/reportbuilder/tests/behat/customreports.feature index a1fab78ee0c..296017516fd 100644 --- a/reportbuilder/tests/behat/customreports.feature +++ b/reportbuilder/tests/behat/customreports.feature @@ -158,6 +158,10 @@ Feature: Manage custom reports | My courses | core_course\reportbuilder\datasource\courses | | And I log in as "admin" When I navigate to "Reports > Report builder > Custom reports" in site administration + And the following should exist in the "Reports list" table: + | Name | Tags | Report source | + | My users | Cat, Dog | Users | + | My courses | | Courses | And I click on "Filters" "button" And I set the following fields in the "" "core_reportbuilder > Filter" to these values: | operator | Is equal to | diff --git a/tag/classes/reportbuilder/local/entities/tag.php b/tag/classes/reportbuilder/local/entities/tag.php index f5c8ce385d5..ed1ef5d31b0 100644 --- a/tag/classes/reportbuilder/local/entities/tag.php +++ b/tag/classes/reportbuilder/local/entities/tag.php @@ -45,6 +45,7 @@ class tag extends base { protected function get_default_tables(): array { return [ 'tag', + 'tag_instance', ]; } @@ -269,4 +270,16 @@ class tag extends base { return $filters; } + + /** + * Return joins necessary for retrieving tags + * + * @param string $component + * @param string $itemtype + * @param string $itemidfield + * @return string[] + */ + public function get_tag_joins(string $component, string $itemtype, string $itemidfield): array { + return $this->get_tag_joins_for_entity($component, $itemtype, $itemidfield); + } } diff --git a/tag/classes/reportbuilder/local/systemreports/tags.php b/tag/classes/reportbuilder/local/systemreports/tags.php index 5217a68cbb2..2b1f4958aa7 100644 --- a/tag/classes/reportbuilder/local/systemreports/tags.php +++ b/tag/classes/reportbuilder/local/systemreports/tags.php @@ -23,7 +23,7 @@ use core_reportbuilder\local\entities\user; use core_reportbuilder\local\report\{action, column}; use core_reportbuilder\system_report; use core_tag\output\{tagflag, tagisstandard, tagname}; -use core_tag\reportbuilder\local\entities\tag; +use core_tag\reportbuilder\local\entities\{instance, tag}; use lang_string; use moodle_url; use pix_icon; @@ -57,12 +57,19 @@ class tags extends system_report { // Limit tags to current collection. $this->add_base_condition_simple("{$tagalias}.tagcollid", $this->get_parameter('collection', 0, PARAM_INT)); + // Join the instance entity to tag. + $instance = new instance(); + $instancealias = $instance->get_table_alias('tag_instance'); + $this->add_entity($instance + ->add_join("LEFT JOIN {tag_instance} {$instancealias} ON {$instancealias}.tagid = {$tagalias}.id") + ); + // Join the user entity to represent the tag author. $user = new user(); $useralias = $user->get_table_alias('user'); $this->add_entity($user->add_join("LEFT JOIN {user} {$useralias} ON {$useralias}.id = {$tagalias}.userid")); - $this->add_columns($tag); + $this->add_columns(); $this->add_filters(); $this->add_actions(); @@ -82,18 +89,18 @@ class tags extends system_report { /** * Report columns - * - * @param tag $tag */ - public function add_columns(tag $tag): void { - $tagentity = $tag->get_entity_name(); + protected function add_columns(): void { + $tag = $this->get_entity('tag'); + + $tagentityname = $tag->get_entity_name(); $tagalias = $tag->get_table_alias('tag'); // Name (editable). $this->add_column((new column( 'nameeditable', new lang_string('name', 'core_tag'), - $tagentity, + $tagentityname, )) ->add_fields("{$tagalias}.name, {$tagalias}.rawname, {$tagalias}.tagcollid, {$tagalias}.id") ->set_type(column::TYPE_TEXT) @@ -108,22 +115,16 @@ class tags extends system_report { // User. $this->add_column_from_entity('user:fullnamewithlink'); - // Count (TODO MDL-76392 use native entity aggregation). - $this->add_column((new column( - 'instancecount', - new lang_string('count', 'core_tag'), - $tagentity, - )) - ->add_field("(SELECT COUNT(*) FROM {tag_instance} WHERE tagid = {$tagalias}.id)", 'instancecount') - ->set_type(column::TYPE_INTEGER) - ->set_is_sortable(true) - ); + // Instance count. + $this->add_column_from_entity('instance:component') + ->set_title(new lang_string('count', 'core_tag')) + ->set_aggregation('count'); // Flag (editable). $this->add_column((new column( 'flageditable', new lang_string('flag', 'core_tag'), - $tagentity, + $tagentityname, )) ->add_fields("{$tagalias}.flag, {$tagalias}.id") ->set_type(column::TYPE_BOOLEAN) @@ -143,7 +144,7 @@ class tags extends system_report { $this->add_column((new column( 'standardeditable', new lang_string('standardtag', 'core_tag'), - $tagentity, + $tagentityname, )) ->add_fields("{$tagalias}.isstandard, {$tagalias}.id") ->set_type(column::TYPE_BOOLEAN) -- 2.43.0