MDL-50851 core_question: use new tag API

diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php
index 88e118e..caf5f07 100644 (file)
--- a/backup/moodle2/restore_stepslib.php
+++ b/backup/moodle2/restore_stepslib.php
@@ -4064,25 +4064,17 @@ class restore_create_categories_and_questions extends restore_structure_step {
             return;
         }
 
-        if (!empty($CFG->usetags)) { // if enabled in server
-            // TODO: This is highly inefficient. Each time we add one tag
-            // we fetch all the existing because tag_set() deletes them
-            // so everything must be reinserted on each call
-            $tags = array();
-            $existingtags = tag_get_tags('question', $newquestion);
-            // Re-add all the existitng tags
-            foreach ($existingtags as $existingtag) {
-                $tags[] = $existingtag->rawname;
-            }
-            // Add the one being restored
-            $tags[] = $data->rawname;
+        if (core_tag_tag::is_enabled('core_question', 'question')) {
+            $tagname = $data->rawname;
             // Get the category, so we can then later get the context.
             $categoryid = $this->get_new_parentid('question_category');
             if (empty($this->cachedcategory) || $this->cachedcategory->id != $categoryid) {
                 $this->cachedcategory = $DB->get_record('question_categories', array('id' => $categoryid));
             }
-            // Send all the tags back to the question
-            tag_set('question', $newquestion, $tags, 'core_question', $this->cachedcategory->contextid);
+            // Add the tag to the question.
+            core_tag_tag::add_item_tag('core_question', 'question', $newquestion,
+                    context::instance_by_id($this->cachedcategory->contextid),
+                    $tagname);
         }
     }
 

diff --git a/lib/questionlib.php b/lib/questionlib.php
index 2a2d106..a4fbf68 100644 (file)
--- a/lib/questionlib.php
+++ b/lib/questionlib.php
@@ -334,7 +334,7 @@ function question_delete_question($questionid) {
             $questionid, $question->contextid);
 
     // Delete all tag instances.
-    $DB->delete_records('tag_instance', array('component' => 'core_question', 'itemid' => $question->id));
+    core_tag_tag::remove_all_item_tags('core_question', 'question', $question->id);
 
     // Now recursively delete all child questions
     if ($children = $DB->get_records('question',
@@ -435,8 +435,7 @@ function question_delete_course_category($category, $newcategory, $feedback=true
         }
 
         // Update the contextid for any tag instances for questions in the old context.
-        $DB->set_field('tag_instance', 'contextid', $newcontext->id, array('component' => 'core_question',
-            'contextid' => $context->id));
+        core_tag_tag::move_context('core_question', 'question', $context, $newcontext);
 
         $DB->set_field('question_categories', 'contextid', $newcontext->id, array('contextid' => $context->id));
 
@@ -544,8 +543,7 @@ function question_move_questions_to_category($questionids, $newcategoryid) {
             "parent $questionidcondition", $params);
 
     // Update the contextid for any tag instances that may exist for these questions.
-    $DB->set_field_select('tag_instance', 'contextid', $newcontextid,
-        "component = 'core_question' AND itemid $questionidcondition", $params);
+    core_tag_tag::change_items_context('core_question', 'question', $questionids, $newcontextid);
 
     // TODO Deal with datasets.
 
@@ -577,12 +575,8 @@ function question_move_category_to_context($categoryid, $oldcontextid, $newconte
         question_bank::notify_question_edited($questionid);
     }
 
-    if ($questionids) {
-        // Update the contextid for any tag instances that may exist for these questions.
-        list($questionids, $params) = $DB->get_in_or_equal(array_keys($questionids));
-        $DB->set_field_select('tag_instance', 'contextid', $newcontextid,
-            "component = 'core_question' AND itemid $questionids", $params);
-    }
+    core_tag_tag::change_items_context('core_question', 'question',
+            array_keys($questionids), $newcontextid);
 
     $subcatids = $DB->get_records_menu('question_categories',
             array('parent' => $categoryid), '', 'id,1');
@@ -765,9 +759,8 @@ function _tidy_question($question, $loadtags = false) {
         unset($question->_partiallyloaded);
     }
 
-    if ($loadtags && !empty($CFG->usetags)) {
-        require_once($CFG->dirroot . '/tag/lib.php');
-        $question->tags = tag_get_tags_array('question', $question->id);
+    if ($loadtags && core_tag_tag::is_enabled('core_question', 'question')) {
+        $question->tags = core_tag_tag::get_item_tags_array('core_question', 'question', $question->id);
     }
 }
 

diff --git a/lib/tests/questionlib_test.php b/lib/tests/questionlib_test.php
index 6455616..1635010 100644 (file)
--- a/lib/tests/questionlib_test.php
+++ b/lib/tests/questionlib_test.php
@@ -29,7 +29,6 @@ global $CFG;
 
 require_once($CFG->libdir . '/questionlib.php');
 require_once($CFG->dirroot . '/mod/quiz/locallib.php');
-require_once($CFG->dirroot . '/tag/lib.php');
 
 // Get the necessary files to perform backup and restore.
 require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php');
@@ -164,21 +163,23 @@ class core_questionlib_testcase extends advanced_testcase {
         $coursecat2 = $this->getDataGenerator()->create_category();
 
         // Create a couple of categories and questions.
+        $context1 = context_coursecat::instance($coursecat1->id);
+        $context2 = context_coursecat::instance($coursecat2->id);
         $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
         $questioncat1 = $questiongenerator->create_question_category(array('contextid' =>
-            context_coursecat::instance($coursecat1->id)->id));
+            $context1->id));
         $questioncat2 = $questiongenerator->create_question_category(array('contextid' =>
-            context_coursecat::instance($coursecat2->id)->id));
+            $context2->id));
         $question1 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat1->id));
         $question2 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat1->id));
         $question3 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat2->id));
         $question4 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat2->id));
 
         // Now lets tag these questions.
