Merge branch 'MDL-43874' of git://github.com/timhunt/moodle

mod/quiz/locallib.php

@@ -1375,6 +1375,7 @@ function quiz_get_review_options($quiz, $attempt, $context) {
         $options->marks = question_display_options::MARK_AND_MAX;
         $options->feedback = question_display_options::VISIBLE;
         $options->numpartscorrect = question_display_options::VISIBLE;
+        $options->manualcomment = question_display_options::VISIBLE;
         $options->generalfeedback = question_display_options::VISIBLE;
         $options->rightanswer = question_display_options::VISIBLE;
         $options->overallfeedback = question_display_options::VISIBLE;
@@ -1936,6 +1937,7 @@ class mod_quiz_display_options extends question_display_options {
         $options->overallfeedback = self::extract($quiz->reviewoverallfeedback, $when);
 
         $options->numpartscorrect = $options->feedback;
+        $options->manualcomment = $options->feedback;
 
         if ($quiz->questiondecimalpoints != -1) {
             $options->markdp = $quiz->questiondecimalpoints;

mod/quiz/tests/quizdisplayoptions_test.php

@@ -55,6 +55,10 @@ class mod_quiz_display_options_testcase extends basic_testcase {
         $this->assertEquals(true, $options->attempt);
         $this->assertEquals(mod_quiz_display_options::VISIBLE, $options->correctness);
         $this->assertEquals(mod_quiz_display_options::MAX_ONLY, $options->marks);
+        $this->assertEquals(mod_quiz_display_options::VISIBLE, $options->feedback);
+        // The next two should be controlled by the same settings as ->feedback.
+        $this->assertEquals(mod_quiz_display_options::VISIBLE, $options->numpartscorrect);
+        $this->assertEquals(mod_quiz_display_options::VISIBLE, $options->manualcomment);
         $this->assertEquals(2, $options->markdp);
 
         $quiz->questiondecimalpoints = 5;
@@ -64,6 +68,9 @@ class mod_quiz_display_options_testcase extends basic_testcase {
         $this->assertEquals(mod_quiz_display_options::MARK_AND_MAX, $options->marks);
         $this->assertEquals(mod_quiz_display_options::VISIBLE, $options->generalfeedback);
         $this->assertEquals(mod_quiz_display_options::HIDDEN, $options->feedback);
+        // The next two should be controlled by the same settings as ->feedback.
+        $this->assertEquals(mod_quiz_display_options::HIDDEN, $options->numpartscorrect);
+        $this->assertEquals(mod_quiz_display_options::HIDDEN, $options->manualcomment);
         $this->assertEquals(5, $options->markdp);
 
         $options = mod_quiz_display_options::make_from_quiz($quiz,

question/behaviour/manualgraded/tests/walkthrough_test.php

@@ -318,4 +318,58 @@ class qbehaviour_manualgraded_walkthrough_testcase extends qbehaviour_walkthroug
         $this->check_current_state(question_state::$mangrwrong);
         $this->check_current_mark(0);
     }
+
+    public function test_manual_graded_respects_display_options() {
+        // This test is for MDL-43874. Manual comments were not respecting the
+        // Display options for feedback.
+
+        // The current text editor depends on the users profile setting - so it needs a valid user.
+        $this->setAdminUser();
+
+        // Create an essay question.
+        $essay = test_question_maker::make_an_essay_question();
+        $this->start_attempt_at_question($essay, 'deferredfeedback', 10);
+
+        // Check the right model is being used.
+        $this->assertEquals('manualgraded', $this->quba->get_question_attempt(
+                $this->slot)->get_behaviour_name());
+
+        // Check the initial state.
+        $this->check_current_state(question_state::$todo);
+        $this->check_current_mark(null);
+        $this->check_current_output($this->get_contains_question_text_expectation($essay),
+                $this->get_does_not_contain_feedback_expectation());
+
+        // Simulate some data submitted by the student.
+        $this->process_submission(array('answer' => 'This is my wonderful essay!', 'answerformat' => FORMAT_HTML));
+
+        // Verify.
+        $this->check_current_state(question_state::$complete);
+        $this->check_current_mark(null);
+        $this->check_current_output(
+                new question_contains_tag_with_attribute('textarea', 'name',
+                $this->quba->get_question_attempt($this->slot)->get_qt_field_name('answer')),
+                $this->get_does_not_contain_feedback_expectation());
+
+        // Finish the attempt.
+        $this->quba->finish_all_questions();
+
+        // Verify.
+        $this->check_current_state(question_state::$needsgrading);
+        $this->check_current_mark(null);
+        $this->assertEquals('This is my wonderful essay!',
+                $this->quba->get_response_summary($this->slot));
+
+        // Process a comment and a grade.
+        $this->manual_grade('This should only appear if the displya options allow it', 5, FORMAT_HTML);
+
+        // Verify.
+        $this->check_current_state(question_state::$mangrpartial);
+        $this->check_current_mark(5);
+
+        $this->displayoptions->manualcomment = question_display_options::HIDDEN;
+        $this->check_output_does_not_contain('This should only appear if the displya options allow it');
+        $this->displayoptions->manualcomment = question_display_options::VISIBLE;
+        $this->check_output_contains('This should only appear if the displya options allow it');
+    }
 }

question/engine/tests/helpers.php

@@ -854,13 +854,22 @@ abstract class qbehaviour_walkthrough_test_base extends question_testcase {
                 'Looking for a hidden input with attributes ' . html_writer::attributes($attributes) . ' in ' . $this->currentoutput);
     }
 
-    protected function check_output_contains_lang_string($identifier, $component = '', $a = null) {
+    protected function check_output_contains($string) {
         $this->render();
-        $string = get_string($identifier, $component, $a);
         $this->assertContains($string, $this->currentoutput,
                 'Expected string ' . $string . ' not found in ' . $this->currentoutput);
     }
 
+    protected function check_output_does_not_contain($string) {
+        $this->render();
+        $this->assertNotContains($string, $this->currentoutput,
+                'String ' . $string . ' unexpectedly found in ' . $this->currentoutput);
+    }
+
+    protected function check_output_contains_lang_string($identifier, $component = '', $a = null) {
+        $this->check_output_contains(get_string($identifier, $component, $a));
+    }
+
     protected function get_tag_matcher($tag, $attributes) {
         return array(
             'tag' => $tag,