diff --git a/lib/questionlib.php b/lib/questionlib.php index 23325c85bed..d78a84da98e 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -940,23 +940,18 @@ function question_load_questions($questionids, $extrafields = '', $join = '') { * @param stdClass[]|null $filtercourses The courses to filter the course tags by. */ function _tidy_question($question, $category, array $tagobjects = null, array $filtercourses = null): void { - // Load question-type specific fields. - if (!question_bank::is_qtype_installed($question->qtype)) { - $question->questiontext = html_writer::tag('p', get_string('warningmissingtype', - 'qtype_missingtype')) . $question->questiontext; - } - - // Convert numeric fields to float (Prevents these being displayed as 1.0000000.). + // Convert numeric fields to float. This prevents these being displayed as 1.0000000. $question->defaultmark += 0; $question->penalty += 0; + // Indicate the question is now fully initialised. if (isset($question->_partiallyloaded)) { unset($question->_partiallyloaded); } $question->categoryobject = $category; - question_bank::get_qtype($question->qtype)->get_question_options($question); + // Add any tags we have been passed. if (!is_null($tagobjects)) { $categorycontext = context::instance_by_id($category->contextid); $sortedtagobjects = question_sort_tags($tagobjects, $categorycontext, $filtercourses); @@ -965,6 +960,14 @@ function _tidy_question($question, $category, array $tagobjects = null, array $f $question->tagobjects = $sortedtagobjects->tagobjects; $question->tags = $sortedtagobjects->tags; } + + // Load question-type specific fields. + if (question_bank::is_qtype_installed($question->qtype)) { + question_bank::get_qtype($question->qtype)->get_question_options($question); + } else { + $question->questiontext = html_writer::tag('p', get_string('warningmissingtype', + 'qtype_missingtype')) . $question->questiontext; + } } /** diff --git a/mod/quiz/classes/question/bank/qbank_helper.php b/mod/quiz/classes/question/bank/qbank_helper.php index fd6cdbf4ef8..f558d4e996a 100644 --- a/mod/quiz/classes/question/bank/qbank_helper.php +++ b/mod/quiz/classes/question/bank/qbank_helper.php @@ -183,7 +183,6 @@ class qbank_helper { $slot->category = 0; $slot->qtype = 'missingtype'; $slot->name = get_string('missingquestion', 'quiz'); - $slot->maxmark = 0; $slot->questiontext = ' '; $slot->questiontextformat = FORMAT_HTML; $slot->length = 1; diff --git a/mod/quiz/report/reportlib.php b/mod/quiz/report/reportlib.php index 592b41b37ad..ff17ccfcafb 100644 --- a/mod/quiz/report/reportlib.php +++ b/mod/quiz/report/reportlib.php @@ -95,7 +95,6 @@ function quiz_has_questions($quizid) { * ->slot, ->id, ->qtype, ->length, ->number, ->maxmark, ->category (for random questions). */ function quiz_report_get_significant_questions($quiz) { - global $DB; $quizobj = mod_quiz\quiz_settings::create($quiz->id); $structure = \mod_quiz\structure::create_for_quiz($quizobj); $slots = $structure->get_slots(); diff --git a/mod/quiz/report/statistics/report.php b/mod/quiz/report/statistics/report.php index ddd52a9e82c..3ca430e539b 100644 --- a/mod/quiz/report/statistics/report.php +++ b/mod/quiz/report/statistics/report.php @@ -835,33 +835,29 @@ class quiz_statistics_report extends report_base { public function load_and_initialise_questions_for_calculations($quiz) { // Load the questions. $questions = quiz_report_get_significant_questions($quiz); - $questionids = []; - $randomquestions = []; + $questiondata = []; foreach ($questions as $qs => $question) { if ($question->qtype === 'random') { $question->id = 0; $question->name = get_string('random', 'quiz'); $question->questiontext = get_string('random', 'quiz'); $question->parenttype = 'random'; - $randomquestions [] = $question; - unset($questions[$qs]); - continue; + $questiondata[$question->slot] = $question; + } else if ($question->qtype === 'missingtype') { + $question->id = is_numeric($question->id) ? (int) $question->id : 0; + $questiondata[$question->slot] = $question; + $question->name = get_string('deletedquestion', 'qtype_missingtype'); + $question->questiontext = get_string('deletedquestiontext', 'qtype_missingtype'); + } else { + $q = question_bank::load_question_data($question->id); + $q->maxmark = $question->maxmark; + $q->slot = $question->slot; + $q->number = $question->number; + $q->parenttype = null; + $questiondata[$question->slot] = $q; } - $questionids[] = $question->id; } - $fullquestions = question_load_questions($questionids); - foreach ($questions as $qno => $question) { - $q = $fullquestions[$question->id]; - $q->maxmark = $question->maxmark; - $q->slot = $question->slot; - $q->number = $question->number; - $q->parenttype = null; - $questiondata[$question->slot] = $q; - } - foreach ($randomquestions as $randomquestion) { - $questiondata[$randomquestion->slot] = $randomquestion; - } - ksort($questiondata); + return $questiondata; } diff --git a/mod/quiz/report/statistics/statistics_table.php b/mod/quiz/report/statistics/statistics_table.php index f0f3e1a6174..4a5eacd9b27 100644 --- a/mod/quiz/report/statistics/statistics_table.php +++ b/mod/quiz/report/statistics/statistics_table.php @@ -201,6 +201,8 @@ class quiz_statistics_table extends flexible_table { protected function col_actions($questionstat) { if ($this->is_calculated_question_summary($questionstat)) { return ''; + } else if ($questionstat->question->qtype === 'missingtype') { + return ''; } else { $random = null; if ($questionstat->question->qtype === 'random') { diff --git a/mod/quiz/report/statistics/tests/behat/missing_questions.feature b/mod/quiz/report/statistics/tests/behat/missing_questions.feature new file mode 100644 index 00000000000..875b57a6a14 --- /dev/null +++ b/mod/quiz/report/statistics/tests/behat/missing_questions.feature @@ -0,0 +1,48 @@ +@mod @mod_quiz @quiz @quiz_statistics +Feature: Robustness of the statistics calculations with missing qusetions + In order to be able to install and uninstall plugins + As a teacher + I need the statistics to work even if a question type has been uninstalled + + Scenario: Statistics can be calculated even after a question type has been uninstalled + Given the following "users" exist: + | username | + | teacher | + | student | + And the following "courses" exist: + | fullname | shortname | + | Course 1 | C1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher | C1 | editingteacher | + | student | C1 | student | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "questions" exist: + | questioncategory | qtype | name | + | Test questions | truefalse | Test question 1 | + | Test questions | truefalse | Test question 2 | + And the following "activities" exist: + | activity | name | course | idnumber | + | quiz | Quiz 1 | C1 | quiz1 | + And quiz "Quiz 1" contains the following questions: + | question | page | + | Test question 1 | 1 | + | Test question 2 | 1 | + And user "student" has attempted "Quiz 1" with responses: + | slot | response | + | 1 | True | + | 2 | True | + And question "Test question 1" is changed to simulate being of an uninstalled type + And question "Test question 2" no longer exists in the database + + When I am on the "Quiz 1" "mod_quiz > Statistics report" page logged in as teacher + + Then I should see "Quiz structure analysis" + And "1" row "Question name" column of "questionstatistics" table should contain "Missing question" + And "1" row "Attempts" column of "questionstatistics" table should contain "1" + And "1" row "Intended weight" column of "questionstatistics" table should contain "50.00%" + And "2" row "Question name" column of "questionstatistics" table should contain "Missing question" + And "2" row "Attempts" column of "questionstatistics" table should contain "1" + And "2" row "Intended weight" column of "questionstatistics" table should contain "50.00%" diff --git a/mod/quiz/report/statistics/tests/behat/random_essays.feature b/mod/quiz/report/statistics/tests/behat/random_essays.feature new file mode 100644 index 00000000000..1cc460cee6b --- /dev/null +++ b/mod/quiz/report/statistics/tests/behat/random_essays.feature @@ -0,0 +1,38 @@ +@mod @mod_quiz @quiz @quiz_statistics +Feature: Robustness of the statistics calculations with random essays + In order not to see errors + As a teacher + I need the statistics to work even if the quiz uses a random selection of essays + + Scenario: Statistics can be calculated even after a question type has been uninstalled + Given the following "users" exist: + | username | + | teacher | + | student | + And the following "courses" exist: + | fullname | shortname | + | Course 1 | C1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher | C1 | editingteacher | + | student | C1 | student | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "questions" exist: + | questioncategory | qtype | template | name | questiontext | + | Test questions | essay | plain | Test question 1 | | + | Test questions | essay | plain | Test question 2 | | + | Test questions | random | | Random (Test questions) | 0 | + And the following "activities" exist: + | activity | name | course | idnumber | + | quiz | Quiz 1 | C1 | quiz1 | + And quiz "Quiz 1" contains the following questions: + | question | page | + | Random (Test questions) | 1 | + And user "student" has attempted "Quiz 1" with responses: + | slot | response | + | 1 | Here is my wonderful essay | + + When I am on the "Quiz 1" "mod_quiz > Statistics report" page logged in as teacher + Then I should see "No attempts have been made at this quiz, or all attempts have questions that need manual grading." diff --git a/question/classes/statistics/questions/all_calculated_for_qubaid_condition.php b/question/classes/statistics/questions/all_calculated_for_qubaid_condition.php index 50279857726..4fca96f8b79 100644 --- a/question/classes/statistics/questions/all_calculated_for_qubaid_condition.php +++ b/question/classes/statistics/questions/all_calculated_for_qubaid_condition.php @@ -26,6 +26,8 @@ namespace core_question\statistics\questions; +use question_bank; + /** * A collection of all the question statistics calculated for an activity instance. * @@ -214,7 +216,13 @@ class all_calculated_for_qubaid_condition { } else { $this->subquestionstats[$fromdb->questionid] = new calculated_for_subquestion(); $this->subquestionstats[$fromdb->questionid]->populate_from_record($fromdb); - $this->subquestionstats[$fromdb->questionid]->question = $this->subquestions[$fromdb->questionid]; + if (isset($this->subquestions[$fromdb->questionid])) { + $this->subquestionstats[$fromdb->questionid]->question = + $this->subquestions[$fromdb->questionid]; + } else { + $this->subquestionstats[$fromdb->questionid]->question = + question_bank::get_qtype('missingtype', false)->make_deleted_instance($fromdb->questionid, 1); + } } } } diff --git a/question/tests/behat/behat_core_question.php b/question/tests/behat/behat_core_question.php index 601d2f884c3..b9252e2e56e 100644 --- a/question/tests/behat/behat_core_question.php +++ b/question/tests/behat/behat_core_question.php @@ -249,4 +249,54 @@ class behat_core_question extends behat_question_base { $this->execute("behat_general::i_click_on", ["#bulkactionsui-container input[name='$action']", "css_element"]); } + + /** + * Change the question type of the give question to a type that does not exist. + * + * This is useful for testing robustness of the code when a question type + * has been uninstalled, even though there are still questions of that type + * or attempts at them. + * + * In order to set things up, you probably need to start by generating + * questions of a valid type, then using this to change the type once the + * data is created. + * + * @Given question :questionname is changed to simulate being of an uninstalled type + * @param string $questionname the question name. + */ + public function change_question_to_nonexistant_type($questionname) { + global $DB; + [$id] = $this->find_question_by_name($questionname); + + // Check our assumption. + $nonexistanttype = 'invalidqtype'; + if (question_bank::is_qtype_installed($nonexistanttype)) { + throw new coding_exception('This code assumes that the qtype_' . $nonexistanttype . + ' is not a valid plugin name, but that plugin now seems to exist!'); + } + + $DB->set_field('question', 'qtype', $nonexistanttype, ['id' => $id]); + question_bank::notify_question_edited($id); + } + + /** + * Forcibly delete a question from the database. + * + * This is useful for testing robustness of the code when a question + * record is no longer in the database, even though it is referred to. + * Obviously, this should never happen, but it has been known to in the past + * and so we sometimes need to be able to test the code can handle this situation. + * + * In order to set things up, you probably need to start by generating + * a valid questions, then using this to remove it once the data is created. + * + * @Given question :questionname no longer exists in the database + * @param string $questionname the question name. + */ + public function remove_question_from_db($questionname) { + global $DB; + [$id] = $this->find_question_by_name($questionname); + $DB->delete_records('question', ['id' => $id]); + question_bank::notify_question_edited($id); + } } diff --git a/question/type/essay/question.php b/question/type/essay/question.php index 9d331f67b93..5d9b6d03893 100644 --- a/question/type/essay/question.php +++ b/question/type/essay/question.php @@ -111,11 +111,15 @@ class qtype_essay_question extends question_with_responses { } public function un_summarise_response(string $summary) { - if (!empty($summary)) { - return ['answer' => text_to_html($summary)]; - } else { + if (empty($summary)) { return []; } + + if (str_contains($this->responseformat, 'editor')) { + return ['answer' => text_to_html($summary), 'answerformat' => FORMAT_HTML]; + } else { + return ['answer' => $summary, 'answerformat' => FORMAT_PLAIN]; + } } public function get_correct_response() {