From 0a606a2be261f7ea0716f48aff500a0b5a70ca1a Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Thu, 28 Mar 2013 16:51:59 +0000 Subject: [PATCH] MDL-38538 question auto-save back end. 1. Autosave works in some ways just like a normal save. We ultimately call $behaviour->process_save() to do the work, and create a new step to hold the data. 2. However, we come in through a completely different route through the API, starting with separate auto-save methods. This keeps the auto-save changes mostly separate, and so reduced the chance of breaking existing working code. 3. When the time comes to store the auto-save step in the database, we save it using a negative sequence number. This is a clever trick that not only distinguises these steps, but also avoids unique key errors when an auto-save and a real action happen simultaneously. (There are unit tests for these tricky edge cases.) 4. When we load the data back from the database, most of the time the auto-save steps are loaded back as if they were a real save, and so the auto-saved data is used when the question is then rendered. 5. However, before we process another action, we remove the auto-saved step, so it does not appear in the final history. --- question/behaviour/behaviourbase.php | 36 ++ question/engine/datalib.php | 6 +- question/engine/lib.php | 10 +- question/engine/questionattempt.php | 93 ++- question/engine/questionusage.php | 65 +- .../engine/tests/questionattempt_db_test.php | 112 ++++ .../tests/questionusage_autosave_test.php | 562 ++++++++++++++++++ question/tests/generator/lib.php | 25 + 8 files changed, 886 insertions(+), 23 deletions(-) create mode 100644 question/engine/tests/questionusage_autosave_test.php diff --git a/question/behaviour/behaviourbase.php b/question/behaviour/behaviourbase.php index bd122dc0909..4219cba2416 100644 --- a/question/behaviour/behaviourbase.php +++ b/question/behaviour/behaviourbase.php @@ -435,6 +435,19 @@ abstract class question_behaviour { */ public abstract function process_action(question_attempt_pending_step $pendingstep); + /** + * Auto-saved data. By default this does nothing. interesting processing is + * done in {@link question_behaviour_with_save}. + * + * @param question_attempt_pending_step $pendingstep a partially initialised step + * containing all the information about the action that is being peformed. This + * information can be accessed using {@link question_attempt_step::get_behaviour_var()}. + * @return bool either {@link question_attempt::KEEP} or {@link question_attempt::DISCARD} + */ + public function process_autosave(question_attempt_pending_step $pendingstep) { + return question_attempt::DISCARD; + } + /** * Implementation of processing a manual comment/grade action that should * be suitable for most subclasses. @@ -570,6 +583,29 @@ abstract class question_behaviour_with_save extends question_behaviour { return $this->question->is_complete_response($pendingstep->get_qt_data()); } + public function process_autosave(question_attempt_pending_step $pendingstep) { + // If already finished. Nothing to do. + if ($this->qa->get_state()->is_finished()) { + return question_attempt::DISCARD; + } + + // If the new data is the same as we already have, then we don't need it. + if ($this->is_same_response($pendingstep)) { + return question_attempt::DISCARD; + } + + // Repeat that test discarding any existing autosaved data. + if ($this->qa->has_autosaved_step()) { + $this->qa->discard_autosaved_step(); + if ($this->is_same_response($pendingstep)) { + return question_attempt::DISCARD; + } + } + + // OK, we need to save. + return $this->process_save($pendingstep); + } + /** * Implementation of processing a save action that should be suitable for * most subclasses. diff --git a/question/engine/datalib.php b/question/engine/datalib.php index 77c5d52de31..b58c05e0374 100644 --- a/question/engine/datalib.php +++ b/question/engine/datalib.php @@ -63,7 +63,7 @@ class question_engine_data_mapper { /** * @param moodle_database $db a database connectoin. Defaults to global $DB. */ - public function __construct($db = null) { + public function __construct(moodle_database $db = null) { if (is_null($db)) { global $DB; $this->db = $DB; @@ -613,12 +613,10 @@ ORDER BY qa.slot * @return array of question_attempts. */ public function load_attempts_at_question($questionid, qubaid_condition $qubaids) { - global $DB; - $params = $qubaids->from_where_params(); $params['questionid'] = $questionid; - $records = $DB->get_recordset_sql(" + $records = $this->db->get_recordset_sql(" SELECT quba.contextid, quba.preferredbehaviour, diff --git a/question/engine/lib.php b/question/engine/lib.php index 9fdfc033aa1..df138901268 100644 --- a/question/engine/lib.php +++ b/question/engine/lib.php @@ -71,10 +71,11 @@ abstract class question_engine { /** * Load a {@link question_usage_by_activity} from the database, based on its id. * @param int $qubaid the id of the usage to load. + * @param moodle_database $db a database connectoin. Defaults to global $DB. * @return question_usage_by_activity loaded from the database. */ - public static function load_questions_usage_by_activity($qubaid) { - $dm = new question_engine_data_mapper(); + public static function load_questions_usage_by_activity($qubaid, moodle_database $db = null) { + $dm = new question_engine_data_mapper($db); return $dm->load_questions_usage_by_activity($qubaid); } @@ -83,9 +84,10 @@ abstract class question_engine { * if the usage was newly created by {@link make_questions_usage_by_activity()} * or loaded from the database using {@link load_questions_usage_by_activity()} * @param question_usage_by_activity the usage to save. + * @param moodle_database $db a database connectoin. Defaults to global $DB. */ - public static function save_questions_usage_by_activity(question_usage_by_activity $quba) { - $dm = new question_engine_data_mapper(); + public static function save_questions_usage_by_activity(question_usage_by_activity $quba, moodle_database $db = null) { + $dm = new question_engine_data_mapper($db); $observer = $quba->get_observer(); if ($observer instanceof question_engine_unit_of_work) { $observer->save($dm); diff --git a/question/engine/questionattempt.php b/question/engine/questionattempt.php index 68211ed26a8..ae4be3820c9 100644 --- a/question/engine/questionattempt.php +++ b/question/engine/questionattempt.php @@ -118,6 +118,12 @@ class question_attempt { /** @var array of {@link question_attempt_step}s. The steps in this attempt. */ protected $steps = array(); + /** + * @var question_attempt_step if, when we loaded the step from the DB, there was + * an autosaved step, we save a pointer to it here. (It is also added to the $steps array.) + */ + protected $autosavedstep = null; + /** @var boolean whether the user has flagged this attempt within the usage. */ protected $flagged = false; @@ -362,6 +368,14 @@ class question_attempt { return end($this->steps); } + /** + * @return boolean whether this question_attempt has autosaved data from + * some time in the past. + */ + public function has_autosaved_step() { + return !is_null($this->autosavedstep); + } + /** * @return question_attempt_step_iterator for iterating over the steps in * this attempt, in order. @@ -788,6 +802,31 @@ class question_attempt { $this->observer->notify_step_added($step, $this, key($this->steps)); } + /** + * Add an auto-saved step to this question attempt. We mark auto-saved steps by + * changing saving the step number with a - sign. + * @param question_attempt_step $step the new step. + */ + protected function add_autosaved_step(question_attempt_step $step) { + $this->steps[] = $step; + $this->autosavedstep = $step; + end($this->steps); + $this->observer->notify_step_added($step, $this, -key($this->steps)); + } + + /** + * Discard any auto-saved data belonging to this question attempt. + */ + public function discard_autosaved_step() { + if (!$this->has_autosaved_step()) { + return; + } + + $autosaved = array_pop($this->steps); + $this->autosavedstep = null; + $this->observer->notify_step_deleted($autosaved, $this); + } + /** * Use a strategy to pick a variant. * @param question_variant_selection_strategy $variantstrategy a strategy. @@ -1045,10 +1084,12 @@ class question_attempt { * Perform the action described by $submitteddata. * @param array $submitteddata the submitted data the determines the action. * @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 $userid the user to attribute the action to. (If not given, use the current user.) + * @param int $existingstepid used by the regrade code. */ public function process_action($submitteddata, $timestamp = null, $userid = null, $existingstepid = null) { $pendingstep = new question_attempt_pending_step($submitteddata, $timestamp, $userid, $existingstepid); + $this->discard_autosaved_step(); if ($this->behaviour->process_action($pendingstep) == self::KEEP) { $this->add_step($pendingstep); if ($pendingstep->response_summary_changed()) { @@ -1057,6 +1098,22 @@ class question_attempt { } } + /** + * Process an autosave. + * @param array $submitteddata the submitted data the determines the action. + * @param int $timestamp the time to record for the action. (If not given, use now.) + * @param int $userid the user to attribute the action to. (If not given, use the current user.) + * @return bool whether anything was saved. + */ + public function process_autosave($submitteddata, $timestamp = null, $userid = null) { + $pendingstep = new question_attempt_pending_step($submitteddata, $timestamp, $userid); + if ($this->behaviour->process_autosave($pendingstep) == self::KEEP) { + $this->add_autosaved_step($pendingstep); + return true; + } + return false; + } + /** * Perform a finish action on this question attempt. This corresponds to an * external finish action, for example the user pressing Submit all and finish @@ -1212,6 +1269,7 @@ class question_attempt { $qa->behaviour = question_engine::make_behaviour( $record->behaviour, $qa, $preferredbehaviour); + $qa->observer = $observer; // If attemptstepid is null (which should not happen, but has happened // due to corrupt data, see MDL-34251) then the current pointer in $records @@ -1223,12 +1281,28 @@ class question_attempt { } $i = 0; + $autosavedstep = null; + $autosavedsequencenumber = null; while ($record && $record->questionattemptid == $questionattemptid && !is_null($record->attemptstepid)) { - $qa->steps[$i] = question_attempt_step::load_from_records($records, $record->attemptstepid); - if ($i == 0) { - $question->apply_attempt_state($qa->steps[0]); + $sequencenumber = $record->sequencenumber; + $nextstep = question_attempt_step::load_from_records($records, $record->attemptstepid); + + if ($sequencenumber < 0) { + if (!$autosavedstep) { + $autosavedstep = $nextstep; + $autosavedsequencenumber = -$sequencenumber; + } else { + // Old redundant data. Mark it for deletion. + $qa->observer->notify_step_deleted($nextstep, $qa); + } + } else { + $qa->steps[$i] = $nextstep; + if ($i == 0) { + $question->apply_attempt_state($qa->steps[0]); + } + $i++; } - $i++; + if ($records->valid()) { $record = $records->current(); } else { @@ -1236,7 +1310,14 @@ class question_attempt { } } - $qa->observer = $observer; + if ($autosavedstep) { + if ($autosavedsequencenumber >= $i) { + $qa->autosavedstep = $autosavedstep; + $qa->steps[$i] = $qa->autosavedstep; + } else { + $qa->observer->notify_step_deleted($autosavedstep, $qa); + } + } return $qa; } diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php index 9fa6e0ce0b2..77541e24217 100644 --- a/question/engine/questionusage.php +++ b/question/engine/questionusage.php @@ -511,7 +511,49 @@ class question_usage_by_activity { * instead of the data from $_POST. */ public function process_all_actions($timestamp = null, $postdata = null) { - // note: we must not use "question_attempt::get_submitted_var()" because there is no attempt instance!!! + foreach ($this->get_slots_in_request($postdata) as $slot) { + if (!$this->validate_sequence_number($slot, $postdata)) { + continue; + } + $submitteddata = $this->extract_responses($slot, $postdata); + $this->process_action($slot, $submitteddata, $timestamp); + } + $this->update_question_flags($postdata); + } + + /** + * Process all the question autosave data in the current request. + * + * If there is a parameter slots included in the post data, then only + * those question numbers will be processed, otherwise all questions in this + * useage will be. + * + * This function also does {@link update_question_flags()}. + * + * @param int $timestamp optional, use this timestamp as 'now'. + * @param array $postdata optional, only intended for testing. Use this data + * instead of the data from $_POST. + */ + public function process_all_autosaves($timestamp = null, $postdata = null) { + foreach ($this->get_slots_in_request($postdata) as $slot) { + if (!$this->is_autosave_required($slot, $postdata)) { + continue; + } + $submitteddata = $this->extract_responses($slot, $postdata); + $this->process_autosave($slot, $submitteddata, $timestamp); + } + $this->update_question_flags($postdata); + } + + /** + * Get the list of slot numbers that should be processed as part of processing + * the current request. + * @param array $postdata optional, only intended for testing. Use this data + * instead of the data from $_POST. + * @return array of slot numbers. + */ + protected function get_slots_in_request($postdata = null) { + // Note: we must not use "question_attempt::get_submitted_var()" because there is no attempt instance!!! if (is_null($postdata)) { $slots = optional_param('slots', null, PARAM_SEQUENCE); } else if (array_key_exists('slots', $postdata)) { @@ -526,14 +568,7 @@ class question_usage_by_activity { } else { $slots = explode(',', $slots); } - foreach ($slots as $slot) { - if (!$this->validate_sequence_number($slot, $postdata)) { - continue; - } - $submitteddata = $this->extract_responses($slot, $postdata); - $this->process_action($slot, $submitteddata, $timestamp); - } - $this->update_question_flags($postdata); + return $slots; } /** @@ -560,6 +595,18 @@ class question_usage_by_activity { $this->observer->notify_attempt_modified($qa); } + /** + * Process an autosave action on a specific question. + * @param int $slot the number used to identify this question within this usage. + * @param $submitteddata the submitted data that constitutes the action. + */ + public function process_autosave($slot, $submitteddata, $timestamp = null) { + $qa = $this->get_question_attempt($slot); + if ($qa->process_autosave($submitteddata, $timestamp)) { + $this->observer->notify_attempt_modified($qa); + } + } + /** * Check that the sequence number, that detects weird things like the student * clicking back, is OK. If the sequence check variable is not present, returns diff --git a/question/engine/tests/questionattempt_db_test.php b/question/engine/tests/questionattempt_db_test.php index 44efa42eb03..1f9f9701f35 100644 --- a/question/engine/tests/questionattempt_db_test.php +++ b/question/engine/tests/questionattempt_db_test.php @@ -140,4 +140,116 @@ class question_attempt_db_test extends data_loading_method_test_base { $this->assertEquals(1, $step->get_user_id()); $this->assertEquals(array(), $step->get_all_data()); } + + public function test_load_with_autosaved_data() { + $records = new question_test_recordset(array( + array('questionattemptid', 'contextid', 'questionusageid', 'slot', + 'behaviour', 'questionid', 'variant', 'maxmark', 'minfraction', 'flagged', + 'questionsummary', 'rightanswer', 'responsesummary', 'timemodified', + 'attemptstepid', 'sequencenumber', 'state', 'fraction', + 'timecreated', 'userid', 'name', 'value'), + array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 4, -3, 'complete', null, 1256233715, 1, 'answer', '1'), + array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 1, 0, 'todo', null, 1256233700, 1, null, null), + array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 2, 1, 'complete', null, 1256233705, 1, 'answer', '1'), + array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 1, '', '', '', 1256233790, 3, 2, 'complete', null, 1256233710, 1, 'answer', '0'), + )); + + $question = test_question_maker::make_question('truefalse', 'true'); + $question->id = -1; + + question_bank::start_unit_test(); + question_bank::load_test_question_data($question); + $qa = question_attempt::load_from_records($records, 1, new question_usage_null_observer(), 'deferredfeedback'); + question_bank::end_unit_test(); + + $this->assertEquals($question->questiontext, $qa->get_question()->questiontext); + + $this->assertEquals(4, $qa->get_num_steps()); + $this->assertTrue($qa->has_autosaved_step()); + + $step = $qa->get_step(0); + $this->assertEquals(question_state::$todo, $step->get_state()); + $this->assertNull($step->get_fraction()); + $this->assertEquals(1256233700, $step->get_timecreated()); + $this->assertEquals(1, $step->get_user_id()); + $this->assertEquals(array(), $step->get_all_data()); + + $step = $qa->get_step(1); + $this->assertEquals(question_state::$complete, $step->get_state()); + $this->assertNull($step->get_fraction()); + $this->assertEquals(1256233705, $step->get_timecreated()); + $this->assertEquals(1, $step->get_user_id()); + $this->assertEquals(array('answer' => '1'), $step->get_all_data()); + + $step = $qa->get_step(2); + $this->assertEquals(question_state::$complete, $step->get_state()); + $this->assertNull($step->get_fraction()); + $this->assertEquals(1256233710, $step->get_timecreated()); + $this->assertEquals(1, $step->get_user_id()); + $this->assertEquals(array('answer' => '0'), $step->get_all_data()); + + $step = $qa->get_step(3); + $this->assertEquals(question_state::$complete, $step->get_state()); + $this->assertNull($step->get_fraction()); + $this->assertEquals(1256233715, $step->get_timecreated()); + $this->assertEquals(1, $step->get_user_id()); + $this->assertEquals(array('answer' => '1'), $step->get_all_data()); + } + + public function test_load_with_unnecessary_autosaved_data() { + // The point here is that the somehow (probably due to two things + // happening concurrently, we have autosaved data in the database that + // has already been superceded by real data, so it should be ignored. + // There is also a second lot of redundant data to delete. + $records = new question_test_recordset(array( + array('questionattemptid', 'contextid', 'questionusageid', 'slot', + 'behaviour', 'questionid', 'variant', 'maxmark', 'minfraction', 'flagged', + 'questionsummary', 'rightanswer', 'responsesummary', 'timemodified', + 'attemptstepid', 'sequencenumber', 'state', 'fraction', + 'timecreated', 'userid', 'name', 'value'), + array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 5, -2, 'complete', null, 1256233715, 1, 'answer', '0'), + array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 4, -1, 'complete', null, 1256233715, 1, 'answer', '0'), + array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 1, 0, 'todo', null, 1256233700, 1, null, null), + array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 0, '', '', '', 1256233790, 2, 1, 'complete', null, 1256233705, 1, 'answer', '1'), + array(1, 123, 1, 1, 'deferredfeedback', -1, 1, 2.0000000, 0.0000000, 1, '', '', '', 1256233790, 3, 2, 'complete', null, 1256233710, 1, 'answer', '0'), + )); + + $question = test_question_maker::make_question('truefalse', 'true'); + $question->id = -1; + + question_bank::start_unit_test(); + question_bank::load_test_question_data($question); + $observer = new testable_question_engine_unit_of_work( + question_engine::make_questions_usage_by_activity('unit_test', context_system::instance())); + $qa = question_attempt::load_from_records($records, 1, $observer, 'deferredfeedback'); + question_bank::end_unit_test(); + + $this->assertEquals($question->questiontext, $qa->get_question()->questiontext); + + $this->assertEquals(3, $qa->get_num_steps()); + $this->assertFalse($qa->has_autosaved_step()); + + $step = $qa->get_step(0); + $this->assertEquals(question_state::$todo, $step->get_state()); + $this->assertNull($step->get_fraction()); + $this->assertEquals(1256233700, $step->get_timecreated()); + $this->assertEquals(1, $step->get_user_id()); + $this->assertEquals(array(), $step->get_all_data()); + + $step = $qa->get_step(1); + $this->assertEquals(question_state::$complete, $step->get_state()); + $this->assertNull($step->get_fraction()); + $this->assertEquals(1256233705, $step->get_timecreated()); + $this->assertEquals(1, $step->get_user_id()); + $this->assertEquals(array('answer' => '1'), $step->get_all_data()); + + $step = $qa->get_step(2); + $this->assertEquals(question_state::$complete, $step->get_state()); + $this->assertNull($step->get_fraction()); + $this->assertEquals(1256233710, $step->get_timecreated()); + $this->assertEquals(1, $step->get_user_id()); + $this->assertEquals(array('answer' => '0'), $step->get_all_data()); + + $this->assertEquals(2, count($observer->get_steps_deleted())); + } } diff --git a/question/engine/tests/questionusage_autosave_test.php b/question/engine/tests/questionusage_autosave_test.php new file mode 100644 index 00000000000..9090d428b1e --- /dev/null +++ b/question/engine/tests/questionusage_autosave_test.php @@ -0,0 +1,562 @@ +. + +/** + * This file contains tests for the autosave code in the question_usage class. + * + * @package moodlecore + * @subpackage questionengine + * @copyright 2013 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once(dirname(__FILE__) . '/../lib.php'); +require_once(dirname(__FILE__) . '/helpers.php'); + + +/** + * Unit tests for the autosave parts of the {@link question_usage} class. + * + * @copyright 2013 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { + + public function test_autosave_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); + + // Process a response and check the expected result. + $this->process_submission(array('answer' => 'first response')); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->render(); + $this->check_output_contains_text_input('answer', 'first response'); + + // Process an autosave. + $this->load_quba(); + $this->process_autosave(array('answer' => 'second response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->load_quba(); + $this->render(); + $this->check_output_contains_text_input('answer', 'second response'); + + $this->delete_quba(); + } + + public function test_autosave_then_autosave_different_data() { + $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); + + // Process a response and check the expected result. + $this->process_submission(array('answer' => 'first response')); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->render(); + $this->check_output_contains_text_input('answer', 'first response'); + + // Process an autosave. + $this->load_quba(); + $this->process_autosave(array('answer' => 'second response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->load_quba(); + $this->render(); + $this->check_output_contains_text_input('answer', 'second response'); + + // Process a second autosave. + $this->load_quba(); + $this->process_autosave(array('answer' => 'third response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->load_quba(); + $this->render(); + $this->check_output_contains_text_input('answer', 'third response'); + + $this->delete_quba(); + } + + public function test_autosave_then_autosave_same_data() { + $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); + + // Process a response and check the expected result. + $this->process_submission(array('answer' => 'first response')); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->render(); + $this->check_output_contains_text_input('answer', 'first response'); + + // Process an autosave. + $this->load_quba(); + $this->process_autosave(array('answer' => 'second response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->load_quba(); + $this->render(); + $this->check_output_contains_text_input('answer', 'second response'); + + $stepid = $this->quba->get_question_attempt($this->slot)->get_last_step()->get_id(); + + // Process a second autosave. + $this->load_quba(); + $this->process_autosave(array('answer' => 'second response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Try to check it is really the same step + $newstepid = $this->quba->get_question_attempt($this->slot)->get_last_step()->get_id(); + $this->assertEquals($stepid, $newstepid); + + // Now check how that is re-displayed. + $this->load_quba(); + $this->render(); + $this->check_output_contains_text_input('answer', 'second response'); + + $this->delete_quba(); + } + + public function test_autosave_then_autosave_original_data() { + $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); + + // Process a response and check the expected result. + $this->process_submission(array('answer' => 'first response')); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->render(); + $this->check_output_contains_text_input('answer', 'first response'); + + // Process an autosave. + $this->load_quba(); + $this->process_autosave(array('answer' => 'second response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->load_quba(); + $this->render(); + $this->check_output_contains_text_input('answer', 'second response'); + + // Process a second autosave saving the original response. + // This should remove the autosave step. + $this->load_quba(); + $this->process_autosave(array('answer' => 'first response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->load_quba(); + $this->render(); + $this->check_output_contains_text_input('answer', 'first response'); + + $this->delete_quba(); + } + + public function test_autosave_then_real_save() { + $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); + + // Process a response and check the expected result. + $this->process_submission(array('answer' => 'first response')); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->render(); + $this->check_output_contains_text_input('answer', 'first response'); + + // Process an autosave. + $this->load_quba(); + $this->process_autosave(array('answer' => 'second response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->load_quba(); + $this->render(); + $this->check_output_contains_text_input('answer', 'second response'); + + // Now save for real a third response. + $this->process_submission(array('answer' => 'third response')); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->render(); + $this->check_output_contains_text_input('answer', 'third response'); + } + + public function test_autosave_then_real_save_same() { + $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); + + // Process a response and check the expected result. + $this->process_submission(array('answer' => 'first response')); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->render(); + $this->check_output_contains_text_input('answer', 'first response'); + + // Process an autosave. + $this->load_quba(); + $this->process_autosave(array('answer' => 'second response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->load_quba(); + $this->render(); + $this->check_output_contains_text_input('answer', 'second response'); + + // Now save for real of the same response. + $this->process_submission(array('answer' => 'second response')); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->render(); + $this->check_output_contains_text_input('answer', 'second response'); + } + + public function test_autosave_then_submit() { + $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); + + // Process a response and check the expected result. + $this->process_submission(array('answer' => 'first response')); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->render(); + $this->check_output_contains_text_input('answer', 'first response'); + + // Process an autosave. + $this->load_quba(); + $this->process_autosave(array('answer' => 'second response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->load_quba(); + $this->render(); + $this->check_output_contains_text_input('answer', 'second response'); + + // Now submit a third response. + $this->process_submission(array('answer' => 'third response')); + $this->quba->finish_all_questions(); + + $this->check_current_state(question_state::$gradedwrong); + $this->check_current_mark(0); + $this->check_step_count(4); + $this->save_quba(); + + // Now check how that is re-displayed. + $this->render(); + $this->check_output_contains_text_input('answer', 'third response', false); + } + + public function test_autosave_and_save_concurrently() { + // This test simulates the following scenario: + // 1. Student looking at a page of the quiz, and edits a field then waits. + // 2. Autosave starts. + // 3. Student immediately clicks Next, which submits the current page. + // In this situation, the real submit should beat the autosave, even + // thought they happen concurrently. We simulate this by opening a + // second db connections. + global $DB; + + // Open second connection + $cfg = $DB->export_dbconfig(); + if (!isset($cfg->dboptions)) { + $cfg->dboptions = array(); + } + $DB2 = moodle_database::get_driver_instance($cfg->dbtype, $cfg->dblibrary); + $DB2->connect($cfg->dbhost, $cfg->dbuser, $cfg->dbpass, $cfg->dbname, $cfg->prefix, $cfg->dboptions); + + // Since we need to commit our transactions in a given order, close the + // standard unit test transaction. + $this->preventResetByRollback(); + + $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->save_quba(); + + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_step_count(1); + + // Start to process an autosave on $DB. + $transaction = $DB->start_delegated_transaction(); + $this->load_quba($DB); + $this->process_autosave(array('answer' => 'autosaved response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba($DB); // Don't commit the transaction yet. + + // Now process a real submit on $DB2 (using a different response). + $transaction2 = $DB2->start_delegated_transaction(); + $this->load_quba($DB2); + $this->process_submission(array('answer' => 'real response')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + + // Now commit the first transaction. + $transaction->allow_commit(); + + // Now commit the other transaction. + $this->save_quba($DB2); + $transaction2->allow_commit(); + + // Now re-load and check how that is re-displayed. + $this->load_quba(); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->render(); + $this->check_output_contains_text_input('answer', 'real response'); + + $DB2->dispose(); + } + + public function test_concurrent_autosaves() { + // This test simulates the following scenario: + // 1. Student opens a page of the quiz in two separate browser. + // 2. Autosave starts in both at the same time. + // In this situation, one autosave will work, and the other one will + // get a unique key violation error. This is OK. + global $DB; + + // Open second connection + $cfg = $DB->export_dbconfig(); + if (!isset($cfg->dboptions)) { + $cfg->dboptions = array(); + } + $DB2 = moodle_database::get_driver_instance($cfg->dbtype, $cfg->dblibrary); + $DB2->connect($cfg->dbhost, $cfg->dbuser, $cfg->dbpass, $cfg->dbname, $cfg->prefix, $cfg->dboptions); + + // Since we need to commit our transactions in a given order, close the + // standard unit test transaction. + $this->preventResetByRollback(); + + $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->save_quba(); + + $this->check_current_state(question_state::$todo); + $this->check_current_mark(null); + $this->check_step_count(1); + + // Start to process an autosave on $DB. + $transaction = $DB->start_delegated_transaction(); + $this->load_quba($DB); + $this->process_autosave(array('answer' => 'autosaved response 1')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba($DB); // Don't commit the transaction yet. + + // Now process a real submit on $DB2 (using a different response). + $transaction2 = $DB2->start_delegated_transaction(); + $this->load_quba($DB2); + $this->process_autosave(array('answer' => 'autosaved response 2')); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + + // Now commit the first transaction. + $transaction->allow_commit(); + + // Now commit the other transaction. + $this->setExpectedException('dml_write_exception'); + $this->save_quba($DB2); + $transaction2->allow_commit(); + + // Now re-load and check how that is re-displayed. + $this->load_quba(); + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->render(); + $this->check_output_contains_text_input('answer', 'autosaved response 1'); + + $DB2->dispose(); + } +} diff --git a/question/tests/generator/lib.php b/question/tests/generator/lib.php index 2fd9f28fb8d..1fc91b2b4cc 100644 --- a/question/tests/generator/lib.php +++ b/question/tests/generator/lib.php @@ -59,4 +59,29 @@ class core_question_generator extends component_generator_base { $record['id'] = $DB->insert_record('question_categories', $record); return (object) $record; } + + /** + * Create a new question. The question is initialised using one of the + * examples from the appropriate {@link question_test_helper} subclass. + * Then, any files you want to change from the value in the base example you + * can override using $overrides. + * @param string $qtype the question type to create an example of. + * @param string $which as for the corresponding argument of + * {@link question_test_helper::get_question_form_data}. null for the default one. + * @param array|stdClass $overrides any fields that should be different from the base example. + */ + public function create_question($qtype, $which = null, $overrides = null) { + global $CFG; + require_once($CFG->dirroot . '/question/engine/tests/helpers.php'); + + $fromform = test_question_maker::get_question_form_data($qtype, $which); + $fromform = (object) $this->datagenerator->combine_defaults_and_record( + (array) $fromform, $overrides); + + $question = new stdClass(); + $question->category = $fromform->category; + $question->qtype = $qtype; + $question->createdby = 0; + return question_bank::get_qtype($qtype)->save_question($question, $fromform); + } }