diff --git a/lang/en/question.php b/lang/en/question.php index 67a520d50d9..ead22db12b3 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -391,6 +391,7 @@ $string['questionbehavioursdisabledexplained'] = 'Enter a comma separated list o $string['questionbehavioursorder'] = 'Question behaviours order'; $string['questionbehavioursorderexplained'] = 'Enter a comma separated list of behaviours in the order you want them to appear in dropdown menu'; $string['questionidmismatch'] = 'Question ids mismatch'; +$string['questionformtagheader'] = '{$a} tags'; $string['questionname'] = 'Question name'; $string['questionnamecopy'] = '{$a} (copy)'; $string['questionpreviewdefaults'] = 'Question preview defaults'; diff --git a/lib/questionlib.php b/lib/questionlib.php index 98ef3c4320f..4266038f382 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -770,9 +770,11 @@ function question_load_questions($questionids, $extrafields = '', $join = '') { * Private function to factor common code out of get_question_options(). * * @param object $question the question to tidy. - * @param boolean $loadtags load the question tags from the tags table. Optional, default false. + * @param stdClass $category The question_categories record for the given $question. + * @param stdClass[]|null $tagobjects The tags for the given $question. + * @param stdClass[]|null $filtercourses The courses to filter the course tags by. */ -function _tidy_question($question, $loadtags = false) { +function _tidy_question($question, $category, array $tagobjects = null, array $filtercourses = null) { global $CFG; // Load question-type specific fields. @@ -790,8 +792,76 @@ function _tidy_question($question, $loadtags = false) { unset($question->_partiallyloaded); } - if ($loadtags && core_tag_tag::is_enabled('core_question', 'question')) { - $question->tags = core_tag_tag::get_item_tags_array('core_question', 'question', $question->id); + $question->categoryobject = $category; + + 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); + } } } @@ -804,17 +874,47 @@ function _tidy_question($question, $loadtags = false) { * * @param mixed $questions Either an array of question objects to be updated * or just a single question object - * @param boolean $loadtags load the question tags from the tags table. Optional, default false. + * @param bool $loadtags load the question tags from the tags table. Optional, default false. + * @param stdClass[] $filtercourses The courses to filter the course tags by. * @return bool Indicates success or failure. */ -function get_question_options(&$questions, $loadtags = false) { - if (is_array($questions)) { // deal with an array of questions - foreach ($questions as $i => $notused) { - _tidy_question($questions[$i], $loadtags); - } - } else { // deal with single question - _tidy_question($questions, $loadtags); +function get_question_options(&$questions, $loadtags = false, $filtercourses = null) { + global $DB; + + $questionlist = is_array($questions) ? $questions : [$questions]; + $categoryids = []; + $questionids = []; + + if (empty($questionlist)) { + return true; } + + foreach ($questionlist as $question) { + $questionids[] = $question->id; + + if (!in_array($question->category, $categoryids)) { + $categoryids[] = $question->category; + } + } + + $categories = $DB->get_records_list('question_categories', 'id', $categoryids); + + if ($loadtags && core_tag_tag::is_enabled('core_question', 'question')) { + $tagobjectsbyquestion = core_tag_tag::get_items_tags('core_question', 'question', $questionids); + } else { + $tagobjectsbyquestion = null; + } + + foreach ($questionlist as $question) { + if (is_null($tagobjectsbyquestion)) { + $tagobjects = null; + } else { + $tagobjects = $tagobjectsbyquestion[$question->id]; + } + + _tidy_question($question, $categories[$question->category], $tagobjects, $filtercourses); + } + return true; } diff --git a/question/question.php b/question/question.php index 984bc6cc58a..acbabcec399 100644 --- a/question/question.php +++ b/question/question.php @@ -120,7 +120,10 @@ if ($id) { if (!$question = $DB->get_record('question', array('id' => $id))) { print_error('questiondoesnotexist', 'question', $returnurl); } - get_question_options($question, true); + // We can use $COURSE here because it's been initialised as part of the + // require_login above. Passing it as the third parameter tells the function + // to filter the course tags by that course. + get_question_options($question, true, [$COURSE]); } else if ($categoryid && $qtype) { // only for creating new questions $question = new stdClass(); @@ -146,9 +149,13 @@ if ($id) { $qtypeobj = question_bank::get_qtype($question->qtype); -// Validate the question category. -if (!$category = $DB->get_record('question_categories', array('id' => $question->category))) { - print_error('categorydoesnotexist', 'question', $returnurl); +if (isset($question->categoryobject)) { + $category = $question->categoryobject; +} else { + // Validate the question category. + if (!$category = $DB->get_record('question_categories', array('id' => $question->category))) { + print_error('categorydoesnotexist', 'question', $returnurl); + } } // Check permissions @@ -261,10 +268,18 @@ if ($mform->is_cancelled()) { print_error('nopermissions', '', '', 'edit'); } } + $question = $qtypeobj->save_question($question, $fromform); if (isset($fromform->tags)) { + // If we have any question context level tags then set those tags now. core_tag_tag::set_item_tags('core_question', 'question', $question->id, - context::instance_by_id($contextid), $fromform->tags); + context::instance_by_id($contextid), $fromform->tags, 0); + } + + if (isset($fromform->coursetags)) { + // If we have and course context level tags then set those now. + core_tag_tag::set_item_tags('core_question', 'question', $question->id, + context_course::instance($fromform->courseid), $fromform->coursetags, 0); } // Purge this question from the cache. diff --git a/question/type/edit_question_form.php b/question/type/edit_question_form.php index 48f35dae6ea..c1e1666b28d 100644 --- a/question/type/edit_question_form.php +++ b/question/type/edit_question_form.php @@ -202,13 +202,7 @@ abstract class question_edit_form extends question_wizard_form { $this->definition_inner($mform); if (core_tag_tag::is_enabled('core_question', 'question')) { - $mform->addElement('header', 'tagsheader', get_string('tags')); - $mform->addElement('tags', 'tags', get_string('tags'), - array('itemtype' => 'question', 'component' => 'core_question')); - - if (!question_has_capability_on($this->question, 'tag')) { - $mform->hardFreeze('tags'); - } + $this->add_tag_fields($mform); } if (!empty($this->question->id)) { @@ -309,6 +303,53 @@ abstract class question_edit_form extends question_wizard_form { return $repeated; } + /** + * Add the tag and course tag fields to the mform. + * + * If the form is being built in a course context then add the field + * for course tags. + * + * If the question category doesn't belong to a course context or we + * aren't editing in a course context then add the tags element to allow + * tags to be added to the question category context. + * + * @param object $mform The form being built + */ + protected function add_tag_fields($mform) { + $hastagcapability = question_has_capability_on($this->question, 'tag'); + // Is the question category in a course context? + $qcontext = $this->categorycontext; + $qcoursecontext = $qcontext->get_course_context(false); + $iscourseoractivityquestion = !empty($qcoursecontext); + // Is the current context we're editing in a course context? + $editingcontext = $this->contexts->lowest(); + $editingcoursecontext = $editingcontext->get_course_context(false); + $iseditingcontextcourseoractivity = !empty($editingcoursecontext); + + $mform->addElement('header', 'tagsheader', get_string('tags')); + $mform->addElement('tags', 'tags', get_string('tags'), + array('itemtype' => 'question', 'component' => 'core_question')); + + if (!$hastagcapability) { + $mform->hardFreeze('tags'); + } + + if ($iseditingcontextcourseoractivity && !$iscourseoractivityquestion) { + // If the question is being edited in a course or activity context + // and the question isn't a course or activity level question then + // allow course tags to be added to the course. + $coursetagheader = get_string('questionformtagheader', 'core_question', + $editingcoursecontext->get_context_name(true)); + $mform->addElement('header', 'coursetagsheader', $coursetagheader); + $mform->addElement('tags', 'coursetags', get_string('tags'), + array('itemtype' => 'question', 'component' => 'core_question')); + + if (!$hastagcapability) { + $mform->hardFreeze('coursetags'); + } + } + } + /** * Add a set of form fields, obtained from get_per_answer_fields, to the form, * one for each existing answer, with some blanks for some new ones.