MDL-68943 assignfeedback_editpdf: Reconvert updated files

moodle · Oct 28, 2022 · 3209423 · 3209423
1 parent dbb2dee
commit 3209423
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 5 deletions.
diff --git a/mod/assign/feedback/editpdf/lib.php b/mod/assign/feedback/editpdf/lib.php
@@ -101,3 +101,34 @@ function assignfeedback_editpdf_pluginfile(
     }
 
 }
+
+/**
+ * Files API hook to remove stale conversion records.
+ *
+ * When a file is update, its contenthash will change, but its ID
+ * remains the same. The document converter API records source file
+ * IDs and destination file IDs. When a file is updated, the document
+ * converter API has no way of knowing that the content of the file
+ * has changed, so it just serves the previously stored destination
+ * file.
+ *
+ * In this hook we check if the contenthash has changed, and if it has
+ * we delete the existing conversion so that a new one will be created.
+ *
+ * @param stdClass $file The updated file record.
+ * @param stdClass $filepreupdate The file record pre-update.
+ */
+function assignfeedback_editpdf_after_file_updated(stdClass $file, stdClass $filepreupdate) {
+    $contenthashchanged = $file->contenthash !== $filepreupdate->contenthash;
+    if ($contenthashchanged && $file->component == 'assignsubmission_file' && $file->filearea == 'submission_files') {
+        $fs = get_file_storage();
+        $file = $fs->get_file_by_id($file->id);
+        $conversions = \core_files\conversion::get_conversions_for_file($file, 'pdf');
+
+        foreach ($conversions as $conversion) {
+            if ($conversion->get('id')) {
+                $conversion->delete();
+            }
+        }
+    }
+}
diff --git a/mod/assign/feedback/editpdf/tests/feedback_test.php b/mod/assign/feedback/editpdf/tests/feedback_test.php
@@ -47,7 +47,14 @@ protected function require_ghostscript() {
         }
     }
 
-    protected function add_file_submission($student, $assign) {
+    /**
+     * Helper method to add a file to a submission.
+     *
+     * @param stdClass $student Student submitting.
+     * @param assign   $assign Assignment being submitted.
+     * @param bool     $textfile Use textfile fixture instead of pdf.
+     */
+    protected function add_file_submission($student, $assign, $textfile = false) {
         global $CFG;
 
         $this->setUser($student);
@@ -56,16 +63,16 @@ protected function add_file_submission($student, $assign) {
         $submission = $assign->get_user_submission($student->id, true);
 
         $fs = get_file_storage();
-        $pdfsubmission = (object) array(
+        $filerecord = (object) array(
             'contextid' => $assign->get_context()->id,
             'component' => 'assignsubmission_file',
             'filearea' => ASSIGNSUBMISSION_FILE_FILEAREA,
             'itemid' => $submission->id,
             'filepath' => '/',
-            'filename' => 'submission.pdf'
+            'filename' => $textfile ? 'submission.txt' : 'submission.pdf'
         );
-        $sourcefile = $CFG->dirroot.'/mod/assign/feedback/editpdf/tests/fixtures/submission.pdf';
-        $fs->create_file_from_pathname($pdfsubmission, $sourcefile);
+        $sourcefile = $CFG->dirroot . '/mod/assign/feedback/editpdf/tests/fixtures/submission.' . ($textfile ? 'txt' : 'pdf');
+        $fs->create_file_from_pathname($filerecord, $sourcefile);
 
         $data = new \stdClass();
         $plugin = $assign->get_submission_plugin_by_type('file');
@@ -554,4 +561,60 @@ public function test_conversion_task_limit() {
         // Confirm, that 100 records were processed and 10 were left for the next task run.
         $this->assertEquals(10, $DB->count_records('assignfeedback_editpdf_queue'));
     }
+
+    /**
+     * Test that overwriting a submission file deletes any associated conversions.
+     *
+     * @covers \core_files\conversion::get_conversions_for_file
+     */
+    public function test_submission_file_overridden() {
+        $this->resetAfterTest();
+        $course = $this->getDataGenerator()->create_course();
+        $student = $this->getDataGenerator()->create_and_enrol($course, 'student');
+        $assign = $this->create_instance($course, [
+            'assignsubmission_onlinetext_enabled' => 1,
+            'assignsubmission_file_enabled' => 1,
+            'assignsubmission_file_maxfiles' => 1,
+            'assignfeedback_editpdf_enabled' => 1,
+            'assignsubmission_file_maxsizebytes' => 1000000,
+        ]);
+
+        $this->add_file_submission($student, $assign, true);
+        $submission = $assign->get_user_submission($student->id, true);
+
+        $fs = get_file_storage();
+        $sourcefile = $fs->get_file(
+            $assign->get_context()->id,
+            'assignsubmission_file',
+            ASSIGNSUBMISSION_FILE_FILEAREA,
+            $submission->id,
+            '/',
+            'submission.txt'
+        );
+
+        $conversion = new \core_files\conversion(0, (object)[
+            'sourcefileid' => $sourcefile->get_id(),
+            'targetformat' => 'pdf'
+        ]);
+        $conversion->create();
+
+        $conversions = \core_files\conversion::get_conversions_for_file($sourcefile, 'pdf');
+        $this->assertCount(1, $conversions);
+
+        $filerecord = (object)[
+            'contextid' => $assign->get_context()->id,
+            'component' => 'core',
+            'filearea'  => 'unittest',
+            'itemid'    => $submission->id,
+            'filepath'  => '/',
+            'filename'  => 'submission.txt'
+        ];
+
+        $fs = get_file_storage();
+        $newfile = $fs->create_file_from_string($filerecord, 'something totally different');
+        $sourcefile->replace_file_with($newfile);
+
+        $conversions = \core_files\conversion::get_conversions_for_file($sourcefile, 'pdf');
+        $this->assertCount(0, $conversions);
+    }
 }
diff --git a/mod/assign/feedback/editpdf/tests/fixtures/submission.txt b/mod/assign/feedback/editpdf/tests/fixtures/submission.txt
@@ -0,0 +1,3 @@
+你行你上啊！
+
+不行别BB