MDL-77656 quiz: fix web services to handle custom question numbers

mod/quiz/classes/external.php

@@ -911,6 +911,10 @@ private static function question_structure() {
                 'slot' => new external_value(PARAM_INT, 'slot number'),
                 'type' => new external_value(PARAM_ALPHANUMEXT, 'question type, i.e: multichoice'),
                 'page' => new external_value(PARAM_INT, 'page of the quiz this question appears on'),
+                'questionnumber' => new external_value(PARAM_RAW,
+                        'The question number to display for this question, e.g. "7", "i" or "Custom-B)".'),
+                'number' => new external_value(PARAM_INT,
+                        'DO NOT USE. Use questionnumber. Only retained for backwards compatibility.', VALUE_OPTIONAL),
                 'html' => new external_value(PARAM_RAW, 'the question rendered'),
                 'responsefileareas' => new external_multiple_structure(
                     new external_single_structure(
@@ -926,7 +930,6 @@ private static function question_structure() {
                 'hasautosavedstep' => new external_value(PARAM_BOOL, 'whether this question attempt has autosaved data',
                                                             VALUE_OPTIONAL),
                 'flagged' => new external_value(PARAM_BOOL, 'whether the question is flagged or not'),
-                'number' => new external_value(PARAM_INT, 'question ordering number in the quiz', VALUE_OPTIONAL),
                 'state' => new external_value(PARAM_ALPHA, 'the state where the question is in.
                     It will not be returned if the user cannot see it due to the quiz display correctness settings.',
                     VALUE_OPTIONAL),
@@ -955,7 +958,6 @@ private static function get_attempt_questions_data(quiz_attempt $attemptobj, $re
         global $PAGE;
 
         $questions = [];
-        $contextid = $attemptobj->get_quizobj()->get_context()->id;
         $displayoptions = $attemptobj->get_display_options($review);
         $renderer = $PAGE->get_renderer('mod_quiz');
         $contextid = $attemptobj->get_quizobj()->get_context()->id;
@@ -992,6 +994,7 @@ private static function get_attempt_questions_data(quiz_attempt $attemptobj, $re
                 'slot' => $slot,
                 'type' => $qtype,
                 'page' => $attemptobj->get_question_page($slot),
+                'questionnumber' => $attemptobj->get_question_number($slot),
                 'flagged' => $attemptobj->is_question_flagged($slot),
                 'html' => $attemptobj->render_question($slot, $review, $renderer) . $PAGE->requires->get_end_code(),
                 'responsefileareas' => $responsefileareas,
@@ -1001,8 +1004,11 @@ private static function get_attempt_questions_data(quiz_attempt $attemptobj, $re
                 'settings' => !empty($settings) ? json_encode($settings) : null,
             ];
 
+            if ($question['questionnumber'] === (string) (int) $question['questionnumber']) {
+                $question['number'] = $question['questionnumber'];
+            }
+
             if ($attemptobj->is_real_question($slot)) {
-                $question['number'] = $attemptobj->get_question_number($slot);
                 $showcorrectness = $displayoptions->correctness && $qattempt->has_marks();
                 if ($showcorrectness) {
                     $question['state'] = (string) $attemptobj->get_question_state($slot);
@@ -1067,7 +1073,7 @@ public static function get_attempt_data($attemptid, $page, $preflightdata = [])
         ];
         $params = self::validate_parameters(self::get_attempt_data_parameters(), $params);
 
-        list($attemptobj, $messages) = self::validate_attempt($params);
+        [$attemptobj, $messages] = self::validate_attempt($params);
 
         if ($attemptobj->is_last_page($params['page'])) {
             $nextpage = -1;

mod/quiz/classes/quiz_attempt.php

@@ -258,7 +258,7 @@ protected function number_questions() {
                     if (!empty($this->slots[$slot]->displaynumber) && !is_null($this->slots[$slot]->displaynumber)) {
                         $this->questionnumbers[$slot] = $this->slots[$slot]->displaynumber;
                     } else {
-                        $this->questionnumbers[$slot] = $number;
+                        $this->questionnumbers[$slot] = (string) $number;
                     }
                     $number += $length;
                 } else {
@@ -909,7 +909,7 @@ public function get_original_slot($slot) {
      * @return string the displayed question number for the question in this slot.
      *      For example '1', '2', '3' or 'i'.
      */
-    public function get_question_number($slot) {
+    public function get_question_number($slot): string {
         return $this->questionnumbers[$slot];
     }
 

mod/quiz/tests/external/external_test.php

@@ -31,6 +31,7 @@
 use mod_quiz\question\display_options;
 use mod_quiz\quiz_attempt;
 use mod_quiz\quiz_settings;
+use mod_quiz\structure;
 use mod_quiz_external;
 use moodle_exception;
 
@@ -1050,11 +1051,16 @@ public function test_get_attempt_data() {
 
         $timenow = time();
         // Create a new quiz with one attempt started.
-        list($quiz, $context, $quizobj, $attempt, $attemptobj) = $this->create_quiz_with_questions(true);
+        [$quiz, , $quizobj, $attempt] = $this->create_quiz_with_questions(true);
+        /** @var structure $structure */
+        $structure = $quizobj->get_structure();
+        $structure->update_slot_display_number($structure->get_slot_id_for_slot(1), '1.a');
 
         // Set correctness mask so questions state can be fetched only after finishing the attempt.
         $DB->set_field('quiz', 'reviewcorrectness', display_options::IMMEDIATELY_AFTER, ['id' => $quiz->id]);
 
+        // Having changed some settings, recreate the objects.
+        $attemptobj = quiz_attempt::create($attempt->id);
         $quizobj = $attemptobj->get_quizobj();
         $quizobj->preload_questions();
         $quizobj->load_questions();
@@ -1071,7 +1077,8 @@ public function test_get_attempt_data() {
         $this->assertCount(0, $result['messages']);
         $this->assertCount(1, $result['questions']);
         $this->assertEquals(1, $result['questions'][0]['slot']);
-        $this->assertEquals(1, $result['questions'][0]['number']);
+        $this->assertArrayNotHasKey('number', $result['questions'][0]);
+        $this->assertEquals('1.a', $result['questions'][0]['questionnumber']);
         $this->assertEquals('numerical', $result['questions'][0]['type']);
         $this->assertArrayNotHasKey('state', $result['questions'][0]);  // We don't receive the state yet.
         $this->assertEquals(get_string('notyetanswered', 'question'), $result['questions'][0]['status']);
@@ -1092,6 +1099,7 @@ public function test_get_attempt_data() {
         $this->assertCount(0, $result['messages']);
         $this->assertCount(1, $result['questions']);
         $this->assertEquals(2, $result['questions'][0]['slot']);
+        $this->assertEquals(2, $result['questions'][0]['questionnumber']);
         $this->assertEquals(2, $result['questions'][0]['number']);
         $this->assertEquals('numerical', $result['questions'][0]['type']);
         $this->assertArrayNotHasKey('state', $result['questions'][0]);  // We don't receive the state yet.

mod/quiz/upgrade.txt

@@ -2,6 +2,12 @@ This files describes API changes in the quiz code.
 
 === 4.2 ===
 
+* For the three quiz web services: mod_quiz_get_attempt_data, mod_quiz_get_attempt_summary and
+  mod_quiz_get_attempt_review, the ->number property of each question is now deprecated
+  (It had the wrong time and was documented wrongly.) In the future, please use the new property
+  ->questionnumber. Note, this question 'number' can include any string (e.g. 'Qs 1 & 2') so it is
+  important to applying htmlspecialchars, or equivalent, to the value if you are outputting to HTML.
+
 * The methods in the quiz_settings class which return a URL now all return a moodle_url. Previously
   some returns a moodle_url and others aa string.