feat: allow editing of packages removed from the database

questionpy-org · Oct 23, 2024 · dff56c5 · dff56c5
1 parent 7608d9b
commit dff56c5
Show file tree

Hide file tree

Showing 9 changed files with 148 additions and 93 deletions.
diff --git a/classes/api/api.php b/classes/api/api.php
@@ -78,6 +78,20 @@ public function package(string $hash, ?stored_file $file = null): package_api {
         return new package_api($this->client, $hash, $file);
     }
 
+    /**
+     * Get a {@see package_raw} from a package hash.
+     *
+     * @param string $hash
+     * @return package_raw
+     * @throws moodle_exception
+     */
+    public function get_package_info(string $hash): package_raw {
+        $connector = connector::default();
+        $response = $connector->get("/packages/$hash");
+        $response->assert_2xx();
+        return array_converter::from_array(package_raw::class, $response->get_data());
+    }
+
     /**
      * Get a {@see package_raw} from a file.
      *
@@ -99,6 +113,7 @@ public static function extract_package_info(stored_file $file): package_raw {
         $response->assert_2xx();
         return array_converter::from_array(package_raw::class, $response->get_data());
     }
+
     /**
      * Get the status and information from the server.
      *

diff --git a/classes/last_used_service.php b/classes/last_used_service.php
@@ -52,12 +52,13 @@ public static function add(int $contextid, int $packageid): void {
     /**
      * Removes every entry with the given package id.
      *
-     * @param int $packageid
+     * @param int ...$packageids
      * @return void
      * @throws moodle_exception
      */
-    public static function remove_by_package(int $packageid): void {
+    public static function remove_by_package(int ...$packageids): void {
         global $DB;
-        $DB->delete_records('qtype_questionpy_lastused', ['packageid' => $packageid]);
+        [$insql, $inparams] = $DB->get_in_or_equal($packageids, SQL_PARAMS_NAMED, "packageids");
+        $DB->delete_records_select('qtype_questionpy_lastused', "packageid $insql", $inparams);
     }
 }
diff --git a/classes/package/package.php b/classes/package/package.php
@@ -103,14 +103,12 @@ public static function delete_by_id(int ...$ids): void {
         }
 
         $transaction = $DB->start_delegated_transaction();
-        foreach ($ids as $id) {
-            $DB->delete_records('qtype_questionpy_pkgversion', ['packageid' => $id]);
-            $DB->delete_records('qtype_questionpy_language', ['packageid' => $id]);
-            $DB->delete_records('qtype_questionpy_pkgtag', ['packageid' => $id]);
-            $DB->delete_records('qtype_questionpy_package', ['id' => $id]);
-            last_used_service::remove_by_package($id);
-        }
 
+        [$insql, $inparams] = $DB->get_in_or_equal($ids, SQL_PARAMS_NAMED, "packageids");
+        $DB->delete_records_select('qtype_questionpy_pkgversion', "packageid $insql", $inparams);
+        $DB->delete_records_select('qtype_questionpy_language', "packageid $insql", $inparams);
+        $DB->delete_records_select('qtype_questionpy_package', "id $insql", $inparams);
+        $DB->delete_records_select('qtype_questionpy_pkgtag', "packageid $insql", $inparams);
         $DB->execute("
             DELETE
             FROM {qtype_questionpy_tag}
@@ -119,6 +117,13 @@ public static function delete_by_id(int ...$ids): void {
                 FROM {qtype_questionpy_pkgtag}
             )
         ");
+        last_used_service::remove_by_package(...$ids);
+
+        $fservice = \core_favourites\service_factory::get_service_for_component('qtype_questionpy');
+        foreach ($ids as $id) {
+            $fservice->delete_favourites_by_type_and_item('package', $id);
+        }
+
         $transaction->allow_commit();
     }
 

diff --git a/classes/question_service.php b/classes/question_service.php
@@ -77,24 +77,15 @@ public function get_question(int $questionid): object {
                 'qtype_questionpy',
                 'package',
                 $record->id,
-                'itemid, filepath, filename',
-                false
+                includedirs: false,
+                limitnum: 1,
             );
