MDL-61410 question: move tag sorting logic to a new function
authorSimey Lameze <simey@moodle.com>
Fri, 2 Mar 2018 00:24:00 +0000 (08:24 +0800)
committerSimey Lameze <simey@moodle.com>
Mon, 19 Mar 2018 03:45:55 +0000 (11:45 +0800)
lib/questionlib.php

index eb99d4f..ff1401f 100644 (file)
@@ -916,72 +916,11 @@ function _tidy_question($question, $category, array $tagobjects = null, array $f
 
     if (!is_null($tagobjects)) {
         $categorycontext = context::instance_by_id($category->contextid);
-
-        // Questions can have two sets of tag instances. One set at the
-        // course context level and another at the context the question
-        // belongs to (e.g. course category, system etc).
-        $question->coursetagobjects = [];
-        $question->coursetags = [];
-        $question->tagobjects = [];
-        $question->tags = [];
-        $taginstanceidstonormalise = [];
-        $filtercoursecontextids = [];
-        $hasfiltercourses = !empty($filtercourses);
-
-        if ($hasfiltercourses) {
-            // If we're being asked to filter the course tags by a set of courses
-            // then get the context ids to filter below.
-            $filtercoursecontextids = array_map(function($course) {
-                $coursecontext = context_course::instance($course->id);
-                return $coursecontext->id;
-            }, $filtercourses);
-        }
-
-        foreach ($tagobjects as $tagobject) {
-            $tagcontextid = $tagobject->taginstancecontextid;
-            $tagcontext = context::instance_by_id($tagcontextid);
-            $tagcoursecontext = $tagcontext->get_course_context(false);
-            // This is a course tag if the tag context is a course context which
-            // doesn't match the question's context. Any tag in the question context
-            // is not considered a course tag, it belongs to the question.
-            $iscoursetag = $tagcoursecontext
-                && $tagcontext->id == $tagcoursecontext->id
-                && $tagcontext->id != $categorycontext->id;
-
-            if ($iscoursetag) {
-                // Any tag instance in a course context level is considered a course tag.
-                if (!$hasfiltercourses || in_array($tagcontextid, $filtercoursecontextids)) {
-                    // Add the tag to the list of course tags if we aren't being
-                    // asked to filter or if this tag is in the list of courses
-                    // we're being asked to filter by.
-                    $question->coursetagobjects[] = $tagobject;
-                    $question->coursetags[$tagobject->id] = $tagobject->get_display_name();
-                }
-            } else {
-                // All non course context level tag instances or tags in the question
-                // context belong to the context that the question was created in.
-                $question->tagobjects[] = $tagobject;
-                $question->tags[$tagobject->id] = $tagobject->get_display_name();
-
-                // Due to legacy tag implementations that don't force the recording
-                // of a context id, some tag instances may have context ids that don't
-                // match either a course context or the question context. In this case
-                // we should take the opportunity to fix up the data and set the correct
-                // context id.
-                if ($tagcontext->id != $categorycontext->id) {
-                    $taginstanceidstonormalise[] = $tagobject->taginstanceid;
-                    // Update the object properties to reflect the DB update that will
-                    // happen below.
-                    $tagobject->taginstancecontextid = $categorycontext->id;
-                }
-            }
-        }
-
-        if (!empty($taginstanceidstonormalise)) {
-            // If we found any tag instances with incorrect context id data then we can
-            // correct those values now by setting them to the question context id.
-            core_tag_tag::change_instances_context($taginstanceidstonormalise, $categorycontext);
-        }
+        $sortedtagobjects = question_sort_tags($tagobjects, $categorycontext, $filtercourses);
+        $question->coursetagobjects = $sortedtagobjects->coursetagobjects;
+        $question->coursetags = $sortedtagobjects->coursetags;
+        $question->tagobjects = $sortedtagobjects->tagobjects;
+        $question->tags = $sortedtagobjects->tags;
     }
 }
 
