From f7e0e8cd96bc0c936f171b417a9e75f64bc154b8 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 22 Oct 2024 13:08:57 +0100 Subject: [PATCH] MDL-83345 reportbuilder: migrate filter storage from user preferences. Use a dedicated persistent table/model for storage of a users' report filter configuration. --- lang/en/deprecated.txt | 1 + lang/en/reportbuilder.php | 8 +- lib/db/install.xml | 20 +++ lib/db/upgrade.php | 32 +++++ .../local/helpers/user_filter_manager.php | 88 +++++------- reportbuilder/classes/local/models/report.php | 5 + .../classes/local/models/user_filter.php | 73 ++++++++++ reportbuilder/classes/privacy/provider.php | 82 ++++++----- .../helpers/user_filter_manager_test.php | 136 ++++++++---------- reportbuilder/tests/privacy/provider_test.php | 127 ++++++---------- version.php | 2 +- 11 files changed, 330 insertions(+), 244 deletions(-) create mode 100644 reportbuilder/classes/local/models/user_filter.php diff --git a/lang/en/deprecated.txt b/lang/en/deprecated.txt index dbbc168bb8f..c72418d6115 100644 --- a/lang/en/deprecated.txt +++ b/lang/en/deprecated.txt @@ -135,6 +135,7 @@ filterbothactive,core_grades filterbyname,core_grades filterfirstactive,core_grades filterlastactive,core_grades +privacy:metadata:preference:reportfilter,core_reportbuilder noreplybouncemessage,core noreplybouncesubject,core configgeneralquestionbank,core_backup diff --git a/lang/en/reportbuilder.php b/lang/en/reportbuilder.php index 1a16e89fc85..760344e4049 100644 --- a/lang/en/reportbuilder.php +++ b/lang/en/reportbuilder.php @@ -200,7 +200,6 @@ $string['privacy:metadata:filter'] = 'Report filter definitions'; $string['privacy:metadata:filter:uniqueidentifier'] = 'Unique identifier of the filter'; $string['privacy:metadata:filter:usercreated'] = 'The ID of the user who created the filter'; $string['privacy:metadata:filter:usermodified'] = 'The ID of the user who last modified the filter'; -$string['privacy:metadata:preference:reportfilter'] = 'Stored report filter values'; $string['privacy:metadata:report'] = 'Report definitions'; $string['privacy:metadata:report:conditiondata'] = 'Configuration data for the report conditions'; $string['privacy:metadata:report:name'] = 'The name of the report'; @@ -226,6 +225,10 @@ $string['privacy:metadata:schedule:timescheduled'] = 'The time the schedule will $string['privacy:metadata:schedule:usercreated'] = 'The ID of the user who created the schedule'; $string['privacy:metadata:schedule:usermodified'] = 'The ID of the user who last modified the schedule'; $string['privacy:metadata:schedule:userviewas'] = 'The ID of the user who the schedule will be viewed as'; +$string['privacy:metadata:user_filter'] = 'Report user filter definitions'; +$string['privacy:metadata:user_filter:filterdata'] = 'Configuration data for the report filters'; +$string['privacy:metadata:user_filter:timecreated'] = 'The time when the user filter was created'; +$string['privacy:metadata:user_filter:timemodified'] = 'The time when the user filter was last modified'; $string['recurrence'] = 'Recurrence'; $string['recurrenceannually'] = 'Annually'; $string['recurrencedaily'] = 'Daily'; @@ -295,3 +298,6 @@ $string['filterdurationunit'] = '{$a} unit'; // Deprecated since Moodle 4.5. $string['filterdatefrom'] = 'Date from'; $string['filterdateto'] = 'Date to'; + +// Deprecated since Moodle 5.0. +$string['privacy:metadata:preference:reportfilter'] = 'Stored report filter values'; diff --git a/lib/db/install.xml b/lib/db/install.xml index dd261b09044..092a55112e3 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -4621,6 +4621,26 @@ + + + + + + + + + + + + + + + + + + + +
diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index b9890fbd08e..39673e519d5 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -1236,5 +1236,37 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2024111500.04); } + if ($oldversion < 2024112900.01) { + + // Define table reportbuilder_user_filter to be created. + $table = new xmldb_table('reportbuilder_user_filter'); + + // Adding fields to table reportbuilder_user_filter. + $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); + $table->add_field('reportid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0'); + $table->add_field('filterdata', XMLDB_TYPE_TEXT, null, null, XMLDB_NOTNULL, null, null); + $table->add_field('usercreated', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0'); + $table->add_field('usermodified', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0'); + $table->add_field('timecreated', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0'); + $table->add_field('timemodified', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0'); + + // Adding keys to table reportbuilder_user_filter. + $table->add_key('primary', XMLDB_KEY_PRIMARY, ['id']); + $table->add_key('reportid', XMLDB_KEY_FOREIGN, ['reportid'], 'reportbuilder_report', ['id']); + $table->add_key('usercreated', XMLDB_KEY_FOREIGN, ['usercreated'], 'user', ['id']); + $table->add_key('usermodified', XMLDB_KEY_FOREIGN, ['usermodified'], 'user', ['id']); + + // Adding indexes to table reportbuilder_user_filter. + $table->add_index('report-user', XMLDB_INDEX_UNIQUE, ['reportid', 'usercreated']); + + // Conditionally launch create table for reportbuilder_user_filter. + if (!$dbman->table_exists($table)) { + $dbman->create_table($table); + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2024112900.01); + } + return true; } diff --git a/reportbuilder/classes/local/helpers/user_filter_manager.php b/reportbuilder/classes/local/helpers/user_filter_manager.php index 55ac67c76a1..338794b455c 100644 --- a/reportbuilder/classes/local/helpers/user_filter_manager.php +++ b/reportbuilder/classes/local/helpers/user_filter_manager.php @@ -18,36 +18,18 @@ declare(strict_types=1); namespace core_reportbuilder\local\helpers; +use core_reportbuilder\local\models\user_filter; use core_text; /** * This class handles the setting and retrieving of a users' filter values for given reports * - * It is currently using the user preference API as a storage mechanism - * * @package core_reportbuilder * @copyright 2021 Paul Holden * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class user_filter_manager { - /** @var int The size of each chunk, matching the maximum length of a single user preference */ - private const PREFERENCE_CHUNK_SIZE = 1333; - - /** @var string The prefix used to name the stored user preferences */ - private const PREFERENCE_NAME_PREFIX = 'reportbuilder-report-'; - - /** - * Generate user preference name for given report - * - * @param int $reportid - * @param int $index - * @return string - */ - private static function user_preference_name(int $reportid, int $index): string { - return static::PREFERENCE_NAME_PREFIX . "{$reportid}-{$index}"; - } - /** * Set user filters for given report * @@ -57,16 +39,20 @@ class user_filter_manager { * @return bool */ public static function set(int $reportid, array $values, ?int $userid = null): bool { - $jsonvalues = json_encode($values); + global $USER; - $jsonchunks = str_split($jsonvalues, static::PREFERENCE_CHUNK_SIZE); - foreach ($jsonchunks as $index => $jsonchunk) { - $userpreference = static::user_preference_name($reportid, $index); - set_user_preference($userpreference, $jsonchunk, $userid); + $userid ??= $USER->id; + + $userfilter = user_filter::get_record(['reportid' => $reportid, 'usercreated' => $userid]); + if ($userfilter === false) { + $userfilter = new user_filter(0, (object) [ + 'reportid' => $reportid, + 'usercreated' => $userid, + ]); } - // Ensure any subsequent preferences are reset (to account for number of chunks decreasing). - static::reset_all($reportid, $userid, $index + 1); + $userfilter->set('filterdata', json_encode($values)) + ->save(); return true; } @@ -79,17 +65,16 @@ class user_filter_manager { * @return array */ public static function get(int $reportid, ?int $userid = null): array { - $jsonvalues = ''; - $index = 0; + global $USER; - // We'll repeatedly append chunks to our JSON string, until we hit one that is below the maximum length. - do { - $userpreference = static::user_preference_name($reportid, $index++); - $jsonchunk = get_user_preferences($userpreference, '', $userid); - $jsonvalues .= $jsonchunk; - } while (core_text::strlen($jsonchunk) === static::PREFERENCE_CHUNK_SIZE); + $userid ??= $USER->id; - return (array) json_decode($jsonvalues); + $userfilter = user_filter::get_record(['reportid' => $reportid, 'usercreated' => $userid]); + if ($userfilter === false) { + return []; + } + + return (array) json_decode($userfilter->get('filterdata')); } /** @@ -111,18 +96,15 @@ class user_filter_manager { * * @param int $reportid * @param int|null $userid - * @param int $index If specified, then preferences will be reset starting from this index + * @param int $index Unused * @return bool */ public static function reset_all(int $reportid, ?int $userid = null, int $index = 0): bool { - // We'll repeatedly retrieve and reset preferences, until we hit one that is below the maximum length. - do { - $userpreference = static::user_preference_name($reportid, $index++); - $jsonchunk = get_user_preferences($userpreference, '', $userid); - unset_user_preference($userpreference, $userid); - } while (core_text::strlen($jsonchunk) === static::PREFERENCE_CHUNK_SIZE); + global $DB, $USER; - return true; + $userid ??= $USER->id; + + return $DB->delete_records(user_filter::TABLE, ['reportid' => $reportid, 'usercreated' => $userid]); } /** @@ -147,27 +129,21 @@ class user_filter_manager { /** * Get all report filters for given user * - * This is primarily designed for the privacy provider, and allows us to preserve all the preference logic within this class. - * * @param int $userid * @return array + * + * @deprecated since Moodle 5.0 - please do not use this function any more */ + #[\core\attribute\deprecated(null, reason: 'It is no longer used', mdl: 'MDL-83345', since: '5.0')] public static function get_all_for_user(int $userid): array { - global $DB; + \core\deprecation::emit_deprecation_if_present([self::class, __FUNCTION__]); + $prefs = []; - // We need to locate the first preference chunk of all report filters. - $select = 'userid = :userid AND ' . $DB->sql_like('name', ':namelike'); - $params = [ - 'userid' => $userid, - 'namelike' => $DB->sql_like_escape(static::PREFERENCE_NAME_PREFIX) . '%-0', - ]; - $preferences = $DB->get_fieldset_select('user_preferences', 'name', $select, $params); - // Retrieve all found filters. + $preferences = user_filter::get_records(['usercreated' => $userid]); foreach ($preferences as $preference) { - preg_match('/^' . static::PREFERENCE_NAME_PREFIX . '(?\d+)\-/', $preference, $matches); - $prefs[static::PREFERENCE_NAME_PREFIX . $matches['reportid']] = static::get((int) $matches['reportid'], $userid); + $prefs['reportbuilder-report-' . $preference->get('reportid')] = (array) json_decode($preference->get('filterdata')); } return $prefs; diff --git a/reportbuilder/classes/local/models/report.php b/reportbuilder/classes/local/models/report.php index c865e27b589..894152daff4 100644 --- a/reportbuilder/classes/local/models/report.php +++ b/reportbuilder/classes/local/models/report.php @@ -128,6 +128,11 @@ class report extends persistent { $filter->delete(); } + // User filters. + foreach (user_filter::get_records($reportparams) as $userfilter) { + $userfilter->delete(); + } + // Audiences. foreach (audience::get_records($reportparams) as $audience) { $audience->delete(); diff --git a/reportbuilder/classes/local/models/user_filter.php b/reportbuilder/classes/local/models/user_filter.php new file mode 100644 index 00000000000..da16a573dc1 --- /dev/null +++ b/reportbuilder/classes/local/models/user_filter.php @@ -0,0 +1,73 @@ +. + +declare(strict_types=1); + +namespace core_reportbuilder\local\models; + +use lang_string; +use core\persistent; + +/** + * Persistent class to represent a user report filter + * + * @package core_reportbuilder + * @copyright 2024 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class user_filter extends persistent { + + /** @var string The table name. */ + public const TABLE = 'reportbuilder_user_filter'; + + /** + * Return the definition of the properties of this model. + * + * @return array + */ + protected static function define_properties(): array { + return [ + 'reportid' => [ + 'type' => PARAM_INT, + ], + 'filterdata' => [ + 'type' => PARAM_RAW, + ], + 'usercreated' => [ + 'type' => PARAM_INT, + 'default' => static function(): int { + global $USER; + + return (int) $USER->id; + }, + ], + ]; + } + + /** + * Validate reportid property + * + * @param int $reportid + * @return bool|lang_string + */ + protected function validate_reportid(int $reportid) { + if (!report::record_exists($reportid)) { + return new lang_string('invaliddata', 'error'); + } + + return true; + } +} diff --git a/reportbuilder/classes/privacy/provider.php b/reportbuilder/classes/privacy/provider.php index 856c31e204c..87bc5d3284e 100644 --- a/reportbuilder/classes/privacy/provider.php +++ b/reportbuilder/classes/privacy/provider.php @@ -27,16 +27,10 @@ use core_privacy\local\request\contextlist; use core_privacy\local\request\core_userlist_provider; use core_privacy\local\request\transform; use core_privacy\local\request\userlist; -use core_privacy\local\request\user_preference_provider; use core_privacy\local\request\writer; use core_reportbuilder\manager; -use core_reportbuilder\local\helpers\user_filter_manager; use core_reportbuilder\local\helpers\schedule as schedule_helper; -use core_reportbuilder\local\models\audience; -use core_reportbuilder\local\models\column; -use core_reportbuilder\local\models\filter; -use core_reportbuilder\local\models\report; -use core_reportbuilder\local\models\schedule; +use core_reportbuilder\local\models\{audience, column, filter, report, schedule, user_filter}; /** * Privacy Subsystem for core_reportbuilder @@ -48,8 +42,7 @@ use core_reportbuilder\local\models\schedule; class provider implements \core_privacy\local\metadata\provider, \core_privacy\local\request\plugin\provider, - core_userlist_provider, - user_preference_provider { + core_userlist_provider { /** * Returns metadata about the component @@ -82,6 +75,12 @@ class provider implements 'usermodified' => 'privacy:metadata:filter:usermodified', ], 'privacy:metadata:filter'); + $collection->add_database_table(user_filter::TABLE, [ + 'filterdata' => 'privacy:metadata:user_filter:filterdata', + 'timecreated' => 'privacy:metadata:user_filter:timecreated', + 'timemodified' => 'privacy:metadata:user_filter:timemodified', + ], 'privacy:metadata:user_filter'); + $collection->add_database_table(audience::TABLE, [ 'classname' => 'privacy:metadata:audience:classname', 'configdata' => 'privacy:metadata:audience:configdata', @@ -109,29 +108,9 @@ class provider implements 'timemodified' => 'privacy:metadata:schedule:timemodified', ], 'privacy:metadata:schedule'); - $collection->add_user_preference('core_reportbuilder', 'privacy:metadata:preference:reportfilter'); - return $collection; } - /** - * Export all user preferences for the component - * - * @param int $userid - */ - public static function export_user_preferences(int $userid): void { - $preferencestring = get_string('privacy:metadata:preference:reportfilter', 'core_reportbuilder'); - - $filters = user_filter_manager::get_all_for_user($userid); - foreach ($filters as $key => $filter) { - writer::export_user_preference('core_reportbuilder', - $key, - json_encode($filter, JSON_PRETTY_PRINT), - $preferencestring - ); - } - } - /** * Get export sub context for a report * @@ -156,7 +135,7 @@ class provider implements public static function get_contexts_for_userid(int $userid): contextlist { $contextlist = new contextlist(); - // Locate all contexts for reports the user has created, or reports they have created audience/schedules for. + // Locate all contexts for reports the user has created, or reports they have created ancillary data for. $sql = ' SELECT r.contextid FROM {' . report::TABLE . '} r @@ -164,6 +143,10 @@ class provider implements AND (r.usercreated = ? OR r.usermodified = ? OR r.id IN ( + SELECT f.reportid + FROM {' . user_filter::TABLE . '} f + WHERE f.usercreated = ? + UNION SELECT a.reportid FROM {' . audience::TABLE . '} a WHERE a.usercreated = ? OR a.usermodified = ? @@ -174,7 +157,7 @@ class provider implements ) )'; - return $contextlist->add_from_sql($sql, array_fill(0, 6, $userid)); + return $contextlist->add_from_sql($sql, array_fill(0, 7, $userid)); } /** @@ -193,6 +176,13 @@ class provider implements $userlist->add_from_sql('usercreated', $sql, $params); $userlist->add_from_sql('usermodified', $sql, $params); + // Users who have created user filters. + $sql = 'SELECT f.usercreated + FROM {' . user_filter::TABLE . '} f + JOIN {' . report::TABLE . '} r ON r.id = f.reportid + WHERE ' . $select; + $userlist->add_from_sql('usercreated', $sql, $params); + // Users who have created audiences. $sql = 'SELECT a.usercreated, a.usermodified FROM {' . audience::TABLE . '} a @@ -224,6 +214,10 @@ class provider implements // We need to get all reports that the user has created, or reports they have created audience/schedules for. $select = 'type = 0 AND (usercreated = ? OR usermodified = ? OR id IN ( + SELECT f.reportid + FROM {' . user_filter::TABLE . '} f + WHERE f.usercreated = ? + UNION SELECT a.reportid FROM {' . audience::TABLE . '} a WHERE a.usercreated = ? OR a.usermodified = ? @@ -232,13 +226,18 @@ class provider implements FROM {' . schedule::TABLE . '} s WHERE s.usercreated = ? OR s.usermodified = ? ))'; - $params = array_fill(0, 6, $user->id); + $params = array_fill(0, 7, $user->id); foreach (report::get_records_select($select, $params) as $report) { $subcontext = static::get_export_subcontext($report); self::export_report($subcontext, $report); + // User filters. + if ($userfilters = user_filter::get_records(['reportid' => $report->get('id'), 'usercreated' => $user->id])) { + static::export_user_filters($report->get_context(), $subcontext, $userfilters); + } + $select = 'reportid = ? AND (usercreated = ? OR usermodified = ?)'; $params = [$report->get('id'), $user->id, $user->id]; @@ -309,6 +308,25 @@ class provider implements writer::with_context($report->get_context())->export_data($subcontext, $reportdata); } + /** + * Export given user filters in context + * + * @param context $context + * @param array $subcontext + * @param user_filter[] $userfilters + */ + protected static function export_user_filters(context $context, array $subcontext, array $userfilters): void { + $userfilterdata = array_map(static function(user_filter $userfilter): stdClass { + return (object) [ + 'filterdata' => $userfilter->get('filterdata'), + 'timecreated' => transform::datetime($userfilter->get('timecreated')), + 'timemodified' => transform::datetime($userfilter->get('timemodified')), + ]; + }, $userfilters); + + writer::with_context($context)->export_related_data($subcontext, 'userfilters', (object) ['data' => $userfilterdata]); + } + /** * Export given audiences in context * diff --git a/reportbuilder/tests/local/helpers/user_filter_manager_test.php b/reportbuilder/tests/local/helpers/user_filter_manager_test.php index 909c3667877..9ed452ca17a 100644 --- a/reportbuilder/tests/local/helpers/user_filter_manager_test.php +++ b/reportbuilder/tests/local/helpers/user_filter_manager_test.php @@ -19,6 +19,9 @@ declare(strict_types=1); namespace core_reportbuilder\local\helpers; use advanced_testcase; +use core_reportbuilder_generator; +use core_reportbuilder\local\models\user_filter; +use core_user\reportbuilder\datasource\users; /** * Unit tests for the user filter helper @@ -28,18 +31,7 @@ use advanced_testcase; * @copyright 2021 Paul Holden * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class user_filter_manager_test extends advanced_testcase { - - /** - * Helper method to return all user preferences for filters - based on the current storage backend using the same - * - * @return array - */ - private function get_filter_preferences(): array { - return array_filter(get_user_preferences(), static function(string $key): bool { - return strpos($key, 'reportbuilder-report-') === 0; - }, ARRAY_FILTER_USE_KEY); - } +final class user_filter_manager_test extends advanced_testcase { /** * Data provider for {@see test_get} @@ -64,48 +56,30 @@ class user_filter_manager_test extends advanced_testcase { public function test_get(string $value): void { $this->resetAfterTest(); + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + $values = [ 'entity:filter_name' => $value, ]; - user_filter_manager::set(5, $values); + user_filter_manager::set($report->get('id'), $values); // Make sure we get the same value back. - $this->assertEquals($values, user_filter_manager::get(5)); - } - - /** - * Test getting filter values that once spanned multiple chunks - */ - public function test_get_large_to_small(): void { - $this->resetAfterTest(); - - // Set a large initial filter value. - user_filter_manager::set(5, [ - 'longvalue' => str_repeat('ABCD', 1000), - ]); - - // Sanity check, there should be 4 (because 4000 characters plus some JSON encoding requires that many chunks). - $preferences = $this->get_filter_preferences(); - $this->assertCount(4, $preferences); - - $values = [ - 'longvalue' => 'ABCD', - ]; - user_filter_manager::set(5, $values); - - // Make sure we get the same value back. - $this->assertEquals($values, user_filter_manager::get(5)); - - // Everything should now fit in a single filter preference. - $preferences = $this->get_filter_preferences(); - $this->assertCount(1, $preferences); + $this->assertEquals($values, user_filter_manager::get($report->get('id'))); } /** * Test getting filter values that haven't been set */ public function test_get_empty(): void { - $this->assertEquals([], user_filter_manager::get(5)); + $this->resetAfterTest(); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + + $this->assertEquals([], user_filter_manager::get($report->get('id'))); } /** @@ -131,18 +105,22 @@ class user_filter_manager_test extends advanced_testcase { public function test_reset_all(string $value): void { $this->resetAfterTest(); - user_filter_manager::set(5, [ - 'entity:filter_name' => $value + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + + user_filter_manager::set($report->get('id'), [ + 'entity:filter_name' => $value, ]); - $reset = user_filter_manager::reset_all(5); + $reset = user_filter_manager::reset_all($report->get('id')); $this->assertTrue($reset); // We should get an empty array back. - $this->assertEquals([], user_filter_manager::get(5)); + $this->assertEquals([], user_filter_manager::get($report->get('id'))); // All filter preferences should be removed. - $this->assertEmpty($this->get_filter_preferences()); + $this->assertFalse(user_filter::get_record(['reportid' => $report->get('id')])); } /** @@ -151,20 +129,24 @@ class user_filter_manager_test extends advanced_testcase { public function test_reset_single(): void { $this->resetAfterTest(); - user_filter_manager::set(5, [ + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + + user_filter_manager::set($report->get('id'), [ 'entity:filter_name' => 'foo', 'entity:filter_value' => 'bar', 'entity:other_name' => 'baz', 'entity:other_value' => 'bax', ]); - $reset = user_filter_manager::reset_single(5, 'entity:other'); + $reset = user_filter_manager::reset_single($report->get('id'), 'entity:other'); $this->assertTrue($reset); $this->assertEquals([ 'entity:filter_name' => 'foo', 'entity:filter_value' => 'bar', - ], user_filter_manager::get(5)); + ], user_filter_manager::get($report->get('id'))); } /** @@ -173,30 +155,29 @@ class user_filter_manager_test extends advanced_testcase { public function test_merge(): void { $this->resetAfterTest(); - $values = [ + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'My report', 'source' => users::class]); + + user_filter_manager::set($report->get('id'), [ 'entity:filter_name' => 'foo', 'entity:filter_value' => 'bar', 'entity:filter2_name' => 'tree', 'entity:filter2_value' => 'house', - ]; + ]); - // Make sure we get the same value back. - user_filter_manager::set(5, $values); - $this->assertEqualsCanonicalizing($values, user_filter_manager::get(5)); - - user_filter_manager::merge(5, [ + // Make sure that both values have been changed and the other values have not been modified. + user_filter_manager::merge($report->get('id'), [ 'entity:filter_name' => 'twotimesfoo', 'entity:filter_value' => 'twotimesbar', ]); - // Make sure that both values have been changed and the other values have not been modified. - $expected = [ + $this->assertEqualsCanonicalizing([ 'entity:filter_name' => 'twotimesfoo', 'entity:filter_value' => 'twotimesbar', 'entity:filter2_name' => 'tree', 'entity:filter2_value' => 'house', - ]; - $this->assertEqualsCanonicalizing($expected, user_filter_manager::get(5)); + ], user_filter_manager::get($report->get('id'))); } /** @@ -204,30 +185,37 @@ class user_filter_manager_test extends advanced_testcase { */ public function test_get_all_for_user(): void { $this->resetAfterTest(); - $user = $this->getDataGenerator()->create_user(); - $this->setUser($user); - $filtervalues1 = [ + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $reportone = $generator->create_report(['name' => 'Report 1', 'source' => users::class]); + $reporttwo = $generator->create_report(['name' => 'Report 2', 'source' => users::class]); + + $userone = $this->getDataGenerator()->create_user(); + $usertwo = $this->getDataGenerator()->create_user(); + + $reportonefilter = [ 'entity:filter_name' => 'foo', 'entity:filter_value' => 'bar', 'entity:other_name' => 'baz', 'entity:other_value' => 'bax', ]; - user_filter_manager::set(5, $filtervalues1); + user_filter_manager::set($reportone->get('id'), $reportonefilter, (int) $userone->id); - $filtervalues2 = [ + $reporttwofilter = [ 'entity:filter_name' => 'blue', 'entity:filter_value' => 'red', ]; - user_filter_manager::set(9, $filtervalues2); + user_filter_manager::set($reporttwo->get('id'), $reporttwofilter, (int) $userone->id); - $this->setAdminUser(); - $values = user_filter_manager::get_all_for_user((int)$user->id); - $this->assertEqualsCanonicalizing([$filtervalues1, $filtervalues2], [reset($values), end($values)]); + // First user has filters in two reports. + $useronefilters = user_filter_manager::get_all_for_user((int) $userone->id); + $this->assertDebuggingCalled(); + $this->assertEqualsCanonicalizing([$reportonefilter, $reporttwofilter], $useronefilters); // Check for a user with no filters. - $user2 = $this->getDataGenerator()->create_user(); - $values = user_filter_manager::get_all_for_user((int)$user2->id); - $this->assertEmpty($values); + $usertwofilters = user_filter_manager::get_all_for_user((int) $usertwo->id); + $this->assertDebuggingCalled(); + $this->assertEmpty($usertwofilters); } } diff --git a/reportbuilder/tests/privacy/provider_test.php b/reportbuilder/tests/privacy/provider_test.php index 4b9d2328223..9868bb9b3e7 100644 --- a/reportbuilder/tests/privacy/provider_test.php +++ b/reportbuilder/tests/privacy/provider_test.php @@ -21,18 +21,12 @@ namespace core_reportbuilder\privacy; use context_system; use core_privacy\local\metadata\collection; use core_privacy\local\metadata\types\database_table; -use core_privacy\local\metadata\types\user_preference; use core_privacy\local\request\userlist; use core_privacy\local\request\writer; use core_privacy\tests\provider_testcase; use core_reportbuilder_generator; -use core_reportbuilder\manager; use core_reportbuilder\local\helpers\user_filter_manager; -use core_reportbuilder\local\models\audience; -use core_reportbuilder\local\models\column; -use core_reportbuilder\local\models\filter; -use core_reportbuilder\local\models\report; -use core_reportbuilder\local\models\schedule; +use core_reportbuilder\local\models\{audience, column, filter, report, schedule, user_filter}; use core_user\reportbuilder\datasource\users; /** @@ -43,7 +37,7 @@ use core_user\reportbuilder\datasource\users; * @copyright 2021 David Matamoros * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class provider_test extends provider_testcase { +final class provider_test extends provider_testcase { /** * Test provider metadata @@ -53,23 +47,14 @@ class provider_test extends provider_testcase { $metadata = provider::get_metadata($collection)->get_collection(); $this->assertCount(6, $metadata); + $this->assertContainsOnlyInstancesOf(database_table::class, $metadata); - $this->assertInstanceOf(database_table::class, $metadata[0]); $this->assertEquals(report::TABLE, $metadata[0]->get_name()); - - $this->assertInstanceOf(database_table::class, $metadata[1]); $this->assertEquals(column::TABLE, $metadata[1]->get_name()); - - $this->assertInstanceOf(database_table::class, $metadata[2]); $this->assertEquals(filter::TABLE, $metadata[2]->get_name()); - - $this->assertInstanceOf(database_table::class, $metadata[3]); - $this->assertEquals(audience::TABLE, $metadata[3]->get_name()); - - $this->assertInstanceOf(database_table::class, $metadata[4]); - $this->assertEquals(schedule::TABLE, $metadata[4]->get_name()); - - $this->assertInstanceOf(user_preference::class, $metadata[5]); + $this->assertEquals(user_filter::TABLE, $metadata[3]->get_name()); + $this->assertEquals(audience::TABLE, $metadata[4]->get_name()); + $this->assertEquals(schedule::TABLE, $metadata[5]->get_name()); } /** @@ -90,6 +75,28 @@ class provider_test extends provider_testcase { $this->assertInstanceOf(context_system::class, $contextlist->current()); } + /** + * Test getting contexts for user who created a user filter for a report by another user + */ + public function test_get_contexts_for_userid_user_filter(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + $report = $generator->create_report(['name' => 'Users', 'source' => users::class]); + + // Switch user, create a report audience. + $user = $this->getDataGenerator()->create_user(); + $this->setUser($user); + + user_filter_manager::set($report->get('id'), ['entity:filter_name' => 1]); + + $contextlist = $this->get_contexts_for_userid((int) $user->id, 'core_reportbuilder'); + $this->assertCount(1, $contextlist); + $this->assertInstanceOf(context_system::class, $contextlist->current()); + } + /** * Test getting contexts for user who created an audience for a report by another user */ @@ -149,6 +156,12 @@ class provider_test extends provider_testcase { $report = $generator->create_report(['name' => 'Users', 'source' => users::class]); + // Switch user, create a user filter. + $filteruser = $this->getDataGenerator()->create_user(); + $this->setUser($filteruser); + + user_filter_manager::set($report->get('id'), ['entity:filter_name' => 1]); + // Switch user, create a report audience. $audienceuser = $this->getDataGenerator()->create_user(); $this->setUser($audienceuser); @@ -166,6 +179,7 @@ class provider_test extends provider_testcase { $this->assertEqualsCanonicalizing([ $reportuser->id, + $filteruser->id, $audienceuser->id, $scheduleuser->id, ], $userlist->get_userids()); @@ -192,6 +206,8 @@ class provider_test extends provider_testcase { 'name' => 'My schedule', ]); + user_filter_manager::set($report->get('id'), ['entity:filter_name' => 1]); + $context = context_system::instance(); $this->export_context_data_for_user((int) $user->id, $context, 'core_reportbuilder'); @@ -210,6 +226,16 @@ class provider_test extends provider_testcase { $this->assertNotEmpty($reportdata->timecreated); $this->assertNotEmpty($reportdata->timemodified); + // Exported user filter data. + $userfilterdata = $writer->get_related_data($subcontext, 'userfilters')->data; + + $this->assertCount(1, $userfilterdata); + $userfilterdata = reset($userfilterdata); + + $this->assertEquals('{"entity:filter_name":1}', $userfilterdata->filterdata); + $this->assertNotEmpty($userfilterdata->timecreated); + $this->assertNotEmpty($userfilterdata->timemodified); + // Exported audience data. $audiencedata = $writer->get_related_data($subcontext, 'audiences')->data; @@ -262,63 +288,4 @@ class provider_test extends provider_testcase { $this->assertFalse(writer::with_context($context)->has_any_data()); } - - /** - * Test to check export_user_preferences. - */ - public function test_export_user_preferences(): void { - $this->resetAfterTest(); - - $user1 = $this->getDataGenerator()->create_user(); - $user2 = $this->getDataGenerator()->create_user(); - $this->setUser($user1); - - // Create report and set some filters for the user. - $report1 = manager::create_report_persistent((object) [ - 'type' => 1, - 'source' => 'class', - ]); - $filtervalues1 = [ - 'task_log:name_operator' => 0, - 'task_log:name_value' => 'My task logs', - ]; - user_filter_manager::set($report1->get('id'), $filtervalues1); - - // Add a filter for user2. - $filtervalues1user2 = [ - 'task_log:name_operator' => 0, - 'task_log:name_value' => 'My task logs user2', - ]; - user_filter_manager::set($report1->get('id'), $filtervalues1user2, (int)$user2->id); - - // Create a second report and set some filters for the user. - $report2 = manager::create_report_persistent((object) [ - 'type' => 1, - 'source' => 'class', - ]); - $filtervalues2 = [ - 'config_change:setting_operator' => 0, - 'config_change:setting_value' => str_repeat('A', 3000), - ]; - user_filter_manager::set($report2->get('id'), $filtervalues2); - - // Switch to admin user (so we can validate preferences of our test user are still exported). - $this->setAdminUser(); - - // Export user preferences. - provider::export_user_preferences((int)$user1->id); - $writer = writer::with_context(context_system::instance()); - $prefs = $writer->get_user_preferences('core_reportbuilder'); - - // Check that user preferences only contain the 2 preferences from user1. - $this->assertCount(2, (array)$prefs); - - // Check that exported user preferences for report1 are correct. - $report1key = 'reportbuilder-report-' . $report1->get('id'); - $this->assertEquals(json_encode($filtervalues1, JSON_PRETTY_PRINT), $prefs->$report1key->value); - - // Check that exported user preferences for report2 are correct. - $report2key = 'reportbuilder-report-' . $report2->get('id'); - $this->assertEquals(json_encode($filtervalues2, JSON_PRETTY_PRINT), $prefs->$report2key->value); - } } diff --git a/version.php b/version.php index 9c929d5230b..1e914f0dcef 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2024112900.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2024112900.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '5.0dev (Build: 20241129)'; // Human-friendly version name