MDL-77090 questions: regrade of interactive attempts lose tries count

The behaviour-specific data was getting corrupted when the regrade
recreated the first step, because $oldstep->get_behaviour_data() was
stripping off the leading '-' characters from the names, and they were
not being added back.
This commit is contained in:
Tim Hunt 2023-01-31 12:33:45 +00:00
parent cb6dc699b8
commit f93fa35ecf
3 changed files with 42 additions and 5 deletions

View File

@ -1474,7 +1474,11 @@ class question_attempt {
} else {
$attemptstatedata = $this->get_question(false)->update_attempt_state_data_for_new_version(
$oldstep, $otherversion);
return array_merge($attemptstatedata, $oldstep->get_behaviour_data());
foreach ($oldstep->get_behaviour_data() as $name => $value) {
$attemptstatedata['-' . $name] = $value;
}
return $attemptstatedata;
}
}

View File

@ -362,7 +362,9 @@ class question_attempt_step {
/**
* Get all the behaviour variables.
* @param array name => value pairs.
*
* @return array name => value pairs. NOTE! the name has the leading - stripped off.
* (If you don't understand the note, read the comment at the top of this class :-))
*/
public function get_behaviour_data() {
$result = array();

View File

@ -18,11 +18,12 @@ namespace core_question;
use question_bank;
use question_display_options;
use question_state;
use test_question_maker;
defined('MOODLE_INTERNAL') || die();
global $CFG;
require_once(__DIR__ . '/..//lib.php');
require_once(__DIR__ . '/../lib.php');
require_once(__DIR__ . '/helpers.php');
@ -38,7 +39,7 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base {
public function test_regrade_does_not_lose_flag() {
// Create a true-false question with correct answer true.
$tf = \test_question_maker::make_question('truefalse', 'true');
$tf = test_question_maker::make_question('truefalse', 'true');
$this->start_attempt_at_question($tf, 'deferredfeedback', 2);
// Process a true answer.
@ -125,4 +126,34 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base {
preg_match_all("/<a ?.*>(.*)<\/a>/", $result, $matches);
$this->assertEquals(1, count($matches[0]));
}
/**
* @covers \question_usage_by_activity::regrade_question
* @covers \question_attempt::regrade
* @covers \question_attempt::get_attempt_state_data_to_regrade_with_version
*/
public function test_regrading_an_interactive_attempt_while_in_progress() {
// Start at attempt at a matching question.
$q = test_question_maker::make_question('match');
$this->start_attempt_at_question($q, 'interactive', 1);
$this->save_quba();
// Verify.
$this->check_current_state(question_state::$todo);
$this->check_current_mark(null);
$this->check_step_count(1);
$this->check_current_output($this->get_tries_remaining_expectation(1));
// Regrade the attempt.
// Duplicating the question here essential to triggering the bug we are trying to reproduce.
$reloadedquestion = clone($q);
$this->quba->regrade_question($this->slot, false, null, $reloadedquestion);
// Verify.
$this->check_current_state(question_state::$todo);
$this->check_current_mark(null);
$this->check_step_count(1);
$this->check_current_output($this->get_tries_remaining_expectation(1));
}
}