From 7db1da61eef52f7d812992e61c631d755264ff04 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 13 Feb 2023 15:36:24 +0000 Subject: [PATCH] MDL-77225 questions: fix regrade of descriptions in finished attempts The added logic is similar to the logic in other behaviours, and the new test fails without it. --- .../behaviour/informationitem/behaviour.php | 35 +++++++++++++ .../tests/walkthrough_test.php | 49 ++++++++++++++++++- 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/question/behaviour/informationitem/behaviour.php b/question/behaviour/informationitem/behaviour.php index 3a71d5fe5d5..16491879bbd 100644 --- a/question/behaviour/informationitem/behaviour.php +++ b/question/behaviour/informationitem/behaviour.php @@ -104,12 +104,47 @@ class qbehaviour_informationitem extends question_behaviour { return parent::process_comment($pendingstep); } + /** + * Handle the 'finish' case of {@see process_action()}. + * + * @param question_attempt_pending_step $pendingstep step representing the action. + * @return bool either {@see question_attempt::KEEP} or {@see question_attempt::DISCARD}. + */ public function process_finish(question_attempt_pending_step $pendingstep) { + if ($this->qa->get_state()->is_finished()) { + return question_attempt::DISCARD; + } + + if (!$this->qa->get_state()->is_active()) { + throw new coding_exception('It should be impossible for question_attempt ' . $this->qa->get_database_id() . + ' using ' . get_class($this) . " to receive a 'finish' action while not active. " . + 'Failing with an error, rather than continuing and doing undefined processing, ' . + 'so the bug can be identified.'); + } + $pendingstep->set_state(question_state::$finished); return question_attempt::KEEP; } + /** + * Handle the 'seen' case of {@see process_action()}. + * + * @param question_attempt_pending_step $pendingstep step representing the action. + * @return bool either {@see question_attempt::KEEP} or {@see question_attempt::DISCARD}. + */ public function process_seen(question_attempt_pending_step $pendingstep) { + if ($this->qa->get_state()->is_finished()) { + return question_attempt::DISCARD; + } + + // Assert so we get a clear error message if the assumptions on which this code relies is invalid. + if (!$this->qa->get_state()->is_active()) { + throw new coding_exception('It should be impossible for question_attempt ' . $this->qa->get_database_id() . + ' using ' . get_class($this) . " to receive a 'seen' action while not active. " . + 'Failing with an error, rather than continuing and doing undefined processing, ' . + 'so the bug can be identified.'); + } + $pendingstep->set_state(question_state::$complete); return question_attempt::KEEP; } diff --git a/question/behaviour/informationitem/tests/walkthrough_test.php b/question/behaviour/informationitem/tests/walkthrough_test.php index 553eebabc2b..575b0e63c53 100644 --- a/question/behaviour/informationitem/tests/walkthrough_test.php +++ b/question/behaviour/informationitem/tests/walkthrough_test.php @@ -32,6 +32,7 @@ require_once(__DIR__ . '/../../../engine/tests/helpers.php'); * @category test * @copyright 2009 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qbehaviour_informationitem */ class walkthrough_test extends \qbehaviour_walkthrough_test_base { public function test_informationitem_feedback_description() { @@ -56,7 +57,7 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base { $this->displayoptions->readonly = false; // Process a submission indicating this question has been seen. - $this->process_submission(array('-seen' => 1)); + $this->process_submission(['-seen' => 1]); $this->check_current_state(question_state::$complete); $this->check_current_mark(null); @@ -87,4 +88,50 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base { $this->expectException('moodle_exception'); $this->manual_grade('Not good enough!', 1, FORMAT_HTML); } + + public function test_informationitem_regrade() { + + // Create a true-false question with correct answer true. + $description = \test_question_maker::make_question('description'); + $this->start_attempt_at_question($description, 'deferredfeedback'); + + // Check the initial state. + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_current_output($this->get_contains_question_text_expectation($description), + $this->get_contains_hidden_expectation( + $this->quba->get_field_prefix($this->slot) . '-seen', 1), + $this->get_does_not_contain_feedback_expectation()); + + // Process a submission indicating this question has been seen. + $this->process_submission(['-seen' => 1]); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_current_output($this->get_does_not_contain_correctness_expectation(), + new \question_no_pattern_expectation( + '/type=\"hidden\"[^>]*name=\"[^"]*seen\"|name=\"[^"]*seen\"[^>]*type=\"hidden\"/'), + $this->get_does_not_contain_feedback_expectation()); + + // Finish the attempt. + $this->quba->finish_all_questions(); + + // Verify. + $this->check_current_state(question_state::$finished); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->check_current_output( + $this->get_contains_question_text_expectation($description), + $this->get_contains_general_feedback_expectation($description)); + + // Regrade the attempt. + $this->quba->regrade_all_questions(true); + + // Verify. + $this->check_current_mark(null); + $this->check_step_count(3); + $this->check_current_output( + $this->get_contains_question_text_expectation($description), + $this->get_contains_general_feedback_expectation($description)); + } }