Skip to content

Commit

Permalink
MDL-72978 reportbuilder: Report name can't have only blank spaces
Browse files Browse the repository at this point in the history
- Don't allow to set a report name containing only blank spaces when
creating a new report or editing an existing report name.
- Removed TODO from code related to MDL-71086
  • Loading branch information
dravek committed Nov 2, 2021
1 parent 8af7bec commit 741f47e
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 7 deletions.
17 changes: 17 additions & 0 deletions reportbuilder/classes/form/report.php
Expand Up @@ -147,4 +147,21 @@ public function set_data_for_dynamic_submission(): void {
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;
}
}
8 changes: 3 additions & 5 deletions reportbuilder/classes/local/helpers/report.php
Expand Up @@ -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);

Expand Down Expand Up @@ -77,7 +75,7 @@ public static function update_report(stdClass $data): report_model {
throw new invalid_parameter_exception('Invalid report');
}

$report->set('name', $data->name)
$report->set('name', trim($data->name))
->update();

return $report;
Expand Down
2 changes: 1 addition & 1 deletion reportbuilder/classes/output/report_name_editable.php
Expand Up @@ -70,7 +70,7 @@ public static function update(int $reportid, string $value): self {
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)
Expand Down
8 changes: 7 additions & 1 deletion reportbuilder/tests/behat/customreports.feature
Expand Up @@ -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"
Expand All @@ -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:
Expand Down

0 comments on commit 741f47e

Please sign in to comment.