From f02a10f38c47eb9783deb5fd86f5c58650cedd58 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Wed, 21 Aug 2013 11:53:23 +0100 Subject: [PATCH] MDL-39980 essay attempt-on-last. Handle the no-files case. This slighly changes the format for the way answers are stroed in the DB in the case where there is some HTML content, but no files. This should not cause any problems. --- question/engine/datalib.php | 33 +++++---- .../type/essay/tests/walkthrough_test.php | 73 +++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) diff --git a/question/engine/datalib.php b/question/engine/datalib.php index 175103d06df..beca1de177b 100644 --- a/question/engine/datalib.php +++ b/question/engine/datalib.php @@ -1279,22 +1279,21 @@ class question_file_saver implements question_response_files { $string .= $file->get_filepath() . $file->get_filename() . '|' . $file->get_contenthash() . '|'; } - - if ($string) { - $hash = md5($string); - } else { - $hash = ''; - } + $hash = md5($string); if (is_null($text)) { - return $hash; + if ($string) { + return $hash; + } else { + return ''; + } } // We add the file hash so a simple string comparison will say if the // files have been changed. First strip off any existing file hash. - $text = preg_replace('/\s*\s*$/', '', $text); - $text = file_rewrite_urls_to_pluginfile($text, $draftitemid); - if ($hash) { + if ($text !== '') { + $text = preg_replace('/\s*\s*$/', '', $text); + $text = file_rewrite_urls_to_pluginfile($text, $draftitemid); $text .= ''; } return $text; @@ -1391,8 +1390,15 @@ class question_file_loader implements question_response_files { */ public function get_question_file_saver() { - // Value will be either a plain MD5 hash, or some real content, followed - // by an MD5 hash in a HTML comment. We only want the value in the latter case. + // There are three possibilities here for what $value will look like: + // 1) some HTML content followed by an MD5 hash in a HTML comment; + // 2) a plain MD5 hash; + // 3) or some real content, without any hash. + // The problem is that 3) is ambiguous in the case where a student writes + // a response that looks exactly like an MD5 hash. For attempts made now, + // we avoid case 3) by always going for case 1) or 2) (except when the + // response is blank. However, there may be case 3) data in the database + // so we need to handle it as best we can. if (preg_match('/\s*\s*$/', $this->value)) { $value = preg_replace('/\s*\s*$/', '', $this->value); @@ -1400,8 +1406,7 @@ class question_file_loader implements question_response_files { $value = null; } else { - throw new coding_exception('$value passed to question_file_loader::get_question_file_saver' . - ' was not of the expected form.'); + $value = $this->value; } list($draftid, $text) = $this->step->prepare_response_files_draft_itemid_with_text( diff --git a/question/type/essay/tests/walkthrough_test.php b/question/type/essay/tests/walkthrough_test.php index fbc7a1ad811..e64149212cd 100644 --- a/question/type/essay/tests/walkthrough_test.php +++ b/question/type/essay/tests/walkthrough_test.php @@ -348,4 +348,77 @@ class qtype_essay_walkthrough_testcase extends qbehaviour_walkthrough_test_base $this->check_current_mark(null); $this->check_step_count(1); } + + public function test_deferred_feedback_html_editor_with_files_attempt_on_last_no_files_uploaded() { + global $CFG, $USER; + + $this->resetAfterTest(true); + $this->setAdminUser(); + $usercontextid = context_user::instance($USER->id)->id; + $fs = get_file_storage(); + + // Create an essay question in the DB. + $generator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $cat = $generator->create_question_category(); + $question = $generator->create_question('essay', 'editorfilepicker', array('category' => $cat->id)); + + // Start attempt at the 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. + // First we need to get the draft item ids. + $this->render(); + if (!preg_match('/env=editor&.*?itemid=(\d+)&/', $this->currentoutput, $matches)) { + throw new coding_exception('Editor draft item id not found.'); + } + $editordraftid = $matches[1]; + if (!preg_match('/env=filemanager&action=browse&.*?itemid=(\d+)&/', $this->currentoutput, $matches)) { + throw new coding_exception('File manager draft item id not found.'); + } + $attachementsdraftid = $matches[1]; + + $this->process_submission(array( + 'answer' => 'I refuse to draw you a picture, so there!', + 'answerformat' => FORMAT_HTML, + 'answer:itemid' => $editordraftid, + 'attachments' => $attachementsdraftid)); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(2); + $this->save_quba(); + + // Now submit all and finish. + $this->finish(); + $this->check_current_state(question_state::$needsgrading); + $this->check_current_mark(null); + $this->check_step_count(3); + $this->save_quba(); + + // Now start a new attempt based on the old one. + $this->load_quba(); + $oldqa = $this->get_question_attempt(); + + $q = question_bank::load_question($question->id); + $this->quba = question_engine::make_questions_usage_by_activity('unit_test', + context_system::instance()); + $this->quba->set_preferred_behaviour('deferredfeedback'); + $this->slot = $this->quba->add_question($q, 1); + $this->quba->start_question_based_on($this->slot, $oldqa); + + $this->check_current_state(question_state::$complete); + $this->check_current_mark(null); + $this->check_step_count(1); + $this->save_quba(); + + // Check the display. + $this->load_quba(); + $this->render(); + $this->assertRegExp('/I refuse to draw you a picture, so there!/', $this->currentoutput); + } }