MDL-57239 qbehaviour_interactive: fix Try again button when reviewing

question/behaviour/interactive/behaviour.php

@@ -32,7 +32,7 @@
  * Question behaviour for the interactive model.
  *
  * Each question has a submit button next to it which the student can use to
- * submit it. Once the qustion is submitted, it is not possible for the
+ * submit it. Once the question is submitted, it is not possible for the
  * student to change their answer any more, but the student gets full feedback
  * straight away.
  *
@@ -41,14 +41,17 @@
  */
 class qbehaviour_interactive extends question_behaviour_with_multiple_tries {
     /**
-     * Special value used for {@link question_display_options::$readonly when
-     * we are showing the try again button to the student during an attempt.
-     * The particular number was chosen randomly. PHP will treat it the same
-     * as true, but in the renderer we reconginse it display the try again
-     * button enabled even though the rest of the question is disabled.
-     * @var integer
+     * Constant used only in {@link adjust_display_options()} below and
+     * {@link (qbehaviour_interactive_renderer}.
+     * @var int
      */
-    const READONLY_EXCEPT_TRY_AGAIN = 23485299;
+    const TRY_AGAIN_VISIBLE = 0x10;
+    /**
+     * Constant used only in {@link adjust_display_options()} below and
+     * {@link (qbehaviour_interactive_renderer}.
+     * @var int
+     */
+    const TRY_AGAIN_VISIBLE_READONLY = 0x11;
 
     public function is_compatible_question(question_definition $question) {
         return $question instanceof question_automatically_gradable;
@@ -82,6 +85,14 @@ public function adjust_display_options(question_display_options $options) {
             return;
         }
 
+        // The question in in a try-again state. We need the to let the renderer know this.
+        // The API for question-rendering is defined by the question engine, but we
+        // don't want to add logic in the renderer, so we are limited in how we can do this.
+        // However, when the question is in this state, all the question-type controls
+        // need to be rendered read-only. Therefore, we can conveniently pass this information
+        // by setting special true-like values in $options->readonly (but this is a bit of a hack).
+        $options->readonly = $options->readonly ? self::TRY_AGAIN_VISIBLE_READONLY : self::TRY_AGAIN_VISIBLE;
+
         // Let the hint adjust the options.
         $hint = $this->get_applicable_hint();
         if (!is_null($hint)) {
@@ -93,12 +104,6 @@ public function adjust_display_options(question_display_options $options) {
         parent::adjust_display_options($options);
         $options->feedback = $save->feedback;
         $options->numpartscorrect = $save->numpartscorrect;
-
-        // In a try-again state, everything except the try again button
-        // Should be read-only. This is a mild hack to achieve this.
-        if (!$options->readonly) {
-            $options->readonly = self::READONLY_EXCEPT_TRY_AGAIN;
-        }
     }
 
     public function get_applicable_hint() {

question/behaviour/interactive/renderer.php

@@ -36,14 +36,19 @@
  */
 class qbehaviour_interactive_renderer extends qbehaviour_renderer {
     public function controls(question_attempt $qa, question_display_options $options) {
-        if ($options->readonly === qbehaviour_interactive::READONLY_EXCEPT_TRY_AGAIN) {
+        if ($options->readonly === qbehaviour_interactive::TRY_AGAIN_VISIBLE ||
+                $options->readonly === qbehaviour_interactive::TRY_AGAIN_VISIBLE_READONLY) {
+            // We are in the try again state, so no submit button.
             return '';
         }
         return $this->submit_button($qa, $options);
     }
 
     public function feedback(question_attempt $qa, question_display_options $options) {
-        if (!$qa->get_state()->is_active() || !$options->readonly) {
+        // Show the Try again button if we are in try-again state.
+        if (!$qa->get_state()->is_active() ||
+                ($options->readonly !== qbehaviour_interactive::TRY_AGAIN_VISIBLE &&
+                        $options->readonly !== qbehaviour_interactive::TRY_AGAIN_VISIBLE_READONLY)) {
             return '';
         }
 
@@ -54,7 +59,8 @@ public function feedback(question_attempt $qa, question_display_options $options
             'value' => get_string('tryagain', 'qbehaviour_interactive'),
             'class' => 'submit btn',
         );
-        if ($options->readonly !== qbehaviour_interactive::READONLY_EXCEPT_TRY_AGAIN) {
+        if ($options->readonly === qbehaviour_interactive::TRY_AGAIN_VISIBLE_READONLY) {
+            // This means the question really was rendered with read-only option.
             $attributes['disabled'] = 'disabled';
         }
         $output = html_writer::empty_tag('input', $attributes);

question/behaviour/interactive/tests/walkthrough_test.php

@@ -18,8 +18,7 @@
  * This file contains tests that walks a question through the interactive
  * behaviour.
  *
- * @package    qbehaviour
- * @subpackage interactive
+ * @package    qbehaviour_interactive
  * @copyright  2009 The Open University
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
@@ -483,4 +482,52 @@ public function test_interactive_regrade_changing_num_tries_finished() {
         // and I am currently interested in testing regrading here.
         $this->check_current_mark(1);
     }
+
+    public function test_review_of_interactive_questions_before_finished() {
+        // Create a multichoice multiple question.
+        $q = test_question_maker::make_question('shortanswer');
+        $q->hints = array(
+                new question_hint_with_parts(0, 'This is the first hint.', FORMAT_HTML, true, true),
+                new question_hint_with_parts(0, 'This is the second hint.', FORMAT_HTML, true, true),
+        );
+        $this->start_attempt_at_question($q, 'interactive', 3);
+
+        // Check the initial state.
+        $this->check_current_state(question_state::$todo);
+        $this->check_current_mark(null);
+        $this->check_current_output(
+                $this->get_contains_submit_button_expectation(true),
+                $this->get_does_not_contain_feedback_expectation(),
+                $this->get_tries_remaining_expectation(3),
+                $this->get_does_not_contain_try_again_button_expectation());
+
+        // Now check what the teacher sees when they review the question.
+        $this->displayoptions->readonly = true;
+        $this->check_current_output(
+                $this->get_contains_submit_button_expectation(false),
+                $this->get_does_not_contain_feedback_expectation(),
+                $this->get_tries_remaining_expectation(3),
+                $this->get_does_not_contain_try_again_button_expectation());
+        $this->displayoptions->readonly = false;
+
+        // Submit a wrong answer.
+        $this->process_submission(array('answer' => 'cat', '-submit' => 1));
+
+        // Check the Try again button now shows up correctly.
+        $this->check_current_state(question_state::$todo);
+        $this->check_current_mark(null);
+        $this->check_current_output(
+                $this->get_does_not_contain_submit_button_expectation(),
+                $this->get_contains_hint_expectation('This is the first hint.'),
+                $this->get_tries_remaining_expectation(2),
+                $this->get_contains_try_again_button_expectation(true));
+
+        // And check that a disabled Try again button shows up when the question is reviewed.
+        $this->displayoptions->readonly = true;
+        $this->check_current_output(
+                $this->get_does_not_contain_submit_button_expectation(),
+                $this->get_contains_hint_expectation('This is the first hint.'),
+                $this->get_tries_remaining_expectation(2),
+                $this->get_contains_try_again_button_expectation(false));
+    }
 }