MDL-43874 quiz, teacher comments should respect display options.

Whether the comments on manually graded questions were visible to
students should have been controlled by the 'Specific feedback' Review
option in the quiz settings. However, the quiz was not setting
$displayoptions->manualcomment, so it did not work.

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

@@ -855,13 +855,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,