Commit 39abc011 authored by Tim Hunt's avatar Tim Hunt
Browse files

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.
parent 117b2401
......@@ -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.';
......
......@@ -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';
......
......@@ -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));
......
......@@ -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
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:
......
......@@ -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.
......
......@@ -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).
......
......@@ -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
......
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
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment