Merge pull request #1458 from maths/broken-save

WIP - Allow saving of a broken question

Author: sangwinc

adminui/dependencies.php

@@ -98,6 +98,9 @@
 echo $OUTPUT->single_button(
     new moodle_url($PAGE->url, ['todo' => 1, 'sesskey' => sesskey()]),
     'Find "todo"');
+echo $OUTPUT->single_button(
+    new moodle_url($PAGE->url, ['broken' => 1, 'sesskey' => sesskey()]),
+    'Find "broken"');
 
 if (data_submitted() && optional_param('includes', false, PARAM_BOOL)) {
     /*
@@ -329,4 +332,37 @@
     echo '</tbody></table>';
 }
 
+if (data_submitted() && optional_param('broken', false, PARAM_BOOL)) {
+    /*
+     * Question marked as broken.
+     */
+    $qs = $DB->get_recordset_sql('SELECT q.id as questionid, q.name as name
+                                  FROM {question} q
+                                  JOIN {question_versions} qv ON qv.questionid = q.id
+                                  JOIN {question_bank_entries} qbe ON qbe.id = qv.questionbankentryid
+                                  JOIN {qtype_stack_options} o ON q.id = o.questionid
+                                  WHERE o.isbroken = 1
+                                    AND qv.version = (
+                                        SELECT MAX(version)
+                                        FROM {question_versions} v
+                                        WHERE v.questionbankentryid = qbe.id
+                                    )');
+    echo '<h4>Questions with most recent version marked as broken</h4>';
+    echo '<table><thead><tr><th>Question</th><th>Version</th></thead><tbody>';
+    // Load the whole question, simpler to get the contexts correct that way.
+    foreach ($qs as $item) {
+        $q = question_bank::load_question($item->questionid);
+        list($context, $seed, $urlparams) = qtype_stack_setup_question_test_page($q);
+        $editurl = new \moodle_url('/question/bank/editquestion/question.php', [
+            'id' => $urlparams['questionid'],
+            'returnURL' => new moodle_url('/question/type/stack/adminui/dependencies.php'),
+            'courseid' => $urlparams['courseid'],
+        ]);
+        echo '<tr><td>' . $item->name . ' ' .
+            $OUTPUT->action_icon($editurl, new pix_icon('t/edit', get_string('edit'))) .
+            '</td><td>' . $q->version . '</td></tr>';
+    }
+    echo '</tbody></table>';
+}
+
 echo $OUTPUT->footer();

api/util/StackQuestionLoader.php

@@ -121,7 +121,7 @@ public static function loadxml($xml, $includetests=false) {
         $question->variantsselectionseed     =
             (string) $xmldata->question->variantsselectionseed ? (string) $xmldata->question->variantsselectionseed : '';
         $question->compiledcache             = [];
-
+        $question->isbroken = (array) $xmldata->question->isbroken ? self::parseboolean($xmldata->question->isbroken) : 0;
         $question->options = new \stack_options();
         $question->options->set_option(
             'multiplicationsign',

backup/moodle2/backup_qtype_stack_plugin.class.php

@@ -56,7 +56,7 @@ protected function define_question_plugin_structure() {
                     'decimals',
                     'scientificnotation',
                     'multiplicationsign', 'sqrtsign',
-                    'complexno', 'inversetrig', 'logicsymbol', 'matrixparens', 'variantsselectionseed',
+                    'complexno', 'inversetrig', 'logicsymbol', 'matrixparens', 'variantsselectionseed', 'isbroken',
                 ]);
 
         $stackinputs = new backup_nested_element('stackinputs');

backup/moodle2/restore_qtype_stack_plugin.class.php

@@ -86,6 +86,9 @@ public static function convert_backup_to_questiondata(array $backupdata): stdCla
         if (!property_exists($questiondata->options, 'stackversion')) {
             $questiondata->options->stackversion = '';
         }
+        if (!property_exists($questiondata->options, 'isbroken')) {
+            $questiondata->options->isbroken = 0;
+        }
 
         if (!property_exists($questiondata->options, 'inversetrig')) {
             $questiondata->options->inversetrig = 'cos-1';
@@ -202,6 +205,10 @@ public function process_qtype_stack_options($data) {
             $data->stackversion = '';
         }
 
+        if (!property_exists($data, 'isbroken')) {
+            $data->isbroken = 0;
+        }
+
         if (!property_exists($data, 'inversetrig')) {
             $data->inversetrig = 'cos-1';
         }

db/install.xml

@@ -9,6 +9,7 @@
         <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
         <FIELD NAME="questionid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Foreign key link to question.id"/>
         <FIELD NAME="stackversion" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="The version of the STACK plugin on which this question was authored."/>
+        <FIELD NAME="isbroken" TYPE="int" LENGTH="4" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Has this question been saved in a broken state."/>
         <FIELD NAME="questionvariables" TYPE="text" NOTNULL="true" SEQUENCE="false" COMMENT="Maxima code that is run when the question is started to initialise variables."/>
         <FIELD NAME="specificfeedback" TYPE="text" NOTNULL="true" SEQUENCE="false" COMMENT="CAS text for the specific feedback for the question."/>
         <FIELD NAME="specificfeedbackformat" TYPE="int" LENGTH="2" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="The FORMAT_... for the specific feedback"/>

db/upgrade.php

@@ -973,6 +973,17 @@ function xmldb_qtype_stack_upgrade($oldversion) {
         // STACK savepoint reached.
         upgrade_plugin_savepoint(true, 2024043000, 'qtype', 'stack');
     }
+
+    if ($oldversion < 2025042500) {
+        $table = new xmldb_table('qtype_stack_options');
+        $field = new xmldb_field('isbroken', XMLDB_TYPE_INTEGER, '4', null, XMLDB_NOTNULL, null, '0', 'stackversion');
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        // STACK savepoint reached.
+        upgrade_plugin_savepoint(true, 2025042500, 'qtype', 'stack');
+    }
     // Add new upgrade blocks just above here.
 
     // Check the version of the Maxima library code that comes with this version

edit_stack_form.php

@@ -118,6 +118,15 @@ public function set_data($question) {
         }
 
         parent::set_data($question);
+        // If the question is broken we run validation when the form is loaded to display errors.
+        // We have to temporarily remove the broken flag from the form to stop the validation
+        // being by-passed.
+        if (!empty($this->question->options->isbroken)) {
+            $mform = $this->_form;
+            $mform->setDefault('isbroken', 0);
+            $this->is_validated();
+            $mform->setDefault('isbroken', 1);
+        }
     }
 
     /**
@@ -182,6 +191,11 @@ protected function definition() {
                 stack_string('fixdollars'), stack_string('fixdollarslabel'));
         $mform->insertElementBefore($fixdollars, 'buttonar');
         $mform->addHelpButton('fixdollars', 'fixdollars', 'qtype_stack');
+        $isbroken = $mform->createElement('checkbox', 'isbroken',
+                stack_string('isbroken'), stack_string('isbrokenlabel'));
+        $mform->setDefault('isbroken', (!empty($this->question->options->isbroken) ? 1 : 0));
+        $mform->insertElementBefore($isbroken, 'buttonar');
+        $mform->addHelpButton('isbroken', 'isbroken', 'qtype_stack');
         $mform->closeHeaderBefore('fixdollars');
 
         // There is no un-closeHeaderBefore, so fake it.
@@ -511,10 +525,13 @@ protected function definition_input($inputname, MoodleQuickForm $mform, $counts)
         // Set a default for the new question.
         if ($inputname === self::DEFAULT_INPUT) {
             $mform->setDefault($inputname . 'modelans', self::DEFAULT_TEACHER_ANSWER);
+        } else {
+            // Default for all parts of an input required in order to save a broken question.
+            $mform->setDefault($inputname . 'modelans', '');
         }
 
         $mform->addElement('text', $inputname . 'boxsize', stack_string('boxsize'), ['size' => 3]);
-        $mform->setDefault($inputname . 'boxsize', $this->stackconfig->inputboxsize);
+        $mform->setDefault($inputname . 'boxsize', (int) $this->stackconfig->inputboxsize);
         $mform->setType($inputname . 'boxsize', PARAM_INT);
         $mform->addHelpButton($inputname . 'boxsize', 'boxsize', 'qtype_stack');
         $mform->hideIf($inputname . 'boxsize', $inputname . 'type', 'in',
@@ -589,6 +606,7 @@ protected function definition_input($inputname, MoodleQuickForm $mform, $counts)
         $mform->hideIf($inputname . 'showvalidation', $inputname . 'type', 'in', []);
 
         $mform->addElement('text', $inputname . 'options', stack_string('inputextraoptions'), ['size' => 30]);
+        $mform->setDefault($inputname . 'options', '');
         $mform->setType($inputname . 'options', PARAM_RAW);
         $mform->addHelpButton($inputname . 'options', 'inputextraoptions', 'qtype_stack');
     }
@@ -819,6 +837,7 @@ protected function data_preprocessing_options($question) {
         $question->questionsimplify      = $opt->questionsimplify;
         $question->assumepositive        = $opt->assumepositive;
         $question->assumereal            = $opt->assumereal;
+        $question->isbroken              = $opt->isbroken;
 
         return $question;
     }
@@ -836,7 +855,8 @@ protected function data_preprocessing_inputs($question) {
         foreach ($question->inputs as $inputname => $input) {
             $question->{$inputname . 'type'}               = $input->type;
             $question->{$inputname . 'modelans'}           = $input->tans;
-            $question->{$inputname . 'boxsize'}            = $input->boxsize;
+            // Cast to int required to avoid erroneous validation messages on loading a broken question.
+            $question->{$inputname . 'boxsize'}            = (int) $input->boxsize;
             // TO-DO: remove this when we delete it from the DB.
             $question->{$inputname . 'strictsyntax'}       = true;
             $question->{$inputname . 'insertstars'}        = $input->insertstars;
@@ -973,9 +993,42 @@ protected function prepare_text_field($field, $text, $format, $itemid, $filearea
     // phpcs:ignore moodle.Commenting.MissingDocblock.Function
     public function validation($fromform, $files) {
         $errors = parent::validation($fromform, $files);
-
         $qtype = new qtype_stack();
-        return $qtype->validate_fromform($fromform, $errors);
+        $allerrors = $qtype->validate_fromform($fromform, $errors, $this->question);
+        $mustconfirm = (array_search(stack_string('youmustconfirm'), $allerrors) === false) ? false : true;
+        // Add not saved warning but only when attempting to save, not on loading question.
+        // Moodle errors and confirmation issues will always be on save as question cannot be saved and thus
+        // cannot be loaded in this state even if marked as broken. If there are STACK issues, we only flag
+        // if question is not marked as broken and it's a submit.
+        if ($errors || $mustconfirm ||
+                ($allerrors && empty($fromform['isbroken']) && !empty(optional_param_array('questiontext', [], PARAM_RAW)))) {
+            $errortext = stack_string('notsaved') . '<br>';
+            if ($errors) {
+                $errortext .= stack_string_error('moodleerrors') . '<br>';
+            }
+            if ($mustconfirm) {
+                $errortext .= stack_string_error('mustconfirm') . '<br>';
+            }
+            if (empty($fromform['isbroken']) && $allerrors != $errors) {
+                $errortext .= stack_string_error('stackerrors') . '<br>';
+            }
+
+            $allerrors['versioninfo'] = isset($allerrors['versioninfo']) ?
+                $errortext . ' ' . $allerrors['versioninfo'] : $errortext;
+        }
+        // Ignore STACK-specific validation if question is marked as broken unless
+        // a confirmation is required.
+        // Moodle validation errors always returned.
+        if (empty($fromform['isbroken'])) {
+            return $allerrors;
+        } else {
+            if ($errors || $mustconfirm) {
+                return $allerrors;
+            } else {
+                // This is going to be [].
+                return $errors;
+            }
+        }
     }
 
     // phpcs:ignore moodle.Commenting.MissingDocblock.Function

lang/en/qtype_stack.php

@@ -39,6 +39,7 @@
 $string['exceptionmessage']  = '{$a}';
 $string['seekhelp']          = 'Please ask your teacher about this.';
 $string['runtimeerror']      = 'This question generated an unexpected internal error.';
+$string['questionbroken']    = 'The question has been marked as broken during editing.';
 $string['runtimefielderr']   = 'The field ""{$a->field}"" generated the following error: {$a->err}';
 $string['version']           = 'Version';
 
@@ -140,6 +141,9 @@
 $string['fixdollarslabel'] = 'Replace <code>$...$</code> with <code>\(...\)</code>, <code>$$...$$</code> with <code>\[...\]</code> and <code>@...@</code> with <code>{@...@}</code> on save.';
 $string['fixdollars_help'] = 'This option is useful if are copying and pasting (or typing) TeX with <code>$...$</code> and <code>$$...$$</code> delimiters. Those delimiters will be replaced by the recommended delimiters during the save process.';
 $string['forbiddendoubledollars'] = 'Please use the delimiters <code>\(...\)</code> for inline maths and <code>\[...\]</code> for display maths. <code>$...$</code> and <code>$$...$$</code> are not permitted. There is an option at the end of the form to fix this automatically.';
+$string['isbroken'] = 'Save as broken';
+$string['isbrokenlabel'] = 'Mark the question as broken. This will allow the question to be saved even with most errors but it will not be displayed to students.';
+$string['isbroken_help'] = 'This option allows you to save your work even if a question is incomplete or has errors. You or a colleague can then return to work on it later. You will still need to supply required fields and tick the boxes to confirm any deletion of inputs or PRTs before the question will save.';
 $string['forbidfloat'] = 'Forbid float';
 $string['forbidfloat_help'] = 'If set to yes, then any answer of the student which has a floating point number will be rejected as invalid.';
 $string['forbidfloat_link'] = '%%WWWROOT%%/question/type/stack/doc/doc.php/Authoring/Inputs.md#Forbid_Floats';
@@ -400,6 +404,10 @@
 $string['variantsselectionseed_help'] = 'Normally you can leave this box blank. If, however, you want two different questions in a quiz to use the same random seed, then type the same string in this box for the two questions (and deploy the same set of random seeds, if you are using deployed variants) and the random seeds for the two questions will be synchronised.';
 $string['verifyquestionandupdate'] = 'Verify the question text and update the form';
 $string['youmustconfirm'] = 'You must confirm here.';
+$string['notsaved'] = '** QUESTION WAS NOT SAVED **';
+$string['mustconfirm'] = 'You have changes to confirm.';
+$string['moodleerrors'] = 'You have errors related to Moodle\'s basic question setup.';
+$string['stackerrors'] = 'You have errors in your question.';
 
 // Strings used by input elements.
 $string['studentinputtoolong'] = 'Your input is longer than permitted by STACK.';

question.php

@@ -251,6 +251,11 @@ class qtype_stack_question extends question_graded_automatically_with_countback
      */
     public $pluginfiles = [];
 
+    /**
+     * @var bool is this question marked as broken.
+     */
+    public $isbroken = false;
+
     /**
      * Make sure the cache is valid for the current response. If not, clear it.
      *
@@ -359,9 +364,13 @@ public function start_attempt(question_attempt_step $step, $variant) {
      * Once we know the random seed, we can initialise all the other parts of the question.
      */
     public function initialise_question_from_seed() {
+        // If the question is marked as broken skip straight to error output.
         // We can detect a logically faulty question by checking if the cache can
         // return anything if it can't then we can simply skip to the output of errors.
-        if ($this->get_cached('units') !== null) {
+        if ($this->isbroken) {
+            // Question is marked as broken. Students should not see it.
+            $this->runtimeerrors[stack_string('questionbroken')] = true;
+        } else if ($this->get_cached('units') !== null) {
             // Build up the question session out of all the bits that need to go into it.
             // 1. question variables.
             $session = new stack_cas_session2([], $this->options, $this->seed);