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.

diff --git a/question/engine/datalib.php b/question/engine/datalib.php
index a75cbd4..e52b80f 100644 (file)
--- 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*<!-- File hash: \w+ -->\s*$/', '', $text);
-        $text = file_rewrite_urls_to_pluginfile($text, $draftitemid);
-        if ($hash) {
+        if ($text !== '') {
+            $text = preg_replace('/\s*<!-- File hash: \w+ -->\s*$/', '', $text);
+            $text = file_rewrite_urls_to_pluginfile($text, $draftitemid);
             $text .= '<!-- File hash: ' . $hash . ' -->';
         }
         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*<!-- File hash: [0-9a-zA-Z]{32} -->\s*$/', $this->value)) {
             $value = preg_replace('/\s*<!-- File hash: [0-9a-zA-Z]{32} -->\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 fbc7a1a..e641492 100644 (file)
--- 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);
+    }
 }