From caddb8f1759f97733efbdbecda692b7ae7a9511c Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 30 Sep 2020 11:27:05 +0200 Subject: [PATCH 1/2] MDL-69823 mod_quiz: Return question options via WS --- mod/quiz/classes/external.php | 8 +++++++- mod/quiz/tests/external_test.php | 5 +++++ mod/quiz/upgrade.txt | 6 ++++++ question/type/questionbase.php | 25 +++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/mod/quiz/classes/external.php b/mod/quiz/classes/external.php index 2c207bdd524..3b6a6f23192 100644 --- a/mod/quiz/classes/external.php +++ b/mod/quiz/classes/external.php @@ -902,6 +902,7 @@ class mod_quiz_external extends external_api { It will be returned only if the user is allowed to see it.', VALUE_OPTIONAL), 'maxmark' => new external_value(PARAM_FLOAT, 'the maximum mark possible for this question attempt. It will be returned only if the user is allowed to see it.', VALUE_OPTIONAL), + 'settings' => new external_value(PARAM_RAW, 'Question settings (JSON encoded).', VALUE_OPTIONAL), ), 'The question data. Some fields may not be returned depending on the quiz display settings.' ); @@ -927,6 +928,7 @@ class mod_quiz_external extends external_api { foreach ($attemptobj->get_slots($page) as $slot) { $qtype = $attemptobj->get_question_type_name($slot); $qattempt = $attemptobj->get_question_attempt($slot); + $questiondef = $qattempt->get_question(true); // Get response files (for questions like essay that allows attachments). $responsefileareas = []; @@ -948,6 +950,9 @@ class mod_quiz_external extends external_api { } } + // Check display settings for question. + $settings = $questiondef->get_question_definition_for_external_rendering($qattempt, $displayoptions); + $question = array( 'slot' => $slot, 'type' => $qtype, @@ -957,7 +962,8 @@ class mod_quiz_external extends external_api { 'responsefileareas' => $responsefileareas, 'sequencecheck' => $qattempt->get_sequence_check_count(), 'lastactiontime' => $qattempt->get_last_step()->get_timecreated(), - 'hasautosavedstep' => $qattempt->has_autosaved_step() + 'hasautosavedstep' => $qattempt->has_autosaved_step(), + 'settings' => !empty($settings) ? json_encode($settings) : null, ); if ($attemptobj->is_real_question($slot)) { diff --git a/mod/quiz/tests/external_test.php b/mod/quiz/tests/external_test.php index 0ea6a4eda6f..9d97fce8c15 100644 --- a/mod/quiz/tests/external_test.php +++ b/mod/quiz/tests/external_test.php @@ -1034,6 +1034,11 @@ class mod_quiz_external_testcase extends externallib_advanced_testcase { $this->assertEquals(false, $result['questions'][0]['hasautosavedstep']); $this->assertEquals(false, $result['questions'][1]['hasautosavedstep']); + // Check question options. + $this->assertNotEmpty(5, $result['questions'][0]['settings']); + // Check at least some settings returned. + $this->assertCount(4, (array) json_decode($result['questions'][0]['settings'])); + // Submit a response for the first question. $tosubmit = array(1 => array('answer' => '3.14')); $attemptobj->process_submitted_actions(time(), false, $tosubmit); diff --git a/mod/quiz/upgrade.txt b/mod/quiz/upgrade.txt index d3f6c09585d..3af165d4297 100644 --- a/mod/quiz/upgrade.txt +++ b/mod/quiz/upgrade.txt @@ -1,5 +1,11 @@ This files describes API changes in the quiz code. +=== 3.10.1 === + +* External functions mod_quiz_external::get_attempt_data, mod_quiz_external::get_attempt_summary + and mod_quiz_external::get_attempt_review now return a new additional optional field: + - settings: Containing the question definition settings for displaying the question in an external system. + === 3.10 === * External functions mod_quiz_external::get_attempt_data, mod_quiz_external::get_attempt_summary diff --git a/question/type/questionbase.php b/question/type/questionbase.php index 4e2ea870fae..d045f868059 100644 --- a/question/type/questionbase.php +++ b/question/type/questionbase.php @@ -427,6 +427,31 @@ abstract class question_definition { return false; } } + + /** + * Return the question settings that define this question as structured data. + * + * This is used by external systems such as the Moodle mobile app, which want to display the question themselves, + * rather than using the renderer provided. + * + * This method should only return the data that the student is allowed to see or know, given the current state of + * the question. For example, do not include the 'General feedback' until the student has completed the question, + * and even then, only include it if the question_display_options say it should be visible. + * + * But, within those rules, it is recommended that you return all the settings for the question, + * to give maximum flexibility to the external system providing its own rendering of the question. + * + * @param question_attempt $qa the current attempt for which we are exporting the settings. + * @param question_display_options $options the question display options which say which aspects of the question + * should be visible. + * @return mixed structure representing the question settings. In web services, this will be JSON-encoded. + */ + public function get_question_definition_for_external_rendering(question_attempt $qa, question_display_options $options) { + + debugging('This question does not implement the get_question_definition_for_external_rendering() method yet.', + DEBUG_DEVELOPER); + return null; + } } From 440aaccb577262f0ae89e805eaf971f18360a969 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 30 Sep 2020 11:27:29 +0200 Subject: [PATCH 2/2] MDL-69823 qtype: Support core question types --- .../type/calculated/tests/question_test.php | 19 +++++++++++++ question/type/essay/question.php | 27 +++++++++++++++++++ question/type/essay/tests/question_test.php | 23 ++++++++++++++++ question/type/gapselect/questionbase.php | 17 ++++++++++++ .../type/gapselect/tests/question_test.php | 13 +++++++++ question/type/match/question.php | 17 ++++++++++++ question/type/match/tests/question_test.php | 12 +++++++++ question/type/multianswer/question.php | 15 +++++++++++ .../type/multianswer/tests/question_test.php | 15 +++++++++++ question/type/multichoice/question.php | 20 ++++++++++++++ .../multichoice/tests/question_multi_test.php | 15 +++++++++++ question/type/numerical/question.php | 23 +++++++++++++++- question/type/numerical/questiontype.php | 1 + .../type/numerical/tests/question_test.php | 19 +++++++++++++ question/type/shortanswer/question.php | 13 +++++++++ .../type/shortanswer/tests/question_test.php | 15 +++++++++++ question/type/truefalse/question.php | 13 +++++++++ .../type/truefalse/tests/question_test.php | 15 +++++++++++ 18 files changed, 291 insertions(+), 1 deletion(-) diff --git a/question/type/calculated/tests/question_test.php b/question/type/calculated/tests/question_test.php index 924f62ee202..28a9d157f50 100644 --- a/question/type/calculated/tests/question_test.php +++ b/question/type/calculated/tests/question_test.php @@ -178,4 +178,23 @@ class qtype_calculated_question_test extends advanced_testcase { $this->assertEquals('category' . $question->category, $question->get_variants_selection_seed()); } + + /** + * test_get_question_definition_for_external_rendering + */ + public function test_get_question_definition_for_external_rendering() { + $this->resetAfterTest(); + + $question = test_question_maker::make_question('calculated'); + $question->start_attempt(new question_attempt_step(), 1); + $qa = test_question_maker::get_a_qa($question); + $displayoptions = new question_display_options(); + + $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); + $this->assertNotEmpty($options); + $this->assertEquals(0, $options['unitgradingtype']); + $this->assertEquals(0, $options['unitpenalty']); + $this->assertEquals(qtype_numerical::UNITNONE, $options['unitdisplay']); + $this->assertEmpty($options['unitsleft']); + } } diff --git a/question/type/essay/question.php b/question/type/essay/question.php index b74cca2d90d..8f1f03a2dda 100644 --- a/question/type/essay/question.php +++ b/question/type/essay/question.php @@ -185,4 +185,31 @@ class qtype_essay_question extends question_with_responses { $filearea, $args, $forcedownload); } } + + /** + * Return the question settings that define this question as structured data. + * + * @param question_attempt $qa the current attempt for which we are exporting the settings. + * @param question_display_options $options the question display options which say which aspects of the question + * should be visible. + * @return mixed structure representing the question settings. In web services, this will be JSON-encoded. + */ + public function get_question_definition_for_external_rendering(question_attempt $qa, question_display_options $options) { + // This is a partial implementation, returning only the most relevant question settings for now, + // ideally, we should return as much as settings as possible (depending on the state and display options). + + $settings = [ + 'responseformat' => $this->responseformat, + 'responserequired' => $this->responserequired, + 'responsefieldlines' => $this->responsefieldlines, + 'attachments' => $this->attachments, + 'attachmentsrequired' => $this->attachmentsrequired, + 'maxbytes' => $this->maxbytes, + 'filetypeslist' => $this->filetypeslist, + 'responsetemplate' => $this->responsetemplate, + 'responsetemplateformat' => $this->responsetemplateformat, + ]; + + return $settings; + } } diff --git a/question/type/essay/tests/question_test.php b/question/type/essay/tests/question_test.php index 2ac7e1661dd..9467ecb9b45 100644 --- a/question/type/essay/tests/question_test.php +++ b/question/type/essay/tests/question_test.php @@ -239,4 +239,27 @@ class qtype_essay_question_test extends advanced_testcase { } + /** + * test_get_question_definition_for_external_rendering + */ + public function test_get_question_definition_for_external_rendering() { + $this->resetAfterTest(); + + $essay = test_question_maker::make_an_essay_question(); + $essay->start_attempt(new question_attempt_step(), 1); + $qa = test_question_maker::get_a_qa($essay); + $displayoptions = new question_display_options(); + + $options = $essay->get_question_definition_for_external_rendering($qa, $displayoptions); + $this->assertNotEmpty($options); + $this->assertEquals('editor', $options['responseformat']); + $this->assertEquals(1, $options['responserequired']); + $this->assertEquals(15, $options['responsefieldlines']); + $this->assertEquals(0, $options['attachments']); + $this->assertEquals(0, $options['attachmentsrequired']); + $this->assertNull($options['maxbytes']); + $this->assertNull($options['filetypeslist']); + $this->assertEquals('', $options['responsetemplate']); + $this->assertEquals(FORMAT_MOODLE, $options['responsetemplateformat']); + } } diff --git a/question/type/gapselect/questionbase.php b/question/type/gapselect/questionbase.php index a64e67c2191..0a93eb4561f 100644 --- a/question/type/gapselect/questionbase.php +++ b/question/type/gapselect/questionbase.php @@ -331,4 +331,21 @@ abstract class qtype_gapselect_question_base extends question_graded_automatical $args, $forcedownload); } } + + /** + * Return the question settings that define this question as structured data. + * + * @param question_attempt $qa the current attempt for which we are exporting the settings. + * @param question_display_options $options the question display options which say which aspects of the question + * should be visible. + * @return mixed structure representing the question settings. In web services, this will be JSON-encoded. + */ + public function get_question_definition_for_external_rendering(question_attempt $qa, question_display_options $options) { + // This is a partial implementation, returning only the most relevant question settings for now, + // ideally, we should return as much as settings as possible (depending on the state and display options). + + return [ + 'shufflechoices' => $this->shufflechoices, + ]; + } } diff --git a/question/type/gapselect/tests/question_test.php b/question/type/gapselect/tests/question_test.php index 4bfdfd6554f..4e4663a85a7 100644 --- a/question/type/gapselect/tests/question_test.php +++ b/question/type/gapselect/tests/question_test.php @@ -246,4 +246,17 @@ class qtype_gapselect_question_test extends basic_testcase { 3 => new question_classified_response(2, 'assiduous', 0), ), $gapselect->classify_response(array('p1' => '0', 'p2' => '1', 'p3' => '2'))); } + + /** + * test_get_question_definition_for_external_rendering + */ + public function test_get_question_definition_for_external_rendering() { + $question = test_question_maker::make_question('gapselect', 'maths'); + $question->start_attempt(new question_attempt_step(), 1); + $qa = test_question_maker::get_a_qa($question); + $displayoptions = new question_display_options(); + + $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); + $this->assertEquals(1, $options['shufflechoices']); + } } diff --git a/question/type/match/question.php b/question/type/match/question.php index 6aba21d82fa..15996ceca59 100644 --- a/question/type/match/question.php +++ b/question/type/match/question.php @@ -354,4 +354,21 @@ class qtype_match_question extends question_graded_automatically_with_countback $args, $forcedownload); } } + + /** + * Return the question settings that define this question as structured data. + * + * @param question_attempt $qa the current attempt for which we are exporting the settings. + * @param question_display_options $options the question display options which say which aspects of the question + * should be visible. + * @return mixed structure representing the question settings. In web services, this will be JSON-encoded. + */ + public function get_question_definition_for_external_rendering(question_attempt $qa, question_display_options $options) { + // This is a partial implementation, returning only the most relevant question settings for now, + // ideally, we should return as much as settings as possible (depending on the state and display options). + + return [ + 'shufflestems' => $this->shufflestems, + ]; + } } diff --git a/question/type/match/tests/question_test.php b/question/type/match/tests/question_test.php index 67ab4012aa3..d516fa3a68f 100644 --- a/question/type/match/tests/question_test.php +++ b/question/type/match/tests/question_test.php @@ -233,4 +233,16 @@ class qtype_match_question_test extends advanced_testcase { $this->assertEquals(array(4, 4), $m->get_num_parts_right($postdata)); } + /** + * test_get_question_definition_for_external_rendering + */ + public function test_get_question_definition_for_external_rendering() { + $question = test_question_maker::make_question('match'); + $question->start_attempt(new question_attempt_step(), 1); + $qa = test_question_maker::get_a_qa($question); + $displayoptions = new question_display_options(); + + $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); + $this->assertEquals(1, $options['shufflestems']); + } } diff --git a/question/type/multianswer/question.php b/question/type/multianswer/question.php index 9b6707d85c0..de5522455a6 100644 --- a/question/type/multianswer/question.php +++ b/question/type/multianswer/question.php @@ -355,4 +355,19 @@ class qtype_multianswer_question extends question_graded_automatically_with_coun $args, $forcedownload); } } + + /** + * Return the question settings that define this question as structured data. + * + * @param question_attempt $qa the current attempt for which we are exporting the settings. + * @param question_display_options $options the question display options which say which aspects of the question + * should be visible. + * @return mixed structure representing the question settings. In web services, this will be JSON-encoded. + */ + public function get_question_definition_for_external_rendering(question_attempt $qa, question_display_options $options) { + // Empty implementation for now in order to avoid debugging in core questions (generated in the parent class), + // ideally, we should return as much as settings as possible (depending on the state and display options). + + return null; + } } diff --git a/question/type/multianswer/tests/question_test.php b/question/type/multianswer/tests/question_test.php index 9390ad2bced..487cb57d708 100644 --- a/question/type/multianswer/tests/question_test.php +++ b/question/type/multianswer/tests/question_test.php @@ -238,4 +238,19 @@ class qtype_multianswer_question_test extends advanced_testcase { $finalgrade = $question->compute_final_grade($responses, 1); $this->assertEquals(1 / 3 * (1 - 3 * 0.2) + 2 / 3 * (1 - 2 * 0.2), $finalgrade); } + + /** + * test_get_question_definition_for_external_rendering + */ + public function test_get_question_definition_for_external_rendering() { + $this->resetAfterTest(); + + $question = test_question_maker::make_question('multianswer'); + $question->start_attempt(new question_attempt_step(), 1); + $qa = test_question_maker::get_a_qa($question); + $displayoptions = new question_display_options(); + + $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); + $this->assertNull($options); + } } diff --git a/question/type/multichoice/question.php b/question/type/multichoice/question.php index 624c9092ef4..ec9916c663a 100644 --- a/question/type/multichoice/question.php +++ b/question/type/multichoice/question.php @@ -147,6 +147,26 @@ abstract class qtype_multichoice_base extends question_graded_automatically { $args, $forcedownload); } } + + /** + * Return the question settings that define this question as structured data. + * + * @param question_attempt $qa the current attempt for which we are exporting the settings. + * @param question_display_options $options the question display options which say which aspects of the question + * should be visible. + * @return mixed structure representing the question settings. In web services, this will be JSON-encoded. + */ + public function get_question_definition_for_external_rendering(question_attempt $qa, question_display_options $options) { + // This is a partial implementation, returning only the most relevant question settings for now, + // ideally, we should return as much as settings as possible (depending on the state and display options). + + return [ + 'shuffleanswers' => $this->shuffleanswers, + 'answernumbering' => $this->answernumbering, + 'showstandardinstruction' => $this->showstandardinstruction, + 'layout' => $this->layout, + ]; + } } diff --git a/question/type/multichoice/tests/question_multi_test.php b/question/type/multichoice/tests/question_multi_test.php index d0620243ccf..f5e532ca621 100644 --- a/question/type/multichoice/tests/question_multi_test.php +++ b/question/type/multichoice/tests/question_multi_test.php @@ -175,4 +175,19 @@ class qtype_multichoice_multi_question_test extends advanced_testcase { } } + /** + * test_get_question_definition_for_external_rendering + */ + public function test_get_question_definition_for_external_rendering() { + $question = test_question_maker::make_a_multichoice_multi_question(); + $question->start_attempt(new question_attempt_step(), 1); + $qa = test_question_maker::get_a_qa($question); + $displayoptions = new question_display_options(); + + $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); + $this->assertEquals(1, $options['shuffleanswers']); + $this->assertEquals('abc', $options['answernumbering']); + $this->assertEquals(0, $options['showstandardinstruction']); + $this->assertEquals(1, $options['shuffleanswers']); + } } diff --git a/question/type/numerical/question.php b/question/type/numerical/question.php index 79733d4aebf..7137ed8d492 100644 --- a/question/type/numerical/question.php +++ b/question/type/numerical/question.php @@ -44,7 +44,8 @@ class qtype_numerical_question extends question_graded_automatically { public $unitgradingtype; /** @var number the penalty for a missing or unrecognised unit. */ public $unitpenalty; - + /** @var boolean whether the units come before or after the number */ + public $unitsleft; /** @var qtype_numerical_answer_processor */ public $ap; @@ -318,6 +319,26 @@ class qtype_numerical_question extends question_graded_automatically { $args, $forcedownload); } } + + /** + * Return the question settings that define this question as structured data. + * + * @param question_attempt $qa the current attempt for which we are exporting the settings. + * @param question_display_options $options the question display options which say which aspects of the question + * should be visible. + * @return mixed structure representing the question settings. In web services, this will be JSON-encoded. + */ + public function get_question_definition_for_external_rendering(question_attempt $qa, question_display_options $options) { + // This is a partial implementation, returning only the most relevant question settings for now, + // ideally, we should return as much as settings as possible (depending on the state and display options). + + return [ + 'unitgradingtype' => $this->unitgradingtype, + 'unitpenalty' => $this->unitpenalty, + 'unitdisplay' => $this->unitdisplay, + 'unitsleft' => $this->unitsleft, + ]; + } } diff --git a/question/type/numerical/questiontype.php b/question/type/numerical/questiontype.php index 6e2a89a94c3..25754a97e2d 100644 --- a/question/type/numerical/questiontype.php +++ b/question/type/numerical/questiontype.php @@ -359,6 +359,7 @@ class qtype_numerical extends question_type { $question->unitdisplay = $questiondata->options->showunits; $question->unitgradingtype = $questiondata->options->unitgradingtype; $question->unitpenalty = $questiondata->options->unitpenalty; + $question->unitsleft = $questiondata->options->unitsleft; $question->ap = $this->make_answer_processor($questiondata->options->units, $questiondata->options->unitsleft); } diff --git a/question/type/numerical/tests/question_test.php b/question/type/numerical/tests/question_test.php index 73a69605b8d..33eaaf5056a 100644 --- a/question/type/numerical/tests/question_test.php +++ b/question/type/numerical/tests/question_test.php @@ -306,4 +306,23 @@ class qtype_numerical_question_test extends advanced_testcase { new question_classified_response(null, '$abc', 0.0)), $num->classify_response(array('answer' => '$abc'))); } + + /** + * test_get_question_definition_for_external_rendering + */ + public function test_get_question_definition_for_external_rendering() { + $this->resetAfterTest(); + + $question = test_question_maker::make_question('numerical', 'unit'); + $question->start_attempt(new question_attempt_step(), 1); + $qa = test_question_maker::get_a_qa($question); + $displayoptions = new question_display_options(); + + $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); + $this->assertNotEmpty($options); + $this->assertEquals(1, $options['unitgradingtype']); + $this->assertEquals(0.5, $options['unitpenalty']); + $this->assertEquals(qtype_numerical::UNITSELECT, $options['unitdisplay']); + $this->assertEmpty($options['unitsleft']); + } } diff --git a/question/type/shortanswer/question.php b/question/type/shortanswer/question.php index a9de53be76e..b8923aabaec 100644 --- a/question/type/shortanswer/question.php +++ b/question/type/shortanswer/question.php @@ -184,4 +184,17 @@ class qtype_shortanswer_question extends question_graded_by_strategy $args, $forcedownload); } } + + /** + * Return the question settings that define this question as structured data. + * + * @param question_attempt $qa the current attempt for which we are exporting the settings. + * @param question_display_options $options the question display options which say which aspects of the question + * should be visible. + * @return mixed structure representing the question settings. In web services, this will be JSON-encoded. + */ + public function get_question_definition_for_external_rendering(question_attempt $qa, question_display_options $options) { + // No need to return anything, external clients do not need additional information for rendering this question type. + return null; + } } diff --git a/question/type/shortanswer/tests/question_test.php b/question/type/shortanswer/tests/question_test.php index c8c05cf9824..89f4d384a81 100644 --- a/question/type/shortanswer/tests/question_test.php +++ b/question/type/shortanswer/tests/question_test.php @@ -226,4 +226,19 @@ class qtype_shortanswer_question_test extends advanced_testcase { question_classified_response::no_response()), $sa->classify_response(array('answer' => ''))); } + + /** + * test_get_question_definition_for_external_rendering + */ + public function test_get_question_definition_for_external_rendering() { + $this->resetAfterTest(); + + $question = test_question_maker::make_question('shortanswer'); + $question->start_attempt(new question_attempt_step(), 1); + $qa = test_question_maker::get_a_qa($question); + $displayoptions = new question_display_options(); + + $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); + $this->assertNull($options); + } } diff --git a/question/type/truefalse/question.php b/question/type/truefalse/question.php index 05d1d694169..320781b1e10 100644 --- a/question/type/truefalse/question.php +++ b/question/type/truefalse/question.php @@ -123,4 +123,17 @@ class qtype_truefalse_question extends question_graded_automatically { $args, $forcedownload); } } + + /** + * Return the question settings that define this question as structured data. + * + * @param question_attempt $qa the current attempt for which we are exporting the settings. + * @param question_display_options $options the question display options which say which aspects of the question + * should be visible. + * @return mixed structure representing the question settings. In web services, this will be JSON-encoded. + */ + public function get_question_definition_for_external_rendering(question_attempt $qa, question_display_options $options) { + // No need to return anything, external clients do not need additional information for rendering this question type. + return null; + } } diff --git a/question/type/truefalse/tests/question_test.php b/question/type/truefalse/tests/question_test.php index 4d68b4b0726..fb530542695 100644 --- a/question/type/truefalse/tests/question_test.php +++ b/question/type/truefalse/tests/question_test.php @@ -107,4 +107,19 @@ class qtype_truefalse_question_test extends advanced_testcase { $tf->id => question_classified_response::no_response()), $tf->classify_response(array())); } + + /** + * test_get_question_definition_for_external_rendering + */ + public function test_get_question_definition_for_external_rendering() { + $this->resetAfterTest(); + + $question = test_question_maker::make_question('truefalse', 'true'); + $question->start_attempt(new question_attempt_step(), 1); + $qa = test_question_maker::get_a_qa($question); + $displayoptions = new question_display_options(); + + $options = $question->get_question_definition_for_external_rendering($qa, $displayoptions); + $this->assertNull($options); + } }