diff --git a/mod/quiz/report/overview/tests/behat/regrading_not_possible.feature b/mod/quiz/report/overview/tests/behat/regrading_not_possible.feature new file mode 100644 index 00000000000..ed982b7ce5b --- /dev/null +++ b/mod/quiz/report/overview/tests/behat/regrading_not_possible.feature @@ -0,0 +1,56 @@ +@mod @mod_quiz @quiz @quiz_overview @javascript +Feature: Quiz regrade when not possible + In order avoid errors + As a teacher + I need the system to prevent impossible regrade scenarios + + Background: + Given the following "users" exist: + | username | firstname | lastname | + | teacher | Mark | Allwright | + | student | Student | One | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher | C1 | editingteacher | + | student | C1 | student | + And the following "activities" exist: + | activity | name | course | idnumber | + | quiz | Quiz for testing regrading | C1 | quiz1 | + And the following "question categories" exist: + | contextlevel | reference | name | + | Activity module | quiz1 | Test questions | + And the following "questions" exist: + | questioncategory | qtype | template | name | + | Test questions | multichoice | one_of_four | MC | + And quiz "Quiz for testing regrading" contains the following questions: + | question | page | maxmark | + | MC | 1 | 10.0 | + And user "student" has attempted "Quiz for testing regrading" with responses: + | slot | response | + | 1 | B | + + Scenario: Try a regrade after the question has been edited to have a different number of choices + # Edit the question so that V2 has the fourth choice removed. + Given I am on the "MC" "core_question > edit" page logged in as teacher + And I set the following fields to these values: + | Choice 4 | | + | id_feedback_3 | | + And I press "id_submitbutton" + + # Try a regrade, and verify what happened is reported. + When I am on the "Quiz for testing regrading" "mod_quiz > grades report" page + And I press "Regrade all" + + Then I should see "Quiz for testing regrading" + And I should see "The following questions could not be regraded in attempt 1 by Student One" + And I should see "Slot 1: The number of choices in the question has changed." + And I should see "Finished regrading (1/1)" + And I should see "Regrade completed" + And I press "Continue" + + # These next tests just serve to check we got back to the report. + And I should see "Quiz for testing regrading" + And I should see "Overall number of students achieving grade ranges" diff --git a/question/type/ddimageortext/lang/en/qtype_ddimageortext.php b/question/type/ddimageortext/lang/en/qtype_ddimageortext.php index 6a6f9b81acd..681281555a7 100644 --- a/question/type/ddimageortext/lang/en/qtype_ddimageortext.php +++ b/question/type/ddimageortext/lang/en/qtype_ddimageortext.php @@ -28,6 +28,7 @@ $string['answer'] = 'Answer'; $string['bgimage'] = 'Background image'; $string['blank'] = 'blank'; $string['correctansweris'] = 'The correct answer is: {$a}'; +$string['deletedchoice'] = '[Deleted choice]'; $string['draggableimage'] = 'Draggable image'; $string['draggableitem'] = 'Draggable item'; $string['draggableitems'] = 'Draggable items'; @@ -75,4 +76,3 @@ $string['summarisechoiceno'] = 'Item {$a}'; $string['summariseplaceno'] = 'Drop zone {$a}'; $string['xleft'] = 'Left'; $string['ytop'] = 'Top'; -$string['deletedchoice'] = '[Deleted choice]'; diff --git a/question/type/multichoice/lang/en/qtype_multichoice.php b/question/type/multichoice/lang/en/qtype_multichoice.php index c420be0bd27..b914aff0dae 100644 --- a/question/type/multichoice/lang/en/qtype_multichoice.php +++ b/question/type/multichoice/lang/en/qtype_multichoice.php @@ -72,6 +72,7 @@ $string['privacy:preference:single'] = 'Whether the answer is single with radio $string['privacy:preference:shuffleanswers'] = 'Whether the answers should be automatically shuffled.'; $string['privacy:preference:answernumbering'] = 'Which numbering style should be used (\'1, 2, 3, ...\', \'a, b, c, ...\' etc.).'; $string['privacy:preference:showstandardinstruction'] = 'Whether showing standard instruction.'; +$string['regradeissuenumchoiceschanged'] = 'The number of choices in the question has changed.'; $string['selectmulti'] = 'Select one or more:'; $string['selectone'] = 'Select one:'; $string['shuffleanswers'] = 'Shuffle the choices?'; diff --git a/question/type/multichoice/question.php b/question/type/multichoice/question.php index ec9916c663a..40c4a2cb3c1 100644 --- a/question/type/multichoice/question.php +++ b/question/type/multichoice/question.php @@ -89,6 +89,34 @@ abstract class qtype_multichoice_base extends question_graded_automatically { } } + public function validate_can_regrade_with_other_version(question_definition $otherversion): ?string { + $basemessage = parent::validate_can_regrade_with_other_version($otherversion); + if ($basemessage) { + return $basemessage; + } + + if (count($this->answers) != count($otherversion->answers)) { + return get_string('regradeissuenumchoiceschanged', 'qtype_multichoice'); + } + + return null; + } + + 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); + + $mapping = array_combine(array_keys($otherversion->answers), array_keys($this->answers)); + + $oldorder = explode(',', $oldstep->get_qt_var('_order')); + $neworder = []; + foreach ($oldorder as $oldid) { + $neworder[] = $mapping[$oldid] ?? $oldid; + } + + return ['_order' => implode(',', $neworder)]; + } + public function get_question_summary() { $question = $this->html_to_text($this->questiontext, $this->questiontextformat); $choices = array(); @@ -203,9 +231,19 @@ class qtype_multichoice_single_question extends qtype_multichoice_base { if (!$this->is_complete_response($response)) { return null; } - $ansid = $this->order[$response['answer']]; - return $this->html_to_text($this->answers[$ansid]->answer, - $this->answers[$ansid]->answerformat); + $answerid = $this->order[$response['answer']]; + return $this->html_to_text($this->answers[$answerid]->answer, + $this->answers[$answerid]->answerformat); + } + + public function un_summarise_response(string $summary) { + foreach ($this->order as $key => $answerid) { + if ($summary === $this->html_to_text($this->answers[$answerid]->answer, + $this->answers[$answerid]->answerformat)) { + return ['answer' => $key]; + } + } + return []; } public function classify_response(array $response) { @@ -370,6 +408,20 @@ class qtype_multichoice_multi_question extends qtype_multichoice_base { return implode('; ', $selectedchoices); } + public function un_summarise_response(string $summary) { + // This implementation is not perfect. It will fail if an answer contains '; ', + // but this method is only for testing, so it is good enough. + $selectedchoices = explode('; ', $summary); + $response = []; + foreach ($this->order as $key => $answerid) { + if (in_array($this->html_to_text($this->answers[$answerid]->answer, + $this->answers[$answerid]->answerformat), $selectedchoices)) { + $response[$this->field($key)] = '1'; + } + } + return $response; + } + public function classify_response(array $response) { $selectedchoices = array(); foreach ($this->order as $key => $ansid) { diff --git a/question/type/multichoice/tests/question_multi_test.php b/question/type/multichoice/tests/question_multi_test.php index 7d6884d52b5..7ca1012495a 100644 --- a/question/type/multichoice/tests/question_multi_test.php +++ b/question/type/multichoice/tests/question_multi_test.php @@ -33,6 +33,7 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); * * @copyright 2009 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qtype_multichoice_multi_question */ class qtype_multichoice_multi_question_test extends advanced_testcase { @@ -127,8 +128,7 @@ class qtype_multichoice_multi_question_test extends advanced_testcase { $mc->shuffleanswers = false; $mc->start_attempt(new question_attempt_step(), 1); - $summary = $mc->summarise_response($mc->prepare_simulated_post_data(array('B' => 1, 'C' => 1)), - test_question_maker::get_a_qa($mc)); + $summary = $mc->summarise_response($mc->prepare_simulated_post_data(['B' => 1, 'C' => 1])); $this->assertEquals('B; C', $summary); } @@ -138,12 +138,21 @@ class qtype_multichoice_multi_question_test extends advanced_testcase { $mc->shuffleanswers = false; $mc->start_attempt(new question_attempt_step(), 1); - $summary = $mc->summarise_response($mc->prepare_simulated_post_data(array('clearchoice' => -1)), - test_question_maker::get_a_qa($mc)); + $summary = $mc->summarise_response($mc->prepare_simulated_post_data(['clearchoice' => -1])); $this->assertNull($summary); } + public function test_un_summarise_response() { + $mc = test_question_maker::make_a_multichoice_multi_question(); + $mc->shuffleanswers = false; + $mc->start_attempt(new question_attempt_step(), 1); + + $this->assertEquals(['choice1' => '1', 'choice2' => '1'], $mc->un_summarise_response('B; C')); + + $this->assertEquals([], $mc->un_summarise_response('')); + } + public function test_classify_response() { $mc = test_question_maker::make_a_multichoice_multi_question(); $mc->start_attempt(new question_attempt_step(), 1); diff --git a/question/type/multichoice/tests/question_single_test.php b/question/type/multichoice/tests/question_single_test.php index dcb08d640bf..2a2c6647f43 100644 --- a/question/type/multichoice/tests/question_single_test.php +++ b/question/type/multichoice/tests/question_single_test.php @@ -33,6 +33,7 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); * * @copyright 2009 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qtype_multichoice_single_question */ class qtype_multichoice_single_question_test extends advanced_testcase { @@ -159,8 +160,18 @@ class qtype_multichoice_single_question_test extends advanced_testcase { $this->assertEquals('A', $summary); - $this->assertNull($mc->summarise_response(array(), test_question_maker::get_a_qa($mc))); - $this->assertNull($mc->summarise_response(array('answer' => '-1'), test_question_maker::get_a_qa($mc))); + $this->assertNull($mc->summarise_response([])); + $this->assertNull($mc->summarise_response(['answer' => '-1'])); + } + + public function test_un_summarise_response() { + $mc = test_question_maker::make_a_multichoice_single_question(); + $mc->shuffleanswers = false; + $mc->start_attempt(new question_attempt_step(), 1); + + $this->assertEquals(['answer' => '1'], $mc->un_summarise_response('B')); + + $this->assertEquals([], $mc->un_summarise_response('')); } public function test_classify_response() { @@ -209,4 +220,61 @@ class qtype_multichoice_single_question_test extends advanced_testcase { $reconstucted = $mc->prepare_simulated_post_data($simulated); $this->assertEquals($originalresponse, $reconstucted); } + + public function test_validate_can_regrade_with_other_version_bad() { + $mc = test_question_maker::make_a_multichoice_single_question(); + + $newmc = clone($mc); + $newmc->answers = array( + 23 => new question_answer(13, 'A', 1, 'A is right', FORMAT_HTML), + 24 => new question_answer(14, 'B', -0.3333333, 'B is wrong', FORMAT_HTML), + ); + + $this->assertEquals(get_string('regradeissuenumchoiceschanged', 'qtype_multichoice'), + $newmc->validate_can_regrade_with_other_version($mc)); + } + + public function test_validate_can_regrade_with_other_version_ok() { + $mc = test_question_maker::make_a_multichoice_single_question(); + + $newmc = clone($mc); + $newmc->answers = array( + 23 => new question_answer(13, 'A', 1, 'A is right', FORMAT_HTML), + 24 => new question_answer(14, 'B', -0.3333333, 'B is wrong', FORMAT_HTML), + 25 => new question_answer(15, 'C', -0.3333333, 'C is wrong', FORMAT_HTML), + ); + + $this->assertNull($newmc->validate_can_regrade_with_other_version($mc)); + } + + public function test_update_attempt_state_date_from_old_version_bad() { + $mc = test_question_maker::make_a_multichoice_single_question(); + + $newmc = clone($mc); + $newmc->answers = array( + 23 => new question_answer(13, 'A', 1, 'A is right', FORMAT_HTML), + 24 => new question_answer(14, 'B', -0.3333333, 'B is wrong', FORMAT_HTML), + ); + + $oldstep = new question_attempt_step(); + $oldstep->set_qt_var('_order', '14,13,15'); + $this->expectExceptionMessage(get_string('regradeissuenumchoiceschanged', 'qtype_multichoice')); + $newmc->update_attempt_state_data_for_new_version($oldstep, $mc); + } + + public function test_update_attempt_state_date_from_old_version_ok() { + $mc = test_question_maker::make_a_multichoice_single_question(); + + $newmc = clone($mc); + $newmc->answers = array( + 23 => new question_answer(13, 'A', 1, 'A is right', FORMAT_HTML), + 24 => new question_answer(14, 'B', -0.3333333, 'B is wrong', FORMAT_HTML), + 25 => new question_answer(15, 'C', -0.3333333, 'C is wrong', FORMAT_HTML), + ); + + $oldstep = new question_attempt_step(); + $oldstep->set_qt_var('_order', '14,13,15'); + $this->assertEquals(['_order' => '24,23,25'], + $newmc->update_attempt_state_data_for_new_version($oldstep, $mc)); + } } diff --git a/question/type/multichoice/tests/walkthrough_test.php b/question/type/multichoice/tests/walkthrough_test.php index 419d4e82b6c..e05e3641c26 100644 --- a/question/type/multichoice/tests/walkthrough_test.php +++ b/question/type/multichoice/tests/walkthrough_test.php @@ -69,6 +69,27 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base { $this->get_contains_correct_expectation(), new \question_pattern_expectation('/class="r0 correct"/'), new \question_pattern_expectation('/class="r1"/')); + + // Regrade with a new version of the question. + $newmc = \test_question_maker::make_a_multichoice_single_question(); + $newmc->answers = [ + 23 => $newmc->answers[13], + 24 => $newmc->answers[14], + 25 => $newmc->answers[15], + ]; + $newmc->answers[23]->fraction = 0.5; + $newmc->answers[23]->feedback = 'A is now only partially right'; + $newmc->answers[24]->fraction = 1; + $newmc->answers[24]->answer = 'B is the new right answer'; + $this->quba->regrade_question($this->slot, true, null, $newmc); + + // Verify. + $this->check_current_mark(1.5); + $this->render(); + $this->assertStringContainsString('A is now only partially right', $this->currentoutput); + $this->assertStringContainsString('B is the new right answer', $this->currentoutput); + $this->assertStringNotContainsString( + get_string('deletedchoice', 'qtype_multichoice'), $this->currentoutput); } public function test_deferredfeedback_feedback_multichoice_single_showstandardunstruction_yes() {