mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-03-22 08:30:04 +01:00
Code Issues Projects Releases Wiki Activity

Merge branch 'MDL-77464_401' of https://github.com/timhunt/moodle into MOODLE_401_STABLE

Browse Source
This commit is contained in:
Sara Arjona 2023-03-08 16:14:47 +01:00
commit 6aafa1eea6
4 changed files with 80 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
mod/quiz/tests/behat/attempt_redo_questions.feature
View File

@ -55,6 +55,10 @@ Feature: Allow students to redo questions in a practice quiz, without starting a
Then I should see "Finished regrading (1/1)"
And I should see "Regrade completed"
And I press "Continue"
# Regrade a second time, to ensure the first regrade did not corrupt any data.
And I press "Regrade all"
And I should see "Finished regrading (1/1)"
And I should see "Regrade completed"
@javascript
Scenario: Start attempt, teacher edits question, redo picks up latest non-draft version

9
question/engine/questionattempt.php
View File

@ -1472,11 +1472,16 @@ class question_attempt {
if ($this->get_question(false) === $otherversion) {
return $oldstep->get_all_data();
} else {
// Update the data belonging to the question type by asking the question to do it.
$attemptstatedata = $this->get_question(false)->update_attempt_state_data_for_new_version(
$oldstep, $otherversion);
foreach ($oldstep->get_behaviour_data() as $name => $value) {
$attemptstatedata['-' . $name] = $value;
// Then copy over all the behaviour and metadata variables.
// This terminology is explained in the class comment on {@see question_attempt_step}.
foreach ($oldstep->get_all_data() as $name => $value) {
if (substr($name, 0, 1) === '-' || substr($name, 0, 2) === ':_') {
$attemptstatedata[$name] = $value;
}
}
return $attemptstatedata;
}

66
question/engine/questionattemptstep.php
View File

@ -28,10 +28,10 @@ defined('MOODLE_INTERNAL') || die();
/**
* Stores one step in a {@link question_attempt}.
* Stores one step in a {@see question_attempt}.
*
* The most important attributes of a step are the state, which is one of the
* {@link question_state} constants, the fraction, which may be null, or a
* {@see question_state} constants, the fraction, which may be null, or a
* number bewteen the attempt's minfraction and maxfraction, and the array of submitted
* data, about which more later.
*
@ -39,8 +39,7 @@ defined('MOODLE_INTERNAL') || die();
* creating it.
*
* The submitted data is basically just an array of name => value pairs, with
* certain conventions about the to divide the variables into four = two times two
* categories.
* certain conventions about the to divide the variables into five = 2 x 2 + 1 categories.
*
* Variables may either belong to the behaviour, in which case the
* name starts with a '-', or they may belong to the question type in which case
@ -50,15 +49,21 @@ defined('MOODLE_INTERNAL') || die();
* which case the name does not start with an _, or they are cached values that
* were created during processing, in which case the name does start with an _.
*
* That is, each name will start with one of '', '_'. '-' or '-_'. The remainder
* of the name should match the regex [a-z][a-z0-9]*.
* In addition, we can store 'metadata', typically only in the first step of a
* question attempt. These are stored with the initial characters ':_'.
*
* These variables can be accessed with {@link get_behaviour_var()} and {@link get_qt_var()},
* That is, each name will start with one of '', '_', '-', '-_' or ':_'. The remainder
* of the name was supposed to match the regex [a-z][a-z0-9]* - but this has never
* been enforced. Question types exist which break this rule. E.g. qtype_combined.
* Perhpas now, an accurate regex would be [a-z][a-z0-9_:]*.
*
* These variables can be accessed with {@see get_behaviour_var()} and {@see get_qt_var()},
* - to be clear, ->get_behaviour_var('x') gets the variable with name '-x' -
* and values whose names start with '_' can be set using {@link set_behaviour_var()}
* and {@link set_qt_var()}. There are some other methods like {@link has_behaviour_var()}
* to check wether a varaible with a particular name is set, and {@link get_behaviour_data()}
* to get all the behaviour data as an associative array.
* and values whose names start with '_' can be set using {@see set_behaviour_var()}
* and {@see set_qt_var()}. There are some other methods like {@see has_behaviour_var()}
* to check wether a varaible with a particular name is set, and {@see get_behaviour_data()}
* to get all the behaviour data as an associative array. There are also
* {@see get_metadata_var()}, {@see set_metadata_var()} and {@see has_metadata_var()},
*
* @copyright 2009 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
@ -71,7 +76,7 @@ class question_attempt_step {
private $id = null;
/**
* @var question_state one of the {@link question_state} constants.
* @var question_state one of the {@see question_state} constants.
* The state after this step.
*/
private $state;
@ -91,7 +96,7 @@ class question_attempt_step {
/** @var array name => value pairs. The submitted data. */
private $data;
/** @var array name => array of {@link stored_file}s. Caches the contents of file areas. */
/** @var array name => array of {@see stored_file}s. Caches the contents of file areas. */
private $files = array();
/** @var stdClass User information. */
@ -99,8 +104,8 @@ class question_attempt_step {
/**
* You should not need to call this constructor in your own code. Steps are
* normally created by {@link question_attempt} methods like
* {@link question_attempt::process_action()}.
* normally created by {@see question_attempt} methods like
* {@see question_attempt::process_action()}.
* @param array $data the submitted data that defines this step.
* @param int $timestamp the time to record for the action. (If not given, use now.)
* @param int $userid the user to attribute the aciton to. (If not given, use the current user.)
@ -147,7 +152,7 @@ class question_attempt_step {
/**
* Set the state. Normally only called by behaviours.
* @param question_state $state one of the {@link question_state} constants.
* @param question_state $state one of the {@see question_state} constants.
*/
public function set_state($state) {
$this->state = $state;
@ -248,7 +253,8 @@ class question_attempt_step {
* type question_attempt::PARAM_FILES.
*
* @param string $name the name of the associated variable.
* @return array of {@link stored_files}.
* @param int $contextid contextid of the question attempt
* @return array of {@see stored_files}.
*/
public function get_qt_files($name, $contextid) {
if (array_key_exists($name, $this->files)) {
@ -300,7 +306,7 @@ class question_attempt_step {
/**
* Rewrite the @@PLUGINFILE@@ tokens in a response variable from this step
* that contains links to file. Normally you should probably call
* {@link question_attempt::rewrite_response_pluginfile_urls()} instead of
* {@see question_attempt::rewrite_response_pluginfile_urls()} instead of
* calling this method directly.
*
* @param string $text the text to update the URLs in.
@ -379,7 +385,7 @@ class question_attempt_step {
/**
* Get all the submitted data, but not the cached data. behaviour
* variables have the - at the start of their name. This is only really
* intended for use by {@link question_attempt::regrade()}, it should not
* intended for use by {@see question_attempt::regrade()}, it should not
* be considered part of the public API.
* @param array name => value pairs.
*/
@ -397,7 +403,7 @@ class question_attempt_step {
/**
* Get all the data. behaviour variables have the - at the start of
* their name. This is only intended for internal use, for example by
* {@link question_engine_data_mapper::insert_question_attempt_step()},
* {@see question_engine_data_mapper::insert_question_attempt_step()},
* however, it can occasionally be useful in test code. It should not be
* considered part of the public API of this class.
* @param array name => value pairs.
@ -410,7 +416,7 @@ class question_attempt_step {
* Set a metadata variable.
*
* Do not call this method directly from your code. It is for internal
* use only. You should call {@link question_usage::set_question_attempt_metadata()}.
* use only. You should call {@see question_usage::set_question_attempt_metadata()}.
*
* @param string $name the name of the variable to set. [a-z][a-z0-9]*.
* @param string $value the value to set.
@ -423,7 +429,7 @@ class question_attempt_step {
* Whether this step has a metadata variable.
*
* Do not call this method directly from your code. It is for internal
* use only. You should call {@link question_usage::get_question_attempt_metadata()}.
* use only. You should call {@see question_usage::get_question_attempt_metadata()}.
*
* @param string $name the name of the variable to set. [a-z][a-z0-9]*.
* @return bool the value to set previously, or null if this variable was never set.
@ -436,7 +442,7 @@ class question_attempt_step {
* Get a metadata variable.
*
* Do not call this method directly from your code. It is for internal
* use only. You should call {@link question_usage::get_question_attempt_metadata()}.
* use only. You should call {@see question_usage::get_question_attempt_metadata()}.
*
* @param string $name the name of the variable to set. [a-z][a-z0-9]*.
* @return string the value to set previously, or null if this variable was never set.
@ -510,10 +516,10 @@ class question_attempt_step {
/**
* A subclass of {@link question_attempt_step} used when processing a new submission.
* A subclass of {@see question_attempt_step} used when processing a new submission.
*
* When we are processing some new submitted data, which may or may not lead to
* a new step being added to the {@link question_usage_by_activity} we create an
* a new step being added to the {@see question_usage_by_activity} we create an
* instance of this class. which is then passed to the question behaviour and question
* type for processing. At the end of processing we then may, or may not, keep it.
*
@ -583,7 +589,7 @@ class question_attempt_pending_step extends question_attempt_step {
/**
* A subclass of {@link question_attempt_step} that cannot be modified.
* A subclass of {@see question_attempt_step} that cannot be modified.
*
* @copyright 2009 The Open University
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
@ -605,8 +611,8 @@ class question_attempt_step_read_only extends question_attempt_step {
/**
* A null {@link question_attempt_step} returned from
* {@link question_attempt::get_last_step()} etc. when a an attempt has just been
* A null {@see question_attempt_step} returned from
* {@see question_attempt::get_last_step()} etc. when a an attempt has just been
* created and there is no actual step.
*
* @copyright 2009 The Open University
@ -628,7 +634,7 @@ class question_null_step {
/**
* This is an adapter class that wraps a {@link question_attempt_step} and
* This is an adapter class that wraps a {@see question_attempt_step} and
* modifies the get/set_*_data methods so that they operate only on the parts
* that belong to a particular subquestion, as indicated by an extra prefix.
*
@ -687,7 +693,7 @@ class question_attempt_step_subquestion_adapter extends question_attempt_step {
* Filter some data to keep only those entries where the key contains
* extraprefix, and remove the extra prefix from the reutrned arrary.
* @param array $data some of the data stored in this step.
* @return array the data with the keys ajusted using {@link remove_prefix()}.
* @return array the data with the keys ajusted using {@see remove_prefix()}.
*/
public function filter_array($data) {
$result = array();

34
question/engine/tests/walkthrough_test.php
View File

@ -134,7 +134,7 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base {
*/
public function test_regrading_an_interactive_attempt_while_in_progress() {
// Start at attempt at a matching question.
// Start an attempt at a matching question.
$q = test_question_maker::make_question('match');
$this->start_attempt_at_question($q, 'interactive', 1);
$this->save_quba();
@ -156,4 +156,36 @@ class walkthrough_test extends \qbehaviour_walkthrough_test_base {
$this->check_step_count(1);
$this->check_current_output($this->get_tries_remaining_expectation(1));
}
/**
* @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_does_not_lose_metadata() {
// Start an attempt at a matching question.
$q = test_question_maker::make_question('match');
$this->start_attempt_at_question($q, 'interactive', 1);
// Like in process_redo_question in mod_quiz.
$this->quba->set_question_attempt_metadata($this->slot, 'originalslot', 42);
$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.
$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));
$this->assertEquals(42, $this->quba->get_question_attempt_metadata($this->slot, 'originalslot'));
}
}