Merge branch 'MDL-61410-master' of git://github.com/lameze/moodle

This commit is contained in:
Eloy Lafuente (stronk7) 2018-03-20 23:53:28 +01:00
commit 9a79102a4c
7 changed files with 309 additions and 92 deletions

View File

@ -921,72 +921,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;
}
}
@ -1043,6 +982,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
*

View File

@ -1352,4 +1352,150 @@ class core_questionlib_testcase extends advanced_testcase {
}
}
}
/**
* question_sort_tags() includes the tags for all questions in the list.
*/
public function test_question_sort_tags_includes_question_tags() {
list($category, $course, $quiz, $qcat, $questions) = $this->setup_quiz_and_questions('category');
$question1 = $questions[0];
$question2 = $questions[1];
$qcontext = context::instance_by_id($qcat->contextid);
core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $qcontext, ['foo', 'bar']);
core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $qcontext, ['baz', 'bop']);
foreach ($questions as $question) {
$tags = core_tag_tag::get_item_tags('core_question', 'question', $question->id);
$categorycontext = context::instance_by_id($qcat->contextid);
$tagobjects = question_sort_tags($tags, $categorycontext);
$expectedtags = [];
$actualtags = $tagobjects->tags;
foreach ($tagobjects->tagobjects as $tag) {
$expectedtags[$tag->id] = $tag->name;
}
// The question should have a tags property populated with each tag id
// and display name as a key vale pair.
$this->assertEquals($expectedtags, $actualtags);
$actualtagobjects = $tagobjects->tagobjects;
sort($tags);
sort($actualtagobjects);
// The question should have a full set of each tag object.
$this->assertEquals($tags, $actualtagobjects);
// The question should not have any course tags.
$this->assertEmpty($tagobjects->coursetagobjects);
}
}
/**
* question_sort_tags() includes course tags for all questions in the list.
*/
public function test_question_sort_tags_includes_question_course_tags() {
global $DB;
list($category, $course, $quiz, $qcat, $questions) = $this->setup_quiz_and_questions('category');
$question1 = $questions[0];
$question2 = $questions[1];
$coursecontext = context_course::instance($course->id);
core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $coursecontext, ['foo', 'bar']);
core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $coursecontext, ['baz', 'bop']);
foreach ($questions as $question) {
$tags = core_tag_tag::get_item_tags('core_question', 'question', $question->id);
$tagobjects = question_sort_tags($tags, $qcat);
$expectedtags = [];
$actualtags = $tagobjects->coursetags;
foreach ($actualtags as $coursetagid => $coursetagname) {
$expectedtags[$coursetagid] = $coursetagname;
}
// The question should have a tags property populated with each tag id
// and display name as a key vale pair.
$this->assertEquals($expectedtags, $actualtags);
$actualtagobjects = $tagobjects->coursetagobjects;
sort($tags);
sort($actualtagobjects);
// The question should have a full set of each tag object.
$this->assertEquals($tags, $actualtagobjects);
// The question should not have any course tags.
$this->assertEmpty($tagobjects->tagobjects);
}
}
/**
* question_sort_tags() should return tags from all course contexts by default.
*/
public function test_question_sort_tags_includes_multiple_courses_tags() {
global $DB;
list($category, $course, $quiz, $qcat, $questions) = $this->setup_quiz_and_questions('category');
$question1 = $questions[0];
$question2 = $questions[1];
$coursecontext = context_course::instance($course->id);
// Create a sibling course.
$siblingcourse = $this->getDataGenerator()->create_course(['category' => $course->category]);
$siblingcoursecontext = context_course::instance($siblingcourse->id);
// Create course tags.
core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $coursecontext, ['c1']);
core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $coursecontext, ['c1']);
core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $siblingcoursecontext, ['c2']);
core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $siblingcoursecontext, ['c2']);
foreach ($questions as $question) {
$tags = core_tag_tag::get_item_tags('core_question', 'question', $question->id);
$tagobjects = question_sort_tags($tags, $qcat);
$this->assertCount(2, $tagobjects->coursetagobjects);
foreach ($tagobjects->coursetagobjects as $tag) {
if ($tag->name == 'c1') {
$this->assertEquals($coursecontext->id, $tag->taginstancecontextid);
} else {
$this->assertEquals($siblingcoursecontext->id, $tag->taginstancecontextid);
}
}
}
}
/**
* question_sort_tags() should filter the course tags by the given list of courses.
*/
public function test_question_sort_tags_includes_filter_course_tags() {
global $DB;
list($category, $course, $quiz, $qcat, $questions) = $this->setup_quiz_and_questions('category');
$question1 = $questions[0];
$question2 = $questions[1];
$coursecontext = context_course::instance($course->id);
// Create a sibling course.
$siblingcourse = $this->getDataGenerator()->create_course(['category' => $course->category]);
$siblingcoursecontext = context_course::instance($siblingcourse->id);
// Create course tags.
core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $coursecontext, ['foo']);
core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $coursecontext, ['bar']);
// Create sibling course tags. These should be filtered out.
core_tag_tag::set_item_tags('core_question', 'question', $question1->id, $siblingcoursecontext, ['filtered1']);
core_tag_tag::set_item_tags('core_question', 'question', $question2->id, $siblingcoursecontext, ['filtered2']);
foreach ($questions as $question) {
$tags = core_tag_tag::get_item_tags('core_question', 'question', $question->id);
$tagobjects = question_sort_tags($tags, $qcat, [$course]);
foreach ($tagobjects->coursetagobjects as $tag) {
// We should only be seeing course tags from $course. The tags from
// $siblingcourse should have been filtered out.
$this->assertEquals($coursecontext->id, $tag->taginstancecontextid);
}
}
}
}

