diff --git a/reportbuilder/classes/manager.php b/reportbuilder/classes/manager.php index 143a7cd20ef..51b7b06db14 100644 --- a/reportbuilder/classes/manager.php +++ b/reportbuilder/classes/manager.php @@ -40,7 +40,7 @@ class manager { /** * Return an instance of a report class from the given report persistent * - * We statically cache the list of loaded reports during request lifecycle, to allow this method to be called + * We statically cache the list of loaded reports per user during request lifecycle, to allow this method to be called * repeatedly without potential performance problems initialising the same report multiple times * * @param report $report @@ -50,7 +50,11 @@ class manager { * @throws source_unavailable_exception */ public static function get_report_from_persistent(report $report, array $parameters = []): base { - $instancekey = $report->get('id'); + global $USER; + + // Cached instance per report/user, to account for initialization dependent on current user. + $instancekey = $report->get('id') . ':' . ($USER->id ?? 0); + if (!array_key_exists($instancekey, static::$instances)) { $source = $report->get('source'); diff --git a/reportbuilder/tests/manager_test.php b/reportbuilder/tests/manager_test.php index ac70d6b6563..68d01b9eb4c 100644 --- a/reportbuilder/tests/manager_test.php +++ b/reportbuilder/tests/manager_test.php @@ -18,14 +18,19 @@ declare(strict_types=1); namespace core_reportbuilder; -use advanced_testcase; use context_system; use core_reportbuilder_generator; +use core_reportbuilder_testcase; use core_user\reportbuilder\datasource\users; use stdClass; use core_reportbuilder\local\models\report; use core_reportbuilder\local\report\base; +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once("{$CFG->dirroot}/reportbuilder/tests/helpers.php"); + /** * Unit tests for the report manager class * @@ -34,7 +39,7 @@ use core_reportbuilder\local\report\base; * @copyright 2020 Paul Holden * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class manager_test extends advanced_testcase { +class manager_test extends core_reportbuilder_testcase { /** * Test creating a report instance from persistent @@ -54,6 +59,44 @@ class manager_test extends advanced_testcase { $this->assertInstanceOf(system_report::class, $systemreport); } + /** + * Test creating a report instance from persistent differs per-user, using a report source whose own initialization is + * dependent on the current user (the users report source, loading available user profile fields) + * + * Note: internally the {@see get_custom_report_content} test helper calls {@see manager::get_report_from_persistent} + */ + public function test_get_report_from_persistent_per_user(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + // Custom profile field, visible only to the admin. + $this->getDataGenerator()->create_custom_profile_field([ + 'shortname' => 'text', 'name' => 'Text field', 'datatype' => 'text', 'visible' => 0]); + $user = $this->getDataGenerator()->create_user(['username' => 'usertwo', 'profile_field_text' => 'Hello']); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + + $report = $generator->create_report(['name' => 'Hidden profile field', 'source' => users::class, 'default' => 0]); + $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:username', 'sortenabled' => 1]); + $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:profilefield_text']); + + $content = $this->get_custom_report_content($report->get('id')); + $this->assertEquals([ + ['admin', ''], + ['usertwo', 'Hello'], + ], array_map('array_values', $content)); + + // Now switch to second, non-admin, user. + $this->setUser($user); + + $content = $this->get_custom_report_content($report->get('id')); + $this->assertEquals([ + ['admin'], + ['usertwo'], + ], array_map('array_values', $content)); + } + /** * Test creating a report instance from persistent with an invalid source */