mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-05-14 20:26:19 +02:00
Code Issues Projects Releases Wiki Activity

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

Browse Source
This commit is contained in:
John Beedell 2018-07-04 12:21:44 +01:00
parent 6902f39141
commit 6189fda47f
24 changed files with 502 additions and 13 deletions

4
backup/moodle2/backup_stepslib.php

@ -2211,7 +2211,7 @@ class backup_questions_structure_step extends backup_structure_step {
$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');
@ -2219,7 +2219,7 @@ class backup_questions_structure_step extends backup_structure_step {
'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);

13
backup/moodle2/restore_stepslib.php

@ -4434,6 +4434,12 @@ class restore_create_categories_and_questions extends restore_structure_step {
$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 @@ class restore_create_categories_and_questions extends restore_structure_step {
// 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

2
lang/en/question.php

@ -170,6 +170,8 @@ $string['getcontextfromfile'] = 'Get context from file';
$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';

6
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>

39
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;
}

23
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.

4
question/category.php

@ -118,10 +118,10 @@ if ($qcobject->catform->is_cancelled()) {
$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())) {

28
question/category_class.php

@ -404,7 +404,8 @@ class question_category_object {
/**
* 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 @@ class question_category_object {
}
}
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 @@ class question_category_object {
$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 @@ class question_category_object {
/**
* 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 @@ class question_category_object {
}
}
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 @@ class question_category_object {
$cat->infoformat = $newinfoformat;
$cat->parent = $parentid;
$cat->contextid = $tocontextid;
if ($idnumber) {
$cat->idnumber = $idnumber;
}
if ($newstamprequired) {
$cat->stamp = make_unique_id_code();
}

33
question/category_form.php

@ -63,6 +63,10 @@ class question_category_edit_form extends moodleform {
$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 @@ class question_category_edit_form extends moodleform {
}
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;
}
}

2
question/engine/tests/helpers.php

@ -170,6 +170,7 @@ class test_question_maker {
$q->id = 0;
$q->category = 0;
$q->idnumber = null;
$q->parent = 0;
$q->questiontextformat = FORMAT_HTML;
$q->generalfeedbackformat = FORMAT_HTML;
@ -190,6 +191,7 @@ class test_question_maker {
$qdata->id = 0;
$qdata->category = 0;
$qdata->idnumber = null;
$qdata->contextid = 0;
$qdata->parent = 0;
$qdata->questiontextformat = FORMAT_HTML;

7
question/format.php

@ -406,6 +406,13 @@ class qformat_default {
$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,

3
question/format/xml/format.php

@ -236,6 +236,8 @@ class qformat_xml extends qformat_default {
$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 @@ class qformat_xml extends qformat_default {
}
$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) {

130
question/format/xml/tests/xmlformat_test.php

@ -48,6 +48,7 @@ class qformat_xml_test extends question_testcase {
$q = new stdClass();
$q->id = 0;
$q->contextid = 0;
$q->idnumber = null;
$q->category = 0;
$q->parent = 0;
$q->questiontextformat = FORMAT_HTML;
@ -342,6 +343,7 @@ END;
$qdata->length = 0;
$qdata->penalty = 0;
$qdata->hidden = 0;
$qdata->idnumber = null;
$exporter = new qformat_xml();
$xml = $exporter->writequestion($qdata);
@ -360,6 +362,7 @@ END;
<defaultgrade>0</defaultgrade>
<penalty>0</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
</question>
';
@ -487,6 +490,7 @@ END;
$qdata->length = 1;
$qdata->penalty = 0;
$qdata->hidden = 0;
$qdata->idnumber = null;
$qdata->options = new stdClass();
$qdata->options->id = 456;
$qdata->options->questionid = 123;
@ -516,6 +520,7 @@ END;
<defaultgrade>1</defaultgrade>
<penalty>0</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
<responseformat>monospaced</responseformat>
<responserequired>0</responserequired>
<responsefieldlines>42</responsefieldlines>
@ -649,6 +654,7 @@ END;
$qdata->length = 1;
$qdata->penalty = 0.3333333;
$qdata->hidden = 0;
$qdata->idnumber = null;
$qdata->options = new stdClass();
$qdata->options->shuffleanswers = 1;
@ -709,6 +715,7 @@ END;
<defaultgrade>1</defaultgrade>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
<shuffleanswers>true</shuffleanswers>
<correctfeedback format="html">
<text>Well done.</text>
@ -880,6 +887,7 @@ END;
$qdata->length = 1;
$qdata->penalty = 0.3333333;
$qdata->hidden = 0;
$qdata->idnumber = null;
$qdata->options = new stdClass();
$qdata->options->single = 0;
@ -922,6 +930,7 @@ END;
<defaultgrade>2</defaultgrade>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
<single>false</single>
<shuffleanswers>false</shuffleanswers>
<answernumbering>abc</answernumbering>
@ -1053,6 +1062,7 @@ END;
$qdata->length = 1;
$qdata->penalty = 0.1;
$qdata->hidden = 0;
$qdata->idnumber = null;
$qdata->options = new stdClass();
$qdata->options->answers = array(
@ -1083,6 +1093,7 @@ END;
<defaultgrade>1</defaultgrade>
<penalty>0.1</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
<answer fraction="100" format="plain_text">
<text>42</text>
<feedback format="html">
@ -1183,6 +1194,7 @@ END;
$qdata->length = 1;
$qdata->penalty = 0.3333333;
$qdata->hidden = 0;
$qdata->idnumber = null;
$qdata->options = new stdClass();
$qdata->options->usecase = 0;
@ -1214,6 +1226,7 @@ END;
<defaultgrade>1</defaultgrade>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
<usecase>0</usecase>
<answer fraction="100" format="plain_text">
<text>Beta</text>
@ -1290,6 +1303,59 @@ END;
$this->assert(new question_check_specified_fields_expectation($expectedq), $q);
}
public function test_import_truefalse_with_idnumber() {
$xml = ' <question type="truefalse">
<name>
<text>True false question</text>
</name>
<questiontext format="html">
<text>The answer is true.</text>
</questiontext>
<generalfeedback>
<text>General feedback: You should have chosen true.</text>
</generalfeedback>
<defaultgrade>1</defaultgrade>
<penalty>1</penalty>
<hidden>0</hidden>
<idnumber>TestIdNum1</idnumber>
<answer fraction="100">
<text>true</text>
<feedback>
<text>Well done!</text>
</feedback>
</answer>
<answer fraction="0">
<text>false</text>
<feedback>
<text>Doh!</text>
</feedback>
</answer>
</question>';
$xmldata = xmlize($xml);
$importer = new qformat_xml();
$q = $importer->import_truefalse($xmldata['question']);
$expectedq = new stdClass();
$expectedq->qtype = 'truefalse';
$expectedq->name = 'True false question';
$expectedq->questiontext = 'The answer is true.';
$expectedq->questiontextformat = FORMAT_HTML;
$expectedq->generalfeedback = 'General feedback: You should have chosen true.';
$expectedq->defaultmark = 1;
$expectedq->length = 1;
$expectedq->penalty = 1;
$expectedq->idnumber = 'TestIdNum1';
$expectedq->feedbacktrue = array('text' => 'Well done!',
'format' => FORMAT_HTML);
$expectedq->feedbackfalse = array('text' => 'Doh!',
'format' => FORMAT_HTML);
$expectedq->correctanswer = true;
$this->assert(new question_check_specified_fields_expectation($expectedq), $q);
}
public function test_export_truefalse() {
$qdata = new stdClass();
$qdata->id = 12;
@ -1304,6 +1370,7 @@ END;
$qdata->length = 1;
$qdata->penalty = 1;
$qdata->hidden = 0;
$qdata->idnumber = null;
$qdata->options = new stdClass();
$qdata->options->answers = array(
@ -1330,6 +1397,67 @@ END;
<defaultgrade>1</defaultgrade>
<penalty>1</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
<answer fraction="100" format="plain_text">
<text>true</text>
<feedback format="html">
<text>Well done!</text>
</feedback>
</answer>
<answer fraction="0" format="plain_text">
<text>false</text>
<feedback format="html">
<text>Doh!</text>
</feedback>
</answer>
</question>
';
$this->assert_same_xml($expectedxml, $xml);
}
public function test_export_truefalse_with_idnumber() {
$qdata = new stdClass();
$qdata->id = 12;
$qdata->contextid = \context_system::instance()->id;
$qdata->qtype = 'truefalse';
$qdata->name = 'True false question';
$qdata->questiontext = 'The answer is true.';
$qdata->questiontextformat = FORMAT_HTML;
$qdata->generalfeedback = 'General feedback: You should have chosen true.';
$qdata->generalfeedbackformat = FORMAT_HTML;
$qdata->defaultmark = 1;
$qdata->length = 1;
$qdata->penalty = 1;
$qdata->hidden = 0;
$qdata->idnumber = 'TestIDNum2';
$qdata->options = new stdClass();
$qdata->options->answers = array(
1 => new question_answer(1, 'True', 1, 'Well done!', FORMAT_HTML),
2 => new question_answer(2, 'False', 0, 'Doh!', FORMAT_HTML),
);
$qdata->options->trueanswer = 1;
$qdata->options->falseanswer = 2;
$exporter = new qformat_xml();
$xml = $exporter->writequestion($qdata);
$expectedxml = '<!-- question: 12 -->
<question type="truefalse">
<name>
<text>True false question</text>
</name>
<questiontext format="html">
<text>The answer is true.</text>
</questiontext>
<generalfeedback format="html">
<text>General feedback: You should have chosen true.</text>
</generalfeedback>
<defaultgrade>1</defaultgrade>
<penalty>1</penalty>
<hidden>0</hidden>
<idnumber>TestIDNum2</idnumber>
<answer fraction="100" format="plain_text">
<text>true</text>
<feedback format="html">
@ -1474,6 +1602,7 @@ END;
</generalfeedback>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
<hint format="html">
<text>Hint 1</text>
</hint>
@ -1505,6 +1634,7 @@ END;
</generalfeedback>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
</question>
';

13
question/tests/backup_test.php

@ -115,8 +115,8 @@ class core_question_backup_testcase extends advanced_testcase {
$qgen = $this->getDataGenerator()->get_plugin_generator('core_question');
$context = context_coursecat::instance($category1->id);
$qcat = $qgen->create_question_category(array('contextid' => $context->id));
$question1 = $qgen->create_question('shortanswer', null, array('category' => $qcat->id));
$question2 = $qgen->create_question('shortanswer', null, array('category' => $qcat->id));
$question1 = $qgen->create_question('shortanswer', null, array('category' => $qcat->id, 'idnumber' => 'q1'));
$question2 = $qgen->create_question('shortanswer', null, array('category' => $qcat->id, 'idnumber' => 'q2'));
// Tag the questions with 2 question tags and 2 course level question tags.
$qcontext = context::instance_by_id($qcat->contextid);
@ -144,10 +144,11 @@ class core_question_backup_testcase extends advanced_testcase {
// The questions should remain in the question category they were which is
// a question category belonging to a course category context.
$questions = $DB->get_records('question', array('category' => $qcat->id));
$questions = $DB->get_records('question', array('category' => $qcat->id), 'idnumber');
$this->assertCount(2, $questions);
// Retrieve tags for each question and check if they are assigned at the right context.
$qcount = 1;
foreach ($questions as $question) {
$tags = core_tag_tag::get_item_tags('core_question', 'question', $question->id);
@ -162,6 +163,10 @@ class core_question_backup_testcase extends advanced_testcase {
}
$this->assertEquals($expected, $tag->taginstancecontextid);
}
// Also check idnumbers have been backed up and restored.
$this->assertEquals('q' . $qcount, $question->idnumber);
$qcount++;
}
// Now, again, delete everything including the course category.
@ -203,4 +208,4 @@ class core_question_backup_testcase extends advanced_testcase {
}
}
}
}

122
question/tests/behat/question_categories_idnumber.feature Normal file

@ -0,0 +1,122 @@
@core @core_question
Feature: A teacher can put questions with idnumbers in categories with idnumbers in the question bank
In order to organize my questions
As a teacher
I create and edit categories and move questions between them (now with idnumbers)
Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
And the following "courses" exist:
| fullname | shortname | format |
| Course 1 | C1 | weeks |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
And I log in as "teacher1"
And I am on "Course 1" course homepage
Scenario: A new question category can only be created with a unique idnumber for a context
# Note need to create the top category each time.
When the following "question categories" exist:
| contextlevel | reference | questioncategory | name | idnumber |
| Course | C1 | Top | top | |
| Course | C1 | top | Used category | c1used |
And I navigate to "Question bank > Categories" in current page administration
And I set the following fields to these values:
| Name | Sub used category |
| Parent category | Used category |
| Category info | Created as a test |
| ID number | c1used |
And I press "Add category"
# Standard warning.
Then I should see "This ID number is already in use"
# Correction to a unique idnumber for the context.
And I set the field "ID number" to "c1unused"
And I press "Add category"
Then I should see "Sub used category (0)"
And I should see "Created as a test" in the "Sub used category" "list_item"
Scenario: A question category can be edited and saved without changing the idnumber
When the following "question categories" exist:
| contextlevel | reference | questioncategory | name | idnumber |
| Course | C1 | Top | top | |
| Course | C1 | top | Used category | c1used |
And I navigate to "Question bank > Categories" in current page administration
And I click on "Edit" "link" in the "Used category" "list_item"
And I press "Save changes"
Then I should not see "This ID number is already in use"
Scenario: A question can only have a unique idnumber within a category
When the following "question categories" exist:
| contextlevel | reference | questioncategory | name | idnumber |
| Course | C1 | Top | top | |
| Course | C1 | top | Used category | c1used |
And the following "questions" exist:
| questioncategory | qtype | name | questiontext | idnumber |
| Used category | essay | Test question 1 | Write about whatever you want | q1 |
| Used category | essay | Test question 2 | Write about whatever you want | q2 |
And I navigate to "Question bank > Questions" in current page administration
And I click on "Edit" "link" in the "Test question 2" "table_row"
And I set the field "ID number" to "q1"
And I press "submitbutton"
# This is the standard form warning reminding the user that the idnumber needs to be unique for a category.
Then I should see "This ID number is already in use"
Scenario: A question can be edited and saved without changing the idnumber
When the following "question categories" exist:
| contextlevel | reference | questioncategory | name | idnumber |
| Course | C1 | Top | top | |
| Course | C1 | top | Used category | c1used |
And the following "questions" exist:
| questioncategory | qtype | name | questiontext | idnumber |
| Used category | essay | Test question 1 | Write about whatever you want | q1 |
And I navigate to "Question bank > Questions" in current page administration
And I click on "Edit" "link" in the "Test question 1" "table_row"
And I press "Save changes"
Then I should not see "This ID number is already in use"
Scenario: Question idnumber conficts found when saving to a different category.
When the following "question categories" exist:
| contextlevel | reference | questioncategory | name |
| Course | C1 | Top | top |
| Course | C1 | top | Category 1 |
| Course | C1 | top | Category 2 |
And the following "questions" exist:
| questioncategory | qtype | name | questiontext | idnumber |
| Category 1 | essay | Question to edit | Write about whatever you want | q1 |
| Category 2 | essay | Other question | Write about whatever you want | q2 |
And I navigate to "Question bank > Questions" in current page administration
And I click on "Edit" "link" in the "Question to edit" "table_row"
And I set the following fields to these values:
| Use this category | 0 |
| ID number | q2 |
| Save in category | Category 2 |
And I press "Save changes"
Then I should see "This ID number is already in use"
@javascript
Scenario: Moving a question between categories can force a change to the idnumber
And the following "question categories" exist:
| contextlevel | reference | questioncategory | name | idnumber |
| Course | C1 | Top | top | |
| Course | C1 | top | Subcategory | c1sub |
| Course | C1 | top | Used category | c1used |
And the following "questions" exist:
| questioncategory | qtype | name | questiontext | idnumber |
| Used category | essay | Test question 1 | Write about whatever you want | q1 |
| Used category | essay | Test question 2 | Write about whatever you want | q2 |
| Subcategory | essay | Test question 3 | Write about whatever you want | q3 |
When I navigate to "Question bank > Questions" in current page administration
And I click on "Edit" "link" in the "Test question 3" "table_row"
# The q1 idnumber is allowed for this question while it is in the Subcategory.
And I set the field "ID number" to "q1"
And I press "submitbutton"
# Javascript is required for the next step.
And I click on "Test question 3" "checkbox" in the "Test question 3" "table_row"
And I set the field "Question category" to "Used category"
And I press "Move to >>"
And I click on "Edit" "link" in the "Test question 3" "table_row"
# The question just moved into this category needs to have a unique idnumber, so a number is appended.
Then the field "ID number" matches value "q1_1"

2
question/tests/generator/lib.php

@ -51,6 +51,7 @@ class core_question_generator extends component_generator_base {
'infoformat' => FORMAT_HTML,
'stamp' => make_unique_id_code(),
'sortorder' => 999,
'idnumber' => null
);
$record = $this->datagenerator->combine_defaults_and_record($defaults, $record);
@ -87,6 +88,7 @@ class core_question_generator extends component_generator_base {
$question->category = $fromform->category;
$question->qtype = $qtype;
$question->createdby = 0;
$question->idnumber = null;
return $this->update_question($question, $which, $overrides);
}

29
question/tests/generator_test.php

@ -51,4 +51,33 @@ class core_question_generator_testcase extends advanced_testcase {
$this->assertSame('My category', $cat->name);
$this->assertSame(1, $cat->sortorder);
}
public function test_idnumbers_in_categories_and_questions() {
$this->resetAfterTest();
$generator = $this->getDataGenerator()->get_plugin_generator('core_question');
list($category, $course, $qcat, $questions) = $generator->setup_course_and_questions();
// Check default idnumbers in question_category and questions.
$this->assertNull($qcat->idnumber);
$this->assertNull($questions[0]->idnumber);
$this->assertNull($questions[1]->idnumber);
// Check created idnumbers.
$qcat1 = $generator->create_question_category(array(
'name' => 'My category', 'sortorder' => 1, 'idnumber' => 'myqcat'));
$this->assertSame('myqcat', $qcat1->idnumber);
$quest1 = $generator->update_question($questions[0], null, ['idnumber' => 'myquest']);
$this->assertSame('myquest', $quest1->idnumber);
$quest3 = $generator->create_question('shortanswer', null,
['name' => 'sa1', 'category' => $qcat1->id, 'idnumber' => 'myquest_3']);
$this->assertSame('myquest_3', $quest3->idnumber);
// Check idnumbers of questions moved. Note have to use load_question_data or we only get to see old cached data.
question_move_questions_to_category([$quest1->id], $qcat1->id);
$this->assertSame('myquest', question_bank::load_question_data($quest1->id)->idnumber);
// Can only change idnumber of quest2 once quest1 has been moved to another category.
$quest2 = $generator->update_question($questions[1], null, ['idnumber' => 'myquest']);
question_move_questions_to_category([$quest2->id], $qcat1->id);
$this->assertSame('myquest_4', question_bank::load_question_data($quest2->id)->idnumber);
// Check can add an idnumber of 0.
$quest4 = $generator->create_question('shortanswer', null, ['name' => 'sa1', 'category' => $qcat1->id, 'idnumber' => '0']);
$this->assertSame('0', $quest4->idnumber);
}
}

4
question/type/ddwtos/tests/questiontype_test.php

@ -75,6 +75,7 @@ class qtype_ddwtos_test extends question_testcase {
$dd->stamp = make_unique_id_code();
$dd->version = make_unique_id_code();
$dd->hidden = 0;
$dd->idnumber = null;
$dd->timecreated = time();
$dd->timemodified = time();
$dd->createdby = $USER->id;
@ -122,6 +123,7 @@ class qtype_ddwtos_test extends question_testcase {
$expected = test_question_maker::make_question('ddwtos');
$expected->stamp = $qdata->stamp;
$expected->version = $qdata->version;
$expected->idnumber = null;
$q = $this->qtype->make_question($qdata);
@ -247,6 +249,7 @@ class qtype_ddwtos_test extends question_testcase {
$qdata = new stdClass();
$qdata->id = 123;
$qdata->contextid = \context_system::instance()->id;
$qdata->idnumber = null;
$qdata->qtype = 'ddwtos';
$qdata->name = 'A drag-and-drop question';
$qdata->questiontext = 'Put these in order: [[1]], [[2]], [[3]].';
@ -304,6 +307,7 @@ class qtype_ddwtos_test extends question_testcase {
<defaultgrade>3</defaultgrade>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
<shuffleanswers>1</shuffleanswers>
<correctfeedback format="moodle_auto_format">
<text><![CDATA[<p>Your answer is correct.</p>]]></text>

30
question/type/edit_question_form.php

@ -198,6 +198,10 @@ abstract class question_edit_form extends question_wizard_form {
$mform->setType('generalfeedback', PARAM_RAW);
$mform->addHelpButton('generalfeedback', 'generalfeedback', 'question');
$mform->addElement('text', 'idnumber', get_string('idnumber', 'question'), 'maxlength="100" size="10"');
$mform->addHelpButton('idnumber', 'idnumber', 'question');
$mform->setType('idnumber', PARAM_RAW);
// Any questiontype specific fields.
$this->definition_inner($mform);
@ -791,6 +795,8 @@ abstract class question_edit_form extends question_wizard_form {
}
public function validation($fromform, $files) {
global $DB;
$errors = parent::validation($fromform, $files);
if (empty($fromform['makecopy']) && isset($this->question->id)
&& ($this->question->formoptions->canedit ||
@ -810,6 +816,30 @@ abstract class question_edit_form extends question_wizard_form {
$errors['defaultmark'] = get_string('defaultmarkmustbepositive', 'question');
}
// Can only have one idnumber per category.
if (strpos($fromform['category'], ',') !== false) {
list($category, $categorycontextid) = explode(',', $fromform['category']);
} else {
$category = $fromform['category'];
}
if (isset($fromform['idnumber']) && ((string) $fromform['idnumber'] !== '')) {
if (empty($fromform['usecurrentcat']) && !empty($fromform['categorymoveto'])) {
$categoryinfo = $fromform['categorymoveto'];
} else {
$categoryinfo = $fromform['category'];
}
list($categoryid, $notused) = explode(',', $categoryinfo);
$conditions = 'category = ? AND idnumber = ?';
$params = [$categoryid, $fromform['idnumber']];
if (!empty($this->question->id)) {
$conditions .= ' AND id <> ?';
$params[] = $this->question->id;
}
if ($DB->record_exists_select('question', $conditions, $params)) {
$errors['idnumber'] = get_string('idnumbertaken', 'error');
}
}
return $errors;
}

3
question/type/gapselect/tests/questiontype_test.php

@ -79,6 +79,7 @@ class qtype_gapselect_test extends question_testcase {
$gapselect->stamp = make_unique_id_code();
$gapselect->version = make_unique_id_code();
$gapselect->hidden = 0;
$gapselect->idnumber = null;
$gapselect->timecreated = time();
$gapselect->timemodified = time();
$gapselect->createdby = $USER->id;
@ -243,6 +244,7 @@ class qtype_gapselect_test extends question_testcase {
$qdata = new stdClass();
$qdata->id = 123;
$qdata->contextid = \context_system::instance()->id;
$qdata->idnumber = null;
$qdata->qtype = 'gapselect';
$qdata->name = 'A select missing words question';
$qdata->questiontext = 'Put these in order: [[1]], [[2]], [[3]].';
@ -294,6 +296,7 @@ class qtype_gapselect_test extends question_testcase {
<defaultgrade>3</defaultgrade>
<penalty>0.3333333</penalty>
<hidden>0</hidden>
<idnumber></idnumber>
<shuffleanswers>1</shuffleanswers>
<correctfeedback format="moodle_auto_format">
<text><![CDATA[<p>Your answer is correct.</p>]]></text>

1
question/type/match/tests/questiontype_test.php

@ -68,6 +68,7 @@ class qtype_match_test extends advanced_testcase {
$q->stamp = make_unique_id_code();
$q->version = make_unique_id_code();
$q->hidden = 0;
$q->idnumber = null;
$q->timecreated = time();
$q->timemodified = time();
$q->createdby = $USER->id;

1
question/type/missingtype/tests/missingtype_test.php

@ -58,6 +58,7 @@ class qtype_missing_test extends question_testcase {
$questiondata->stamp = make_unique_id_code();
$questiondata->version = make_unique_id_code();
$questiondata->hidden = 0;
$questiondata->idnumber = null;
$questiondata->timecreated = 0;
$questiondata->timemodified = 0;
$questiondata->createdby = 0;

14
question/type/questiontypebase.php

@ -350,6 +350,19 @@ class question_type {
$question->defaultmark = $form->defaultmark;
}
if (isset($form->idnumber) && ((string) $form->idnumber !== '')) {
// While this check already exists in the form validation, this is a backstop preventing unnecessary errors.
if (strpos($form->category, ',') !== false) {
list($category, $categorycontextid) = explode(',', $form->category);
} else {
$category = $form->category;
}
if (!$DB->record_exists('question',
['idnumber' => $form->idnumber, 'category' => $category])) {
$question->idnumber = $form->idnumber;
}
}
// If the question is new, create it.
if (empty($question->id)) {
// Set the unique code.
@ -851,6 +864,7 @@ class question_type {
$question->stamp = $questiondata->stamp;
$question->version = $questiondata->version;
$question->hidden = $questiondata->hidden;
$question->idnumber = $questiondata->idnumber;
$question->timecreated = $questiondata->timecreated;
$question->timemodified = $questiondata->timemodified;
$question->createdby = $questiondata->createdby;

2
version.php

@ -29,7 +29,7 @@
defined('MOODLE_INTERNAL') || die();
$version = 2018092000.00; // YYYYMMDD = weekly release date of this DEV branch.
$version = 2018092100.04; // YYYYMMDD = weekly release date of this DEV branch.
// RR = release increments - 00 in DEV branches.
// .XX = incremental changes.