View File

@ -51,7 +51,6 @@ class qformat_default {
public $stoponerror = true;
public $translator = null;
public $canaccessbackupdata = true;
protected $importcontext = null;
// functions to indicate import/export functionality
@ -425,8 +424,31 @@ class qformat_default {
$result = question_bank::get_qtype($question->qtype)->save_question_options($question);
if (isset($question->tags)) {
core_tag_tag::set_item_tags('core_question', 'question', $question->id, $question->context, $question->tags);
if (core_tag_tag::is_enabled('core_question', 'question')) {
// Is the current context we're importing in a course context?
$importingcontext = $this->importcontext;
$importingcoursecontext = $importingcontext->get_course_context(false);
$isimportingcontextcourseoractivity = !empty($importingcoursecontext);
if (!empty($question->coursetags)) {
if ($isimportingcontextcourseoractivity) {
$mergedtags = array_merge($question->coursetags, $question->tags);
core_tag_tag::set_item_tags('core_question', 'question', $question->id,
$question->context, $mergedtags);
} else {
core_tag_tag::set_item_tags('core_question', 'question', $question->id,
context_course::instance($this->course->id), $question->coursetags);
if (!empty($question->tags)) {
core_tag_tag::set_item_tags('core_question', 'question', $question->id,
$importingcontext, $question->tags);
}
}
} else {
core_tag_tag::set_item_tags('core_question', 'question', $question->id,
$question->context, $question->tags);
}
}
if (!empty($result->error)) {

View File

@ -383,12 +383,20 @@ class qformat_xml extends qformat_default {
public function import_question_tags($qo, $questionxml) {
global $CFG;
if (core_tag_tag::is_enabled('core_question', 'question')
&& array_key_exists('tags', $questionxml['#'])
&& !empty($questionxml['#']['tags'][0]['#']['tag'])) {
$qo->tags = array();
foreach ($questionxml['#']['tags'][0]['#']['tag'] as $tagdata) {
$qo->tags[] = $this->getpath($tagdata, array('#', 'text', 0, '#'), '', true);
if (core_tag_tag::is_enabled('core_question', 'question')) {
$qo->tags = [];
if (!empty($questionxml['#']['tags'][0]['#']['tag'])) {
foreach ($questionxml['#']['tags'][0]['#']['tag'] as $tagdata) {
$qo->tags[] = $this->getpath($tagdata, array('#', 'text', 0, '#'), '', true);
}
}
$qo->coursetags = [];
if (!empty($questionxml['#']['coursetags'][0]['#']['tag'])) {
foreach ($questionxml['#']['coursetags'][0]['#']['tag'] as $tagdata) {
$qo->coursetags[] = $this->getpath($tagdata, array('#', 'text', 0, '#'), '', true);
}
}
}
}
@ -1467,13 +1475,30 @@ class qformat_xml extends qformat_default {
$expout .= $this->write_hints($question);
// Write the question tags.
$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";
if (core_tag_tag::is_enabled('core_question', 'question')) {
$tagobjects = core_tag_tag::get_item_tags('core_question', 'question', $question->id);
if (!empty($tagobjects)) {
$context = context::instance_by_id($contextid);
$sortedtagobjects = question_sort_tags($tagobjects, $context, [$this->course]);
if (!empty($sortedtagobjects->coursetags)) {
// Set them on the form to be rendered as existing tags.
$expout .= " <coursetags>\n";
foreach ($sortedtagobjects->coursetags as $coursetag) {
$expout .= " <tag>" . $this->writetext($coursetag, 0, true) . "</tag>\n";
}
$expout .= " </coursetags>\n";
}
if (!empty($sortedtagobjects->tags)) {
$expout .= " <tags>\n";
foreach ($sortedtagobjects->tags as $tag) {
$expout .= " <tag>" . $this->writetext($tag, 0, true) . "</tag>\n";
}
$expout .= " </tags>\n";
}
}
$expout .= " </tags>\n";
}
// Close the question tag.

View File

@ -147,6 +147,7 @@ class qformat_xml_test extends question_testcase {
public function test_write_hint_basic() {
$q = $this->make_test_question();
$q->contextid = \context_system::instance()->id;
$q->name = 'Short answer question';
$q->questiontext = 'Name an amphibian: __________';
$q->generalfeedback = 'Generalfeedback: frog or toad would have been OK.';
@ -176,6 +177,7 @@ class qformat_xml_test extends question_testcase {
public function test_write_hint_with_parts() {
$q = $this->make_test_question();
$q->contextid = \context_system::instance()->id;
$q->name = 'Matching question';
$q->questiontext = 'Classify the animals.';
$q->generalfeedback = 'Frogs and toads are amphibians, the others are mammals.';
@ -329,7 +331,7 @@ END;
public function test_export_description() {
$qdata = new stdClass();
$qdata->id = 123;
$qdata->contextid = 0;
$qdata->contextid = \context_system::instance()->id;
$qdata->qtype = 'description';
$qdata->name = 'A description';
$qdata->questiontext = 'The question text.';
@ -474,7 +476,7 @@ END;
public function test_export_essay() {
$qdata = new stdClass();
$qdata->id = 123;
$qdata->contextid = 0;
$qdata->contextid = \context_system::instance()->id;
$qdata->qtype = 'essay';
$qdata->name = 'An essay';
$qdata->questiontext = 'Write something.';
@ -636,7 +638,7 @@ END;
public function test_export_match() {
$qdata = new stdClass();
$qdata->id = 123;
$qdata->contextid = 0;
$qdata->contextid = \context_system::instance()->id;
$qdata->qtype = 'match';
$qdata->name = 'Matching question';
$qdata->questiontext = 'Match the upper and lower case letters.';
@ -867,7 +869,7 @@ END;
public function test_export_multichoice() {
$qdata = new stdClass();
$qdata->id = 123;
$qdata->contextid = 0;
$qdata->contextid = \context_system::instance()->id;
$qdata->qtype = 'multichoice';
$qdata->name = 'Multiple choice question';
$qdata->questiontext = 'Which are the even numbers?';
@ -1040,7 +1042,7 @@ END;
$qdata = new stdClass();
$qdata->id = 123;
$qdata->contextid = 0;
$qdata->contextid = \context_system::instance()->id;
$qdata->qtype = 'numerical';
$qdata->name = 'Numerical question';
$qdata->questiontext = 'What is the answer?';
@ -1170,7 +1172,7 @@ END;
public function test_export_shortanswer() {
$qdata = new stdClass();
$qdata->id = 123;
$qdata->contextid = 0;
$qdata->contextid = \context_system::instance()->id;
$qdata->qtype = 'shortanswer';
$qdata->name = 'Short answer question';
$qdata->questiontext = 'Fill in the gap in this sequence: Alpha, ________, Gamma.';
@ -1291,7 +1293,7 @@ END;
public function test_export_truefalse() {
$qdata = new stdClass();
$qdata->id = 12;
$qdata->contextid = 0;
$qdata->contextid = \context_system::instance()->id;
$qdata->qtype = 'truefalse';
$qdata->name = 'True false question';
$qdata->questiontext = 'The answer is true.';
@ -1455,7 +1457,7 @@ END;
public function test_export_multianswer() {
$qdata = test_question_maker::get_question_data('multianswer', 'twosubq');
$qdata->contextid = \context_system::instance()->id;
$exporter = new qformat_xml();
$xml = $exporter->writequestion($qdata);
@ -1486,7 +1488,7 @@ END;
public function test_export_multianswer_withdollars() {
$qdata = test_question_maker::get_question_data('multianswer', 'dollarsigns');
$qdata->contextid = \context_system::instance()->id;
$exporter = new qformat_xml();
$xml = $exporter->writequestion($qdata);

View File

@ -439,7 +439,7 @@ class qtype_ddwtos_test extends question_testcase {
public function test_xml_export() {
$qdata = new stdClass();
$qdata->id = 123;
$qdata->contextid = 0;
$qdata->contextid = \context_system::instance()->id;
$qdata->qtype = 'ddwtos';
$qdata->name = 'A drag-and-drop question';
$qdata->questiontext = 'Put these in order: [[1]], [[2]], [[3]].';

View File

@ -242,7 +242,7 @@ class qtype_gapselect_test extends question_testcase {
public function test_xml_export() {
$qdata = new stdClass();
$qdata->id = 123;
$qdata->contextid = 0;
$qdata->contextid = \context_system::instance()->id;
$qdata->qtype = 'gapselect';
$qdata->name = 'A select missing words question';
$qdata->questiontext = 'Put these in order: [[1]], [[2]], [[3]].';