Merge pull request #13 from maths/main

Update branch to main

Author: anst-i

.github/workflows/ci.yml

@@ -12,6 +12,12 @@ jobs:
           - php: '8.3'
             moodle-branch: 'main'
             database: 'pgsql'
+          - php: '8.3'
+            moodle-branch: 'MOODLE_501_STABLE'
+            database: 'pgsql'
+          - php: '8.3'
+            moodle-branch: 'MOODLE_500_STABLE'
+            database: 'pgsql'
           - php: '8.3'
             moodle-branch: 'MOODLE_405_STABLE'
             database: 'pgsql'
@@ -33,7 +39,7 @@ jobs:
 
     services:
       postgres:
-        image: postgres:14
+        image: postgres:16
         env:
           POSTGRES_USER: 'postgres'
           POSTGRES_HOST_AUTH_METHOD: 'trust'
@@ -42,7 +48,7 @@ jobs:
         options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 3
 
       mariadb:
-        image: mariadb:10
+        image: mariadb:10.6
         env:
           MYSQL_USER: 'root'
           MYSQL_ALLOW_EMPTY_PASSWORD: "true"

CHANGES.md

@@ -1,5 +1,19 @@
 # Change log for the Import question as new version question bank plugin
 
+## Changes in version 1.4
+
+* Changed to default page style instead of popup style
+* Added pass through of error messages and notices during import
+* Add verified compatibility with Moodle 5.1
+
+## Changes in version 1.3
+
+* Add verified compatibility with Moodle 5.0
+
+## Changes in version 1.2
+
+* Add verified compatibility with Moodle 4.4 and 4.5
+
 ## Changes in version 1.1
 
 * Provide compatibility with Moodle 4.3.

README.md

@@ -16,12 +16,18 @@ Install from the Moodle plugins database https://moodle.org/plugins/qbank_import
 
 ### Install using git
 
-Or you can install using git. Type this commands in the root of your Moodle install
+Or you can install using git. For Moodle versions up to 5.0, type these commands in the root folder of your Moodle install:
 
     git clone https://github.com/maths/moodle-qbank_importasversion.git question/bank/importasversion
     echo /question/bank/importasversion/ >> .git/info/exclude
 
+Moodle 5.1 introduced a new folder structure. From the root folder, type these commands:
+
+    git clone https://github.com/maths/moodle-qbank_importasversion.git public/question/bank/importasversion
+    echo /public/question/bank/importasversion/ >> .git/info/exclude
+
 Then run the moodle update process
+
 Site administration > Notifications
 
 

classes/event/question_version_imported.php