-            if (count($files) === 0) {
+            if (!$files) {
                 throw new \coding_exception(
                     "No local package version file with hash '{$record->pkgversionhash}' was found despite being referenced" .
                     " by question {$questionid}"
                 );
             }
-        } else {
-            // Package was selected.
-            $package = package_version::get_by_id($record->pkgversionid);
-            if (is_null($package)) {
-                throw new \coding_exception(
-                    "No package version record with ID '{$record->pkgversionid}' was found despite being referenced" .
-                    " by question {$questionid}"
-                );
-            }
         }
         $result->qpy_id = $record->id;
         $result->qpy_package_hash = $record->pkgversionhash;
@@ -123,25 +114,30 @@ public function upsert_question(object $question): void {
         $question->qpy_package_hash ??= $question->qpy_package_file_hash;
 
         $file = null;
-        $pkgversionid = null;
         if ($question->qpy_package_source === 'upload') {
             if (isset($question->qpy_package_path_name_hash)) {
                 $file = $filestorage->get_file_by_hash($question->qpy_package_path_name_hash);
             } else {
                 $file = $this->packagefileservice->get_draft_file($question->qpy_package_file);
             }
+            $rawpackage = api::extract_package_info($file);
+            $pkgversionhash = $rawpackage->hash;
+            $pkgversionnamespace = $rawpackage->namespace;
+            $pkgversionshortname = $rawpackage->shortname;
         } else {
-            $pkgversionid = package_version::get_by_hash($question->qpy_package_hash)->id ?? null;
-            if (!$pkgversionid) {
-                throw new moodle_exception(
-                    'package_not_found',
-                    'qtype_questionpy',
-                    '',
-                    (object)['packagehash' => $question->qpy_package_hash]
-                );
+            $pkgversion = package_version::get_by_hash($question->qpy_package_hash) ?? null;
+            if ($pkgversion) {
+                $package = package::get_by_version($pkgversion->id);
+                $pkgversionhash = $pkgversion->hash;
+                $pkgversionnamespace = $package->namespace;
+                $pkgversionshortname = $package->shortname;
+                last_used_service::add($question->context->id, $package->id);
+            } else {
+                $packageinfo = $this->api->get_package_info($question->qpy_package_hash);
+                $pkgversionhash = $packageinfo->hash;
+                $pkgversionnamespace = $packageinfo->namespace;
+                $pkgversionshortname = $packageinfo->shortname;
             }
-            $packageid = package::get_by_version($pkgversionid)->id;
-            last_used_service::add($question->context->id, $packageid);
         }
 
         $existingrecord = $DB->get_record(self::QUESTION_TABLE, [
@@ -161,12 +157,13 @@ public function upsert_question(object $question): void {
             // Question record already exists, update it if necessary.
             $update = ['id' => $existingrecord->id];
 
+            if ($existingrecord->pkgversionhash !== $pkgversionhash) {
+                $update['pkgversionhash'] = $pkgversionhash;
+            }
+
             if ($existingrecord->state !== $response->state) {
                 $update['state'] = $response->state;
             }
-            if ($pkgversionid !== $existingrecord->pkgversionid) {
-                $update['pkgversionid'] = $pkgversionid;
-            }
 
             if (count($update) > 1) {
                 $DB->update_record(self::QUESTION_TABLE, (object) $update);
@@ -176,9 +173,9 @@ public function upsert_question(object $question): void {
             // Insert a new record with the question state only containing the options.
             $questionid = $DB->insert_record(self::QUESTION_TABLE, [
                 'questionid' => $question->id,
-                'feedback' => '',
+                // TODO: retrieve the identifier directly from the package?
+                'packageidentifier' => "@{$pkgversionnamespace}/{$pkgversionshortname}",
                 'pkgversionhash' => $question->qpy_package_hash,
-                'pkgversionid' => $pkgversionid,
                 'islocal' => $islocal,
                 'state' => $response->state,
             ]);

diff --git a/db/install.xml b/db/install.xml
@@ -25,16 +25,14 @@
       <FIELDS>
         <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
         <FIELD NAME="questionid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" COMMENT="Foreign key references question.id" />
-        <FIELD NAME="feedback" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="Feedback shown for any response."/>
+        <FIELD NAME="packageidentifier" TYPE="char" LENGTH="255" NOTNULL="true" SEQUENCE="false" />
         <FIELD NAME="pkgversionhash" TYPE="char" LENGTH="255" NOTNULL="true" SEQUENCE="false"/>
-        <FIELD NAME="pkgversionid" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false"/>
         <FIELD NAME="islocal" TYPE="int" LENGTH="1" NOTNULL="true" SEQUENCE="false"/>
         <FIELD NAME="state" TYPE="text" NOTNULL="true" SEQUENCE="false"/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id" />
         <KEY NAME="questionid" TYPE="foreign-unique" FIELDS="questionid" REFTABLE="question" REFFIELDS="id" />
-        <KEY NAME="pkgversionid" TYPE="foreign" FIELDS="pkgversionid" REFTABLE="qtype_questionpy_pkgversion" REFFIELDS="id"/>
       </KEYS>
     </TABLE>
 

diff --git a/edit_questionpy_form.php b/edit_questionpy_form.php
@@ -109,12 +109,12 @@ private function definition_package_search_container(MoodleQuickForm $mform): vo
      * @param package_base $package
      * @param string $packagehash
      * @param string $packageversion
-     * @param bool $isfavourite
+     * @param bool|null $isfavourite null if the package can not be marked as favourite
      * @param stored_file|null $file
      * @throws moodle_exception
      */
     private function definition_package_settings(MoodleQuickForm $mform, package_base $package, string $packagehash,
-                                                 string $packageversion, bool $isfavourite,
+                                                 string $packageversion, ?bool $isfavourite = null,
                                                  ?stored_file $file = null): void {
         global $OUTPUT;
 
@@ -126,7 +126,7 @@ private function definition_package_settings(MoodleQuickForm $mform, package_bas
         $packagearray['versions'] = ['hash' => $packagehash, 'version' => $packageversion];
         $packagearray['islocal'] = !is_null($file);
         $packagearray['isfavourite'] = $isfavourite;
-
+        $packagearray['ismarkableasfavourite'] = !is_null($isfavourite);
         $group = [];
         $group[] = $mform->createElement(
             'html',
@@ -176,7 +176,7 @@ private function definition_package_settings_upload(MoodleQuickForm $mform, bool
         }
         $package = api::extract_package_info($file);
 
-        $this->definition_package_settings($mform, $package, $package->hash, $package->version, false, $file);
+        $this->definition_package_settings($mform, $package, $package->hash, $package->version, file: $file);
     }
 
     /**
@@ -194,14 +194,22 @@ private function definition_package_settings_search(MoodleQuickForm $mform) {
         // Get package version.
         $packagehash = $this->optional_param('qpy_package_hash', $this->question->qpy_package_hash ?? null, PARAM_ALPHANUM);
         $pkgversion = package_version::get_by_hash($packagehash);
-        $package = package::get_by_version($pkgversion->id);
 
-        // Get favourite status.
-        $usercontext = context_user::instance($USER->id);
-        $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext);
-        $isfavourite = $ufservice->favourite_exists('qtype_questionpy', 'package', $package->id, $usercontext);
+        if ($pkgversion) {
+            $package = package::get_by_version($pkgversion->id);
+            // Get favourite status.
+            $usercontext = context_user::instance($USER->id);
+            $ufservice = \core_favourites\service_factory::get_service_for_user_context($usercontext);
+            $isfavourite = $ufservice->favourite_exists('qtype_questionpy', 'package', $package->id, $usercontext);
+            $version = $pkgversion->version;
+        } else {
+            $api = new api();
+            $package = $api->get_package_info($packagehash);
+            $isfavourite = null;
+            $version = $package->version;
+        }
 
-        $this->definition_package_settings($mform, $package, $pkgversion->hash, $pkgversion->version, $isfavourite);
+        $this->definition_package_settings($mform, $package, $packagehash, $version, $isfavourite);
     }
 
     /**

diff --git a/lang/en/qtype_questionpy.php b/lang/en/qtype_questionpy.php
@@ -60,8 +60,11 @@
 $string['select_package'] = 'Select';
 $string['select_package_element_aria'] = 'Choose version.';
 $string['selection_custom_package_header'] = 'Custom Package';
-$string['selection_custom_package_text'] = 'This package was uploaded by a user and might not appear in the package search.';
+$string['selection_custom_package_text'] = 'This package version was uploaded by a user and might not appear in the package"
+    . " search.';
 $string['selection_no_icon'] = 'Could not load the icon.';
+$string['selection_package_no_longer_in_database_header'] = 'Discontinued';
+$string['selection_package_no_longer_in_database_text'] = 'This package version is no longer available through the package search.';
 $string['selection_required'] = 'Please select a package.';
 $string['selection_title'] = 'Select QuestionPy Package';
 $string['selection_title_selected'] = 'Selected Package';

diff --git a/templates/package/package_selection.mustache b/templates/package/package_selection.mustache
@@ -37,6 +37,7 @@
     * url - The url of the QuestionPy package,
     * isselected - Whether the current QuestionPy package is selected or not,
     * islocal - Whether the current QuestionPy package was uplaoded by a user or not,
+    * ismarkableasfavourite - Whether the current QuestionPy package can be marked as favourite or not,
     * isfavourite - Whether the current QuestionPy package is marked as favourite or not,
 
     Example context (json):
@@ -50,6 +51,7 @@
         "url": "https://example.com",
         "isselected": true,
         "islocal": false,
+        "ismarkableasfavourite": true,
         "isfavourite": false
     }
 }}
@@ -67,7 +69,7 @@
                         {{#pix}} i/moremenu {{/pix}}
                     </a>
                     <div class="dropdown-menu">
-                        {{^islocal}}
+                        {{#ismarkableasfavourite}}
                             <a class="dropdown-item" href="#" data-for="favourite-button" {{#isfavourite}} data-is-favourite {{/isfavourite}}>
                                 <span class="qpy-favourite-button-text">
                                     {{#pix}} i/star-rating {{/pix}}{{#str}} mark_as_favourite, qtype_questionpy {{/str}}
@@ -76,7 +78,7 @@
                                     {{#pix}} i/star-o {{/pix}}{{#str}} unmark_as_favourite, qtype_questionpy {{/str}}
                                 </span>
                             </a>
-                        {{/islocal}}
+                        {{/ismarkableasfavourite}}
                         {{#url}}
                             <a class="dropdown-item" href="{{url}}" target="_blank">{{#pix}} i/mnethost {{/pix}}{{#str}} open_website, qtype_questionpy {{/str}}</a>
                         {{/url}}
@@ -98,6 +100,14 @@
                             &#9432; {{#str}} selection_custom_package_header, qtype_questionpy {{/str}}
                         </span>
                     {{/islocal}}
+                    {{^islocal}}
+                        {{^ismarkableasfavourite}}
+                            {{! This package was previously in the database. }}
+                            <span class="badge badge-info user-select-none mr-auto mb-2" data-toggle="tooltip" data-placement="top" data-title="{{#cleanstr}} selection_package_no_longer_in_database_text, qtype_questionpy {{/cleanstr}}">
+                                &#9432; {{#str}} selection_package_no_longer_in_database_header, qtype_questionpy {{/str}}
+                            </span>
+                        {{/ismarkableasfavourite}}
+                    {{/islocal}}
                     <button class="btn btn-danger qpy-version-selection-button">{{#str}} change_package, qtype_questionpy {{/str}}</button>
                 </div>
             {{/isselected}}