From 67a11150de488716e5098f3ba3c64afeb14cc71d Mon Sep 17 00:00:00 2001 From: Sara Arjona Date: Wed, 19 Aug 2020 18:14:14 +0200 Subject: [PATCH] MDL-68909 h5p: move temporary editor files to draft area --- h5p/classes/editor.php | 4 +-- h5p/classes/file_storage.php | 44 ++++++++++++++++++++++------- h5p/lib.php | 1 - h5p/tests/editor_test.php | 3 ++ h5p/tests/generator/lib.php | 22 +++++++++++---- h5p/tests/h5p_file_storage_test.php | 18 ++++++++---- lib/upgrade.txt | 2 ++ 7 files changed, 70 insertions(+), 24 deletions(-) diff --git a/h5p/classes/editor.php b/h5p/classes/editor.php index a4b4af7ef06..f46edc479ac 100644 --- a/h5p/classes/editor.php +++ b/h5p/classes/editor.php @@ -382,7 +382,7 @@ class editor { // Add JavaScript settings. $root = $CFG->wwwroot; - $filespathbase = "{$root}/pluginfile.php/{$context->id}/core_h5p/"; + $filespathbase = \moodle_url::make_draftfile_url(0, '', ''); $factory = new factory(); $contentvalidator = $factory->get_content_validator(); @@ -390,7 +390,7 @@ class editor { $editorajaxtoken = core::createToken(editor_ajax::EDITOR_AJAX_TOKEN); $sesskey = sesskey(); $settings['editor'] = [ - 'filesPath' => $filespathbase . 'editor', + 'filesPath' => $filespathbase->out(), 'fileIcon' => [ 'path' => $url . 'images/binary-file.png', 'width' => 50, diff --git a/h5p/classes/file_storage.php b/h5p/classes/file_storage.php index c1e24ade3c7..ba032a42dad 100644 --- a/h5p/classes/file_storage.php +++ b/h5p/classes/file_storage.php @@ -48,7 +48,12 @@ class file_storage implements \H5PFileStorage { public const EXPORT_FILEAREA = 'export'; /** The icon filename */ public const ICON_FILENAME = 'icon.svg'; - /** The editor file area */ + + /** + * The editor file area. + * @deprecated since Moodle 3.10 MDL-68909. Please do not use this constant any more. + * @todo MDL-69530 This will be deleted in Moodle 4.2. + */ public const EDITOR_FILEAREA = 'editor'; /** @@ -331,10 +336,22 @@ class file_storage implements \H5PFileStorage { * @return int The id of the saved file. */ public function saveFile($file, $contentid) { + global $USER; + + $context = $this->context->id; + $component = self::COMPONENT; + $filearea = self::CONTENT_FILEAREA; + if ($contentid === 0) { + $usercontext = \context_user::instance($USER->id); + $context = $usercontext->id; + $component = 'user'; + $filearea = 'draft'; + } + $record = array( - 'contextid' => $this->context->id, - 'component' => self::COMPONENT, - 'filearea' => $contentid === 0 ? self::EDITOR_FILEAREA : self::CONTENT_FILEAREA, + 'contextid' => $context, + 'component' => $component, + 'filearea' => $filearea, 'itemid' => $contentid, 'filepath' => '/' . $file->getType() . 's/', 'filename' => $file->getName() @@ -357,8 +374,8 @@ class file_storage implements \H5PFileStorage { */ public function cloneContentFile($file, $fromid, $tocontent): void { // Determine source filearea and itemid. - if ($fromid === self::EDITOR_FILEAREA) { - $sourcefilearea = self::EDITOR_FILEAREA; + if ($fromid === 'editor') { + $sourcefilearea = 'draft'; $sourceitemid = 0; } else { $sourcefilearea = self::CONTENT_FILEAREA; @@ -791,15 +808,22 @@ class file_storage implements \H5PFileStorage { * @return stored_file|null */ private function get_file(string $filearea, int $itemid, string $file): ?stored_file { - if ($filearea === 'editor') { + global $USER; + + $component = self::COMPONENT; + $context = $this->context->id; + if ($filearea === 'draft') { $itemid = 0; + $component = 'user'; + $usercontext = \context_user::instance($USER->id); + $context = $usercontext->id; } $filepath = '/'. dirname($file). '/'; $filename = basename($file); // Load file. - $existingfile = $this->fs->get_file($this->context->id, self::COMPONENT, $filearea, $itemid, $filepath, $filename); + $existingfile = $this->fs->get_file($context, $component, $filearea, $itemid, $filepath, $filename); if (!$existingfile) { return null; } @@ -824,8 +848,8 @@ class file_storage implements \H5PFileStorage { // Create file record for content. $record = array( 'contextid' => $this->context->id, - 'component' => self::COMPONENT, - 'filearea' => $contentid > 0 ? self::CONTENT_FILEAREA : self::EDITOR_FILEAREA, + 'component' => $contentid > 0 ? self::COMPONENT : 'user', + 'filearea' => $contentid > 0 ? self::CONTENT_FILEAREA : 'draft', 'itemid' => $contentid > 0 ? $contentid : 0, 'filepath' => '/' . $foldername . '/', 'filename' => $filename diff --git a/h5p/lib.php b/h5p/lib.php index 025c58bed7f..b640925fde9 100644 --- a/h5p/lib.php +++ b/h5p/lib.php @@ -94,7 +94,6 @@ function core_h5p_pluginfile($course, $cm, $context, string $filearea, array $ar } $itemid = array_shift($args); break; - case \core_h5p\file_storage::EDITOR_FILEAREA: case \core_h5p\file_storage::CACHED_ASSETS_FILEAREA: case \core_h5p\file_storage::EXPORT_FILEAREA: $itemid = 0; diff --git a/h5p/tests/editor_test.php b/h5p/tests/editor_test.php index dff602216a8..8145ab51a08 100644 --- a/h5p/tests/editor_test.php +++ b/h5p/tests/editor_test.php @@ -182,6 +182,9 @@ class editor_testcase extends advanced_testcase { public function test_add_editor_to_form() { global $PAGE, $CFG; + $this->resetAfterTest(); + $this->setAdminUser(); + // Get form data. $form = $this->get_test_form(); $mform = $form->getform(); diff --git a/h5p/tests/generator/lib.php b/h5p/tests/generator/lib.php index 88c229d25e5..72e80870220 100644 --- a/h5p/tests/generator/lib.php +++ b/h5p/tests/generator/lib.php @@ -414,6 +414,8 @@ class core_h5p_generator extends \component_generator_base { * @throws coding_exception */ public function create_content_file(string $file, string $filearea, int $contentid = 0): stored_file { + global $USER; + $filepath = '/'.dirname($file).'/'; $filename = basename($file); @@ -421,15 +423,25 @@ class core_h5p_generator extends \component_generator_base { throw new coding_exception('Files belonging to an H5P content must specify the H5P content id'); } - $content = 'fake content'; + if ($filearea === 'draft') { + $usercontext = \context_user::instance($USER->id); + $context = $usercontext->id; + $component = 'user'; + $itemid = 0; + } else { + $systemcontext = context_system::instance(); + $context = $systemcontext->id; + $component = \core_h5p\file_storage::COMPONENT; + $itemid = $contentid; + } - $systemcontext = context_system::instance(); + $content = 'fake content'; $filerecord = array( - 'contextid' => $systemcontext->id, - 'component' => \core_h5p\file_storage::COMPONENT, + 'contextid' => $context, + 'component' => $component, 'filearea' => $filearea, - 'itemid' => ($filearea === 'editor') ? 0 : $contentid, + 'itemid' => $itemid, 'filepath' => $filepath, 'filename' => $filename, ); diff --git a/h5p/tests/h5p_file_storage_test.php b/h5p/tests/h5p_file_storage_test.php index d94d395831b..8cce6c2ed8c 100644 --- a/h5p/tests/h5p_file_storage_test.php +++ b/h5p/tests/h5p_file_storage_test.php @@ -625,6 +625,7 @@ class h5p_file_storage_testcase extends \advanced_testcase { */ public function test_get_file(): void { + $this->setAdminUser(); $file = 'img/fake.png'; $h5pcontentid = 3; @@ -641,9 +642,9 @@ class h5p_file_storage_testcase extends \advanced_testcase { $this->assertInstanceOf('stored_file', $contentfile); // Add a file to editor. - $this->h5p_generator->create_content_file($file, file_storage::EDITOR_FILEAREA, $h5pcontentid); + $this->h5p_generator->create_content_file($file, 'draft', $h5pcontentid); - $editorfile = $method->invoke(new file_storage(), file_storage::EDITOR_FILEAREA, $h5pcontentid, $file); + $editorfile = $method->invoke(new file_storage(), 'draft', $h5pcontentid, $file); // Check that it returns an instance of store_file. $this->assertInstanceOf('stored_file', $editorfile); @@ -692,6 +693,9 @@ class h5p_file_storage_testcase extends \advanced_testcase { */ public function test_cloneContentFile(): void { + $admin = get_admin(); + $usercontext = \context_user::instance($admin->id); + $this->setUser($admin); // Upload a file to the editor. $file = 'images/fake.jpg'; $filepath = '/'.dirname($file).'/'; @@ -700,9 +704,9 @@ class h5p_file_storage_testcase extends \advanced_testcase { $content = 'abcd'; $filerecord = array( - 'contextid' => $this->h5p_fs_context->id, - 'component' => file_storage::COMPONENT, - 'filearea' => file_storage::EDITOR_FILEAREA, + 'contextid' => $usercontext->id, + 'component' => 'user', + 'filearea' => 'draft', 'itemid' => 0, 'filepath' => $filepath, 'filename' => $filename, @@ -731,7 +735,9 @@ class h5p_file_storage_testcase extends \advanced_testcase { $filename = basename($file); $sourcecontentid = 111; - $filerecord['filearea'] = 'content'; + $filerecord['contextid'] = $this->h5p_fs_context->id; + $filerecord['component'] = file_storage::COMPONENT; + $filerecord['filearea'] = file_storage::CONTENT_FILEAREA; $filerecord['itemid'] = $sourcecontentid; $filerecord['filepath'] = $filepath; $filerecord['filename'] = $filename; diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 23c54ee256c..5e5c3d1a612 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -42,6 +42,8 @@ information provided here is intended especially for developers. be called before executing a task, and a new function \core\task\manager::get_running_tasks() returns information about currently-running tasks. * New library function rename_to_unused_name() to rename a file within its current location. +* Constant \core_h5p\file_storage::EDITOR_FILEAREA has been deprecated + because it's not required any more. === 3.9 === * Following function has been deprecated, please use \core\task\manager::run_from_cli(). -- 2.43.0