refactor: delete options files the way god intended

Author: MHajoha

classes/constants.php

@@ -37,6 +37,11 @@ class constants {
     /** @var string */
     public const QT_VAR_RESPONSE = 'qpy_response';
 
+    /** @var string */
+    public const FILEAREA_OPTIONS = 'options';
+    /** @var string */
+    public const FILEAREA_STATIC = 'static';
+
     /** @var string */
     public const QPY_OPTIONS_URL_PATTERN = ';qpy://options/(?P<fileref>[a-zA-Z0-9\-_=]{1,64});';
 

classes/local/files/options_file_service.php

@@ -23,6 +23,7 @@
 use file_exception;
 use moodle_exception;
 use moodle_url;
+use qtype_questionpy\constants;
 use qtype_questionpy\local\form\elements\file_upload_element;
 use qtype_questionpy\local\form\elements\file_upload_options;
 use qtype_questionpy_question;
@@ -40,10 +41,6 @@
 class options_file_service implements handles_qpy_url_type {
     // TODO: Support subdirectories.
 
-    /** @var string */
-    public const FILEAREA_UPLOADS = 'options';
-
-
     /**
      * Saves all the files in the given draft item to the permanent file area for the given question.
      *
@@ -61,7 +58,7 @@ public function save_draft_area_files(int $contextid, int $questionid, int $user
         $existingfiles = $fs->get_area_files(
             $contextid,
             'qtype_questionpy',
-            self::FILEAREA_UPLOADS,
+            constants::FILEAREA_OPTIONS,
             $questionid,
             includedirs: false
         );
@@ -85,18 +82,6 @@ public function save_draft_area_files(int $contextid, int $questionid, int $user
         }
     }
 
-    /**
-     * Deletes all files in the permanent file area of the given question.
-     *
-     * @param int $contextid
-     * @param int $questionid
-     * @return void
-     */
-    public function delete_all_saved_files_belonging_to_question(int $contextid, int $questionid): void {
-        $fs = get_file_storage();
-        $fs->delete_area_files($contextid, 'qtype_questionpy', self::FILEAREA_UPLOADS, $questionid);
-    }
-
     /**
      * Populates the given draft area with files listed in `$filemetas` and stored in the permanent question file area.
      *
@@ -113,7 +98,7 @@ public function delete_all_saved_files_belonging_to_question(int $contextid, int
      */
     public function prepare_draft_area(int $contextid, int $questionid, array $filemetas, int $userid, int $draftitemid): void {
         $fs = get_file_storage();
-        $files = $fs->get_area_files($contextid, 'qtype_questionpy', self::FILEAREA_UPLOADS, $questionid, includedirs: false);
+        $files = $fs->get_area_files($contextid, 'qtype_questionpy', constants::FILEAREA_OPTIONS, $questionid, includedirs: false);
 
         foreach ($filemetas as $filemetadata) {
             $matchingfiles = array_filter($files, fn($file) => $file->get_filename() === $filemetadata->fileref);
@@ -223,7 +208,7 @@ public function check_upload_restrictions(
         $existingfiles = $questionid ? $fs->get_area_files(
             $contextid,
             'qtype_questionpy',
-            self::FILEAREA_UPLOADS,
+            constants::FILEAREA_OPTIONS,
             $questionid,
             includedirs: false
         ) : [];
@@ -258,7 +243,7 @@ public function check_upload_restrictions(
      */
     public function get_saved_file(int $contextid, int $questionid, string $fileref): ?stored_file {
         $fs = get_file_storage();
-        $files = $fs->get_area_files($contextid, 'qtype_questionpy', self::FILEAREA_UPLOADS, $questionid, includedirs: false);
+        $files = $fs->get_area_files($contextid, 'qtype_questionpy', constants::FILEAREA_OPTIONS, $questionid, includedirs: false);
         foreach ($files as $file) {
             if ($file->get_filename() === $fileref) {
                 return $file;
@@ -296,7 +281,7 @@ public function resolve_qpy_url(string $path, qtype_questionpy_question $questio
         return moodle_url::make_pluginfile_url(
             $question->contextid,
             'qtype_questionpy',
-            self::FILEAREA_UPLOADS,
+            constants::FILEAREA_OPTIONS,
             $question->id,
             '/',
             $path

classes/local/files/static_file_service.php

@@ -22,6 +22,7 @@
 use GuzzleHttp\Exception\GuzzleException;
 use invalid_dataroot_permissions;
 use moodle_url;
+use qtype_questionpy\constants;
 use qtype_questionpy\exception\request_error;
 use qtype_questionpy\local\api\api;
 use qtype_questionpy\package_file_service;
@@ -104,7 +105,7 @@ public function resolve_qpy_url(string $path, qtype_questionpy_question $questio
         return moodle_url::make_pluginfile_url(
             $question->contextid,
             'qtype_questionpy',
-            'static',
+            constants::FILEAREA_STATIC,
             null,
             '/' . $question->packagehash . dirname($path) . '/',
             basename($path)

classes/question_service.php

@@ -224,14 +224,11 @@ public function upsert_question(object $question): void {
      * Deletes all QuestionPy-specific data for the given question.
      *
      * @param int $questionid
-     * @param int $contextid The context this question belongs to.
      * @throws dml_exception
      */
-    public function delete_question(int $questionid, int $contextid) {
+    public function delete_question(int $questionid) {
         global $DB;
         $DB->delete_records(self::QUESTION_TABLE, ['questionid' => $questionid]);
         // TODO: Also delete packages when they are no longer used by any question.
-
-        $this->ofs->delete_all_saved_files_belonging_to_question($contextid, $questionid);
     }
 }

questiontype.php

@@ -23,7 +23,9 @@
  */
 
 use core\di;
+use qtype_questionpy\constants;
 use qtype_questionpy\local\api\api;
+use qtype_questionpy\local\files\options_file_service;
 use qtype_questionpy\package_file_service;
 use qtype_questionpy\question_service;
 
@@ -49,6 +51,9 @@ class qtype_questionpy extends question_type {
     /** @var question_service */
     private question_service $questionservice;
 
+    /** @var options_file_service */
+    private options_file_service $ofs;
+
     /**
      * Initializes the instance. Called by Moodle.
      */
@@ -57,6 +62,7 @@ public function __construct() {
         $this->api = di::get(api::class);
         $this->packagefileservice = di::get(package_file_service::class);
         $this->questionservice = di::get(question_service::class);
+        $this->ofs = di::get(options_file_service::class);
     }
 
     /**
@@ -88,7 +94,7 @@ public function is_question_manual_graded($question, $otherquestionsinuse) {
      * @throws moodle_exception
      */
     public function delete_question($questionid, $contextid): void {
-        $this->questionservice->delete_question($questionid, $contextid);
+        $this->questionservice->delete_question($questionid);
         parent::delete_question($questionid, $contextid);
     }
 
@@ -116,13 +122,16 @@ public function move_files($questionid, $oldcontextid, $newcontextid) {
      * @param int $contextid
      * @return void
      */
-    protected function delete_files($questionid, $contextid) {
+    protected function delete_files($questionid, $contextid): void {
         parent::delete_files($questionid, $contextid);
         $this->delete_files_in_answers($questionid, $contextid);
         $this->delete_files_in_hints($questionid, $contextid);
 
         $fs = get_file_storage();
         $fs->delete_area_files($contextid, 'qtype_questionpy', 'package', $questionid);
+
+        $fs1 = get_file_storage();
+        $fs1->delete_area_files($contextid, 'qtype_questionpy', constants::FILEAREA_OPTIONS, $questionid);
     }
 
     /**

tests/question_service_test.php

@@ -434,7 +434,7 @@ public function test_delete_question(): void {
         global $DB, $PAGE;
         $this->assertEquals(1, $DB->count_records('qtype_questionpy'));
 
-        $this->questionservice->delete_question(1, $PAGE->context->id);
+        $this->questionservice->delete_question(1);
 
         $this->assertEquals(0, $DB->count_records('qtype_questionpy'));
     }