From 1d1b5908db2cd658a7659280ee497decb5630006 Mon Sep 17 00:00:00 2001
From: Tomo Tsuyuki <tomotsuyuki@catalyst-au.net>
Date: Tue, 18 Jul 2023 11:52:26 +1000
Subject: [PATCH] MDL-77378 qtype_multianswer: Fix division by zero errors

---
 question/type/multianswer/question.php        |  9 ++++
 question/type/multianswer/tests/helper.php    | 46 ++++++++++++++++++-
 .../type/multianswer/tests/question_test.php  | 19 ++++++++
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/question/type/multianswer/question.php b/question/type/multianswer/question.php
index 62036c263a4..a52eec83b05 100644
--- a/question/type/multianswer/question.php
+++ b/question/type/multianswer/question.php
@@ -147,6 +147,9 @@ class qtype_multianswer_question extends question_graded_automatically_with_coun
             $fractionmax += $subq->defaultmark;
             $fractionsum += $subq->defaultmark * $subq->get_min_fraction();
         }
+        if (empty($fractionsum)) {
+            return 0;
+        }
         return $fractionsum / (!empty($this->subquestions) ? $fractionmax : 1);
     }
 
@@ -157,6 +160,9 @@ class qtype_multianswer_question extends question_graded_automatically_with_coun
             $fractionmax += $subq->defaultmark;
             $fractionsum += $subq->defaultmark * $subq->get_max_fraction();
         }
+        if (empty($fractionsum)) {
+            return 1;
+        }
         return $fractionsum / (!empty($this->subquestions) ? $fractionmax : 1);
     }
 
@@ -299,6 +305,9 @@ class qtype_multianswer_question extends question_graded_automatically_with_coun
                 $overallstate = $this->combine_states($overallstate, $newstate);
             }
         }
+        if (empty($fractionmax)) {
+            return array(null, $overallstate ?? question_state::$finished);
+        }
         return array($fractionsum / $fractionmax, $overallstate);
     }
 
diff --git a/question/type/multianswer/tests/helper.php b/question/type/multianswer/tests/helper.php
index c3f6ba80503..15c3a3a4d9d 100644
--- a/question/type/multianswer/tests/helper.php
+++ b/question/type/multianswer/tests/helper.php
@@ -37,7 +37,7 @@ require_once($CFG->dirroot . '/question/type/multianswer/question.php');
  */
 class qtype_multianswer_test_helper extends question_test_helper {
     public function get_test_questions() {
-        return array('twosubq', 'fourmc', 'numericalzero', 'dollarsigns', 'multiple');
+        return array('twosubq', 'fourmc', 'numericalzero', 'dollarsigns', 'multiple', 'zeroweight');
     }
 
     /**
@@ -484,4 +484,48 @@ class qtype_multianswer_test_helper extends question_test_helper {
         return $q;
     }
 
+    /**
+     * Makes a multianswer question with zero weight.
+     * This is used for testing the MDL-77378 bug.
+     * @return qtype_multianswer_question
+     */
+    public function make_multianswer_question_zeroweight() {
+        question_bank::load_question_definition_classes('multianswer');
+        $q = new qtype_multianswer_question();
+        test_question_maker::initialise_a_question($q);
+        $q->name = 'Zero weight';
+        $q->questiontext =
+            'Optional question: {#1}.';
+        $q->generalfeedback = '';
+        $q->qtype = question_bank::get_qtype('multianswer');
+        $q->textfragments = array(
+            'Optional question: ',
+            '.',
+        );
+        $q->places = array('1' => '1');
+
+        // Shortanswer subquestion.
+        question_bank::load_question_definition_classes('shortanswer');
+        $sa = new qtype_shortanswer_question();
+        test_question_maker::initialise_a_question($sa);
+        $sa->name = 'Zero weight';
+        $sa->questiontext = '{0:SHORTANSWER:~%0%Input box~%100%*}';
+        $sa->questiontextformat = FORMAT_HTML;
+        $sa->generalfeedback = '';
+        $sa->generalfeedbackformat = FORMAT_HTML;
+        $sa->usecase = true;
+        $sa->answers = array(
+            13 => new question_answer(13, 'Input box', 0.0, '', FORMAT_HTML),
+            14 => new question_answer(14, '*', 1.0, '', FORMAT_HTML),
+        );
+        $sa->qtype = question_bank::get_qtype('shortanswer');
+        $sa->defaultmark = 0;
+
+        $q->subquestions = array(
+            1 => $sa,
+        );
+
+        return $q;
+    }
+
 }
diff --git a/question/type/multianswer/tests/question_test.php b/question/type/multianswer/tests/question_test.php
index 407d8b97487..5d0beed7a8f 100644
--- a/question/type/multianswer/tests/question_test.php
+++ b/question/type/multianswer/tests/question_test.php
@@ -350,4 +350,23 @@ class question_test extends \advanced_testcase {
                 $newquestion->update_attempt_state_data_for_new_version($oldstep, $question));
     }
 
+    /**
+     * Test functions work with zero weight.
+     * This is used for testing the MDL-77378 bug.
+     */
+    public function test_zeroweight() {
+        $this->resetAfterTest();
+        /** @var \qtype_multianswer_question $question */
+        $question = \test_question_maker::make_question('multianswer', 'zeroweight');
+        $question->start_attempt(new question_attempt_step(), 1);
+
+        $this->assertEquals([null, question_state::$gradedright], $question->grade_response(
+            ['sub1_answer' => 'Something']));
+        $this->assertEquals([null, question_state::$gradedwrong], $question->grade_response(
+            ['sub1_answer' => 'Input box']));
+
+        $this->assertEquals(1, $question->get_max_fraction());
+        $this->assertEquals(0, $question->get_min_fraction());
+    }
+
 }