-        tag_set('question', $question1->id, array('tag 1', 'tag 2'), 'core_question', $questioncat1->contextid);
-        tag_set('question', $question2->id, array('tag 3', 'tag 4'), 'core_question', $questioncat1->contextid);
-        tag_set('question', $question3->id, array('tag 5', 'tag 6'), 'core_question', $questioncat2->contextid);
-        tag_set('question', $question4->id, array('tag 7', 'tag 8'), 'core_question', $questioncat2->contextid);
+        core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $context1, array('tag 1', 'tag 2'));
+        core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $context1, array('tag 3', 'tag 4'));
+        core_tag_tag::set_item_tags('core_question', 'question', $question3->id, $context2, array('tag 5', 'tag 6'));
+        core_tag_tag::set_item_tags('core_question', 'question', $question4->id, $context2, array('tag 7', 'tag 8'));
 
         // Test moving the questions to another category.
         question_move_questions_to_category(array($question1->id, $question2->id), $questioncat2->id);
@@ -224,14 +225,15 @@ class core_questionlib_testcase extends advanced_testcase {
         $course = $this->getDataGenerator()->create_course();
 
         // Create some question categories and questions in this course.
+        $coursecontext = context_course::instance($course->id);
         $questioncat = $questiongenerator->create_question_category(array('contextid' =>
-            context_course::instance($course->id)->id));
+            $coursecontext->id));
         $question1 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat->id));
         $question2 = $questiongenerator->create_question('shortanswer', null, array('category' => $questioncat->id));
 
         // Add some tags to these questions.
-        tag_set('question', $question1->id, array('tag 1', 'tag 2'), 'core_question', $questioncat->contextid);
-        tag_set('question', $question2->id, array('tag 1', 'tag 2'), 'core_question', $questioncat->contextid);
+        core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $coursecontext, array('tag 1', 'tag 2'));
+        core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $coursecontext, array('tag 1', 'tag 2'));
 
         // Create a course that we are going to restore the other course to.
         $course2 = $this->getDataGenerator()->create_course();

