diff --git a/question/type/match/question.php b/question/type/match/question.php index ea35fabeafc..80509e3e56a 100644 --- a/question/type/match/question.php +++ b/question/type/match/question.php @@ -126,7 +126,7 @@ class qtype_match_question extends question_graded_automatically_with_countback public function update_attempt_state_data_for_new_version( question_attempt_step $oldstep, question_definition $otherversion) { - parent::update_attempt_state_data_for_new_version($oldstep, $otherversion); + $startdata = parent::update_attempt_state_data_for_new_version($oldstep, $otherversion); // Process stems. $mapping = array_combine(array_keys($otherversion->stems), array_keys($this->stems)); @@ -135,6 +135,7 @@ class qtype_match_question extends question_graded_automatically_with_countback foreach ($oldstemorder as $oldid) { $newstemorder[] = $mapping[$oldid] ?? $oldid; } + $startdata['_stemorder'] = implode(',', $newstemorder); // Process choices. $mapping = array_combine(array_keys($otherversion->choices), array_keys($this->choices)); @@ -143,8 +144,9 @@ class qtype_match_question extends question_graded_automatically_with_countback foreach ($oldchoiceorder as $oldid) { $newchoiceorder[] = $mapping[$oldid] ?? $oldid; } + $startdata['_choiceorder'] = implode(',', $newchoiceorder); - return ['_stemorder' => implode(',', $newstemorder), '_choiceorder' => implode(',', $newchoiceorder)]; + return $startdata; } public function get_question_summary() { diff --git a/question/type/match/tests/question_test.php b/question/type/match/tests/question_test.php index 431c705a58a..65296420f8a 100644 --- a/question/type/match/tests/question_test.php +++ b/question/type/match/tests/question_test.php @@ -274,7 +274,7 @@ class question_test extends \advanced_testcase { $newm->validate_can_regrade_with_other_version($m)); } - public function test_update_attempt_state_date_from_old_version_bad() { + public function test_update_attempt_state_date_from_old_version_bad() { $m = \test_question_maker::make_question('match'); $newm = clone($m); diff --git a/question/type/multichoice/question.php b/question/type/multichoice/question.php index 40c4a2cb3c1..bf9b43c6657 100644 --- a/question/type/multichoice/question.php +++ b/question/type/multichoice/question.php @@ -104,7 +104,7 @@ abstract class qtype_multichoice_base extends question_graded_automatically { public function update_attempt_state_data_for_new_version( question_attempt_step $oldstep, question_definition $otherversion) { - parent::update_attempt_state_data_for_new_version($oldstep, $otherversion); + $startdata = parent::update_attempt_state_data_for_new_version($oldstep, $otherversion); $mapping = array_combine(array_keys($otherversion->answers), array_keys($this->answers)); @@ -113,8 +113,9 @@ abstract class qtype_multichoice_base extends question_graded_automatically { foreach ($oldorder as $oldid) { $neworder[] = $mapping[$oldid] ?? $oldid; } + $startdata['_order'] = implode(',', $neworder); - return ['_order' => implode(',', $neworder)]; + return $startdata; } public function get_question_summary() { diff --git a/question/type/multichoice/tests/walkthrough_test.php b/question/type/multichoice/tests/walkthrough_test.php index 40ee8f94dae..87e330ab89c 100644 --- a/question/type/multichoice/tests/walkthrough_test.php +++ b/question/type/multichoice/tests/walkthrough_test.php @@ -35,6 +35,8 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); * @package qtype_multichoice * @copyright 2010 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qtype_multichoice_single_question + * @covers \qtype_multichoice_single_base */ class walkthrough_test extends \qbehaviour_walkthrough_test_base { public function test_deferredfeedback_feedback_multichoice_single() { @@ -211,6 +213,71 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base { new \question_pattern_expectation('/class="r1"/')); } + public function test_each_attempt_builds_on_last_and_regrade() { + + // Create a multichoice, single question. + $mc = \test_question_maker::make_a_multichoice_single_question(); + $mc->shuffleanswers = false; + + $rightchoice = 0; + + $this->start_attempt_at_question(clone($mc), 'deferredfeedback', 3); + + // Submit the answer false (correct). + $this->process_submission(['answer' => $rightchoice]); + + // Finish the attempt. + $this->quba->finish_all_questions(); + + // Check the state. + $this->check_current_state(question_state::$gradedright); + $this->check_current_mark(3); + $this->check_current_output( + $this->get_contains_mc_radio_expectation($rightchoice, false, true), + $this->get_contains_mc_radio_expectation($rightchoice + 1, false, false), + $this->get_contains_mc_radio_expectation($rightchoice + 2, false, false)); + + // Start a new attempt based on the first one. + $firstattemptqa = $this->quba->get_question_attempt($this->slot); + $this->quba = \question_engine::make_questions_usage_by_activity('unit_test', + \context_system::instance()); + $this->quba->set_preferred_behaviour('deferredfeedback'); + $this->slot = $this->quba->add_question(clone($mc), 3); + $this->quba->start_question_based_on($this->slot, $firstattemptqa); + + // Verify. + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_mc_radio_expectation($rightchoice, true, true), + $this->get_contains_mc_radio_expectation($rightchoice + 1, true, false), + $this->get_contains_mc_radio_expectation($rightchoice + 2, true, false)); + + // Finish the attempt without changing the answer. + $this->quba->finish_all_questions(); + + // Check the state. + $this->check_current_state(question_state::$gradedright); + $this->check_current_mark(3); + $this->check_current_output( + $this->get_contains_mc_radio_expectation($rightchoice, false, true), + $this->get_contains_mc_radio_expectation($rightchoice + 1, false, false), + $this->get_contains_mc_radio_expectation($rightchoice + 2, false, false)); + + // Regrade the attempt - code based on question_usage_by_activity::regrade_question. + $oldqa = $this->quba->get_question_attempt($this->slot); + $newqa = new \question_attempt(clone($mc), + $oldqa->get_usage_id(), $this->quba->get_observer()); + $newqa->set_database_id($oldqa->get_database_id()); + $newqa->set_slot($oldqa->get_slot()); + $newqa->set_max_mark(3); + $newqa->regrade($oldqa, true); + + // Check the state. + $this->assertEquals(question_state::$gradedright, $newqa->get_state()); + $this->assertEquals(3, $newqa->get_mark()); + } + public function test_deferredfeedback_feedback_multichoice_multi_showstandardunstruction_yes() { // Create a multichoice, multi question. diff --git a/question/type/questionbase.php b/question/type/questionbase.php index 874b109406b..0febd2fc466 100644 --- a/question/type/questionbase.php +++ b/question/type/questionbase.php @@ -238,6 +238,9 @@ abstract class question_definition { * A different version of the question will have different question_answers with different ids. However, the list of * choices should be similar, and so we need to shuffle the new list of ids in the same way that the old one was. * + * Note: be sure to return all the data that was originally in $oldstep, while updating the fields that + * require it. Otherwise you might break features like 'Each attempt builds on last' in the quiz. + * * This method should only be called if {@see validate_can_regrade_with_other_version()} did not * flag up a potential problem. So, this method will throw a {@see coding_exception} if it is not * possible to work out a return value. diff --git a/question/type/truefalse/tests/walkthrough_test.php b/question/type/truefalse/tests/walkthrough_test.php index ae7ec1d2ef1..cbae25ee7d2 100644 --- a/question/type/truefalse/tests/walkthrough_test.php +++ b/question/type/truefalse/tests/walkthrough_test.php @@ -30,6 +30,7 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); * @package qtype_truefalse * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qtype_truefalse_question */ class walkthrough_test extends \qbehaviour_walkthrough_test_base { public function test_false_right_does_not_show_feedback_when_not_answered() { @@ -67,6 +68,63 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base { } + public function test_each_attempt_builds_on_last_and_regrade() { + + // Create a true-false question with correct answer false. + $tf = \test_question_maker::make_question('truefalse', 'false'); + $this->start_attempt_at_question(clone($tf), 'deferredfeedback', 1); + + // Submit the answer false (correct). + $this->process_submission(['answer' => 0]); + + // Finish the attempt. + $this->quba->finish_all_questions(); + + // Check the state. + $this->check_current_state(question_state::$gradedright); + $this->check_current_mark(1); + $this->check_current_output( + $this->get_contains_tf_true_radio_expectation(false, false), + $this->get_contains_tf_false_radio_expectation(false, true)); + + // Start a new attempt based on the first one. + $firstattemptqa = $this->quba->get_question_attempt($this->slot); + $this->quba = \question_engine::make_questions_usage_by_activity('unit_test', + \context_system::instance()); + $this->quba->set_preferred_behaviour('deferredfeedback'); + $this->slot = $this->quba->add_question(clone($tf), 1); + $this->quba->start_question_based_on($this->slot, $firstattemptqa); + + // Verify. + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_current_output( + $this->get_contains_tf_true_radio_expectation(true, false), + $this->get_contains_tf_false_radio_expectation(true, true)); + + // Finish the attempt without changing the answer. + $this->quba->finish_all_questions(); + + // Check the state. + $this->check_current_state(question_state::$gradedright); + $this->check_current_mark(1); + $this->check_current_output( + $this->get_contains_tf_true_radio_expectation(false, false), + $this->get_contains_tf_false_radio_expectation(false, true)); + + // Regrade the attempt - code based on question_usage_by_activity::regrade_question. + $oldqa = $this->quba->get_question_attempt($this->slot); + $newqa = new \question_attempt(clone($tf), + $oldqa->get_usage_id(), $this->quba->get_observer()); + $newqa->set_database_id($oldqa->get_database_id()); + $newqa->set_slot($oldqa->get_slot()); + $newqa->regrade($oldqa, true); + + // Check the state. + $this->assertEquals(question_state::$gradedright, $newqa->get_state()); + $this->assertEquals(1, $newqa->get_mark()); + } + /** * @covers \qtype_truefalse_renderer::formulation_and_controls */