@@ -1038,6 +977,89 @@ function get_question_options(&$questions, $loadtags = false, $filtercourses = n
     return true;
 }
 
+/**
+ * Sort question tags by course or normal tags.
+ *
+ * This function also search tag instances that may have a context id that don't match either a course or
+ * question context and fix the data setting the correct context id.
+ *
+ * @param stdClass[] $tagobjects The tags for the given $question.
+ * @param stdClass $categorycontext The question categories context.
+ * @param stdClass[]|null $filtercourses The courses to filter the course tags by.
+ * @return stdClass $sortedtagobjects Sorted tag objects.
+ */
+function question_sort_tags($tagobjects, $categorycontext, $filtercourses = null) {
+
+    // Questions can have two sets of tag instances. One set at the
+    // course context level and another at the context the question
+    // belongs to (e.g. course category, system etc).
+    $sortedtagobjects = new stdClass();
+    $sortedtagobjects->coursetagobjects = [];
+    $sortedtagobjects->coursetags = [];
+    $sortedtagobjects->tagobjects = [];
+    $sortedtagobjects->tags = [];
+    $taginstanceidstonormalise = [];
+    $filtercoursecontextids = [];
+    $hasfiltercourses = !empty($filtercourses);
+
+    if ($hasfiltercourses) {
+        // If we're being asked to filter the course tags by a set of courses
+        // then get the context ids to filter below.
+        $filtercoursecontextids = array_map(function($course) {
+            $coursecontext = context_course::instance($course->id);
+            return $coursecontext->id;
+        }, $filtercourses);
+    }
+
+    foreach ($tagobjects as $tagobject) {
+        $tagcontextid = $tagobject->taginstancecontextid;
+        $tagcontext = context::instance_by_id($tagcontextid);
+        $tagcoursecontext = $tagcontext->get_course_context(false);
+        // This is a course tag if the tag context is a course context which
+        // doesn't match the question's context. Any tag in the question context
+        // is not considered a course tag, it belongs to the question.
+        $iscoursetag = $tagcoursecontext
+            && $tagcontext->id == $tagcoursecontext->id
+            && $tagcontext->id != $categorycontext->id;
+
+        if ($iscoursetag) {
+            // Any tag instance in a course context level is considered a course tag.
+            if (!$hasfiltercourses || in_array($tagcontextid, $filtercoursecontextids)) {
+                // Add the tag to the list of course tags if we aren't being
+                // asked to filter or if this tag is in the list of courses
+                // we're being asked to filter by.
+                $sortedtagobjects->coursetagobjects[] = $tagobject;
+                $sortedtagobjects->coursetags[$tagobject->id] = $tagobject->get_display_name();
+            }
+        } else {
+            // All non course context level tag instances or tags in the question
+            // context belong to the context that the question was created in.
+            $sortedtagobjects->tagobjects[] = $tagobject;
+            $sortedtagobjects->tags[$tagobject->id] = $tagobject->get_display_name();
+
+            // Due to legacy tag implementations that don't force the recording
+            // of a context id, some tag instances may have context ids that don't
+            // match either a course context or the question context. In this case
+            // we should take the opportunity to fix up the data and set the correct
+            // context id.
+            if ($tagcontext->id != $categorycontext->id) {
+                $taginstanceidstonormalise[] = $tagobject->taginstanceid;
+                // Update the object properties to reflect the DB update that will
+                // happen below.
+                $tagobject->taginstancecontextid = $categorycontext->id;
+            }
+        }
+    }
+
+    if (!empty($taginstanceidstonormalise)) {
+        // If we found any tag instances with incorrect context id data then we can
+        // correct those values now by setting them to the question context id.
+        core_tag_tag::change_instances_context($taginstanceidstonormalise, $categorycontext);
+    }
+
+    return $sortedtagobjects;
+}
+
 /**
  * Print the icon for the question type
  *