-
Improvement
-
Resolution: Won't Do
-
Minor
-
None
-
3.0.10, 3.3, Future Dev
-
MOODLE_30_STABLE, MOODLE_33_STABLE
-
Easy
-
The block override method instance_config_save has no way to indicate a failure, and thus halt the page redirect back to the course view page.
In the case where the instance configs have been extended via edit_form, and the developer needs to either validate, or do something a little more complex than just save the data (e.g. make a webservice call), there's no way to halt the flow of redirecting to the course view page when there is an error of some sort.
The block developer page even indicates that the instance_config_save should return true or false. Why? Nobody's listening.
By making four trivial changes to the block_manager->process_url_edit routine, an overriden instance_config_save routine can signal an error, which will allow the user to get meaningful feedback.
Those four trivial changes to blocklib.php are (master branch references):
1) assign a return value from the call to $block->instance_config_save on line 1749, e.g.
$configsaveresult = $block->instance_config_save($config);
2) Make the redirect (line 1777) to the course page conditional upon that assigned result, e.g.
if ($configsaveresult === false) {
$configsaveresult = get_string('unknownerror');
} else if ($configsaveresult === true || empty($configsaveresult)) {
redirect($this->page->url);
} else if(!is_string($configsaveresult)) {
$configsaveresult = get_string('unknownerror');
}
3) Make the last conditional branch (lines 1780 to 1798) of that routine (process_url_edit) unconditional, that is, take it out of the else block. Doing so would have no effect on any existing code, since that's the last branch of an 'else if', and the other two branches both have bailouts (in the form of redirects).
4) In that last code block immediately after the heading is output (currently line 1796), add a way to show the user what error, if any, occurred, e.g.
echo $output->heading($strheading, 2);
if (!empty($configsaveresult)) {
echo $output->notification($configsaveresult, \core\output\notification::NOTIFY_ERROR);
}
$mform->display();
I suppose you could also insert a warning or static form element into the $mform, but I like the way the error notification looks, and the user can click its close button if they want.
I'll fix up a git branch once this ticket is created.
Making those four changes would then allow a block's custom instance_config_save routine to indicate a failure, prevent the immediate redirect back to the course view page, and supply error and/or feedback information to the user