From fd8184a29586c4470d88adcbe4ef60ed8e7cb0f3 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 18 Apr 2023 18:40:15 +0100 Subject: [PATCH] MDL-77954 moodlenet: store packaged activities in user draft. We shouldn't invent fileareas with invalid values (specifically in this case, the `itemid` overflowed maximum length of the field on Oracle). Given the generated file is transitory in nature, instead store it in the users draft area. --- lib/classes/moodlenet/activity_packager.php | 19 +++++++------------ .../moodlenet/activity_packager_test.php | 9 +++++---- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/classes/moodlenet/activity_packager.php b/lib/classes/moodlenet/activity_packager.php index ef69c20b4fd..7e6f7ec862e 100644 --- a/lib/classes/moodlenet/activity_packager.php +++ b/lib/classes/moodlenet/activity_packager.php @@ -20,6 +20,7 @@ use backup; use backup_controller; use backup_root_task; use cm_info; +use core\context\user; defined('MOODLE_INTERNAL') || die(); @@ -52,7 +53,6 @@ class activity_packager { throw new \coding_exception("Cannot backup module $cminfo->modname. This module doesn't support the backup feature."); } - $this->cminfo = $cminfo; $this->controller = new backup_controller ( backup::TYPE_1ACTIVITY, $cminfo->id, @@ -147,15 +147,14 @@ class activity_packager { } // Create the location we want to copy this file to. - $time = time(); $fr = [ - 'contextid' => \context_course::instance($this->cminfo->course)->id, - 'component' => 'core', - 'filearea' => 'moodlenet_resource', + 'contextid' => user::instance($this->userid)->id, + 'userid' => $this->userid, + 'component' => 'user', + 'filearea' => 'draft', + 'itemid' => file_get_unused_draft_itemid(), + 'filepath' => '/', 'filename' => $this->cminfo->modname . '_backup.mbz', - // Add timestamp to itemid to make it unique, to avoid any collisions. - 'itemid' => $this->cminfo->id . $time, - 'timemodified' => $time, ]; // Create the local file based on the backup. @@ -167,10 +166,6 @@ class activity_packager { // Delete the backup now it has been created in the file area. $backupfile->delete(); - if (!$packagedfiledata['storedfile']) { - throw new \moodle_exception("Failed to copy backup file to moodlenet_activity area."); - } - // Ensure we can handle files at the upper end of the limit supported by MoodleNet. raise_memory_limit(activity_sender::MAX_FILESIZE); diff --git a/lib/tests/moodlenet/activity_packager_test.php b/lib/tests/moodlenet/activity_packager_test.php index f39c23498ae..7c53b1f3455 100644 --- a/lib/tests/moodlenet/activity_packager_test.php +++ b/lib/tests/moodlenet/activity_packager_test.php @@ -16,6 +16,8 @@ namespace core\moodlenet; +use core\context\user; + /** * Unit tests for {@see activity_packager}. * @@ -107,7 +109,6 @@ class activity_packager_test extends \advanced_testcase { $currenttime = time(); $generator = $this->getDataGenerator(); $course = $generator->create_course(); - $coursecontext = \context_course::instance($course->id); $assigndata = [ 'course' => $course->id, @@ -131,9 +132,9 @@ class activity_packager_test extends \advanced_testcase { // Check some known values in the returned stored_file object to confirm they match the file we have packaged. $this->assertNotEmpty($package['storedfile']->get_contenthash()); - $this->assertEquals($coursecontext->id, $package['storedfile']->get_contextid()); - $this->assertEquals('core', $package['storedfile']->get_component()); - $this->assertEquals('moodlenet_resource', $package['storedfile']->get_filearea()); + $this->assertEquals(user::instance($USER->id)->id, $package['storedfile']->get_contextid()); + $this->assertEquals('user', $package['storedfile']->get_component()); + $this->assertEquals('draft', $package['storedfile']->get_filearea()); $this->assertEquals('assign_backup.mbz', $package['storedfile']->get_filename()); $this->assertGreaterThan(0, $package['storedfile']->get_filesize()); $timecreated = $package['storedfile']->get_timecreated();