diff --git a/mod/quiz/editrandom.php b/mod/quiz/editrandom.php index 9e32148330b..f1e94554bec 100644 --- a/mod/quiz/editrandom.php +++ b/mod/quiz/editrandom.php @@ -149,7 +149,8 @@ if ($mform->is_cancelled()) { // Now, delete the remaining records. if (!empty($recordstokeep)) { list($select, $params) = $DB->get_in_or_equal($recordstokeep, SQL_PARAMS_QM, 'param', false); - $DB->delete_records_select('quiz_slot_tags', "id $select", $params); + array_unshift($params, $slot->id); + $DB->delete_records_select('quiz_slot_tags', "slotid = ? AND id $select", $params); } else { $DB->delete_records('quiz_slot_tags', array('slotid' => $slot->id)); } diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index b572b633b24..a90a3575c42 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -2522,13 +2522,13 @@ function quiz_is_overriden_calendar_event(\calendar_event $event) { * has one tag, and the third has zero tags. The return structure will look like: * [ * 1 => [ - * { ...tag data... }, - * { ...tag data... }, + * quiz_slot_tags.id => { ...tag data... }, + * quiz_slot_tags.id => { ...tag data... }, * ], * 2 => [ - * { ...tag data... } + * quiz_slot_tags.id => { ...tag data... }, * ], - * 3 => [] + * 3 => [], * ] * * @param int[] $slotids The list of id for the quiz slots. @@ -2580,7 +2580,7 @@ function quiz_retrieve_tags_for_slot_ids($slotids) { } } - $carry[$slottag->slotid][] = $slottag; + $carry[$slottag->slotid][$slottag->id] = $slottag; return $carry; }, $emptytagids diff --git a/mod/quiz/tests/behat/editing_add_random.feature b/mod/quiz/tests/behat/editing_add_random.feature index 65c6a57f1d7..d47e990c73c 100644 --- a/mod/quiz/tests/behat/editing_add_random.feature +++ b/mod/quiz/tests/behat/editing_add_random.feature @@ -18,33 +18,34 @@ Feature: Adding random questions to a quiz based on category and tags | activity | name | intro | course | idnumber | | quiz | Quiz 1 | Quiz 1 for testing the Add random question form | C1 | quiz1 | And the following "question categories" exist: - | contextlevel | reference | name | - | Course | C1 | Questions Category 1| - | Course | C1 | Questions Category 2| + | contextlevel | reference | name | + | Course | C1 | Questions Category 1 | + | Course | C1 | Questions Category 2 | And the following "questions" exist: | questioncategory | qtype | name | user | questiontext | | Questions Category 1 | essay | question 1 name | admin | Question 1 text | | Questions Category 1 | essay | question 2 name | teacher1 | Question 2 text | + And the following "core_question > Tags" exist: + | question | tag | + | question 1 name | foo | + | question 2 name | bar | Scenario: Available tags are shown in the autocomplete tag field - Given I log in as "teacher1" - And I am on "Course 1" course homepage - When I navigate to "Question bank > Questions" in current page administration - And I choose "Edit question" action for "question 1 name" in the question bank - And I set the following fields to these values: - | Tags | foo | - And I press "id_submitbutton" - And I choose "Manage tags" action for "question 2 name" in the question bank - And I set the following fields in the "Question tags" "dialogue" to these values: - | Tags | bar | - And I press "Save changes" - And I am on the "Quiz 1" "mod_quiz > Edit" page - And I open the "last" add to quiz menu + Given I am on the "Quiz 1" "mod_quiz > Edit" page logged in as "teacher1" + When I open the "last" add to quiz menu And I follow "a random question" And I open the autocomplete suggestions list Then "foo" "autocomplete_suggestions" should exist And "bar" "autocomplete_suggestions" should exist + Scenario: A random question can be added to the quiz + Given I am on the "Quiz 1" "mod_quiz > Edit" page logged in as "teacher1" + When I open the "last" add to quiz menu + And I follow "a random question" + And I set the field "Tags" to "foo" + And I press "Add random question" + Then I should see "Random (Questions Category 1, tags: foo)" on quiz page "1" + Scenario: Teacher without moodle/question:useall should not see the add a random question menu item Given the following "permission overrides" exist: | capability | permission | role | contextlevel | reference | diff --git a/mod/quiz/tests/behat/editing_edit_random.feature b/mod/quiz/tests/behat/editing_edit_random.feature new file mode 100644 index 00000000000..d853b4ee0de --- /dev/null +++ b/mod/quiz/tests/behat/editing_edit_random.feature @@ -0,0 +1,55 @@ +@mod @mod_quiz @javascript +Feature: Editing random questions already in a quiz based on category and tags + In order to have better assessment + As a teacher + I want to be able to update how questions are randomly picked from the question bank + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | t1@example.com | + And the following "courses" exist: + | fullname | shortname | + | Course 1 | C1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + And the following "activities" exist: + | activity | name | intro | course | idnumber | + | quiz | Quiz 1 | Quiz 1 for testing the Add random question form | C1 | quiz1 | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Questions Category 1| + | Course | C1 | Questions Category 2| + And the following "questions" exist: + | questioncategory | qtype | name | user | questiontext | + | Questions Category 1 | essay | question 1 name | admin | Question 1 text | + | Questions Category 1 | essay | question 2 name | teacher1 | Question 2 text | + And the following "core_question > Tags" exist: + | question | tag | + | question 1 name | easy | + | question 1 name | essay | + | question 2 name | hard | + | question 2 name | essay | + + Scenario: Editing tags on one slot does not delete the rest + Given I am on the "Quiz 1" "mod_quiz > Edit" page logged in as "teacher1" + And I open the "last" add to quiz menu + And I follow "a random question" + # To actually reproduce MDL-68733 it would be better to set tags easy,essay here, and then below just delete one tag. + # However, the state of Behat for autocomplete fields does not let us actually do that. + And I set the field "Tags" to "easy" + And I press "Add random question" + And I open the "Page 1" add to quiz menu + And I follow "a random question" + And I set the field "Tags" to "hard" + And I press "Add random question" + And I follow "Add page break" + When I click on "Configure question" "link" in the "Random (Questions Category 1, tags: easy)" "list_item" + And I click on "easy" "autocomplete_selection" + And I set the field "Tags" to "essay" + And I press "Save changes" + Then I should see "Random (Questions Category 1, tags: essay)" on quiz page "1" + And I should see "Random (Questions Category 1, tags: hard)" on quiz page "2" + And I click on "Configure question" "link" in the "Questions Category 1, tags: hard" "list_item" + And "hard" "autocomplete_selection" should be visible diff --git a/question/tests/generator/behat_core_question_generator.php b/question/tests/generator/behat_core_question_generator.php new file mode 100644 index 00000000000..0dbd430591b --- /dev/null +++ b/question/tests/generator/behat_core_question_generator.php @@ -0,0 +1,60 @@ +. + +/** + * Behat data generator for core_question. + * + * @package core_question + * @category test + * @copyright 2020 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + + +/** + * Behat data generator for core_question. + */ +class behat_core_question_generator extends behat_generator_base { + + protected function get_creatable_entities(): array { + // Note, for historical reasons, questions and question categories + // are generated by behat_core_generator. + return [ + 'Tags' => [ + 'datagenerator' => 'question_tag', + 'required' => ['question', 'tag'], + 'switchids' => ['question' => 'questionid'], + ], + ]; + } + + /** + * Look up the id of a question from its name. + * + * @param string $questionname the question name, for example 'Question 1'. + * @return int corresponding id. + */ + protected function get_question_id(string $questionname): int { + global $DB; + + if (!$id = $DB->get_field('question', 'id', ['name' => $questionname])) { + throw new Exception('There is no question with name "' . $questionname . '".'); + } + return $id; + } +} diff --git a/question/tests/generator/lib.php b/question/tests/generator/lib.php index 5d10eb812d0..dcd4390afbf 100644 --- a/question/tests/generator/lib.php +++ b/question/tests/generator/lib.php @@ -71,10 +71,12 @@ class core_question_generator extends component_generator_base { * examples from the appropriate {@link question_test_helper} subclass. * Then, any files you want to change from the value in the base example you * can override using $overrides. + * * @param string $qtype the question type to create an example of. * @param string $which as for the corresponding argument of * {@link question_test_helper::get_question_form_data}. null for the default one. * @param array|stdClass $overrides any fields that should be different from the base example. + * @return stdClass the question data. */ public function create_question($qtype, $which = null, $overrides = null) { global $CFG; @@ -93,6 +95,17 @@ class core_question_generator extends component_generator_base { return $this->update_question($question, $which, $overrides); } + /** + * Create a tag on a question. + * + * @param array $data with two elements ['questionid' => 123, 'tag' => 'mytag']. + */ + public function create_question_tag(array $data): void { + $question = question_bank::load_question($data['questionid']); + core_tag_tag::add_item_tag('core_question', 'question', $question->id, + context::instance_by_id($question->contextid), $data['tag'], 0); + } + /** * Update an existing question. * @@ -100,6 +113,7 @@ class core_question_generator extends component_generator_base { * @param string $which as for the corresponding argument of * {@link question_test_helper::get_question_form_data}. null for the default one. * @param array|stdClass $overrides any fields that should be different from the base example. + * @return stdClass the question data. */ public function update_question($question, $which = null, $overrides = null) { global $CFG, $DB; diff --git a/question/type/questiontypebase.php b/question/type/questiontypebase.php index c2fe4e93d52..5c353f87b91 100644 --- a/question/type/questiontypebase.php +++ b/question/type/questiontypebase.php @@ -461,7 +461,7 @@ class question_type { $event->trigger(); } - $transaction->allow_commit (); + $transaction->allow_commit(); return $question; } diff --git a/question/type/random/questiontype.php b/question/type/random/questiontype.php index b3999863088..c4305d554bd 100644 --- a/question/type/random/questiontype.php +++ b/question/type/random/questiontype.php @@ -166,7 +166,7 @@ class qtype_random extends question_type { if ($categoryname) { $a->category = $categoryname; } - $a->tags = implode(',', array_map(function($tagname) { + $a->tags = implode(', ', array_map(function($tagname) { return explode(',', $tagname)[1]; }, $tagnames)); } else {