MDL-29772 Adaptive question behaviour: improve penalty information

also fixes MDL-29780: suppress penalty information if question penalty is 0

question/behaviour/adaptive/lang/en/qbehaviour_adaptive.php

@@ -24,7 +24,8 @@
  */
 
 $string['gradingdetails'] = 'Marks for this submission: {$a->raw}/{$a->max}.';
-$string['gradingdetailsadjustment'] = 'With previous penalties this gives <strong>{$a->cur}/{$a->max}</strong>.';
+$string['gradingdetailsadjustment'] = 'Accounting for previous tries, this gives <strong>{$a->cur}/{$a->max}</strong>.';
 $string['gradingdetailspenalty'] = 'This submission attracted a penalty of {$a}.';
+$string['gradingdetailspenaltytotal'] = 'Total penalties so far: {$a}.';
 $string['notcomplete'] = 'Not complete';
 $string['pluginname'] = 'Adaptive mode';

question/behaviour/adaptive/renderer.php

@@ -88,7 +88,11 @@ public function feedback(question_attempt $qa, question_display_options $options
      */
     protected function penalty_info(question_attempt $qa, $mark,
             question_display_options $options) {
-        if (!$qa->get_question()->penalty) {
+
+        $currentpenalty = $qa->get_question()->penalty * $qa->get_max_mark();
+        $totalpenalty = $currentpenalty * $qa->get_last_behaviour_var('_try', 0);
+
+        if ($currentpenalty == 0) {
             return '';
         }
         $output = '';
@@ -101,7 +105,13 @@ protected function penalty_info(question_attempt $qa, $mark,
         // Print information about any new penalty, only relevant if the answer can be improved.
         if ($qa->get_behaviour()->is_state_improvable($qa->get_state())) {
             $output .= ' ' . get_string('gradingdetailspenalty', 'qbehaviour_adaptive',
-                    format_float($qa->get_question()->penalty, $options->markdp));
+                    format_float($currentpenalty, $options->markdp));
+
+            // Print information about total penalties so far, if larger than current penalty.
+            if ($totalpenalty > $currentpenalty) {
+                $output .= ' ' . get_string('gradingdetailspenaltytotal', 'qbehaviour_adaptive',
+                        format_float($totalpenalty, $options->markdp));
+            }
         }
 
         return $output;

question/behaviour/adaptive/simpletest/testwalkthrough.php

@@ -51,6 +51,18 @@ protected function get_does_not_contain_penalty_info_expectation() {
         return new NoPatternExpectation($penaltypattern);
     }
 
+    protected function get_contains_total_penalty_expectation($penalty) {
+        $penaltyinfo = get_string('gradingdetailspenaltytotal', 'qbehaviour_adaptive',
+                                  format_float($penalty, $this->displayoptions->markdp));
+        return new PatternExpectation('/'.preg_quote($penaltyinfo).'/');
+    }
+
+    protected function get_does_not_contain_total_penalty_expectation() {
+        $penaltyinfo = get_string('gradingdetailspenaltytotal', 'qbehaviour_adaptive', 'XXXXX');
+        $penaltypattern = '/'.str_replace('XXXXX', '\\w*', preg_quote($penaltyinfo)).'/';
+        return new NoPatternExpectation($penaltypattern);
+    }
+
     public function test_adaptive_multichoice() {
 
         // Create a multiple choice, single response question.
@@ -85,7 +97,8 @@ public function test_adaptive_multichoice() {
                 $this->get_contains_mc_radio_expectation(($wrongindex + 1) % 3, true, false),
                 $this->get_contains_mc_radio_expectation(($wrongindex + 2) % 3, true, false),
                 $this->get_contains_incorrect_expectation(),
-                $this->get_contains_penalty_info_expectation(0.33));
+                $this->get_contains_penalty_info_expectation(1.00),
+                $this->get_does_not_contain_total_penalty_expectation());
         $this->assertPattern('/B|C/',
                 $this->quba->get_response_summary($this->slot));
 
@@ -115,7 +128,8 @@ public function test_adaptive_multichoice() {
                 $this->get_contains_mc_radio_expectation(($rightindex + 1) % 3, true, false),
                 $this->get_contains_mc_radio_expectation(($rightindex + 2) % 3, true, false),
                 $this->get_contains_correct_expectation(),
-                $this->get_does_not_contain_penalty_info_expectation());
+                $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation());
         $this->assertEqual('A',
                 $this->quba->get_response_summary($this->slot));
 
@@ -186,7 +200,8 @@ public function test_adaptive_multichoice2() {
                 $this->get_contains_mark_summary(2),
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_correct_expectation(),
-                $this->get_does_not_contain_penalty_info_expectation());
+                $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation());
 
         // Save the same correct answer again. Should not do anything.
         $numsteps = $this->get_step_count();
@@ -235,6 +250,7 @@ public function test_adaptive_shortanswer_partially_right() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_partcorrect_expectation(),
                 $this->get_contains_penalty_info_expectation(0.33),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Submit an incorrect answer.
@@ -248,6 +264,7 @@ public function test_adaptive_shortanswer_partially_right() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_incorrect_expectation(),
                 $this->get_contains_penalty_info_expectation(0.33),
+                $this->get_contains_total_penalty_expectation(0.67),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Submit a correct answer.
@@ -261,6 +278,7 @@ public function test_adaptive_shortanswer_partially_right() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_correct_expectation(),
                 $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Finish the attempt.
@@ -280,7 +298,7 @@ public function test_adaptive_shortanswer_wrong_right_wrong() {
 
         // Create a short answer question
         $sa = test_question_maker::make_a_shortanswer_question();
-        $this->start_attempt_at_question($sa, 'adaptive');
+        $this->start_attempt_at_question($sa, 'adaptive', 6);
 
         // Check the initial state.
         $this->check_current_state(question_state::$todo);
@@ -300,7 +318,8 @@ public function test_adaptive_shortanswer_wrong_right_wrong() {
                 $this->get_contains_mark_summary(0),
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_incorrect_expectation(),
-                $this->get_contains_penalty_info_expectation(0.33),
+                $this->get_contains_penalty_info_expectation(2.00),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Submit the same wrong answer again. Nothing should change.
@@ -313,43 +332,46 @@ public function test_adaptive_shortanswer_wrong_right_wrong() {
                 $this->get_contains_mark_summary(0),
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_incorrect_expectation(),
-                $this->get_contains_penalty_info_expectation(0.33),
+                $this->get_contains_penalty_info_expectation(2.00),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Submit a correct answer.
         $this->process_submission(array('-submit' => 1, 'answer' => 'frog'));
 
         // Verify.
         $this->check_current_state(question_state::$complete);
-        $this->check_current_mark(0.66666667);
+        $this->check_current_mark(4.00);
         $this->check_current_output(
-                $this->get_contains_mark_summary(0.67),
+                $this->get_contains_mark_summary(4.00),
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_correct_expectation(),
                 $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Submit another incorrect answer.
         $this->process_submission(array('-submit' => 1, 'answer' => 'bumblebee'));
 
         // Verify.
         $this->check_current_state(question_state::$complete);
-        $this->check_current_mark(0.66666667);
+        $this->check_current_mark(4.00);
         $this->check_current_output(
-                $this->get_contains_mark_summary(0.67),
+                $this->get_contains_mark_summary(4.00),
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_incorrect_expectation(),
                 $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Finish the attempt.
         $this->quba->finish_all_questions();
 
         // Verify.
         $this->check_current_state(question_state::$gradedwrong);
-        $this->check_current_mark(0.66666667);
+        $this->check_current_mark(4.00);
         $this->check_current_output(
-                $this->get_contains_mark_summary(0.67),
+                $this->get_contains_mark_summary(4.00),
                 $this->get_contains_submit_button_expectation(false),
                 $this->get_contains_incorrect_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
@@ -380,6 +402,7 @@ public function test_adaptive_shortanswer_invalid_after_complete() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_incorrect_expectation(),
                 $this->get_contains_penalty_info_expectation(0.33),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Submit a correct answer.
@@ -393,6 +416,7 @@ public function test_adaptive_shortanswer_invalid_after_complete() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_correct_expectation(),
                 $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Submit an empty answer.
@@ -405,6 +429,7 @@ public function test_adaptive_shortanswer_invalid_after_complete() {
                 $this->get_contains_mark_summary(0.67),
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_contains_validation_error_expectation());
 
         // Submit another wrong answer.
@@ -418,6 +443,7 @@ public function test_adaptive_shortanswer_invalid_after_complete() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_incorrect_expectation(),
                 $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Finish the attempt.
@@ -433,6 +459,63 @@ public function test_adaptive_shortanswer_invalid_after_complete() {
                 $this->get_does_not_contain_validation_error_expectation());
     }
 
+    public function test_adaptive_shortanswer_zero_penalty() {
+
+        // Create a short answer question
+        $sa = test_question_maker::make_a_shortanswer_question();
+        // Disable penalties for this question
+        $sa->penalty = 0;
+        $this->start_attempt_at_question($sa, 'adaptive');
+
+        // Check the initial state.
+        $this->check_current_state(question_state::$todo);
+        $this->check_current_mark(null);
+        $this->check_current_output(
+                $this->get_contains_marked_out_of_summary(),
+                $this->get_contains_submit_button_expectation(true),
+                $this->get_does_not_contain_feedback_expectation());
+
+        // Submit a wrong answer.
+        $this->process_submission(array('-submit' => 1, 'answer' => 'hippopotamus'));
+
+        // Verify.
+        $this->check_current_state(question_state::$todo);
+        $this->check_current_mark(0);
+        $this->check_current_output(
+                $this->get_contains_mark_summary(0),
+                $this->get_contains_submit_button_expectation(true),
+                $this->get_contains_incorrect_expectation(),
+                $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
+                $this->get_does_not_contain_validation_error_expectation());
+
+        // Submit a correct answer.
+        $this->process_submission(array('-submit' => 1, 'answer' => 'frog'));
+
+        // Verify.
+        $this->check_current_state(question_state::$complete);
+        $this->check_current_mark(1.0);
+        $this->check_current_output(
+                $this->get_contains_mark_summary(1.0),
+                $this->get_contains_submit_button_expectation(true),
+                $this->get_contains_correct_expectation(),
+                $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
+                $this->get_does_not_contain_validation_error_expectation());
+
+        // Finish the attempt.
+        $this->quba->finish_all_questions();
+
+        // Verify.
+        $this->check_current_state(question_state::$gradedright);
+        $this->check_current_mark(1.0);
+        $this->check_current_output(
+                $this->get_contains_mark_summary(1.0),
+                $this->get_contains_submit_button_expectation(false),
+                $this->get_contains_correct_expectation(),
+                $this->get_does_not_contain_validation_error_expectation());
+    }
+
     public function test_adaptive_shortanswer_try_to_submit_blank() {
 
         // Create a short answer question with correct answer true.
@@ -458,6 +541,7 @@ public function test_adaptive_shortanswer_try_to_submit_blank() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_does_not_contain_correctness_expectation(),
                 $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_contains_validation_error_expectation());
         $this->assertNull($this->quba->get_response_summary($this->slot));
 
@@ -472,6 +556,7 @@ public function test_adaptive_shortanswer_try_to_submit_blank() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_partcorrect_expectation(),
                 $this->get_contains_penalty_info_expectation(0.33),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Now submit blank again.
@@ -485,6 +570,7 @@ public function test_adaptive_shortanswer_try_to_submit_blank() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_partcorrect_expectation(),
                 $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_contains_validation_error_expectation());
     }
 
@@ -513,6 +599,7 @@ public function test_adaptive_numerical() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_correct_expectation(),
                 $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Submit an incorrect answer.
@@ -526,6 +613,7 @@ public function test_adaptive_numerical() {
                 $this->get_contains_submit_button_expectation(true),
                 $this->get_contains_incorrect_expectation(),
                 $this->get_does_not_contain_penalty_info_expectation(),
+                $this->get_does_not_contain_total_penalty_expectation(),
                 $this->get_does_not_contain_validation_error_expectation());
 
         // Finish the attempt.