quiz-data - Bug fixes

classes/cli_helper.php

@@ -198,8 +198,8 @@ public function validate_and_clean_args(): void {
         if (isset($cliargs['manifestpath'])) {
             $cliargs['manifestpath'] = $this->trim_slashes($cliargs['manifestpath']);
             if (isset($cliargs['coursename']) || isset($cliargs['modulename'])
-                        || isset($cliargs['coursecategory']) || (isset($cliargs['instanceid'])
-                        || isset($cliargs['contextlevel']) )) {
+                        || isset($cliargs['coursecategory']) || isset($cliargs['instanceid'])
+                        || isset($cliargs['contextlevel'])) {
                 echo "\nYou have specified a manifest file (possibly as a default in your config file). " .
                         "Contextlevel, instance id, course name, module name and/or course category are not needed. " .
                         "Context data can be extracted from the file.\n";
@@ -281,8 +281,8 @@ public function validate_and_clean_args(): void {
                     break;
             }
         }
-        if (!(isset($cliargs['manifestpath']) || isset($cliargs['quizmanifestpath'])
-                || (isset($cliargs['nonquizmanifestpath']) && isset($cliargs['instanceid']))) && !isset($cliargs['contextlevel'])) {
+        if (!isset($cliargs['manifestpath']) && !isset($cliargs['quizmanifestpath'])
+                    && !isset($cliargs['contextlevel']) && !isset($cliargs['quizdatapath'])) {
             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 " .
@@ -545,6 +545,7 @@ public function commit_hash_update(object $activity): void {
         if (!$this->get_arguments()['usegit']) {
             return;
         }
+        chdir(dirname($activity->manifestpath));
         foreach ($activity->manifestcontents->questions as $question) {
             $commithash = exec('git log -n 1 --pretty=format:%H -- "' . substr($question->filepath, 1) . '"');
             if ($commithash) {
@@ -572,10 +573,8 @@ public function commit_hash_setup(object $activity): void {
         $this->create_gitignore($activity->manifestpath);
         $manifestdirname = dirname($activity->manifestpath);
         chdir($manifestdirname);
-        if (empty($this->get_arguments()['subcall'])) {
-            exec("git add --all");
-            exec('git commit -m "Initial Commit - ' . basename($activity->manifestpath)  . '"');
-        }
+        exec("git add --all");
+        exec('git commit -m "Initial Commit - ' . basename($activity->manifestpath)  . '"');
         foreach ($activity->manifestcontents->questions as $question) {
             $commithash = exec('git log -n 1 --pretty=format:%H -- "' . substr($question->filepath, 1) . '"');
             if ($commithash) {

classes/create_repo.php

@@ -109,6 +109,12 @@ class create_repo {
      * @var string
      */
     public string $moodleurl;
+    /**
+     * Are we using git?.
+     * Set in config. Adds commit hash to manifest.
+     * @var bool
+     */
+    public bool $usegit;
 
     /**
      * Constructor.
@@ -121,6 +127,7 @@ public function __construct(cli_helper $clihelper, array $moodleinstances) {
         // (Moodle code rules don't allow 'extract()').
         $arguments = $clihelper->get_arguments();
         $moodleinstance = $arguments['moodleinstance'];
+        $this->usegit = $arguments['usegit'];
         if ($arguments['directory']) {
             $this->directory = $arguments['rootdirectory'] . '/' . $arguments['directory'];
         } else {
@@ -304,7 +311,8 @@ public function call_repo_creation(string $rootdirectory, string $moodleinstance
                                        string $token, string $ignorecat, string $scriptdirectory
                                       ): ?string {
         chdir($scriptdirectory);
-        return shell_exec('php createrepo.php -w -r "' . $rootdirectory .  '" -i "' . $moodleinstance .
+        return shell_exec('php createrepo.php -u ' . $this->usegit . ' -w -r "' .
+                $rootdirectory .  '" -i "' . $moodleinstance .
                 '" -l "module" -n ' . $instanceid . ' -t "' . $token . '" -x ' . $ignorecat);
     }
 
@@ -319,7 +327,8 @@ public function call_repo_creation(string $rootdirectory, string $moodleinstance
     public function call_export_quiz(string $moodleinstance, string $token,
                                     string $quizmanifestpath, string $scriptdirectory): ?string {
         chdir($scriptdirectory);
-        return shell_exec('php exportquizstructurefrommoodle.php -w -r "" -i "' . $moodleinstance . '" -t "'
+        return shell_exec('php exportquizstructurefrommoodle.php -u ' . $this->usegit .
+                ' -w -r "" -i "' . $moodleinstance . '" -t "'
                 . $token. '" -p "' . $this->manifestpath . '" -f "' . $quizmanifestpath . '"');
     }
 }

classes/export_repo.php

@@ -98,6 +98,12 @@ class export_repo {
      * @var string|null
      */
     public ?string $ignorecat;
+    /**
+     * Are we using git?.
+     * Set in config. Adds commit hash to manifest.
+     * @var bool
+     */
+   public bool $usegit;
 
     /**
      * Constructor
@@ -110,6 +116,7 @@ public function __construct(cli_helper $clihelper, array $moodleinstances) {
         // (Moodle code rules don't allow 'extract()').
         $arguments = $clihelper->get_arguments();
         $moodleinstance = $arguments['moodleinstance'];
+        $this->usegit = $arguments['usegit'];
         $defaultwarning = false;
         $this->manifestpath = $arguments['rootdirectory'] . '/' . $arguments['manifestpath'];
         if (is_array($arguments['token'])) {
@@ -364,7 +371,7 @@ public function call_repo_creation(string $rootdirectory, string $moodleinstance
                                        string $token, string $ignorecat, string $scriptdirectory
                                       ): ?string {
         chdir($scriptdirectory);
-        return shell_exec('php createrepo.php -w -r "' . $rootdirectory .  '" -i "' . $moodleinstance .
+        return shell_exec('php createrepo.php -u ' . $this->usegit . ' -w -r "' . $rootdirectory .  '" -i "' . $moodleinstance .
                 '" -l "module" -n ' . $instanceid . ' -t ' . $token . ' -x ' . $ignorecat);
     }
 
@@ -379,7 +386,7 @@ public function call_repo_creation(string $rootdirectory, string $moodleinstance
     public function call_export_quiz(string $moodleinstance, string $token,
                                     string $quizmanifestpath, string $scriptdirectory): ?string {
         chdir($scriptdirectory);
-        return shell_exec('php exportquizstructurefrommoodle.php -w -r "" -i "' . $moodleinstance . ' -t '
+        return shell_exec('php exportquizstructurefrommoodle.php -u ' . $this->usegit . ' -w -r "" -i "' . $moodleinstance . '" -t '
                 . $token. ' -p "' . $this->manifestpath . '" -f "' . $quizmanifestpath . '"');
     }
 
@@ -396,7 +403,7 @@ public function call_export_quiz(string $moodleinstance, string $token,
     public function call_export_repo(string $rootdirectory, string $moodleinstance, string $token,
                 string $quizmanifestname, string $scriptdirectory): ?string {
         chdir($scriptdirectory);
-        return shell_exec('php exportrepofrommoodle.php -w -r "' . $rootdirectory . '" -i "' .
+        return shell_exec('php exportrepofrommoodle.php -u ' . $this->usegit . ' -w -r "' . $rootdirectory . '" -i "' .
                             $moodleinstance . '" -f "' . $quizmanifestname . '" -t ' . $token);
     }
 }

classes/external/import_quiz_data.php

@@ -81,11 +81,11 @@ public static function execute_parameters() {
             ),
             'feedback' => new external_multiple_structure(
                 new external_single_structure([
-                    'feedbacktext' => new external_value(PARAM_TEXT, 'Feedback text'),
-                    'feedbacktextformat' => new external_value(PARAM_SEQUENCE, 'Format of feedback'),
-                    'mingrade' => new external_value(PARAM_TEXT, 'minimum mark'),
-                    'maxgrade' => new external_value(PARAM_TEXT, 'maximum mark'),
-                ])
+                    'feedbacktext' => new external_value(PARAM_TEXT, 'Feedback text', VALUE_OPTIONAL),
+                    'feedbacktextformat' => new external_value(PARAM_SEQUENCE, 'Format of feedback', VALUE_OPTIONAL),
+                    'mingrade' => new external_value(PARAM_TEXT, 'minimum mark', VALUE_OPTIONAL),
+                    'maxgrade' => new external_value(PARAM_TEXT, 'maximum mark', VALUE_OPTIONAL),
+                ]), '', VALUE_DEFAULT, []
             ),
         ]);
     }
@@ -106,10 +106,10 @@ public static function execute_returns(): external_single_structure {
      * @param array $quiz
      * @param array $sections
      * @param array $questions
-     * @param array $feedback
+     * @param array|null $feedback
      * @return object containing outcome
      */
-    public static function execute(array $quiz, array $sections, array $questions, array $feedback): object {
+    public static function execute(array $quiz, array $sections, array $questions, ?array $feedback = []): object {
         global $CFG, $DB;
         $params = self::validate_parameters(self::execute_parameters(), [
             'quiz' => $quiz,
@@ -239,16 +239,6 @@ public static function execute(array $quiz, array $sections, array $questions, a
             \question_make_default_categories([$quizcontext->context]);
         }
 
-        if (!count($params['feedback'])) {
-            $params['feedback'] = [
-                [
-                    'feedbacktext' => '',
-                    'feedbacktextformat' => 1,
-                    'mingrade' => 0,
-                    'maxgrade' => (float) $params['quiz']['grade'] + 1,
-                ],
-            ];
-        }
         foreach ($params['feedback'] as $feedback) {
             $feedback['quizid'] = $moduleinfo->instance;
             $DB->insert_record('quiz_feedback', $feedback);

classes/import_repo.php

@@ -890,8 +890,7 @@ public function update_quizzes($clihelper, $scriptdirectory) {
                 // Don't import from anything that isn't a directory or has a name in the wrong format.
                 continue;
             }
-            $x = array_column($quizlocations, null, 'directory');
-            $instanceid = array_column($quizlocations, null, 'directory')['/' . $quizdirectory]->moduleid ?? false;
+            $instanceid = array_column($quizlocations, null, 'directory')[$quizdirectory]->moduleid ?? false;
             $rootdirectory = dirname($basedirectory) . '/' . $quizdirectory;
             $quizfiles = scandir($rootdirectory);
             $structurefile = null;
@@ -1000,10 +999,10 @@ public function call_import_repo(string $rootdirectory, string $moodleinstance,
                                     ?string $quizmanifestname, ?string $quizcmid, string $scriptdirectory): string {
         chdir($scriptdirectory);
         if ($quizmanifestname) {
-            return shell_exec('php importrepotomoodle.php -w -r "' . $rootdirectory .
+            return shell_exec('php importrepotomoodle.php -u ' . $this->usegit . ' -w -r "' . $rootdirectory .
                             '" -i "' . $moodleinstance . '" -f "' . $quizmanifestname . '" -t ' . $token);
         } else {
-            return shell_exec('php importrepotomoodle.php -w -r "' . $rootdirectory .
+            return shell_exec('php importrepotomoodle.php -u ' . $this->usegit . ' -w -r "' . $rootdirectory .
                             '" -i "' . $moodleinstance . '" -l "module" -n ' . $quizcmid . ' -t ' . $token);
         }
     }
@@ -1023,11 +1022,13 @@ public function call_import_quiz_data(string $moodleinstance, string $token, str
                                     ?string $quizmanifestpath, string $structurefilepath, string $scriptdirectory): ?string {
         chdir($scriptdirectory);
         if ($quizmanifestpath) {
-            return shell_exec('php importquizstructuretomoodle.php -w -r "" -i "' . $moodleinstance . '" -n ' .
+            return shell_exec('php importquizstructuretomoodle.php -u ' . $this->usegit .
+                    ' -w -r "" -i "' . $moodleinstance . '" -n ' .
                     $instanceid . ' -t ' . $token. ' -p "' . $this->manifestpath . '" -f "' .
                     $quizmanifestpath . '" -a "' . $structurefilepath . '"');
         } else {
-            return shell_exec('php importquizstructuretomoodle.php -w -r "" -i "' . $moodleinstance . '" -n ' .
+            return shell_exec('php importquizstructuretomoodle.php -u ' . $this->usegit .
+                    ' -w -r "" -i "' . $moodleinstance . '" -n ' .
                     $instanceid . ' -t ' . $token. ' -p "' . $this->manifestpath. '" -a "' . $structurefilepath . '"');
         }
     }

cli/importquizstructuretomoodle.php

@@ -131,11 +131,11 @@
 
 $clihelper = new cli_helper($options);
 $importquiz = new import_quiz($clihelper, $moodleinstances);
-if ($importquiz->nonquizmanifestpath) {
+if ($importquiz->nonquizmanifestpath && empty($clihelper->get_arguments()['subcall'])) {
     echo "Checking repo...\n";
     $clihelper->check_for_changes($importquiz->nonquizmanifestpath);
 }
-if ($importquiz->quizmanifestpath) {
+if ($importquiz->quizmanifestpath && empty($clihelper->get_arguments()['subcall'])) {
     echo "Checking quiz repo...\n";
     $clihelper->check_for_changes($importquiz->quizmanifestpath);
 }

cli/importwholecoursetomoodle.php

@@ -77,10 +77,9 @@
         'longopt' => 'contextlevel',
         'shortopt' => 'l',
         'description' => 'Context from which to extract questions. Should always be course.',
-        'default' => 'course',
+        'default' => null,
         'variable' => 'contextlevel',
         'valuerequired' => true,
-        'hidden' => true,
     ],
     [
         'longopt' => 'coursename',
@@ -99,6 +98,15 @@
         'valuerequired' => true,
         'hidden' => true
     ],
+    [
+        'longopt' => 'coursecategory',
+        'shortopt' => 'g',
+        'description' => 'Not used.',
+        'default' => null,
+        'variable' => 'coursecategory',
+        'valuerequired' => true,
+        'hidden' => true,
+    ],
     [
         'longopt' => 'instanceid',
         'shortopt' => 'n',

doc/importwholecoursetomoodle.md

@@ -36,8 +36,8 @@ To update the questions in Moodle from the repo, point the script to your course
 `php importrepotomoodle.php -f "scratch-wholecourse/scratch-course"`
 
 Example 2:
-To import course context questions, quiz context questions and quiz structures into a course for the first time, you will need to point the script to the course directory and the course to import into (using either coursename or id).
-`php importwholecoursetomoodle.php -r "C:\question_repos" -d "scratch-wholecourse/scratch-course" -c "Course 2"`
+To import course context questions, quiz context questions and quiz structures into a course for the first time, you will need to point the script to the course directory and the course to import into (using either coursename -c or id -n). You will also need to specify context level but this will always be course (`-l "course"`).
+`php importwholecoursetomoodle.php -r "C:\question_repos" -d "scratch-wholecourse/scratch-course" -l "course" -c "Course 2"`
 
 ## Scenario 1: Importing questions into Moodle from an existing repo
 

tests/export_repo_test.php

@@ -96,6 +96,7 @@ public function setUp(): void {
             'token' => 'XXXXXX',
             'help' => false,
             'ignorecat' => null,
+            'usegit' => true,
         ];
         $this->clihelper = $this->getMockBuilder(\qbank_gitsync\cli_helper::class)->onlyMethods([
             'get_arguments', 'check_context',

tests/export_trait_test.php

@@ -72,6 +72,7 @@ public function setUp(): void {
             'token' => 'XXXXXX',
             'help' => false,
             'ignorecat' => null,
+            'usegit' => true,
         ];
         $this->clihelper = $this->getMockBuilder(\qbank_gitsync\cli_helper::class)->onlyMethods([
             'get_arguments', 'check_context',

tests/external/import_quiz_data_test.php

@@ -334,12 +334,7 @@ public function test_import_without_sections_and_feedback(): void {
         $this->assertEquals(0, $slot2->requireprevious);
 
         $feedback = $DB->get_records('quiz_feedback');
-        $this->assertEquals(1, count($feedback));
-        $feedback1 = array_shift($feedback);
-        $this->assertEquals('', $feedback1->feedbacktext);
-        $this->assertEquals(1, $feedback1->feedbacktextformat);
-        $this->assertEquals(0, $feedback1->mingrade);
-        $this->assertEquals(101, $feedback1->maxgrade);
+        $this->assertEquals(0, count($feedback));
     }
 
     /**

tests/import_repo_test.php

@@ -1526,7 +1526,7 @@ public function test_full_course_quiz_already_imported(): void {
         $this->rootpath . '/testrepo_quiz_quiz-1/fakeimport_module_course-1_quiz-1_question_manifest.json');
         $holder1 = new \StdClass();
         $holder1->moduleid = '1';
-        $holder1->directory = '/testrepo_quiz_quiz-1';
+        $holder1->directory = 'testrepo_quiz_quiz-1';
         $this->importrepo->manifestcontents->quizzes = [$holder1];
         $this->importrepo->update_quizzes($this->clihelper, $this->rootpath . '/testrepoparent');
 
@@ -1563,7 +1563,7 @@ public function test_full_course_quiz_in_moodle(): void {
         $this->rootpath . '/testrepo_quiz_quiz-1/fakeimport_module_course-1_quiz-1_question_manifest.json');
         $holder1 = new \StdClass();
         $holder1->moduleid = '1';
-        $holder1->directory = '/testrepo_quiz_quiz-1';
+        $holder1->directory = 'testrepo_quiz_quiz-1';
         $this->importrepo->manifestcontents->quizzes = [$holder1];
         $this->importrepo->update_quizzes($this->clihelper, $this->rootpath . '/testrepoparent');
 

tests/tidy_trait_test.php

@@ -72,6 +72,7 @@ public function setUp(): void {
             'token' => 'XXXXXX',
             'help' => false,
             'ignorecat' => null,
+            'usegit' => true,
         ];
         $this->clihelper = $this->getMockBuilder(\qbank_gitsync\cli_helper::class)->onlyMethods([
             'get_arguments', 'check_context',