From 1317aee9e167bcce30f4b27ef682a421c3b80727 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Thu, 19 Sep 2024 09:09:30 +0100 Subject: [PATCH] MDL-77065 reportbuilder: audience filter for the report access list. --- .../classes/local/filters/audience.php | 96 +++++++++++++++ .../classes/local/helpers/audience.php | 47 ++++--- .../systemreports/report_access_list.php | 16 +++ reportbuilder/tests/behat/audience.feature | 28 +++++ .../tests/local/filters/audience_test.php | 116 ++++++++++++++++++ .../tests/local/helpers/audience_test.php | 62 +++++++++- 6 files changed, 350 insertions(+), 15 deletions(-) create mode 100644 reportbuilder/classes/local/filters/audience.php create mode 100644 reportbuilder/tests/local/filters/audience_test.php diff --git a/reportbuilder/classes/local/filters/audience.php b/reportbuilder/classes/local/filters/audience.php new file mode 100644 index 00000000000..fb98dfe60e9 --- /dev/null +++ b/reportbuilder/classes/local/filters/audience.php @@ -0,0 +1,96 @@ +. + +declare(strict_types=1); + +namespace core_reportbuilder\local\filters; + +use core_reportbuilder\local\helpers\audience as audience_helper; +use core_reportbuilder\local\models\audience as audience_model; + +/** + * Report audience filter + * + * Specific to the report access list, to allow for filtering of the user list according to the audience they belong to + * + * In order to specify for which report we are viewing the access list for, the following options must be passed + * to the filter {@see \core_reportbuilder\local\report\filter::set_options} method + * + * ['reportid' => '...'] + * + * @package core_reportbuilder + * @copyright 2024 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class audience extends select { + + /** + * Return the options for the filter as an array, to be used to populate the select input field + * + * @return array + */ + protected function get_select_options(): array { + $options = []; + + $audiences = audience_helper::get_base_records($this->filter->get_options()['reportid'] ?? 0); + foreach ($audiences as $audience) { + $persistent = $audience->get_persistent(); + + // Check for a custom name, otherwise fall back to default. + if ('' === $audiencelabel = $persistent->get_formatted_heading()) { + $audiencelabel = $audience->get_name(); + } + + $options[$persistent->get('id')] = $audiencelabel; + } + + return $options; + } + + /** + * Return filter SQL + * + * @param array $values + * @return array + */ + public function get_sql_filter(array $values): array { + $reportid = $this->filter->get_options()['reportid'] ?? 0; + + $operator = (int) ($values["{$this->name}_operator"] ?? self::ANY_VALUE); + $audienceid = (int) ($values["{$this->name}_value"] ?? 0); + + switch ($operator) { + case self::EQUAL_TO: + case self::NOT_EQUAL_TO: + $audience = audience_model::get_record(['id' => $audienceid, 'reportid' => $reportid]); + if ($audience === false) { + return ['', []]; + } + + // Generate audience SQL, invert it for "not equal to". + [$select, $params] = audience_helper::user_audience_single_sql($audience, $this->filter->get_field_sql()); + if ($operator === self::NOT_EQUAL_TO) { + $select = "NOT {$select}"; + } + + break; + default: + return ['', []]; + } + + return [$select, $params]; + } +} diff --git a/reportbuilder/classes/local/helpers/audience.php b/reportbuilder/classes/local/helpers/audience.php index 738d429cccb..dfb2e50fd8f 100644 --- a/reportbuilder/classes/local/helpers/audience.php +++ b/reportbuilder/classes/local/helpers/audience.php @@ -209,31 +209,50 @@ class audience { } /** - * Return appropriate list of where clauses and params for given audiences + * Return appropriate select clause and params for given audience + * + * @param audience_model $audience + * @param string $userfieldsql + * @return array [$select, $params] + */ + public static function user_audience_single_sql(audience_model $audience, string $userfieldsql): array { + $select = ''; + $params = []; + + if ($instance = base::instance(0, $audience->to_record())) { + $innerusertablealias = database::generate_alias(); + [$join, $where, $params] = $instance->get_sql($innerusertablealias); + + $select = "{$userfieldsql} IN ( + SELECT {$innerusertablealias}.id + FROM {user} {$innerusertablealias} + {$join} + WHERE {$where} + )"; + } + + return [$select, $params]; + } + + /** + * Return appropriate list of select clauses and params for given audiences * * @param audience_model[] $audiences * @param string $usertablealias - * @return array[] [$wheres, $params] + * @return array[] [$selects, $params] */ public static function user_audience_sql(array $audiences, string $usertablealias = 'u'): array { - $wheres = $params = []; + $selects = $params = []; foreach ($audiences as $audience) { - if ($instance = base::instance(0, $audience->to_record())) { - $instancetablealias = database::generate_alias(); - [$instancejoin, $instancewhere, $instanceparams] = $instance->get_sql($instancetablealias); - - $wheres[] = "{$usertablealias}.id IN ( - SELECT {$instancetablealias}.id - FROM {user} {$instancetablealias} - {$instancejoin} - WHERE {$instancewhere} - )"; + [$instanceselect, $instanceparams] = self::user_audience_single_sql($audience, "{$usertablealias}.id"); + if ($instanceselect !== '') { + $selects[] = $instanceselect; $params += $instanceparams; } } - return [$wheres, $params]; + return [$selects, $params]; } /** diff --git a/reportbuilder/classes/local/systemreports/report_access_list.php b/reportbuilder/classes/local/systemreports/report_access_list.php index 230de69ffcb..78e3228c798 100644 --- a/reportbuilder/classes/local/systemreports/report_access_list.php +++ b/reportbuilder/classes/local/systemreports/report_access_list.php @@ -18,12 +18,15 @@ declare(strict_types=1); namespace core_reportbuilder\local\systemreports; +use core\lang_string; use core_reportbuilder\local\models\audience; use core_reportbuilder\local\models\report; use core_reportbuilder\permission; use core_reportbuilder\system_report; use core_reportbuilder\local\entities\user; +use core_reportbuilder\local\filters\audience as audience_filter; use core_reportbuilder\local\helpers\audience as audience_helper; +use core_reportbuilder\local\report\filter; /** * Report access list @@ -100,6 +103,19 @@ class report_access_list extends system_report { protected function add_filters(user $userentity): void { $this->add_filter($userentity->get_filter('fullname')); + // Include audience filter. + $this->add_filter((new filter( + audience_filter::class, + 'audience', + new lang_string('audience', 'core_reportbuilder'), + $userentity->get_entity_name(), + $userentity->get_table_alias('user') . '.id', + )) + ->set_options([ + 'reportid' => $this->get_parameter('id', 0, PARAM_INT), + ]) + ); + // Include all identity field filters. $identityfilters = $userentity->get_identity_filters($this->get_context()); foreach ($identityfilters as $identityfilter) { diff --git a/reportbuilder/tests/behat/audience.feature b/reportbuilder/tests/behat/audience.feature index 887d58bec36..3f3090913b0 100644 --- a/reportbuilder/tests/behat/audience.feature +++ b/reportbuilder/tests/behat/audience.feature @@ -112,6 +112,34 @@ Feature: Configure access to reports based on intended audience And "Add audience 'Member of cohort'" "link" should not exist And the "title" attribute of "//div[@data-region='sidebar-menu']/descendant::div[normalize-space(.)='Member of cohort']" "xpath_element" should contain "Not available" + Scenario: Configure and filter report audience with multiple types + Given the following "core_reportbuilder > Audiences" exist: + | report | classname | configdata | + | My report | \core_reportbuilder\reportbuilder\audience\admins | | + | My report | \core_reportbuilder\reportbuilder\audience\allusers | | + When I am on the "My report" "reportbuilder > Editor" page logged in as "admin" + And I click on the "Access" dynamic tab + Then the following should exist in the "Report access list" table: + | -1- | + | Admin User | + | User 1 | + | User 2 | + | User 3 | + # Now let's filter them. + And I click on "Filters" "button" + And I set the following fields in the "Audience" "core_reportbuilder > Filter" to these values: + | Audience operator | Is equal to | + | Audience value | Site administrators | + And I click on "Apply" "button" in the "[data-region='report-filters']" "css_element" + And the following should exist in the "Report access list" table: + | -1- | + | Admin User | + And the following should not exist in the "Report access list" table: + | -1- | + | User 1 | + | User 2 | + | User 3 | + Scenario: Configure report audience as user who cannot use specific audience Given the following "users" exist: | username | firstname | lastname | diff --git a/reportbuilder/tests/local/filters/audience_test.php b/reportbuilder/tests/local/filters/audience_test.php new file mode 100644 index 00000000000..763cfc90947 --- /dev/null +++ b/reportbuilder/tests/local/filters/audience_test.php @@ -0,0 +1,116 @@ +. + +declare(strict_types=1); + +namespace core_reportbuilder\local\filters; + +use advanced_testcase; +use core\lang_string; +use core_reportbuilder_generator; +use core_reportbuilder\local\report\filter; +use core_reportbuilder\reportbuilder\audience\manual; +use core_user\reportbuilder\datasource\users; + +/** + * Unit tests for report audience filter + * + * @package core_reportbuilder + * @covers \core_reportbuilder\local\filters\audience + * @copyright 2024 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class audience_test extends advanced_testcase { + + /** + * Data provider for {@see test_get_sql_filter} + * + * @return array + */ + public static function get_sql_filter_provider(): array { + return [ + [select::ANY_VALUE, null, ['user1', 'user2', 'admin', 'guest']], + [select::EQUAL_TO, 'audience1', ['user1']], + [select::EQUAL_TO, 'audience2', ['user2']], + [select::NOT_EQUAL_TO, 'audience1', ['user2', 'admin', 'guest']], + [select::NOT_EQUAL_TO, 'audience2', ['user1', 'admin', 'guest']], + ]; + } + + /** + * Test getting filter SQL + * + * @param int $operator + * @param string|null $audiencename + * @param string[] $expected + * + * @dataProvider get_sql_filter_provider + */ + public function test_get_sql_filter(int $operator, ?string $audiencename, array $expected): void { + global $DB; + + $this->resetAfterTest(); + + $userone = $this->getDataGenerator()->create_user(['username' => 'user1']); + $usertwo = $this->getDataGenerator()->create_user(['username' => 'user2']); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + + // Store created audiences in a lookup table so they can be referenced by name. + $audiences = [ + 'audience1' => $generator->create_audience([ + 'reportid' => $report->get('id'), + 'classname' => manual::class, + 'configdata' => [ + 'users' => [$userone->id], + ], + ]), + 'audience2' => $generator->create_audience([ + 'reportid' => $report->get('id'), + 'classname' => manual::class, + 'configdata' => [ + 'users' => [$usertwo->id], + ], + ]), + ]; + + $audienceid = 0; + if (array_key_exists($audiencename, $audiences)) { + $audienceid = $audiences[$audiencename]->get_persistent()->get('id'); + } + + $filter = (new filter( + audience::class, + 'test', + new lang_string('yes'), + 'testentity', + 'id' + ))->set_options([ + 'reportid' => $report->get('id'), + ]); + + // Create instance of our filter, passing given operator. + [$select, $params] = audience::create($filter)->get_sql_filter([ + $filter->get_unique_identifier() . '_operator' => $operator, + $filter->get_unique_identifier() . '_value' => $audienceid, + ]); + + $usernames = $DB->get_fieldset_select('user', 'username', $select, $params); + $this->assertEqualsCanonicalizing($expected, $usernames); + } +} diff --git a/reportbuilder/tests/local/helpers/audience_test.php b/reportbuilder/tests/local/helpers/audience_test.php index 1ad90c64669..b2c78b3878b 100644 --- a/reportbuilder/tests/local/helpers/audience_test.php +++ b/reportbuilder/tests/local/helpers/audience_test.php @@ -34,7 +34,7 @@ use invalid_parameter_exception; * @copyright 2021 David Matamoros * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class audience_test extends advanced_testcase { +final class audience_test extends advanced_testcase { /** * Test reports list is empty for a normal user without any audience records configured @@ -333,6 +333,66 @@ class audience_test extends advanced_testcase { $this->assertEquals([$report->get('id')], $reports); } + /** + * Test retrieving SQL for single audience + */ + public function test_user_audience_single_sql(): void { + global $DB; + + $this->resetAfterTest(); + + $userone = $this->getDataGenerator()->create_user(); + $usertwo = $this->getDataGenerator()->create_user(); + $userthree = $this->getDataGenerator()->create_user(); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + + $audience = $generator->create_audience(['reportid' => $report->get('id'), 'classname' => manual::class, + 'configdata' => [ + 'users' => [$userone->id, $usertwo->id], + ], + ]); + + [$select, $params] = audience::user_audience_single_sql($audience->get_persistent(), 'u.id'); + $users = $DB->get_fieldset_sql("SELECT u.id FROM {user} u WHERE {$select}", $params); + $this->assertEqualsCanonicalizing([$userone->id, $usertwo->id], $users); + } + + /** + * Test retrieving SQL for multiple audiences + */ + public function test_user_audience_sql(): void { + global $DB; + + $this->resetAfterTest(); + + $userone = $this->getDataGenerator()->create_user(); + $usertwo = $this->getDataGenerator()->create_user(); + $userthree = $this->getDataGenerator()->create_user(); + $userfour = $this->getDataGenerator()->create_user(); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + + $audienceone = $generator->create_audience(['reportid' => $report->get('id'), 'classname' => manual::class, + 'configdata' => [ + 'users' => [$userone->id, $usertwo->id], + ], + ]); + $audiencetwo = $generator->create_audience(['reportid' => $report->get('id'), 'classname' => manual::class, + 'configdata' => [ + 'users' => [$usertwo->id, $userthree->id], + ], + ]); + + [$selects, $params] = audience::user_audience_sql([$audienceone->get_persistent(), $audiencetwo->get_persistent()]); + $users = $DB->get_fieldset_sql("SELECT u.id FROM {user} u WHERE " . implode(' OR ', $selects), $params); + $this->assertEqualsCanonicalizing([$userone->id, $usertwo->id, $userthree->id], $users); + } + /** * Test getting list of audiences in use within schedules for a report */