Commit f6579bea authored by Tim Hunt's avatar Tim Hunt
Browse files

MDL-40992 question engine: new ways modify question usages

* A method to change the max mark for one question_attempt in the usage

* A method to replace one question in a usage with another, moving the
old question_attempt to the end.

* Methods to set and get metadata (string name value pairs) for each
question_attempt in the usage. This gets stored in the first step in a
way that should not interfere with anything else.
parent 47be39ef
......@@ -252,6 +252,48 @@ class question_engine_data_mapper {
return $this->prepare_step_data($step, $record->id, $context);
}
/**
* Store new metadata for an existing {@link question_attempt} in the database.
*
* Private method, only for use by other parts of the question engine.
*
* @param question_attempt $qa the question attempt to store meta data for.
* @param array $names the names of the metadata variables to store.
* @return array of question_attempt_step_data rows, that still need to be inserted.
*/
public function insert_question_attempt_metadata(question_attempt $qa, array $names) {
$firststep = $qa->get_step(0);
$rows = array();
foreach ($names as $name) {
$data = new stdClass();
$data->attemptstepid = $firststep->get_id();
$data->name = ':_' . $name;
$data->value = $firststep->get_metadata_var($name);
$rows[] = $data;
}
return $rows;
}
/**
* Updates existing metadata for an existing {@link question_attempt} in the database.
*
* Private method, only for use by other parts of the question engine.
*
* @param question_attempt $qa the question attempt to store meta data for.
* @param array $names the names of the metadata variables to store.
* @return array of question_attempt_step_data rows, that still need to be inserted.
*/
public function update_question_attempt_metadata(question_attempt $qa, array $names) {
global $DB;
list($condition, $params) = $DB->get_in_or_equal($names);
$params[] = $qa->get_step(0)->get_id();
$DB->delete_records_select('question_attempt_step_data',
'name ' . $condition . ' AND attemptstepid = ?', $params);
return $this->insert_question_attempt_metadata($qa, $names);
}
/**
* Load a {@link question_attempt_step} from the database.
*
......@@ -867,6 +909,7 @@ ORDER BY
public function update_question_attempt(question_attempt $qa) {
$record = new stdClass();
$record->id = $qa->get_database_id();
$record->slot = $qa->get_slot();
$record->variant = $qa->get_variant();
$record->maxmark = $qa->get_max_mark();
$record->minfraction = $qa->get_min_fraction();
......@@ -880,16 +923,6 @@ ORDER BY
$this->db->update_record('question_attempts', $record);
}
/**
* Delete a question_attempts row to reflect any changes in a question_attempt
* (but not any of its steps).
* @param question_attempt $qa the question attempt that has been deleted.
*/
public function delete_question_attempt(question_attempt $qa) {
$conditions = array('questionusageid' => $qa->get_usage_id(), 'slot' => $qa->get_slot());
$this->db->delete_records('question_attempts', $conditions);
}
/**
* Delete a question_usage_by_activity and all its associated
*
......@@ -1256,21 +1289,15 @@ class question_engine_unit_of_work implements question_usage_observer {
/**
* @var array list of slot => {@link question_attempt}s that
* were already in the usage, and which have been modified.
*/
protected $attemptsmodified = array();
/**
* @var array list of slot => {@link question_attempt}s that
* were already in the usage, and which have been deleted.
* have been added to the usage.
*/
protected $attemptsdeleted = array();
protected $attemptsadded = array();
/**
* @var array list of slot => {@link question_attempt}s that
* have been added to the usage.
* were already in the usage, and which have been modified.
*/
protected $attemptsadded = array();
protected $attemptsmodified = array();
/**
* @var array of array(question_attempt_step, question_attempt id, seq number)
......@@ -1290,6 +1317,16 @@ class question_engine_unit_of_work implements question_usage_observer {
*/
protected $stepsdeleted = array();
/**
* @var array int slot => string name => question_attempt.
*/
protected $metadataadded = array();
/**
* @var array int slot => string name => question_attempt.
*/
protected $metadatamodified = array();
/**
* Constructor.
* @param question_usage_by_activity $quba the usage to track.
......@@ -1302,6 +1339,10 @@ class question_engine_unit_of_work implements question_usage_observer {
$this->modified = true;
}
public function notify_attempt_added(question_attempt $qa) {
$this->attemptsadded[$qa->get_slot()] = $qa;
}
public function notify_attempt_modified(question_attempt $qa) {
$slot = $qa->get_slot();
if (!array_key_exists($slot, $this->attemptsadded)) {
......@@ -1309,27 +1350,27 @@ class question_engine_unit_of_work implements question_usage_observer {
}
}
/**
* Notify when attempt deleted
*
* @see question_usage_observer::notify_attempt_deleted()
*/
public function notify_attempt_deleted(question_attempt $qa) {
$slot = $qa->get_slot();
if (!array_key_exists($slot, $this->attemptsadded)) {
$this->attemptsdeleted[$slot] = $qa;
public function notify_attempt_moved(question_attempt $qa, $oldslot) {
$newslot = $qa->get_slot();
if (array_key_exists($oldslot, $this->attemptsadded)) {
unset($this->attemptsadded[$oldslot]);
$this->attemptsadded[$newslot] = $qa;
return;
}
}
/**
* Notify when attempt added
*
* @see question_usage_observer::notify_attempt_added()
*/
public function notify_attempt_added(question_attempt $qa) {
$slot = $qa->get_slot();
if (!array_key_exists($slot, $this->attemptsadded)) {
$this->attemptsadded[$slot] = $qa;
if (array_key_exists($oldslot, $this->attemptsmodified)) {
unset($this->attemptsmodified[$oldslot]);
}
$this->attemptsmodified[$newslot] = $qa;
if (array_key_exists($oldslot, $this->metadataadded)) {
$this->metadataadded[$newslot] = $this->metadataadded[$oldslot];
unset($this->metadataadded[$oldslot]);
}
if (array_key_exists($oldslot, $this->metadatamodified)) {
$this->metadatamodified[$newslot] = $this->metadatamodified[$oldslot];
unset($this->metadatamodified[$oldslot]);
}
}
......@@ -1407,6 +1448,42 @@ class question_engine_unit_of_work implements question_usage_observer {
$this->stepsdeleted[$stepid] = $step;
}
public function notify_metadata_added(question_attempt $qa, $name) {
if (array_key_exists($qa->get_slot(), $this->attemptsadded)) {
return;
}
if ($this->is_step_added($qa->get_step(0)) !== false) {
return;
}
if (isset($this->metadataadded[$qa->get_slot()][$name])) {
return;
}
$this->metadataadded[$qa->get_slot()][$name] = $qa;
}
public function notify_metadata_modified(question_attempt $qa, $name) {
if (array_key_exists($qa->get_slot(), $this->attemptsadded)) {
return;
}
if ($this->is_step_added($qa->get_step(0)) !== false) {
return;
}
if (isset($this->metadataadded[$qa->get_slot()][$name])) {
return;
}
if (isset($this->metadatamodified[$qa->get_slot()][$name])) {
return;
}
$this->metadatamodified[$qa->get_slot()][$name] = $qa;
}
/**
* @param question_attempt_step $step a step
* @return int|false if the step is in the list of steps to be added, return
......@@ -1473,8 +1550,8 @@ class question_engine_unit_of_work implements question_usage_observer {
$step, $questionattemptid, $seq, $this->quba->get_owning_context());
}
foreach ($this->attemptsdeleted as $qa) {
$dm->delete_question_attempt($qa);
foreach ($this->attemptsmodified as $qa) {
$dm->update_question_attempt($qa);
}
foreach ($this->attemptsadded as $qa) {
......@@ -1482,18 +1559,31 @@ class question_engine_unit_of_work implements question_usage_observer {
$qa, $this->quba->get_owning_context());
}
foreach ($this->attemptsmodified as $qa) {
$dm->update_question_attempt($qa);
foreach ($this->metadataadded as $info) {
$qa = reset($info);
$stepdata[] = $dm->insert_question_attempt_metadata($qa, array_keys($info));
}
foreach ($this->metadatamodified as $info) {
$qa = reset($info);
$stepdata[] = $dm->update_question_attempt_metadata($qa, array_keys($info));
}
if ($this->modified) {
$dm->update_questions_usage_by_activity($this->quba);
}
if (!$stepdata) {
return;
if ($stepdata) {
$dm->insert_all_step_data(call_user_func_array('array_merge', $stepdata));
}
$dm->insert_all_step_data(call_user_func_array('array_merge', $stepdata));
$this->stepsdeleted = array();
$this->stepsmodified = array();
$this->stepsadded = array();
$this->attemptsdeleted = array();
$this->attemptsadded = array();
$this->attemptsmodified = array();
$this->modified = false;
}
}
......
......@@ -362,7 +362,7 @@ class question_attempt {
/**
* Get one of the steps in this attempt.
*
* @param int $i the step number.
* @param int $i the step number, which counts from 0.
* @return question_attempt_step
*/
public function get_step($i) {
......@@ -748,6 +748,30 @@ class question_attempt {
return $this->behaviour->summarise_action($step);
}
/**
* Return one of the bits of metadata for a this question attempt.
* @param string $name the name of the metadata variable to return.
* @return string the value of that metadata variable.
*/
public function get_metadata($name) {
return $this->get_step(0)->get_metadata_var($name);
}
/**
* Set some metadata for this question attempt.
* @param string $name the name of the metadata variable to return.
* @param string $value the value to set that metadata variable to.
*/
public function set_metadata($name, $value) {
$firststep = $this->get_step(0);
if (!$firststep->has_metadata_var($name)) {
$this->observer->notify_metadata_added($this, $name);
} else if ($value !== $firststep->get_metadata_var($name)) {
$this->observer->notify_metadata_modified($this, $name);
}
$firststep->set_metadata_var($name, $value);
}
/**
* Helper function used by {@link rewrite_pluginfile_urls()} and
* {@link rewrite_response_pluginfile_urls()}.
......@@ -931,6 +955,10 @@ class question_attempt {
public function start($preferredbehaviour, $variant, $submitteddata = array(),
$timestamp = null, $userid = null, $existingstepid = null) {
if ($this->get_num_steps() > 0) {
throw new coding_exception('Cannot start a question that is already started.');
}
// Initialise the behaviour.
$this->variant = $variant;
if (is_string($preferredbehaviour)) {
......@@ -1266,6 +1294,15 @@ class question_attempt {
}
}
/**
* Change the max mark for this question_attempt.
* @param float $maxmark the new max mark.
*/
public function set_max_mark($maxmark) {
$this->maxmark = $maxmark;
$this->observer->notify_attempt_modified($this);
}
/**
* Perform a manual grading action on this attempt.
* @param string $comment the comment being added.
......
......@@ -370,6 +370,48 @@ class question_attempt_step {
return $this->data;
}
/**
* 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()}.
*
* @param string $name the name of the variable to set. [a-z][a-z0-9]*.
* @param string $value the value to set.
*/
public function set_metadata_var($name, $value) {
$this->data[':_' . $name] = $value;
}
/**
* 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()}.
*
* @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.
*/
public function has_metadata_var($name) {
return isset($this->data[':_' . $name]);
}
/**
* 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()}.
*
* @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.
*/
public function get_metadata_var($name) {
if (!$this->has_metadata_var($name)) {
return null;
}
return $this->data[':_' . $name];
}
/**
* Create a question_attempt_step from records loaded from the database.
* @param Iterator $records Raw records loaded from the database.
......
......@@ -172,6 +172,42 @@ class question_usage_by_activity {
return $qa->get_slot();
}
/**
* Add another question to this usage, in the place of an existing slot.
* The question_attempt that was in that slot is moved to the end at a new
* slot number, which is returned.
*
* The added question is not started until you call {@link start_question()}
* on it.
*
* @param int $slot the slot-number of the question to replace.
* @param question_definition $question the question to add.
* @param number $maxmark the maximum this question will be marked out of in
* this attempt (optional). If not given, the max mark from the $qa we
* are replacing is used.
* @return int the new slot number of the question that was displaced.
*/
public function add_question_in_place_of_other($slot, question_definition $question, $maxmark = null) {
$newslot = $this->next_slot_number();
$oldqa = $this->get_question_attempt($slot);
$oldqa->set_slot($newslot);
$this->questionattempts[$newslot] = $oldqa;
if ($maxmark === null) {
$maxmark = $oldqa->get_max_mark();
}
$qa = new question_attempt($question, $this->get_id(), $this->observer, $maxmark);
$qa->set_slot($slot);
$this->questionattempts[$slot] = $qa;
$this->observer->notify_attempt_moved($oldqa, $slot);
$this->observer->notify_attempt_added($qa);
return $newslot;
}
/**
* The slot number that will be allotted to the next question added.
*/
......@@ -377,6 +413,27 @@ class question_usage_by_activity {
return $this->get_question_attempt($slot)->get_right_answer_summary();
}
/**
* Return one of the bits of metadata for a particular question attempt in
* this usage.
* @param int $slot the slot number of the question of inereest.
* @param string $name the name of the metadata variable to return.
* @return string the value of that metadata variable.
*/
public function get_question_attempt_metadata($slot, $name) {
return $this->get_question_attempt($slot)->get_metadata($name);
}
/**
* Set some metadata for a particular question attempt in this usage.
* @param int $slot the slot number of the question of inerest.
* @param string $name the name of the metadata variable to return.
* @param string $value the value to set that metadata variable to.
*/
public function set_question_attempt_metadata($slot, $name, $value) {
$this->get_question_attempt($slot)->set_metadata($name, $value);
}
/**
* Get the {@link core_question_renderer}, in collaboration with appropriate
* {@link qbehaviour_renderer} and {@link qtype_renderer} subclasses, to generate the
......@@ -822,22 +879,6 @@ class question_usage_by_activity {
$this->observer->notify_attempt_modified($newqa);
}
/**
* Replace a question in this usage.
* @param int $slot the number used to identify this question within this usage.*
*/
public function replace_question($slot) {
global $OUTPUT;
$oldqa = $this->get_question_attempt($slot);
$newqa = new question_attempt($oldqa->get_question(), $oldqa->get_usage_id(), $this->observer);
$newqa->set_database_id($oldqa->get_database_id());
$newqa->set_slot($oldqa->get_slot());
$this->questionattempts[$slot] = $newqa;
$this->observer->notify_attempt_deleted($oldqa);
$this->observer->notify_attempt_added($newqa);
$this->start_question($slot);
}
/**
* Regrade all the questions in this usage (without changing their max mark).
* @param bool $finished whether each question should be forced to be finished
......@@ -849,6 +890,15 @@ class question_usage_by_activity {
}
}
/**
* Change the max mark for this question_attempt.
* @param int $slot the slot number of the question of inerest.
* @param float $maxmark the new max mark.
*/
public function set_max_mark($slot, $maxmark) {
$this->get_question_attempt($slot)->set_max_mark($maxmark);
}
/**
* Create a question_usage_by_activity from records loaded from the database.
*
......@@ -984,22 +1034,23 @@ interface question_usage_observer {
public function notify_modified();
/**
* Called when the fields of a question attempt in this usage are modified.
* Called when a new question attempt is added to this usage.
* @param question_attempt $qa the newly added question attempt.
*/
public function notify_attempt_modified(question_attempt $qa);
public function notify_attempt_added(question_attempt $qa);
/**
* Called when a new question attempt is added to this usage.
* Called when the fields of a question attempt in this usage are modified.
* @param question_attempt $qa the newly added question attempt.
*/
public function notify_attempt_added(question_attempt $qa);
public function notify_attempt_modified(question_attempt $qa);
/**
* Called when the fields of a question attempt in this usage are deleted.
* @param question_attempt $qa
* Called when a question_attempt has been moved to a new slot.
* @param question_attempt $qa The question attempt that was moved.
* @param int $oldslot The previous slot number of that attempt.
*/
public function notify_attempt_deleted(question_attempt $qa);
public function notify_attempt_moved(question_attempt $qa, $oldslot);
/**
* Called when a new step is added to a question attempt in this usage.
......@@ -1024,6 +1075,19 @@ interface question_usage_observer {
*/
public function notify_step_deleted(question_attempt_step $step, question_attempt $qa);
/**
* Called when a new metadata variable is set on a question attempt in this usage.
* @param question_attempt $qa the question attempt the metadata is being added to.
* @param int $name the name of the metadata variable added.
*/
public function notify_metadata_added(question_attempt $qa, $name);
/**
* Called when a metadata variable on a question attempt in this usage is updated.
* @param question_attempt $qa the question attempt where the metadata is being modified.
* @param int $name the name of the metadata variable modified.
*/
public function notify_metadata_modified(question_attempt $qa, $name);
}
......@@ -1037,11 +1101,11 @@ interface question_usage_observer {
class question_usage_null_observer implements question_usage_observer {
public function notify_modified() {
}
public function notify_attempt_added(question_attempt $qa) {
}
public function notify_attempt_modified(question_attempt $qa) {
}
public function notify_attempt_deleted(question_attempt $qa) {
}
public function notify_attempt_added(question_attempt $qa) {
public function notify_attempt_moved(question_attempt $qa, $oldslot) {
}
public function notify_step_added(question_attempt_step $step, question_attempt $qa, $seq) {
}
......@@ -1049,4 +1113,8 @@ class question_usage_null_observer implements question_usage_observer {
}
public function notify_step_deleted(question_attempt_step $step, question_attempt $qa) {
}
public function notify_metadata_added(question_attempt $qa, $name) {
}
public function notify_metadata_modified(question_attempt $qa, $name) {
}
}
......@@ -84,6 +84,14 @@ class testable_question_engine_unit_of_work extends question_engine_unit_of_work
public function get_steps_deleted() {
return $this->stepsdeleted;
}
public function get_metadata_added() {
return $this->metadataadded;
}
public function get_metadata_modified() {
return $this->metadatamodified;
}
}
......
......@@ -91,8 +91,8 @@ class question_engine_unit_of_work_test extends data_loading_method_test_base {
array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 1.0000000, 0.0000000, 1.0000000, 0, '', '', '', 1256233790, 2, 1, 'todo', null, 1256233720, 1, '-submit', 1),
array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 1.0000000, 0.0000000, 1.0000000, 0, '', '', '', 1256233790, 2, 1, 'todo', null, 1256233720, 1, '-_triesleft', 1),
array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 1.0000000, 0.0000000, 1.0000000, 0, '', '', '', 1256233790, 3, 2, 'todo', null, 1256233740, 1, '-tryagain', 1),
array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 1.0000000, 0.0000000, 1.0000000, 0, '', '', '', 1256233790, 5, 3, 'gradedright', null, 1256233790, 1, 'answer', 'frog'),
array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 1.0000000, 0.0000000, 1.0000000, 0, '', '', '', 1256233790, 5, 3, 'gradedright', 1.0000000, 1256233790, 1, '-submit', 1),
array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 1.0000000, 0.0000000, 1.0000000, 0, '', '', '', 1256233790, 5, 3, 'gradedright', 0.6666667, 1256233790, 1, 'answer', 'frog'),
array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 1.0000000, 0.0000000, 1.0000000, 0, '', '', '', 1256233790, 5, 3, 'gradedright', 0.6666667, 1256233790, 1, '-submit', 1),
);
}
......@@ -103,6 +103,8 @@ class question_engine_unit_of_work_test extends data_loading_method_test_base {
$this->assertEquals(0, count($this->observer->get_steps_added()));
$this->assertEquals(0, count($this->observer->get_steps_modified()));
$this->assertEquals(0, count($this->observer->get_steps_deleted()));
$this->assertEquals(0, count($this->observer->get_metadata_added()));
$this->assertEquals(0, count($this->observer->get_metadata_modified()));
}
public function test_update_usage() {
......@@ -120,6 +122,9 @@ class question_engine_unit_of_work_test extends data_loading_method_test_base {
$this->assertEquals(1, count($newattempts));
$this->assertTrue($this->quba->get_question_attempt($slot) === reset($newattempts));
$this->assertSame($slot, key($newattempts));
$this->assertEquals(0, count($this->observer->get_metadata_added()));
$this->assertEquals(0, count($this->observer->get_metadata_modified()));
}
public function test_add_and_start_question() {
......@@ -136,6 +141,9 @@ class question_engine_unit_of_work_test extends data_loading_method_test_base {
$this->assertTrue($this->quba->get_question_attempt($slot) === reset($newattempts));
$this->assertSame($slot, key($newattempts));
$this->assertEquals(0, count($this->observer->get_steps_added()));
$this->assertEquals(0, count($this->observer->get_metadata_added()));
$this->assertEquals(0, count($this->observer->get_metadata_modified()));
}