diff --git a/reportbuilder/classes/form/report.php b/reportbuilder/classes/form/report.php index 303b84a222a..ba9321a6adb 100644 --- a/reportbuilder/classes/form/report.php +++ b/reportbuilder/classes/form/report.php @@ -147,4 +147,21 @@ class report extends dynamic_form { public function get_page_url_for_dynamic_submission(): moodle_url { return new moodle_url('/reportbuilder/index.php'); } + + /** + * Perform some extra moodle validation + * + * @param array $data + * @param array $files + * @return array + */ + public function validation($data, $files): array { + $errors = []; + + if (trim($data['name']) === '') { + $errors['name'] = get_string('required'); + } + + return $errors; + } } diff --git a/reportbuilder/classes/local/helpers/report.php b/reportbuilder/classes/local/helpers/report.php index ac2e5fc8b1d..e0011f37455 100644 --- a/reportbuilder/classes/local/helpers/report.php +++ b/reportbuilder/classes/local/helpers/report.php @@ -45,10 +45,8 @@ class report { * @return report_model */ public static function create_report(stdClass $data, bool $default = true): report_model { - // TODO move this properties_definition validation into the persistents, or resolve MDL-71086. - $data = (object) array_merge(array_intersect_key((array) $data, report_model::properties_definition()), [ - 'type' => datasource::TYPE_CUSTOM_REPORT, - ]); + $data->name = trim($data->name); + $data->type = datasource::TYPE_CUSTOM_REPORT; $reportpersistent = manager::create_report_persistent($data); @@ -77,7 +75,7 @@ class report { throw new invalid_parameter_exception('Invalid report'); } - $report->set('name', $data->name) + $report->set('name', trim($data->name)) ->update(); return $report; diff --git a/reportbuilder/classes/output/report_name_editable.php b/reportbuilder/classes/output/report_name_editable.php index 93f6889c839..882865df7d2 100644 --- a/reportbuilder/classes/output/report_name_editable.php +++ b/reportbuilder/classes/output/report_name_editable.php @@ -70,7 +70,7 @@ class report_name_editable extends inplace_editable { core_external::validate_context($report->get_context()); permission::require_can_edit_report($report); - $value = clean_param($value, PARAM_TEXT); + $value = trim(clean_param($value, PARAM_TEXT)); if ($value !== '') { $report ->set('name', $value) diff --git a/reportbuilder/tests/behat/customreports.feature b/reportbuilder/tests/behat/customreports.feature index 35495e4ac51..bab58efcd15 100644 --- a/reportbuilder/tests/behat/customreports.feature +++ b/reportbuilder/tests/behat/customreports.feature @@ -41,9 +41,13 @@ Feature: Manage custom reports When I navigate to "Reports > Report builder > Custom reports" in site administration And I click on "New report" "button" And I set the following fields in the "New report" "dialogue" to these values: - | Name | My report | | Report source | Users | | Include default setup | 0 | + # Try to set the report name to some blank spaces. + And I set the field "Name" in the "New report" "dialogue" to " " + And I click on "Save" "button" in the "New report" "dialogue" + And I should see "Required" + And I set the field "Name" in the "New report" "dialogue" to "My report" And I click on "Save" "button" in the "New report" "dialogue" Then I should see "My report" And I should see "Nothing to display" @@ -58,6 +62,8 @@ Feature: Manage custom reports | 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 + # Try to set the report name to some blank spaces. It should simply ignore the change. + And I set the field "Edit report name" in the "My report" "table_row" to " " And I set the field "Edit report name" in the "My report" "table_row" to "My renamed report" And I reload the page Then the following should exist in the "reportbuilder-table" table: