From 6c73dd959349600e02eb121693707ec905daf5ac Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 22 Sep 2017 17:31:34 +0100 Subject: [PATCH 1/2] MDL-57228 quiz: fix editing when there are one-question sections Fix based on a patch by Joost van der Borg . --- mod/quiz/classes/structure.php | 17 +++-------------- mod/quiz/locallib.php | 28 ++++++++++++++++++++++------ mod/quiz/tests/structure_test.php | 31 +++++++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/mod/quiz/classes/structure.php b/mod/quiz/classes/structure.php index aae5c245ac4..d003dbdbc77 100644 --- a/mod/quiz/classes/structure.php +++ b/mod/quiz/classes/structure.php @@ -802,14 +802,8 @@ class structure { } // Update section fist slots. - $DB->execute(" - UPDATE {quiz_sections} - SET firstslot = firstslot + ? - WHERE quizid = ? - AND firstslot > ? - AND firstslot < ? - ", array($headingmovedirection, $this->get_quizid(), - $headingmoveafter, $headingmovebefore)); + quiz_update_section_firstslots($this->get_quizid(), $headingmovedirection, + $headingmoveafter, $headingmovebefore); // If any pages are now empty, remove them. $emptypages = $DB->get_fieldset_sql(" @@ -915,12 +909,7 @@ class structure { question_delete_question($slot->questionid); } - $DB->execute(" - UPDATE {quiz_sections} - SET firstslot = firstslot - 1 - WHERE quizid = ? - AND firstslot > ? - ", array($this->get_quizid(), $slotnumber)); + quiz_update_section_firstslots($this->get_quizid(), -1, $slotnumber); unset($this->questions[$slot->questionid]); $this->refresh_page_numbers_and_update_db(); diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 82a3fa870d6..c1164f60ad9 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -2063,12 +2063,7 @@ function quiz_add_quiz_question($questionid, $quiz, $page = 0, $maxmark = null) $slot->slot = $lastslotbefore + 1; $slot->page = min($page, $maxpage + 1); - $DB->execute(" - UPDATE {quiz_sections} - SET firstslot = firstslot + 1 - WHERE quizid = ? - AND firstslot > ? - ", array($quiz->id, max($lastslotbefore, 1))); + quiz_update_section_firstslots($quiz->id, 1, max($lastslotbefore, 1)); } else { $lastslot = end($slots); @@ -2088,6 +2083,27 @@ function quiz_add_quiz_question($questionid, $quiz, $page = 0, $maxmark = null) $trans->allow_commit(); } +/** + * Move all the section headings in a certain slot range by a certain offset. + * + * @param int $quizid the id of a quiz + * @param int $direction amount to adjust section heading positions. Normally +1 or -1. + * @param int $afterslot adjust headings that start after this slot. + * @param int|null $beforeslot optionally, only adjust headings before this slot. + */ +function quiz_update_section_firstslots($quizid, $direction, $afterslot, $beforeslot = null) { + global $DB; + $where = 'quizid = ? AND firstslot > ?'; + $params = [$direction, $quizid, $afterslot]; + if ($beforeslot) { + $where .= ' AND firstslot < ?'; + $params[] = $beforeslot; + } + $firstslotschanges = $DB->get_records_select_menu('quiz_sections', + $where, $params, '', 'firstslot, firstslot + ?'); + update_field_with_unique_index('quiz_sections', 'firstslot', $firstslotschanges, ['quizid' => $quizid]); +} + /** * Add a random question to the quiz at a given point. * @param object $quiz the quiz settings. diff --git a/mod/quiz/tests/structure_test.php b/mod/quiz/tests/structure_test.php index 9874635ee70..add60070082 100644 --- a/mod/quiz/tests/structure_test.php +++ b/mod/quiz/tests/structure_test.php @@ -736,7 +736,6 @@ class mod_quiz_structure_testcase extends advanced_testcase { 'Heading 2', array('TF2', 2, 'truefalse'), )); - $structure = \mod_quiz\structure::create_for_quiz($quizobj); $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); $cat = $questiongenerator->create_question_category(); @@ -754,13 +753,41 @@ class mod_quiz_structure_testcase extends advanced_testcase { ), $structure); } + public function test_add_question_updates_headings_even_with_one_question_sections() { + $quizobj = $this->create_test_quiz(array( + 'Heading 1', + array('TF1', 1, 'truefalse'), + 'Heading 2', + array('TF2', 2, 'truefalse'), + 'Heading 3', + array('TF3', 3, 'truefalse'), + )); + + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $cat = $questiongenerator->create_question_category(); + $q = $questiongenerator->create_question('truefalse', null, + array('name' => 'TF4', 'category' => $cat->id)); + + quiz_add_quiz_question($q->id, $quizobj->get_quiz(), 1); + + $structure = \mod_quiz\structure::create_for_quiz($quizobj); + $this->assert_quiz_layout(array( + 'Heading 1', + array('TF1', 1, 'truefalse'), + array('TF4', 1, 'truefalse'), + 'Heading 2', + array('TF2', 2, 'truefalse'), + 'Heading 3', + array('TF3', 3, 'truefalse'), + ), $structure); + } + public function test_add_question_at_end_does_not_update_headings() { $quizobj = $this->create_test_quiz(array( array('TF1', 1, 'truefalse'), 'Heading 2', array('TF2', 2, 'truefalse'), )); - $structure = \mod_quiz\structure::create_for_quiz($quizobj); $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); $cat = $questiongenerator->create_question_category(); From f061709e62c68d4926fec9c3f2be63b765961bcd Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Mon, 25 Sep 2017 12:54:05 +0100 Subject: [PATCH 2/2] MDL-57228 quiz editing: unit test for the move case --- mod/quiz/tests/structure_test.php | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/mod/quiz/tests/structure_test.php b/mod/quiz/tests/structure_test.php index add60070082..96433d91930 100644 --- a/mod/quiz/tests/structure_test.php +++ b/mod/quiz/tests/structure_test.php @@ -648,6 +648,34 @@ class mod_quiz_structure_testcase extends advanced_testcase { ), $structure); } + public function test_move_slot_does_not_violate_heading_unique_key() { + $quizobj = $this->create_test_quiz(array( + 'Heading 1', + array('TF1', 1, 'truefalse'), + 'Heading 2', + array('TF2', 2, 'truefalse'), + 'Heading 3', + array('TF3', 3, 'truefalse'), + array('TF4', 3, 'truefalse'), + )); + $structure = \mod_quiz\structure::create_for_quiz($quizobj); + + $idtomove = $structure->get_question_in_slot(4)->slotid; + $idmoveafter = $structure->get_question_in_slot(1)->slotid; + $structure->move_slot($idtomove, $idmoveafter, 1); + + $structure = \mod_quiz\structure::create_for_quiz($quizobj); + $this->assert_quiz_layout(array( + 'Heading 1', + array('TF1', 1, 'truefalse'), + array('TF4', 1, 'truefalse'), + 'Heading 2', + array('TF2', 2, 'truefalse'), + 'Heading 3', + array('TF3', 3, 'truefalse'), + ), $structure); + } + public function test_quiz_remove_slot() { $quizobj = $this->create_test_quiz(array( array('TF1', 1, 'truefalse'),