@@ -36,7 +36,6 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class question_version_imported extends question_base {
-
     #[\Override]
     protected function init() {
         parent::init();

classes/form/import_form.php

@@ -42,13 +42,17 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class import_form extends moodleform {
-
     #[\Override]
     protected function definition() {
         $mform = $this->_form;
 
-        $mform->addElement('filepicker', 'newfile', get_string('import'),
-                null, ['accepted_types' => '.xml']);
+        $mform->addElement(
+            'filepicker',
+            'newfile',
+            get_string('import'),
+            null,
+            ['accepted_types' => '.xml']
+        );
         $mform->addRule('newfile', null, 'required', null, 'client');
 
         $mform->addElement('hidden', 'id');

classes/import_as_version_action_column.php

@@ -21,12 +21,16 @@
 // Support multiple Moodle versions. This can be cleaned up once 4.3 is the lowest supported version.
 if (!class_exists('\core_question\local\bank\question_action_base')) {
     // Moodle up to 4.2.x.
-    class_alias('\core_question\local\bank\menu_action_column_base',
-            'qbank_importasversion\qbank_importasversion_column_parent_class');
+    class_alias(
+        '\core_question\local\bank\menu_action_column_base',
+        'qbank_importasversion\qbank_importasversion_column_parent_class'
+    );
 } else {
     // Moodle 4.3+.
-    class_alias('\core_question\local\bank\question_action_base',
-            'qbank_importasversion\qbank_importasversion_column_parent_class');
+    class_alias(
+        '\core_question\local\bank\question_action_base',
+        'qbank_importasversion\qbank_importasversion_column_parent_class'
+    );
 }
 
 /**
@@ -37,7 +41,6 @@ class_alias('\core_question\local\bank\question_action_base',
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class import_as_version_action_column extends qbank_importasversion_column_parent_class {
-
     /** @var string store the value of the name lang string for performance. */
     protected $actionname;
 

classes/importer.php

@@ -39,37 +39,43 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class importer extends qformat_xml {
-
     /**
      * Import a file to update a given question.
      *
      * @param qformat_xml $qformat an instance of
      * @param question_definition $question the question to add a version to.
      * @param string $importedquestionfile filename of the file to import.
-     * @return bool true on success. False on failure.
+     * @return object|boolean Either a simple object with error and/or notice properties when there are issues
+     * or true on success.
      */
     public static function import_file(
         qformat_xml $qformat,
         question_definition $question,
         string $importedquestionfile
     ) {
-        global $USER, $DB, $OUTPUT;
+        global $USER, $DB;
 
         $context = context::instance_by_id($question->contextid);
 
         // STAGE 1: Parse the file.
         if (! $importedlines = $qformat->readdata($importedquestionfile)) {
-            throw new exception(get_string('cannotread', 'question'));
+            $result = new stdClass();
+            $result->error = get_string('cannotread', 'question');
+            return $result;
         }
 
         // Extract all the questions.
         if (!$importedquestions = $qformat->readquestions($importedlines)) {
-            throw new exception(get_string('noquestionsinfile', 'question'));
+            $result = new stdClass();
+            $result->error = get_string('noquestionsinfile', 'question');
+            return $result;
         }
 
         // Check if there's only one question in the file -- remove this once we do batch processing!
         if (count($importedquestions) != 1) {
-            throw new exception(get_string('toomanyquestionsinfile', 'qbank_importasversion'));
+            $result = new stdClass();
+            $result->error = get_string('toomanyquestionsinfile', 'qbank_importasversion');
+            return $result;
         }
 
         // Count number of questions processed.
@@ -111,23 +117,45 @@ public static function import_file(
         $questionversion->id = $DB->insert_record('question_versions', $questionversion);
 
         if (isset($newquestion->questiontextitemid)) {
-            $newquestion->questiontext = file_save_draft_area_files($newquestion->questiontextitemid,
-                    $context->id, 'question', 'questiontext', $newquestion->id,
-                    $fileoptions, $newquestion->questiontext);
+            $newquestion->questiontext = file_save_draft_area_files(
+                $newquestion->questiontextitemid,
+                $context->id,
+                'question',
+                'questiontext',
+                $newquestion->id,
+                $fileoptions,
+                $newquestion->questiontext
+            );
         } else if (isset($newquestion->questiontextfiles)) {
             foreach ($newquestion->questiontextfiles as $file) {
                 question_bank::get_qtype($newquestion->qtype)->import_file(
-                        $context, 'question', 'questiontext', $newquestion->id, $file);
+                    $context,
+                    'question',
+                    'questiontext',
+                    $newquestion->id,
+                    $file
+                );
             }
         }
         if (isset($newquestion->generalfeedbackitemid)) {
-            $newquestion->generalfeedback = file_save_draft_area_files($newquestion->generalfeedbackitemid,
-                    $context->id, 'question', 'generalfeedback', $newquestion->id,
-                    $fileoptions, $newquestion->generalfeedback);
+            $newquestion->generalfeedback = file_save_draft_area_files(
+                $newquestion->generalfeedbackitemid,
+                $context->id,
+                'question',
+                'generalfeedback',
+                $newquestion->id,
+                $fileoptions,
+                $newquestion->generalfeedback
+            );
         } else if (isset($newquestion->generalfeedbackfiles)) {
             foreach ($newquestion->generalfeedbackfiles as $file) {
                 question_bank::get_qtype($newquestion->qtype)->import_file(
-                        $context, 'question', 'generalfeedback', $newquestion->id, $file);
+                    $context,
+                    'question',
+                    'generalfeedback',
+                    $newquestion->id,
+                    $file
+                );
             }
         }
         $DB->update_record('question', $newquestion);
@@ -148,29 +176,48 @@ public static function import_file(
                 if ($isimportingcontextcourseoractivity) {
                     $mergedtags = array_merge($newquestion->coursetags, $newquestion->tags);
 
-                    core_tag_tag::set_item_tags('core_question', 'question', $newquestion->id,
-                        $newquestion->context, $mergedtags);
+                    core_tag_tag::set_item_tags(
+                        'core_question',
+                        'question',
+                        $newquestion->id,
+                        $newquestion->context,
+                        $mergedtags
+                    );
                 } else {
-                    core_tag_tag::set_item_tags('core_question', 'question', $newquestion->id,
-                        context_course::instance($qformat->course->id), $newquestion->coursetags);
+                    core_tag_tag::set_item_tags(
+                        'core_question',
+                        'question',
+                        $newquestion->id,
+                        context_course::instance($qformat->course->id),
+                        $newquestion->coursetags
+                    );
 
                     if (!empty($newquestion->tags)) {
-                        core_tag_tag::set_item_tags('core_question', 'question', $newquestion->id,
-                            $importingcontext, $newquestion->tags);
+                        core_tag_tag::set_item_tags(
+                            'core_question',
+                            'question',
+                            $newquestion->id,
+                            $importingcontext,
+                            $newquestion->tags
+                        );
                     }
                 }
             } else if (!empty($newquestion->tags)) {
-                core_tag_tag::set_item_tags('core_question', 'question', $newquestion->id,
-                    $newquestion->context, $newquestion->tags);
+                core_tag_tag::set_item_tags(
+                    'core_question',
+                    'question',
+                    $newquestion->id,
+                    $newquestion->context,
+                    $newquestion->tags
+                );
             }
         }
 
         if (!empty($result->error)) {
-            echo $OUTPUT->notification($result->error);
             // Can't use $transaction->rollback(); since it requires an exception,
             // and I don't want to rewrite this code to change the error handling now.
             $DB->force_transaction_rollback();
-            return false;
+            return $result;
         }
 
         question_version_imported::create([
@@ -185,11 +232,19 @@ public static function import_file(
 
         $transaction->allow_commit();
 
-        if (!empty($result->notice)) {
-            echo $OUTPUT->notification($result->notice);
-            return true;
+        if ($result === null) {
+            // Some question types don't have a return value when saving options.
+            // If it hasn't thrown an Exception then it's fine.
+            $result = true;
+        }
+        if ($result === false) {
+            // This probably shouldn't happen but given all the question types out there
+            // it's probably worth making sure we're handling it.
+            $result = new stdClass();
+            $result->error = get_string('unknownerror', 'qbank_importasversion');
+            return $result;
         }
 
-        return true;
+        return $result;
     }
 }

classes/plugin_feature.php

@@ -27,7 +27,6 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class plugin_feature extends plugin_features_base {
-
     #[\Override]
     public function get_question_actions(view $qbank): array {
         // This is what is used in Moodle 4.3+.

classes/privacy/provider.php

@@ -27,7 +27,6 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class provider implements null_provider {
-
     #[\Override]
     public static function get_reason(): string {
         return 'privacy:metadata';