feat: apply suggested changes

Author: janbritz

classes/local/api/question_data.php

@@ -98,24 +98,6 @@ public static function from_question_response(question_response $response): self
         );
     }
 
-    /**
-     * Creates {@see question_data} from the question id.
-     *
-     * @param int $id
-     * @return ?self
-     * @throws moodle_exception
-     */
-    public static function from_question_id(int $id): ?self {
-        global $DB;
-
-        $field = $DB->get_field('qtype_questionpy', 'questiondata', ['questionid' => $id]);
-        if ($field === false) {
-            return null;
-        }
-
-        return self::from_json($field);
-    }
-
     /**
      * Creates {@see question_data} from JSON.
      *

classes/question_service.php

@@ -90,8 +90,9 @@ public function get_question(int $questionid): object {
         }
         $result->qpy_id = $record->id;
         $result->qpy_package_hash = $record->pkgversionhash;
-        $result->qpy_state = $record->state;
         $result->qpy_is_local = $record->islocal;
+        $result->qpy_state = $record->state;
+        $result->qpy_question_data = $record->questiondata;
         return $result;
     }
 

question.php

@@ -47,6 +47,8 @@ class qtype_questionpy_question extends question_graded_automatically_with_count
     public string $packagehash;
     /** @var string */
     public string $questionstate;
+    /** @var question_data */
+    public question_data $questiondata;
     /** @var stored_file|null */
     private ?stored_file $packagefile;
 
@@ -75,14 +77,18 @@ class qtype_questionpy_question extends question_graded_automatically_with_count
      *
      * @param string $packagehash
      * @param string $questionstate
+     * @param question_data $questiondata
      * @param stored_file|null $packagefile
      * @param api $api
      */
-    public function __construct(string $packagehash, string $questionstate, ?stored_file $packagefile, api $api) {
+    public function __construct(
+        string $packagehash, string $questionstate, question_data $questiondata, ?stored_file $packagefile, api $api
+    ) {
         parent::__construct();
         $this->api = $api;
         $this->packagehash = $packagehash;
         $this->questionstate = $questionstate;
+        $this->questiondata = $questiondata;
         $this->packagefile = $packagefile;
     }
 
@@ -118,8 +124,7 @@ public function start_attempt(question_attempt_step $step, $variant): void {
         global $PAGE;
 
         try {
-            // Unfortunately, we cannot access attempt data, as the attempt is not stored in the database at this point of time.
-            $attributes = null;
+            $attributes = $this->get_requested_attributes();
             $attempt = $this->api->package($this->packagehash, $this->packagefile)
                 ->start_attempt($this->questionstate, $variant, $attributes);
 
@@ -184,18 +189,11 @@ public function apply_attempt_state(question_attempt_step $step) {
         /* TODO: This method is also called from question_attempt->regrade and
                  question_attempt->start_question_based_on, where we shouldn't need to get the UI. */
         try {
-            if ($this->id !== null) {
-                $questiondata = question_data::from_question_id($this->id);
-                $requestedattributes = $questiondata->permissions?->attributes;
-                if ($requestedattributes) {
-                    $attributes = $this->get_bridge()->get_attributes($requestedattributes);
-                }
-            }
-
+            $attributes = $this->get_requested_attributes();
             $attempt = $this->api->package($this->packagehash, $this->packagefile)
                 ->view_attempt(
                     $this->questionstate,
-                    $attributes ?? null,
+                    $attributes,
                     $this->attemptstate,
                     $this->scoringstate,
                     $lastresponse
@@ -433,11 +431,14 @@ public function make_behaviour(question_attempt $qa, $preferredbehaviour): quest
      * Get the QuestionPy bridge used to retrieve additional information about an attempt.
      *
      * @throws moodle_exception
-     * @return question_bridge_base
+     * @return question_bridge_base|null
      */
-    public function get_bridge(): question_bridge_base {
+    public function get_bridge(): ?question_bridge_base {
         if ($this->bridge === null) {
-            $this->bridge = question_bridge_base::create($this->get_behaviour()->get_qa());
+            $attempt = $this->get_behaviour()->get_qa();
+            if ($attempt->get_database_id() !== null && is_numeric($attempt->get_usage_id())) {
+                $this->bridge = question_bridge_base::create($attempt);
+            }
         }
         return $this->bridge;
     }
@@ -454,4 +455,15 @@ public function get_bridge(): question_bridge_base {
     public function set_bridge(question_bridge_base $bridge): void {
         $this->bridge = $bridge;
     }
+
+    /**
+     * Retrieves the requested attributes if any.
+     *
+     * @return array|null
+     * @throws moodle_exception
+     */
+    private function get_requested_attributes(): ?array {
+        $attributes = $this->questiondata->permissions?->attributes;
+        return $attributes ? $this->get_bridge()?->get_attributes($attributes) : null;
+    }
 }

questiontype.php

@@ -24,6 +24,7 @@
 
 use core\di;
 use qtype_questionpy\local\api\api;
+use qtype_questionpy\local\api\question_data;
 use qtype_questionpy\package_file_service;
 use qtype_questionpy\question_service;
 
@@ -186,8 +187,9 @@ protected function make_question_instance($questiondata) {
         return new qtype_questionpy_question(
             $questiondata->qpy_package_hash,
             $questiondata->qpy_state,
+            question_data::from_json($questiondata->qpy_question_data),
             $packagefile,
-            $this->api
+            $this->api,
         );
     }
 }

tests/local/attempt_ui/question_ui_renderer_test.php

@@ -26,6 +26,7 @@
 use PHPUnit\Framework\MockObject\Stub;
 use qtype_questionpy\constants;
 use qtype_questionpy\local\api\api;
+use qtype_questionpy\local\api\question_data;
 use qtype_questionpy_question;
 use question_attempt;
 use question_attempt_step;
@@ -508,7 +509,13 @@ private function create_question_attempt_stub(?string $packagehash = null, ?int
                                                   array $lastresponse = []): question_attempt {
         $packagehash ??= hash('sha256', random_string(64));
         $id ??= mt_rand();
-        $question = new qtype_questionpy_question($packagehash, '{}', null, $this->createStub(api::class));
+        $question = new qtype_questionpy_question(
+            $packagehash,
+            '{}',
+            $this->createStub(question_data::class),
+            null,
+            $this->createStub(api::class),
+        );
 
         $step = new question_attempt_step([constants::QT_VAR_RESPONSE => json_encode((object) $lastresponse)]);
 

tests/question_test.php

@@ -24,6 +24,7 @@
 use qtype_questionpy\event\viewing_attempt_failed;
 use qtype_questionpy\local\api\api;
 use qtype_questionpy\local\api\package_api;
+use qtype_questionpy\local\api\question_data;
 use qtype_questionpy\local\attempt_ui\question_ui_metadata_extractor;
 use qtype_questionpy_question;
 use question_bank;
@@ -79,6 +80,7 @@ private function create_question(): qtype_questionpy_question {
         $question = new qtype_questionpy_question(
             hash('sha256', 'hash'),
             'state',
+            $this->createStub(question_data::class),
             packagefile: null,
             api: $this->api
         );