Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge branch 'MDL-30484_22' of git://github.com/timhunt/moodle into M…

…OODLE_22_STABLE
  • Loading branch information...
commit 26be1bf17a3f608712759724283c592554c972fd 2 parents 5a1e6f2 + 699f075
@stronk7 stronk7 authored
View
251 question/engine/datalib.php
@@ -17,6 +17,25 @@
/**
* Code for loading and saving question attempts to and from the database.
*
+ * A note for future reference. This code is pretty efficient but there are two
+ * potential optimisations that could be contemplated, at the cost of making the
+ * code more complex:
+ *
+ * 1. (This is the easier one, but probably not worth doing.) In the unit-of-work
+ * save method, we could get all the ids for steps due to be deleted or modified,
+ * and delete all the question_attempt_step_data for all of those steps in one
+ * query. That would save one DB query for each ->stepsupdated. However that number
+ * is 0 except when re-grading, and when regrading, there are many more inserts
+ * into question_attempt_step_data than deletes, so it is really hardly worth it.
+ *
+ * 2. A more significant optimisation would be to write an efficient
+ * $DB->insert_records($arrayofrecords) method (for example using functions
+ * like pg_copy_from) and then whenever we save stuff (unit_of_work->save and
+ * insert_questions_usage_by_activity) collect together all the records that
+ * need to be inserted into question_attempt_step_data, and insert them with
+ * a single call to $DB->insert_records. This is likely to be the biggest win.
+ * We do a lot of separate inserts into question_attempt_step_data.
+ *
* @package moodlecore
* @subpackage questionengine
* @copyright 2009 The Open University
@@ -76,7 +95,7 @@ public function insert_questions_usage_by_activity(question_usage_by_activity $q
* Store an entire {@link question_attempt} in the database,
* including all the question_attempt_steps that comprise it.
* @param question_attempt $qa the question attempt to store.
- * @param object $context the context of the owning question_usage_by_activity.
+ * @param context $context the context of the owning question_usage_by_activity.
*/
public function insert_question_attempt(question_attempt $qa, $context) {
$record = new stdClass();
@@ -105,14 +124,13 @@ public function insert_question_attempt(question_attempt $qa, $context) {
}
/**
- * Store a {@link question_attempt_step} in the database.
- * @param question_attempt_step $qa the step to store.
+ * Helper method used by insert_question_attempt_step and update_question_attempt_step
+ * @param question_attempt_step $step the step to store.
* @param int $questionattemptid the question attept id this step belongs to.
* @param int $seq the sequence number of this stop.
- * @param object $context the context of the owning question_usage_by_activity.
+ * @return stdClass data to insert into the database.
*/
- public function insert_question_attempt_step(question_attempt_step $step,
- $questionattemptid, $seq, $context) {
+ protected function make_step_record(question_attempt_step $step, $questionattemptid, $seq) {
$record = new stdClass();
$record->questionattemptid = $questionattemptid;
$record->sequencenumber = $seq;
@@ -120,16 +138,23 @@ public function insert_question_attempt_step(question_attempt_step $step,
$record->fraction = $step->get_fraction();
$record->timecreated = $step->get_timecreated();
$record->userid = $step->get_user_id();
+ return $record;
+ }
- $record->id = $this->db->insert_record('question_attempt_steps', $record);
-
+ /**
+ * Helper method used by insert_question_attempt_step and update_question_attempt_step
+ * @param question_attempt_step $step the step to store.
+ * @param int $stepid the id of the step.
+ * @param context $context the context of the owning question_usage_by_activity.
+ */
+ protected function insert_step_data(question_attempt_step $step, $stepid, $context) {
foreach ($step->get_all_data() as $name => $value) {
if ($value instanceof question_file_saver) {
- $value->save_files($record->id, $context);
+ $value->save_files($stepid, $context);
}
$data = new stdClass();
- $data->attemptstepid = $record->id;
+ $data->attemptstepid = $stepid;
$data->name = $name;
$data->value = $value;
$this->db->insert_record('question_attempt_step_data', $data, false);
@@ -137,6 +162,41 @@ public function insert_question_attempt_step(question_attempt_step $step,
}
/**
+ * Store a {@link question_attempt_step} in the database.
+ * @param question_attempt_step $step the step to store.
+ * @param int $questionattemptid the question attept id this step belongs to.
+ * @param int $seq the sequence number of this stop.
+ * @param context $context the context of the owning question_usage_by_activity.
+ */
+ public function insert_question_attempt_step(question_attempt_step $step,
+ $questionattemptid, $seq, $context) {
+
+ $record = $this->make_step_record($step, $questionattemptid, $seq);
+ $record->id = $this->db->insert_record('question_attempt_steps', $record);
+
+ $this->insert_step_data($step, $record->id, $context);
+ }
+
+ /**
+ * Update a {@link question_attempt_step} in the database.
+ * @param question_attempt_step $qa the step to store.
+ * @param int $questionattemptid the question attept id this step belongs to.
+ * @param int $seq the sequence number of this stop.
+ * @param context $context the context of the owning question_usage_by_activity.
+ */
+ public function update_question_attempt_step(question_attempt_step $step,
+ $questionattemptid, $seq, $context) {
+
+ $record = $this->make_step_record($step, $questionattemptid, $seq);
+ $record->id = $step->get_id();
+ $this->db->update_record('question_attempt_steps', $record);
+
+ $this->db->delete_records('question_attempt_step_data',
+ array('attemptstepid' => $record->id));
+ $this->insert_step_data($step, $record->id, $context);
+ }
+
+ /**
* Load a {@link question_attempt_step} from the database.
* @param int $stepid the id of the step to load.
* @param question_attempt_step the step that was loaded.
@@ -727,29 +787,27 @@ protected function delete_attempt_steps_for_mysql($test, $params) {
/**
* Delete all the steps for a question attempt.
* @param int $qaids question_attempt id.
+ * @param context $context the context that the $quba belongs to.
*/
- public function delete_steps_for_question_attempts($qaids, $context) {
- if (empty($qaids)) {
+ public function delete_steps($stepids, $context) {
+ if (empty($stepids)) {
return;
}
- list($test, $params) = $this->db->get_in_or_equal($qaids, SQL_PARAMS_NAMED);
+ list($test, $params) = $this->db->get_in_or_equal($stepids, SQL_PARAMS_NAMED);
- $this->delete_response_files($context->id, "IN (
- SELECT id
- FROM {question_attempt_steps}
- WHERE questionattemptid $test)", $params);
+ if ($deletefiles) {
+ $this->delete_response_files($context->id, $test, $params);
+ }
if ($this->db->get_dbfamily() == 'mysql') {
$this->delete_attempt_steps_for_mysql($test, $params);
return;
}
- $this->db->delete_records_select('question_attempt_step_data', "attemptstepid IN (
- SELECT qas.id
- FROM {question_attempt_steps} qas
- WHERE questionattemptid $test)", $params);
+ $this->db->delete_records_select('question_attempt_step_data',
+ "attemptstepid $test", $params);
$this->db->delete_records_select('question_attempt_steps',
- 'questionattemptid ' . $test, $params);
+ "attemptstepid $test", $params);
}
/**
@@ -953,28 +1011,34 @@ class question_engine_unit_of_work implements question_usage_observer {
protected $modified = false;
/**
- * @var array list of number in usage => {@link question_attempt}s that
+ * @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 number in usage => {@link question_attempt}s that
+ * @var array list of slot => {@link question_attempt}s that
* have been added to the usage.
*/
protected $attemptsadded = array();
/**
- * @var array list of question attempt ids to delete the steps for, before
- * inserting new steps.
+ * @var array of array(question_attempt_step, question_attempt id, seq number)
+ * of steps that have been added to question attempts in this usage.
*/
- protected $attemptstodeletestepsfor = array();
+ protected $stepsadded = array();
/**
- * @var array list of array(question_attempt_step, question_attempt id, seq number)
- * of steps that have been added to question attempts in this usage.
+ * @var array of array(question_attempt_step, question_attempt id, seq number)
+ * of steps that have been modified in their attempt.
*/
- protected $stepsadded = array();
+ protected $stepsmodified = array();
+
+ /**
+ * @var array list of question_attempt_step.id => question_attempt_step of steps
+ * that were previously stored in the database, but which are no longer required.
+ */
+ protected $stepsdeleted = array();
/**
* Constructor.
@@ -989,9 +1053,9 @@ public function notify_modified() {
}
public function notify_attempt_modified(question_attempt $qa) {
- $no = $qa->get_slot();
- if (!array_key_exists($no, $this->attemptsadded)) {
- $this->attemptsmodified[$no] = $qa;
+ $slot = $qa->get_slot();
+ if (!array_key_exists($slot, $this->attemptsadded)) {
+ $this->attemptsmodified[$slot] = $qa;
}
}
@@ -999,27 +1063,121 @@ public function notify_attempt_added(question_attempt $qa) {
$this->attemptsadded[$qa->get_slot()] = $qa;
}
- public function notify_delete_attempt_steps(question_attempt $qa) {
-
+ public function notify_step_added(question_attempt_step $step, question_attempt $qa, $seq) {
if (array_key_exists($qa->get_slot(), $this->attemptsadded)) {
return;
}
- $qaid = $qa->get_database_id();
- foreach ($this->stepsadded as $key => $stepinfo) {
- if ($stepinfo[1] == $qaid) {
- unset($this->stepsadded[$key]);
+ if (($key = $this->is_step_added($step)) !== false) {
+ return;
+ }
+
+ if (($key = $this->is_step_modified($step)) !== false) {
+ throw new coding_exception('Cannot add a step that has already been modified.');
+ }
+
+ if (($key = $this->is_step_deleted($step)) !== false) {
+ unset($this->stepsdeleted[$step->get_id()]);
+ $this->stepsmodified[] = array($step, $qa->get_database_id(), $seq);
+ return;
+ }
+
+ $stepid = $step->get_id();
+ if ($stepid) {
+ if (array_key_exists($stepid, $this->stepsdeleted)) {
+ unset($this->stepsdeleted[$stepid]);
}
+ $this->stepsmodified[] = array($step, $qa->get_database_id(), $seq);
+
+ } else {
+ $this->stepsadded[] = array($step, $qa->get_database_id(), $seq);
+ }
+ }
+
+ public function notify_step_modified(question_attempt_step $step, question_attempt $qa, $seq) {
+ if (array_key_exists($qa->get_slot(), $this->attemptsadded)) {
+ return;
}
- $this->attemptstodeletestepsfor[$qaid] = 1;
+ if (($key = $this->is_step_added($step)) !== false) {
+ return;
+ }
+
+ if (($key = $this->is_step_deleted($step)) !== false) {
+ throw new coding_exception('Cannot modify a step after it has been deleted.');
+ }
+
+ $stepid = $step->get_id();
+ if (empty($stepid)) {
+ throw new coding_exception('Cannot modify a step that has never been stored in the database.');
+ }
+
+ $this->stepsmodified[] = array($step, $qa->get_database_id(), $seq);
}
- public function notify_step_added(question_attempt_step $step, question_attempt $qa, $seq) {
+ public function notify_step_deleted(question_attempt_step $step, question_attempt $qa) {
if (array_key_exists($qa->get_slot(), $this->attemptsadded)) {
return;
}
- $this->stepsadded[] = array($step, $qa->get_database_id(), $seq);
+
+ if (($key = $this->is_step_added($step)) !== false) {
+ unset($this->stepsadded[$key]);
+ return;
+ }
+
+ if (($key = $this->is_step_modified($step)) !== false) {
+ unset($this->stepsmodified[$key]);
+ }
+
+ $stepid = $step->get_id();
+ if (empty($stepid)) {
+ return; // Was never in the database.
+ }
+
+ $this->stepsdeleted[$stepid] = $step;
+ }
+
+ /**
+ * @param question_attempt_step $step a step
+ * @return int|false if the step is in the list of steps to be added, return
+ * the key, otherwise return false.
+ */
+ protected function is_step_added(question_attempt_step $step) {
+ foreach ($this->stepsadded as $key => $data) {
+ list($addedstep, $qaid, $seq) = $data;
+ if ($addedstep === $step) {
+ return $key;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * @param question_attempt_step $step a step
+ * @return int|false if the step is in the list of steps to be modified, return
+ * the key, otherwise return false.
+ */
+ protected function is_step_modified(question_attempt_step $step) {
+ foreach ($this->stepsmodified as $key => $data) {
+ list($modifiedstep, $qaid, $seq) = $data;
+ if ($modifiedstep === $step) {
+ return $key;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * @param question_attempt_step $step a step
+ * @return bool whether the step is in the list of steps to be deleted.
+ */
+ protected function is_step_deleted(question_attempt_step $step) {
+ foreach ($this->stepsdeleted as $deletedstep) {
+ if ($deletedstep === $step) {
+ return true;
+ }
+ }
+ return false;
}
/**
@@ -1027,8 +1185,13 @@ public function notify_step_added(question_attempt_step $step, question_attempt
* @param question_engine_data_mapper $dm the mapper to use to update the database.
*/
public function save(question_engine_data_mapper $dm) {
- $dm->delete_steps_for_question_attempts(array_keys($this->attemptstodeletestepsfor),
- $this->quba->get_owning_context());
+ $dm->delete_steps(array_keys($this->stepsdeleted), $this->quba->get_owning_context());
+
+ foreach ($this->stepsmodified as $stepinfo) {
+ list($step, $questionattemptid, $seq) = $stepinfo;
+ $dm->update_question_attempt_step($step, $questionattemptid, $seq,
+ $this->quba->get_owning_context());
+ }
foreach ($this->stepsadded as $stepinfo) {
list($step, $questionattemptid, $seq) = $stepinfo;
View
32 question/engine/questionattempt.php
@@ -187,7 +187,7 @@ public function get_variant() {
* For internal use only.
* @param int $slot
*/
- public function set_number_in_usage($slot) {
+ public function set_slot($slot) {
$this->slot = $slot;
}
@@ -213,6 +213,15 @@ public function set_database_id($id) {
$this->id = $id;
}
+ /**
+ * You should almost certainly not call this method from your code. It is for
+ * internal use only.
+ * @param question_usage_observer that should be used to tracking changes made to this qa.
+ */
+ public function set_observer($observer) {
+ $this->observer = $observer;
+ }
+
/** @return int|string the id of the {@link question_usage_by_activity} we belong to. */
public function get_usage_id() {
return $this->usageid;
@@ -802,9 +811,11 @@ public function select_variant(question_variant_selection_strategy $variantstrat
* @param array $submitteddata optional, used when re-starting to keep the same initial state.
* @param int $timestamp optional, the timstamp to record for this action. Defaults to now.
* @param int $userid optional, the user to attribute this action to. Defaults to the current user.
+ * @param int $existingstepid optional, if this step is going to replace an existing step
+ * (for example, during a regrade) this is the id of the previous step we are replacing.
*/
public function start($preferredbehaviour, $variant, $submitteddata = array(),
- $timestamp = null, $userid = null) {
+ $timestamp = null, $userid = null, $existingstepid = null) {
// Initialise the behaviour.
$this->variant = $variant;
@@ -820,7 +831,7 @@ public function start($preferredbehaviour, $variant, $submitteddata = array(),
$this->minfraction = $this->behaviour->get_min_fraction();
// Initialise the first step.
- $firststep = new question_attempt_step($submitteddata, $timestamp, $userid);
+ $firststep = new question_attempt_step($submitteddata, $timestamp, $userid, $existingstepid);
$firststep->set_state(question_state::$todo);
if ($submitteddata) {
$this->question->apply_attempt_state($firststep);
@@ -1041,8 +1052,8 @@ public function get_right_answer_summary() {
* @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.)
*/
- public function process_action($submitteddata, $timestamp = null, $userid = null) {
- $pendingstep = new question_attempt_pending_step($submitteddata, $timestamp, $userid);
+ public function process_action($submitteddata, $timestamp = null, $userid = null, $existingstepid = null) {
+ $pendingstep = new question_attempt_pending_step($submitteddata, $timestamp, $userid, $existingstepid);
if ($this->behaviour->process_action($pendingstep) == self::KEEP) {
$this->add_step($pendingstep);
if ($pendingstep->response_summary_changed()) {
@@ -1074,13 +1085,14 @@ public function finish($timestamp = null, $userid = null) {
public function regrade(question_attempt $oldqa, $finished) {
$first = true;
foreach ($oldqa->get_step_iterator() as $step) {
+ $this->observer->notify_step_deleted($step, $this);
if ($first) {
$first = false;
$this->start($oldqa->behaviour, $oldqa->get_variant(), $step->get_all_data(),
- $step->get_timecreated(), $step->get_user_id());
+ $step->get_timecreated(), $step->get_user_id(), $step->get_id());
} else {
$this->process_action($step->get_submitted_data(),
- $step->get_timecreated(), $step->get_user_id());
+ $step->get_timecreated(), $step->get_user_id(), $step->get_id());
}
}
if ($finished) {
@@ -1147,7 +1159,7 @@ public function classify_response() {
*
* @param Iterator $records Raw records loaded from the database.
* @param int $questionattemptid The id of the question_attempt to extract.
- * @return question_attempt The newly constructed question_attempt_step.
+ * @return question_attempt The newly constructed question_attempt.
*/
public static function load_from_records($records, $questionattemptid,
question_usage_observer $observer, $preferredbehaviour) {
@@ -1172,7 +1184,7 @@ public static function load_from_records($records, $questionattemptid,
$qa = new question_attempt($question, $record->questionusageid,
null, $record->maxmark + 0);
$qa->set_database_id($record->questionattemptid);
- $qa->set_number_in_usage($record->slot);
+ $qa->set_slot($record->slot);
$qa->variant = $record->variant + 0;
$qa->minfraction = $record->minfraction + 0;
$qa->set_flagged($record->flagged);
@@ -1278,7 +1290,7 @@ public function set_database_id($id) {
public function set_flagged($flagged) {
coding_exception('Cannot modify a question_attempt_with_restricted_history.');
}
- public function set_number_in_usage($slot) {
+ public function set_slot($slot) {
coding_exception('Cannot modify a question_attempt_with_restricted_history.');
}
public function set_question_summary($questionsummary) {
View
17 question/engine/questionattemptstep.php
@@ -98,8 +98,11 @@ class question_attempt_step {
* @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.)
+ * @param int $existingstepid if this step is going to replace an existing step
+ * (for example, during a regrade) this is the id of the previous step we are replacing.
*/
- public function __construct($data = array(), $timecreated = null, $userid = null) {
+ public function __construct($data = array(), $timecreated = null, $userid = null,
+ $existingstepid = null) {
global $USER;
if (!is_array($data)) {
@@ -117,6 +120,18 @@ public function __construct($data = array(), $timecreated = null, $userid = null
} else {
$this->userid = $userid;
}
+
+ if (!is_null($existingstepid)) {
+ $this->id = $existingstepid;
+ }
+ }
+
+ /**
+ * @return int|null The id of this step in the database. null if this step
+ * is not stored in the database.
+ */
+ public function get_id() {
+ return $this->id;
}
/** @return question_state The state after this step. */
View
62 question/engine/questionusage.php
@@ -124,11 +124,6 @@ public function get_id() {
return $this->id;
}
- /** @return question_usage_observer that is tracking changes made to this usage. */
- public function get_observer() {
- return $this->observer;
- }
-
/**
* For internal use only. Used by {@link question_engine_data_mapper} to set
* the id when a usage is saved to the database.
@@ -141,6 +136,23 @@ public function set_id_from_database($id) {
}
}
+ /** @return question_usage_observer that is tracking changes made to this usage. */
+ public function get_observer() {
+ return $this->observer;
+ }
+
+ /**
+ * You should almost certainly not call this method from your code. It is for
+ * internal use only.
+ * @param question_usage_observer that should be used to tracking changes made to this usage.
+ */
+ public function set_observer($observer) {
+ $this->observer = $observer;
+ foreach ($this->questionattempts as $qa) {
+ $qa->set_observer($observer);
+ }
+ }
+
/**
* Add another question to this usage.
*
@@ -159,7 +171,7 @@ public function add_question(question_definition $question, $maxmark = null) {
} else {
$this->questionattempts[] = $qa;
}
- $qa->set_number_in_usage(end(array_keys($this->questionattempts)));
+ $qa->set_slot(end(array_keys($this->questionattempts)));
$this->observer->notify_attempt_added($qa);
return $qa->get_slot();
}
@@ -647,11 +659,10 @@ public function regrade_question($slot, $finished = false, $newmaxmark = null) {
$newmaxmark = $oldqa->get_max_mark();
}
- $this->observer->notify_delete_attempt_steps($oldqa);
-
$newqa = new question_attempt($oldqa->get_question(), $oldqa->get_usage_id(),
$this->observer, $newmaxmark);
$newqa->set_database_id($oldqa->get_database_id());
+ $newqa->set_slot($oldqa->get_slot());
$newqa->regrade($oldqa, $finished);
$this->questionattempts[$slot] = $newqa;
@@ -676,7 +687,7 @@ public function regrade_all_questions($finished = false) {
*
* @param Iterator $records Raw records loaded from the database.
* @param int $questionattemptid The id of the question_attempt to extract.
- * @return question_attempt The newly constructed question_attempt_step.
+ * @return question_usage_by_activity The newly constructed usage.
*/
public static function load_from_records($records, $qubaid) {
$record = $records->current();
@@ -808,19 +819,28 @@ public function notify_attempt_modified(question_attempt $qa);
public function notify_attempt_added(question_attempt $qa);
/**
- * Called we want to delete the old step records for an attempt, prior to
- * inserting newones. This is used by regrading.
- * @param question_attempt $qa the question attempt to delete the steps for.
+ * Called when a new step is added to a question attempt in this usage.
+ * @param question_attempt_step $step the new step.
+ * @param question_attempt $qa the usage it is being added to.
+ * @param int $seq the sequence number of the new step.
*/
- public function notify_delete_attempt_steps(question_attempt $qa);
+ public function notify_step_added(question_attempt_step $step, question_attempt $qa, $seq);
/**
- * Called when a new step is added to a question attempt in this usage.
- * @param $step the new step.
- * @param $qa the usage it is being added to.
- * @param $seq the sequence number of the new step.
+ * Called when a new step is updated in a question attempt in this usage.
+ * @param question_attempt_step $step the step that was updated.
+ * @param question_attempt $qa the usage it is being added to.
+ * @param int $seq the sequence number of the new step.
*/
- public function notify_step_added(question_attempt_step $step, question_attempt $qa, $seq);
+ public function notify_step_modified(question_attempt_step $step, question_attempt $qa, $seq);
+
+ /**
+ * Called when a new step is updated in a question attempt in this usage.
+ * @param question_attempt_step $step the step to delete.
+ * @param question_attempt $qa the usage it is being added to.
+ */
+ public function notify_step_deleted(question_attempt_step $step, question_attempt $qa);
+
}
@@ -838,8 +858,10 @@ public function notify_attempt_modified(question_attempt $qa) {
}
public function notify_attempt_added(question_attempt $qa) {
}
- public function notify_delete_attempt_steps(question_attempt $qa) {
- }
public function notify_step_added(question_attempt_step $step, question_attempt $qa, $seq) {
}
+ public function notify_step_modified(question_attempt_step $step, question_attempt $qa, $seq) {
+ }
+ public function notify_step_deleted(question_attempt_step $step, question_attempt $qa) {
+ }
}
View
6 question/engine/simpletest/testquestionattempt.php
@@ -70,8 +70,8 @@ public function test_maxmark_beats_default_mark() {
$this->assertEqual(2, $qa->get_max_mark());
}
- public function test_get_set_number_in_usage() {
- $this->qa->set_number_in_usage(7);
+ public function test_get_set_slot() {
+ $this->qa->set_slot(7);
$this->assertEqual(7, $this->qa->get_slot());
}
@@ -97,7 +97,7 @@ public function test_get_behaviour_field_name() {
}
public function test_get_field_prefix() {
- $this->qa->set_number_in_usage(7);
+ $this->qa->set_slot(7);
$name = $this->qa->get_field_prefix();
$this->assertPattern('/' . preg_quote($this->usageid) . '/', $name);
$this->assertPattern('/' . preg_quote($this->qa->get_slot()) . '/', $name);
View
64 question/engine/simpletest/testquestionusagebyactivity.php
@@ -158,4 +158,66 @@ public function test_access_out_of_sequence_throws_exception() {
$this->expectException('question_out_of_sequence_exception');
$quba->process_all_actions($slot, $postdata);
}
-}
+}
+
+/**
+ * Unit tests for loading data into the {@link question_usage_by_activity} class.
+ *
+ * @copyright 2012 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class question_usage_db_test extends data_loading_method_test_base {
+ public function test_load() {
+ $records = new test_recordset(array(
+ array('qubaid', 'contextid', 'component', 'preferredbehaviour',
+ 'questionattemptid', 'contextid', 'questionusageid', 'slot',
+ 'behaviour', 'questionid', 'variant', 'maxmark', 'minfraction', 'flagged',
+ 'questionsummary', 'rightanswer', 'responsesummary', 'timemodified',
+ 'attemptstepid', 'sequencenumber', 'state', 'fraction',
+ 'timecreated', 'userid', 'name', 'value'),
+ array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 1, 0, 'todo', null, 1256233700, 1, null, null),
+ array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 2, 1, 'todo', null, 1256233705, 1, 'answer', '1'),
+ array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 5, 2, 'gradedright', 1.0000000, 1256233720, 1, '-finish', '1'),
+ ));
+
+ $question = test_question_maker::make_question('truefalse', 'true');
+ $question->id = -1;
+
+ question_bank::start_unit_test();
+ question_bank::load_test_question_data($question);
+ $quba = question_usage_by_activity::load_from_records($records, 1);
+ question_bank::end_unit_test();
+
+ $this->assertEqual('unit_test', $quba->get_owning_component());
+ $this->assertEqual(1, $quba->get_id());
+ $this->assertIsA($quba->get_observer(), 'question_engine_unit_of_work');
+ $this->assertEqual('interactive', $quba->get_preferred_behaviour());
+
+ $qa = $quba->get_question_attempt(1);
+
+ $this->assertEqual($question->questiontext, $qa->get_question()->questiontext);
+
+ $this->assertEqual(3, $qa->get_num_steps());
+
+ $step = $qa->get_step(0);
+ $this->assertEqual(question_state::$todo, $step->get_state());
+ $this->assertNull($step->get_fraction());
+ $this->assertEqual(1256233700, $step->get_timecreated());
+ $this->assertEqual(1, $step->get_user_id());
+ $this->assertEqual(array(), $step->get_all_data());
+
+ $step = $qa->get_step(1);
+ $this->assertEqual(question_state::$todo, $step->get_state());
+ $this->assertNull($step->get_fraction());
+ $this->assertEqual(1256233705, $step->get_timecreated());
+ $this->assertEqual(1, $step->get_user_id());
+ $this->assertEqual(array('answer' => '1'), $step->get_all_data());
+
+ $step = $qa->get_step(2);
+ $this->assertEqual(question_state::$gradedright, $step->get_state());
+ $this->assertEqual(1, $step->get_fraction());
+ $this->assertEqual(1256233720, $step->get_timecreated());
+ $this->assertEqual(1, $step->get_user_id());
+ $this->assertEqual(array('-finish' => '1'), $step->get_all_data());
+ }
+}
View
296 question/engine/simpletest/testunitofwork.php
@@ -0,0 +1,296 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * This file contains tests for the question_engine_unit_of_work class.
+ *
+ * @package moodlecore
+ * @subpackage questionengine
+ * @copyright 2012 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+
+defined('MOODLE_INTERNAL') || die();
+
+require_once(dirname(__FILE__) . '/../lib.php');
+require_once(dirname(__FILE__) . '/helpers.php');
+
+
+/**
+ * Test subclass to allow access to some protected data so that the correct
+ * behaviour can be verified.
+ *
+ * @copyright 2012 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class testable_question_engine_unit_of_work extends question_engine_unit_of_work {
+ public function get_modified() {
+ return $this->modified;
+ }
+
+ public function get_attempts_added() {
+ return $this->attemptsadded;
+ }
+
+ public function get_attempts_modified() {
+ return $this->attemptsmodified;
+ }
+
+ public function get_steps_added() {
+ return $this->stepsadded;
+ }
+
+ public function get_steps_modified() {
+ return $this->stepsmodified;
+ }
+
+ public function get_steps_deleted() {
+ return $this->stepsdeleted;
+ }
+}
+
+
+/**
+ * Unit tests for the {@link question_engine_unit_of_work} class.
+ *
+ * @copyright 2012 The Open University
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class question_engine_unit_of_work_test extends data_loading_method_test_base {
+ /** @var question_usage_by_activity the test question usage. */
+ protected $quba;
+
+ /** @var int the slot number of the one qa in the test usage.*/
+ protected $slot;
+
+ /** @var testable_question_engine_unit_of_work the unit of work we are testing. */
+ protected $observer;
+
+ public function setUp() {
+ // Create a usage in an initial state, with one shortanswer question added,
+ // and attempted in interactive mode submitted responses 'toad' then 'frog'.
+ // Then set it to use a new unit of work for any subsequent changes.
+ // Create a short answer question.
+ $question = test_question_maker::make_question('shortanswer');
+ $question->hints = array(
+ new question_hint(0, 'This is the first hint.', FORMAT_HTML),
+ new question_hint(0, 'This is the second hint.', FORMAT_HTML),
+ );
+ $question->id = -1;
+ question_bank::start_unit_test();
+ question_bank::load_test_question_data($question);
+
+ $this->setup_initial_test_state($this->get_test_data());
+ }
+
+ public function testDown() {
+ question_bank::end_unit_test();
+ }
+
+ protected function setup_initial_test_state($testdata) {
+ $records = new test_recordset($testdata);
+
+ $this->quba = question_usage_by_activity::load_from_records($records, 1);
+
+ $this->slot = 1;
+ $this->observer = new testable_question_engine_unit_of_work($this->quba);
+ $this->quba->set_observer($this->observer);
+ }
+
+ protected function get_test_data() {
+ return array(
+ array('qubaid', 'contextid', 'component', 'preferredbehaviour',
+ 'questionattemptid', 'contextid', 'questionusageid', 'slot',
+ 'behaviour', 'questionid', 'variant', 'maxmark', 'minfraction', 'flagged',
+ 'questionsummary', 'rightanswer', 'responsesummary', 'timemodified',
+ 'attemptstepid', 'sequencenumber', 'state', 'fraction',
+ 'timecreated', 'userid', 'name', 'value'),
+ array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 1.0000000, 0.0000000, 0, '', '', '', 1256233790, 1, 0, 'todo', null, 1256233700, 1, '-_triesleft', 3),
+ array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 1.0000000, 0.0000000, 0, '', '', '', 1256233790, 2, 1, 'todo', null, 1256233720, 1, 'answer', 'toad'),
+ array(1, 1, 'unit_test', 'interactive', 1, 123, 1, 1, 'interactive', -1, 1, 1.0000000, 0.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, 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, 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, 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, 0, '', '', '', 1256233790, 5, 3, 'gradedright', 1.0000000, 1256233790, 1, '-finish', 1),
+ );
+ }
+
+ public function test_initial_state() {
+ $this->assertFalse($this->observer->get_modified());
+ $this->assertEqual(0, count($this->observer->get_attempts_added()));
+ $this->assertEqual(0, count($this->observer->get_attempts_modified()));
+ $this->assertEqual(0, count($this->observer->get_steps_added()));
+ $this->assertEqual(0, count($this->observer->get_steps_modified()));
+ $this->assertEqual(0, count($this->observer->get_steps_deleted()));
+ }
+
+ public function test_update_usage() {
+
+ $this->quba->set_preferred_behaviour('deferredfeedback');
+
+ $this->assertTrue($this->observer->get_modified());
+ }
+
+ public function test_add_question() {
+
+ $slot = $this->quba->add_question(test_question_maker::make_question('truefalse'));
+
+ $newattempts = $this->observer->get_attempts_added();
+ $this->assertEqual(1, count($newattempts));
+ $this->assertIdentical($this->quba->get_question_attempt($slot), reset($newattempts));
+ $this->assertIdentical($slot, key($newattempts));
+ }
+
+ public function test_add_and_start_question() {
+
+ $slot = $this->quba->add_question(test_question_maker::make_question('truefalse'));
+ $this->quba->start_question($slot);
+
+ // The point here is that, although we have added a step, it is not listed
+ // separately becuase it is part of a newly added attempt, and all steps
+ // for a newly added attempt are automatically added to the DB, so it does
+ // not need to be tracked separately.
+ $newattempts = $this->observer->get_attempts_added();
+ $this->assertEqual(1, count($newattempts));
+ $this->assertIdentical($this->quba->get_question_attempt($slot),
+ reset($newattempts));
+ $this->assertIdentical($slot, key($newattempts));
+ $this->assertEqual(0, count($this->observer->get_steps_added()));
+ }
+
+ public function test_process_action() {
+
+ $this->quba->manual_grade($this->slot, 'Acutally, that is not quite right', 0.5);
+
+ // Here, however, were we are adding a step to an existing qa, we do need to track that.
+ $this->assertEqual(0, count($this->observer->get_attempts_added()));
+
+ $updatedattempts = $this->observer->get_attempts_modified();
+ $this->assertEqual(1, count($updatedattempts));
+
+ $updatedattempt = reset($updatedattempts);
+ $this->assertIdentical($this->quba->get_question_attempt($this->slot), $updatedattempt);
+ $this->assertIdentical($this->slot, key($updatedattempts));
+
+ $newsteps = $this->observer->get_steps_added();
+ $this->assertEqual(1, count($newsteps));
+
+ list($newstep, $qaid, $seq) = reset($newsteps);
+ $this->assertIdentical($this->quba->get_question_attempt($this->slot)->get_last_step(), $newstep);
+ }
+
+ public function test_regrade_same_steps() {
+
+ // Change the question in a minor way and regrade.
+ $this->quba->get_question($this->slot)->answer[14]->fraction = 0.5;
+ $this->quba->regrade_all_questions();
+
+ // Here, the qa, and all the steps, should be marked as updated.
+ // Here, however, were we are adding a step to an existing qa, we do need to track that.
+ $this->assertEqual(0, count($this->observer->get_attempts_added()));
+ $this->assertEqual(0, count($this->observer->get_steps_added()));
+ $this->assertEqual(0, count($this->observer->get_steps_deleted()));
+
+ $updatedattempts = $this->observer->get_attempts_modified();
+ $this->assertEqual(1, count($updatedattempts));
+
+ $updatedattempt = reset($updatedattempts);
+ $this->assertIdentical($this->quba->get_question_attempt($this->slot), $updatedattempt);
+
+ $updatedsteps = $this->observer->get_steps_modified();
+ $this->assertEqual($updatedattempt->get_num_steps(), count($updatedsteps));
+
+ foreach ($updatedattempt->get_step_iterator() as $seq => $step) {
+ $this->assertIdentical(array($step, $updatedattempt->get_database_id(), $seq),
+ $updatedsteps[$seq]);
+ }
+ }
+
+ public function test_regrade_losing_steps() {
+
+ // Change the question so that 'toad' is also right, and regrade. This
+ // will mean that the try again, and second try states are no longer
+ // needed, so they should be dropped.
+ $this->quba->get_question($this->slot)->answers[14]->fraction = 1;
+ $this->quba->regrade_all_questions();
+
+ $this->assertEqual(0, count($this->observer->get_attempts_added()));
+ $this->assertEqual(0, count($this->observer->get_steps_added()));
+
+ $updatedattempts = $this->observer->get_attempts_modified();
+ $this->assertEqual(1, count($updatedattempts));
+
+ $updatedattempt = reset($updatedattempts);
+ $this->assertIdentical($this->quba->get_question_attempt($this->slot), $updatedattempt);
+
+ $updatedsteps = $this->observer->get_steps_modified();
+ $this->assertEqual($updatedattempt->get_num_steps(), count($updatedsteps));
+
+ foreach ($updatedattempt->get_step_iterator() as $seq => $step) {
+ $this->assertIdentical(array($step, $updatedattempt->get_database_id(), $seq),
+ $updatedsteps[$seq]);
+ }
+
+ $deletedsteps = $this->observer->get_steps_deleted();
+ $this->assertEqual(2, count($deletedsteps));
+
+ $firstdeletedstep = reset($deletedsteps);
+ $this->assertEqual(array('-tryagain' => 1), $firstdeletedstep->get_all_data());
+
+ $seconddeletedstep = end($deletedsteps);
+ $this->assertEqual(array('answer' => 'frog', '-finish' => 1),
+ $seconddeletedstep->get_all_data());
+ }
+
+ public function test_tricky_regrade() {
+
+ // The tricky thing here is that we take a half-complete question-attempt,
+ // and then as one transaction, we submit some more responses, and then
+ // change the question attempt as in test_regrade_losing_steps, and regrade
+ // before the steps are even written to the database the first time.
+ $somedata = $this->get_test_data();
+ $somedata = array_slice($somedata, 0, 5);
+ $this->setup_initial_test_state($somedata);
+
+ $this->quba->process_action($this->slot, array('-tryagain' => 1));
+ $this->quba->process_action($this->slot, array('answer' => 'frog', '-submit' => 1));
+ $this->quba->finish_all_questions();
+
+ $this->quba->get_question($this->slot)->answers[14]->fraction = 1;
+ $this->quba->regrade_all_questions();
+
+ $this->assertEqual(0, count($this->observer->get_attempts_added()));
+
+ $updatedattempts = $this->observer->get_attempts_modified();
+ $this->assertEqual(1, count($updatedattempts));
+
+ $updatedattempt = reset($updatedattempts);
+ $this->assertIdentical($this->quba->get_question_attempt($this->slot), $updatedattempt);
+
+ $this->assertEqual(0, count($this->observer->get_steps_added()));
+
+ $updatedsteps = $this->observer->get_steps_modified();
+ $this->assertEqual($updatedattempt->get_num_steps(), count($updatedsteps));
+
+ foreach ($updatedattempt->get_step_iterator() as $seq => $step) {
+ $this->assertIdentical(array($step, $updatedattempt->get_database_id(), $seq),
+ $updatedsteps[$seq]);
+ }
+
+ $this->assertEqual(0, count($this->observer->get_steps_deleted()));
+ }
+}
Please sign in to comment.
Something went wrong with that request. Please try again.