MDL-62708 question: Add idnumbers to question and question category

backup/moodle2/backup_stepslib.php

@@ -2211,15 +2211,15 @@ protected function define_structure() {
         $qcategory = new backup_nested_element('question_category', array('id'), array(
             'name', 'contextid', 'contextlevel', 'contextinstanceid',
             'info', 'infoformat', 'stamp', 'parent',
-            'sortorder'));
+            'sortorder', 'idnumber'));
 
         $questions = new backup_nested_element('questions');
 
         $question = new backup_nested_element('question', array('id'), array(
             'parent', 'name', 'questiontext', 'questiontextformat',
             'generalfeedback', 'generalfeedbackformat', 'defaultmark', 'penalty',
             'qtype', 'length', 'stamp', 'version',
-            'hidden', 'timecreated', 'timemodified', 'createdby', 'modifiedby'));
+            'hidden', 'timecreated', 'timemodified', 'createdby', 'modifiedby', 'idnumber'));
 
         // attach qtype plugin structure to $question element, only one allowed
         $this->add_plugin_structure('qtype', $question, false);

backup/moodle2/restore_stepslib.php

@@ -4434,6 +4434,12 @@ protected function process_question_category($data) {
                 $data->stamp = make_unique_id_code();
             }
 
+            // The idnumber if it exists also needs to be unique within a context or reset it to null.
+            if (!empty($data->idnumber) && $DB->record_exists('question_categories',
+                    ['idnumber' => $data->idnumber, 'contextid' => $data->contextid])) {
+                unset($data->idnumber);
+            }
+
             // Let's create the question_category and save mapping.
             $newitemid = $DB->insert_record('question_categories', $data);
             $this->set_mapping('question_category', $oldid, $newitemid);
@@ -4479,6 +4485,13 @@ protected function process_question($data) {
 
         // With newitemid = 0, let's create the question
         if (!$questionmapping->newitemid) {
+
+            // The idnumber if it exists also needs to be unique within a category or reset it to null.
+            if (!empty($data->idnumber) && $DB->record_exists('question',
+                    ['idnumber' => $data->idnumber, 'category' => $data->category])) {
+                unset($data->idnumber);
+            }
+
             $newitemid = $DB->insert_record('question', $data);
             $this->set_mapping('question', $oldid, $newitemid);
             // Also annotate them as question_created, we need

lang/en/question.php

@@ -170,6 +170,8 @@
 $string['changepublishstatuscat'] = '<a href="{$a->caturl}">Category "{$a->name}"</a> in course "{$a->coursename}" will have it\'s sharing status changed from <strong>{$a->changefrom} to {$a->changeto}</strong>.';
 $string['chooseqtypetoadd'] = 'Choose a question type to add';
 $string['editquestions'] = 'Edit questions';
+$string['idnumber'] = 'ID number';
+$string['idnumber_help'] = 'If used, the ID number must be unique within each question category. It provides another way of identifying a question which is sometimes useful, but can usually be left blank.';
 $string['ignorebroken'] = 'Ignore broken links';
 $string['impossiblechar'] = 'Impossible character {$a} detected as parenthesis character';
 $string['import'] = 'Import';

lib/db/install.xml

@@ -1333,6 +1333,7 @@
         <FIELD NAME="stamp" TYPE="char" LENGTH="255" NOTNULL="true" SEQUENCE="false"/>
         <FIELD NAME="parent" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
         <FIELD NAME="sortorder" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="999" SEQUENCE="false"/>
+        <FIELD NAME="idnumber" TYPE="char" LENGTH="100" NOTNULL="false" SEQUENCE="false"/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>
@@ -1341,6 +1342,7 @@
       <INDEXES>
         <INDEX NAME="contextid" UNIQUE="false" FIELDS="contextid" COMMENT="links to context table"/>
         <INDEX NAME="contextidstamp" UNIQUE="true" FIELDS="contextid, stamp"/>
+        <INDEX NAME="contextididnumber" UNIQUE="true" FIELDS="contextid, idnumber"/>
       </INDEXES>
     </TABLE>
     <TABLE NAME="question" COMMENT="The questions themselves">
@@ -1364,6 +1366,7 @@
         <FIELD NAME="timemodified" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="time that question was last modified"/>
         <FIELD NAME="createdby" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="userid of person who created this question"/>
         <FIELD NAME="modifiedby" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="userid of person who last edited this question"/>
+        <FIELD NAME="idnumber" TYPE="char" LENGTH="100" NOTNULL="false" SEQUENCE="false"/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>
@@ -1374,6 +1377,7 @@
       </KEYS>
       <INDEXES>
         <INDEX NAME="qtype" UNIQUE="false" FIELDS="qtype"/>
+        <INDEX NAME="categoryidnumber" UNIQUE="true" FIELDS="category, idnumber"/>
       </INDEXES>
     </TABLE>
     <TABLE NAME="question_answers" COMMENT="Answers, with a fractional grade (0-1) and feedback">
@@ -3861,4 +3865,4 @@
       </INDEXES>
     </TABLE>
   </TABLES>
-</XMLDB>
+</XMLDB>

lib/db/upgrade.php

@@ -2360,5 +2360,44 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2018091700.01);
     }
 
