Merge branch 'MDL-61614-master' of git://github.com/rezaies/moodle

mod/quiz/attemptlib.php

@@ -136,7 +136,7 @@ public function create_attempt_object($attemptdata) {
     public function preload_questions() {
         $this->questions = question_preload_questions(null,
                 'slot.maxmark, slot.id AS slotid, slot.slot, slot.page,
-                 slot.questioncategoryid AS randomfromcategory, slot.tags AS randomfromtags,
+                 slot.questioncategoryid AS randomfromcategory,
                  slot.includingsubcategories AS randomincludingsubcategories',
                 '{quiz_slots} slot ON slot.quizid = :quizid AND q.id = slot.questionid',
                 array('quizid' => $this->quiz->id), 'slot.slot');
@@ -569,7 +569,7 @@ public function __construct($attempt, $quiz, $cm, $course, $loadquestions = true
         $this->quba = question_engine::load_questions_usage_by_activity($this->attempt->uniqueid);
         $this->slots = $DB->get_records('quiz_slots',
                 array('quizid' => $this->get_quizid()), 'slot',
-                'slot, requireprevious, questionid, includingsubcategories, tags');
+                'slot, requireprevious, questionid, includingsubcategories');
         $this->sections = array_values($DB->get_records('quiz_sections',
                 array('quizid' => $this->get_quizid()), 'firstslot'));
 
@@ -1874,7 +1874,7 @@ public function process_redo_question($slot, $timestamp) {
         if ($questiondata->qtype != 'random') {
             $newqusetionid = $questiondata->id;
         } else {
-            $tagids = quiz_extract_random_question_tag_ids($this->slots[$slot]->tags);
+            $tagids = quiz_retrieve_slot_tag_ids($this->slots[$slot]->id);
 
             $randomloader = new \core_question\bank\random_question_loader($qubaids, array());
             $newqusetionid = $randomloader->get_next_question_id($questiondata->category,

mod/quiz/backup/moodle2/backup_quiz_stepslib.php

@@ -58,7 +58,10 @@ protected function define_structure() {
         $qinstances = new backup_nested_element('question_instances');
 
         $qinstance = new backup_nested_element('question_instance', array('id'), array(
-            'slot', 'page', 'requireprevious', 'questionid', 'questioncategoryid', 'includingsubcategories', 'tags', 'maxmark'));
+            'slot', 'page', 'requireprevious', 'questionid', 'questioncategoryid', 'includingsubcategories', 'maxmark'));
+
+        $qinstancetags = new backup_nested_element('tags');
+        $qinstancetag = new backup_nested_element('tag', array('id'), array('tagid', 'tagname'));
 
         $sections = new backup_nested_element('sections');
 
@@ -98,6 +101,9 @@ protected function define_structure() {
         $quiz->add_child($qinstances);
         $qinstances->add_child($qinstance);
 
+        $qinstance->add_child($qinstancetags);
+        $qinstancetags->add_child($qinstancetag);
+
         $quiz->add_child($sections);
         $sections->add_child($section);
 
@@ -119,6 +125,9 @@ protected function define_structure() {
         $qinstance->set_source_table('quiz_slots',
                 array('quizid' => backup::VAR_PARENTID));
 
+        $qinstancetag->set_source_table('quiz_slot_tags',
+                array('slotid' => backup::VAR_PARENTID));
+
         $section->set_source_table('quiz_sections',
                 array('quizid' => backup::VAR_PARENTID));
 

mod/quiz/backup/moodle2/restore_quiz_stepslib.php

@@ -59,6 +59,8 @@ protected function define_structure() {
 
         $paths[] = new restore_path_element('quiz_question_instance',
                 '/activity/quiz/question_instances/question_instance');
+        $paths[] = new restore_path_element('quiz_slot_tags',
+                '/activity/quiz/question_instances/question_instance/tags/tag');
         $paths[] = new restore_path_element('quiz_section', '/activity/quiz/sections/section');
         $paths[] = new restore_path_element('quiz_feedback', '/activity/quiz/feedbacks/feedback');
         $paths[] = new restore_path_element('quiz_override', '/activity/quiz/overrides/override');
@@ -254,6 +256,7 @@ protected function process_quiz_question_instance($data) {
         global $CFG, $DB;
 
         $data = (object)$data;
+        $oldid = $data->id;
 
         // Backwards compatibility for old field names (MDL-43670).
         if (!isset($data->questionid) && isset($data->question)) {
@@ -301,14 +304,32 @@ protected function process_quiz_question_instance($data) {
             $data->includingsubcategories = $questionmapping->info->questiontext ? 1 : 0;
         }
 
-        if (isset($data->tags)) {
-            require_once($CFG->dirroot . '/mod/quiz/locallib.php');
+        $newitemid = $DB->insert_record('quiz_slots', $data);
+        // Add mapping, restore of slot tags (for random questions) need it.
+        $this->set_mapping('quiz_question_instance', $oldid, $newitemid);
+    }
+
+    /**
+     * Process a quiz_slot_tags restore
+     *
+     * @param stdClass|array $data The quiz_slot_tags data
+     */
+    protected function process_quiz_slot_tags($data) {
+        global $DB;
 
-            $tags = quiz_extract_random_question_tags($data->tags, $this->task->is_samesite());
-            $data->tags = quiz_build_random_question_tag_json($tags);
+        $data = (object)$data;
+
+        $data->slotid = $this->get_new_parentid('quiz_question_instance');
+        if ($this->task->is_samesite() && $tag = core_tag_tag::get($data->tagid, 'id, name')) {
+            $data->tagname = $tag->name;
+        } else if ($tag = core_tag_tag::get_by_name(0, $data->tagname, 'id, name')) {
+            $data->tagid = $tag->id;
+        } else {
+            $data->tagid = null;
+            $data->tagname = $tag->name;
         }
 
-        $DB->insert_record('quiz_slots', $data);
+        $DB->insert_record('quiz_slot_tags', $data);
     }
 
     protected function process_quiz_section($data) {

mod/quiz/classes/local/structure/slot_random.php

@@ -43,6 +43,11 @@ class slot_random {
      */
     protected $quiz = null;
 
+    /**
+     * @var \core_tag_tag[] List of tags for this slot.
+     */
+    protected $tags = [];
+
     /**
      * slot_random constructor.
      *
@@ -52,9 +57,8 @@ public function __construct($slotrecord = null) {
         $this->record = new \stdClass();
 
         $properties = array(
-                'id', 'slot', 'quizid', 'page', 'requireprevious',
-                'questionid', 'questioncategoryid', 'includingsubcategories',
-                'tags', 'maxmark');
+            'id', 'slot', 'quizid', 'page', 'requireprevious', 'questionid',
+            'questioncategoryid', 'includingsubcategories', 'maxmark');
 
         foreach ($properties as $property) {
             if (isset($slotrecord->$property)) {
@@ -95,6 +99,29 @@ public function set_quiz($quiz) {
         $this->record->quizid = $quiz->id;
     }
 
+    /**
+     * Set some tags for this quiz slot.
+     *
+     * @param \core_tag_tag[] $tags
+     */
+    public function set_tags($tags) {
+        $this->tags = [];
+        foreach ($tags as $tag) {
+            // We use $tag->id as the key for the array so not only it handles duplicates of the same tag being given,
+            // but also it is consistent with the behaviour of set_tags_by_id() below.
+            $this->tags[$tag->id] = $tag;
+        }
+    }
+
+    /**
+     * Set some tags for this quiz slot. This function uses tag ids to find tags.
+     *
+     * @param int[] $tagids
+     */
+    public function set_tags_by_id($tagids) {
+        $this->tags = \core_tag_tag::get_bulk($tagids, 'id, name');
+    }
+
     /**
      * Inserts the quiz slot at the $page page.
      * It is required to call this function if you are building a quiz slot object from scratch.
@@ -151,6 +178,19 @@ public function insert($page) {
         }
 
         $this->record->id = $DB->insert_record('quiz_slots', $this->record);
+
+        if (!empty($this->tags)) {
+            $recordstoinsert = [];
+            foreach ($this->tags as $tag) {
+                $recordstoinsert[] = (object)[
+                    'slotid' => $this->record->id,
+                    'tagid' => $tag->id,
+                    'tagname' => $tag->name
+                ];
+            }
+            $DB->insert_records('quiz_slot_tags', $recordstoinsert);
+        }
+
         $trans->allow_commit();
     }
 }

mod/quiz/classes/structure.php

@@ -914,6 +914,7 @@ public function remove_slot($slotnumber) {
         $maxslot = $DB->get_field_sql('SELECT MAX(slot) FROM {quiz_slots} WHERE quizid = ?', array($this->get_quizid()));
 
         $trans = $DB->start_delegated_transaction();
+        $DB->delete_records('quiz_slot_tags', array('slotid' => $slot->id));
         $DB->delete_records('quiz_slots', array('id' => $slot->id));
         for ($i = $slot->slot + 1; $i <= $maxslot; $i++) {
             $DB->set_field('quiz_slots', 'slot', $i - 1,

mod/quiz/db/install.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="mod/quiz/db" VERSION="20180207" COMMENT="XMLDB file for Moodle mod/quiz"
+<XMLDB PATH="mod/quiz/db" VERSION="20180407" COMMENT="XMLDB file for Moodle mod/quiz"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../../lib/xmldb/xmldb.xsd"
 >
@@ -65,7 +65,6 @@
         <FIELD NAME="questionid" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Foreign key references question.id."/>
         <FIELD NAME="questioncategoryid" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="The question category that the random question will be picked from. Will be null if and only if the question is not a random question."/>
         <FIELD NAME="includingsubcategories" TYPE="int" LENGTH="4" NOTNULL="false" SEQUENCE="false" COMMENT="Whether the random question can be picked from sub categories or not. Will be null if questioncategoryid is null."/>
-        <FIELD NAME="tags" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="Contains data about the tags that a question must have so that it can be selected for this slot. This field is an array in the form of [tagid,tagname] which is stored as JSON. Will be null if questioncategoryid is null."/>
         <FIELD NAME="maxmark" TYPE="number" LENGTH="12" NOTNULL="true" DEFAULT="0" SEQUENCE="false" DECIMALS="7" COMMENT="How many marks this question contributes to quiz.sumgrades."/>
       </FIELDS>
       <KEYS>
@@ -186,5 +185,18 @@
         <INDEX NAME="name" UNIQUE="true" FIELDS="name"/>
       </INDEXES>
     </TABLE>
+    <TABLE NAME="quiz_slot_tags" COMMENT="Stores data about the tags that a question must have so that it can be selected for a quiz slot (when having a random question by tags on that slot).">
+      <FIELDS>
+        <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
+        <FIELD NAME="slotid" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="The quiz slot that this tag belong to"/>
+        <FIELD NAME="tagid" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false"/>
+        <FIELD NAME="tagname" TYPE="char" LENGTH="255" NOTNULL="false" SEQUENCE="false" COMMENT="The tag name is to be stored as well, so we won't lose data if the tag is removed from Moodle (A tag with the same name might be added in future)."/>
+      </FIELDS>
+      <KEYS>
+        <KEY NAME="primary" TYPE="primary" FIELDS="id"/>
+        <KEY NAME="slotid" TYPE="foreign" FIELDS="slotid" REFTABLE="quiz_slots" REFFIELDS="id"/>
+        <KEY NAME="tagid" TYPE="foreign" FIELDS="tagid" REFTABLE="tag" REFFIELDS="id"/>
+      </KEYS>
+    </TABLE>
   </TABLES>
 </XMLDB>

mod/quiz/db/upgrade.php

@@ -117,13 +117,6 @@ function xmldb_quiz_upgrade($oldversion) {
             $dbman->add_field($table, $field);
         }
 
-        // Define field tags to be added to quiz_slots.
-        $field = new xmldb_field('tags', XMLDB_TYPE_TEXT, null, null, null, null, null, 'includingsubcategories');
-        // Conditionally launch add field tags.
-        if (!$dbman->field_exists($table, $field)) {
-            $dbman->add_field($table, $field);
-        }
-
         // Quiz savepoint reached.
         upgrade_mod_savepoint(true, 2018020700, 'quiz');
     }
@@ -163,5 +156,45 @@ function xmldb_quiz_upgrade($oldversion) {
         upgrade_mod_savepoint(true, 2018020701, 'quiz');
     }
 
+    if ($oldversion < 2018040700) {
+
+        // Define field tags to be dropped from quiz_slots. This field was added earlier to master only.
+        $table = new xmldb_table('quiz_slots');
+        $field = new xmldb_field('tags');
+
+        // Conditionally launch drop field quizid.
+        if ($dbman->field_exists($table, $field)) {
+            $dbman->drop_field($table, $field);
+        }
+
+        // Quiz savepoint reached.
+        upgrade_mod_savepoint(true, 2018040700, 'quiz');
+    }
+
+    if ($oldversion < 2018040800) {
+
+        // Define table quiz_slot_tags to be created.
+        $table = new xmldb_table('quiz_slot_tags');
+
+        // Adding fields to table quiz_slot_tags.
+        $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
+        $table->add_field('slotid', XMLDB_TYPE_INTEGER, '10', null, null, null, null);
+        $table->add_field('tagid', XMLDB_TYPE_INTEGER, '10', null, null, null, null);
+        $table->add_field('tagname', XMLDB_TYPE_CHAR, '255', null, null, null, null);
+
+        // Adding keys to table quiz_slot_tags.
+        $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
+        $table->add_key('slotid', XMLDB_KEY_FOREIGN, array('slotid'), 'quiz_slots', array('id'));
+        $table->add_key('tagid', XMLDB_KEY_FOREIGN, array('tagid'), 'tag', array('id'));
+
+        // Conditionally launch create table for quiz_slot_tags.
+        if (!$dbman->table_exists($table)) {
+            $dbman->create_table($table);
+        }
+
+        // Quiz savepoint reached.
+        upgrade_mod_savepoint(true, 2018040800, 'quiz');
+    }
+
     return true;
 }

mod/quiz/editrandom.php

@@ -78,11 +78,9 @@
 $toform->category = "{$category->id},{$category->contextid}";
 $toform->includesubcategories = $slot->includingsubcategories;
 $toform->fromtags = array();
-if ($slot->tags) {
-    $tags = quiz_extract_random_question_tags($slot->tags);
-    foreach ($tags as $tag) {
-        $toform->fromtags[] = "{$tag->id},{$tag->name}";
-    }
+$currentslottags = quiz_retrieve_slot_tags($slot->id);
+foreach ($currentslottags as $slottag) {
+    $toform->fromtags[] = "{$slottag->tagid},{$slottag->tagname}";
 }
 $toform->returnurl = $returnurl;
 
@@ -117,6 +115,8 @@
     $slot->questioncategoryid = $fromform->category;
     $slot->includingsubcategories = $fromform->includesubcategories;
 
+    $DB->update_record('quiz_slots', $slot);
+
     $tags = [];
     foreach ($fromform->fromtags as $tagstring) {
         list($tagid, $tagname) = explode(',', $tagstring);
@@ -125,9 +125,39 @@
             'name' => $tagname
         ];
     }
-    $slot->tags = quiz_build_random_question_tag_json($tags);
 
-    $DB->update_record('quiz_slots', $slot);
+    $recordstokeep = [];
+    $recordstoinsert = [];
+    $searchableslottags = array_map(function($slottag) {
+        return ['tagid' => $slottag->tagid, 'tagname' => $slottag->tagname];
+    }, $currentslottags);
+
+    foreach ($tags as $tag) {
+        if ($key = array_search(['tagid' => $tag->id, 'tagname' => $tag->name], $searchableslottags)) {
+            // If found, $key would be the id field in the quiz_slot_tags table.
+            // Therefore, there was no need to check !== false here.
+            $recordstokeep[] = $key;
+        } else {
+            $recordstoinsert[] = (object)[
+                'slotid' => $slot->id,
+                'tagid' => $tag->id,
+                'tagname' => $tag->name
+            ];
+        }
+    }
+
+    // Now, delete the remaining records.
+    if (!empty($recordstokeep)) {
+        list($select, $params) = $DB->get_in_or_equal($recordstokeep, SQL_PARAMS_QM, 'param', false);
+        $DB->delete_records_select('quiz_slot_tags', "id $select", $params);
+    } else {
+        $DB->delete_records('quiz_slot_tags', array('slotid' => $slot->id));
+    }
+
+    // And now, insert the extra records if there is any.
+    if (!empty($recordstoinsert)) {
+        $DB->insert_records('quiz_slot_tags', $recordstoinsert);
+    }
 
     // Purge this question from the cache.
     question_bank::notify_question_edited($question->id);

mod/quiz/lib.php

@@ -183,7 +183,9 @@ function quiz_delete_instance($id) {
              WHERE slot.quizid = ? AND q.qtype = ?";
     $questionids = $DB->get_fieldset_sql($sql, array($quiz->id, 'random'));
 
-    // We need to do this before we try and delete randoms, otherwise they would still be 'in use'.
+    // We need to do the following deletes before we try and delete randoms, otherwise they would still be 'in use'.
+    $quizslots = $DB->get_fieldset_select('quiz_slots', 'id', 'quizid = ?', array($quiz->id));
+    $DB->delete_records_list('quiz_slot_tags', 'slotid', $quizslots);
     $DB->delete_records('quiz_slots', array('quizid' => $quiz->id));
     $DB->delete_records('quiz_sections', array('quizid' => $quiz->id));