From 39abc011971605301247d894c604e267d61b52d7 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 16 May 2022 13:59:33 +0100 Subject: [PATCH 1/5] MDL-74752 question regrading: implement the required hooks This commit implements the necessary core hooks to ensure we only allow a regrade of a quetion attempt to take place if the new and old versions of the question are sufficiently similar. It will be followed by commits to each question type where the new method needs to be implemented. Automated tests will be included in the first of those (mulitple choice) becuse we need a question type that implements the hooks to test the core changes. --- lang/en/question.php | 1 + .../report/overview/lang/en/quiz_overview.php | 6 ++- mod/quiz/report/overview/report.php | 24 +++++++++- .../overview/tests/behat/regrading.feature | 32 ++++++------- mod/quiz/upgrade.txt | 8 ++++ question/engine/questionattempt.php | 34 +++++++++++++- question/engine/questionusage.php | 12 +++++ question/type/questionbase.php | 47 +++++++++++++++++++ question/type/upgrade.txt | 26 ++++++++-- 9 files changed, 164 insertions(+), 26 deletions(-) diff --git a/lang/en/question.php b/lang/en/question.php index 48b6a4b25cb..6c4c48dce0d 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -58,6 +58,7 @@ $string['cannotmovequestion'] = 'You can\'t use this script to move questions th $string['cannotopenforwriting'] = 'Cannot open for writing: {$a}'; $string['cannotpreview'] = 'You can\'t preview these questions!'; $string['cannotread'] = 'Cannot read import file (or file is empty)'; +$string['cannotregradedifferentqtype'] = 'Cannot regrade with a question of a different type.'; $string['cannotretrieveqcat'] = 'Could not retrieve question category'; $string['cannotunhidequestion'] = 'Failed to unhide the question.'; $string['cannotunzip'] = 'Could not unzip file.'; diff --git a/mod/quiz/report/overview/lang/en/quiz_overview.php b/mod/quiz/report/overview/lang/en/quiz_overview.php index 67024620999..852f6806877 100644 --- a/mod/quiz/report/overview/lang/en/quiz_overview.php +++ b/mod/quiz/report/overview/lang/en/quiz_overview.php @@ -63,11 +63,13 @@ $string['regradealldrydo'] = 'Regrade attempts marked as needing regrading ({$a} $string['regradealldrydogroup'] = 'Regrade attempts ({$a->countregradeneeded}) marked as needing regrading in group \'{$a->groupname}\''; $string['regradealldrygroup'] = 'Dry run a full regrade for group \'{$a->groupname}\''; $string['regradeallgroup'] = 'Full regrade for group \'{$a->groupname}\''; -$string['regradecomplete'] = 'Regrade completed successfully'; -$string['regradedsuccessfullyxofy'] = 'Successfully regraded ({$a->done}/{$a->count})'; +$string['regradecomplete'] = 'Regrade completed'; +$string['regradedsuccessfullyxofy'] = 'Finished regrading ({$a->done}/{$a->count})'; $string['regradeheader'] = 'Regrading'; $string['regradeselected'] = 'Regrade selected attempts'; +$string['regradingattemptissue'] = 'Slot {$a->slot}: {$a->reason}'; $string['regradingattemptxofy'] = 'Regrading attempt ({$a->done}/{$a->count})'; +$string['regradingattemptxofyproblem'] = 'The following questions could not be regraded in attempt {$a->attemptnum} by {$a->name} (id {$a->attemptid})'; $string['regradingattemptxofywithdetails'] = 'Regrading attempt ({$a->done}/{$a->count}) - Attempt {$a->attemptnum} by {$a->name} (id {$a->attemptid})'; $string['show'] = 'Show / download'; $string['showattempts'] = 'Only show / download attempts'; diff --git a/mod/quiz/report/overview/report.php b/mod/quiz/report/overview/report.php index b0f66d27853..e8fca33fd0e 100644 --- a/mod/quiz/report/overview/report.php +++ b/mod/quiz/report/overview/report.php @@ -337,8 +337,9 @@ class quiz_overview_report extends quiz_attempts_report { * @param bool $dryrun if true, do a pretend regrade, otherwise do it for real. * @param array $slots if null, regrade all questions, otherwise, just regrade * the questions with those slots. + * @return array messages array with keys slot number, and values reasons why that slot cannot be regraded. */ - public function regrade_attempt($attempt, $dryrun = false, $slots = null) { + public function regrade_attempt($attempt, $dryrun = false, $slots = null): array { global $DB; // Need more time for a quiz with many questions. core_php_time_limit::raise(300); @@ -351,12 +352,19 @@ class quiz_overview_report extends quiz_attempts_report { $slots = $quba->get_slots(); } + $messages = []; $finished = $attempt->state == quiz_attempt::FINISHED; foreach ($slots as $slot) { $qqr = new stdClass(); $qqr->oldfraction = $quba->get_question_fraction($slot); $otherquestionversion = $this->get_new_question_for_regrade($attempt, $quba, $slot); + $message = $quba->validate_can_regrade_with_other_version($slot, $otherquestionversion); + if ($message) { + $messages[$slot] = $message; + continue; + } + $quba->regrade_question($slot, $finished, null, $otherquestionversion); $qqr->newfraction = $quba->get_question_fraction($slot); @@ -391,6 +399,7 @@ class quiz_overview_report extends quiz_attempts_report { $quba = null; $transaction = null; gc_collect_cycles(); + return $messages; } /** @@ -555,6 +564,7 @@ class quiz_overview_report extends quiz_attempts_report { */ protected function regrade_batch_of_attempts($quiz, array $attempts, bool $dryrun, \core\dml\sql_join $groupstudentsjoins) { + global $OUTPUT; $this->clear_regrade_table($quiz, $groupstudentsjoins); $progressbar = new progress_bar('quiz_overview_regrade', 500, true); @@ -572,7 +582,17 @@ class quiz_overview_report extends quiz_attempts_report { } $progressbar->update($a['done'], $a['count'], get_string('regradingattemptxofywithdetails', 'quiz_overview', $a)); - $this->regrade_attempt($attempt, $dryrun, $attempt->regradeonlyslots); + $messages = $this->regrade_attempt($attempt, $dryrun, $attempt->regradeonlyslots); + if ($messages) { + $items = []; + foreach ($messages as $slot => $message) { + $items[] = get_string('regradingattemptissue', 'quiz_overview', + ['slot' => $slot, 'reason' => $message]); + } + echo $OUTPUT->notification( + html_writer::tag('p', get_string('regradingattemptxofyproblem', 'quiz_overview', $a)) . + html_writer::alist($items), \core\output\notification::NOTIFY_WARNING); + } } $progressbar->update($a['done'], $a['count'], get_string('regradedsuccessfullyxofy', 'quiz_overview', $a)); diff --git a/mod/quiz/report/overview/tests/behat/regrading.feature b/mod/quiz/report/overview/tests/behat/regrading.feature index d12594b2134..d6ab3f47ffd 100644 --- a/mod/quiz/report/overview/tests/behat/regrading.feature +++ b/mod/quiz/report/overview/tests/behat/regrading.feature @@ -52,8 +52,8 @@ Feature: Regrading quiz attempts using the Grades report # Also, nothing has changed in the quiz, so the regrade won't alter any scores, # but this is still a useful test that the regrade process completes without errors. Then I should see "Quiz for testing regrading" - And I should see "Successfully regraded (2/2)" - And I should see "Regrade completed successfully" + And I should see "Finished regrading (2/2)" + And I should see "Regrade completed" And I press "Continue" # These next tests just serve to check we got back to the report. @@ -67,8 +67,8 @@ Feature: Regrading quiz attempts using the Grades report And I press "Regrade selected attempts" Then I should see "Quiz for testing regrading" - And I should see "Successfully regraded (1/1)" - And I should see "Regrade completed successfully" + 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. @@ -88,8 +88,8 @@ Feature: Regrading quiz attempts using the Grades report # Note, the order is not defined, so we can only check part of the message. Then I should see "Quiz for testing regrading" - And I should see "Successfully regraded (2/2)" - And I should see "Regrade completed successfully" + And I should see "Finished regrading (2/2)" + And I should see "Regrade completed" And I press "Continue" And "Student One" row "Regrade" column of "attempts" table should not contain "Needed" @@ -100,8 +100,8 @@ Feature: Regrading quiz attempts using the Grades report And "Student TwoReview attempt" row "Grade/100.00Sort by Grade/100.00 Ascending" column of "attempts" table should contain "90.00/75.00" And I press "Regrade attempts marked as needing regrading (1)" And I should see "Quiz for testing regrading" - And I should see "Successfully regraded (1/1)" - And I should see "Regrade completed successfully" + 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. @@ -117,8 +117,8 @@ Feature: Regrading quiz attempts using the Grades report And I navigate to "Results" in current page administration When I press "Dry run a full regrade" Then I should see "Quiz for testing regrading" - And I should see "Successfully regraded (2/2)" - And I should see "Regrade completed successfully" + And I should see "Finished regrading (2/2)" + And I should see "Regrade completed" And I press "Continue" And I should see "Quiz for testing regrading" And I should see "Overall number of students achieving grade ranges" @@ -132,12 +132,12 @@ Feature: Regrading quiz attempts using the Grades report And I click on "v2 (latest)" "option" And I navigate to "Results" in current page administration And I press "Dry run a full regrade" - And I should see "Regrade completed successfully" + And I should see "Regrade completed" And I press "Continue" And "student1@example.com" row "Regrade" column of "attempts" table should contain "Needed" And "Correct" "icon" should appear before "50.00/0.00" "text" And I press "Regrade all" - And I should see "Regrade completed successfully" + And I should see "Regrade completed" And I press "Continue" Then "student1@example.com" row "Regrade" column of "attempts" table should contain "Done" And "Student OneReview attempt" row "Q. 1/50.00Sort by Q. 1/50.00 Ascending" column of "attempts" table should contain "50.00/0.00" @@ -177,8 +177,8 @@ Feature: Regrading quiz attempts using the Grades report And I click on "Always latest" "option" And I navigate to "Results" in current page administration And I press "Regrade all" - And I should see "Successfully regraded (1/1)" - And I should see "Regrade completed successfully" + And I should see "Finished regrading (1/1)" + And I should see "Regrade completed" And I press "Continue" Then "student3@example.com" row "Q. 1/50.00Sort by Q. 1/50.00 Ascending" column of "attempts" table should contain "50.00/0.00" And "Incorrect" "icon" should appear before "50.00/0.00" "text" @@ -218,8 +218,8 @@ Feature: Regrading quiz attempts using the Grades report And "student3@example.com" row "Q. 1/100.00Sort by Q. 1/100.00 Ascending" column of "attempts" table should contain "100.00" And "Correct" "icon" should be visible And I press "Regrade all" - And I should see "Successfully regraded (1/1)" - And I should see "Regrade completed successfully" + And I should see "Finished regrading (1/1)" + And I should see "Regrade completed" And I press "Continue" Then "student3@example.com" row "Q. 1/100.00Sort by Q. 1/100.00 Ascending" column of "attempts" table should contain "100.00/0.00" And "Incorrect" "icon" should be visible diff --git a/mod/quiz/upgrade.txt b/mod/quiz/upgrade.txt index 2419d36bf44..17a73e8d847 100644 --- a/mod/quiz/upgrade.txt +++ b/mod/quiz/upgrade.txt @@ -1,5 +1,13 @@ This files describes API changes in the quiz code. +=== 4.0.2, 4.1 === + +* No external code should be calling quiz_overview_report::regrade_attempt because it is an + internal method of the quiz_overview plugin. But if you are incorrectly using it, be aware + that the API changed slightly. It now returns an array listing any questions which could + not be regraded. + + === 4.0 === * The following API methods have a new parameter, $studentisonline, to define whether the student is currently interacting: diff --git a/question/engine/questionattempt.php b/question/engine/questionattempt.php index ba41f67118a..097465c7a42 100644 --- a/question/engine/questionattempt.php +++ b/question/engine/questionattempt.php @@ -1396,6 +1396,16 @@ class question_attempt { $this->process_action(array('-finish' => 1), $timestamp, $userid); } + /** + * Verify if this question_attempt in can be regraded with that other question version. + * + * @param question_definition $otherversion a different version of the question to use in the regrade. + * @return string|null null if the regrade can proceed, else a reason why not. + */ + public function validate_can_regrade_with_other_version(question_definition $otherversion): ?string { + return $this->get_question(false)->validate_can_regrade_with_other_version($otherversion); + } + /** * Perform a regrade. This replays all the actions from $oldqa into this * attempt. @@ -1412,7 +1422,8 @@ class question_attempt { if ($first) { // First step of the attempt. $first = false; - $this->start($oldqa->behaviour, $oldqa->get_variant(), $step->get_all_data(), + $this->start($oldqa->behaviour, $oldqa->get_variant(), + $this->get_attempt_state_data_to_regrade_with_version($step, $oldqa->get_question()), $step->get_timecreated(), $step->get_user_id(), $step->get_id()); } else if ($step->has_behaviour_var('finish') && count($step->get_submitted_data()) > 1) { @@ -1446,6 +1457,27 @@ class question_attempt { $this->set_flagged($oldqa->is_flagged()); } + /** + * Helper used by regrading. + * + * Get the data from the first step of the old attempt and, if necessary, + * update it to be suitable for use with the other version of the question. + * + * @param question_attempt_step $oldstep First step at an attempt at $otherversion of this question. + * @param question_definition $otherversion Another version of the question being attempted. + * @return array updated data required to restart an attempt with the current version of this question. + */ + protected function get_attempt_state_data_to_regrade_with_version(question_attempt_step $oldstep, + question_definition $otherversion): array { + if ($this->get_question(false) === $otherversion) { + return $oldstep->get_all_data(); + } else { + $attemptstatedata = $this->get_question(false)->update_attempt_state_data_for_new_version( + $oldstep, $otherversion); + return array_merge($attemptstatedata, $oldstep->get_behaviour_data()); + } + } + /** * Change the max mark for this question_attempt. * @param float $maxmark the new max mark. diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php index 291285684f5..116eca571de 100644 --- a/question/engine/questionusage.php +++ b/question/engine/questionusage.php @@ -875,9 +875,21 @@ class question_usage_by_activity { $this->observer->notify_attempt_modified($qa); } + /** + * Verify if the question_attempt in the given slot can be regraded with that other question version. + * + * @param int $slot the number used to identify this question within this usage. + * @param question_definition $otherversion a different version of the question to use in the regrade. + * @return string|null null if the regrade can proceed, else a reason why not. + */ + public function validate_can_regrade_with_other_version(int $slot, question_definition $otherversion): ?string { + return $this->get_question_attempt($slot)->validate_can_regrade_with_other_version($otherversion); + } + /** * Regrade a question in this usage. This replays the sequence of submitted * actions to recompute the outcomes. + * * @param int $slot the number used to identify this question within this usage. * @param bool $finished whether the question attempt should be forced to be finished * after the regrade, or whether it may still be in progress (default false). diff --git a/question/type/questionbase.php b/question/type/questionbase.php index e7d63600571..874b109406b 100644 --- a/question/type/questionbase.php +++ b/question/type/questionbase.php @@ -211,6 +211,53 @@ abstract class question_definition { public function apply_attempt_state(question_attempt_step $step) { } + /** + * Verify if an attempt at this question can be re-graded using the other question version. + * + * To put it another way, will {@see update_attempt_state_date_from_old_version()} be able to work? + * + * It is expected that this relationship is symmetrical, so if you can regrade from V1 to V3, then + * you can change back from V3 to V1. + * + * @param question_definition $otherversion a different version of the question to use in the regrade. + * @return string|null null if the regrade can proceed, else a reason why not. + */ + public function validate_can_regrade_with_other_version(question_definition $otherversion): ?string { + if (get_class($otherversion) !== get_class($this)) { + return get_string('cannotregradedifferentqtype', 'question'); + } + + return null; + } + + /** + * Update the data representing the initial state of an attempt another version of this question, to allow for the changes. + * + * What is required is probably most easily understood using an example. Think about multiple choice questions. + * The first step has a variable '_order' which is a comma-separated list of question_answer ids. + * 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. + * + * 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. + * + * @param question_attempt_step $oldstep the first step of a {@see question_attempt} at $oldquestion. + * @param question_definition $oldquestion the previous version of the question, which $oldstate comes from. + * @return array the submit data which can be passed to {@see apply_attempt_state} to start + * an attempt at this version of this question, corresponding to the attempt at the old question. + * @throws coding_exception if this can't be done. + */ + public function update_attempt_state_data_for_new_version( + question_attempt_step $oldstep, question_definition $oldquestion) { + $message = $this->validate_can_regrade_with_other_version($oldquestion); + if ($message) { + throw new coding_exception($message); + } + + return $oldstep->get_qt_data(); + } + /** * Generate a brief, plain-text, summary of this question. This is used by * various reports. This should show the particular variant of the question diff --git a/question/type/upgrade.txt b/question/type/upgrade.txt index 87f56205535..10d1a0c3ddc 100644 --- a/question/type/upgrade.txt +++ b/question/type/upgrade.txt @@ -1,17 +1,32 @@ This files describes API changes for question type plugins. +=== 4.0.2, 4.1 === + +* There was one issue caused by the changes in Moodle 4.0 which requires changes in question types, + and that is regrading. There are two new methods which some question types will need to implement, + - validate_can_regrade_with_other_version + - update_attempt_state_data_for_new_version + these methods are introduced in this commit, and there are details PHPdoc comments about what they + must do. Then the immediately following commits implement them in the core question types where + they are required. + Generally, you will need to implement one or both of these if you question type does something + significant in the apply_attempt_state method. If you have not implemented that method, then + almost certainly you don't need to worry about this. + + === 4.0 === -1) The major question bank changes should not affect most basic question type plugins. - The navigation changes may affect Behat tests. If you encounter this, - the best way to fix it is to use the new navigation steps in MDL-74130. +* The major question bank changes should not affect most basic question type plugins. + The navigation changes may affect Behat tests. If you encounter this, + the best way to fix it is to use the new navigation steps in MDL-74130. -2) The qualification 'most' is because some question types do more complex things, which - will require changes related to question versionning. Some examples that come to mind: +* The qualification 'most' is because some question types do more complex things, which + will require changes related to question versionning. Some examples that come to mind: - the way qtype_mulitanswer (or qtype_combined) aggregates several sub-questions into a parent question. - the way some contrib plugins (e.g. qtype_stack, qtype_pmatch) store additional data (question tests) linked to questions. That relationship will need to be updated. + === 3.11 === * Introducing the following \question_type base class methods to save/fetch the last form values @@ -42,6 +57,7 @@ This files describes API changes for question type plugins. use it in a question type at https://github.com/moodleou/moodle-qtype_pmatch/commit/2aefa8b5dcc7bab768f4707a4ffb7befcf4c2540. + === 3.8, 3.7.3, 3.6.7 === * Coming up in Moodle 3.8 are some changes to the question bank UI. These will break any From e230bfa47f50648fc9c6ff42acf0b8807c253e68 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 16 May 2022 16:45:46 +0100 Subject: [PATCH 2/5] MDL-74752 qtype_multichoice: fix regrading logic The implements the new regrade-related hooks, and also has tests for the changes to the core system, now that we have a question type we can use for them. --- .../behat/regrading_not_possible.feature | 56 +++++++++++++++ .../lang/en/qtype_ddimageortext.php | 2 +- .../multichoice/lang/en/qtype_multichoice.php | 1 + question/type/multichoice/question.php | 58 ++++++++++++++- .../multichoice/tests/question_multi_test.php | 17 +++-- .../tests/question_single_test.php | 72 ++++++++++++++++++- .../multichoice/tests/walkthrough_test.php | 21 ++++++ 7 files changed, 217 insertions(+), 10 deletions(-) create mode 100644 mod/quiz/report/overview/tests/behat/regrading_not_possible.feature 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() { From 823b5bb740a16e3906e8b14b26d10b7ff8671ddd Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 23 May 2022 17:16:29 +0100 Subject: [PATCH 3/5] MDL-74752 qtype_match: implement regrading hooks As much as they can be before MDL-5560 is addressed. --- question/type/match/lang/en/qtype_match.php | 6 +++-- question/type/match/question.php | 17 ++++++++++++ question/type/match/tests/question_test.php | 29 +++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/question/type/match/lang/en/qtype_match.php b/question/type/match/lang/en/qtype_match.php index 72c857cfaab..83fc1142892 100644 --- a/question/type/match/lang/en/qtype_match.php +++ b/question/type/match/lang/en/qtype_match.php @@ -33,8 +33,6 @@ $string['nomatchinganswer'] = 'You must specify an answer matching the question $string['nomatchinganswerforq'] = 'You must specify an answer for this question.'; $string['notenoughqsandas'] = 'You must supply at least {$a->q} questions and {$a->a} answers.'; $string['notenoughquestions'] = 'You must supply at least {$a} question and answer pairs.'; -$string['shuffle'] = 'Shuffle'; -$string['shuffle_help'] = 'If enabled, the order of the questions is randomly shuffled for each attempt, provided that "Shuffle within questions" in the activity settings is also enabled. In a matching question, only the questions are affected by this parameter. The answer choices will always be shuffled.'; $string['pleaseananswerallparts'] = 'Please answer all parts of the question.'; $string['pluginname'] = 'Matching'; $string['pluginname_help'] = 'Matching questions require the respondent to correctly match a list of names or statements (questions) to another list of names or statements (answers).'; @@ -46,3 +44,7 @@ $string['privacy:metadata'] = 'Matching question type plugin allows question aut $string['privacy:preference:defaultmark'] = 'The default mark set for a given question.'; $string['privacy:preference:penalty'] = 'The penalty for each incorrect try when questions are run using the \'Interactive with multiple tries\' or \'Adaptive mode\' behaviour.'; $string['privacy:preference:shuffleanswers'] = 'Whether the answers should be automatically shuffled.'; +$string['regradeissuenumchoiceschanged'] = 'The number of choices has changed.'; +$string['regradeissuenumstemschanged'] = 'The number of sub-questions has changed.'; +$string['shuffle'] = 'Shuffle'; +$string['shuffle_help'] = 'If enabled, the order of the questions is randomly shuffled for each attempt, provided that "Shuffle within questions" in the activity settings is also enabled. In a matching question, only the questions are affected by this parameter. The answer choices will always be shuffled.'; diff --git a/question/type/match/question.php b/question/type/match/question.php index 15996ceca59..84c99d4f30b 100644 --- a/question/type/match/question.php +++ b/question/type/match/question.php @@ -107,6 +107,23 @@ class qtype_match_question extends question_graded_automatically_with_countback } } + 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->stems) != count($otherversion->stems)) { + return get_string('regradeissuenumstemschanged', 'qtype_match'); + } + + if (count($this->choices) != count($otherversion->choices)) { + return get_string('regradeissuenumchoiceschanged', 'qtype_match'); + } + + return null; + } + public function get_question_summary() { $question = $this->html_to_text($this->questiontext, $this->questiontextformat); $stems = array(); diff --git a/question/type/match/tests/question_test.php b/question/type/match/tests/question_test.php index 1419bac9d1f..e5532c4212b 100644 --- a/question/type/match/tests/question_test.php +++ b/question/type/match/tests/question_test.php @@ -33,6 +33,7 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); * @package qtype_match * @copyright 2009 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qtype_match_question */ class question_test extends \advanced_testcase { @@ -244,4 +245,32 @@ class question_test extends \advanced_testcase { $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); $this->assertEquals(1, $options['shufflestems']); } + + public function test_validate_can_regrade_with_other_version_ok() { + $m = \test_question_maker::make_question('match'); + + $newm = clone($m); + + $this->assertNull($newm->validate_can_regrade_with_other_version($m)); + } + + public function test_validate_can_regrade_with_other_version_bad_stems() { + $m = \test_question_maker::make_question('match'); + + $newm = clone($m); + unset($newm->stems[4]); + + $this->assertEquals(get_string('regradeissuenumstemschanged', 'qtype_match'), + $newm->validate_can_regrade_with_other_version($m)); + } + + public function test_validate_can_regrade_with_other_version_bad_choices() { + $m = \test_question_maker::make_question('match'); + + $newm = clone($m); + unset($newm->choices[3]); + + $this->assertEquals(get_string('regradeissuenumchoiceschanged', 'qtype_match'), + $newm->validate_can_regrade_with_other_version($m)); + } } From 8ca7e3b74c496379a3dd09400d1fa548c425260f Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 23 May 2022 17:46:12 +0100 Subject: [PATCH 4/5] MDL-74752 qtypes gapselect & ddwtos: implement regrading hooks --- .../gapselect/lang/en/qtype_gapselect.php | 2 ++ question/type/gapselect/questionbase.php | 19 ++++++++++++ .../type/gapselect/tests/question_test.php | 30 +++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/question/type/gapselect/lang/en/qtype_gapselect.php b/question/type/gapselect/lang/en/qtype_gapselect.php index 81b74549728..9d3647adb1c 100644 --- a/question/type/gapselect/lang/en/qtype_gapselect.php +++ b/question/type/gapselect/lang/en/qtype_gapselect.php @@ -45,6 +45,8 @@ $string['privacy:metadata'] = 'Select missing words question type plugin allows $string['privacy:preference:defaultmark'] = 'The default mark set for a given question.'; $string['privacy:preference:penalty'] = 'The penalty for each incorrect try when questions are run using the \'Interactive with multiple tries\' or \'Adaptive mode\' behaviour.'; $string['privacy:preference:shuffleanswers'] = 'Whether the answers should be automatically shuffled.'; +$string['regradeissuenumchoiceschanged'] = 'The number of choices in group {$a} changed.'; +$string['regradeissuenumgroupsschanged'] = 'The number of groups of choices has changed.'; $string['shuffle'] = 'Shuffle'; $string['tagsnotallowed'] = '{$a->tag} is not allowed. (Only {$a->allowed} are permitted.)'; $string['tagsnotallowedatall'] = '{$a->tag} is not allowed. (No HTML is allowed here.)'; diff --git a/question/type/gapselect/questionbase.php b/question/type/gapselect/questionbase.php index 0a93eb4561f..10595d011c1 100644 --- a/question/type/gapselect/questionbase.php +++ b/question/type/gapselect/questionbase.php @@ -112,6 +112,25 @@ abstract class qtype_gapselect_question_base extends question_graded_automatical } } + 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->choices) != count($otherversion->choices)) { + return get_string('regradeissuenumgroupsschanged', 'qtype_gapselect'); + } + + foreach ($this->choices as $group => $choices) { + if (count($this->choices[$group]) != count($otherversion->choices[$group])) { + return get_string('regradeissuenumchoiceschanged', 'qtype_gapselect', $group); + } + } + + return null; + } + public function get_question_summary() { $question = $this->html_to_text($this->questiontext, $this->questiontextformat); $groups = array(); diff --git a/question/type/gapselect/tests/question_test.php b/question/type/gapselect/tests/question_test.php index 450e3008c01..5aef925f250 100644 --- a/question/type/gapselect/tests/question_test.php +++ b/question/type/gapselect/tests/question_test.php @@ -34,6 +34,8 @@ require_once($CFG->dirroot . '/question/type/gapselect/tests/helper.php'); * @package qtype_gapselect * @copyright 2012 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qtype_gapselect_question_base + * @covers \qtype_gapselect_question */ class question_test extends \basic_testcase { @@ -258,4 +260,32 @@ class question_test extends \basic_testcase { $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); $this->assertEquals(1, $options['shufflechoices']); } + + public function test_validate_can_regrade_with_other_version_ok() { + $question = \test_question_maker::make_question('gapselect'); + + $newquestion = clone($question); + + $this->assertNull($newquestion->validate_can_regrade_with_other_version($question)); + } + + public function test_validate_can_regrade_with_other_version_bad_groups() { + $question = \test_question_maker::make_question('gapselect'); + + $newquestion = clone($question); + unset($newquestion->choices[3]); + + $this->assertEquals(get_string('regradeissuenumgroupsschanged', 'qtype_gapselect'), + $newquestion->validate_can_regrade_with_other_version($question)); + } + + public function test_validate_can_regrade_with_other_version_bad_choices() { + $question = \test_question_maker::make_question('gapselect'); + + $newquestion = clone($question); + unset($newquestion->choices[2][2]); + + $this->assertEquals(get_string('regradeissuenumchoiceschanged', 'qtype_gapselect', 2), + $newquestion->validate_can_regrade_with_other_version($question)); + } } From 314b90d1f53f66a812e1e6c48df5a90fd9747408 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 23 May 2022 21:49:38 +0100 Subject: [PATCH 5/5] MDL-74752 qtype_multianswer: implement regrading hooks --- .../multianswer/lang/en/qtype_multianswer.php | 1 + question/type/multianswer/question.php | 37 +++++++ .../type/multianswer/tests/question_test.php | 97 +++++++++++++++++++ 3 files changed, 135 insertions(+) diff --git a/question/type/multianswer/lang/en/qtype_multianswer.php b/question/type/multianswer/lang/en/qtype_multianswer.php index e6e1ce1ecdf..cc6009d4419 100644 --- a/question/type/multianswer/lang/en/qtype_multianswer.php +++ b/question/type/multianswer/lang/en/qtype_multianswer.php @@ -67,6 +67,7 @@ $string['questionsaveasedited'] = 'The question will be saved as edited'; $string['questiontypechanged'] = 'Question type changed'; $string['questiontypechangedcomment'] = 'At least one question type has been changed.
Did you add, delete or move a question?
Look ahead.'; $string['questionusedinquiz'] = 'This question is used in {$a->nb_of_quiz} quiz(s), total attempt(s) : {$a->nb_of_attempts} '; +$string['regradeissuenumsubquestionschanged'] = 'The number embedded sub-questions in the question has changed.'; $string['storedqtype'] = 'Stored question type {$a}'; $string['subqresponse'] = 'part {$a->i}: {$a->response}'; $string['unknownquestiontypeofsubquestion'] = 'Unknown question type: {$a->type} of question part # {$a->sub}'; diff --git a/question/type/multianswer/question.php b/question/type/multianswer/question.php index 2910fca66d9..62036c263a4 100644 --- a/question/type/multianswer/question.php +++ b/question/type/multianswer/question.php @@ -78,6 +78,43 @@ class qtype_multianswer_question extends question_graded_automatically_with_coun } } + 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->subquestions) != count($otherversion->subquestions)) { + return get_string('regradeissuenumsubquestionschanged', 'qtype_multianswer'); + } + + foreach ($this->subquestions as $i => $subq) { + $subqmessage = $subq->validate_can_regrade_with_other_version($otherversion->subquestions[$i]); + if ($subqmessage) { + return $subqmessage; + } + } + + return null; + } + + public function update_attempt_state_data_for_new_version( + question_attempt_step $oldstep, question_definition $oldquestion) { + parent::update_attempt_state_data_for_new_version($oldstep, $oldquestion); + + $result = []; + foreach ($this->subquestions as $i => $subq) { + $substep = $this->get_substep($oldstep, $i); + $statedata = $subq->update_attempt_state_data_for_new_version( + $substep, $oldquestion->subquestions[$i]); + foreach ($statedata as $name => $value) { + $result[$substep->add_prefix($name)] = $value; + } + } + + return $result; + } + public function get_question_summary() { $summary = $this->html_to_text($this->questiontext, $this->questiontextformat); foreach ($this->subquestions as $i => $subq) { diff --git a/question/type/multianswer/tests/question_test.php b/question/type/multianswer/tests/question_test.php index 0744ee128ea..ae38468730a 100644 --- a/question/type/multianswer/tests/question_test.php +++ b/question/type/multianswer/tests/question_test.php @@ -32,6 +32,7 @@ require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); * @package qtype_multianswer * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qtype_multianswer_question */ class question_test extends \advanced_testcase { public function test_get_expected_data() { @@ -252,4 +253,100 @@ class question_test extends \advanced_testcase { $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); $this->assertNull($options); } + + /** + * Helper method to make a simulated second version of the standard multianswer test question. + * + * The key think is that all the answer ids are changed (increased by 20). + * + * @param \qtype_multianswer_question $question + * @return \qtype_multianswer_question + */ + protected function make_second_version( + \qtype_multianswer_question $question): \qtype_multianswer_question { + $newquestion = fullclone($question); + + $newquestion->subquestions[1]->answers = [ + 36 => new \question_answer(16, 'Apple', 0.3333333, + 'Good', FORMAT_HTML), + 37 => new \question_answer(17, 'Burger', -0.5, + '', FORMAT_HTML), + 38 => new \question_answer(18, 'Hot dog', -0.5, + 'Not a fruit', FORMAT_HTML), + 39 => new \question_answer(19, 'Pizza', -0.5, + '', FORMAT_HTML), + 40 => new \question_answer(20, 'Orange', 0.3333333, + 'Correct', FORMAT_HTML), + 41 => new \question_answer(21, 'Banana', 0.3333333, + '', FORMAT_HTML), + ]; + + $newquestion->subquestions[2]->answers = [ + 42 => new \question_answer(22, 'Raddish', 0.5, + 'Good', FORMAT_HTML), + 43 => new \question_answer(23, 'Chocolate', -0.5, + '', FORMAT_HTML), + 44 => new \question_answer(24, 'Biscuit', -0.5, + 'Not a vegetable', FORMAT_HTML), + 45 => new \question_answer(25, 'Cheese', -0.5, + '', FORMAT_HTML), + 46 => new \question_answer(26, 'Carrot', 0.5, + 'Correct', FORMAT_HTML), + ]; + + return $newquestion; + } + + public function test_validate_can_regrade_with_other_version_ok() { + /** @var \qtype_multianswer_question $question */ + $question = \test_question_maker::make_question('multianswer', 'multiple'); + + $newquestion = $this->make_second_version($question); + + $this->assertNull($newquestion->validate_can_regrade_with_other_version($question)); + } + + public function test_validate_can_regrade_with_other_version_wrong_subquestions() { + /** @var \qtype_multianswer_question $question */ + $question = \test_question_maker::make_question('multianswer', 'multiple'); + + $newquestion = $this->make_second_version($question); + unset($newquestion->subquestions[2]); + + $this->assertEquals( + get_string('regradeissuenumsubquestionschanged', 'qtype_multianswer'), + $newquestion->validate_can_regrade_with_other_version($question)); + } + + public function test_validate_can_regrade_with_other_version_one_wrong_subquestion() { + /** @var \qtype_multianswer_question $question */ + $question = \test_question_maker::make_question('multianswer', 'multiple'); + + $newquestion = $this->make_second_version($question); + unset($newquestion->subquestions[1]->answers[41]); + + $this->assertEquals( + get_string('regradeissuenumchoiceschanged', 'qtype_multichoice'), + $newquestion->validate_can_regrade_with_other_version($question)); + } + + public function test_update_attempt_state_date_from_old_version_ok() { + /** @var \qtype_multianswer_question $question */ + $question = \test_question_maker::make_question('multianswer', 'multiple'); + + $newquestion = $this->make_second_version($question); + + $oldstep = new question_attempt_step(); + $oldstep->set_qt_var('_sub1_order', '16,17,18,19,20,21'); + $oldstep->set_qt_var('_sub2_order', '22,23,24,25,26'); + + $expected = [ + '_sub1_order' => '36,37,38,39,40,41', + '_sub2_order' => '42,43,44,45,46', + ]; + + $this->assertEquals($expected, + $newquestion->update_attempt_state_data_for_new_version($oldstep, $question)); + } + }