+    // Add idnumber fields to question and question_category tables.
+    // This is done in four parts to aid error recovery during upgrade, should that occur.
+    if ($oldversion < 2018092100.01) {
+        $table = new xmldb_table('question');
+        $field = new xmldb_field('idnumber', XMLDB_TYPE_CHAR, '100', null, null, null, null, 'modifiedby');
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+        upgrade_main_savepoint(true, 2018092100.01);
+    }
+
+    if ($oldversion < 2018092100.02) {
+        $table = new xmldb_table('question');
+        $index = new xmldb_index('categoryidnumber', XMLDB_INDEX_UNIQUE, array('category, idnumber'));
+        if (!$dbman->index_exists($table, $index)) {
+            $dbman->add_index($table, $index);
+        }
+        upgrade_main_savepoint(true, 2018092100.02);
+    }
+
+    if ($oldversion < 2018092100.03) {
+        $table = new xmldb_table('question_categories');
+        $field = new xmldb_field('idnumber', XMLDB_TYPE_CHAR, '100', null, null, null, null, 'sortorder');
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+        upgrade_main_savepoint(true, 2018092100.03);
+    }
+
+    if ($oldversion < 2018092100.04) {
+        $table = new xmldb_table('question_categories');
+        $index = new xmldb_index('contextididnumber', XMLDB_INDEX_UNIQUE, array('contextid, idnumber'));
+        if (!$dbman->index_exists($table, $index)) {
+            $dbman->add_index($table, $index);
+        }
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2018092100.04);
+    }
+
     return true;
 }

lib/questionlib.php

@@ -673,7 +673,7 @@ function question_move_questions_to_category($questionids, $newcategoryid) {
             array('id' => $newcategoryid));
     list($questionidcondition, $params) = $DB->get_in_or_equal($questionids);
     $questions = $DB->get_records_sql("
-            SELECT q.id, q.qtype, qc.contextid
+            SELECT q.id, q.qtype, qc.contextid, q.idnumber
               FROM {question} q
               JOIN {question_categories} qc ON q.category = qc.id
              WHERE  q.id $questionidcondition", $params);
@@ -682,6 +682,27 @@ function question_move_questions_to_category($questionids, $newcategoryid) {
             question_bank::get_qtype($question->qtype)->move_files(
                     $question->id, $question->contextid, $newcontextid);
         }
+        // Check whether there could be a clash of idnumbers in the new category.
+        if (((string) $question->idnumber !== '') &&
+                $DB->record_exists('question', ['idnumber' => $question->idnumber, 'category' => $newcategoryid])) {
+            $rec = $DB->get_records_select('question', "category = ? AND idnumber LIKE ?",
+                    [$newcategoryid, $question->idnumber . '_%'], 'idnumber DESC', 'id, idnumber', 0, 1);
+            $unique = 1;
+            if (count($rec)) {
+                $rec = reset($rec);
+                $idnumber = $rec->idnumber;
+                if (strpos($idnumber, '_') !== false) {
+                    $unique = substr($idnumber, strpos($idnumber, '_') + 1) + 1;
+                }
+            }
+            // For the move process, add a numerical increment to the idnumber. This means that if a question is
+            // mistakenly moved then the idnumber will not be completely lost.
+            $q = new stdClass();
+            $q->id = $question->id;
+            $q->category = $newcategoryid;
+            $q->idnumber = $question->idnumber . '_' . $unique;
+            $DB->update_record('question', $q);
+        }
     }
 
     // Move the questions themselves.

