MDL-63488 core_grade: Use the rewrite_pluginurls to handle files

* Use the rewrite_plugin_urls to handled embedded files
* Add phpunit tests for default and updated functionality of format_text

diff --git a/grade/export/lib.php b/grade/export/lib.php
index 6dabe7a..c05d31a 100644 (file)
--- a/grade/export/lib.php
+++ b/grade/export/lib.php
@@ -282,10 +282,24 @@ abstract class grade_export {
     /**
      * Returns formatted grade feedback
      * @param object $feedback object with properties feedback and feedbackformat
+     * @param object $grade Grade object with grade properties
      * @return string
      */
-    public function format_feedback($feedback) {
-        return strip_tags(format_text($feedback->feedback, $feedback->feedbackformat));
+    public function format_feedback($feedback, $grade = null) {
+        $string = $feedback->feedback;
+        if (!empty($grade)) {
+            // Rewrite links to get the export working for 36, refer MDL-63488.
+            $string = file_rewrite_pluginfile_urls(
+                $feedback->feedback,
+                'pluginfile.php',
+                $grade->get_context()->id,
+                GRADE_FILE_COMPONENT,
+                GRADE_FEEDBACK_FILEAREA,
+                $grade->id
+            );
+        }
+
+        return strip_tags(format_text($string, $feedback->feedbackformat));
     }
 
     /**

diff --git a/grade/export/ods/grade_export_ods.php b/grade/export/ods/grade_export_ods.php
index 5454456..40bbd90 100644 (file)
--- a/grade/export/ods/grade_export_ods.php
+++ b/grade/export/ods/grade_export_ods.php
@@ -116,7 +116,7 @@ class grade_export_ods extends grade_export {
 
                 // writing feedback if requested
                 if ($this->export_feedback) {
-                    $myxls->write_string($i, $j++, $this->format_feedback($userdata->feedbacks[$itemid]));
+                    $myxls->write_string($i, $j++, $this->format_feedback($userdata->feedbacks[$itemid], $grade));
                 }
             }
             // Time exported.

diff --git a/grade/export/txt/grade_export_txt.php b/grade/export/txt/grade_export_txt.php
index 7ec6aa4..e262d93 100644 (file)
--- a/grade/export/txt/grade_export_txt.php
+++ b/grade/export/txt/grade_export_txt.php
@@ -109,7 +109,7 @@ class grade_export_txt extends grade_export {
                 }
 
                 if ($this->export_feedback) {
-                    $exportdata[] = $this->format_feedback($userdata->feedbacks[$itemid]);
+                    $exportdata[] = $this->format_feedback($userdata->feedbacks[$itemid], $grade);
                 }
             }
             // Time exported.

diff --git a/grade/export/xls/grade_export_xls.php b/grade/export/xls/grade_export_xls.php
index 83e720d..f81b0e2 100644 (file)
--- a/grade/export/xls/grade_export_xls.php
+++ b/grade/export/xls/grade_export_xls.php
@@ -110,7 +110,7 @@ class grade_export_xls extends grade_export {
                 }
                 // writing feedback if requested
                 if ($this->export_feedback) {
-                    $myxls->write_string($i, $j++, $this->format_feedback($userdata->feedbacks[$itemid]));
+                    $myxls->write_string($i, $j++, $this->format_feedback($userdata->feedbacks[$itemid], $grade));
                 }
             }
             // Time exported.

diff --git a/grade/export/xml/grade_export_xml.php b/grade/export/xml/grade_export_xml.php
index c91122b..659fa6b 100644 (file)
--- a/grade/export/xml/grade_export_xml.php
+++ b/grade/export/xml/grade_export_xml.php
@@ -101,7 +101,7 @@ class grade_export_xml extends grade_export {
                 }
 
                 if ($this->export_feedback) {
-                    $feedbackstr = $this->format_feedback($userdata->feedbacks[$itemid]);
+                    $feedbackstr = $this->format_feedback($userdata->feedbacks[$itemid], $grade);
                     fwrite($handle,  "\t\t<feedback>$feedbackstr</feedback>\n");
                 }
                 fwrite($handle,  "\t</result>\n");

diff --git a/grade/tests/export_test.php b/grade/tests/export_test.php
index 9a16a71..b2cc79a 100644 (file)
--- a/grade/tests/export_test.php
+++ b/grade/tests/export_test.php
@@ -35,7 +35,7 @@ require_once($CFG->dirroot.'/grade/export/lib.php');
 class core_grade_export_test extends advanced_testcase {
 
     /**
-     * Ensure that feedback is correct formatted.
+     * Ensure that feedback is correct formatted. Test the default implementation of format_feedback
      *
      * @dataProvider    format_feedback_provider
      * @param   string  $input The input string to test
@@ -59,6 +59,71 @@ class core_grade_export_test extends advanced_testcase {
             );
     }
 
+    /**
+     * Ensure that feedback is correctly formatted. Test augmented functionality to handle file links
+     */
+    public function test_format_feedback_with_grade() {
+        $this->resetAfterTest();
+        $dg = $this->getDataGenerator();
+        $c1 = $dg->create_course();
+        $u1 = $dg->create_user();
+        $gi1a = new grade_item($dg->create_grade_item(['courseid' => $c1->id]), false);
+        $gi1a->update_final_grade($u1->id, 1, 'test');
+        $contextid = $gi1a->get_context()->id;
+        $gradeid = $gi1a->id;
+
+        $tests = [
+            'Has server based image (HTML)' => [
+                '<p>See this reference: <img src="@@PLUGINFILE@@/test.img"></p>',
+                FORMAT_HTML,
+                "See this reference: "
+            ],
+            'Has server based image and more (HTML)' => [
+                '<p>See <img src="@@PLUGINFILE@@/test.img"> for <em>reference</em></p>',
+                FORMAT_HTML,
+                "See  for reference"
+            ],
+            'Has server based video and more (HTML)' => [
+                '<p>See <video src="@@PLUGINFILE@@/test.img">video of a duck</video> for <em>reference</em></p>',
+                FORMAT_HTML,
+                'See video of a duck for reference'
+            ],
+            'Has server based video with text and more (HTML)' => [
+                '<p>See <video src="@@PLUGINFILE@@/test.img">@@PLUGINFILE@@/test.img</video> for <em>reference</em></p>',
+                FORMAT_HTML,
+                "See https://www.example.com/moodle/pluginfile.php/$contextid/grade/feedback/$gradeid/test.img for reference"
+            ],
+            'Multiple videos (HTML)' => [
+                '<p>See <video src="@@PLUGINFILE@@/test.img">video of a duck</video> and '.
+                '<video src="http://example.com/myimage.jpg">video of a cat</video> for <em>reference</em></p>',
+                FORMAT_HTML,
+                'See video of a duck and video of a cat for reference'
+            ],
+        ];
+
+        $feedback = $this->getMockForAbstractClass(
+            \grade_export::class,
+            [],
+            '',
+            false
+        );
+
+        foreach ($tests as $key => $testdetails) {
+            $expected = $testdetails[2];
+            $input = $testdetails[0];
+            $inputformat = $testdetails[1];
+
+            $this->assertEquals(
+                $expected,
+                $feedback->format_feedback((object) [
+                    'feedback' => $input,
+                    'feedbackformat' => $inputformat,
+                ], $gi1a),
+                $key
+            );
+        }
+    }
+
     /**
      * Data provider for the format_feedback tests.
      *
@@ -79,12 +144,12 @@ class core_grade_export_test extends advanced_testcase {
             'Has image (HTML)' => [
                 '<p>See this reference: <img src="http://example.com/myimage.jpg"></p>',
                 FORMAT_HTML,
-                'See this reference:  ',
+                'See this reference: ',
             ],
             'Has image and more (HTML)' => [
                 '<p>See <img src="http://example.com/myimage.jpg"> for <em>reference</em></p>',
                 FORMAT_HTML,
-                'See for reference',
+                'See  for reference',
             ],
             'Has video and more (HTML)' => [
                 '<p>See <video src="http://example.com/myimage.jpg">video of a duck</video> for <em>reference</em></p>',
@@ -92,15 +157,17 @@ class core_grade_export_test extends advanced_testcase {
                 'See video of a duck for reference',
             ],
             'Multiple videos (HTML)' => [
-                '<p>See <video src="http://example.com/myimage.jpg">video of a duck</video> and <video src="http://example.com/myimage.jpg">video of a cat</video> for <em>reference</em></p>',
+                '<p>See <video src="http://example.com/myimage.jpg">video of a duck</video> and '.
+                '<video src="http://example.com/myimage.jpg">video of a cat</video> for <em>reference</em></p>',
                 FORMAT_HTML,
-                'See video of a duck and video of a cat for reference',
+                'See video of a duck and video of a cat for reference'
             ],
             'HTML Looking tags in PLAIN' => [
                 'The way you have written the <img thing looks pretty fun >',
                 FORMAT_PLAIN,
-                'The way you have written the <img thing looks pretty fun >',
+                'The way you have written the &lt;img thing looks pretty fun &gt;',
             ],
+
         ];
     }
 }