Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-60149

block instance_config_save() method can't indicate success or failure

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Development in progress
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 3.0.10, 3.3, Future Dev
    • Fix Version/s: None
    • Component/s: Blocks
    • Testing Instructions:
      Hide

      Return a string value from a block's instance_config_save override method. Edit an instance of that block and attempt to save. Should see a notification message on the edit form, rather than being redirected to the course view page.

      Show
      Return a string value from a block's instance_config_save override method. Edit an instance of that block and attempt to save. Should see a notification message on the edit form, rather than being redirected to the course view page.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_30_STABLE, MOODLE_33_STABLE
    • Pull Master Branch:

      Description

      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

        Attachments

          Activity

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated: