From 1ed6318d1331d552e89e9e5cbc396adeacdc57ed Mon Sep 17 00:00:00 2001
From: Julien Boulen <julien.boulen@univ-rennes2.fr>
Date: Wed, 27 Mar 2024 13:13:21 +0100
Subject: [PATCH] MDL-76317 questions: improve error message when importing
 invalid grades

---
 lang/en/question.php                          |  1 +
 question/format.php                           |  4 +-
 .../tests/fixtures/error_invalid_grades.xml   | 63 +++++++++++++++++++
 .../tests/qformat_xml_import_export_test.php  | 25 ++++++++
 4 files changed, 91 insertions(+), 2 deletions(-)
 create mode 100644 question/format/xml/tests/fixtures/error_invalid_grades.xml

diff --git a/lang/en/question.php b/lang/en/question.php
index 70f91188fb3..90610290c31 100644
--- a/lang/en/question.php
+++ b/lang/en/question.php
@@ -218,6 +218,7 @@ $string['invalidcategoryidtomove'] = 'Invalid category id to move!';
 $string['invalidconfirm'] = 'Confirmation string was incorrect';
 $string['invalidcontextinhasanyquestions'] = 'Invalid context passed to question_context_has_any_questions.';
 $string['invalidgrade'] = 'Grades ({$a}) do not match grade options - question skipped.';
+$string['invalidgradequestion'] = 'Grades ({$a->grades}) do not match grade options - question \'{$a->question}\' skipped.';
 $string['invalidpenalty'] = 'Invalid penalty';
 $string['invalidwizardpage'] = 'Incorrect or no wizard page specified!';
 $string['lastmodifiedby'] = 'Last modified by';
diff --git a/question/format.php b/question/format.php
index 7f4a118b51e..c8fc1fd5b21 100644
--- a/question/format.php
+++ b/question/format.php
@@ -388,8 +388,8 @@ class qformat_default {
                     }
                 }
                 if ($invalidfractions) {
-                    echo $OUTPUT->notification(get_string('invalidgrade', 'question',
-                            implode(', ', $invalidfractions)));
+                    $a = ['grades' => implode(', ', $invalidfractions), 'question' => $question->name];
+                    echo $OUTPUT->notification(get_string('invalidgradequestion', 'question', $a));
                     ++$gradeerrors;
                     continue;
                 } else {
diff --git a/question/format/xml/tests/fixtures/error_invalid_grades.xml b/question/format/xml/tests/fixtures/error_invalid_grades.xml
new file mode 100644
index 00000000000..ccf3406df05
--- /dev/null
+++ b/question/format/xml/tests/fixtures/error_invalid_grades.xml
@@ -0,0 +1,63 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<quiz>
+<!-- question: 1  -->
+  <question type="multichoice">
+    <name>
+      <text><![CDATA[Question with <strong>invalid grades</strong> : x > 1 & x < 2]]></text>
+    </name>
+    <questiontext format="html">
+      <text><![CDATA[Select all prime numbers :<br>]]></text>
+    </questiontext>
+    <generalfeedback format="html">
+      <text></text>
+    </generalfeedback>
+    <defaultgrade>1.0000000</defaultgrade>
+    <penalty>0.3333333</penalty>
+    <hidden>0</hidden>
+    <idnumber></idnumber>
+    <single>false</single>
+    <shuffleanswers>true</shuffleanswers>
+    <answernumbering>abc</answernumbering>
+    <showstandardinstruction>0</showstandardinstruction>
+    <correctfeedback format="html">
+      <text>Your answer is correct.</text>
+    </correctfeedback>
+    <partiallycorrectfeedback format="html">
+      <text>Your answer is partially correct.</text>
+    </partiallycorrectfeedback>
+    <incorrectfeedback format="html">
+      <text>Your answer is incorrect.</text>
+    </incorrectfeedback>
+    <shownumcorrect/>
+    <answer fraction="-50" format="html">
+      <text><![CDATA[<p dir="ltr" style="text-align: left;">1<br></p>]]></text>
+      <feedback format="html">
+        <text></text>
+      </feedback>
+    </answer>
+    <answer fraction="33" format="html"> <!-- Invalid grade. -->
+      <text><![CDATA[<p dir="ltr" style="text-align: left;">2<br></p>]]></text>
+      <feedback format="html">
+        <text></text>
+      </feedback>
+    </answer>
+    <answer fraction="33.33333" format="html">
+      <text><![CDATA[<p dir="ltr" style="text-align: left;">3<br></p>]]></text>
+      <feedback format="html">
+        <text></text>
+      </feedback>
+    </answer>
+    <answer fraction="-50" format="html">
+      <text><![CDATA[<p dir="ltr" style="text-align: left;">4<br></p>]]></text>
+      <feedback format="html">
+        <text></text>
+      </feedback>
+    </answer>
+    <answer fraction="33.33333" format="html">
+      <text><![CDATA[<p dir="ltr" style="text-align: left;">5<br></p>]]></text>
+      <feedback format="html">
+        <text></text>
+      </feedback>
+    </answer>
+  </question>
+</quiz>
diff --git a/question/format/xml/tests/qformat_xml_import_export_test.php b/question/format/xml/tests/qformat_xml_import_export_test.php
index f8d671c3348..975770ff4e6 100644
--- a/question/format/xml/tests/qformat_xml_import_export_test.php
+++ b/question/format/xml/tests/qformat_xml_import_export_test.php
@@ -235,6 +235,31 @@ class qformat_xml_import_export_test extends advanced_testcase {
         $this->assert_category_has_parent('Tau', 'Sigma');
     }
 
+    /**
+     * Check exception when importing questions with invalid grades.
+     *
+     * @covers \qformat_default::importprocess
+     */
+    public function test_import_invalid_grades(): void {
+        global $OUTPUT;
+
+        $this->resetAfterTest(true);
+        $course = $this->getDataGenerator()->create_course();
+        $this->setAdminUser();
+        $qformat = $this->create_qformat('error_invalid_grades.xml', $course);
+
+        ob_start();
+        $imported = $qformat->importprocess();
+        $output = ob_get_clean();
+
+        $a = ['grades' => '0.33', 'question' => 'Question with invalid grades : x > 1 & x < 2'];
+        $expectedoutput = $OUTPUT->notification(get_string('invalidgradequestion', 'question', $a));
+        $expectedoutput .= $OUTPUT->notification(get_string('importparseerror', 'question'));
+
+        $this->assertFalse($imported);
+        $this->assertEquals($expectedoutput, $output);
+    }
+
     /**
      * Simple check for exporting a category.
      */