From 0aa04a7faf2b3cb21a0a3f7b09a155e49fa3cca2 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 23 Sep 2013 19:34:15 +0100 Subject: [PATCH] MDL-16464 remove multichoice answers field. This field is not used, but sometimes causes errors when we try to instert a value that is too long to fit. Therefore remove it. At the same time, rename tables and columns to match the coding guidelines. --- question/type/multianswer/questiontype.php | 4 +- .../backup_qtype_multichoice_plugin.class.php | 6 +- ...restore_qtype_multichoice_plugin.class.php | 22 ++--- question/type/multichoice/db/install.xml | 7 +- question/type/multichoice/db/upgrade.php | 87 ++++++++++++++++++- question/type/multichoice/questiontype.php | 17 ++-- .../multichoice/tests/questiontype_test.php | 2 +- question/type/multichoice/version.php | 2 +- 8 files changed, 107 insertions(+), 40 deletions(-) diff --git a/question/type/multianswer/questiontype.php b/question/type/multianswer/questiontype.php index 5df9f2cce59..40a82b6a532 100644 --- a/question/type/multianswer/questiontype.php +++ b/question/type/multianswer/questiontype.php @@ -109,8 +109,8 @@ class qtype_multianswer extends question_type { if ($oldwrappedquestion->qtype != $wrapped->qtype) { switch ($oldwrappedquestion->qtype) { case 'multichoice': - $DB->delete_records('question_multichoice', - array('question' => $oldwrappedquestion->id)); + $DB->delete_records('qtype_multichoice_options', + array('questionid' => $oldwrappedquestion->id)); break; case 'shortanswer': $DB->delete_records('qtype_shortanswer_options', diff --git a/question/type/multichoice/backup/moodle2/backup_qtype_multichoice_plugin.class.php b/question/type/multichoice/backup/moodle2/backup_qtype_multichoice_plugin.class.php index 930b97189c5..fa9441ce2c0 100644 --- a/question/type/multichoice/backup/moodle2/backup_qtype_multichoice_plugin.class.php +++ b/question/type/multichoice/backup/moodle2/backup_qtype_multichoice_plugin.class.php @@ -53,7 +53,7 @@ class backup_qtype_multichoice_plugin extends backup_qtype_plugin { // Now create the qtype own structures. $multichoice = new backup_nested_element('multichoice', array('id'), array( - 'layout', 'answers', 'single', 'shuffleanswers', + 'layout', 'single', 'shuffleanswers', 'correctfeedback', 'correctfeedbackformat', 'partiallycorrectfeedback', 'partiallycorrectfeedbackformat', 'incorrectfeedback', 'incorrectfeedbackformat', 'answernumbering', 'shownumcorrect')); @@ -62,8 +62,8 @@ class backup_qtype_multichoice_plugin extends backup_qtype_plugin { $pluginwrapper->add_child($multichoice); // Set source to populate the data. - $multichoice->set_source_table('question_multichoice', - array('question' => backup::VAR_PARENTID)); + $multichoice->set_source_table('qtype_multichoice_options', + array('questionid' => backup::VAR_PARENTID)); // Don't need to annotate ids nor files. diff --git a/question/type/multichoice/backup/moodle2/restore_qtype_multichoice_plugin.class.php b/question/type/multichoice/backup/moodle2/restore_qtype_multichoice_plugin.class.php index c9f0247e09a..cceada35aa1 100644 --- a/question/type/multichoice/backup/moodle2/restore_qtype_multichoice_plugin.class.php +++ b/question/type/multichoice/backup/moodle2/restore_qtype_multichoice_plugin.class.php @@ -68,24 +68,14 @@ class restore_qtype_multichoice_plugin extends restore_qtype_plugin { $questioncreated = (bool) $this->get_mappingid('question_created', $oldquestionid); // If the question has been created by restore, we need to create its - // question_multichoice too. + // qtype_multichoice_options too. if ($questioncreated) { // Adjust some columns. - $data->question = $newquestionid; - // Map sequence of question_answer ids. - if ($data->answers) { - $answersarr = explode(',', $data->answers); - } else { - $answersarr = array(); - } - foreach ($answersarr as $key => $answer) { - $answersarr[$key] = $this->get_mappingid('question_answer', $answer); - } - $data->answers = implode(',', $answersarr); + $data->questionid = $newquestionid; // Insert record. - $newitemid = $DB->insert_record('question_multichoice', $data); + $newitemid = $DB->insert_record('qtype_multichoice_options', $data); // Create mapping (needed for decoding links). - $this->set_mapping('question_multichoice', $oldid, $newitemid); + $this->set_mapping('qtype_multichoice_options', $oldid, $newitemid); } } @@ -163,8 +153,8 @@ class restore_qtype_multichoice_plugin extends restore_qtype_plugin { $contents = array(); $fields = array('correctfeedback', 'partiallycorrectfeedback', 'incorrectfeedback'); - $contents[] = new restore_decode_content('question_multichoice', - $fields, 'question_multichoice'); + $contents[] = new restore_decode_content('qtype_multichoice_options', + $fields, 'qtype_multichoice_options'); return $contents; } diff --git a/question/type/multichoice/db/install.xml b/question/type/multichoice/db/install.xml index eb7bb86288f..1c446fcf95f 100644 --- a/question/type/multichoice/db/install.xml +++ b/question/type/multichoice/db/install.xml @@ -4,12 +4,11 @@ xsi:noNamespaceSchemaLocation="../../../../lib/xmldb/xmldb.xsd" > - +
- + - @@ -23,7 +22,7 @@ - +
diff --git a/question/type/multichoice/db/upgrade.php b/question/type/multichoice/db/upgrade.php index 63b4cb8f097..489e29cc096 100644 --- a/question/type/multichoice/db/upgrade.php +++ b/question/type/multichoice/db/upgrade.php @@ -36,21 +36,102 @@ function xmldb_qtype_multichoice_upgrade($oldversion) { $dbman = $DB->get_manager(); - // Moodle v2.2.0 release upgrade line // Put any upgrade step following this. // Moodle v2.3.0 release upgrade line // Put any upgrade step following this. - // Moodle v2.4.0 release upgrade line // Put any upgrade step following this. - // Moodle v2.5.0 release upgrade line. // Put any upgrade step following this. + if ($oldversion < 2013092300) { + // Find duplicate rows before they break the 2013092304 step below. + $problemids = $DB->get_recordset_sql(" + SELECT question, MIN(id) AS recordidtokeep + FROM {question_multichoice} + GROUP BY question + HAVING COUNT(1) > 1 + "); + foreach ($problemids as $problem) { + $DB->delete_records_select('question_multichoice', + 'question = ? AND id > ?', + array($problem->question, $problem->recordidtokeep)); + } + $problemids->close(); + + // Shortanswer savepoint reached. + upgrade_plugin_savepoint(true, 2013092300, 'qtype', 'multichoice'); + } + + if ($oldversion < 2013092301) { + + // Define table question_multichoice to be renamed to qtype_multichoice_options. + $table = new xmldb_table('question_multichoice'); + + // Launch rename table for question_multichoice. + $dbman->rename_table($table, 'qtype_multichoice_options'); + + // Record that qtype_match savepoint was reached. + upgrade_plugin_savepoint(true, 2013092301, 'qtype', 'multichoice'); + } + + if ($oldversion < 2013092302) { + + // Define key question (foreign) to be dropped form qtype_multichoice_options. + $table = new xmldb_table('qtype_multichoice_options'); + $key = new xmldb_key('question', XMLDB_KEY_FOREIGN, array('question'), 'question', array('id')); + + // Launch drop key question. + $dbman->drop_key($table, $key); + + // Record that qtype_match savepoint was reached. + upgrade_plugin_savepoint(true, 2013092302, 'qtype', 'multichoice'); + } + + if ($oldversion < 2013092303) { + + // Rename field question on table qtype_multichoice_options to questionid. + $table = new xmldb_table('qtype_multichoice_options'); + $field = new xmldb_field('question', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0', 'id'); + + // Launch rename field question. + $dbman->rename_field($table, $field, 'questionid'); + + // Record that qtype_match savepoint was reached. + upgrade_plugin_savepoint(true, 2013092303, 'qtype', 'multichoice'); + } + + if ($oldversion < 2013092304) { + + // Define key questionid (foreign-unique) to be added to qtype_multichoice_options. + $table = new xmldb_table('qtype_multichoice_options'); + $key = new xmldb_key('questionid', XMLDB_KEY_FOREIGN_UNIQUE, array('questionid'), 'question', array('id')); + + // Launch add key questionid. + $dbman->add_key($table, $key); + + // Record that qtype_match savepoint was reached. + upgrade_plugin_savepoint(true, 2013092304, 'qtype', 'multichoice'); + } + + if ($oldversion < 2013092305) { + + // Define field answers to be dropped from qtype_multichoice_options. + $table = new xmldb_table('qtype_multichoice_options'); + $field = new xmldb_field('answers'); + + // Conditionally launch drop field answers. + if ($dbman->field_exists($table, $field)) { + $dbman->drop_field($table, $field); + } + + // Record that qtype_match savepoint was reached. + upgrade_plugin_savepoint(true, 2013092305, 'qtype', 'multichoice'); + } return true; } diff --git a/question/type/multichoice/questiontype.php b/question/type/multichoice/questiontype.php index 352018ed4d6..164accee4cc 100644 --- a/question/type/multichoice/questiontype.php +++ b/question/type/multichoice/questiontype.php @@ -39,8 +39,8 @@ require_once($CFG->libdir . '/questionlib.php'); class qtype_multichoice extends question_type { public function get_question_options($question) { global $DB, $OUTPUT; - $question->options = $DB->get_record('question_multichoice', - array('question' => $question->id), '*', MUST_EXIST); + $question->options = $DB->get_record('qtype_multichoice_options', + array('questionid' => $question->id), '*', MUST_EXIST); parent::get_question_options($question); } @@ -67,7 +67,6 @@ class qtype_multichoice extends question_type { // Insert all the new answers. $totalfraction = 0; $maxfraction = -1; - $answers = array(); foreach ($question->answer as $key => $answerdata) { if (trim($answerdata['text']) == '') { continue; @@ -93,7 +92,6 @@ class qtype_multichoice extends question_type { $answer->feedbackformat = $question->feedback[$key]['format']; $DB->update_record('question_answers', $answer); - $answers[] = $answer->id; if ($question->fraction[$key] > 0) { $totalfraction += $question->fraction[$key]; @@ -110,17 +108,16 @@ class qtype_multichoice extends question_type { $DB->delete_records('question_answers', array('id' => $oldanswer->id)); } - $options = $DB->get_record('question_multichoice', array('question' => $question->id)); + $options = $DB->get_record('qtype_multichoice_options', array('questionid' => $question->id)); if (!$options) { $options = new stdClass(); - $options->question = $question->id; + $options->questionid = $question->id; $options->correctfeedback = ''; $options->partiallycorrectfeedback = ''; $options->incorrectfeedback = ''; - $options->id = $DB->insert_record('question_multichoice', $options); + $options->id = $DB->insert_record('qtype_multichoice_options', $options); } - $options->answers = implode(',', $answers); $options->single = $question->single; if (isset($question->layout)) { $options->layout = $question->layout; @@ -128,7 +125,7 @@ class qtype_multichoice extends question_type { $options->answernumbering = $question->answernumbering; $options->shuffleanswers = $question->shuffleanswers; $options = $this->save_combined_feedback_helper($options, $question, $context, true); - $DB->update_record('question_multichoice', $options); + $DB->update_record('qtype_multichoice_options', $options); $this->save_hints($question, true); @@ -179,7 +176,7 @@ class qtype_multichoice extends question_type { public function delete_question($questionid, $contextid) { global $DB; - $DB->delete_records('question_multichoice', array('question' => $questionid)); + $DB->delete_records('qtype_multichoice_options', array('question' => $questionid)); parent::delete_question($questionid, $contextid); } diff --git a/question/type/multichoice/tests/questiontype_test.php b/question/type/multichoice/tests/questiontype_test.php index 4c05df1453b..fa93d268d7c 100644 --- a/question/type/multichoice/tests/questiontype_test.php +++ b/question/type/multichoice/tests/questiontype_test.php @@ -139,7 +139,7 @@ class qtype_multichoice_test extends advanced_testcase { } foreach ($questiondata->options as $optionname => $value) { - if (!in_array($optionname, array('answers'))) { + if ($optionname != 'answers') { $this->assertAttributeEquals($value, $optionname, $actualquestiondata->options); } } diff --git a/question/type/multichoice/version.php b/question/type/multichoice/version.php index 15355a9f070..f27a7689663 100644 --- a/question/type/multichoice/version.php +++ b/question/type/multichoice/version.php @@ -26,7 +26,7 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'qtype_multichoice'; -$plugin->version = 2013050100; +$plugin->version = 2013092305; $plugin->requires = 2013050100;