question deletion MDL-25173 when a question is deleted, all the associated files...

diff --git a/lib/questionlib.php b/lib/questionlib.php
index 30e0433..1eab656 100644 (file)
--- a/lib/questionlib.php
+++ b/lib/questionlib.php
@@ -566,7 +566,12 @@ function delete_attempt($attemptid) {
 function delete_question($questionid) {
     global $QTYPES, $DB;
 
-    if (!$question = $DB->get_record('question', array('id'=>$questionid))) {
+    $question = $DB->get_record_sql('
+            SELECT q.*, qc.contextid
+            FROM {question} q
+            JOIN {question_categories} qc ON qc.id = q.category
+            WHERE q.id = ?', array($questionid));
+    if (!$question) {
         // In some situations, for example if this was a child of a
         // Cloze question that was previously deleted, the question may already
         // have gone. In this case, just do nothing.
@@ -580,12 +585,8 @@ function delete_question($questionid) {
 
     // delete questiontype-specific data
     question_require_capability_on($question, 'edit');
-    if ($question) {
-        if (isset($QTYPES[$question->qtype])) {
-            $QTYPES[$question->qtype]->delete_question($questionid);
-        }
-    } else {
-        echo "Question with id $questionid does not exist.<br />";
+    if (isset($QTYPES[$question->qtype])) {
+        $QTYPES[$question->qtype]->delete_question($questionid, $question->contextid);
     }
 
     if ($states = $DB->get_records('question_states', array('question'=>$questionid))) {
@@ -597,11 +598,11 @@ function delete_question($questionid) {
         }
     }
 
-    // delete entries from all other question tables
+    // Delete entries from all other question tables
     // It is important that this is done only after calling the questiontype functions
-    $DB->delete_records("question_answers", array("question"=>$questionid));
-    $DB->delete_records("question_states", array("question"=>$questionid));
-    $DB->delete_records("question_sessions", array("questionid"=>$questionid));
+    $DB->delete_records('question_answers', array('question' => $questionid));
+    $DB->delete_records('question_states', array('question' => $questionid));
+    $DB->delete_records('question_sessions', array('questionid' => $questionid));
 
     // Now recursively delete all child questions
     if ($children = $DB->get_records('question', array('parent' => $questionid), '', 'id,qtype')) {
@@ -614,8 +615,6 @@ function delete_question($questionid) {
 
     // Finally delete the question record itself
     $DB->delete_records('question', array('id'=>$questionid));
-
-    return;
 }
 
 /**
@@ -2770,13 +2769,13 @@ function question_has_capability_on($question, $cap, $cachecat = -1){
     static $questions = array();
     static $categories = array();
     static $cachedcat = array();
-    if ($cachecat != -1 && (array_search($cachecat, $cachedcat)===FALSE)){
-        $questions += $DB->get_records('question', array('category'=>$cachecat));
+    if ($cachecat != -1 && array_search($cachecat, $cachedcat) === false) {
+        $questions += $DB->get_records('question', array('category' => $cachecat));
         $cachedcat[] = $cachecat;
     }
     if (!is_object($question)){
         if (!isset($questions[$question])){
-            if (!$questions[$question] = $DB->get_record('question', array('id'=>$question), 'id,category,createdby')) {
+            if (!$questions[$question] = $DB->get_record('question', array('id' => $question), 'id,category,createdby')) {
                 print_error('questiondoesnotexist', 'question');
             }
         }
@@ -2788,11 +2787,12 @@ function question_has_capability_on($question, $cap, $cachecat = -1){
         }
     }
     $category = $categories[$question->category];
+    $context = get_context_instance_by_id($category->contextid);
 
     if (array_search($cap, $question_questioncaps)!== FALSE){
-        if (!has_capability('moodle/question:'.$cap.'all', get_context_instance_by_id($category->contextid))){
+        if (!has_capability('moodle/question:'.$cap.'all', $context)){
             if ($question->createdby == $USER->id){
-                return has_capability('moodle/question:'.$cap.'mine', get_context_instance_by_id($category->contextid));
+                return has_capability('moodle/question:'.$cap.'mine', $context);
             } else {
                 return false;
             }
@@ -2800,7 +2800,7 @@ function question_has_capability_on($question, $cap, $cachecat = -1){
             return true;
         }
     } else {
-        return has_capability('moodle/question:'.$cap, get_context_instance_by_id($category->contextid));
+        return has_capability('moodle/question:'.$cap, $context);
     }
 
 }

diff --git a/question/type/calculated/questiontype.php b/question/type/calculated/questiontype.php
index 847bf59..745c65e 100644 (file)
--- a/question/type/calculated/questiontype.php
+++ b/question/type/calculated/questiontype.php
@@ -744,13 +744,8 @@ class question_calculated_qtype extends default_questiontype {
         }
         return $question;
     }
-    /**
-     * Deletes question from the question-type specific tables
-     *
-     * @return boolean Success/Failure
-     * @param object $question  The question being deleted
-     */
-    function delete_question($questionid) {
+
+    function delete_question($questionid, $contextid) {
         global $DB;
 
         $DB->delete_records("question_calculated", array("question" => $questionid));
@@ -768,13 +763,16 @@ class question_calculated_qtype extends default_questiontype {
             }
         }
         $DB->delete_records("question_datasets", array("question" => $questionid));
-        return true;
+
+        parent::delete_question($questionid, $contextid);
     }
+
     function test_response(&$question, &$state, $answer) {
         $virtualqtype = $this->get_virtual_qtype();
         return $virtualqtype->test_response($question, $state, $answer);
 
     }
+
     function compare_responses(&$question, $state, $teststate) {
 
         $virtualqtype = $this->get_virtual_qtype();
@@ -2115,7 +2113,15 @@ class question_calculated_qtype extends default_questiontype {
         $this->move_files_in_answers($questionid, $oldcontextid, $newcontextid);
 
         $fs->move_area_files_to_new_context($oldcontextid,
-                $newcontextid, 'qtype_numerical', 'instruction', $questionid);
+                $newcontextid, 'qtype_calculated', 'instruction', $questionid);
+    }
+
+    protected function delete_files($questionid, $contextid) {
+        $fs = get_file_storage();
+
+        parent::delete_files($questionid, $contextid);
+        $this->delete_files_in_answers($questionid, $contextid);
+        $fs->delete_area_files($contextid, 'qtype_calculated', 'instruction', $questionid);
     }
 
     function check_file_access($question, $state, $options, $contextid, $component,

diff --git a/question/type/calculatedmulti/questiontype.php b/question/type/calculatedmulti/questiontype.php
index e4f3abf..9d44a65 100644 (file)
--- a/question/type/calculatedmulti/questiontype.php
+++ b/question/type/calculatedmulti/questiontype.php
@@ -562,6 +562,16 @@ class question_calculatedmulti_qtype extends question_calculated_qtype {
                 $newcontextid, 'qtype_calculatedmulti', 'incorrectfeedback', $questionid);
     }
 
+    protected function delete_files($questionid, $contextid) {
+        $fs = get_file_storage();
+
+        parent::delete_files($questionid, $contextid);
+        $this->delete_files_in_answers($questionid, $contextid, true);
+        $fs->delete_area_files($contextid, 'qtype_calculatedmulti', 'correctfeedback', $questionid);
+        $fs->delete_area_files($contextid, 'qtype_calculatedmulti', 'partiallycorrectfeedback', $questionid);
+        $fs->delete_area_files($contextid, 'qtype_calculatedmulti', 'incorrectfeedback', $questionid);
+    }
+
     function check_file_access($question, $state, $options, $contextid, $component,
             $filearea, $args) {
         $itemid = reset($args);

diff --git a/question/type/calculatedsimple/questiontype.php b/question/type/calculatedsimple/questiontype.php
index 621bd27..05d4e93 100644 (file)
--- a/question/type/calculatedsimple/questiontype.php
+++ b/question/type/calculatedsimple/questiontype.php
@@ -352,6 +352,14 @@ class question_calculatedsimple_qtype extends question_calculated_qtype {
                 $newcontextid, 'qtype_calculatedsimple', 'instruction', $questionid);
     }
 
+    protected function delete_files($questionid, $contextid) {
+        $fs = get_file_storage();
+
+        parent::delete_files($questionid, $contextid);
+        $this->delete_files_in_answers($questionid, $contextid);
+        $fs->delete_area_files($contextid, 'qtype_calculatedsimple', 'instruction', $questionid);
+    }
+
     function check_file_access($question, $state, $options, $contextid, $component,
             $filearea, $args) {
         $itemid = reset($args);

diff --git a/question/type/essay/questiontype.php b/question/type/essay/questiontype.php
index 55dfaeb..b7163ec 100644 (file)
--- a/question/type/essay/questiontype.php
+++ b/question/type/essay/questiontype.php
@@ -161,6 +161,11 @@ class question_essay_qtype extends default_questiontype {
         $this->move_files_in_answers($questionid, $oldcontextid, $newcontextid);
     }
 
+    protected function delete_files($questionid, $contextid) {
+        parent::delete_files($questionid, $contextid);
+        $this->delete_files_in_answers($questionid, $contextid);
+    }
+
     function check_file_access($question, $state, $options, $contextid, $component,
             $filearea, $args) {
         if ($component == 'question' && $filearea == 'answerfeedback') {

diff --git a/question/type/match/questiontype.php b/question/type/match/questiontype.php
index a865666..333db6b 100644 (file)
--- a/question/type/match/questiontype.php
+++ b/question/type/match/questiontype.php
@@ -123,17 +123,12 @@ class question_match_qtype extends default_questiontype {
         return true;
     }
 
-    /**
-    * Deletes question from the question-type specific tables
-    *
-    * @return boolean Success/Failure
-    * @param integer $question->id
-    */
-    function delete_question($questionid) {
+    function delete_question($questionid, $contextid) {
         global $DB;
-        $DB->delete_records("question_match", array("question" => $questionid));
-        $DB->delete_records("question_match_sub", array("question" => $questionid));
-        return true;
+        $DB->delete_records('question_match', array('question' => $questionid));
+        $DB->delete_records('question_match_sub', array('question' => $questionid));
+
+        parent::delete_question($questionid, $contextid);
     }
 
     function create_session_and_responses(&$question, &$state, $cmoptions, $attempt) {
@@ -525,6 +520,19 @@ class question_match_qtype extends default_questiontype {
         }
     }
 
+    protected function delete_files($questionid, $contextid) {
+        global $DB;
+        $fs = get_file_storage();
+
+        parent::delete_files($questionid, $contextid);
+
+        $subquestionids = $DB->get_records_menu('question_match_sub',
+                array('question' => $questionid), 'id', 'id,1');
+        foreach ($subquestionids as $subquestionid => $notused) {
+            $fs->delete_area_files($contextid, 'qtype_match', 'subquestion', $subquestionid);
+        }
+    }
+
     function check_file_access($question, $state, $options, $contextid, $component,
             $filearea, $args) {
 

diff --git a/question/type/multianswer/questiontype.php b/question/type/multianswer/questiontype.php
index b32cd61..4519cba 100644 (file)
--- a/question/type/multianswer/questiontype.php
+++ b/question/type/multianswer/questiontype.php
@@ -214,16 +214,11 @@ class embedded_cloze_qtype extends default_questiontype {
         return true;
     }
 
-    /**
-    * Deletes question from the question-type specific tables
-    *
-    * @return boolean Success/Failure
-    * @param object $question  The question being deleted
-    */
-    function delete_question($questionid) {
+    function delete_question($questionid, $contextid) {
         global $DB;
         $DB->delete_records("question_multianswer", array("question" => $questionid));
-        return true;
+
+        parent::delete_question($questionid, $contextid);
     }
 
     function get_correct_responses(&$question, &$state) {

diff --git a/question/type/multichoice/questiontype.php b/question/type/multichoice/questiontype.php
index 88c189b..bef67f0 100644 (file)
--- a/question/type/multichoice/questiontype.php
+++ b/question/type/multichoice/questiontype.php
@@ -168,16 +168,11 @@ class question_multichoice_qtype extends default_questiontype {
         return true;
     }
 
-    /**
-     * Deletes question from the question-type specific tables
-     *
-     * @return boolean Success/Failure
-     * @param object $question  The question being deleted
-     */
-    function delete_question($questionid) {
+    function delete_question($questionid, $contextid) {
         global $DB;
-        $DB->delete_records("question_multichoice", array("question" => $questionid));
-        return true;
+        $DB->delete_records('question_multichoice', array('question' => $questionid));
+
+        parent::delete_question($questionid, $contextid);
     }
 
     function create_session_and_responses(&$question, &$state, $cmoptions, $attempt) {
@@ -528,6 +523,16 @@ class question_multichoice_qtype extends default_questiontype {
                 $newcontextid, 'qtype_multichoice', 'incorrectfeedback', $questionid);
     }
 
+    protected function delete_files($questionid, $contextid) {
+        $fs = get_file_storage();
+
+        parent::delete_files($questionid, $contextid);
+        $this->delete_files_in_answers($questionid, $contextid, true);
+        $fs->delete_area_files($contextid, 'qtype_multichoice', 'correctfeedback', $questionid);
+        $fs->delete_area_files($contextid, 'qtype_multichoice', 'partiallycorrectfeedback', $questionid);
+        $fs->delete_area_files($contextid, 'qtype_multichoice', 'incorrectfeedback', $questionid);
+    }
+
     function check_file_access($question, $state, $options, $contextid, $component,
             $filearea, $args) {
         $itemid = reset($args);

diff --git a/question/type/numerical/questiontype.php b/question/type/numerical/questiontype.php
index 6edb3ed..6f1c0fb 100644 (file)
--- a/question/type/numerical/questiontype.php
+++ b/question/type/numerical/questiontype.php
@@ -463,19 +463,15 @@ class question_numerical_qtype extends question_shortanswer_qtype {
         return true;
     }
 
-    /**
-     * Deletes question from the question-type specific tables
-     *
-     * @return boolean Success/Failure
-     * @param object $question  The question being deleted
-     */
-    function delete_question($questionid) {
+    function delete_question($questionid, $contextid) {
         global $DB;
-        $DB->delete_records("question_numerical", array("question" => $questionid));
-        $DB->delete_records("question_numerical_options", array("question" => $questionid));
-        $DB->delete_records("question_numerical_units", array("question" => $questionid));
-        return true;
+        $DB->delete_records('question_numerical', array('question' => $questionid));
+        $DB->delete_records('question_numerical_options', array('question' => $questionid));
+        $DB->delete_records('question_numerical_units', array('question' => $questionid));
+
+        parent::delete_question($questionid, $contextid);
     }
+
     /**
     * This function has been reinserted in numerical/questiontype.php to simplify
     * the separate rendering of number and unit
@@ -1391,6 +1387,14 @@ class question_numerical_qtype extends question_shortanswer_qtype {
                 $newcontextid, 'qtype_numerical', 'instruction', $questionid);
     }
 
+    protected function delete_files($questionid, $contextid) {
+        $fs = get_file_storage();
+
+        parent::delete_files($questionid, $contextid);
+        $this->delete_files_in_answers($questionid, $contextid);
+        $fs->delete_area_files($contextid, 'qtype_numerical', 'instruction', $questionid);
+    }
+
     function check_file_access($question, $state, $options, $contextid, $component,
             $filearea, $args) {
         $itemid = reset($args);

diff --git a/question/type/numerical/simpletest/testquestiontype.php b/question/type/numerical/simpletest/testquestiontype.php
index 24f2346..4fcb527 100644 (file)
--- a/question/type/numerical/simpletest/testquestiontype.php
+++ b/question/type/numerical/simpletest/testquestiontype.php
@@ -31,42 +31,6 @@ class question_numerical_qtype_test extends UnitTestCase {
         $this->assertEqual($this->qtype->name(), 'numerical');
     }
 
-//    function test_get_question_options() {
-//    }
-//
-//    function test_get_numerical_units() {
-//    }
-//
-//    function test_get_default_numerical_unit() {
-//    }
-//
-//    function test_save_question_options() {
-//    }
-//
-//    function test_save_numerical_units() {
-//    }
-//
-//    function test_delete_question() {
-//    }
-//
-//    function test_compare_responses() {
-//    }
-//
-//    function test_test_response() {
-//    }
-//
-//    function test_check_response(){
-//    }
-//
-//    function test_grade_responses() {
-//    }
-//
-//    function test_get_correct_responses() {
-//    }
-//
-//    function test_get_all_responses() {
-//    }
-
     function test_get_tolerance_interval() {
         $answer = new stdClass;
         $answer->tolerance = 0.01;

diff --git a/question/type/questiontype.php b/question/type/questiontype.php
index ca23764..4f322c2 100644 (file)
--- a/question/type/questiontype.php
+++ b/question/type/questiontype.php
@@ -514,32 +514,30 @@ class default_questiontype {
     }
 
     /**
-    * Deletes a question from the question-type specific tables
-    *
-    * @return boolean Success/Failure
-    * @param object $question  The question being deleted
-    */
-    function delete_question($questionid) {
-        global $CFG, $DB;
-        $success = true;
+     * Deletes the question-type specific data when a question is deleted.
+     * @param integer $question the question being deleted.
+     * @param integer $contextid the context this quesiotn belongs to.
+     */
+    function delete_question($questionid, $contextid) {
+        global $DB;
+
+        $this->delete_files($questionid, $contextid);
 
         $extra_question_fields = $this->extra_question_fields();
         if (is_array($extra_question_fields)) {
             $question_extension_table = array_shift($extra_question_fields);
-            $success = $success && $DB->delete_records($question_extension_table,
+            $DB->delete_records($question_extension_table,
                     array($this->questionid_column_name() => $questionid));
         }
 
         $extra_answer_fields = $this->extra_answer_fields();
         if (is_array($extra_answer_fields)) {
             $answer_extension_table = array_shift($extra_answer_fields);
-            $success = $success && $DB->delete_records_select($answer_extension_table,
+            $DB->delete_records_select($answer_extension_table,
                 "answerid IN (SELECT qa.id FROM {question_answers} qa WHERE qa.question = ?)", array($questionid));
         }
 
-        $success = $success && $DB->delete_records('question_answers', array('question' => $questionid));
-
-        return $success;
+        $DB->delete_records('question_answers', array('question' => $questionid));
     }
 
     /**
@@ -1657,7 +1655,7 @@ class default_questiontype {
 
     /**
      * Move all the files belonging to this question from one context to another.
-     * @param object $question the question to move.
+     * @param integer $questionid the question being moved.
      * @param integer $oldcontextid the context it is moving from.
      * @param integer $newcontextid the context it is moving to.
      */
@@ -1670,8 +1668,9 @@ class default_questiontype {
     }
 
     /**
-     * 
-     * @param object $question the question to move.
+     * Move all the files belonging to this question's answers when the question
+     * is moved from one context to another.
+     * @param integer $questionid the question being moved.
      * @param integer $oldcontextid the context it is moving from.
      * @param integer $newcontextid the context it is moving to.
      * @param boolean $answerstoo whether there is an 'answer' question area,
@@ -1680,6 +1679,7 @@ class default_questiontype {
     protected function move_files_in_answers($questionid, $oldcontextid, $newcontextid, $answerstoo = false) {
         global $DB;
         $fs = get_file_storage();
+
         $answerids = $DB->get_records_menu('question_answers',
                 array('question' => $questionid), 'id', 'id,1');
         foreach ($answerids as $answerid => $notused) {
@@ -1692,6 +1692,38 @@ class default_questiontype {
         }
     }
 
+    /**
+     * Delete all the files belonging to this question.
+     * @param integer $questionid the question being deleted.
+     * @param integer $contextid the context the question is in.
+     */
+    protected function delete_files($questionid, $contextid) {
+        $fs = get_file_storage();
+        $fs->delete_area_files($contextid, 'question', 'questiontext', $questionid);
+        $fs->delete_area_files($contextid, 'question', 'generalfeedback', $questionid);
+    }
+
+    /**
+     * Delete all the files belonging to this question's answers.
+     * @param integer $questionid the question being deleted.
+     * @param integer $contextid the context the question is in.
+     * @param boolean $answerstoo whether there is an 'answer' question area,
+     *      as well as an 'answerfeedback' one. Default false.
+     */
+    protected function delete_files_in_answers($questionid, $contextid, $answerstoo = false) {
+        global $DB;
+        $fs = get_file_storage();
+
+        $answerids = $DB->get_records_menu('question_answers',
+                array('question' => $questionid), 'id', 'id,1');
+        foreach ($answerids as $answerid => $notused) {
+            if ($answerstoo) {
+                $fs->delete_area_files($contextid, 'question', 'answer', $answerid);
+            }
+            $fs->delete_area_files($contextid, 'question', 'answerfeedback', $answerid);
+        }
+    }
+
     function import_file($context, $component, $filearea, $itemid, $file) {
         $fs = get_file_storage();
         $record = new stdclass;

diff --git a/question/type/randomsamatch/questiontype.php b/question/type/randomsamatch/questiontype.php
index 230d799..0bb2aa8 100644 (file)
--- a/question/type/randomsamatch/questiontype.php
+++ b/question/type/randomsamatch/questiontype.php
@@ -61,16 +61,11 @@ class question_randomsamatch_qtype extends question_match_qtype {
         return true;
     }
 
-    /**
-    * Deletes question from the question-type specific tables
-    *
-    * @return boolean Success/Failure
-    * @param object $question  The question being deleted
-    */
-    function delete_question($questionid) {
+    function delete_question($questionid, $contextid) {
         global $DB;
-        $DB->delete_records("question_randomsamatch", array("question" => $questionid));
-        return true;
+        $DB->delete_records('question_randomsamatch', array('question' => $questionid));
+
+        parent::delete_question($questionid, $contextid);
     }
 
     function create_session_and_responses(&$question, &$state, $cmoptions, $attempt) {

diff --git a/question/type/shortanswer/questiontype.php b/question/type/shortanswer/questiontype.php
index 76eba7b..e449904 100644 (file)
--- a/question/type/shortanswer/questiontype.php
+++ b/question/type/shortanswer/questiontype.php
@@ -54,6 +54,11 @@ class question_shortanswer_qtype extends default_questiontype {
         $this->move_files_in_answers($questionid, $oldcontextid, $newcontextid);
     }
 
+    protected function delete_files($questionid, $contextid) {
+        parent::delete_files($questionid, $contextid);
+        $this->delete_files_in_answers($questionid, $contextid);
+    }
+
     function save_question_options($question) {
         global $DB;
         $result = new stdClass;

diff --git a/question/type/truefalse/questiontype.php b/question/type/truefalse/questiontype.php
index f4d73e3..afcf468 100644 (file)
--- a/question/type/truefalse/questiontype.php
+++ b/question/type/truefalse/questiontype.php
@@ -151,16 +151,11 @@ class question_truefalse_qtype extends default_questiontype {
         return true;
     }
 
-    /**
-    * Deletes question from the question-type specific tables
-    *
-    * @return boolean Success/Failure
-    * @param object $question  The question being deleted
-    */
-    function delete_question($questionid) {
+    function delete_question($questionid, $contextid) {
         global $DB;
-        $DB->delete_records("question_truefalse", array("question" => $questionid));
-        return true;
+        $DB->delete_records('question_truefalse', array('question' => $questionid));
+
+        parent::delete_question($questionid, $contextid);
     }
 
     function compare_responses($question, $state, $teststate) {
@@ -262,6 +257,11 @@ class question_truefalse_qtype extends default_questiontype {
         $this->move_files_in_answers($questionid, $oldcontextid, $newcontextid);
     }
 
+    protected function delete_files($questionid, $contextid) {
+        parent::delete_files($questionid, $contextid);
+        $this->delete_files_in_answers($questionid, $contextid);
+    }
+
     function check_file_access($question, $state, $options, $contextid, $component,
             $filearea, $args) {
         if ($component == 'question' && $filearea == 'answerfeedback') {