From adddcf394f1c019bf3dc8471302cda869c7929ad Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Tue, 9 May 2023 13:37:00 +0800 Subject: [PATCH] MDL-78168 mod_assign: Stop storing properties dynamically In PHP 8.2 and later, setting a value to an undeclared class property is deprecated and emits a deprecation notice. So we need to add missing class properties that still need to be declared. In these cases, mod_assign was treating other unrelated objects as its personal object store. These have been updated to not require this. --- mod/assign/classes/output/renderer.php | 45 +++++++++++++++-- mod/assign/renderable.php | 69 +++++++++++--------------- mod/assign/upgrade.txt | 6 +++ 3 files changed, 75 insertions(+), 45 deletions(-) diff --git a/mod/assign/classes/output/renderer.php b/mod/assign/classes/output/renderer.php index 3b1f5cf0214..2d7afe12e8d 100644 --- a/mod/assign/classes/output/renderer.php +++ b/mod/assign/classes/output/renderer.php @@ -24,12 +24,16 @@ namespace mod_assign\output; +use assign_files; +use html_writer; +use mod_assign\output\grading_app; +use portfolio_add_button; +use stored_file; + defined('MOODLE_INTERNAL') || die(); require_once($CFG->dirroot . '/mod/assign/locallib.php'); -use \mod_assign\output\grading_app; - /** * A custom renderer class that extends the plugin_renderer_base and is used by the assign module. * @@ -1397,11 +1401,13 @@ class renderer extends \plugin_renderer_base { $result .= '
  • ' . '
    ' . '
    ' . $image . ' ' . - $file->fileurl . ' ' . + html_writer::link($tree->get_file_url($file), $file->get_filename(), [ + 'target' => '_blank', + ]) . ' ' . $plagiarismlinks . ' ' . - $file->portfoliobutton . ' ' . + $this->get_portfolio_button($tree, $file) . ' ' . '
    ' . - '
    ' . $file->timemodified . '
    ' . + '
    ' . $tree->get_modified_time($file) . '
    ' . '
    ' . '
    • '; } @@ -1411,6 +1417,35 @@ class renderer extends \plugin_renderer_base { return $result; } + /** + * Get the portfolio button content for the specified file. + * + * @param assign_files $tree + * @param stored_file $file + * @return string + */ + protected function get_portfolio_button(assign_files $tree, stored_file $file): string { + global $CFG; + if (empty($CFG->enableportfolios)) { + return ''; + } + + if (!has_capability('mod/assign:exportownsubmission', $tree->context)) { + return ''; + } + + require_once($CFG->libdir . '/portfoliolib.php'); + + $button = new portfolio_add_button(); + $portfolioparams = [ + 'cmid' => $tree->cm->id, + 'fileid' => $file->get_id(), + ]; + $button->set_callback_options('assign_portfolio_caller', $portfolioparams, 'mod_assign'); + $button->set_format_by_file($file); + return $button->to_html(PORTFOLIO_ADD_ICON_LINK); + } + /** * Helper method dealing with the fact we can not just fetch the output of flexible_table * diff --git a/mod/assign/renderable.php b/mod/assign/renderable.php index a6d2ec51993..bd9417c1171 100644 --- a/mod/assign/renderable.php +++ b/mod/assign/renderable.php @@ -746,49 +746,38 @@ class assign_files implements renderable { * @param array $dir * @param string $filearea * @param string $component - * @return void */ public function preprocess($dir, $filearea, $component) { - global $CFG; + // Nothing to do here any more. + } - foreach ($dir['subdirs'] as $subdir) { - $this->preprocess($subdir, $filearea, $component); - } - foreach ($dir['files'] as $file) { - $file->portfoliobutton = ''; + /** + * Get the modified time of the specified file. + * @param stored_file $file + * @return string + */ + public function get_modified_time(stored_file $file): string { + return userdate( + $file->get_timemodified(), + get_string('strftimedatetime', 'langconfig'), + ); + } - $file->timemodified = userdate( - $file->get_timemodified(), - get_string('strftimedatetime', 'langconfig') - ); - - if (!empty($CFG->enableportfolios)) { - require_once($CFG->libdir . '/portfoliolib.php'); - $button = new portfolio_add_button(); - if (has_capability('mod/assign:exportownsubmission', $this->context)) { - $portfolioparams = array('cmid' => $this->cm->id, 'fileid' => $file->get_id()); - $button->set_callback_options('assign_portfolio_caller', - $portfolioparams, - 'mod_assign'); - $button->set_format_by_file($file); - $file->portfoliobutton = $button->to_html(PORTFOLIO_ADD_ICON_LINK); - } - } - $path = '/' . - $this->context->id . - '/' . - $component . - '/' . - $filearea . - '/' . - $file->get_itemid() . - $file->get_filepath() . - $file->get_filename(); - $url = file_encode_url("$CFG->wwwroot/pluginfile.php", $path, true); - $filename = $file->get_filename(); - $file->fileurl = html_writer::link($url, $filename, [ - 'target' => '_blank', - ]); - } + /** + * Get the URL used to view the file. + * + * @param stored_file + * @return moodle_url + */ + public function get_file_url(stored_file $file): moodle_url { + return \moodle_url::make_pluginfile_url( + $this->context->id, + $file->get_component(), + $file->get_filearea(), + $file->get_itemid(), + $file->get_filepath(), + $file->get_filename(), + true, + ); } } diff --git a/mod/assign/upgrade.txt b/mod/assign/upgrade.txt index 68c4818957d..a2ac4572b6e 100644 --- a/mod/assign/upgrade.txt +++ b/mod/assign/upgrade.txt @@ -6,6 +6,12 @@ This files describes API changes in the assign code. - `assign::format_grade_for_log` - `assign::format_submission_for_log` - `assign_plugin::format_for_log` +* The assign_files renderable no longer abuses the dynamic nature of PHP and puts random properties onto stored_file + instances. + If you were previously using these properties, please update to use the new method on the tree: + * $file->portfoliobutton $renderer->get_portfolio_button($file) + * $file->timemodified $tree->get_modified_time($file) + * $file->fileurl $tree->get_file_url($file) === 4.2 === * The upgradelib.php file has been removed from mod_assign as it was only being used by mod_assignment and mod_assignment has been removed from core.