diff --git a/question/format.php b/question/format.php
index 2f3744f..b2ddad2 100644 (file)
--- a/question/format.php
+++ b/question/format.php
@@ -425,9 +425,8 @@ class qformat_default {
 
             $result = question_bank::get_qtype($question->qtype)->save_question_options($question);
 
-            if (!empty($CFG->usetags) && isset($question->tags)) {
-                require_once($CFG->dirroot . '/tag/lib.php');
-                tag_set('question', $question->id, $question->tags, 'core_question', $question->context->id);
+            if (isset($question->tags)) {
+                core_tag_tag::set_item_tags('core_question', 'question', $question->context, $question->id, $question->tags);
             }
 
             if (!empty($result->error)) {

diff --git a/question/format/xml/format.php b/question/format/xml/format.php
index 8bcada5..efd209c 100644 (file)
--- a/question/format/xml/format.php
+++ b/question/format/xml/format.php
@@ -383,9 +383,9 @@ class qformat_xml extends qformat_default {
     public function import_question_tags($qo, $questionxml) {
         global $CFG;
 
-        if (!empty($CFG->usetags) && array_key_exists('tags', $questionxml['#'])
+        if (core_tag_tag::is_enabled('core_question', 'question')
+                && array_key_exists('tags', $questionxml['#'])
                 && !empty($questionxml['#']['tags'][0]['#']['tag'])) {
-            require_once($CFG->dirroot.'/tag/lib.php');
             $qo->tags = array();
             foreach ($questionxml['#']['tags'][0]['#']['tag'] as $tagdata) {
                 $qo->tags[] = $this->getpath($tagdata, array('#', 'text', 0, '#'), '', true);
@@ -1467,16 +1467,13 @@ class qformat_xml extends qformat_default {
         $expout .= $this->write_hints($question);
 
         // Write the question tags.
-        if (!empty($CFG->usetags)) {
-            require_once($CFG->dirroot.'/tag/lib.php');
-            $tags = tag_get_tags_array('question', $question->id);
-            if (!empty($tags)) {
-                $expout .= "    <tags>\n";
-                foreach ($tags as $tag) {
-                    $expout .= "      <tag>" . $this->writetext($tag, 0, true) . "</tag>\n";
-                }
-                $expout .= "    </tags>\n";
+        $tags = core_tag_tag::get_item_tags_array('core_question', 'question', $question->id);
+        if (!empty($tags)) {
+            $expout .= "    <tags>\n";
+            foreach ($tags as $tag) {
+                $expout .= "      <tag>" . $this->writetext($tag, 0, true) . "</tag>\n";
             }
+            $expout .= "    </tags>\n";
         }
 
         // Close the question tag.

diff --git a/question/format/xml/tests/xmlformat_test.php b/question/format/xml/tests/xmlformat_test.php
index e7c180d..8ed0271 100644 (file)
--- a/question/format/xml/tests/xmlformat_test.php
+++ b/question/format/xml/tests/xmlformat_test.php
@@ -29,7 +29,6 @@ global $CFG;
 require_once($CFG->libdir . '/questionlib.php');
 require_once($CFG->dirroot . '/question/format/xml/format.php');
 require_once($CFG->dirroot . '/question/engine/tests/helpers.php');
-require_once($CFG->dirroot . '/tag/lib.php');
 
 
 /**

diff --git a/question/question.php b/question/question.php
index 0e75f4d..c47be70 100644 (file)
--- a/question/question.php
+++ b/question/question.php
@@ -262,11 +262,9 @@ if ($mform->is_cancelled()) {
         }
     }
     $question = $qtypeobj->save_question($question, $fromform);
-    if (!empty($CFG->usetags) && isset($fromform->tags)) {
-        // A wizardpage from multipe pages questiontype like calculated may not
-        // allow editing the question tags, hence the isset($fromform->tags) test.
-        require_once($CFG->dirroot.'/tag/lib.php');
-        tag_set('question', $question->id, $fromform->tags, 'core_question', $contextid);
+    if (isset($fromform->tags)) {
+        core_tag_tag::set_item_tags('core_question', 'question', $question->id,
+                context::instance_by_id($contextid), $fromform->tags);
     }
 
     // Purge this question from the cache.

diff --git a/question/type/edit_question_form.php b/question/type/edit_question_form.php
index d0e3d27..636c858 100644 (file)
--- a/question/type/edit_question_form.php
+++ b/question/type/edit_question_form.php
@@ -201,10 +201,11 @@ abstract class question_edit_form extends question_wizard_form {
         // Any questiontype specific fields.
         $this->definition_inner($mform);
 
-        if (!empty($CFG->usetags)) {
+        if (core_tag_tag::is_enabled('core_question', 'question')) {
             $mform->addElement('header', 'tagsheader', get_string('tags'));
-            $mform->addElement('tags', 'tags', get_string('tags'));
         }
+        $mform->addElement('tags', 'tags', get_string('tags'),
+                array('itemtype' => 'question', 'component' => 'core_question'));
 
         if (!empty($this->question->id)) {
             $mform->addElement('header', 'createdmodifiedheader',