From 9ebd801c076d8c866225cb1657d87fb04faf61ad Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Fri, 11 Feb 2022 10:15:59 +0000 Subject: [PATCH] MDL-73842 reportbuilder: add tags elements to course/user entities. Both entities define columns and filters for related tags (referred to as "interests" for users). Create new generic tags filter type to facilitate this, which can be re-used by any entities for components which support tagging. --- .../classes/local/entities/course.php | 56 ++++++++ reportbuilder/classes/local/entities/user.php | 56 ++++++++ reportbuilder/classes/local/filters/tags.php | 133 ++++++++++++++++++ .../tests/behat/conditioneditor.feature | 20 ++- .../tests/local/entities/course_test.php | 14 ++ .../tests/local/entities/user_test.php | 16 ++- .../tests/local/filters/tags_test.php | 102 ++++++++++++++ reportbuilder/upgrade.txt | 1 + 8 files changed, 394 insertions(+), 4 deletions(-) create mode 100644 reportbuilder/classes/local/filters/tags.php create mode 100644 reportbuilder/tests/local/filters/tags_test.php diff --git a/reportbuilder/classes/local/entities/course.php b/reportbuilder/classes/local/entities/course.php index 982b3a08a41..2c49d15421e 100644 --- a/reportbuilder/classes/local/entities/course.php +++ b/reportbuilder/classes/local/entities/course.php @@ -20,10 +20,12 @@ namespace core_reportbuilder\local\entities; use context_course; use context_helper; +use core_tag_tag; use core_reportbuilder\local\filters\boolean_select; use core_reportbuilder\local\filters\course_selector; use core_reportbuilder\local\filters\date; use core_reportbuilder\local\filters\select; +use core_reportbuilder\local\filters\tags; use core_reportbuilder\local\filters\text; use core_reportbuilder\local\helpers\custom_fields; use core_reportbuilder\local\helpers\format; @@ -58,6 +60,8 @@ class course extends base { return [ 'course' => 'c', 'context' => 'cctx', + 'tag_instance' => 'ti', + 'tag' => 't', ]; } @@ -193,6 +197,26 @@ class course extends base { return $fieldtype; } + /** + * Return joins necessary for retrieving tags + * + * @return string[] + */ + private function get_tag_joins(): array { + $course = $this->get_table_alias('course'); + $taginstance = $this->get_table_alias('tag_instance'); + $tag = $this->get_table_alias('tag'); + + return [ + "LEFT JOIN {tag_instance} {$taginstance} + ON {$taginstance}.component = 'core' + AND {$taginstance}.itemtype = 'course' + AND {$taginstance}.itemid = {$course}.id", + "LEFT JOIN {tag} {$tag} + ON {$tag}.id = {$taginstance}.tagid", + ]; + } + /** * Returns list of all available columns. * @@ -272,6 +296,22 @@ class course extends base { $columns[] = $column; } + // Tags. + $tag = $this->get_table_alias('tag'); + $columns[] = (new column( + 'tags', + new lang_string('tags'), + $this->get_entity_name() + )) + ->add_joins($this->get_joins()) + ->add_joins($this->get_tag_joins()) + ->set_type(column::TYPE_TEXT) + ->add_fields("{$tag}.name, {$tag}.rawname") + ->set_is_sortable(true) + ->add_callback(static function($value, stdClass $tag): string { + return core_tag_tag::make_display_name($tag); + }); + return $columns; } @@ -323,6 +363,22 @@ class course extends base { $filters[] = $filter; } + // Tags. + $tag = $this->get_table_alias('tag'); + $filters[] = (new filter( + tags::class, + 'tags', + new lang_string('tags'), + $this->get_entity_name(), + "{$tag}.id" + )) + ->add_joins($this->get_joins()) + ->add_joins($this->get_tag_joins()) + ->set_options([ + 'component' => 'core', + 'itemtype' => 'course', + ]); + // We add our own custom course selector filter. $filters[] = (new filter( course_selector::class, diff --git a/reportbuilder/classes/local/entities/user.php b/reportbuilder/classes/local/entities/user.php index 5337efd725b..8e7c3432509 100644 --- a/reportbuilder/classes/local/entities/user.php +++ b/reportbuilder/classes/local/entities/user.php @@ -21,6 +21,7 @@ namespace core_reportbuilder\local\entities; use context_helper; use context_system; use context_user; +use core_tag_tag; use html_writer; use lang_string; use moodle_url; @@ -29,6 +30,7 @@ use core_user\fields; use core_reportbuilder\local\filters\boolean_select; use core_reportbuilder\local\filters\date; use core_reportbuilder\local\filters\select; +use core_reportbuilder\local\filters\tags; use core_reportbuilder\local\filters\text; use core_reportbuilder\local\filters\user as user_filter; use core_reportbuilder\local\helpers\user_profile_fields; @@ -56,6 +58,8 @@ class user extends base { return [ 'user' => 'u', 'context' => 'uctx', + 'tag_instance' => 'ti', + 'tag' => 't', ]; } @@ -105,6 +109,26 @@ class user extends base { return $userprofilefields; } + /** + * Return joins necessary for retrieving tags + * + * @return string[] + */ + private function get_tag_joins(): array { + $user = $this->get_table_alias('user'); + $taginstance = $this->get_table_alias('tag_instance'); + $tag = $this->get_table_alias('tag'); + + return [ + "LEFT JOIN {tag_instance} {$taginstance} + ON {$taginstance}.component = 'core' + AND {$taginstance}.itemtype = 'user' + AND {$taginstance}.itemid = {$user}.id", + "LEFT JOIN {tag} {$tag} + ON {$tag}.id = {$taginstance}.tagid", + ]; + } + /** * Returns list of all available columns * @@ -219,6 +243,22 @@ class user extends base { return !empty($row->id) ? $OUTPUT->user_picture($row, ['link' => false, 'alttext' => false]) : ''; }); + // Interests (tags). + $tag = $this->get_table_alias('tag'); + $columns[] = (new column( + 'interests', + new lang_string('interests'), + $this->get_entity_name() + )) + ->add_joins($this->get_joins()) + ->add_joins($this->get_tag_joins()) + ->set_type(column::TYPE_TEXT) + ->add_fields("{$tag}.name, {$tag}.rawname") + ->set_is_sortable(true) + ->add_callback(static function($value, stdClass $tag): string { + return core_tag_tag::make_display_name($tag); + }); + // Add all other user fields. $userfields = $this->get_user_fields(); foreach ($userfields as $userfield => $userfieldlang) { @@ -460,6 +500,22 @@ class user extends base { $filters[] = $filter; } + // Interests (tags). + $tag = $this->get_table_alias('tag'); + $filters[] = (new filter( + tags::class, + 'interests', + new lang_string('interests'), + $this->get_entity_name(), + "{$tag}.id" + )) + ->add_joins($this->get_joins()) + ->add_joins($this->get_tag_joins()) + ->set_options([ + 'component' => 'core', + 'itemtype' => 'user', + ]); + // User select filter. $filters[] = (new filter( user_filter::class, diff --git a/reportbuilder/classes/local/filters/tags.php b/reportbuilder/classes/local/filters/tags.php new file mode 100644 index 00000000000..6a71729a5a5 --- /dev/null +++ b/reportbuilder/classes/local/filters/tags.php @@ -0,0 +1,133 @@ +. + +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; + +/** + * Class containing logic for the tags filter + * + * The field SQL should be the field containing the ID of the {tag} table + * + * The following array properties must be passed to the {@see \core_reportbuilder\local\report\filter::set_options} method when + * defining this filter, to define the component/itemtype you are using for tags: + * + * ['component' => 'core', 'itemtype' => 'user'] + * + * @package core_reportbuilder + * @copyright 2022 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class tags extends base { + + /** @var int Any value */ + public const ANY_VALUE = 0; + + /** @var int Tags are present */ + public const NOT_EMPTY = 1; + + /** @var int Filter for selected tags */ + public const EQUAL_TO = 2; + + /** + * Returns an array of comparison operators + * + * @return array + */ + private function get_operators(): array { + $operators = [ + self::ANY_VALUE => new lang_string('filterisanyvalue', 'core_reportbuilder'), + self::NOT_EMPTY => new lang_string('filterisnotempty', 'core_reportbuilder'), + self::EQUAL_TO => new lang_string('filterisequalto', 'core_reportbuilder'), + ]; + + return $this->filter->restrict_limited_operators($operators); + } + + /** + * Setup form + * + * @param MoodleQuickForm $mform + * @throws coding_exception If component/itemtype options are missing + */ + public function setup_form(MoodleQuickForm $mform): void { + global $DB; + + $options = $this->filter->get_options(); + if (!array_key_exists('component', $options) || !array_key_exists('itemtype', $options)) { + throw new coding_exception('Missing \'component\' and/or \'itemtype\' in filter options'); + } + + $operatorlabel = get_string('filterfieldoperator', 'core_reportbuilder', $this->get_header()); + $mform->addElement('select', "{$this->name}_operator", $operatorlabel, $this->get_operators()) + ->setHiddenLabel(true); + + $sql = 'SELECT DISTINCT t.id, t.name, t.rawname + FROM {tag} t + JOIN {tag_instance} ti ON ti.tagid = t.id + WHERE ti.component = :component AND ti.itemtype = :itemtype + 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, ['component' => $options['component'], 'itemtype' => $options['itemtype']])); + + $valuelabel = get_string('filterfieldvalue', 'core_reportbuilder', $this->get_header()); + $mform->addElement('autocomplete', "{$this->name}_value", $valuelabel, $tags, ['multiple' => true]) + ->setHiddenLabel(true); + $mform->hideIf("{$this->name}_value", "{$this->name}_operator", 'neq', self::EQUAL_TO); + } + + /** + * Return filter SQL + * + * @param array $values + * @return array + */ + public function get_sql_filter(array $values): array { + global $DB; + + $fieldsql = $this->filter->get_field_sql(); + $params = $this->filter->get_field_params(); + + $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::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 { + // Invalid/inactive (any value) filter.. + return ['', []]; + } + + return [$select, $params]; + } +} diff --git a/reportbuilder/tests/behat/conditioneditor.feature b/reportbuilder/tests/behat/conditioneditor.feature index 10ebfe60e38..00a3b6b5a27 100644 --- a/reportbuilder/tests/behat/conditioneditor.feature +++ b/reportbuilder/tests/behat/conditioneditor.feature @@ -13,9 +13,9 @@ Feature: Manage custom report conditions | My report | user:fullname | | My report | user:email | And the following "users" exist: - | username | firstname | lastname | email | - | user01 | User | One | user01@example.com | - | user02 | User | Two | user02@example.com | + | username | firstname | lastname | email | interests | + | user01 | User | One | user01@example.com | lionel, dancing | + | user02 | User | Two | user02@example.com | | Scenario: Add condition to report Given I am on the "My report" "reportbuilder > Editor" page logged in as "admin" @@ -33,6 +33,20 @@ Feature: Manage custom report conditions And I should see "User One" in the "reportbuilder-table" "table" And I should not see "User Two" in the "reportbuilder-table" "table" + Scenario: Add tags condition to report + Given the following "core_reportbuilder > Condition" exists: + | report | My report | + | uniqueidentifier | user:interests | + And I am on the "My report" "reportbuilder > Editor" page logged in as "admin" + When I click on "Show/hide 'Conditions'" "button" + And I set the following fields in the "Interests" "core_reportbuilder > Condition" to these values: + | Interests operator | Is equal to | + | Interests value | dancing | + And I click on "Apply" "button" in the "[data-region='settings-conditions']" "css_element" + Then I should see "Conditions applied" + And I should see "User One" in the "reportbuilder-table" "table" + And I should not see "User Two" in the "reportbuilder-table" "table" + Scenario: Move condition in report Given the following "core_reportbuilder > Conditions" exist: | report | uniqueidentifier | diff --git a/reportbuilder/tests/local/entities/course_test.php b/reportbuilder/tests/local/entities/course_test.php index 893d8fdf833..b445508a53c 100644 --- a/reportbuilder/tests/local/entities/course_test.php +++ b/reportbuilder/tests/local/entities/course_test.php @@ -27,6 +27,7 @@ use core_reportbuilder\manager; use core_reportbuilder\testable_system_report_table; use core_reportbuilder\local\filters\boolean_select; 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\user_filter_manager; @@ -76,6 +77,7 @@ class course_test extends advanced_testcase { 'calendartype' => 'Gregorian', 'theme' => 'afterburner', 'lang' => 'en', + 'tags' => ['dancing'], ]); $coursecategory2 = $this->getDataGenerator()->create_category(); @@ -154,6 +156,7 @@ class course_test extends advanced_testcase { $this->assertEquals('Gregorian', $courserow['calendartype']); $this->assertEquals('afterburner', $courserow['theme']); $this->assertEquals(get_string_manager()->get_list_of_translations()['en'], $courserow['lang']); + $this->assertEquals('dancing', $courserow['tags']); $expected = 'Course 1'; $this->assertEquals($expected, $courserow['coursefullnamewithlink']); $expected = 'C1'; @@ -171,6 +174,8 @@ class course_test extends advanced_testcase { * Test filtering report by course fields */ public function test_filters(): void { + global $DB; + $this->resetAfterTest(); [$coursecategory1] = $this->generate_courses(); @@ -243,6 +248,15 @@ class course_test extends advanced_testcase { $this->assertEquals([ 'Course 1', ], array_column($tablerows, 'fullname')); + + // Filter by tags field. + $tablerows = $this->get_report_table_rows([ + 'course:tags_operator' => tags::EQUAL_TO, + 'course:tags_value' => [ + $DB->get_field('tag', 'id', ['name' => 'dancing'], MUST_EXIST), + ], + ]); + $this->assertEquals(['Course 1'], array_column($tablerows, 'fullname')); } /** diff --git a/reportbuilder/tests/local/entities/user_test.php b/reportbuilder/tests/local/entities/user_test.php index 879b65891e9..1e3e063c3ca 100644 --- a/reportbuilder/tests/local/entities/user_test.php +++ b/reportbuilder/tests/local/entities/user_test.php @@ -26,6 +26,7 @@ use core_reportbuilder\user_entity_report; use core_reportbuilder\local\filters\boolean_select; use core_reportbuilder\local\filters\date; use core_reportbuilder\local\filters\select; +use core_reportbuilder\local\filters\tags; use core_reportbuilder\local\filters\text; use core_reportbuilder\local\helpers\user_filter_manager; @@ -70,6 +71,7 @@ class user_test extends advanced_testcase { 'suspended' => 1, 'confirmed' => 0, 'country' => 'ES', + 'interests' => ['dancing'], 'profile_field_favcolor' => 'Blue', 'profile_field_favsuperpower' => 'Time travel', ]); @@ -83,6 +85,7 @@ class user_test extends advanced_testcase { $this->assertEquals('Yes', $userrow['suspended']); $this->assertEquals('No', $userrow['confirmed']); $this->assertEquals('Spain', $userrow['country']); + $this->assertEquals('dancing', $userrow['interests']); $this->assertEquals('Blue', $userrow['profilefield_favcolor']); $this->assertEquals('Time travel', $userrow['profilefield_favsuperpower']); } @@ -138,10 +141,12 @@ class user_test extends advanced_testcase { * Test filtering report by user fields */ public function test_filters(): void { + global $DB; + $this->resetAfterTest(); $this->getDataGenerator()->create_user(['firstname' => 'Daffy', 'lastname' => 'Duck', 'email' => 'daffy@test.com', - 'city' => 'LA', 'lastaccess' => time() - YEARSECS, 'suspended' => 1]); + 'city' => 'LA', 'lastaccess' => time() - YEARSECS, 'suspended' => 1, 'interests' => ['dancing']]); $this->getDataGenerator()->create_user(['firstname' => 'Donald', 'lastname' => 'Duck', 'email' => 'donald@test.com', 'city' => 'Chicago', 'lastaccess' => time(), 'suspended' => 0]); @@ -225,6 +230,15 @@ class user_test extends advanced_testcase { $this->assertEquals([ 'Daffy Duck', ], array_column($tablerows, 'fullname')); + + // Filter by interests (tags) field. + $tablerows = $this->get_report_table_rows([ + 'user:interests_operator' => tags::EQUAL_TO, + 'user:interests_value' => [ + $DB->get_field('tag', 'id', ['name' => 'dancing'], MUST_EXIST), + ], + ]); + $this->assertEquals(['Daffy Duck'], array_column($tablerows, 'fullname')); } /** diff --git a/reportbuilder/tests/local/filters/tags_test.php b/reportbuilder/tests/local/filters/tags_test.php new file mode 100644 index 00000000000..8e80a02f3c8 --- /dev/null +++ b/reportbuilder/tests/local/filters/tags_test.php @@ -0,0 +1,102 @@ +. + +declare(strict_types=1); + +namespace core_reportbuilder\local\filters; + +use advanced_testcase; +use lang_string; +use core_reportbuilder\local\report\filter; + +/** + * Unit tests for tags report filter + * + * @package core_reportbuilder + * @covers \core_reportbuilder\local\filters\base + * @covers \core_reportbuilder\local\filters\tags + * @copyright 2022 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class tags_test extends advanced_testcase { + + /** + * Data provider for {@see test_get_sql_filter} + * + * @return array[] + */ + public 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']], + 'Equal to unselected' => [tags::EQUAL_TO, null, ['course01', 'course01', 'course02', 'course03']], + 'Equal to selected tag' => [tags::EQUAL_TO, 'cat', ['course01']], + ]; + } + + /** + * Test getting filter SQL + * + * @param int $operator + * @param string|null $tagname + * @param array $expectedcoursenames + * + * @dataProvider get_sql_filter_provider + */ + public function test_get_sql_filter(int $operator, ?string $tagname, array $expectedcoursenames): void { + global $DB; + + $this->resetAfterTest(); + + $this->getDataGenerator()->create_course(['fullname' => 'course01', 'tags' => ['cat', 'dog']]); + $this->getDataGenerator()->create_course(['fullname' => 'course02', 'tags' => ['fish']]); + $this->getDataGenerator()->create_course(['fullname' => 'course03']); + + $filter = (new filter( + tags::class, + 'tags', + new lang_string('tags'), + 'testentity', + 't.id' + )); + + // 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 c.fullname + FROM {course} c + LEFT JOIN {tag_instance} ti ON ti.itemid = c.id + LEFT JOIN {tag} t ON t.id = ti.tagid + WHERE c.id != ' . SITEID; + + if ($select) { + $sql .= " AND {$select}"; + } + + $courses = $DB->get_fieldset_sql($sql, $params); + $this->assertEqualsCanonicalizing($expectedcoursenames, $courses); + } +} diff --git a/reportbuilder/upgrade.txt b/reportbuilder/upgrade.txt index f0ee2a9bf1f..3c769a2da33 100644 --- a/reportbuilder/upgrade.txt +++ b/reportbuilder/upgrade.txt @@ -19,3 +19,4 @@ Information provided here is intended especially for developers. - `[require_]can_create_report` * New method `get_default_condition_values()` in base datasource class, to be overridden by sources that wish to define default values for conditions upon report creation. +* New `tags` filter type for reports that contain entities with support for core_tag API -- 2.43.0