From c7fbfe46f91e3bb0f71c7b224ef9571e48a5a90a Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Thu, 4 Apr 2013 15:32:32 +0100 Subject: [PATCH] MDL-38538 question autosave: fix sequencecheck handling. --- question/engine/questionattempt.php | 15 ++++++++++ question/engine/questionusage.php | 4 +-- question/engine/renderer.php | 2 +- question/engine/tests/helpers.php | 12 +++++++- .../tests/questionusage_autosave_test.php | 28 +++++++++++++++++-- 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/question/engine/questionattempt.php b/question/engine/questionattempt.php index 06afd49106e..8def4331182 100644 --- a/question/engine/questionattempt.php +++ b/question/engine/questionattempt.php @@ -347,6 +347,21 @@ class question_attempt { return $this->steps[$i]; } + /** + * Get the number of real steps in this attempt. + * This is put as a hidden field in the HTML, so that when we receive some + * data to process, then we can check that it came from the question + * in the state we are now it. + * @return int a number that summarises the current state of this question attempt. + */ + public function get_sequence_check_count() { + $numrealsteps = $this->get_num_steps(); + if ($this->has_autosaved_step()) { + $numrealsteps -= 1; + } + return $numrealsteps; + } + /** * Get the number of steps in this attempt. * For internal/test code use only. diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php index a1f949481c0..ae329a47cfb 100644 --- a/question/engine/questionusage.php +++ b/question/engine/questionusage.php @@ -623,7 +623,7 @@ class question_usage_by_activity { $qa->get_control_field_name('sequencecheck'), PARAM_INT, $postdata); if (is_null($sequencecheck)) { return false; - } else if ($sequencecheck != $qa->get_num_steps()) { + } else if ($sequencecheck != $qa->get_sequence_check_count()) { throw new question_out_of_sequence_exception($this->id, $slot, $postdata); } else { return true; @@ -642,7 +642,7 @@ class question_usage_by_activity { $qa->get_control_field_name('sequencecheck'), PARAM_INT, $postdata); if (is_null($sequencecheck)) { return false; - } else if ($sequencecheck != $qa->get_num_steps()) { + } else if ($sequencecheck != $qa->get_sequence_check_count()) { return false; } else { return true; diff --git a/question/engine/renderer.php b/question/engine/renderer.php index d0f44c0ad18..f6793d4113d 100644 --- a/question/engine/renderer.php +++ b/question/engine/renderer.php @@ -325,7 +325,7 @@ class core_question_renderer extends plugin_renderer_base { $output .= html_writer::empty_tag('input', array( 'type' => 'hidden', 'name' => $qa->get_control_field_name('sequencecheck'), - 'value' => $qa->get_num_steps())); + 'value' => $qa->get_sequence_check_count())); $output .= $qtoutput->formulation_and_controls($qa, $options); if ($options->clearwrong) { $output .= $qtoutput->clear_wrong($qa); diff --git a/question/engine/tests/helpers.php b/question/engine/tests/helpers.php index 8ff4e443819..746a981b6b0 100644 --- a/question/engine/tests/helpers.php +++ b/question/engine/tests/helpers.php @@ -693,7 +693,7 @@ abstract class qbehaviour_walkthrough_test_base extends question_testcase { $prefix = $this->quba->get_field_prefix($this->slot); $fulldata = array( 'slots' => $this->slot, - $prefix . ':sequencecheck' => $this->get_question_attempt()->get_num_steps(), + $prefix . ':sequencecheck' => $this->get_question_attempt()->get_sequence_check_count(), ); foreach ($data as $name => $value) { $fulldata[$prefix . $name] = $value; @@ -786,6 +786,16 @@ abstract class qbehaviour_walkthrough_test_base extends question_testcase { } } + protected function check_output_contains_hidden_input($name, $value) { + $attributes = array( + 'type' => 'hidden', + 'name' => $this->quba->get_field_prefix($this->slot) . $name, + 'value' => $value, + ); + $this->assertTag($this->get_tag_matcher('input', $attributes), $this->currentoutput, + 'Looking for a hidden input with attributes ' . html_writer::attributes($attributes) . ' in ' . $this->currentoutput); + } + protected function get_tag_matcher($tag, $attributes) { return array( 'tag' => $tag, diff --git a/question/engine/tests/questionusage_autosave_test.php b/question/engine/tests/questionusage_autosave_test.php index 304c94102e3..8b0a45c8282 100644 --- a/question/engine/tests/questionusage_autosave_test.php +++ b/question/engine/tests/questionusage_autosave_test.php @@ -65,6 +65,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'first response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Process an autosave. $this->load_quba(); @@ -78,6 +79,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'second response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); $this->delete_quba(); } @@ -108,6 +110,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'first response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Process an autosave. $this->load_quba(); @@ -121,6 +124,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'second response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Process a second autosave. $this->load_quba(); @@ -134,6 +138,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'third response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); $this->delete_quba(); } @@ -164,6 +169,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'first response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Process an autosave. $this->load_quba(); @@ -177,6 +183,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'second response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); $stepid = $this->quba->get_question_attempt($this->slot)->get_last_step()->get_id(); @@ -196,6 +203,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'second response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); $this->delete_quba(); } @@ -226,6 +234,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'first response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Process an autosave. $this->load_quba(); @@ -239,6 +248,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'second response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Process a second autosave saving the original response. // This should remove the autosave step. @@ -253,6 +263,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'first response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); $this->delete_quba(); } @@ -283,6 +294,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'first response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Process an autosave. $this->load_quba(); @@ -296,6 +308,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'second response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Now save for real a third response. $this->process_submission(array('answer' => 'third response')); @@ -308,6 +321,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'third response'); + $this->check_output_contains_hidden_input(':sequencecheck', 3); } public function test_autosave_then_real_save_same() { @@ -336,6 +350,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'first response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Process an autosave. $this->load_quba(); @@ -349,6 +364,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'second response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Now save for real of the same response. $this->process_submission(array('answer' => 'second response')); @@ -361,6 +377,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'second response'); + $this->check_output_contains_hidden_input(':sequencecheck', 3); } public function test_autosave_then_submit() { @@ -389,6 +406,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'first response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Process an autosave. $this->load_quba(); @@ -402,6 +420,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'second response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); // Now submit a third response. $this->process_submission(array('answer' => 'third response')); @@ -415,6 +434,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'third response', false); + $this->check_output_contains_hidden_input(':sequencecheck', 4); } public function test_autosave_and_save_concurrently() { @@ -485,6 +505,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->check_step_count(2); $this->render(); $this->check_output_contains_text_input('answer', 'real response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); $DB2->dispose(); } @@ -556,6 +577,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->check_step_count(2); $this->render(); $this->check_output_contains_text_input('answer', 'autosaved response 1'); + $this->check_output_contains_hidden_input(':sequencecheck', 1); $DB2->dispose(); } @@ -586,11 +608,12 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { // Now check how that is re-displayed. $this->render(); $this->check_output_contains_text_input('answer', 'first response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); - // Process an autosave with a sequence number 1 to small (so from the past). + // Process an autosave with a sequence number 1 too small (so from the past). $this->load_quba(); $postdata = $this->response_data_to_post(array('answer' => 'obsolete response')); - $postdata[$this->quba->get_field_prefix($this->slot) . ':sequencecheck'] = $this->get_question_attempt()->get_num_steps() - 1; + $postdata[$this->quba->get_field_prefix($this->slot) . ':sequencecheck'] = $this->get_question_attempt()->get_sequence_check_count() - 1; $this->quba->process_all_autosaves(null, $postdata); $this->check_current_state(question_state::$complete); $this->check_current_mark(null); @@ -601,6 +624,7 @@ class question_usage_autosave_test extends qbehaviour_walkthrough_test_base { $this->load_quba(); $this->render(); $this->check_output_contains_text_input('answer', 'first response'); + $this->check_output_contains_hidden_input(':sequencecheck', 2); $this->delete_quba(); }