diff --git a/mod/quiz/classes/output/edit_renderer.php b/mod/quiz/classes/output/edit_renderer.php index 8d3a045ff05..021a19fb4a8 100644 --- a/mod/quiz/classes/output/edit_renderer.php +++ b/mod/quiz/classes/output/edit_renderer.php @@ -966,6 +966,7 @@ class edit_renderer extends \plugin_renderer_base { $question = $structure->get_question_in_slot($slotnumber); $slot = $structure->get_slot_by_number($slotnumber); + $slottags = $structure->get_slot_tags_for_slot_id($slot->id); $editurl = new \moodle_url('/mod/quiz/editrandom.php', array('returnurl' => $pageurl->out_as_local_url(), 'slotid' => $slot->id)); @@ -980,13 +981,19 @@ class edit_renderer extends \plugin_renderer_base { 'class' => 'icon activityicon', 'alt' => ' ', 'role' => 'presentation')); $editicon = $this->pix_icon('t/edit', $configuretitle, 'moodle', array('title' => '')); + $qbankurlparams = array( + 'cmid' => $structure->get_cmid(), + 'cat' => $question->category . ',' . $question->contextid, + 'recurse' => !empty($question->questiontext) + ); + + foreach ($slottags as $index => $slottag) { + $qbankurlparams["qtagids[{$index}]"] = $slottag->tagid; + } // If this is a random question, display a link to show the questions // selected from in the question bank. - $qbankurl = new \moodle_url('/question/edit.php', array( - 'cmid' => $structure->get_cmid(), - 'cat' => $question->category . ',' . $question->contextid, - 'recurse' => !empty($question->questiontext))); + $qbankurl = new \moodle_url('/question/edit.php', $qbankurlparams); $qbanklink = ' ' . \html_writer::link($qbankurl, get_string('seequestions', 'quiz'), array('class' => 'mod_quiz_random_qbank_link')); diff --git a/mod/quiz/classes/structure.php b/mod/quiz/classes/structure.php index 5f56a8adf1a..c1e915008f0 100644 --- a/mod/quiz/classes/structure.php +++ b/mod/quiz/classes/structure.php @@ -62,6 +62,14 @@ class structure { /** @var bool caches the results of can_be_edited. */ protected $canbeedited = null; + /** @var bool tracks whether tags have been loaded */ + protected $hasloadedtags = false; + + /** + * @var \stdClass[] the tags for slots. Indexed by slot id. + */ + protected $slottags = array(); + /** * Create an instance of this class representing an empty quiz. * @return structure @@ -1053,4 +1061,28 @@ class structure { } $DB->delete_records('quiz_sections', array('id' => $sectionid)); } + + /** + * Set up this class with the slot tags for each of the slots. + */ + protected function populate_slot_tags() { + $slotids = array_keys($this->slots); + $this->slottags = quiz_retrieve_tags_for_slot_ids($slotids); + } + + /** + * Retrieve the list of slot tags for the given slot id. + * + * @param int $slotid The id for the slot + * @return \stdClass[] The list of slot tag records + */ + public function get_slot_tags_for_slot_id($slotid) { + if (!$this->hasloadedtags) { + // Lazy load the tags just in case they are never required. + $this->populate_slot_tags(); + $this->hasloadedtags = true; + } + + return isset($this->slottags[$slotid]) ? $this->slottags[$slotid] : []; + } } diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 3761a46379b..a8c158b7169 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -2431,6 +2431,80 @@ function quiz_is_overriden_calendar_event(\calendar_event $event) { return $DB->record_exists('quiz_overrides', $overrideparams); } +/** + * Retrieves tag information for the given list of quiz slot ids. + * Currently the only slots that have tags are random question slots. + * + * Example: + * If we have 3 slots with id 1, 2, and 3. The first slot has two tags, the second + * has one tag, and the third has zero tags. The return structure will look like: + * [ + * 1 => [ + * { ...tag data... }, + * { ...tag data... }, + * ], + * 2 => [ + * { ...tag data... } + * ], + * 3 => [] + * ] + * + * @param int[] $slotids The list of id for the quiz slots. + * @return array[] List of quiz_slot_tags records indexed by slot id. + */ +function quiz_retrieve_tags_for_slot_ids($slotids) { + global $DB; + + if (empty($slotids)) { + return []; + } + + $slottags = $DB->get_records_list('quiz_slot_tags', 'slotid', $slotids); + $tagsbyid = core_tag_tag::get_bulk(array_filter(array_column($slottags, 'tagid')), 'id, name'); + $tagsbyname = false; // It will be loaded later if required. + $emptytagids = array_reduce($slotids, function($carry, $slotid) { + $carry[$slotid] = []; + return $carry; + }, []); + + return array_reduce( + $slottags, + function($carry, $slottag) use ($slottags, $tagsbyid, $tagsbyname) { + if (isset($tagsbyid[$slottag->tagid])) { + // Make sure that we're returning the most updated tag name. + $slottag->tagname = $tagsbyid[$slottag->tagid]->name; + } else { + if ($tagsbyname === false) { + // We were hoping that this query could be avoided, but life + // showed its other side to us! + $tagcollid = core_tag_area::get_collection('core', 'question'); + $tagsbyname = core_tag_tag::get_by_name_bulk( + $tagcollid, + array_column($slottags, 'tagname'), + 'id, name' + ); + } + if (isset($tagsbyname[$slottag->tagname])) { + // Make sure that we're returning the current tag id that matches + // the given tag name. + $slottag->tagid = $tagsbyname[$slottag->tagname]->id; + } else { + // The tag does not exist anymore (neither the tag id nor the tag name + // matches an existing tag). + // We still need to include this row in the result as some callers might + // be interested in these rows. An example is the editing forms that still + // need to display tag names even if they don't exist anymore. + $slottag->tagid = null; + } + } + + $carry[$slottag->slotid][] = $slottag; + return $carry; + }, + $emptytagids + ); +} + /** * Retrieves tag information for the given quiz slot. * A quiz slot have some tags if and only if it is representing a random question by tags. @@ -2439,37 +2513,8 @@ function quiz_is_overriden_calendar_event(\calendar_event $event) { * @return stdClass[] List of quiz_slot_tags records. */ function quiz_retrieve_slot_tags($slotid) { - global $DB; - - $slottags = $DB->get_records('quiz_slot_tags', ['slotid' => $slotid]); - - $tagsbyid = core_tag_tag::get_bulk(array_filter(array_column($slottags, 'tagid')), 'id, name'); - - $tagcollid = core_tag_area::get_collection('core', 'question'); - $tagsbyname = false; // It will be loaded later if required. - - foreach ($slottags as $slottag) { - if (isset($tagsbyid[$slottag->tagid])) { - $slottag->tagname = $tagsbyid[$slottag->tagid]->name; // Make sure that we're returning the most updated tag name. - } else { - if ($tagsbyname === false) { - // We were hoping that this query could be avoided, but life showed its other side to us! - $tagsbyname = core_tag_tag::get_by_name_bulk($tagcollid, array_column($slottags, 'tagname'), 'id, name'); - } - if (isset($tagsbyname[$slottag->tagname])) { - $slottag->tagid = $tagsbyname[$slottag->tagname]->id; // Make sure that we're returning the current tag id - // that matches the given tag name. - } else { - $slottag->tagid = null; // The tag does not exist anymore (neither the tag id nor the tag name - // matches an existing tag). - // We still need to include this row in the result as some callers might - // be interested in these rows. An example is the editing forms that still - // need to display tag names even if they don't exist anymore. - } - } - } - - return $slottags; + $slottags = quiz_retrieve_tags_for_slot_ids([$slotid]); + return $slottags[$slotid]; } /** diff --git a/mod/quiz/tests/locallib_test.php b/mod/quiz/tests/locallib_test.php index e5c396aa7c1..6dfb819fa41 100644 --- a/mod/quiz/tests/locallib_test.php +++ b/mod/quiz/tests/locallib_test.php @@ -585,4 +585,244 @@ class mod_quiz_locallib_testcase extends advanced_testcase { $this->assertEquals([], $tagids, '', 0.0, 10, true); } + + /** + * Data provider for the get_random_question_summaries test. + */ + public function get_quiz_retrieve_tags_for_slot_ids_test_cases() { + return [ + 'no questions' => [ + 'questioncount' => 0, + 'randomquestioncount' => 0, + 'randomquestiontags' => [], + 'unusedtags' => [], + 'removeslottagids' => [], + 'expected' => [] + ], + 'only regular questions' => [ + 'questioncount' => 2, + 'randomquestioncount' => 0, + 'randomquestiontags' => [], + 'unusedtags' => ['unused1', 'unused2'], + 'removeslottagids' => [], + 'expected' => [ + 1 => [], + 2 => [] + ] + ], + 'only random questions 1' => [ + 'questioncount' => 0, + 'randomquestioncount' => 2, + 'randomquestiontags' => [ + 0 => ['foo'], + 1 => [] + ], + 'unusedtags' => ['unused1', 'unused2'], + 'removeslottagids' => [], + 'expected' => [ + 1 => ['foo'], + 2 => [] + ] + ], + 'only random questions 2' => [ + 'questioncount' => 0, + 'randomquestioncount' => 2, + 'randomquestiontags' => [ + 0 => ['foo', 'bop'], + 1 => ['bar'] + ], + 'unusedtags' => ['unused1', 'unused2'], + 'removeslottagids' => [], + 'expected' => [ + 1 => ['foo', 'bop'], + 2 => ['bar'] + ] + ], + 'only random questions 3' => [ + 'questioncount' => 0, + 'randomquestioncount' => 2, + 'randomquestiontags' => [ + 0 => ['foo', 'bop'], + 1 => ['bar', 'foo'] + ], + 'unusedtags' => ['unused1', 'unused2'], + 'removeslottagids' => [], + 'expected' => [ + 1 => ['foo', 'bop'], + 2 => ['bar', 'foo'] + ] + ], + 'combination of questions 1' => [ + 'questioncount' => 2, + 'randomquestioncount' => 2, + 'randomquestiontags' => [ + 0 => ['foo'], + 1 => [] + ], + 'unusedtags' => ['unused1', 'unused2'], + 'removeslottagids' => [], + 'expected' => [ + 1 => [], + 2 => [], + 3 => ['foo'], + 4 => [] + ] + ], + 'combination of questions 2' => [ + 'questioncount' => 2, + 'randomquestioncount' => 2, + 'randomquestiontags' => [ + 0 => ['foo', 'bop'], + 1 => ['bar'] + ], + 'unusedtags' => ['unused1', 'unused2'], + 'removeslottagids' => [], + 'expected' => [ + 1 => [], + 2 => [], + 3 => ['foo', 'bop'], + 4 => ['bar'] + ] + ], + 'combination of questions 3' => [ + 'questioncount' => 2, + 'randomquestioncount' => 2, + 'randomquestiontags' => [ + 0 => ['foo', 'bop'], + 1 => ['bar', 'foo'] + ], + 'unusedtags' => ['unused1', 'unused2'], + 'removeslottagids' => [], + 'expected' => [ + 1 => [], + 2 => [], + 3 => ['foo', 'bop'], + 4 => ['bar', 'foo'] + ] + ], + 'load from name 1' => [ + 'questioncount' => 2, + 'randomquestioncount' => 2, + 'randomquestiontags' => [ + 0 => ['foo'], + 1 => [] + ], + 'unusedtags' => ['unused1', 'unused2'], + 'removeslottagids' => [3], + 'expected' => [ + 1 => [], + 2 => [], + 3 => ['foo'], + 4 => [] + ] + ], + 'load from name 2' => [ + 'questioncount' => 2, + 'randomquestioncount' => 2, + 'randomquestiontags' => [ + 0 => ['foo', 'bop'], + 1 => ['bar'] + ], + 'unusedtags' => ['unused1', 'unused2'], + 'removeslottagids' => [3], + 'expected' => [ + 1 => [], + 2 => [], + 3 => ['foo', 'bop'], + 4 => ['bar'] + ] + ], + 'load from name 3' => [ + 'questioncount' => 2, + 'randomquestioncount' => 2, + 'randomquestiontags' => [ + 0 => ['foo', 'bop'], + 1 => ['bar', 'foo'] + ], + 'unusedtags' => ['unused1', 'unused2'], + 'removeslottagids' => [3], + 'expected' => [ + 1 => [], + 2 => [], + 3 => ['foo', 'bop'], + 4 => ['bar', 'foo'] + ] + ] + ]; + } + + /** + * Test the quiz_retrieve_tags_for_slot_ids function with various parameter + * combinations. + * + * @dataProvider get_quiz_retrieve_tags_for_slot_ids_test_cases() + * @param int $questioncount The number of regular questions to create + * @param int $randomquestioncount The number of random questions to create + * @param array $randomquestiontags The tags for the random questions + * @param string[] $unusedtags Additional tags to create to populate the DB with data + * @param int[] $removeslottagids Slot numbers to remove tag ids for + * @param array $expected The expected output of tag names indexed by slot number + */ + public function test_quiz_retrieve_tags_for_slot_ids_combinations( + $questioncount, + $randomquestioncount, + $randomquestiontags, + $unusedtags, + $removeslottagids, + $expected + ) { + global $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + list($quiz, $tags) = $this->setup_quiz_and_tags( + $questioncount, + $randomquestioncount, + $randomquestiontags, + $unusedtags + ); + + $slots = $DB->get_records('quiz_slots', ['quizid' => $quiz->id]); + $slotids = []; + $slotsbynumber = []; + foreach ($slots as $slot) { + $slotids[] = $slot->id; + $slotsbynumber[$slot->slot] = $slot; + } + + if (!empty($removeslottagids)) { + // The slots to remove are the slot numbers not the slot id so we need + // to get the ids for the DB call. + $idstonull = array_map(function($slot) use ($slotsbynumber) { + return $slotsbynumber[$slot]->id; + }, $removeslottagids); + list($sql, $params) = $DB->get_in_or_equal($idstonull); + // Null out the tagid column to force the code to look up the tag by name. + $DB->set_field_select('quiz_slot_tags', 'tagid', null, "slotid {$sql}", $params); + } + + $slottagsbyslotids = quiz_retrieve_tags_for_slot_ids($slotids); + // Convert the result into an associative array of slotid => [... tag names..] + // to make it easier to compare. + $actual = array_map(function($slottags) { + $names = array_map(function($slottag) { + return $slottag->tagname; + }, $slottags); + // Make sure the names are sorted for comparison. + sort($names); + return $names; + }, $slottagsbyslotids); + + $formattedexptected = []; + // The expected values are indexed by slot number rather than id so let + // convert it to use the id so that we can compare the results. + foreach ($expected as $slot => $tagnames) { + sort($tagnames); + $slotid = $slotsbynumber[$slot]->id; + $formattedexptected[$slotid] = $tagnames; + } + + $this->assertEquals($formattedexptected, $actual); + } } diff --git a/mod/quiz/tests/structure_test.php b/mod/quiz/tests/structure_test.php index 96433d91930..f43d4c8c7c5 100644 --- a/mod/quiz/tests/structure_test.php +++ b/mod/quiz/tests/structure_test.php @@ -889,4 +889,148 @@ class mod_quiz_structure_testcase extends advanced_testcase { $structure = \mod_quiz\structure::create_for_quiz($quizobj); $this->assertEquals(0, $structure->is_question_dependent_on_previous_slot(2)); } + + /** + * Data provider for the get_slot_tags_for_slot test. + */ + public function get_slot_tags_for_slot_test_cases() { + return [ + 'incorrect slot id' => [ + 'layout' => [ + ['TF1', 1, 'truefalse'], + ['TF2', 1, 'truefalse'], + ['TF3', 1, 'truefalse'] + ], + 'tagnames' => [ + ['foo'], + ['bar'], + ['baz'] + ], + 'slotnumber' => null, + 'expected' => [] + ], + 'no tags' => [ + 'layout' => [ + ['TF1', 1, 'truefalse'], + ['TF2', 1, 'truefalse'], + ['TF3', 1, 'truefalse'] + ], + 'tagnames' => [ + ['foo'], + [], + ['baz'] + ], + 'slotnumber' => 2, + 'expected' => [] + ], + 'one tag 1' => [ + 'layout' => [ + ['TF1', 1, 'truefalse'], + ['TF2', 1, 'truefalse'], + ['TF3', 1, 'truefalse'] + ], + 'tagnames' => [ + ['foo'], + ['bar'], + ['baz'] + ], + 'slotnumber' => 1, + 'expected' => ['foo'] + ], + 'one tag 2' => [ + 'layout' => [ + ['TF1', 1, 'truefalse'], + ['TF2', 1, 'truefalse'], + ['TF3', 1, 'truefalse'] + ], + 'tagnames' => [ + ['foo'], + ['bar'], + ['baz'] + ], + 'slotnumber' => 2, + 'expected' => ['bar'] + ], + 'multiple tags 1' => [ + 'layout' => [ + ['TF1', 1, 'truefalse'], + ['TF2', 1, 'truefalse'], + ['TF3', 1, 'truefalse'] + ], + 'tagnames' => [ + ['foo', 'bar'], + ['bar'], + ['baz'] + ], + 'slotnumber' => 1, + 'expected' => ['foo', 'bar'] + ], + 'multiple tags 2' => [ + 'layout' => [ + ['TF1', 1, 'truefalse'], + ['TF2', 1, 'truefalse'], + ['TF3', 1, 'truefalse'] + ], + 'tagnames' => [ + ['foo', 'bar'], + ['bar', 'baz'], + ['baz'] + ], + 'slotnumber' => 2, + 'expected' => ['bar', 'baz'] + ] + ]; + } + + /** + * @dataProvider get_slot_tags_for_slot_test_cases() + * @param array $layout Quiz layout for create_test_quiz function + * @param array $tagnames Tags to create for each question slot + * @param int $slotnumber The slot number to select tags from + * @param string[] $expected The tags expected for the given $slotnumber + */ + public function test_get_slot_tags_for_slot($layout, $tagnames, $slotnumber, $expected) { + global $DB; + $this->resetAfterTest(); + + $quiz = $this->create_test_quiz($layout); + $structure = \mod_quiz\structure::create_for_quiz($quiz); + $collid = core_tag_area::get_collection('core', 'question'); + $slottagrecords = []; + + if (is_null($slotnumber)) { + // Null slot number means to create a non-existent slot id. + $slot = $structure->get_last_slot(); + $slotid = $slot->id + 100; + } else { + $slot = $structure->get_slot_by_number($slotnumber); + $slotid = $slot->id; + } + + foreach ($tagnames as $index => $slottagnames) { + $tagslotnumber = $index + 1; + $tagslotid = $structure->get_slot_id_for_slot($tagslotnumber); + $tags = core_tag_tag::create_if_missing($collid, $slottagnames); + $records = array_map(function($tag) use ($tagslotid) { + return (object) [ + 'slotid' => $tagslotid, + 'tagid' => $tag->id, + 'tagname' => $tag->name + ]; + }, array_values($tags)); + $slottagrecords = array_merge($slottagrecords, $records); + } + + $DB->insert_records('quiz_slot_tags', $slottagrecords); + + $actualslottags = $structure->get_slot_tags_for_slot_id($slotid); + $actual = array_map(function($slottag) { + return $slottag->tagname; + }, $actualslottags); + + sort($expected); + sort($actual); + + $this->assertEquals($expected, $actual); + } }