diff --git a/classes/cli_helper.php b/classes/cli_helper.php index 8d68294..84e420e 100644 --- a/classes/cli_helper.php +++ b/classes/cli_helper.php @@ -281,12 +281,13 @@ public function validate_and_clean_args(): void { break; } } - if (!(isset($cliargs['manifestpath']) || isset($cliargs['quizmanifestpath']) || isset($cliargs['nonquizmanifestpath'])) && !isset($cliargs['contextlevel'])) { + if (!(isset($cliargs['manifestpath']) || isset($cliargs['quizmanifestpath'])) && !isset($cliargs['contextlevel'])) { echo "\nYou have not specified context. " . "You must specify context level (--contextlevel) unless " . "using a function where this information can be read from a manifest file, in which case " . "you could set a manifest path (--manifestpath) instead. If using exportrepofrommoodle, you " . - "must set manifest path only. If you still see this message, you may be using invalid arguments.\n"; + "must set manifest path only. If dealing with import/export of quizzes, you must specify --quizmanifestpath. " . + "If you still see this message, you may be using invalid arguments.\n"; static::call_exit(); } diff --git a/classes/create_repo.php b/classes/create_repo.php index af738c1..8f5dabf 100644 --- a/classes/create_repo.php +++ b/classes/create_repo.php @@ -266,7 +266,7 @@ public function create_quiz_directories($clihelper, $scriptdirectory) { $quizmanifestname = cli_helper::get_manifest_path($moodleinstance, 'module', null, $contextinfo->contextinfo->coursename, $quiz->name, $rootdirectory); chdir($scriptdirectory); - $output = shell_exec('php exportquizstructurefrommoodle.php -w -r "" -i "' . $moodleinstance . '" -n ' . $instanceid . ' -t ' . $token. ' -p "' . $this->manifestpath . '" -f "' . $quizmanifestname . '"'); + $output = shell_exec('php exportquizstructurefrommoodle.php -w -r "" -i "' . $moodleinstance . ' -t ' . $token. ' -p "' . $this->manifestpath . '" -f "' . $quizmanifestname . '"'); $quizlocation = new \StdClass(); $quizlocation->moduleid = $instanceid; $quizlocation->directory = basename($rootdirectory); diff --git a/classes/export_quiz.php b/classes/export_quiz.php index 3d52009..f406054 100644 --- a/classes/export_quiz.php +++ b/classes/export_quiz.php @@ -90,11 +90,6 @@ class export_quiz { * @var \stdClass|null */ public ?\stdClass $nonquizmanifestcontents = null; - /** - * URL of Moodle instance - * - * @var string - */ /** * Full path to output file * @@ -112,18 +107,15 @@ public function __construct(cli_helper $clihelper, array $moodleinstances) { // Convert command line options into variables. $arguments = $clihelper->get_arguments(); $moodleinstance = $arguments['moodleinstance']; - $instanceid = $arguments['instanceid']; - $rootdirectory = ($arguments['rootdirectory']) ? $arguments['rootdirectory'] . '/' : ''; - if ($arguments['quizmanifestpath']) { - $this->quizmanifestpath = ($arguments['quizmanifestpath']) ? - $rootdirectory . $arguments['quizmanifestpath'] : null; - $this->quizmanifestcontents = json_decode(file_get_contents($this->quizmanifestpath)); - if (!$this->quizmanifestcontents) { - echo "\nUnable to access or parse manifest file: {$this->quizmanifestpath}\nAborting.\n"; - $this->call_exit(); - } - $instanceid = $this->quizmanifestcontents->context->instanceid; + $rootdirectory = ($arguments['rootdirectory']) ? $arguments['rootdirectory'] . '/' : ''; + $this->quizmanifestpath = ($arguments['quizmanifestpath']) ? + $rootdirectory . $arguments['quizmanifestpath'] : null; + $this->quizmanifestcontents = json_decode(file_get_contents($this->quizmanifestpath)); + if (!$this->quizmanifestcontents) { + echo "\nUnable to access or parse manifest file: {$this->quizmanifestpath}\nAborting.\n"; + $this->call_exit(); } + $instanceid = $this->quizmanifestcontents->context->instanceid; if ($arguments['nonquizmanifestpath']) { $this->nonquizmanifestpath = ($arguments['nonquizmanifestpath']) ? $rootdirectory . $arguments['nonquizmanifestpath'] : null; @@ -147,8 +139,6 @@ public function __construct(cli_helper $clihelper, array $moodleinstances) { 'wstoken' => $token, 'wsfunction' => 'qbank_gitsync_export_quiz_data', 'moodlewsrestformat' => 'json', - 'coursename' => $arguments['coursename'], - 'quizname' => $arguments['modulename'], 'moduleid' => $instanceid, ]; $this->curlrequest->set_option(CURLOPT_RETURNTRANSFER, true); @@ -209,19 +199,22 @@ public function export_quiz_data() { if (!$responsejson) { echo "Broken JSON returned from Moodle:\n"; echo $response . "\n"; - echo "{$this->filepath} not updated.\n"; + echo "Quiz data file not updated.\n"; $this->call_exit(); + $responsejson = json_decode('{"quiz": {"name": ""}, "questions": []}'); // For unit test purposes. } else if (property_exists($responsejson, 'exception')) { echo "{$responsejson->message}\n"; if (property_exists($responsejson, 'debuginfo')) { echo "{$responsejson->debuginfo}\n"; } - echo "{$this->filepath} not updated.\n"; + echo "Quiz data file not updated.\n"; $this->call_exit(); + $responsejson = json_decode('{"quiz": {"name": ""}, "questions": []}'); // For unit test purposes. } $quizmanifestentries = []; $nonquizmanifestentries = []; - // Determine quiz info location based on loactions of manifest paths. + $missingquestions = false; + // Determine quiz info location based on locations of manifest paths. if ($this->quizmanifestpath) { $this->filepath = cli_helper::get_quiz_structure_path($responsejson->quiz->name, dirname($this->quizmanifestpath)); $quizmanifestentries = array_column($this->quizmanifestcontents->questions, null, 'questionbankentryid'); @@ -242,20 +235,29 @@ public function export_quiz_data() { $question->nonquizfilepath = $nonquizmanifestentry->filepath; unset($question->questionbankentryid); } else { + $missingquestions = true; $multiple = ($this->quizmanifestpath && $this->nonquizmanifestpath) ? 's' : ''; - echo "Question: {$question->questionbankentryid}\n"; - echo "This question is in the quiz but not in the supplied manifest file" . $multiple . ".\n"; - echo "Questions must either be in the repo for the quiz context defined by a supplied quiz manifest " . - "(--quizmanifestpath) or in the context (e.g. course) " . - "defined by a different manifest (--nonquizmanifestpath).\n"; - echo "You can supply either or both. If your quiz questions are spread between 3 or more contexts " . - "consider consolidating them.\n"; + echo "\nQuestion: {$question->questionbankentryid}\n"; + echo "This question is in the quiz but not in the supplied manifest file{$multiple}\n"; + } + } + if ($missingquestions) { + echo "Questions must either be in the repo for the quiz context defined by a supplied quiz manifest " . + "(--quizmanifestpath) or in the context (e.g. course) " . + "defined by a different manifest (--nonquizmanifestpath).\n"; + echo "You can supply either or both. If your quiz questions are spread between 3 or more contexts " . + "you will need to consolidate them.\n"; + echo "Quiz structure file: {$this->filepath} not updated.\n"; + } else { + // Save exported information (including relative file location but not QBE id so Moodle independent). + $success = file_put_contents($this->filepath, json_encode($responsejson)); + if ($success === false) { + echo "\nUnable to update quiz structure file: {$this->filepath}\n Aborting.\n"; + $this->call_exit(); } + echo "Quiz data exported to:\n"; + echo "{$this->filepath}\n"; } - // Save exported information (including relative file location but not QBE id so Moodle independent). - file_put_contents($this->filepath, json_encode($responsejson)); - echo "Quiz data exported to:\n"; - echo "{$this->filepath}\n"; } /** diff --git a/classes/export_repo.php b/classes/export_repo.php index 2fa923a..7cda0dc 100644 --- a/classes/export_repo.php +++ b/classes/export_repo.php @@ -335,7 +335,7 @@ public function update_quiz_directories($clihelper, $scriptdirectory) { $quizmanifestname = cli_helper::get_manifest_path($moodleinstance, 'module', null, $contextinfo->contextinfo->coursename, $quiz->name, $rootdirectory); chdir($scriptdirectory); - $output = shell_exec('php exportquizstructurefrommoodle.php -w -r "" -i "' . $moodleinstance . '" -n ' . $instanceid . ' -t ' . $token. ' -p "' . $this->manifestpath. '" -f "' . $quizmanifestname . '"'); + $output = shell_exec('php exportquizstructurefrommoodle.php -w -r "" -i "' . $moodleinstance . ' -t ' . $token. ' -p "' . $this->manifestpath. '" -f "' . $quizmanifestname . '"'); echo $output; } } diff --git a/cli/exportquizstructurefrommoodle.php b/cli/exportquizstructurefrommoodle.php index 3ad6370..4c62ff1 100644 --- a/cli/exportquizstructurefrommoodle.php +++ b/cli/exportquizstructurefrommoodle.php @@ -87,30 +87,6 @@ 'variable' => 'usegit', 'valuerequired' => true, ], - [ - 'longopt' => 'coursename', - 'shortopt' => 'c', - 'description' => 'Unique course name for course or module context.', - 'default' => null, - 'variable' => 'coursename', - 'valuerequired' => true, - ], - [ - 'longopt' => 'modulename', - 'shortopt' => 'm', - 'description' => 'Unique (within course) quiz name.', - 'default' => null, - 'variable' => 'modulename', - 'valuerequired' => true, - ], - [ - 'longopt' => 'instanceid', - 'shortopt' => 'n', - 'description' => 'Numerical course module id of quiz.', - 'default' => null, - 'variable' => 'instanceid', - 'valuerequired' => true, - ], [ 'longopt' => 'subcall', 'shortopt' => 'w', diff --git a/testrepoparent/testrepo_quiz_quiz-1/fakeexportquiz_module_course-1_quiz-1_question_manifest.json b/testrepoparent/testrepo_quiz_quiz-1/fakeexportquiz_module_course-1_quiz-1_question_manifest.json index 10aec08..6e99a0b 100644 --- a/testrepoparent/testrepo_quiz_quiz-1/fakeexportquiz_module_course-1_quiz-1_question_manifest.json +++ b/testrepoparent/testrepo_quiz_quiz-1/fakeexportquiz_module_course-1_quiz-1_question_manifest.json @@ -15,6 +15,13 @@ "importedversion": "1", "exportedversion": "1", "format": "xml" + }, + { + "questionbankentryid":"36002", + "filepath": "\/top\/quiz-cat\/Quiz-Question-2.xml", + "importedversion": "1", + "exportedversion": "1", + "format": "xml" } ] } \ No newline at end of file diff --git a/testrepoparent/testrepo_quiz_quiz-1/top/gitsync_category.xml b/testrepoparent/testrepo_quiz_quiz-1/top/gitsync_category.xml index f5864d9..2322c43 100644 --- a/testrepoparent/testrepo_quiz_quiz-1/top/gitsync_category.xml +++ b/testrepoparent/testrepo_quiz_quiz-1/top/gitsync_category.xml @@ -2,7 +2,7 @@ - top/cat 1 + top First imported folder diff --git a/testrepoparent/testrepo_quiz_quiz-1/top/quiz-cat/Quiz-Question-2.xml b/testrepoparent/testrepo_quiz_quiz-1/top/quiz-cat/Quiz-Question-2.xml new file mode 100644 index 0000000..66b71f5 --- /dev/null +++ b/testrepoparent/testrepo_quiz_quiz-1/top/quiz-cat/Quiz-Question-2.xml @@ -0,0 +1,25 @@ + + + + + Quiz Question 2 + + + This is a test question.

]]> + + + + + 1 + 0.3333333 + 0 + + 0 + + This is a test answer. + + + + + + \ No newline at end of file diff --git a/testrepoparent/testrepo_quiz_quiz-1/top/quiz-cat/Quiz-Question-3.xml b/testrepoparent/testrepo_quiz_quiz-1/top/quiz-cat/Quiz-Question-3.xml new file mode 100644 index 0000000..b109781 --- /dev/null +++ b/testrepoparent/testrepo_quiz_quiz-1/top/quiz-cat/Quiz-Question-3.xml @@ -0,0 +1,25 @@ + + + + + Quiz Question 3 + + + This is a test question.

]]> + + + + + 1 + 0.3333333 + 0 + + 0 + + This is a test answer. + + + + + + \ No newline at end of file diff --git a/testrepoparent/testrepo_quiz_quiz-1/top/quiz-cat/gitsync_category.xml b/testrepoparent/testrepo_quiz_quiz-1/top/quiz-cat/gitsync_category.xml new file mode 100644 index 0000000..fbdd8e2 --- /dev/null +++ b/testrepoparent/testrepo_quiz_quiz-1/top/quiz-cat/gitsync_category.xml @@ -0,0 +1,12 @@ + + + + + top/quiz-cat + + + First imported folder + + + + \ No newline at end of file diff --git a/tests/export_quiz_test.php b/tests/export_quiz_test.php index 461da14..0954288 100644 --- a/tests/export_quiz_test.php +++ b/tests/export_quiz_test.php @@ -81,14 +81,13 @@ class export_quiz_test extends advanced_testcase { const FEEDBACK = 'Quiz feedback'; const HEADING1 = 'Heading 1'; const HEADING2 = 'Heading 2'; + const COURSENAME = 'Course 1'; /** @var array input parameters */ protected array $quizoutput = [ 'quiz' => [ 'name' => self::QUIZNAME, 'intro' => self::QUIZINTRO, 'introformat' => '0', - 'coursename' => null, - 'courseid' => null, 'questionsperpage' => '0', 'grade' => '100.00000', 'navmethod' => 'free', @@ -145,6 +144,15 @@ public function setUp(): void { 'help' => false, 'subcall' => false, ]; + + } + + /** + * Mock set up + * + * @return void + */ + public function set_up_mocks() { $this->clihelper = $this->getMockBuilder(\qbank_gitsync\cli_helper::class)->onlyMethods([ 'get_arguments', 'check_context', ])->setConstructorArgs([[]])->getMock(); @@ -171,8 +179,8 @@ public function setUp(): void { /** * Test the full process. */ - public function test_process(): void { - // Will get questions in order from manifest file in testrepo. + public function x_test_process(): void { + $this->set_up_mocks(); $this->curl->expects($this->exactly(1))->method('execute')->willReturnOnConsecutiveCalls( json_encode($this->quizoutput) ); @@ -187,4 +195,204 @@ public function test_process(): void { $this->expectOutputRegex('/^Quiz data exported to:\n.*testrepo_quiz_quiz-1\/quiz-1_quiz.json\n$/s'); } + /** + * Test message if export JSON broken. + */ + public function x_test_broken_json_on_export(): void { + $this->set_up_mocks(); + $this->curl->expects($this->any())->method('execute')->willReturn( + '{"quiz": "}' + ); + + $this->exportquiz->process(); + + $this->expectOutputRegex('/Broken JSON returned from Moodle:' . + '.*{"quiz": <\/Question>"}/s'); + } + + /** + * Test message if export exception. + */ + public function x_test_exception_on_export(): void { + $this->set_up_mocks(); + $this->curl->expects($this->any())->method('execute')->willReturn( + '{"exception":"moodle_exception","message":"No token"}' + ); + + $this->exportquiz->process(); + + $this->expectOutputRegex('/No token/'); + } + + /** + * Test message if manifest file update issue. + */ + public function x_test_manifest_file_update_error(): void { + $this->set_up_mocks(); + $this->curl->expects($this->any())->method('execute')->willReturn( + json_encode($this->quizoutput) + ); + $filepath = cli_helper::get_quiz_structure_path(self::QUIZNAME, dirname($this->exportquiz->quizmanifestpath)); + file_put_contents($filepath, ''); + chmod($filepath, 0000); + + @$this->exportquiz->process(); + $this->expectOutputRegex('/\nUnable to update quiz structure file.*Aborting.*$/s'); + } + + /** + * Test if quiz context questions. + */ + public function x_test_quiz_context_questions(): void { + $this->quizoutput['questions'][] = + [ + 'questionbankentryid' => '36002', + 'slot' => '2', + 'page' => '2', + 'requireprevious' => 0, + 'maxmark' => '1.0000000', + ]; + $this->set_up_mocks(); + $this->curl->expects($this->any())->method('execute')->willReturn( + json_encode($this->quizoutput) + ); + $this->exportquiz->process(); + $structurecontents = json_decode(file_get_contents($this->exportquiz->filepath)); + $this->assertEquals(2, count($structurecontents->questions)); + $this->assertEquals(false, isset($structurecontents->questions[0]->questionbankentryid)); + $this->assertEquals(false, isset($structurecontents->questions[0]->nonquizfilepath)); + $this->assertEquals("/top/Quiz-Question.xml", $structurecontents->questions[0]->quizfilepath); + $this->assertEquals(false, isset($structurecontents->questions[1]->questionbankentryid)); + $this->assertEquals(false, isset($structurecontents->questions[1]->nonquizfilepath)); + $this->assertEquals("/top/quiz-cat/Quiz-Question-2.xml", $structurecontents->questions[1]->quizfilepath); + $this->expectOutputRegex('/Quiz data exported to.*testrepo_quiz_quiz-1\/quiz-1_quiz.json.*$/s'); + } + + /** + * Test if course context questions. + */ + public function x_test_course_context_questions(): void { + $this->quizoutput['questions'] = [ + [ + 'questionbankentryid' => '35002', + 'slot' => '2', + 'page' => '2', + 'requireprevious' => 0, + 'maxmark' => '1.0000000', + ], + [ + 'questionbankentryid' => '35003', + 'slot' => '2', + 'page' => '2', + 'requireprevious' => 0, + 'maxmark' => '1.0000000', + ] + ]; + $this->set_up_mocks(); + $this->curl->expects($this->any())->method('execute')->willReturn( + json_encode($this->quizoutput) + ); + $this->exportquiz->process(); + $structurecontents = json_decode(file_get_contents($this->exportquiz->filepath)); + $this->assertEquals(2, count($structurecontents->questions)); + $this->assertEquals(false, isset($structurecontents->questions[0]->questionbankentryid)); + $this->assertEquals(false, isset($structurecontents->questions[0]->quizfilepath)); + $this->assertEquals("/top/cat-2/subcat-2_1/Third-Question.xml", $structurecontents->questions[0]->nonquizfilepath); + $this->assertEquals(false, isset($structurecontents->questions[1]->questionbankentryid)); + $this->assertEquals(false, isset($structurecontents->questions[1]->quizfilepath)); + $this->assertEquals("/top/cat-2/Second-Question.xml", $structurecontents->questions[1]->nonquizfilepath); + $this->expectOutputRegex('/Quiz data exported to.*testrepo_quiz_quiz-1\/quiz-1_quiz.json.*$/s'); + } + + /** + * Test if missing questions. + */ + public function x_test_missing_questions(): void { + $this->quizoutput['questions'] = [ + [ + 'questionbankentryid' => '35002', + 'slot' => '2', + 'page' => '2', + 'requireprevious' => 0, + 'maxmark' => '1.0000000', + ],[ + 'questionbankentryid' => '36001', + 'slot' => '2', + 'page' => '2', + 'requireprevious' => 0, + 'maxmark' => '1.0000000', + ], + [ + 'questionbankentryid' => '37001', + 'slot' => '2', + 'page' => '2', + 'requireprevious' => 0, + 'maxmark' => '1.0000000', + ] + ]; + $this->set_up_mocks(); + $this->curl->expects($this->any())->method('execute')->willReturn( + json_encode($this->quizoutput) + ); + $this->exportquiz->process(); + $this->assertEquals(false, is_file($this->exportquiz->filepath)); + $this->expectOutputRegex('/\nQuestion: 37001\nThis question is in the quiz but not in the supplied manifest files\n' . + 'Questions must either be in the repo.*testrepo_quiz_quiz-1\/quiz-1_quiz.json not updated.\n$/s'); + } + + /** + * Test if mixed questions. + */ + public function test_mixed_questions(): void { + $this->quizoutput['questions'] = [ + [ + 'questionbankentryid' => '35001', + 'slot' => '2', + 'page' => '2', + 'requireprevious' => 0, + 'maxmark' => '1.0000000', + ], + [ + 'questionbankentryid' => '35002', + 'slot' => '2', + 'page' => '2', + 'requireprevious' => 0, + 'maxmark' => '1.0000000', + ], + [ + 'questionbankentryid' => '36001', + 'slot' => '3', + 'page' => '3', + 'requireprevious' => 0, + 'maxmark' => '1.0000000', + ], + [ + 'questionbankentryid' => '36002', + 'slot' => '4', + 'page' => '4', + 'requireprevious' => 0, + 'maxmark' => '1.0000000', + ] + ]; + $this->set_up_mocks(); + $this->curl->expects($this->any())->method('execute')->willReturn( + json_encode($this->quizoutput) + ); + $this->exportquiz->process(); + $structurecontents = json_decode(file_get_contents($this->exportquiz->filepath)); + $this->assertEquals(4, count($structurecontents->questions)); + $this->assertEquals(false, isset($structurecontents->questions[0]->questionbankentryid)); + $this->assertEquals(false, isset($structurecontents->questions[0]->quizfilepath)); + $this->assertEquals("/top/cat-1/First-Question.xml", $structurecontents->questions[0]->nonquizfilepath); + $this->assertEquals(false, isset($structurecontents->questions[1]->questionbankentryid)); + $this->assertEquals(false, isset($structurecontents->questions[1]->quizfilepath)); + $this->assertEquals("/top/cat-2/subcat-2_1/Third-Question.xml", $structurecontents->questions[1]->nonquizfilepath); + $this->assertEquals(false, isset($structurecontents->questions[2]->questionbankentryid)); + $this->assertEquals(false, isset($structurecontents->questions[2]->nonquizfilepath)); + $this->assertEquals("/top/Quiz-Question.xml", $structurecontents->questions[2]->quizfilepath); + $this->assertEquals(false, isset($structurecontents->questions[3]->questionbankentryid)); + $this->assertEquals(false, isset($structurecontents->questions[3]->nonquizfilepath)); + $this->assertEquals("/top/quiz-cat/Quiz-Question-2.xml", $structurecontents->questions[3]->quizfilepath); + $this->expectOutputRegex('/Quiz data exported to.*testrepo_quiz_quiz-1\/quiz-1_quiz.json.*$/s'); + } }