Skip to content

Commit

Permalink
MDL-58026 question: fix regrading in progress attempt
Browse files Browse the repository at this point in the history
Do not convert autosave steps to true steps when regrading.

Thanks Eric Merrill for providing the unit test code.
  • Loading branch information
MartinGauk committed Jul 8, 2019
1 parent b198428 commit 96c24a1
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 2 deletions.
9 changes: 7 additions & 2 deletions question/engine/questionattempt.php
Original file line number Diff line number Diff line change
Expand Up @@ -1369,8 +1369,13 @@ public function regrade(question_attempt $oldqa, $finished) {

} else {
// This is the normal case. Replay the next step of the attempt.
$this->process_action($step->get_submitted_data(),
$step->get_timecreated(), $step->get_user_id(), $step->get_id());
if ($step === $oldqa->autosavedstep) {
$this->process_autosave($step->get_submitted_data(),
$step->get_timecreated(), $step->get_user_id());
} else {
$this->process_action($step->get_submitted_data(),
$step->get_timecreated(), $step->get_user_id(), $step->get_id());
}
}
}

Expand Down
61 changes: 61 additions & 0 deletions question/engine/tests/questionusage_autosave_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,67 @@ public function test_finish_with_unhandled_autosave_data() {
$this->delete_quba();
}

/**
* Test that regrading doesn't convert autosave steps to finished steps.
* This can result in students loosing data (due to question_out_of_sequence_exception) if a teacher
* regrades an attempt while it is in progress.
*/
public function test_autosave_and_regrade_then_display() {
$this->resetAfterTest();
$generator = $this->getDataGenerator()->get_plugin_generator('core_question');
$cat = $generator->create_question_category();
$question = $generator->create_question('shortanswer', null,
array('category' => $cat->id));

// Start attempt at a shortanswer question.
$q = question_bank::load_question($question->id);
$this->start_attempt_at_question($q, 'deferredfeedback', 1);

$this->check_current_state(question_state::$todo);
$this->check_current_mark(null);
$this->check_step_count(1);

// First see if the starting sequence is right.
$this->render();
$this->check_output_contains_hidden_input(':sequencecheck', 1);

// Add a submission.
$this->process_submission(array('answer' => 'first response'));
$this->save_quba();

// Check the submission and that the sequence went up.
$this->render();
$this->check_output_contains_text_input('answer', 'first response');
$this->check_output_contains_hidden_input(':sequencecheck', 2);
$this->assertFalse($this->get_question_attempt()->has_autosaved_step());

// Add a autosave response.
$this->load_quba();
$this->process_autosave(array('answer' => 'second response'));
$this->save_quba();

// Confirm that the autosave value shows up, but that the sequence hasn't increased.
$this->render();
$this->check_output_contains_text_input('answer', 'second response');
$this->check_output_contains_hidden_input(':sequencecheck', 2);
$this->assertTrue($this->get_question_attempt()->has_autosaved_step());

// Call regrade.
$this->load_quba();
$this->quba->regrade_all_questions();
$this->save_quba();

// Check and see if the autosave response is still there, that the sequence didn't increase,
// and that there is an autosave step.
$this->load_quba();
$this->render();
$this->check_output_contains_text_input('answer', 'second response');
$this->check_output_contains_hidden_input(':sequencecheck', 2);
$this->assertTrue($this->get_question_attempt()->has_autosaved_step());

$this->delete_quba();
}

protected function tearDown() {
// This test relies on the destructor for the second DB connection being called before running the next test.
// Without this change - there will be unit test failures on "some" DBs (MySQL).
Expand Down

0 comments on commit 96c24a1

Please sign in to comment.