question/category.php

@@ -118,10 +118,10 @@
     $catformdata->info       = $catformdata->info['text'];
     if (!$catformdata->id) {//new category
         $qcobject->add_category($catformdata->parent, $catformdata->name,
-                $catformdata->info, false, $catformdata->infoformat);
+                $catformdata->info, false, $catformdata->infoformat, $catformdata->idnumber);
     } else {
         $qcobject->update_category($catformdata->id, $catformdata->parent,
-                $catformdata->name, $catformdata->info, $catformdata->infoformat);
+                $catformdata->name, $catformdata->info, $catformdata->infoformat, $catformdata->idnumber);
     }
     redirect($thispageurl);
 } else if ((!empty($param->delete) and (!$questionstomove) and confirm_sesskey())) {

question/category_class.php

@@ -404,7 +404,8 @@ public function move_questions($oldcat, $newcat){
     /**
      * Creates a new category with given params
      */
-    public function add_category($newparent, $newcategory, $newinfo, $return = false, $newinfoformat = FORMAT_HTML) {
+    public function add_category($newparent, $newcategory, $newinfo, $return = false, $newinfoformat = FORMAT_HTML,
+            $idnumber = null) {
         global $DB;
         if (empty($newcategory)) {
             print_error('categorynamecantbeblank', 'question');
@@ -419,6 +420,14 @@ public function add_category($newparent, $newcategory, $newinfo, $return = false
             }
         }
 
+        if (((string) $idnumber !== '') && !empty($contextid)) {
+            // While this check already exists in the form validation, this is a backstop preventing unnecessary errors.
+            if ($DB->record_exists('question_categories',
+                    ['idnumber' => $idnumber, 'contextid' => $contextid])) {
+                $idnumber = null;
+            }
+        }
+
         $cat = new stdClass();
         $cat->parent = $parentid;
         $cat->contextid = $contextid;
@@ -427,6 +436,9 @@ public function add_category($newparent, $newcategory, $newinfo, $return = false
         $cat->infoformat = $newinfoformat;
         $cat->sortorder = 999;
         $cat->stamp = make_unique_id_code();
+        if ($idnumber) {
+            $cat->idnumber = $idnumber;
+        }
         $categoryid = $DB->insert_record("question_categories", $cat);
 
         // Log the creation of this category.
@@ -447,7 +459,8 @@ public function add_category($newparent, $newcategory, $newinfo, $return = false
     /**
      * Updates an existing category with given params
      */
-    public function update_category($updateid, $newparent, $newname, $newinfo, $newinfoformat = FORMAT_HTML) {
+    public function update_category($updateid, $newparent, $newname, $newinfo, $newinfoformat = FORMAT_HTML,
+            $idnumber = null) {
         global $CFG, $DB;
         if (empty($newname)) {
             print_error('categorynamecantbeblank', 'question');
@@ -480,6 +493,14 @@ public function update_category($updateid, $newparent, $newname, $newinfo, $newi
             }
         }
 
+        if (((string) $idnumber !== '') && !empty($tocontextid)) {
+            // While this check already exists in the form validation, this is a backstop preventing unnecessary errors.
+            if ($DB->record_exists('question_categories',
+                    ['idnumber' => $idnumber, 'contextid' => $tocontextid])) {
+                $idnumber = null;
+            }
+        }
+
         // Update the category record.
         $cat = new stdClass();
         $cat->id = $updateid;
@@ -488,6 +509,9 @@ public function update_category($updateid, $newparent, $newname, $newinfo, $newi
         $cat->infoformat = $newinfoformat;
         $cat->parent = $parentid;
         $cat->contextid = $tocontextid;
+        if ($idnumber) {
+            $cat->idnumber = $idnumber;
+        }
         if ($newstamprequired) {
             $cat->stamp = make_unique_id_code();
         }

question/category_form.php

@@ -63,6 +63,10 @@ protected function definition() {
         $mform->setDefault('info', '');
         $mform->setType('info', PARAM_RAW);
 
+        $mform->addElement('text', 'idnumber', get_string('idnumber', 'question'), 'maxlength="100"  size="10"');
+        $mform->addHelpButton('idnumber', 'idnumber', 'question');
+        $mform->setType('idnumber', PARAM_RAW);
+
         $this->add_action_buttons(false, get_string('addcategory', 'question'));
 
         $mform->addElement('hidden', 'id', 0);
@@ -81,4 +85,33 @@ public function set_data($current) {
         }
         parent::set_data($current);
     }
+
+    /**
+     * Validation.
+     *
+     * @param array $data
+     * @param array $files
+     * @return array the errors that were found
+     */
+    public function validation($data, $files) {
+        global $DB;
+
+        $errors = parent::validation($data, $files);
+
+        // Add field validation check for duplicate idnumber.
+        list($parentid, $contextid) = explode(',', $data['parent']);
+        if (((string) $data['idnumber'] !== '') && !empty($contextid)) {
+            $conditions = 'contextid = ? AND idnumber = ?';
+            $params = [$contextid, $data['idnumber']];
+            if (!empty($data['id'])) {
+                $conditions .= ' AND id <> ?';
+                $params[] = $data['id'];
+            }
+            if ($DB->record_exists_select('question_categories', $conditions, $params)) {
+                $errors['idnumber'] = get_string('idnumbertaken', 'error');
+            }
+        }
+
+        return $errors;
+    }
 }

question/engine/tests/helpers.php

@@ -170,6 +170,7 @@ public static function initialise_a_question($q) {
 
         $q->id = 0;
         $q->category = 0;
+        $q->idnumber = null;
         $q->parent = 0;
         $q->questiontextformat = FORMAT_HTML;
         $q->generalfeedbackformat = FORMAT_HTML;
@@ -190,6 +191,7 @@ public static function initialise_question_data($qdata) {
 
         $qdata->id = 0;
         $qdata->category = 0;
+        $qdata->idnumber = null;
         $qdata->contextid = 0;
         $qdata->parent = 0;
         $qdata->questiontextformat = FORMAT_HTML;

question/format.php

@@ -406,6 +406,13 @@ public function importprocess() {
             $question->timecreated = time();
             $question->modifiedby = $USER->id;
             $question->timemodified = time();
+            if (isset($question->idnumber) && (string) $question->idnumber !== '') {
+                if ($DB->record_exists('question', ['idnumber' => $question->idnumber, 'category' => $question->category])) {
+                    // We cannot have duplicate idnumbers in a category.
+                    unset($question->idnumber);
+                }
+            }
+
             $fileoptions = array(
                     'subdirs' => true,
                     'maxfiles' => -1,

question/format/xml/format.php

@@ -236,6 +236,8 @@ public function import_headers($question) {
             $qo->questiontext .= ' <img src="@@PLUGINFILE@@/' . $filename . '" />';
         }
 
+        $qo->idnumber = $this->getpath($question, ['#', 'idnumber', 0, '#'], null);
+
         // Restore files in generalfeedback.
         $generalfeedback = $this->import_text_with_files($question,
                 array('#', 'generalfeedback', 0), $qo->generalfeedback, $this->get_format($qo->questiontextformat));
@@ -1217,6 +1219,7 @@ public function writequestion($question) {
         }
         $expout .= "    <penalty>{$question->penalty}</penalty>\n";
         $expout .= "    <hidden>{$question->hidden}</hidden>\n";
+        $expout .= "    <idnumber>{$question->idnumber}</idnumber>\n";
 
         // The rest of the output depends on question type.
         switch($question->qtype) {