From ec0fc897eeb26b8982f0089506685040c18e0a08 Mon Sep 17 00:00:00 2001 From: Richard Lobb Date: Sun, 14 Apr 2024 21:03:36 +1200 Subject: [PATCH] Add files attribute to combinator grader outcome, to allow replacement of data urls with urls that reference files saved in the Moodle file system. --- Readme.md | 17 ++- classes/combinator_grader_outcome.php | 195 +++++++++++++++++++++----- lib.php | 18 +++ 3 files changed, 192 insertions(+), 38 deletions(-) diff --git a/Readme.md b/Readme.md index e87d03ea7..abc531635 100644 --- a/Readme.md +++ b/Readme.md @@ -1954,7 +1954,7 @@ template grader", i.e. a TemplateGrader with the `Is combinator` checkbox checke In this mode, the JSON string output by the template grader should again contain a 'fraction' field, this time for the total mark, and may contain zero or more of 'prologuehtml', 'testresults', 'columnformats', -'epiloguehtml', 'showoutputonly', 'showdifferences' and 'graderstate'. +'epiloguehtml', 'instructorhtml', 'files', 'showoutputonly', 'showdifferences' and 'graderstate'. attributes. The 'prologuehtml' and 'epiloguehtml' fields are html that is displayed respectively before and after the (optional) result table. The @@ -1967,6 +1967,9 @@ actually displayed but 0 or 1 values in the column can be used to turn on and off row visibility. Students do not see hidden rows but markers and other staff do. +'instructorhtml' is a special version of 'epiloguehtml' that is displayed only +to teachers. + If a 'testresults' field is present, there can also be a 'columnformats' field. This should have one format specifier per table column and each format specifier should either be '%s', in which case all formatting is left to the renderer @@ -1990,6 +1993,18 @@ the standard 'Show differences' button after the result table; it is displayed only if there is actually a result table present and if full marks were not awarded to the question. +The 'files' attribute is a JSON object mapping from filenames to the corresponding +base4 encoded +file contents. This parameter is intended primarily for returning image files +that will be displayed in the feedback, but could have other uses. If a 'files' +attribute is present, the files are written to the Moodle file area and download +URLs generated. files are timestamped so the same filename can be used unambiguously +in multiple grade responses. The URLs are then used to update any occurrences of the strings + `src="filename"` or `href="filename"` within the 'prologuehtml', 'testresults', + 'epiloguehtml' and 'instructorhtml' attributes to use the full URL instead of just the + filename. Unmatched filenames are disregarded. Single quotes instead of double + quotes can also be used in the 'src' and 'href' attribute assignments. + The 'graderstate' attribute is a string value that is stored in the database with the question attempt and is passed back to the combinator template grader code on the next attempt of that question as the field 'graderstate' of the diff --git a/classes/combinator_grader_outcome.php b/classes/combinator_grader_outcome.php index c61cc1da4..82d8071b7 100644 --- a/classes/combinator_grader_outcome.php +++ b/classes/combinator_grader_outcome.php @@ -32,6 +32,12 @@ class qtype_coderunner_combinator_grader_outcome extends qtype_coderunner_testin /** @var ?string Html that is displayed after the result table. */ public $prologuehtml; + /** @var array A map from filename to feedback file contents, base64 */ + public $files; + + /** @var array A map from filename to the URLs of the saved files */ + public $fileurls; + /** @var array A per-column array of %s (string) or %h (html) values to control column formatting */ public $columnformats; @@ -50,14 +56,14 @@ class qtype_coderunner_combinator_grader_outcome extends qtype_coderunner_testin /** @var array Array where each item is a rows of test result table */ public $testresults; - /** @var ?string The feedback for a given question attempt */ + /** @var ?string The feedback for a given question attempt (legacy support only) */ public $feedbackhtml; /** @var bool Whether or no show differences is selected */ public $showdifferences; // A list of the allowed attributes in the combinator template grader return value. - public $allowedfields = ['fraction', 'prologuehtml', 'testresults', 'epiloguehtml', + public $allowedfields = ['fraction', 'prologuehtml', 'testresults', 'files', 'epiloguehtml', 'feedbackhtml', 'columnformats', 'showdifferences', 'showoutputonly', 'graderstate', 'instructorhtml', ]; @@ -74,20 +80,109 @@ public function __construct($isprecheck) { } + /** + * Process all the files in $files, saving them to the Moodle file area + * and generating an URL to reference the saved file. The URLs are + * time-stamped to allow re-use of the same name over + * multiple submissions of a question. + * @param array $files An associate array mapping filenames to base64-encoded contents. + * @param array An associate array mapping filenames to URLs that reference that file. + */ + private function save_files($files) { + global $USER; + + $fileurls = []; + foreach ($files as $filename => $base64) { + $decoded = base64_decode($base64); + + // Prepare file record object + $contextid = context_user::instance($USER->id)->id; + $fileinfo = array( + 'contextid' => $contextid, + 'component' => 'qtype_coderunner', + 'filearea' => 'feedbackfiles', // Custom file area for your plugin + 'itemid' => time(), // Item ID - use Unix time stamp + 'filepath' => '/', // File path within the context + 'filename' => $filename); // Desired name of the file + + // Get the file storage object + $fs = get_file_storage(); + + // Check if the file already exists to avoid duplicates + if (!$fs->file_exists($contextid, $fileinfo['component'], $fileinfo['filearea'], + $fileinfo['itemid'], $fileinfo['filepath'], $fileinfo['filename'])) { + + // Create the file in Moodle's filesystem + $file = $fs->create_file_from_string($fileinfo, $decoded); + } + + // Generate a URL to the saved file + $url = moodle_url::make_pluginfile_url($contextid, $fileinfo['component'], $fileinfo['filearea'], + $fileinfo['itemid'], $fileinfo['filepath'], $fileinfo['filename'], false); + + $fileurls[$filename] = $url; + } + return $fileurls; + } + + + /** + * Replace any occurrences of substrings of the form src="filename" or + * href=filename within the given html string to reference the URL of + * the file, if the filename is found in $this->fileurls. + * No action is taken for non-matching filename. + * @param string html The html string to be updates. + * @param array $urls An associative array mapping from filenames to URLs + * @return string The html string with the above-described substitutions made. + */ + private function insert_file_urls($html, $urls) { + if ($urls) { + foreach($urls as $filename => $url) { + $filename = preg_quote($filename, '/'); + $patterns = [ + "src *= *'$filename'", + "src *= *\"$filename\"", + "href *= *'$filename'", + "href *= *\"$filename\"" + ]; + foreach($patterns as $pat) { + if (strpos($pat, 'src') !== false) { + $html = preg_replace("/$pat/", "src=\"$url\"", $html); + } else { + $html = preg_replace("/$pat/", "href=\"$url\"", $html); + } + } + } + } + return $html; // return the modified HTML + } + + /** * Method to set the mark and the various feedback values (prologuehtml, * testresults, columnformats, epiloguehtml, graderstate). * @param float $markfraction the mark in the range 0 - 1 - * @param array $feedback Associative array of attributes to add to the - * outcome object, usually zero or more of prologuehtml, testresults, - * columnformats and epiloguehtml. + * @param array $feedback Associative array of additional attributes as + * listed in the $this->allowedfields. */ public function set_mark_and_feedback($markfraction, $feedback) { $this->actualmark = $markfraction; // Combinators work in the range 0 - 1. - foreach ($feedback as $key => $value) { - $this->$key = $value; + $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); + $htmlfields = ['feedbackhtml', 'prologuehtml', 'epiloguehtml', 'instructorhtml']; + foreach($htmlfields as $field) { + if ($this->$field) { + $this->$field = $this->insert_file_urls($this->$field, $urls); + } + }; + } + $this->format_results_table($testresults, $columnformats, $urls); } - $this->validate_table_formats(); } @@ -253,29 +348,33 @@ public function validation_error_message() { */ public function get_test_results(qtype_coderunner_question $q) { if (empty($this->testresults) || self::can_view_hidden()) { - return $this->format_table($this->testresults); + return $this->testresults; } else { - return $this->format_table($this->visible_rows($this->testresults)); + return self::visible_rows($this->testresults); } } - // Function to apply the formatting specified in $this->columnformats - // to the given table. This simply wraps cells in columns with a '%h' format - // specifier in html_wrapper objects leaving other cells unchanged. - // ishidden and iscorrect columns are copied across unchanged. - private function format_table($table) { - if (empty($table)) { - return $table; - } - if (!$this->columnformats) { - $newtable = $table; + /** + * Build the testresults table from the initial given testresults by + * applying all the column formats. Cells with '%h' format are first + * processed to replace any src or href assignments with references to + * the $urls of the saved files. Then the cells are wrapped in + * html_wrapper objects. All other cells are unchanged. + * ishidden and iscorrect columns are copied across unchanged. + * @param array $testresults The 'raw' $testresults from the run. + * @param array $columnformats The array of '%s' or '%h' column formats + * @param array $urls. The map from filename to URL + */ + private function format_results_table($testresults, $columnformats, $urls) { + if (!$testresults || !$columnformats) { + $this->testresults = $testresults; } else { - $formats = $this->columnformats; - $columnheaders = $table[0]; + $formats = $columnformats; + $columnheaders = $testresults[0]; $newtable = [$columnheaders]; - $nrows = count($table); + $nrows = count($testresults); for ($i = 1; $i < $nrows; $i++) { - $row = $table[$i]; + $row = $testresults[$i]; $newrow = []; $formatindex = 0; $ncols = count($row); @@ -284,8 +383,10 @@ private function format_table($table) { if (in_array($columnheaders[$col], ['ishidden', 'iscorrect'])) { $newrow[] = $cell; // Copy control column values directly. } else { + // Non-control columns are either '%s' or '%h' format. if ($formats[$formatindex++] === '%h') { + $cell = $this->insert_file_urls($cell, $urls); $newrow[] = new qtype_coderunner_html_wrapper($cell); } else { $newrow[] = $cell; @@ -295,7 +396,7 @@ private function format_table($table) { $newtable[] = $newrow; } } - return $newtable; + $this->testresults = $newtable; } public function get_prologue() { @@ -326,27 +427,43 @@ public function get_grader_state() { } - // Check that if a columnformats field is supplied - // the number of entries is correct and that each entry is either '%s' - // or '%h'. If not, an appropriate status error message is set. - private function validate_table_formats() { - if ($this->columnformats && $this->testresults) { + /** + * Check if the given values of column formats and test results are + * valid in the sense that the number of entries in column formats + * matches the number of columns in the table and that each column + * foramt is either '%s' + * or '%h'. If not, an appropriate status error message is set and + * the return value is false. Otherwise the return value is true. + * @param array $testresults An array of test results, where the + * first row is the column headers and the remaining rows are the + * results of each test. + * @param array $columnformats An array of strings specifying the + * column formats, each either %s or %h. + * @return bool True if either of $columnformats or $testresults is + * null or empty or if all column formats are valid. Otherwise + * return false, and set an error status. + */ + + private static function valid_table_formats($testresults,$columnformats, ) { + $ok = true; + if ($columnformats && $testresults) { $numcols = 0; - foreach ($this->testresults[0] as $colhdr) { + foreach ($testresults[0] as $colhdr) { // Count columns in header, excluding iscorrect and ishidden. if ($colhdr !== 'iscorrect' && $colhdr !== 'ishidden') { $numcols += 1; } } - if (count($this->columnformats) !== $numcols) { + if (count($columnformats) !== $numcols) { $error = get_string( 'wrongnumberofformats', 'qtype_coderunner', - ['expected' => $numcols, 'got' => count($this->columnformats)] + ['expected' => $numcols, 'got' => count($columnformats)] ); $this->set_status(self::STATUS_BAD_COMBINATOR, $error); + $ok = false; } else { - foreach ($this->columnformats as $format) { + foreach ($columnformats as $format) { if ($format !== '%s' && $format !== '%h') { $error = get_string( 'illegalformat', @@ -354,17 +471,21 @@ private function validate_table_formats() { ['format' => $format] ); $this->set_status(self::STATUS_BAD_COMBINATOR, $error); + $ok = false; break; } } } } + return $ok; } - // Private method to filter result table so only visible rows are shown - // to students. Only called if the user is not allowed to see hidden rows - // And if there is a non-null non-empty resulttable. + /** + * Filter the given result table to return only the visible rows. + * Only called if the user is not allowed to see hidden rows + * And if there is a non-null non-empty resulttable. + * */ private static function visible_rows($resulttable) { $header = $resulttable[0]; $ishiddencolumn = -1; diff --git a/lib.php b/lib.php index d169401d7..5c963fc90 100644 --- a/lib.php +++ b/lib.php @@ -35,6 +35,24 @@ */ function qtype_coderunner_pluginfile($course, $cm, $context, $filearea, $args, $forcedownload, array $options = []) { global $CFG; + if ($filearea === 'feedbackfiles') { + require_login($course, false, $cm); + + // Note: Implement additional checks here, as needed, to ensure users are authorized to access the file + + // Serve the file + $fs = get_file_storage(); + $filename = array_pop($args); + $itemid = intval(array_shift($args)); + $filepath = '/'; + $contextid = $context->id; + $file = $fs->get_file($contextid, 'qtype_coderunner', $filearea, $itemid, $filepath, $filename); + if (!$file) { + send_file_not_found(); + } + send_stored_file($file, 0, 0, $forcedownload, $options); // Adjust options as necessary + } require_once($CFG->libdir . '/questionlib.php'); question_pluginfile($course, $context, 'qtype_coderunner', $filearea, $args, $forcedownload, $options); } +