diff --git a/contentbank/classes/content.php b/contentbank/classes/content.php index 37e3385de72..227294bb60d 100644 --- a/contentbank/classes/content.php +++ b/contentbank/classes/content.php @@ -28,6 +28,7 @@ use core_text; use stored_file; use stdClass; use coding_exception; +use context; use moodle_url; use core\event\contentbank_content_updated; @@ -85,6 +86,17 @@ abstract class content { return $this->content->contenttype; } + /** + * Return the contenttype instance of this content. + * + * @return contenttype The content type instance + */ + public function get_content_type_instance(): contenttype { + $context = context::instance_by_id($this->content->contextid); + $contenttypeclass = "\\{$this->content->contenttype}\\contenttype"; + return new $contenttypeclass($context); + } + /** * Returns $this->content->timemodified. * diff --git a/contentbank/classes/contentbank.php b/contentbank/classes/contentbank.php index 307b94938db..1e5c93415bb 100644 --- a/contentbank/classes/contentbank.php +++ b/contentbank/classes/contentbank.php @@ -334,4 +334,18 @@ class contentbank { return $contenttypes; } + + /** + * Return a content class form a content id. + * + * @throws coding_exception if the ID is not valid or some class does no exists + * @param int $id the content id + * @return content the content class instance + */ + public function get_content_from_id(int $id): content { + global $DB; + $record = $DB->get_record('contentbank_content', ['id' => $id], '*', MUST_EXIST); + $contentclass = "\\$record->contenttype\\content"; + return new $contentclass($record); + } } diff --git a/contentbank/classes/contenttype.php b/contentbank/classes/contenttype.php index 57bf4248611..8f9fec3be42 100644 --- a/contentbank/classes/contenttype.php +++ b/contentbank/classes/contenttype.php @@ -121,6 +121,21 @@ abstract class contenttype { return $content; } + /** + * Replace a content using an uploaded file. + * + * @throws file_exception If file operations fail + * @throws dml_exception if the content creation fails + * @param stored_file $file the uploaded file + * @param content $content the original content record + * @return content Object with the updated content bank information. + */ + public function replace_content(stored_file $file, content $content): content { + $content->import_file($file); + $content->update_content(); + return $content; + } + /** * Delete this content from the content_bank. * This method can be overwritten by the plugins if they need to delete specific information. diff --git a/contentbank/contenttype/h5p/tests/behat/admin_replace_content.feature b/contentbank/contenttype/h5p/tests/behat/admin_replace_content.feature new file mode 100644 index 00000000000..bc5202c50b5 --- /dev/null +++ b/contentbank/contenttype/h5p/tests/behat/admin_replace_content.feature @@ -0,0 +1,30 @@ +@core @core_contentbank @contenttype_h5p @_file_upload @_switch_iframe @javascript +Feature: Replace H5P file from an existing content + In order to replace an H5P content from the content bank + As an admin + I need to be able to replace the content with a new .h5p file + + Background: + Given the following "contentbank content" exist: + | contextlevel | reference | contenttype | user | contentname | filepath | + | System | | contenttype_h5p | admin | filltheblanks.h5p | /h5p/tests/fixtures/filltheblanks.h5p | + And I log in as "admin" + And I press "Customise this page" + And I add the "Navigation" block if not present + And I expand "Site pages" node + And I click on "Content bank" "link" + + Scenario: Admins can replace the original .h5p file with a new one + Given I click on "filltheblanks.h5p" "link" + And I switch to "h5p-player" class iframe + And I switch to "h5p-iframe" class iframe + And I should see "Of which countries" + And I switch to the main frame + When I open the action menu in "region-main-settings-menu" "region" + And I choose "Replace with file" in the open action menu + And I upload "h5p/tests/fixtures/ipsums.h5p" file to "Upload content" filemanager + And I click on "Save changes" "button" + Then I switch to "h5p-player" class iframe + And I switch to "h5p-iframe" class iframe + And I should see "Lorum ipsum" + And I switch to the main frame diff --git a/contentbank/contenttype/h5p/tests/behat/teacher_replace_content.feature b/contentbank/contenttype/h5p/tests/behat/teacher_replace_content.feature new file mode 100644 index 00000000000..5a53f5ca616 --- /dev/null +++ b/contentbank/contenttype/h5p/tests/behat/teacher_replace_content.feature @@ -0,0 +1,67 @@ +@core @core_contentbank @contenttype_h5p @_file_upload @_switch_iframe @javascript +Feature: Replace H5P file from an existing content requires special capabilities + In order replace an H5P content from the content bank + As a teacher + I need to be able to replace the content only if certain capabilities are allowed + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + And the following "categories" exist: + | name | category | idnumber | + | Cat 1 | 0 | CAT1 | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | CAT1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + And the following "contentbank content" exist: + | contextlevel | reference | contenttype | user | contentname | filepath | + | Course | C1 | contenttype_h5p | admin | admincontent | /h5p/tests/fixtures/ipsums.h5p | + | Course | C1 | contenttype_h5p | teacher1 | teachercontent | /h5p/tests/fixtures/filltheblanks.h5p | + And I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on + And I add the "Navigation" block if not present + And I expand "Site pages" node + And I click on "Content bank" "link" + # Force the content deploy + And I click on "admincontent" "link" + And I click on "Content bank" "link" + + Scenario: Teacher can replace its own H5P files + Given I click on "teachercontent" "link" + When I open the action menu in "region-main-settings-menu" "region" + And I choose "Replace with file" in the open action menu + And I upload "h5p/tests/fixtures/ipsums.h5p" file to "Upload content" filemanager + And I click on "Save changes" "button" + Then I switch to "h5p-player" class iframe + And I switch to "h5p-iframe" class iframe + And I should see "Lorum ipsum" + And I switch to the main frame + + Scenario: Teacher cannot replace another user's H5P files + When I click on "admincontent" "link" + Then "region-main-settings-menu" "region" should not exist + + Scenario: Teacher cannot replace a content without having upload capability + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/contentbank:upload | Prevent | editingteacher | Course | C1 | + When I click on "teachercontent" "link" + Then "region-main-settings-menu" "region" should not exist + + Scenario: Teacher cannot replace a content without having the H5P upload capability + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | contenttype/h5p:upload | Prevent | editingteacher | Course | C1 | + When I click on "teachercontent" "link" + Then "region-main-settings-menu" "region" should not exist + + Scenario: Teacher cannot replace a content without having the manage own content capability + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/contentbank:manageowncontent | Prevent | editingteacher | Course | C1 | + When I click on "teachercontent" "link" + Then "region-main-settings-menu" "region" should not exist diff --git a/contentbank/files_form.php b/contentbank/files_form.php index d94f61fa563..6a001f05675 100644 --- a/contentbank/files_form.php +++ b/contentbank/files_form.php @@ -44,6 +44,11 @@ class contentbank_files_form extends moodleform { $mform->addElement('hidden', 'contextid', $this->_customdata['contextid']); $mform->setType('contextid', PARAM_INT); + if (!empty($this->_customdata['id'])) { + $mform->addElement('hidden', 'id', $this->_customdata['id']); + $mform->setType('id', PARAM_INT); + } + $options = $this->_customdata['options']; $mform->addElement('filepicker', 'file', get_string('file', 'core_contentbank'), null, $options); $mform->addHelpButton('file', 'file', 'core_contentbank'); diff --git a/contentbank/tests/content_test.php b/contentbank/tests/content_test.php index ddfff9422ae..4c7ec2b8a95 100644 --- a/contentbank/tests/content_test.php +++ b/contentbank/tests/content_test.php @@ -275,4 +275,27 @@ class core_contenttype_content_testcase extends \advanced_testcase { $contentfile = $content->get_file($file); $this->assertEquals($importedfile->get_id(), $contentfile->get_id()); } + + /** + * Tests for 'get_content_type_instance' + * + * @covers ::get_content_type_instance + */ + public function test_get_content_type_instance(): void { + global $USER; + $this->resetAfterTest(); + $this->setAdminUser(); + $context = context_system::instance(); + + $type = new contenttype($context); + $record = (object)[ + 'name' => 'content name', + 'usercreated' => $USER->id, + ]; + $content = $type->create_content($record); + + $contenttype = $content->get_content_type_instance(); + + $this->assertInstanceOf(get_class($type), $contenttype); + } } diff --git a/contentbank/tests/contentbank_test.php b/contentbank/tests/contentbank_test.php index cd22e80a0e0..3d6a703a77f 100644 --- a/contentbank/tests/contentbank_test.php +++ b/contentbank/tests/contentbank_test.php @@ -31,6 +31,7 @@ use advanced_testcase; use context_course; use context_coursecat; use context_system; +use Exception; global $CFG; require_once($CFG->dirroot . '/contentbank/tests/fixtures/testable_contenttype.php'); @@ -603,4 +604,31 @@ class core_contentbank_testcase extends advanced_testcase { $actual = $cb->get_contenttypes_with_capability_feature('test2', null, $enabled); $this->assertEquals($contenttypescanfeature, array_values($actual)); } + + /** + * Test the behaviour of get_content_from_id() + * + * @covers ::get_content_from_id + */ + public function test_get_content_from_id() { + + $this->resetAfterTest(); + $cb = new \core_contentbank\contentbank(); + + // Create a category and two courses. + $systemcontext = context_system::instance(); + + // Add some content to the content bank. + $generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank'); + $contents = $generator->generate_contentbank_data(null, 3, 0, $systemcontext); + $content = reset($contents); + + // Get the content instance form id. + $newinstance = $cb->get_content_from_id($content->get_id()); + $this->assertEquals($content->get_id(), $newinstance->get_id()); + + // Now produce and exception with an innexistent id. + $this->expectException(Exception::class); + $cb->get_content_from_id(0); + } } diff --git a/contentbank/tests/contenttype_test.php b/contentbank/tests/contenttype_test.php index 2726fc712fb..1995357ffd5 100644 --- a/contentbank/tests/contenttype_test.php +++ b/contentbank/tests/contenttype_test.php @@ -290,6 +290,84 @@ class core_contenttype_contenttype_testcase extends \advanced_testcase { $this->assertEquals(1, $DB->count_records('files', ['contenthash' => $dummyfile->get_contenthash()])); } + /** + * Tests for behaviour of replace_content() using a dummy file. + * + * @covers ::replace_content + */ + public function test_replace_content(): void { + global $USER; + + $this->resetAfterTest(); + $this->setAdminUser(); + $context = context_system::instance(); + + // Add some content to the content bank. + $generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank'); + $contents = $generator->generate_contentbank_data('contenttype_testable', 3, 0, $context); + $content = reset($contents); + + $dummy = [ + 'contextid' => context_user::instance($USER->id)->id, + 'component' => 'user', + 'filearea' => 'draft', + 'itemid' => 1, + 'filepath' => '/', + 'filename' => 'file.h5p', + 'userid' => $USER->id, + ]; + $fs = get_file_storage(); + $dummyfile = $fs->create_file_from_string($dummy, 'Dummy content'); + + $contenttype = new contenttype(context_system::instance()); + $content = $contenttype->replace_content($dummyfile, $content); + + $this->assertEquals('contenttype_testable', $content->get_content_type()); + $this->assertInstanceOf('\\contenttype_testable\\content', $content); + + $file = $content->get_file(); + $this->assertEquals($dummyfile->get_userid(), $file->get_userid()); + $this->assertEquals($dummyfile->get_contenthash(), $file->get_contenthash()); + $this->assertEquals('contentbank', $file->get_component()); + $this->assertEquals('public', $file->get_filearea()); + $this->assertEquals('/', $file->get_filepath()); + } + + /** + * Tests for behaviour of replace_content() using an error file. + * + * @covers ::replace_content + */ + public function test_replace_content_exception(): void { + global $USER; + + $this->resetAfterTest(); + $this->setAdminUser(); + $context = context_system::instance(); + + // Add some content to the content bank. + $generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank'); + $contents = $generator->generate_contentbank_data('contenttype_testable', 3, 0, $context); + $content = reset($contents); + + $dummy = [ + 'contextid' => context_user::instance($USER->id)->id, + 'component' => 'user', + 'filearea' => 'draft', + 'itemid' => 1, + 'filepath' => '/', + 'filename' => 'error.txt', + 'userid' => $USER->id, + ]; + $fs = get_file_storage(); + $dummyfile = $fs->create_file_from_string($dummy, 'Dummy content'); + + $contenttype = new contenttype(context_system::instance()); + + $this->expectException(Exception::class); + $content = $contenttype->replace_content($dummyfile, $content); + } + /** * Test the behaviour of can_delete(). */ diff --git a/contentbank/upload.php b/contentbank/upload.php index 00cc40cbf07..4410de44a25 100644 --- a/contentbank/upload.php +++ b/contentbank/upload.php @@ -34,6 +34,17 @@ $context = context::instance_by_id($contextid, MUST_EXIST); require_capability('moodle/contentbank:upload', $context); +$cb = new \core_contentbank\contentbank(); + +$id = optional_param('id', null, PARAM_INT); +if ($id) { + $content = $cb->get_content_from_id($id); + $contenttype = $content->get_content_type_instance(); + if (!$contenttype->can_manage($content) || !$contenttype->can_upload()) { + print_error('nopermissions', 'error', $returnurl, get_string('replacecontent', 'contentbank')); + } +} + $title = get_string('contentbank'); \core_contentbank\helper::get_page_ready($context, $title, true); if ($PAGE->course) { @@ -55,8 +66,12 @@ if (has_capability('moodle/user:ignoreuserquota', $context)) { $maxareabytes = FILE_AREA_MAX_BYTES_UNLIMITED; } -$cb = new \core_contentbank\contentbank(); -$accepted = $cb->get_supported_extensions_as_string($context); +if ($id) { + $extensions = $contenttype->get_manageable_extensions(); + $accepted = implode(',', $extensions); +} else { + $accepted = $cb->get_supported_extensions_as_string($context); +} $data = new stdClass(); $options = array( @@ -68,7 +83,7 @@ $options = array( ); file_prepare_standard_filemanager($data, 'files', $options, $context, 'contentbank', 'public', 0); -$mform = new contentbank_files_form(null, ['contextid' => $contextid, 'data' => $data, 'options' => $options]); +$mform = new contentbank_files_form(null, ['contextid' => $contextid, 'data' => $data, 'options' => $options, 'id' => $id]); $error = ''; @@ -82,7 +97,11 @@ if ($mform->is_cancelled()) { $files = $fs->get_area_files($usercontext->id, 'user', 'draft', $formdata->file, 'itemid, filepath, filename', false); if (!empty($files)) { $file = reset($files); - $content = $cb->create_content_from_file($context, $USER->id, $file); + if ($id) { + $content = $contenttype->replace_content($file, $content); + } else { + $content = $cb->create_content_from_file($context, $USER->id, $file); + } $viewurl = new \moodle_url('/contentbank/view.php', ['id' => $content->get_id(), 'contextid' => $contextid]); redirect($viewurl); } else { diff --git a/contentbank/view.php b/contentbank/view.php index a482d8034cc..4c84b1bbd81 100644 --- a/contentbank/view.php +++ b/contentbank/view.php @@ -83,6 +83,15 @@ if ($contenttype->can_manage($content)) { false, $attributes )); + + if ($contenttype->can_upload()) { + $actionmenu->add_secondary_action(new action_menu_link( + new moodle_url('/contentbank/upload.php', ['contextid' => $context->id, 'id' => $content->get_id()]), + new pix_icon('i/upload', get_string('upload')), + get_string('replacecontent', 'contentbank'), + false + )); + } } if ($contenttype->can_download($content)) { // Add the download content item to the menu. diff --git a/lang/en/contentbank.php b/lang/en/contentbank.php index f614161c111..6535c9354fb 100644 --- a/lang/en/contentbank.php +++ b/lang/en/contentbank.php @@ -36,6 +36,7 @@ $string['contenttypenoedit'] = 'You can not edit this content'; $string['emptynamenotallowed'] = 'Empty name is not allowed'; $string['eventcontentcreated'] = 'Content created'; $string['eventcontentdeleted'] = 'Content deleted'; +$string['eventcontentreplaced'] = 'Content replaced with file'; $string['eventcontentupdated'] = 'Content updated'; $string['eventcontentuploaded'] = 'Content uploaded'; $string['eventcontentviewed'] = 'Content viewed'; @@ -64,6 +65,7 @@ $string['privacy:metadata:userid'] = 'The ID of the user creating or modifying c $string['privacy:request:preference:set'] = 'The value of the setting \'{$a->name}\' was \'{$a->value}\''; $string['rename'] = 'Rename'; $string['renamecontent'] = 'Rename content'; +$string['replacecontent'] = 'Replace with file'; $string['searchcontentbankbyname'] = 'Search for content by name'; $string['size'] = 'Size'; $string['timecreated'] = 'Time created';