From 30598cf2a05593a5435e0844379452f2df9be0f0 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Tue, 28 Nov 2023 10:04:11 +0100 Subject: [PATCH 1/3] MDL-79295 quiz: Add proper WS support for sequential nav. --- mod/quiz/classes/external.php | 64 ++++++++++++++--------- mod/quiz/tests/external/external_test.php | 12 +++-- 2 files changed, 46 insertions(+), 30 deletions(-) diff --git a/mod/quiz/classes/external.php b/mod/quiz/classes/external.php index 4cc7a7aa7a9..5c3b4098756 100644 --- a/mod/quiz/classes/external.php +++ b/mod/quiz/classes/external.php @@ -968,41 +968,18 @@ class mod_quiz_external extends external_api { $qattempt = $attemptobj->get_question_attempt($slot); $questiondef = $qattempt->get_question(true); - // Get response files (for questions like essay that allows attachments). - $responsefileareas = []; - foreach (question_bank::get_qtype($qtype)->response_file_areas() as $area) { - if ($files = $attemptobj->get_question_attempt($slot)->get_last_qt_files($area, $contextid)) { - $responsefileareas[$area]['area'] = $area; - $responsefileareas[$area]['files'] = []; - - foreach ($files as $file) { - $responsefileareas[$area]['files'][] = [ - 'filename' => $file->get_filename(), - 'fileurl' => $qattempt->get_response_file_url($file), - 'filesize' => $file->get_filesize(), - 'filepath' => $file->get_filepath(), - 'mimetype' => $file->get_mimetype(), - 'timemodified' => $file->get_timemodified(), - ]; - } - } - } - // Check display settings for question. $settings = $questiondef->get_question_definition_for_external_rendering($qattempt, $displayoptions); + // Navigation information. $question = [ '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, 'sequencecheck' => $qattempt->get_sequence_check_count(), 'lastactiontime' => $qattempt->get_last_step()->get_timecreated(), 'hasautosavedstep' => $qattempt->has_autosaved_step(), - 'settings' => !empty($settings) ? json_encode($settings) : null, ]; if ($question['questionnumber'] === (string) (int) $question['questionnumber']) { @@ -1023,9 +1000,44 @@ class mod_quiz_external extends external_api { if ($displayoptions->marks >= question_display_options::MARK_AND_MAX) { $question['mark'] = $attemptobj->get_question_mark($slot); } - if ($attemptobj->check_page_access($attemptobj->get_question_page($slot), false)) { - $questions[] = $question; + + // Check access. This is needed especially when sequential navigation is enforced. To prevent the student see "future" questions. + $haveaccess = $attemptobj->check_page_access($attemptobj->get_question_page($slot), false); + if (!$haveaccess) { + $question['type'] = ''; + $question['html'] = ''; } + + // For visited pages/questions it is ok to keep data the user already saw. + $questionalreadyseen = $attemptobj->get_currentpage() >= $attemptobj->get_question_page($slot); + + // Information when only the user has access to the question at any moment (free navigation) or already seen. + if ($haveaccess || $questionalreadyseen) { + // Get response files (for questions like essay that allows attachments). + $responsefileareas = []; + foreach (question_bank::get_qtype($qtype)->response_file_areas() as $area) { + if ($files = $attemptobj->get_question_attempt($slot)->get_last_qt_files($area, $contextid)) { + $responsefileareas[$area]['area'] = $area; + $responsefileareas[$area]['files'] = []; + + foreach ($files as $file) { + $responsefileareas[$area]['files'][] = [ + 'filename' => $file->get_filename(), + 'fileurl' => $qattempt->get_response_file_url($file), + 'filesize' => $file->get_filesize(), + 'filepath' => $file->get_filepath(), + 'mimetype' => $file->get_mimetype(), + 'timemodified' => $file->get_timemodified(), + ]; + } + } + } + $question['type'] = $qtype; + $question['html'] = $attemptobj->render_question($slot, $review, $renderer) . $PAGE->requires->get_end_code(); + $question['responsefileareas'] = $responsefileareas; + $question['settings'] = !empty($settings) ? json_encode($settings) : null; + } + $questions[] = $question; } return $questions; } diff --git a/mod/quiz/tests/external/external_test.php b/mod/quiz/tests/external/external_test.php index d3796d5fbf7..5718aec9c85 100644 --- a/mod/quiz/tests/external/external_test.php +++ b/mod/quiz/tests/external/external_test.php @@ -2074,17 +2074,20 @@ class external_test extends externallib_advanced_testcase { } /** - * Test that a sequential navigation quiz is not allowing to see questions in advance for a student + * Test that a sequential navigation quiz is not allowing to see questions content in advance for a student. */ public function test_sequential_navigation_attempt_summary() { // Test user with full capabilities. $quiz = $this->prepare_sequential_quiz(); $attemptobj = $this->create_quiz_attempt_object($quiz); $this->setUser($this->student); - // Check that we do not return other questions than the one currently viewed. + // Check that we do not return content from other questions except than the ones currently viewed. $result = mod_quiz_external::get_attempt_summary($attemptobj->get_attemptid()); - $this->assertCount(1, $result['questions']); - $this->assertStringContainsString('Question (1)', $result['questions'][0]['html']); + $this->assertStringContainsString('Question (1)', $result['questions'][0]['html']); // Current question. + $this->assertEmpty($result['questions'][1]['html']); // Next question. + $this->assertEmpty($result['questions'][2]['html']); // And more. + $this->assertEmpty($result['questions'][3]['html']); // And more. + $this->assertEmpty($result['questions'][4]['html']); // And more. } /** @@ -2137,6 +2140,7 @@ class external_test extends externallib_advanced_testcase { $data = [ 'course' => $this->course->id, 'sumgrades' => 2, + 'questionsperpage' => 1, 'preferredbehaviour' => 'deferredfeedback', 'navmethod' => QUIZ_NAVMETHOD_SEQ ]; From d54086f838c985cd60b6a67e2f91eaaec5664950 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Tue, 28 Nov 2023 10:08:31 +0100 Subject: [PATCH 2/3] MDL-79295 quiz: Return state class for questions via WS --- mod/quiz/classes/external.php | 9 +++++++-- mod/quiz/tests/external/external_test.php | 12 ++++++++++++ mod/quiz/upgrade.txt | 3 +++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/mod/quiz/classes/external.php b/mod/quiz/classes/external.php index 5c3b4098756..9a6ea9ae429 100644 --- a/mod/quiz/classes/external.php +++ b/mod/quiz/classes/external.php @@ -931,10 +931,13 @@ class mod_quiz_external extends external_api { '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'), - 'state' => new external_value(PARAM_ALPHA, 'the state where the question is in. + 'state' => new external_value(PARAM_ALPHA, 'the state where the question is in terms of correctness. It will not be returned if the user cannot see it due to the quiz display correctness settings.', VALUE_OPTIONAL), - 'status' => new external_value(PARAM_RAW, 'current formatted state of the question', VALUE_OPTIONAL), + 'stateclass' => new external_value(PARAM_NOTAGS, + 'A machine-readable class name for the state that this question attempt is in, as returned by question_usage_by_activity::get_question_state_class(). + Always returned.', VALUE_OPTIONAL), + 'status' => new external_value(PARAM_RAW, 'Human readable state of the question.', VALUE_OPTIONAL), 'blockedbyprevious' => new external_value(PARAM_BOOL, 'whether the question is blocked by the previous question', VALUE_OPTIONAL), 'mark' => new external_value(PARAM_RAW, 'the mark awarded. @@ -991,6 +994,8 @@ class mod_quiz_external extends external_api { if ($showcorrectness) { $question['state'] = (string) $attemptobj->get_question_state($slot); } + // The stateclass is used for CSS classes but also for the lang strings. + $question['stateclass'] = $attemptobj->get_question_state_class($slot, $displayoptions->correctness); $question['status'] = $attemptobj->get_question_status($slot, $displayoptions->correctness); $question['blockedbyprevious'] = $attemptobj->is_blocked_by_previous_question($slot); } diff --git a/mod/quiz/tests/external/external_test.php b/mod/quiz/tests/external/external_test.php index 5718aec9c85..f4768067858 100644 --- a/mod/quiz/tests/external/external_test.php +++ b/mod/quiz/tests/external/external_test.php @@ -1084,7 +1084,9 @@ class external_test extends externallib_advanced_testcase { $this->assertArrayNotHasKey('number', $result['questions'][0]); $this->assertEquals('1.a', $result['questions'][0]['questionnumber']); $this->assertEquals('numerical', $result['questions'][0]['type']); + $this->assertEquals('notyetanswered', $result['questions'][0]['stateclass']); $this->assertArrayNotHasKey('state', $result['questions'][0]); // We don't receive the state yet. + $this->assertEquals('notyetanswered', $result['questions'][0]['stateclass']); $this->assertEquals(get_string('notyetanswered', 'question'), $result['questions'][0]['status']); $this->assertFalse($result['questions'][0]['flagged']); $this->assertEquals(0, $result['questions'][0]['page']); @@ -1106,6 +1108,7 @@ class external_test extends externallib_advanced_testcase { $this->assertEquals(2, $result['questions'][0]['questionnumber']); $this->assertEquals(2, $result['questions'][0]['number']); $this->assertEquals('numerical', $result['questions'][0]['type']); + $this->assertEquals('notyetanswered', $result['questions'][0]['stateclass']); $this->assertArrayNotHasKey('state', $result['questions'][0]); // We don't receive the state yet. $this->assertEquals(get_string('notyetanswered', 'question'), $result['questions'][0]['status']); $this->assertFalse($result['questions'][0]['flagged']); @@ -1120,6 +1123,7 @@ class external_test extends externallib_advanced_testcase { // Now we should receive the question state. $result = mod_quiz_external::get_attempt_review($attempt->id, 1); $result = external_api::clean_returnvalue(mod_quiz_external::get_attempt_review_returns(), $result); + $this->assertEquals('notanswered', $result['questions'][0]['stateclass']); $this->assertEquals('gaveup', $result['questions'][0]['state']); // Change setting and expect two pages. @@ -1216,7 +1220,9 @@ class external_test extends externallib_advanced_testcase { // Check the state, flagged and mark data is correct. $this->assertEquals('todo', $result['questions'][0]['state']); + $this->assertEquals('notyetanswered', $result['questions'][0]['stateclass']); $this->assertEquals('todo', $result['questions'][1]['state']); + $this->assertEquals('notyetanswered', $result['questions'][1]['stateclass']); $this->assertEquals(1, $result['questions'][0]['number']); $this->assertEquals(2, $result['questions'][1]['number']); $this->assertFalse($result['questions'][0]['flagged']); @@ -1243,7 +1249,9 @@ class external_test extends externallib_advanced_testcase { // Check it's marked as completed only the first one. $this->assertEquals('complete', $result['questions'][0]['state']); + $this->assertEquals('answersaved', $result['questions'][0]['stateclass']); $this->assertEquals('todo', $result['questions'][1]['state']); + $this->assertEquals('notyetanswered', $result['questions'][1]['stateclass']); $this->assertEquals(1, $result['questions'][0]['number']); $this->assertEquals(2, $result['questions'][1]['number']); $this->assertFalse($result['questions'][0]['flagged']); @@ -1289,7 +1297,9 @@ class external_test extends externallib_advanced_testcase { // Check it's marked as completed only the first one. $this->assertEquals('complete', $result['questions'][0]['state']); + $this->assertEquals('answersaved', $result['questions'][0]['stateclass']); $this->assertEquals('todo', $result['questions'][1]['state']); + $this->assertEquals('notyetanswered', $result['questions'][1]['stateclass']); $this->assertEquals(1, $result['questions'][0]['number']); $this->assertEquals(2, $result['questions'][1]['number']); $this->assertFalse($result['questions'][0]['flagged']); @@ -1322,8 +1332,10 @@ class external_test extends externallib_advanced_testcase { // Check it's marked as completed only the first one. $this->assertEquals('complete', $result['questions'][0]['state']); + $this->assertEquals('answersaved', $result['questions'][0]['stateclass']); $this->assertEquals(1, $result['questions'][0]['sequencecheck']); $this->assertEquals('complete', $result['questions'][1]['state']); + $this->assertEquals('answersaved', $result['questions'][1]['stateclass']); $this->assertEquals(1, $result['questions'][1]['sequencecheck']); } diff --git a/mod/quiz/upgrade.txt b/mod/quiz/upgrade.txt index ba58d8552ec..b9bb2ed5e0b 100644 --- a/mod/quiz/upgrade.txt +++ b/mod/quiz/upgrade.txt @@ -4,6 +4,9 @@ This files describes API changes in the quiz code. * A quiz_structure_modified callback has been added for quiz_ plugins, called from grade_calculator::recompute_quiz_sumgrades(). Plugins can implement this by creating a `quiz_structure_modified` class in their namespace with a static `callback` method, see quiz_statistics as an example. +* External functions returning question information, currently get_attempt_summary, get_attempt_data and get_attemp_review + now return a new field called "stateclass". A machine-readable class name for the state that this question attempt is in, + as returned by question_usage_by_activity::get_question_state_class(). === 4.3 === From 9c38e2882699f9a6fcd42956fcc93813af9feb77 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Tue, 28 Nov 2023 10:14:46 +0100 Subject: [PATCH 3/3] MDL-79295 quiz: Return total unasnwered questions via summary WS --- mod/quiz/classes/external.php | 7 +++++++ mod/quiz/tests/external/external_test.php | 4 +++- mod/quiz/upgrade.txt | 4 +++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/mod/quiz/classes/external.php b/mod/quiz/classes/external.php index 9a6ea9ae429..24bfbf352f7 100644 --- a/mod/quiz/classes/external.php +++ b/mod/quiz/classes/external.php @@ -1180,6 +1180,12 @@ class mod_quiz_external extends external_api { $result['warnings'] = $warnings; $result['questions'] = self::get_attempt_questions_data($attemptobj, false, 'all'); + if ($attemptobj->get_state() == quiz_attempt::IN_PROGRESS && $attemptobj->get_quiz()->navmethod == 'free') { + // Only count the unanswered question if the navigation method is set to free. + $result['totalunanswered'] = $attemptobj->get_number_of_unanswered_questions(); + } + + return $result; } @@ -1193,6 +1199,7 @@ class mod_quiz_external extends external_api { return new external_single_structure( [ 'questions' => new external_multiple_structure(self::question_structure()), + 'totalunanswered' => new external_value(PARAM_INT, 'Total unanswered questions.', VALUE_OPTIONAL), 'warnings' => new external_warnings(), ] ); diff --git a/mod/quiz/tests/external/external_test.php b/mod/quiz/tests/external/external_test.php index f4768067858..2b8ccfcd64a 100644 --- a/mod/quiz/tests/external/external_test.php +++ b/mod/quiz/tests/external/external_test.php @@ -1240,6 +1240,7 @@ class external_test extends externallib_advanced_testcase { $this->assertNotEmpty(5, $result['questions'][0]['settings']); // Check at least some settings returned. $this->assertCount(4, (array) json_decode($result['questions'][0]['settings'])); + $this->assertEquals(2, $result['totalunanswered']); // All questions are unanswered. // Submit a response for the first question. $tosubmit = [1 => ['answer' => '3.14']]; @@ -1264,7 +1265,7 @@ class external_test extends externallib_advanced_testcase { $this->assertGreaterThanOrEqual($timenow, $result['questions'][1]['lastactiontime']); $this->assertEquals(false, $result['questions'][0]['hasautosavedstep']); $this->assertEquals(false, $result['questions'][1]['hasautosavedstep']); - + $this->assertEquals(1, $result['totalunanswered']); // Only one question is unanswered. } /** @@ -2100,6 +2101,7 @@ class external_test extends externallib_advanced_testcase { $this->assertEmpty($result['questions'][2]['html']); // And more. $this->assertEmpty($result['questions'][3]['html']); // And more. $this->assertEmpty($result['questions'][4]['html']); // And more. + $this->assertNotContains('totalunanswered', $result); // For sequential quizzes, unanswered questions are not considered. } /** diff --git a/mod/quiz/upgrade.txt b/mod/quiz/upgrade.txt index b9bb2ed5e0b..891cacfa6d4 100644 --- a/mod/quiz/upgrade.txt +++ b/mod/quiz/upgrade.txt @@ -4,9 +4,11 @@ This files describes API changes in the quiz code. * A quiz_structure_modified callback has been added for quiz_ plugins, called from grade_calculator::recompute_quiz_sumgrades(). Plugins can implement this by creating a `quiz_structure_modified` class in their namespace with a static `callback` method, see quiz_statistics as an example. -* External functions returning question information, currently get_attempt_summary, get_attempt_data and get_attemp_review +* External functions returning question information, currently get_attempt_summary(), get_attempt_data() and get_attemp_review() now return a new field called "stateclass". A machine-readable class name for the state that this question attempt is in, as returned by question_usage_by_activity::get_question_state_class(). +* External function mod_quiz_external::get_attempt_summary() now returns a new field "totalunanswered", with the total number of + unanswered questions. === 4.3 ===