From 26bb378e88d1a093abe96b06b594f176c2aa76fd Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 15 Oct 2024 20:42:31 +0100 Subject: [PATCH] MDL-82809 reportbuilder: always apply conditions to custom report. Including those that are otherwise unavailable to the current user, ensuring that they are always applied when viewing report output. --- .upgradenotes/MDL-82809-2024101521112024.yml | 8 +++++ reportbuilder/classes/datasource.php | 13 ++++--- reportbuilder/classes/local/report/base.php | 3 +- .../classes/table/base_report_table.php | 2 +- reportbuilder/tests/datasource_test.php | 34 +++++++++++++++++++ 5 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 .upgradenotes/MDL-82809-2024101521112024.yml diff --git a/.upgradenotes/MDL-82809-2024101521112024.yml b/.upgradenotes/MDL-82809-2024101521112024.yml new file mode 100644 index 00000000000..2feb3d2ff0a --- /dev/null +++ b/.upgradenotes/MDL-82809-2024101521112024.yml @@ -0,0 +1,8 @@ +issueNumber: MDL-82809 +notes: + core_reportbuilder: + - message: >- + The `get_active_conditions` method of the base report class has a new + `$checkavailable` parameter to determine whether to check the returned + conditions availability + type: changed diff --git a/reportbuilder/classes/datasource.php b/reportbuilder/classes/datasource.php index f25e4949831..3aec964d30b 100644 --- a/reportbuilder/classes/datasource.php +++ b/reportbuilder/classes/datasource.php @@ -194,8 +194,7 @@ abstract class datasource extends base { " {$instance->get_is_deprecated_message()}", DEBUG_DEVELOPER); } - $this->activefilters['values'][$instance->get_unique_identifier()] = - $instance->set_persistent($filter); + $this->activefilters['values'][$instance->get_unique_identifier()] = $instance->set_persistent($filter); } } @@ -277,9 +276,10 @@ abstract class datasource extends base { * Override parent method, returning only those conditions specifically added to the custom report (rather than all that are * available) * + * @param bool $checkavailable * @return filter[] */ - public function get_active_conditions(): array { + public function get_active_conditions(bool $checkavailable = true): array { $reportid = $this->get_report_persistent()->get('id'); // Determine whether we already retrieved the conditions since the report was last modified. @@ -294,15 +294,14 @@ abstract class datasource extends base { foreach ($activeconditions as $condition) { $instance = $this->get_condition($condition->get('uniqueidentifier')); - // Ensure the condition is still present and available. - if ($instance !== null && $instance->get_is_available()) { + // Ensure the condition is still present and available (if checking available status). + if ($instance !== null && (!$checkavailable || $instance->get_is_available())) { if ($instance->get_is_deprecated()) { debugging("The condition '{$instance->get_unique_identifier()}' is deprecated, please do not use it any more." . " {$instance->get_is_deprecated_message()}", DEBUG_DEVELOPER); } - $this->activeconditions['values'][$instance->get_unique_identifier()] = - $instance->set_persistent($condition); + $this->activeconditions['values'][$instance->get_unique_identifier()] = $instance->set_persistent($condition); } } diff --git a/reportbuilder/classes/local/report/base.php b/reportbuilder/classes/local/report/base.php index 4b9c9bf4aa2..0a335f78bfe 100644 --- a/reportbuilder/classes/local/report/base.php +++ b/reportbuilder/classes/local/report/base.php @@ -553,9 +553,10 @@ abstract class base { /** * Return all active report conditions (by default, all available conditions) * + * @param bool $checkavailable * @return filter[] */ - public function get_active_conditions(): array { + public function get_active_conditions(bool $checkavailable = true): array { $conditions = $this->get_conditions(); foreach ($conditions as $condition) { if ($condition->get_is_deprecated()) { diff --git a/reportbuilder/classes/table/base_report_table.php b/reportbuilder/classes/table/base_report_table.php index d33f8167bf1..3ca83e63b27 100644 --- a/reportbuilder/classes/table/base_report_table.php +++ b/reportbuilder/classes/table/base_report_table.php @@ -79,7 +79,7 @@ abstract class base_report_table extends table_sql implements dynamic, renderabl // For each condition, we need to ensure their values are always accounted for in the report. $conditionvalues = $this->report->get_condition_values(); - foreach ($this->report->get_active_conditions() as $condition) { + foreach ($this->report->get_active_conditions(false) as $condition) { [$conditionsql, $conditionparams] = $this->get_filter_sql($condition, $conditionvalues, 'c' . $conditionindex++); if ($conditionsql !== '') { $joins = array_merge($joins, $condition->get_joins()); diff --git a/reportbuilder/tests/datasource_test.php b/reportbuilder/tests/datasource_test.php index 58349409f24..f7c34cdcc24 100644 --- a/reportbuilder/tests/datasource_test.php +++ b/reportbuilder/tests/datasource_test.php @@ -288,6 +288,40 @@ final class datasource_test extends advanced_testcase { $this->assertCount($expectedcountconditions, $instance->get_conditions()); } + /** + * Test getting active conditions + * + * @covers ::get_active_conditions + */ + public function test_get_active_conditions(): void { + $instance = $this->get_datasource_test_source(); + + $method = (new ReflectionClass($instance))->getMethod('add_conditions_from_entity'); + $method->invoke($instance, 'datasource_test_entity'); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + + $reportid = $instance->get_report_persistent()->get('id'); + $generator->create_condition(['reportid' => $reportid, 'uniqueidentifier' => 'datasource_test_entity:first']); + $generator->create_condition(['reportid' => $reportid, 'uniqueidentifier' => 'datasource_test_entity:second']); + + // Set the second condition as unavailable. + $instance->get_condition('datasource_test_entity:second')->set_is_available(false); + + $this->assertEquals([ + 'datasource_test_entity:first', + ], array_keys($instance->get_active_conditions(true))); + + // Ensure report elements are reloaded. + $instance::report_elements_modified($reportid); + + $this->assertEquals([ + 'datasource_test_entity:first', + 'datasource_test_entity:second', + ], array_keys($instance->get_active_conditions(false))); + } + /** * Create and return our test datasource instance *