From 0d5c68bfd877d274cf7d838d7cecabe86a6833f5 Mon Sep 17 00:00:00 2001 From: Eric Merrill Date: Wed, 4 Jun 2014 10:46:48 -0400 Subject: [PATCH 1/2] MDL-20154 questions: question_delete_question should not check perms By Moodle convention, library functions should not implement permission checks, they are to be handled by the invoking code. --- lib/questionlib.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/questionlib.php b/lib/questionlib.php index 21bdeb3f5ca..1b2837a3773 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -321,9 +321,6 @@ function question_delete_question($questionid) { return; } - // Check permissions. - question_require_capability_on($question, 'edit'); - $dm = new question_engine_data_mapper(); $dm->delete_previews($questionid); From c11ac4886815a0e831eeed6b4297eda5de8ceb99 Mon Sep 17 00:00:00 2001 From: Eric Merrill Date: Thu, 29 May 2014 14:20:26 -0400 Subject: [PATCH 2/2] MDL-20154 quiz: Remove unused random questions When a random question is removed from a quiz, and it is not in use elsewhere, it should be deleted. --- mod/quiz/editlib.php | 7 ++++ mod/quiz/lib.php | 13 +++++++ mod/quiz/tests/editlib_test.php | 67 ++++++++++++++++++++++++++++++++- mod/quiz/tests/lib_test.php | 44 +++++++++++++++++++++- 4 files changed, 128 insertions(+), 3 deletions(-) diff --git a/mod/quiz/editlib.php b/mod/quiz/editlib.php index 47932922f83..4376365617b 100644 --- a/mod/quiz/editlib.php +++ b/mod/quiz/editlib.php @@ -84,6 +84,13 @@ function quiz_remove_slot($quiz, $slotnumber) { $DB->set_field('quiz_slots', 'slot', $i - 1, array('quizid' => $quiz->id, 'slot' => $i)); } + + $qtype = $DB->get_field('question', 'qtype', array('id' => $slot->questionid)); + if ($qtype === 'random') { + // This function automatically checks if the question is in use, and won't delete if it is. + question_delete_question($slot->questionid); + } + $trans->allow_commit(); } diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index 6ed0e3f5012..cb3e0eb5c30 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -166,7 +166,20 @@ function quiz_delete_instance($id) { quiz_delete_all_attempts($quiz); quiz_delete_all_overrides($quiz); + // Look for random questions that may no longer be used when this quiz is gone. + $sql = "SELECT q.id + FROM {quiz_slots} slot + JOIN {question} q ON q.id = slot.questionid + WHERE slot.quizid = ? AND q.qtype = ?"; + $questionids = $DB->get_fieldset_sql($sql, array($quiz->id, 'random')); + + // We need to do this before we try and delete randoms, otherwise they would still be 'in use'. $DB->delete_records('quiz_slots', array('quizid' => $quiz->id)); + + foreach ($questionids as $questionid) { + question_delete_question($questionid); + } + $DB->delete_records('quiz_feedback', array('quizid' => $quiz->id)); quiz_access_manager::delete_settings($quiz); diff --git a/mod/quiz/tests/editlib_test.php b/mod/quiz/tests/editlib_test.php index 8acf9883e42..ce9cb6a3fd5 100644 --- a/mod/quiz/tests/editlib_test.php +++ b/mod/quiz/tests/editlib_test.php @@ -36,7 +36,7 @@ require_once($CFG->dirroot . '/mod/quiz/editlib.php'); * @copyright 2009 Tim Hunt * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class mod_quiz_editlib_testcase extends basic_testcase { +class mod_quiz_editlib_testcase extends advanced_testcase { public function test_quiz_question_tostring() { $question = new stdClass(); $question->qtype = 'multichoice'; @@ -48,4 +48,69 @@ class mod_quiz_editlib_testcase extends basic_testcase { $this->assertEquals('The question name' . 'What sort of INEQUALITY is x < y[?]', $summary); } + + /** + * Test removing slots from a quiz. + */ + public function test_quiz_remove_slot() { + global $SITE, $DB; + $this->resetAfterTest(true); + $this->setAdminUser(); + + // Setup a quiz with 1 standard and 1 random question. + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(array('course' => $SITE->id, 'questionsperpage' => 3, 'grade' => 100.0)); + + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $cat = $questiongenerator->create_question_category(); + $standardq = $questiongenerator->create_question('shortanswer', null, array('category' => $cat->id)); + + quiz_add_quiz_question($standardq->id, $quiz); + quiz_add_random_questions($quiz, 0, $cat->id, 1, false); + + // Get the random question. + $randomq = $DB->get_record('question', array('qtype' => 'random')); + + $slotssql = "SELECT qs.*, q.qtype AS qtype + FROM {quiz_slots} AS qs + JOIN {question} AS q ON qs.questionid = q.id + WHERE qs.quizid = ? + ORDER BY qs.slot"; + $slots = $DB->get_records_sql($slotssql, array($quiz->id)); + + // Check that the setup looks right. + $this->assertEquals(2, count($slots)); + $slot = array_shift($slots); + $this->assertEquals($standardq->id, $slot->questionid); + $slot = array_shift($slots); + $this->assertEquals($randomq->id, $slot->questionid); + $this->assertEquals(2, $slot->slot); + + // Remove the standard question. + quiz_remove_slot($quiz, 1); + + $slots = $DB->get_records_sql($slotssql, array($quiz->id)); + + // Check the new ordering, and that the slot number was updated. + $this->assertEquals(1, count($slots)); + $slot = array_shift($slots); + $this->assertEquals($randomq->id, $slot->questionid); + $this->assertEquals(1, $slot->slot); + + // Check the the standard question was not deleted. + $count = $DB->count_records('question', array('id' => $standardq->id)); + $this->assertEquals(1, $count); + + // Remove the random question. + quiz_remove_slot($quiz, 1); + + $slots = $DB->get_records_sql($slotssql, array($quiz->id)); + + // Check that new ordering. + $this->assertEquals(0, count($slots)); + + // Check that the random question was deleted. + $count = $DB->count_records('question', array('id' => $randomq->id)); + $this->assertEquals(0, $count); + } } diff --git a/mod/quiz/tests/lib_test.php b/mod/quiz/tests/lib_test.php index 49d35343085..783f694d20f 100644 --- a/mod/quiz/tests/lib_test.php +++ b/mod/quiz/tests/lib_test.php @@ -27,14 +27,14 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; -require_once($CFG->dirroot . '/mod/quiz/lib.php'); +require_once($CFG->dirroot . '/mod/quiz/editlib.php'); /** * @copyright 2008 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU Public License */ -class mod_quiz_lib_testcase extends basic_testcase { +class mod_quiz_lib_testcase extends advanced_testcase { public function test_quiz_has_grades() { $quiz = new stdClass(); $quiz->grade = '100.0000'; @@ -75,4 +75,44 @@ class mod_quiz_lib_testcase extends basic_testcase { $this->assertEquals(quiz_format_question_grade($quiz, 0), format_float(0, 4)); $this->assertEquals(quiz_format_question_grade($quiz, 1.000000000000), format_float(1, 4)); } + + /** + * Test deleting a quiz instance. + */ + public function test_quiz_delete_instance() { + global $SITE, $DB; + $this->resetAfterTest(true); + $this->setAdminUser(); + + // Setup a quiz with 1 standard and 1 random question. + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + $quiz = $quizgenerator->create_instance(array('course' => $SITE->id, 'questionsperpage' => 3, 'grade' => 100.0)); + + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $cat = $questiongenerator->create_question_category(); + $standardq = $questiongenerator->create_question('shortanswer', null, array('category' => $cat->id)); + + quiz_add_quiz_question($standardq->id, $quiz); + quiz_add_random_questions($quiz, 0, $cat->id, 1, false); + + // Get the random question. + $randomq = $DB->get_record('question', array('qtype' => 'random')); + + quiz_delete_instance($quiz->id); + + // Check that the random question was deleted. + $count = $DB->count_records('question', array('id' => $randomq->id)); + $this->assertEquals(0, $count); + // Check that the standard question was not deleted. + $count = $DB->count_records('question', array('id' => $standardq->id)); + $this->assertEquals(1, $count); + + // Check that all the slots were removed. + $count = $DB->count_records('quiz_slots', array('quizid' => $quiz->id)); + $this->assertEquals(0, $count); + + // Check that the quiz was removed. + $count = $DB->count_records('quiz', array('id' => $quiz->id)); + $this->assertEquals(0, $count); + } }