From e9e7375e6421b45c704f649489e3e06affcfdee2 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Thu, 21 Nov 2024 22:04:38 +0000 Subject: [PATCH] MDL-74488 reportbuilder: method for retrieving report row counts. --- .upgradenotes/MDL-74488-2024120518031275.yml | 13 ++++++ .upgradenotes/MDL-83718-2024111916571879.yml | 4 +- .../classes/local/helpers/report.php | 20 +++++++++ .../classes/local/helpers/schedule.php | 9 ++-- .../classes/table/base_report_table.php | 18 ++++++++ reportbuilder/classes/task/send_schedule.php | 8 ++-- .../tests/local/helpers/report_test.php | 42 ++++++++++++++++++- .../tests/local/helpers/schedule_test.php | 1 + 8 files changed, 103 insertions(+), 12 deletions(-) create mode 100644 .upgradenotes/MDL-74488-2024120518031275.yml diff --git a/.upgradenotes/MDL-74488-2024120518031275.yml b/.upgradenotes/MDL-74488-2024120518031275.yml new file mode 100644 index 0000000000000..377352e6f4ada --- /dev/null +++ b/.upgradenotes/MDL-74488-2024120518031275.yml @@ -0,0 +1,13 @@ +issueNumber: MDL-74488 +notes: + core_reportbuilder: + - message: >- + New `report` helper class `get_report_row_count` method for retrieving + row count of custom or system report, without having to retrieve the + report content + type: improved + - message: >- + The `schedule` helper class `get_schedule_report_count` method is now + deprecated, existing code should instead use + `report::get_report_row_count` + type: deprecated diff --git a/.upgradenotes/MDL-83718-2024111916571879.yml b/.upgradenotes/MDL-83718-2024111916571879.yml index ee1b8fffc8514..b74905aa585a9 100644 --- a/.upgradenotes/MDL-83718-2024111916571879.yml +++ b/.upgradenotes/MDL-83718-2024111916571879.yml @@ -4,6 +4,6 @@ notes: - message: >- Report table instances no longer populate the `countsql` and `countparams` class properties. Instead calling code can access - `totalrows` to obtain the same value, rather than manually counting via - the prior properties + `totalrows` to obtain the same value, or by calling the helper + method `report::get_report_row_count` type: changed diff --git a/reportbuilder/classes/local/helpers/report.php b/reportbuilder/classes/local/helpers/report.php index cc8535cf972eb..805d187cc33d4 100644 --- a/reportbuilder/classes/local/helpers/report.php +++ b/reportbuilder/classes/local/helpers/report.php @@ -23,6 +23,7 @@ use core\persistent; use core_reportbuilder\datasource; use core_reportbuilder\manager; +use core_reportbuilder\table\{custom_report_table_view, system_report_table}; use core_reportbuilder\local\models\{audience as audience_model, column, filter, report as report_model, schedule}; use core_tag_tag; @@ -497,6 +498,25 @@ public static function reorder_report_filter(int $reportid, int $filterid, int $ return static::reorder_persistents_by_field($filter, $filters, $position, 'filterorder'); } + /** + * Get total row count for given custom or system report + * + * @param int $reportid + * @param array $parameters Applicable for system reports only + * @return int + */ + public static function get_report_row_count(int $reportid, array $parameters = []): int { + $report = new report_model($reportid); + if ($report->get('type') === datasource::TYPE_CUSTOM_REPORT) { + $table = custom_report_table_view::create($report->get('id')); + } else { + $table = system_report_table::create($report->get('id'), $parameters); + $table->guess_base_url(); + } + $table->setup(); + return $table->get_total_row_count(); + } + /** * @deprecated since Moodle 4.1 - please do not use this function any more, {@see custom_report_column_cards_exporter} */ diff --git a/reportbuilder/classes/local/helpers/schedule.php b/reportbuilder/classes/local/helpers/schedule.php index e92498f38a2a0..d5126ae8af4a0 100644 --- a/reportbuilder/classes/local/helpers/schedule.php +++ b/reportbuilder/classes/local/helpers/schedule.php @@ -141,13 +141,14 @@ public static function get_schedule_report_users(model $schedule): array { * * @param model $schedule * @return int + * + * @deprecated since Moodle 5.0 - please do not use this function any more, {@see report::get_report_row_count} */ + #[\core\attribute\deprecated('report::get_report_row_count', since: '5.0', mdl: 'MDL-74488')] public static function get_schedule_report_count(model $schedule): int { - $table = custom_report_table_view::create($schedule->get('reportid')); - $table->setup(); - $table->query_db(0, false); + \core\deprecation::emit_deprecation_if_present([self::class, __FUNCTION__]); - return $table->totalrows; + return report::get_report_row_count($schedule->get('reportid')); } /** diff --git a/reportbuilder/classes/table/base_report_table.php b/reportbuilder/classes/table/base_report_table.php index 19f0386c275bd..0a7d7ecae3626 100644 --- a/reportbuilder/classes/table/base_report_table.php +++ b/reportbuilder/classes/table/base_report_table.php @@ -203,6 +203,24 @@ public function query_db($pagesize, $useinitialsbar = true): void { } } + /** + * Return total row count for report table. Note we'd typically use {@see query_db} and then read the {@see totalrows} + * property to reduce DB calls, however we can use this method when we specifically don't also need to obtain all data + * + * @return int + */ + public function get_total_row_count(): int { + global $DB; + + $counttablesql = $this->get_table_sql(false); + $counttablealias = database::generate_alias(); + + return $DB->count_records_sql( + "SELECT COUNT(1) FROM ({$counttablesql}) {$counttablealias}", + $this->sql->params, + ); + } + /** * Override parent method of the same, to ensure that any columns with custom sort fields are accounted for * diff --git a/reportbuilder/classes/task/send_schedule.php b/reportbuilder/classes/task/send_schedule.php index f021d92430dc3..a62e07ef41f30 100644 --- a/reportbuilder/classes/task/send_schedule.php +++ b/reportbuilder/classes/task/send_schedule.php @@ -21,7 +21,7 @@ use core\{clock, di}; use core\task\adhoc_task; use core_user; -use core_reportbuilder\local\helpers\schedule as helper; +use core_reportbuilder\local\helpers\{report, schedule as helper}; use core_reportbuilder\local\models\schedule; use moodle_exception; @@ -109,8 +109,8 @@ public function execute(): void { } // Apply special handling if report is empty (default is to send it anyway). - if ($schedulereportempty === schedule::REPORT_EMPTY_DONT_SEND && - $scheduleattachment !== null && helper::get_schedule_report_count($schedule) === 0) { + if ($schedulereportempty === schedule::REPORT_EMPTY_DONT_SEND && $scheduleattachment !== null + && report::get_report_row_count($schedule->get('reportid')) === 0) { $this->log('Empty report, skipping'); } else { @@ -126,7 +126,7 @@ public function execute(): void { \core\cron::setup_user($user); if ($schedulereportempty === schedule::REPORT_EMPTY_DONT_SEND && - helper::get_schedule_report_count($schedule) === 0) { + report::get_report_row_count($schedule->get('reportid')) === 0) { $this->log('Empty report, skipping', 2); continue; diff --git a/reportbuilder/tests/local/helpers/report_test.php b/reportbuilder/tests/local/helpers/report_test.php index 8c911268d771c..2082533bc3b53 100644 --- a/reportbuilder/tests/local/helpers/report_test.php +++ b/reportbuilder/tests/local/helpers/report_test.php @@ -19,12 +19,14 @@ namespace core_reportbuilder\local\helpers; use advanced_testcase; +use core_admin\reportbuilder\local\systemreports\users as adminusers; +use core\context\system; use core_reportbuilder_generator; -use invalid_parameter_exception; -use core_reportbuilder\datasource; +use core_reportbuilder\{datasource, system_report_factory}; use core_reportbuilder\local\models\{audience, column, filter, schedule}; use core_tag_tag; use core_user\reportbuilder\datasource\users; +use invalid_parameter_exception; /** * Unit tests for the report helper class @@ -827,4 +829,40 @@ public function test_reorder_report_filter_invalid(): void { $this->expectExceptionMessage('Invalid filter'); report::reorder_report_filter($report->get('id'), 42, 1); } + + /** + * Test getting row count for a custom report + */ + public function test_get_report_row_count_custom_report(): void { + $this->resetAfterTest(); + + $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, 'default' => 0]); + $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'user:fullname']); + $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'cohort:name']); + $generator->create_column(['reportid' => $report->get('id'), 'uniqueidentifier' => 'tag:name']); + + // There are two users, the admin plus that we just created. + $this->assertEquals(2, report::get_report_row_count($report->get('id'))); + } + + /** + * Test getting row count for a system report + */ + public function test_get_report_row_count_system_report(): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + $this->getDataGenerator()->create_user(); + + $report = system_report_factory::create(adminusers::class, system::instance()) + ->get_report_persistent(); + + // There are two users, the admin plus that we just created. + $this->assertEquals(2, report::get_report_row_count($report->get('id'), [])); + } } diff --git a/reportbuilder/tests/local/helpers/schedule_test.php b/reportbuilder/tests/local/helpers/schedule_test.php index 21bafb0841329..805090e236377 100644 --- a/reportbuilder/tests/local/helpers/schedule_test.php +++ b/reportbuilder/tests/local/helpers/schedule_test.php @@ -263,6 +263,7 @@ public function test_get_schedule_report_count(): void { // There is only one row in the report (the only user on the site). $count = schedule::get_schedule_report_count($schedule); + $this->assertDebuggingCalled(); $this->assertEquals(1, $count); }