From a484fcb09d117d18d005c57d25d9e514dabf775e Mon Sep 17 00:00:00 2001 From: Ferran Recio Date: Mon, 7 Mar 2022 14:31:45 +0100 Subject: [PATCH] MDL-74104 mod_folder: fix recent activity with forcedownload links The folder module has a setting to force the download links. However, the recent activity block ignored this setting until now. --- mod/folder/lib.php | 68 +++++++++++++--------- mod/folder/tests/lib_test.php | 106 ++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 26 deletions(-) diff --git a/mod/folder/lib.php b/mod/folder/lib.php index 2fe3f23e0f6..5591816a3dc 100644 --- a/mod/folder/lib.php +++ b/mod/folder/lib.php @@ -589,49 +589,52 @@ function folder_downloaded($folder, $course, $cm, $context) { * @param int $groupid not used, but required for compatibilty with other modules */ function folder_get_recent_mod_activity(&$activities, &$index, $timestart, $courseid, $cmid, $userid=0, $groupid=0) { - global $COURSE, $DB, $OUTPUT; + global $DB, $OUTPUT; - if ($COURSE->id == $courseid) { - $course = $COURSE; - } else { - $course = $DB->get_record('course', array('id' => $courseid)); - } - - $modinfo = get_fast_modinfo($course); + $modinfo = get_fast_modinfo($courseid); $cm = $modinfo->cms[$cmid]; $context = context_module::instance($cm->id); if (!has_capability('mod/folder:view', $context)) { return; } + $instance = $DB->get_record('folder', ['id' => $cm->instance], '*', MUST_EXIST); + $files = folder_get_recent_activity($context, $timestart, $userid); - foreach ($files as $file) { - $tmpactivity = new stdClass(); + $tmpactivity = (object) [ + 'type' => 'folder', + 'cmid' => $cm->id, + 'sectionnum' => $cm->sectionnum, + 'timestamp' => $file->get_timemodified(), + 'user' => core_user::get_user($file->get_userid()), + ]; - $tmpactivity->type = 'folder'; - $tmpactivity->cmid = $cm->id; - $tmpactivity->sectionnum = $cm->sectionnum; - $tmpactivity->timestamp = $file->get_timemodified(); - $tmpactivity->user = core_user::get_user($file->get_userid()); - - $tmpactivity->content = new stdClass(); - $tmpactivity->content->url = moodle_url::make_pluginfile_url($file->get_contextid(), 'mod_folder', 'content', - $file->get_itemid(), $file->get_filepath(), $file->get_filename()); + $url = moodle_url::make_pluginfile_url( + $file->get_contextid(), + 'mod_folder', + 'content', + $file->get_itemid(), + $file->get_filepath(), + $file->get_filename(), + !empty($instance->forcedownload) + ); if (file_extension_in_typegroup($file->get_filename(), 'web_image')) { - $image = $tmpactivity->content->url->out(false, array('preview' => 'tinyicon', 'oid' => $file->get_timemodified())); + $image = $url->out(false, array('preview' => 'tinyicon', 'oid' => $file->get_timemodified())); $image = html_writer::empty_tag('img', array('src' => $image)); } else { $image = $OUTPUT->pix_icon(file_file_icon($file, 24), $file->get_filename(), 'moodle'); } - $tmpactivity->content->image = $image; - $tmpactivity->content->filename = $file->get_filename(); + $tmpactivity->content = (object) [ + 'image' => $image, + 'filename' => $file->get_filename(), + 'url' => $url, + ]; $activities[$index++] = $tmpactivity; } - } /** @@ -724,7 +727,10 @@ function folder_print_recent_activity($course, $viewfullnames, $timestart) { return false; } - $newfiles = array(); + // The list of all new files. + $newfiles = []; + // Save the force download setting of all instances with files indexed by context. + $forcedownloads = []; $modinfo = get_fast_modinfo($course); foreach ($folders as $folder) { @@ -737,6 +743,9 @@ function folder_print_recent_activity($course, $viewfullnames, $timestart) { // Get the files uploaded in the current time frame. $newfiles = array_merge($newfiles, folder_get_recent_activity($context, $timestart)); + if (!isset($forcedownloads[$context->id])) { + $forcedownloads[$context->id] = !empty($folder->forcedownload); + } } if (empty($newfiles)) { @@ -748,8 +757,15 @@ function folder_print_recent_activity($course, $viewfullnames, $timestart) { $list = html_writer::start_tag('ul', ['class' => 'unlist']); foreach ($newfiles as $file) { $filename = $file->get_filename(); - $url = moodle_url::make_pluginfile_url($file->get_contextid(), 'mod_folder', 'content', - $file->get_itemid(), $file->get_filepath(), $filename); + $contextid = $file->get_contextid(); + $url = moodle_url::make_pluginfile_url( + $contextid, + 'mod_folder', + 'content', + $file->get_itemid(), + $file->get_filepath(), $filename, + $forcedownloads[$contextid] ?? false + ); $list .= html_writer::start_tag('li'); $list .= html_writer::start_div('head'); diff --git a/mod/folder/tests/lib_test.php b/mod/folder/tests/lib_test.php index b454866cce5..15825f7388d 100644 --- a/mod/folder/tests/lib_test.php +++ b/mod/folder/tests/lib_test.php @@ -25,6 +25,9 @@ */ namespace mod_folder; +use context_user; +use context_module; + defined('MOODLE_INTERNAL') || die(); @@ -294,4 +297,107 @@ class lib_test extends \advanced_testcase { return \calendar_event::create($event); } + + /** + * Test Get recent mod activity method. + * @covers ::folder_get_recent_mod_activity + * @dataProvider folder_get_recent_mod_activity_provider + * + * @param int $forcedownload The forcedownload option. + * @param bool $hascapability if the user has the mod/folder:view capability + * @param int $count The expected recent activities entries. + */ + public function test_folder_get_recent_mod_activity(int $forcedownload, bool $hascapability, int $count) { + global $USER, $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $course = $this->getDataGenerator()->create_course(); + + // Add files to draft area. + $filesitem = file_get_unused_draft_itemid(); + $usercontext = context_user::instance($USER->id); + $filerecord = [ + 'component' => 'user', + 'filearea' => 'draft', + 'contextid' => $usercontext->id, + 'itemid' => $filesitem, + 'filename' => 'file1.txt', 'filepath' => '/', + ]; + $fs = get_file_storage(); + $fs->create_file_from_string($filerecord, 'First test file contents'); + // And a second file. + $filerecord['filename'] = 'file2.txt'; + $fs->create_file_from_string($filerecord, 'Second test file contents'); + + // Create the activity. + $module = $this->getDataGenerator()->create_module( + 'folder', + ['course' => $course->id, 'forcedownload' => $forcedownload, 'files' => $filesitem] + ); + + // Get some additional data. + $cm = get_coursemodule_from_instance('folder', $module->id); + $context = context_module::instance($cm->id); + + // Add user with the specific capability. + $user = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user->id, $course->id, 'editingteacher'); + if (!$hascapability) { + // The recent activiy uses "folder:view" capability which is allowed by default. + $role = $DB->get_record('role', ['shortname' => 'editingteacher'], '*', MUST_EXIST); + assign_capability('mod/folder:view', CAP_PROHIBIT, $role->id, $context->id, true); + } + $this->setUser($user); + + // Get the recent activity. + $index = 1; + $activities = []; + folder_get_recent_mod_activity($activities, $index, time() - HOURSECS, $course->id, $cm->id); + + // Check recent activity. + $this->assertCount($count, $activities); + foreach ($activities as $index => $activity) { + $this->assertEquals('folder', $activity->type); + $content = $activity->content; + $this->assertEquals("file{$index}.txt", $content->filename); + $urlparams = $content->url->params(); + if ($forcedownload) { + $this->assertEquals(1, $urlparams['forcedownload']); + } else { + $this->assertArrayNotHasKey('forcedownload', $urlparams); + } + } + } + + /** + * Data provider for test_folder_get_recent_mod_activity(). + * + * @return array + */ + public function folder_get_recent_mod_activity_provider(): array { + return [ + 'Teacher with force download' => [ + 'forcedownload' => 1, + 'hascapability' => true, + 'count' => 2, + ], + 'Teacher with no force download' => [ + 'forcedownload' => 0, + 'hascapability' => true, + 'count' => 2, + ], + 'Invalid user with force download' => [ + 'forcedownload' => 1, + 'hascapability' => false, + 'count' => 0, + ], + 'Invalid user with no force download' => [ + 'forcedownload' => 0, + 'hascapability' => false, + 'count' => 0, + ], + ]; + } }