Remove files attribute from combinator outcome, plus some code tidying.

trampgeek · Apr 16, 2024 · bcf86e6 · bcf86e6
1 parent 6682d85
commit bcf86e6
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 25 deletions.
diff --git a/classes/combinator_grader_outcome.php b/classes/combinator_grader_outcome.php
@@ -57,8 +57,7 @@ class qtype_coderunner_combinator_grader_outcome extends qtype_coderunner_testin
 
     // A list of the allowed attributes in the combinator template grader return value.
     public $allowedfields = ['fraction', 'prologuehtml', 'testresults', 'files', 'epiloguehtml',
-                    'feedbackhtml', 'columnformats', 'showdifferences',
-                    'showoutputonly', 'graderstate', 'instructorhtml',
+                    'columnformats', 'showdifferences', 'showoutputonly', 'graderstate', 'instructorhtml',
     ];
 
     public function __construct($isprecheck) {
@@ -174,22 +173,20 @@ private function insert_file_urls($html, $urls) {
      */
     public function set_mark_and_feedback($markfraction, $feedback) {
         $this->actualmark = $markfraction;  // Combinators work in the range 0 - 1.
-        $columnformats = $feedback['columnformats'] ?? null;
         $testresults = $feedback['testresults'] ?? null;
         $files = $feedback['files'] ?? null;
-        $urls = null;
-        if ($this->valid_table_formats($testresults, $columnformats)) {
-            if ($files) {
-                $urls = $this->save_files($files);
-            }
-            foreach ($feedback as $field => $value) {
-                if ($urls && in_array($field, ['prologuehtml', 'epiloguehtml', 'instructorhtml', 'feedbackhtml'])) {
-                    $this->$field = $this->insert_file_urls($value, $urls);
-                } else {
-                    $this->$field = $value;
-                }
+        $urls = $files ? $this->save_files($files) : null;
+
+        foreach ($feedback as $field => $value) {
+            if ($urls && in_array($field, ['prologuehtml', 'epiloguehtml', 'instructorhtml'])) {
+                $this->$field = $this->insert_file_urls($value, $urls);
+            } else if (! in_array($field, ['files', 'testresults'])) {
+                $this->$field = $value;
             }
-            $this->format_results_table($testresults, $columnformats, $urls);
+        }
+
+        if ($this->valid_table_formats($testresults, $this->columnformats)) {
+            $this->format_results_table($testresults, $this->columnformats, $urls);
         }
     }
 
@@ -212,7 +209,7 @@ public function iscombinatorgrader() {
     // but just output to be displayed as supplied. There is no message
     // regarding success or failure with such questions.
     public function is_output_only() {
-        return isset($this->outputonly) && $this->outputonly;
+        return $this->outputonly ?? false;
     }
 
 

diff --git a/classes/jobrunner.php b/classes/jobrunner.php
@@ -374,9 +374,11 @@ private function do_combinator_grading($run, $isprecheck) {
             // A successful combinator run (so far).
             $fract = $outcome->is_output_only() ? 1.0 : $result->fraction;
             $feedback = [];
-            if (isset($result->feedback_html)) {  // Legacy combinator grader?
-                $result->feedbackhtml = $result->feedback_html; // Change to modern version.
-                unset($result->feedback_html);
+            foreach (['feedback_html', 'feedbackhtml'] as $legacykey) {
+                if (isset($result->$legacykey)) {  // Legacy combinator grader?
+                    $result->epiloguehtml = $result->$legacykey; // Use it as epiloguehtml.
+                    unset($result->$legacykey);
+                }
             }
             foreach ($result as $key => $value) {
                 if (!in_array($key, $outcome->allowedfields)) {
@@ -387,12 +389,7 @@ private function do_combinator_grading($run, $isprecheck) {
                     );
                     throw new Exception($error);
                 }
-                if ($key === 'feedbackhtml' || $key === 'feedback_html') {
-                    // For compatibility with older combinator graders.
-                    $feedback['epiloguehtml'] = $result->$key;
-                } else {
-                    $feedback[$key] = $value;
-                }
+                $feedback[$key] = $value;
             }
             $outcome->set_mark_and_feedback($fract, $feedback);  // Further valididty checks done in here.
         } catch (Exception $except) {