From e101ec9fd7078ff56705a580d408db864a6f3088 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Mon, 27 Jan 2014 11:57:55 +0000 Subject: [PATCH] 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 | 2 + mod/quiz/tests/quizdisplayoptions_test.php | 7 +++ .../manualgraded/tests/walkthrough_test.php | 54 ++++++++++++++++++++++ question/engine/tests/helpers.php | 13 +++++- 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 8b93a9e2164..bc62fbef37e 100644 --- a/mod/quiz/locallib.php +++ b/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; diff --git a/mod/quiz/tests/quizdisplayoptions_test.php b/mod/quiz/tests/quizdisplayoptions_test.php index 5efb6e1827d..e0b51b0076b 100644 --- a/mod/quiz/tests/quizdisplayoptions_test.php +++ b/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, diff --git a/question/behaviour/manualgraded/tests/walkthrough_test.php b/question/behaviour/manualgraded/tests/walkthrough_test.php index 1207ad75040..f17c681e54b 100644 --- a/question/behaviour/manualgraded/tests/walkthrough_test.php +++ b/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'); + } } diff --git a/question/engine/tests/helpers.php b/question/engine/tests/helpers.php index 69c02d08f33..72e8f6f8427 100644 --- a/question/engine/tests/helpers.php +++ b/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, -- 2.11.4.GIT