From fbfcb6733daa50b66892f5d4f88e17703ad8013b Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Thu, 23 Nov 2023 14:41:15 +0000 Subject: [PATCH] MDL-80245 reportbuilder: implement interface elements for report tags. Allow them to be edited during report creation/updating, display in report listings table with filtering, and implement tag callback to allow them to be discoverable across the site. --- lang/en/reportbuilder.php | 1 + reportbuilder/classes/form/report.php | 10 ++- .../local/systemreports/reports_list.php | 50 ++++++++++++ reportbuilder/lib.php | 57 ++++++++++++++ .../tests/behat/customreports.feature | 35 +++++++-- .../system_report_data_exporter_test.php | 14 +++- .../external/systemreports/retrieve_test.php | 14 +++- reportbuilder/tests/lib_test.php | 78 +++++++++++++++++++ 8 files changed, 246 insertions(+), 13 deletions(-) create mode 100644 reportbuilder/tests/lib_test.php diff --git a/lang/en/reportbuilder.php b/lang/en/reportbuilder.php index 4c2e6adbfe6..2ba2d394d87 100644 --- a/lang/en/reportbuilder.php +++ b/lang/en/reportbuilder.php @@ -81,6 +81,7 @@ $string['courseidnumberewithlink'] = 'Course ID number with link'; $string['courseshortnamewithlink'] = 'Course short name with link'; $string['courseselect'] = 'Select course'; $string['customfieldcolumn'] = '{$a}'; +$string['customreport'] = 'Custom report'; $string['customreports'] = 'Custom reports'; $string['customreportslimit'] = 'Custom reports limit'; $string['customreportslimit_desc'] = 'The number of custom reports may be limited for performance reasons. If set to zero, then there is no limit.'; diff --git a/reportbuilder/classes/form/report.php b/reportbuilder/classes/form/report.php index 2bf6c569db5..8d70cea0389 100644 --- a/reportbuilder/classes/form/report.php +++ b/reportbuilder/classes/form/report.php @@ -26,6 +26,7 @@ use core_form\dynamic_form; use core_reportbuilder\datasource; use core_reportbuilder\manager; use core_reportbuilder\local\helpers\report as reporthelper; +use core_tag_tag; defined('MOODLE_INTERNAL') || die(); @@ -114,6 +115,10 @@ class report extends dynamic_form { $mform->addElement('advcheckbox', 'uniquerows', get_string('uniquerows', 'core_reportbuilder')); $mform->addHelpButton('uniquerows', 'uniquerows', 'core_reportbuilder'); + + $mform->addElement('tags', 'tags', get_string('tags'), [ + 'component' => 'core_reportbuilder', 'itemtype' => 'reportbuilder_report', + ]); } /** @@ -137,8 +142,9 @@ class report extends dynamic_form { * Load in existing data as form defaults */ public function set_data_for_dynamic_submission(): void { - if ($report = $this->get_custom_report()) { - $this->set_data($report->get_report_persistent()->to_record()); + if ($persistent = $this->get_custom_report()?->get_report_persistent()) { + $tags = core_tag_tag::get_item_tags_array('core_reportbuilder', 'reportbuilder_report', $persistent->get('id')); + $this->set_data(array_merge((array) $persistent->to_record(), ['tags' => $tags])); } } diff --git a/reportbuilder/classes/local/systemreports/reports_list.php b/reportbuilder/classes/local/systemreports/reports_list.php index b588a89943d..ba726e71801 100644 --- a/reportbuilder/classes/local/systemreports/reports_list.php +++ b/reportbuilder/classes/local/systemreports/reports_list.php @@ -28,6 +28,7 @@ use core_reportbuilder\manager; use core_reportbuilder\system_report; use core_reportbuilder\local\entities\user; use core_reportbuilder\local\filters\date; +use core_reportbuilder\local\filters\tags; use core_reportbuilder\local\filters\text; use core_reportbuilder\local\filters\select; use core_reportbuilder\local\helpers\audience; @@ -38,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_tag; /** * Reports list @@ -113,6 +115,8 @@ 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. @@ -158,6 +162,37 @@ 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')) + ->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))); + }) + ); + // Time created column. $this->add_column((new column( 'timecreated', @@ -218,6 +253,21 @@ class reports_list extends system_report { }) ); + // Tags filter. + $this->add_filter((new filter( + tags::class, + 'tags', + new lang_string('tags'), + $this->get_report_entity_name(), + "{$tablealias}.id", + )) + ->set_options([ + 'component' => 'core_reportbuilder', + 'itemtype' => 'reportbuilder_report', + ]) + ->set_is_available(core_tag_tag::is_enabled('core_reportbuilder', 'reportbuilder_report')) + ); + // Time created filter. $this->add_filter((new filter( date::class, diff --git a/reportbuilder/lib.php b/reportbuilder/lib.php index 4a0f575c1b4..08c888b0195 100644 --- a/reportbuilder/lib.php +++ b/reportbuilder/lib.php @@ -27,6 +27,9 @@ declare(strict_types=1); use core\output\inplace_editable; use core_reportbuilder\form\audience; use core_reportbuilder\form\filter; +use core_reportbuilder\local\helpers\audience as audience_helper; +use core_reportbuilder\local\models\report; +use core_tag\output\{tagfeed, tagindex}; /** * Return the filters form fragment @@ -74,6 +77,60 @@ function core_reportbuilder_output_fragment_audience_form(array $params): string return $renderer->render_from_template('core_reportbuilder/local/audience/form', $context); } +/** + * Callback to return tagged reports + * + * @param core_tag_tag $tag + * @param bool $exclusivemode + * @param int|null $fromcontextid + * @param int|null $contextid + * @param bool $recurse + * @param int $page + * @return tagindex + */ +function core_reportbuilder_get_tagged_reports( + core_tag_tag $tag, + bool $exclusivemode = false, + ?int $fromcontextid = 0, + ?int $contextid = 0, + bool $recurse = true, + int $page = 0, +): tagindex { + global $OUTPUT; + + // Limit the returned list to those reports the current user can access. + [$where, $params] = audience_helper::user_reports_list_access_sql('it'); + + $tagcount = $tag->count_tagged_items('core_reportbuilder', 'reportbuilder_report', $where, $params); + $perpage = $exclusivemode ? 20 : 5; + $pagecount = ceil($tagcount / $perpage); + + $content = ''; + + if ($tagcount > 0) { + $tagfeed = new tagfeed(); + + $pixicon = new pix_icon('i/report', new lang_string('customreport', 'core_reportbuilder')); + + $reports = $tag->get_tagged_items('core_reportbuilder', 'reportbuilder_report', $page * $perpage, $perpage, + $where, $params); + foreach ($reports as $report) { + $tagfeed->add( + $OUTPUT->render($pixicon), + html_writer::link( + new moodle_url('/reportbuilder/view.php', ['id' => $report->id]), + (new report(0, $report))->get_formatted_name(), + ), + ); + } + + $content = $OUTPUT->render_from_template('core_tag/tagfeed', $tagfeed->export_for_template($OUTPUT)); + } + + return new tagindex($tag, 'core_reportbuilder', 'reportbuilder_report', $content, $exclusivemode, $fromcontextid, + $contextid, $recurse, $page, $pagecount); +} + /** * Plugin inplace editable implementation * diff --git a/reportbuilder/tests/behat/customreports.feature b/reportbuilder/tests/behat/customreports.feature index d69bdfe7edc..3cccb734b08 100644 --- a/reportbuilder/tests/behat/customreports.feature +++ b/reportbuilder/tests/behat/customreports.feature @@ -92,8 +92,12 @@ Feature: Manage custom reports And I set the following fields in the "New report" "dialogue" to these values: | Name | Manager report | | Report source | Users | + | Tags | Cat, Dog | And I click on "Save" "button" in the "New report" "dialogue" And I click on "Close 'Manager report' editor" "button" + And the following should exist in the "Reports list" table: + | Name | Tags | Report source | + | Manager report | Cat, Dog | Users | # Manager can edit their own report, but not those of other users. And I set the field "Edit report name" in the "Manager report" "table_row" to "Manager report (renamed)" Then the "Edit report content" item should exist in the "Actions" action menu of the "Manager report (renamed)" "table_row" @@ -140,16 +144,18 @@ Feature: Manage custom reports When I press "Edit report details" action in the "My report" report row And I set the following fields in the "Edit report details" "dialogue" to these values: | Name | My renamed report | + | Tags | Cat, Dog | And I click on "Save" "button" in the "Edit report details" "dialogue" Then I should see "Report updated" And the following should exist in the "Reports list" table: - | Name | Report source | - | My renamed report | Users | + | Name | Tags | Report source | + | My renamed report | Cat, Dog | Users | Scenario Outline: Filter custom reports Given the following "core_reportbuilder > Reports" exist: - | name | source | - | My users | core_user\reportbuilder\datasource\users | + | name | source | tags | + | My users | core_user\reportbuilder\datasource\users | Cat, Dog | + | 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 I click on "Filters" "button" @@ -158,11 +164,30 @@ Feature: Manage custom reports | value | | And I click on "Apply" "button" in the "[data-region='report-filters']" "css_element" Then I should see "Filters applied" - And I should see "My users" in the "Reports list" "table" + And the following should exist in the "Reports list" table: + | Name | Tags | Report source | + | My users | Cat, Dog | Users | + And I should not see "My courses" in the "Reports list" "table" Examples: | filter | value | | Name | My users | | Report source | Users | + | Tags | Cat | + + Scenario: Custom report tags are not displayed if tagging is disabled + Given the following config values are set as admin: + | usetags | 0 | + And the following "core_reportbuilder > Reports" exist: + | name | source | + | My report | core_user\reportbuilder\datasource\users | + And I log in as "admin" + When I navigate to "Reports > Report builder > Custom reports" in site administration + Then the following should exist in the "Reports list" table: + | Name | Report source | + | My report | Users | + And "Tags" "link" should not exist in the "Reports list" "table" + And I click on "Filters" "button" + And "Tags" "core_reportbuilder > Filter" should not exist Scenario: Delete custom report Given the following "core_reportbuilder > Reports" exist: diff --git a/reportbuilder/tests/external/system_report_data_exporter_test.php b/reportbuilder/tests/external/system_report_data_exporter_test.php index 881a97bb213..637615c49f1 100644 --- a/reportbuilder/tests/external/system_report_data_exporter_test.php +++ b/reportbuilder/tests/external/system_report_data_exporter_test.php @@ -50,20 +50,28 @@ class system_report_data_exporter_test extends advanced_testcase { // Two reports, created one second apart to ensure consistent ordering by time created. $generator->create_report(['name' => 'My first report', 'source' => users::class]); $this->waitForSecond(); - $generator->create_report(['name' => 'My second report', 'source' => users::class]); + $generator->create_report(['name' => 'My second report', 'source' => users::class, 'tags' => ['cat', 'dog']]); $reportinstance = system_report_factory::create(reports_list::class, system::instance()); $exporter = new system_report_data_exporter(null, ['report' => $reportinstance, 'page' => 0, 'perpage' => 1]); $export = $exporter->export($PAGE->get_renderer('core_reportbuilder')); - $this->assertEquals(['Name', 'Report source', 'Time created', 'Time modified', 'Modified by'], $export->headers); + $this->assertEquals([ + 'Name', + 'Report source', + 'Tags', + 'Time created', + 'Time modified', + 'Modified by', + ], $export->headers); $this->assertCount(1, $export->rows); - [$name, $source, $timecreated, $timemodified, $modifiedby] = $export->rows[0]['columns']; + [$name, $source, $tags, $timecreated, $timemodified, $modifiedby] = $export->rows[0]['columns']; $this->assertStringContainsString('My second report', $name); $this->assertEquals(users::get_name(), $source); + $this->assertEquals('cat, dog', $tags); $this->assertNotEmpty($timecreated); $this->assertNotEmpty($timemodified); $this->assertEquals('Admin User', $modifiedby); diff --git a/reportbuilder/tests/external/systemreports/retrieve_test.php b/reportbuilder/tests/external/systemreports/retrieve_test.php index 0b0d151d2a3..0a410ad6bf0 100644 --- a/reportbuilder/tests/external/systemreports/retrieve_test.php +++ b/reportbuilder/tests/external/systemreports/retrieve_test.php @@ -54,20 +54,28 @@ class retrieve_test extends externallib_advanced_testcase { // Two reports, created one second apart to ensure consistent ordering by time created. $generator->create_report(['name' => 'My first report', 'source' => users::class]); $this->waitForSecond(); - $generator->create_report(['name' => 'My second report', 'source' => users::class]); + $generator->create_report(['name' => 'My second report', 'source' => users::class, 'tags' => ['cat', 'dog']]); // Retrieve paged results. $result = retrieve::execute(reports_list::class, ['contextid' => system::instance()->id], '', '', 0, [], 0, 1); $result = external_api::clean_returnvalue(retrieve::execute_returns(), $result); $this->assertArrayHasKey('data', $result); - $this->assertEquals(['Name', 'Report source', 'Time created', 'Time modified', 'Modified by'], $result['data']['headers']); + $this->assertEquals([ + 'Name', + 'Report source', + 'Tags', + 'Time created', + 'Time modified', + 'Modified by', + ], $result['data']['headers']); $this->assertCount(1, $result['data']['rows']); - [$name, $source, $timecreated, $timemodified, $modifiedby] = $result['data']['rows'][0]['columns']; + [$name, $source, $tags, $timecreated, $timemodified, $modifiedby] = $result['data']['rows'][0]['columns']; $this->assertStringContainsString('My second report', $name); $this->assertEquals(users::get_name(), $source); + $this->assertEquals('cat, dog', $tags); $this->assertNotEmpty($timecreated); $this->assertNotEmpty($timemodified); $this->assertEquals('Admin User', $modifiedby); diff --git a/reportbuilder/tests/lib_test.php b/reportbuilder/tests/lib_test.php new file mode 100644 index 00000000000..1a01a141157 --- /dev/null +++ b/reportbuilder/tests/lib_test.php @@ -0,0 +1,78 @@ +. + +declare(strict_types=1); + +namespace core_reportbuilder; + +use advanced_testcase; +use core_reportbuilder_generator; +use core_tag_tag; +use core_user\reportbuilder\datasource\users; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once("{$CFG->dirroot}/reportbuilder/lib.php"); + +/** + * Unit tests for the component callbacks + * + * @package core_reportbuilder + * @copyright 2023 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class lib_test extends advanced_testcase { + + /** + * Test getting tagged reports + * + * @covers ::core_reportbuilder_get_tagged_reports + */ + public function test_core_reportbuilder_get_tagged_reports(): void { + $this->resetAfterTest(); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + + // Create three tagged reports. + $reportone = $generator->create_report(['name' => 'Report 1', 'source' => users::class, 'tags' => ['cat']]); + $reporttwo = $generator->create_report(['name' => 'Report 2', 'source' => users::class, 'tags' => ['dog']]); + $reportthree = $generator->create_report(['name' => 'Report 3', 'source' => users::class, 'tags' => ['cat']]); + + // Add all users audience to report one and two. + $generator->create_audience(['reportid' => $reportone->get('id'), 'configdata' => []]); + $generator->create_audience(['reportid' => $reporttwo->get('id'), 'configdata' => []]); + + $tag = core_tag_tag::get_by_name(0, 'cat'); + + // Current user can only access report one with "cat" tag. + $user = $this->getDataGenerator()->create_user(); + $this->setUser($user); + + $tagindex = core_reportbuilder_get_tagged_reports($tag); + $this->assertStringContainsString($reportone->get_formatted_name(), $tagindex->content); + $this->assertStringNotContainsString($reporttwo->get_formatted_name(), $tagindex->content); + $this->assertStringNotContainsString($reportthree->get_formatted_name(), $tagindex->content); + + // Admin can access both reports with "cat" tag. + $this->setAdminUser(); + $tagindex = core_reportbuilder_get_tagged_reports($tag); + $this->assertStringContainsString($reportone->get_formatted_name(), $tagindex->content); + $this->assertStringNotContainsString($reporttwo->get_formatted_name(), $tagindex->content); + $this->assertStringContainsString($reportthree->get_formatted_name(), $tagindex->content); + } +}