From 47790ea315e8932dbb65374376d62e4d8ae95aca Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Mon, 20 Nov 2023 08:51:18 +0000 Subject: [PATCH] MDL-80245 tag: allow for tags filter to operate in component mode. This allows the filter to be more easily re-used for component system reports that want to filter by their own tags. --- reportbuilder/classes/local/filters/tags.php | 103 ++++++++++++++---- .../tests/local/filters/tags_test.php | 78 ++++++++++++- reportbuilder/upgrade.txt | 2 + 3 files changed, 160 insertions(+), 23 deletions(-) diff --git a/reportbuilder/classes/local/filters/tags.php b/reportbuilder/classes/local/filters/tags.php index a0730d24c2c..b04de1d305e 100644 --- a/reportbuilder/classes/local/filters/tags.php +++ b/reportbuilder/classes/local/filters/tags.php @@ -18,17 +18,24 @@ declare(strict_types=1); namespace core_reportbuilder\local\filters; -use coding_exception; use core_tag_tag; use lang_string; use MoodleQuickForm; use stdClass; use core_reportbuilder\local\helpers\database; +use core_reportbuilder\local\report\filter; /** * Class containing logic for the tags filter * - * The field SQL should be the field containing the ID of the {tag} table + * The filter can operate in two modes: + * + * 1. Filtering of tags directly from the {tag} table, in which case the field SQL expression should return the ID of that table; + * 2. Filtering of component tags, in which case the field SQL expression should return the ID of the component table that would + * join to the {tag_instance} itemid field + * + * If filtering component tags then the following must be passed to the {@see filter::get_options} method when using this filter + * in a report: ['component' => 'mycomponent', 'itemtype' => 'myitem'] * * @package core_reportbuilder * @copyright 2022 Paul Holden @@ -80,14 +87,26 @@ class tags extends base { $mform->addElement('select', "{$this->name}_operator", $operatorlabel, $this->get_operators()) ->setHiddenLabel(true); - $sql = 'SELECT DISTINCT t.id, t.name, t.rawname + // If we're filtering component tags, show only those related to the component itself. + $options = (array) $this->filter->get_options(); + if (array_key_exists('component', $options) && array_key_exists('itemtype', $options)) { + $taginstancejoin = 'JOIN {tag_instance} ti ON ti.tagid = t.id + WHERE ti.component = :component AND ti.itemtype = :itemtype'; + $params = array_intersect_key($options, array_flip(['component', 'itemtype'])); + } else { + $taginstancejoin = ''; + $params = []; + } + + $sql = "SELECT DISTINCT t.id, t.name, t.rawname FROM {tag} t - ORDER BY t.name'; + {$taginstancejoin} + ORDER BY t.name"; // Transform tag records into appropriate display name, for selection in the autocomplete element. $tags = array_map(static function(stdClass $record): string { return core_tag_tag::make_display_name($record); - }, $DB->get_records_sql($sql)); + }, $DB->get_records_sql($sql, $params)); $valuelabel = get_string('filterfieldvalue', 'core_reportbuilder', $this->get_header()); $mform->addElement('autocomplete', "{$this->name}_value", $valuelabel, $tags, ['multiple' => true]) @@ -110,26 +129,66 @@ class tags extends base { $operator = (int) ($values["{$this->name}_operator"] ?? self::ANY_VALUE); $tags = (array) ($values["{$this->name}_value"] ?? []); - if ($operator === self::NOT_EMPTY) { - $select = "{$fieldsql} IS NOT NULL"; - } else if ($operator === self::EMPTY) { - $select = "{$fieldsql} IS NULL"; - } else if ($operator === self::EQUAL_TO && !empty($tags)) { - [$tagselect, $tagselectparams] = $DB->get_in_or_equal($tags, SQL_PARAMS_NAMED, - database::generate_param_name('_')); + // If we're filtering component tags, we need to perform [not] exists queries to ensure no row duplication occurs. + $options = (array) $this->filter->get_options(); + if (array_key_exists('component', $options) && array_key_exists('itemtype', $options)) { + [$paramcomponent, $paramitemtype] = database::generate_param_names(2); - $select = "{$fieldsql} {$tagselect}"; - $params = array_merge($params, $tagselectparams); - } else if ($operator === self::NOT_EQUAL_TO && !empty($tags)) { - [$tagselect, $tagselectparams] = $DB->get_in_or_equal($tags, SQL_PARAMS_NAMED, - database::generate_param_name('_'), false); + $componenttagselect = <<get_in_or_equal($tags, SQL_PARAMS_NAMED, + database::generate_param_name('_')); + + $select = "EXISTS ({$componenttagselect} AND t.id {$tagselect})"; + $params = array_merge($params, $tagselectparams); + } else if ($operator === self::NOT_EQUAL_TO && !empty($tags)) { + [$tagselect, $tagselectparams] = $DB->get_in_or_equal($tags, SQL_PARAMS_NAMED, + database::generate_param_name('_')); + + // We should also return those elements that aren't tagged at all. + $select = "NOT EXISTS ({$componenttagselect} AND t.id {$tagselect})"; + $params = array_merge($params, $tagselectparams); + } else { + // Invalid/inactive (any value) filter.. + return ['', []]; + } } else { - // Invalid/inactive (any value) filter.. - return ['', []]; + + // We're filtering directly from the tag table. + if ($operator === self::NOT_EMPTY) { + $select = "{$fieldsql} IS NOT NULL"; + } else if ($operator === self::EMPTY) { + $select = "{$fieldsql} IS NULL"; + } else if ($operator === self::EQUAL_TO && !empty($tags)) { + [$tagselect, $tagselectparams] = $DB->get_in_or_equal($tags, SQL_PARAMS_NAMED, + database::generate_param_name('_')); + + $select = "{$fieldsql} {$tagselect}"; + $params = array_merge($params, $tagselectparams); + } else if ($operator === self::NOT_EQUAL_TO && !empty($tags)) { + [$tagselect, $tagselectparams] = $DB->get_in_or_equal($tags, SQL_PARAMS_NAMED, + database::generate_param_name('_'), false); + + // We should also return those elements that aren't tagged at all. + $select = "COALESCE({$fieldsql}, 0) {$tagselect}"; + $params = array_merge($params, $tagselectparams); + } else { + // Invalid/inactive (any value) filter.. + return ['', []]; + } } return [$select, $params]; diff --git a/reportbuilder/tests/local/filters/tags_test.php b/reportbuilder/tests/local/filters/tags_test.php index a3148673c08..9d42c591eb8 100644 --- a/reportbuilder/tests/local/filters/tags_test.php +++ b/reportbuilder/tests/local/filters/tags_test.php @@ -20,7 +20,9 @@ namespace core_reportbuilder\local\filters; use advanced_testcase; use lang_string; +use core_reportbuilder_generator; use core_reportbuilder\local\report\filter; +use core_user\reportbuilder\datasource\users; /** * Unit tests for tags report filter @@ -38,7 +40,7 @@ class tags_test extends advanced_testcase { * * @return array[] */ - public function get_sql_filter_provider(): array { + public static function get_sql_filter_provider(): array { return [ 'Any value' => [tags::ANY_VALUE, null, ['course01', 'course01', 'course02', 'course03']], 'Not empty' => [tags::NOT_EMPTY, null, ['course01', 'course01', 'course02']], @@ -102,4 +104,78 @@ class tags_test extends advanced_testcase { $courses = $DB->get_fieldset_sql($sql, $params); $this->assertEqualsCanonicalizing($expectedcoursenames, $courses); } + + /** + * Data provider for {@see test_get_sql_filter_component} + * + * @return array[] + */ + public static function get_sql_filter_component_provider(): array { + return [ + 'Any value' => [tags::ANY_VALUE, null, ['report01', 'report02']], + 'Not empty' => [tags::NOT_EMPTY, null, ['report01']], + 'Empty' => [tags::EMPTY, null, ['report02']], + 'Equal to unselected' => [tags::EQUAL_TO, null, ['report01', 'report02']], + 'Equal to selected tag' => [tags::EQUAL_TO, 'fish', ['report01']], + 'Equal to selected tag (different component)' => [tags::EQUAL_TO, 'cat', []], + 'Not equal to unselected' => [tags::NOT_EQUAL_TO, null, ['report01', 'report02']], + 'Not equal to selected tag' => [tags::NOT_EQUAL_TO, 'fish', ['report02']], + 'Not Equal to selected tag (different component)' => [tags::NOT_EQUAL_TO, 'cat', ['report01', 'report02']], + ]; + } + + /** + * Test getting filter SQL + * + * @param int $operator + * @param string|null $tagname + * @param array $expectedreportnames + * + * @dataProvider get_sql_filter_component_provider + */ + public function test_get_sql_filter_component(int $operator, ?string $tagname, array $expectedreportnames): void { + global $DB; + + $this->resetAfterTest(); + + // Create a course with tags, we shouldn't ever get this data back when specifying another component. + $this->getDataGenerator()->create_course(['tags' => ['cat', 'dog']]); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $generator->create_report(['name' => 'report01', 'source' => users::class, 'tags' => ['fish']]); + $generator->create_report(['name' => 'report02', 'source' => users::class]); + + $filter = (new filter( + tags::class, + 'tags', + new lang_string('tags'), + 'testentity', + 'r.id' + ))->set_options([ + 'component' => 'core_reportbuilder', + 'itemtype' => 'reportbuilder_report', + ]); + + // Create instance of our filter, passing ID of the tag if specified. + if ($tagname !== null) { + $tagid = $DB->get_field('tag', 'id', ['name' => $tagname], MUST_EXIST); + $value = [$tagid]; + } else { + $value = null; + } + + [$select, $params] = tags::create($filter)->get_sql_filter([ + $filter->get_unique_identifier() . '_operator' => $operator, + $filter->get_unique_identifier() . '_value' => $value, + ]); + + $sql = 'SELECT r.name FROM {reportbuilder_report} r'; + if ($select) { + $sql .= " WHERE {$select}"; + } + + $reports = $DB->get_fieldset_sql($sql, $params); + $this->assertEqualsCanonicalizing($expectedreportnames, $reports); + } } diff --git a/reportbuilder/upgrade.txt b/reportbuilder/upgrade.txt index 382a2a77bb4..8dd39c7a036 100644 --- a/reportbuilder/upgrade.txt +++ b/reportbuilder/upgrade.txt @@ -17,6 +17,8 @@ Information provided here is intended especially for developers. and `$exclude` parameters * Custom reports now implement the tag API, with options for specifying in the `report::[create|update]_report` helper methods as well as in the `create_report` test generator method +* The `tags` filter has been improved to also allow for filtering by component/itemtype core_tag definition - this is more + suited for system reports * New report filter types: - `filesize` for reports containing filesize data