diff --git a/reportbuilder/classes/form/schedule.php b/reportbuilder/classes/form/schedule.php index 6501e998898..8c8c16c0d52 100644 --- a/reportbuilder/classes/form/schedule.php +++ b/reportbuilder/classes/form/schedule.php @@ -136,14 +136,14 @@ class schedule extends dynamic_form { $persistent = $audience->get_persistent(); // Check for a custom name, otherwise fall back to default. - if ('' === $audiencelabel = (string) $persistent->get('heading')) { + if ('' === $audiencelabel = $persistent->get_formatted_heading($context)) { $audiencelabel = get_string('audiencelabel', 'core_reportbuilder', (object) [ 'name' => $audience->get_name(), 'description' => $audience->get_description(), ]); } - $audiencecheckboxes[] = $mform->createElement('checkbox', $audience->get_persistent()->get('id'), $audiencelabel); + $audiencecheckboxes[] = $mform->createElement('checkbox', $persistent->get('id'), $audiencelabel); } $mform->addElement('group', 'audiences', '', $audiencecheckboxes, html_writer::div('', 'w-100 mb-2')); diff --git a/reportbuilder/classes/local/systemreports/report_schedules.php b/reportbuilder/classes/local/systemreports/report_schedules.php index 1a7d0587f1e..dc77ddebf25 100644 --- a/reportbuilder/classes/local/systemreports/report_schedules.php +++ b/reportbuilder/classes/local/systemreports/report_schedules.php @@ -18,6 +18,7 @@ declare(strict_types=1); namespace core_reportbuilder\local\systemreports; +use context; use lang_string; use moodle_url; use pix_icon; @@ -62,7 +63,9 @@ class report_schedules extends system_report { $this->add_join('JOIN {' . report::TABLE . '} rb ON rb.id = sc.reportid'); $this->add_base_condition_simple('sc.reportid', $this->get_parameter('reportid', 0, PARAM_INT)); - $this->add_base_fields('sc.id, sc.name, sc.enabled'); // Necessary for actions/row class. + + // Select fields required for actions, permission checks, and row class callbacks. + $this->add_base_fields('sc.id, sc.name, sc.enabled, rb.contextid'); // Join user entity for "User modified" column. $entityuser = new user(); @@ -280,21 +283,39 @@ class report_schedules extends system_report { )); // Send now action. - $this->add_action(new action( + $this->add_action((new action( new moodle_url('#'), new pix_icon('t/email', ''), ['data-action' => 'schedule-send', 'data-schedule-id' => ':id', 'data-schedule-name' => ':name'], false, new lang_string('sendschedule', 'core_reportbuilder') - )); + )) + ->add_callback(function(stdClass $row): bool { + + // Ensure data name attribute is properly formatted. + $row->name = (new schedule(0, $row))->get_formatted_name( + context::instance_by_id($row->contextid)); + + return true; + }) + ); // Delete action. - $this->add_action(new action( + $this->add_action((new action( new moodle_url('#'), new pix_icon('t/delete', ''), ['data-action' => 'schedule-delete', 'data-schedule-id' => ':id', 'data-schedule-name' => ':name'], false, new lang_string('deleteschedule', 'core_reportbuilder') - )); + )) + ->add_callback(function(stdClass $row): bool { + + // Ensure data name attribute is properly formatted. + $row->name = (new schedule(0, $row))->get_formatted_name( + context::instance_by_id($row->contextid)); + + return true; + }) + ); } } diff --git a/reportbuilder/classes/local/systemreports/reports_list.php b/reportbuilder/classes/local/systemreports/reports_list.php index 5b79bc88c18..44d2fc630a3 100644 --- a/reportbuilder/classes/local/systemreports/reports_list.php +++ b/reportbuilder/classes/local/systemreports/reports_list.php @@ -64,9 +64,10 @@ class reports_list extends system_report { */ protected function initialise(): void { $this->set_main_table('reportbuilder_report', 'rb'); - $this->add_base_condition_simple('rb.type', self::TYPE_CUSTOM_REPORT); - $this->add_base_fields('rb.id, rb.name, rb.source, rb.type, rb.usercreated'); // Necessary for actions/row class. + + // Select fields required for actions, permission checks, and row class callbacks. + $this->add_base_fields('rb.id, rb.name, rb.source, rb.type, rb.usercreated, rb.contextid'); // If user can't view all reports, limit the returned list to those reports they can see. [$where, $params] = $this->filter_by_allowed_reports_sql(); @@ -244,7 +245,7 @@ class reports_list extends system_report { new lang_string('editreportcontent', 'core_reportbuilder') )) ->add_callback(function(stdClass $row): bool { - return $this->report_source_valid($row->source) && permission::can_edit_report($this->get_report_from_row($row)); + return $this->report_source_valid($row->source) && permission::can_edit_report(new report(0, $row)); }) ); @@ -257,7 +258,7 @@ class reports_list extends system_report { new lang_string('editreportdetails', 'core_reportbuilder') )) ->add_callback(function(stdClass $row): bool { - return $this->report_source_valid($row->source) && permission::can_edit_report($this->get_report_from_row($row)); + return $this->report_source_valid($row->source) && permission::can_edit_report(new report(0, $row)); }) ); @@ -271,7 +272,7 @@ class reports_list extends system_report { )) ->add_callback(function(stdClass $row): bool { // We check this only to give the action to editors, because normal users can just click on the report name. - return $this->report_source_valid($row->source) && permission::can_edit_report($this->get_report_from_row($row)); + return $this->report_source_valid($row->source) && permission::can_edit_report(new report(0, $row)); }) ); @@ -284,8 +285,13 @@ class reports_list extends system_report { new lang_string('deletereport', 'core_reportbuilder') )) ->add_callback(function(stdClass $row): bool { + + // Ensure data name attribute is properly formatted. + $report = new report(0, $row); + $row->name = $report->get_formatted_name(); + // We don't check whether report is valid to ensure editor can always delete them. - return permission::can_edit_report($this->get_report_from_row($row)); + return permission::can_edit_report($report); }) ); } @@ -300,23 +306,6 @@ class reports_list extends system_report { return manager::report_source_exists($source, datasource::class) && manager::report_source_available($source); } - /** - * Helper to return the report persistent from the row object. - * - * Note that this persistent, for performance reasons, is not complete and only contains id/type/usercreated fields, which - * are needed for the permission methods. - * - * @param stdClass $row - * @return report - */ - private function get_report_from_row(stdClass $row): report { - return new report(0, (object)[ - 'id' => $row->id, - 'type' => $row->type, - 'usercreated' => $row->usercreated, - ]); - } - /** * Filters the list of reports to return only the ones the user has access to * diff --git a/reportbuilder/tests/behat/audience.feature b/reportbuilder/tests/behat/audience.feature index c37e9c6e52b..011d2a414a5 100644 --- a/reportbuilder/tests/behat/audience.feature +++ b/reportbuilder/tests/behat/audience.feature @@ -111,6 +111,18 @@ Feature: Configure access to reports based on intended audience And I reload the page Then I should see "All my lovely users" in the "[data-region='audience-card']" "css_element" + Scenario: Rename report audience using filters + Given the "multilang" filter is "on" + And the "multilang" filter applies to "content and headings" + And I am on the "My report" "reportbuilder > Editor" page logged in as "admin" + And I click on the "Audience" dynamic tab + And I click on "Add audience 'All users'" "link" + And I press "Save changes" + When I set the field "Rename audience 'All users'" to "EnglishSpanish" + And I reload the page + Then I should see "English" in the "[data-region='audience-card']" "css_element" + And I should not see "Spanish" in the "[data-region='audience-card']" "css_element" + Scenario: Delete report audience Given I am on the "My report" "reportbuilder > Editor" page logged in as "admin" And I click on the "Audience" dynamic tab diff --git a/reportbuilder/tests/behat/customreports.feature b/reportbuilder/tests/behat/customreports.feature index 9efe19703a6..d7eedc017e8 100644 --- a/reportbuilder/tests/behat/customreports.feature +++ b/reportbuilder/tests/behat/customreports.feature @@ -69,6 +69,23 @@ Feature: Manage custom reports | Name | Report source | | My renamed report | Users | + Scenario: Rename custom report using filters + Given the "multilang" filter is "on" + And the "multilang" filter applies to "content and headings" + And the following "core_reportbuilder > Reports" exist: + | name | source | + | My report | core_user\reportbuilder\datasource\users | + And I log in as "admin" + When I navigate to "Reports > Report builder > Custom reports" in site administration + And I set the field "Edit report name" in the "My report" "table_row" to "EnglishSpanish" + And I reload the page + Then I should see "English" in the "reportbuilder-table" "table" + And I should not see "Spanish" in the "reportbuilder-table" "table" + # Confirm report name is correctly shown in action. + And I press "Delete report" action in the "English" report row + And I should see "Are you sure you want to delete the report 'English' and all associated data?" in the "Delete report" "dialogue" + And I click on "Cancel" "button" in the "Delete report" "dialogue" + Scenario: Edit custom report from the custom reports page Given the following "core_reportbuilder > Reports" exist: | name | source | diff --git a/reportbuilder/tests/behat/schedules.feature b/reportbuilder/tests/behat/schedules.feature index dede1eace80..5cee0371c06 100644 --- a/reportbuilder/tests/behat/schedules.feature +++ b/reportbuilder/tests/behat/schedules.feature @@ -44,6 +44,18 @@ Feature: Manage custom report schedules | Name | Starting from | Time last sent | Modified by | | My schedule | ##tomorrow 11:00##%A, %d %B %Y, %H:%M## | Never | Admin User | + Scenario: Create report schedule for audience renamed using filters + Given the "multilang" filter is "on" + And the "multilang" filter applies to "content and headings" + And I am on the "My report" "reportbuilder > Editor" page logged in as "admin" + And I click on the "Audience" dynamic tab + And I set the field "Rename audience 'All users'" to "EnglishSpanish" + When I click on the "Schedules" dynamic tab + And I press "New schedule" + Then I should see "English" in the "New schedule" "dialogue" + And I should not see "Spanish" in the "New schedule" "dialogue" + And I click on "Cancel" "button" in the "New schedule" "dialogue" + Scenario: Rename report schedule Given the following "core_reportbuilder > Schedule" exists: | report | My report | @@ -54,6 +66,26 @@ Feature: Manage custom report schedules And I reload the page Then I should see "My renamed schedule" in the "reportbuilder-table" "table" + Scenario: Rename report schedule using filters + Given the "multilang" filter is "on" + And the "multilang" filter applies to "content and headings" + And the following "core_reportbuilder > Schedule" exists: + | report | My report | + | name | My schedule | + And I am on the "My report" "reportbuilder > Editor" page logged in as "admin" + And I click on the "Schedules" dynamic tab + When I set the field "Edit schedule name" in the "My schedule" "table_row" to "EnglishSpanish" + And I reload the page + Then I should see "English" in the "reportbuilder-table" "table" + And I should not see "Spanish" in the "reportbuilder-table" "table" + # Confirm schedule name is correctly shown in actions. + And I press "Send schedule" action in the "English" report row + And I should see "Are you sure you want to queue the schedule 'English' for sending immediately?" in the "Send schedule" "dialogue" + And I click on "Cancel" "button" in the "Send schedule" "dialogue" + And I press "Delete schedule" action in the "English" report row + And I should see "Are you sure you want to delete the schedule 'English'?" in the "Delete schedule" "dialogue" + And I click on "Cancel" "button" in the "Delete schedule" "dialogue" + Scenario: Toggle report schedule Given the following "core_reportbuilder > Schedules" exist: | report | name |