MDL-68338 qformat_gift: support idnumbers and tags
authorTim Hunt <T.J.Hunt@open.ac.uk>
Sun, 5 Apr 2020 16:49:41 +0000 (17:49 +0100)
committerTim Hunt <T.J.Hunt@open.ac.uk>
Sun, 5 Apr 2020 16:49:41 +0000 (17:49 +0100)
Hopefully the new format is forwards and backwards compatible.

question/format/gift/format.php
question/format/gift/tests/giftformat_test.php

index d66f9e7..763e8a5 100644 (file)
@@ -163,18 +163,18 @@ class qformat_gift extends qformat_default {
         // converts it into a question object suitable for processing and insertion into Moodle.
 
         $question = $this->defaultquestion();
-        $comment = null;
         // Define replaced by simple assignment, stop redefine notices.
         $giftanswerweightregex = '/^%\-*([0-9]{1,2})\.?([0-9]*)%/';
 
-        // REMOVED COMMENTED LINES and IMPLODE.
+        // Separate comments and implode.
+        $comments = '';
         foreach ($lines as $key => $line) {
             $line = trim($line);
             if (substr($line, 0, 2) == '//') {
+                $comments .= $line . "\n";
                 $lines[$key] = ' ';
             }
         }
-
         $text = trim(implode("\n", $lines));
 
         if ($text == '') {
@@ -313,6 +313,10 @@ class qformat_gift extends qformat_default {
             }
         }
 
+        // Extract any idnumber and tags from the comments.
+        list($question->idnumber, $question->tags) =
+                $this->extract_idnumber_and_tags_from_comment($comments);
+
         if (!isset($question->qtype)) {
             $giftqtypenotset = get_string('giftqtypenotset', 'qformat_gift');
             $this->error($giftqtypenotset, $text);
@@ -600,6 +604,55 @@ class qformat_gift extends qformat_default {
         }
     }
 
+    /**
+     * Extract any tags or idnumber declared in the question comment.
+     *
+     * @param string $comment E.g. "// Line 1.\n//Line 2.\n".
+     * @return array with two elements. string $idnumber (or '') and string[] of tags.
+     */
+    public function extract_idnumber_and_tags_from_comment(string $comment): array {
+
+        // Find the idnumber, if any. There should not be more than one, but if so, we just find the first.
+        $idnumber = '';
+        if (preg_match('~
+                # Start of id token.
+                \[id:
+
+                # Any number of (non-control) characters, with any ] escaped.
+                # This is the bit we want so capture it.
+                (
+                    (?:\\\\]|[^][:cntrl:]])+
+                )
+
+                # End of id token.
+                ]
+                ~x', $comment, $match)) {
+            $idnumber = str_replace('\]', ']', trim($match[1]));
+        }
+
+        // Find any tags.
+        $tags = [];
+        if (preg_match_all('~
+                # Start of tag token.
+                \[tag:
+
+                # Any number of allowed characters (see PARAM_TAG), with any ] escaped.
+                # This is the bit we want so capture it.
+                (
+                    (?:\\\\]|[^]<>`[:cntrl:]]|)+
+                )
+
+                # End of tag token.
+                ]
+                ~x', $comment, $matches)) {
+            foreach ($matches[1] as $rawtag) {
+                $tags[] = str_replace('\]', ']', trim($rawtag));
+            }
+        }
+
+        return [$idnumber, $tags];
+    }
+
     public function write_name($name) {
         return '::' . $this->repchar($name) . '::';
     }
@@ -635,10 +688,10 @@ class qformat_gift extends qformat_default {
     }
 
     public function writequestion($question) {
-        global $OUTPUT;
 
         // Start with a comment.
         $expout = "// question: {$question->id}  name: {$question->name}\n";
+        $expout .= $this->write_idnumber_and_tags($question);
 
         // Output depends on question type.
         switch($question->qtype) {
@@ -775,4 +828,47 @@ class qformat_gift extends qformat_default {
         $expout .= "\n";
         return $expout;
     }
+
+    /**
+     * Prepare any question idnumber or tags for export.
+     *
+     * @param stdClass $questiondata the question data we are exporting.
+     * @return string a string that can be written as a line in the GIFT file,
+     *      e.g. "// [id:myid] [tag:some-tag]\n". Will be '' if none.
+     */
+    public function write_idnumber_and_tags(stdClass $questiondata): string {
+        if ($questiondata->qtype == 'category') {
+            return '';
+        }
+
+        $bits = [];
+
+        if (isset($questiondata->idnumber) && $questiondata->idnumber !== '') {
+            $bits[] = '[id:' . str_replace(']', '\]', $questiondata->idnumber) . ']';
+        }
+
+        // Write the question tags.
+        if (core_tag_tag::is_enabled('core_question', 'question')) {
+            $tagobjects = core_tag_tag::get_item_tags('core_question', 'question', $questiondata->id);
+
+            if (!empty($tagobjects)) {
+                $context = context::instance_by_id($questiondata->contextid);
+                $sortedtagobjects = question_sort_tags($tagobjects, $context, [$this->course]);
+
+                // Currently we ignore course tags. This should probably be fixed in future.
+
+                if (!empty($sortedtagobjects->tags)) {
+                    foreach ($sortedtagobjects->tags as $tag) {
+                        $bits[] = '[tag:' . str_replace(']', '\]', $tag) . ']';
+                    }
+                }
+            }
+        }
+
+        if (!$bits) {
+            return '';
+        }
+
+        return '// ' . implode(' ', $bits) . "\n";
+    }
 }
index 90c89ef..92251cc 100644 (file)
@@ -754,7 +754,6 @@ class qformat_gift_test extends question_testcase {
             'options' => (object) array(
                 'id' => 123,
                 'question' => 666,
-                'showunits' => 0,
                 'unitsleft' => 0,
                 'showunits' => 2,
                 'unitgradingtype' => 0,
@@ -1294,4 +1293,102 @@ FALSE#42 is the Ultimate Answer.#You gave the right answer.}";
 
         $this->assert(new question_check_specified_fields_expectation($expectedq), $q);
     }
+
+    public function test_import_question_with_tags() {
+        $gift = '
+// This question is to test importing tags: [tag:tag] [tag:other-tag].
+// And an idnumber: [id:myid].
+::Question name:: How are you? {}';
+        $lines = preg_split('/[\\n\\r]/', str_replace("\r\n", "\n", $gift));
+
+        $importer = new qformat_gift();
+        $q = $importer->readquestion($lines);
+
+        $expectedq = (object) array(
+            'name' => 'Question name',
+            'questiontext' => 'How are you?',
+            'questiontextformat' => FORMAT_MOODLE,
+            'generalfeedback' => '',
+            'generalfeedbackformat' => FORMAT_MOODLE,
+            'qtype' => 'essay',
+            'defaultmark' => 1,
+            'penalty' => 0.3333333,
+            'length' => 1,
+            'responseformat' => 'editor',
+            'responsefieldlines' => 15,
+            'attachments' => 0,
+            'graderinfo' => array(
+                'text' => '',
+                'format' => FORMAT_HTML,
+                'files' => array()),
+            'tags' => ['tag', 'other-tag'],
+            'idnumber' => 'myid',
+        );
+
+        $this->assert(new question_check_specified_fields_expectation($expectedq), $q);
+    }
+
+    /**
+     * Data provider for test_extract_idnumber_and_tags_from_comment.
+     *
+     * @return array the test cases.
+     */
+    public function extract_idnumber_and_tags_from_comment_testcases() {
+        return [
+            'blank comment' => ['', [], ''],
+            'nothing in comment' => ['', [], '// A basic comment.'],
+            'idnumber only' => ['frog', [], '// A comment with [id:frog] <-- an idnumber.'],
+            'tags only' => ['', ['frog', 'toad'], '// Look tags: [tag:frog] [tag:toad].'],
+            'everything' => ['four', ['add', 'basic'], '// [tag:add] [tag:basic] [id:four]'],
+            'everything mixed up' => ['four', ['basic', 'add'],
+                    "// [tag:  basic] Here is  \n// a [id:   four   ] que[tag:add   ]stion."],
+            'split over line' => ['', [], "// Ceci n\'est pas une [tag:\n\\ frog]."],
+            'escape ] idnumber' => ['i]d', [], '// [id:i\]d].'],
+            'escape ] tag' => ['', ['t]ag'], '// [tag:t\]ag].'],
+        ];
+    }
+
+    /**
+     * Test extract_idnumber_and_tags_from_comment.
+     *
+     * @dataProvider extract_idnumber_and_tags_from_comment_testcases
+     * @param string $expectedidnumber the expected idnumber.
+     * @param array $expectedtags the expected tags.
+     * @param string $comment the comment to parse.
+     */
+    public function test_extract_idnumber_and_tags_from_comment(
+            string $expectedidnumber, array $expectedtags, string $comment) {
+        $importer = new qformat_gift();
+
+        list($idnumber, $tags) = $importer->extract_idnumber_and_tags_from_comment($comment);
+        $this->assertSame($expectedidnumber, $idnumber);
+        $this->assertSame($expectedtags, $tags);
+    }
+
+    public function test_export_question_with_tags_and_idnumber() {
+        $this->resetAfterTest();
+
+        // Create a question with tags.
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_question');
+        $category = $generator->create_question_category();
+        $question = $generator->create_question('truefalse', null,
+                ['category' => $category->id, 'idnumber' => 'myid']);
+        core_tag_tag::set_item_tags('core_question', 'question', $question->id,
+                context::instance_by_id($category->contextid), ['tag1', 'tag2'], 0);
+
+        // Export it.
+        $questiondata = question_bank::load_question_data($question->id);
+        $exporter = new qformat_gift();
+        $exporter->course = get_course(SITEID);
+        $gift = $exporter->writequestion($questiondata);
+
+        // Verify.
+        $expectedgift = "// question: {$question->id}  name: True/false question
+// [id:myid] [tag:tag1] [tag:tag2]
+::True/false question::[html]The answer is true.{TRUE#This is the wrong answer.#This is the right answer.####You should have selected true.}
+
+";
+
+        $this->assert_same_gift($expectedgift, $gift);
+    }
 }