From 66455e02d8e293572d4ad523fdea0ed287c4cce3 Mon Sep 17 00:00:00 2001 From: Sara Arjona Date: Thu, 13 Aug 2020 14:36:26 +0200 Subject: [PATCH] MDL-69269 contentbank: add support to download content --- contentbank/classes/contenttype.php | 62 +++++++++++++++ .../contenttype/h5p/classes/contenttype.php | 2 +- .../h5p/tests/contenttype_h5p_test.php | 36 +++++++++ .../tests/behat/download_content.feature | 61 +++++++++++++++ contentbank/tests/contenttype_test.php | 77 ++++++++++++++++++- contentbank/view.php | 9 +++ lang/en/role.php | 1 + lib/db/access.php | 11 +++ version.php | 2 +- 9 files changed, 255 insertions(+), 6 deletions(-) create mode 100644 contentbank/tests/behat/download_content.feature diff --git a/contentbank/classes/contenttype.php b/contentbank/classes/contenttype.php index 699cee0cdb7..614dd4ed958 100644 --- a/contentbank/classes/contenttype.php +++ b/contentbank/classes/contenttype.php @@ -46,6 +46,11 @@ abstract class contenttype { /** Plugin implements edition feature */ const CAN_EDIT = 'edit'; + /** Plugin implements download feature + * @since Moodle 3.10 + */ + const CAN_DOWNLOAD = 'download'; + /** @var \context This contenttype's context. **/ protected $context = null; @@ -220,6 +225,31 @@ abstract class contenttype { return ''; } + /** + * Returns the URL to download the content. + * + * @since Moodle 3.10 + * @param content $content The content to be downloaded. + * @return string URL with the content to download. + */ + public function get_download_url(content $content): string { + $downloadurl = ''; + $file = $content->get_file(); + if (!empty($file)) { + $url = \moodle_url::make_pluginfile_url( + $file->get_contextid(), + $file->get_component(), + $file->get_filearea(), + $file->get_itemid(), + $file->get_filepath(), + $file->get_filename() + ); + $downloadurl = $url->out(false); + } + + return $downloadurl; + } + /** * Returns the HTML code to render the icon for content bank contents. * @@ -392,6 +422,38 @@ abstract class contenttype { return true; } + /** + * Returns whether or not the user has permission to download the content. + * + * @since Moodle 3.10 + * @param content $content The content to be downloaded. + * @return bool True if the user can download the content. False otherwise. + */ + final public function can_download(content $content): bool { + if (!$this->is_feature_supported(self::CAN_DOWNLOAD)) { + return false; + } + + if (!$this->can_access()) { + return false; + } + + $hascapability = has_capability('moodle/contentbank:downloadcontent', $this->context); + return $hascapability && $this->is_download_allowed($content); + } + + /** + * Returns plugin allows downloading. + * + * @since Moodle 3.10 + * @param content $content The content to be downloaed. + * @return bool True if plugin allows downloading. False otherwise. + */ + protected function is_download_allowed(content $content): bool { + // Plugins can overwrite this function to add any check they need. + return true; + } + /** * Returns the plugin supports the feature. * diff --git a/contentbank/contenttype/h5p/classes/contenttype.php b/contentbank/contenttype/h5p/classes/contenttype.php index 1c1f2ea1df9..e16a30e6a12 100644 --- a/contentbank/contenttype/h5p/classes/contenttype.php +++ b/contentbank/contenttype/h5p/classes/contenttype.php @@ -110,7 +110,7 @@ class contenttype extends \core_contentbank\contenttype { * @return array */ protected function get_implemented_features(): array { - return [self::CAN_UPLOAD, self::CAN_EDIT]; + return [self::CAN_UPLOAD, self::CAN_EDIT, self::CAN_DOWNLOAD]; } /** diff --git a/contentbank/contenttype/h5p/tests/contenttype_h5p_test.php b/contentbank/contenttype/h5p/tests/contenttype_h5p_test.php index c708516718a..c8061345eea 100644 --- a/contentbank/contenttype/h5p/tests/contenttype_h5p_test.php +++ b/contentbank/contenttype/h5p/tests/contenttype_h5p_test.php @@ -147,4 +147,40 @@ class contenttype_h5p_contenttype_plugin_testcase extends advanced_testcase { $this->assertNotEquals($defaulticon, $findicon); $this->assertContains('find', $findicon, '', true); } + + /** + * Tests get_download_url result. + * + * @covers ::get_download_url + */ + public function test_get_download_url() { + global $CFG; + + $this->resetAfterTest(); + $systemcontext = context_system::instance(); + $this->setAdminUser(); + $contenttype = new contenttype_h5p\contenttype($systemcontext); + + // Add an H5P fill the blanks file to the content bank. + $filename = 'filltheblanks.h5p'; + $filepath = $CFG->dirroot . '/h5p/tests/fixtures/' . $filename; + $generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank'); + $contents = $generator->generate_contentbank_data('contenttype_h5p', 1, 0, $systemcontext, true, $filepath); + $filltheblanks = array_shift($contents); + + // Check before deploying the URL is returned OK. + $url1 = $contenttype->get_download_url($filltheblanks); + $this->assertNotEmpty($url1); + $this->assertContains($filename, $url1); + + // Deploy the contents though the player to create the H5P DB entries and know specific content type. + $h5pplayer = new \core_h5p\player($filltheblanks->get_file_url(), new \stdClass(), true); + $h5pplayer->add_assets_to_page(); + $h5pplayer->output(); + + // Once the H5P has been deployed, the URL is still the same. + $url2 = $contenttype->get_download_url($filltheblanks); + $this->assertNotEmpty($url2); + $this->assertEquals($url1, $url2); + } } diff --git a/contentbank/tests/behat/download_content.feature b/contentbank/tests/behat/download_content.feature new file mode 100644 index 00000000000..9fca55706fa --- /dev/null +++ b/contentbank/tests/behat/download_content.feature @@ -0,0 +1,61 @@ +@core @core_contentbank @contentbank_h5p @_file_upload @javascript +Feature: Download H5P content from the content bank + In order export H5P content from the content bank + As an admin + I need to be able to download any H5P content from the content bank + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | manager | Max | Manager | man@example.com | + And the following "role assigns" exist: + | user | role | contextlevel | reference | + | manager | manager | System | | + And the following "contentbank content" exist: + | contextlevel | reference | contenttype | user | contentname | filepath | + | System | | contenttype_h5p | admin | filltheblanksadmin.h5p | /h5p/tests/fixtures/filltheblanks.h5p | + | System | | contenttype_h5p | manager | filltheblanksmanager.h5p | /h5p/tests/fixtures/filltheblanks.h5p | + And I log in as "admin" + And I am on site homepage + And I turn editing mode on + And I add the "Navigation" block if not present + And I configure the "Navigation" block + And I set the following fields to these values: + | Page contexts | Display throughout the entire site | + And I press "Save changes" + + Scenario: Admins can download content from the content bank + Given I click on "Site pages" "list_item" in the "Navigation" "block" + And I click on "Content bank" "link" in the "Navigation" "block" + And I follow "filltheblanksmanager.h5p" + And I open the action menu in "region-main-settings-menu" "region" + And I should see "Download" + When I choose "Download" in the open action menu + Then I should see "filltheblanksmanager.h5p" + + Scenario: Users can download content created by different users + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/contentbank:manageanycontent | Prohibit | manager | System | | + And I log out + And I log in as "manager" + When I click on "Site pages" "list_item" in the "Navigation" "block" + And I click on "Content bank" "link" in the "Navigation" "block" + And I should see "filltheblanksadmin.h5p" + And I follow "filltheblanksadmin.h5p" + And I open the action menu in "region-main-settings-menu" "region" + Then I should see "Download" + And I should not see "Rename" + + Scenario: Users without the required capability cannot download content + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/contentbank:downloadcontent | Prohibit | manager | System | | + And I log out + And I log in as "manager" + When I click on "Site pages" "list_item" in the "Navigation" "block" + And I click on "Content bank" "link" in the "Navigation" "block" + And I should see "filltheblanksmanager.h5p" + And I follow "filltheblanksmanager.h5p" + And I open the action menu in "region-main-settings-menu" "region" + Then I should not see "Download" diff --git a/contentbank/tests/contenttype_test.php b/contentbank/tests/contenttype_test.php index c8ae908696e..2726fc712fb 100644 --- a/contentbank/tests/contenttype_test.php +++ b/contentbank/tests/contenttype_test.php @@ -348,7 +348,7 @@ class core_contenttype_contenttype_testcase extends \advanced_testcase { /** * Helper function to setup 3 users (manager1, manager2 and user) and 4 contents (3 created by manager1 and 1 by user). */ - protected function contenttype_setup_scenario_data(): void { + protected function contenttype_setup_scenario_data(string $contenttype = 'contenttype_testable'): void { global $DB; $systemcontext = context_system::instance(); @@ -358,14 +358,17 @@ class core_contenttype_contenttype_testcase extends \advanced_testcase { $this->managerroleid = $DB->get_field('role', 'id', array('shortname' => 'manager')); $this->getDataGenerator()->role_assign($this->managerroleid, $this->manager1->id); $this->getDataGenerator()->role_assign($this->managerroleid, $this->manager2->id); + $editingteacherrolerid = $DB->get_field('role', 'id', ['shortname' => 'editingteacher']); $this->user = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->role_assign($editingteacherrolerid, $this->user->id); // Add some content to the content bank. $generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank'); - $this->contents[$this->manager1->id] = $generator->generate_contentbank_data(null, 3, $this->manager1->id); - $this->contents[$this->user->id] = $generator->generate_contentbank_data(null, 1, $this->user->id); + $this->contents[$this->manager1->id] = $generator->generate_contentbank_data($contenttype, 3, $this->manager1->id); + $this->contents[$this->user->id] = $generator->generate_contentbank_data($contenttype, 1, $this->user->id); - $this->contenttype = new \contenttype_testable\contenttype($systemcontext); + $contenttypeclass = "\\$contenttype\\contenttype"; + $this->contenttype = new $contenttypeclass($systemcontext); } /** @@ -510,4 +513,70 @@ class core_contenttype_contenttype_testcase extends \advanced_testcase { $this->assertFalse($contenttype->can_manage($contentbyteacher)); $this->assertFalse($contenttype->can_manage($contentbyadmin)); } + + /** + * Test the behaviour of can_download(). + * + * @covers ::can_download + */ + public function test_can_download() { + global $DB; + + $this->resetAfterTest(); + $this->contenttype_setup_scenario_data('contenttype_h5p'); + + $managercontent = array_shift($this->contents[$this->manager1->id]); + $usercontent = array_shift($this->contents[$this->user->id]); + + // Check the content has been created as expected. + $records = $DB->count_records('contentbank_content'); + $this->assertEquals(4, $records); + + // Check user can download content created by anybody. + $this->setUser($this->user); + $this->assertTrue($this->contenttype->can_download($usercontent)); + $this->assertTrue($this->contenttype->can_download($managercontent)); + + // Check manager can download all the content too. + $this->setUser($this->manager1); + $this->assertTrue($this->contenttype->can_download($managercontent)); + $this->assertTrue($this->contenttype->can_download($usercontent)); + + // Unassign capability to manager role and check she cannot download content anymore. + unassign_capability('moodle/contentbank:downloadcontent', $this->managerroleid); + $this->assertFalse($this->contenttype->can_download($managercontent)); + $this->assertFalse($this->contenttype->can_download($usercontent)); + } + + /** + * Tests get_download_url result. + * + * @covers ::get_download_url + */ + public function test_get_download_url() { + global $CFG; + + $this->resetAfterTest(); + $this->setAdminUser(); + $systemcontext = context_system::instance(); + + // Add some content to the content bank. + $filename = 'filltheblanks.h5p'; + $filepath = $CFG->dirroot . '/h5p/tests/fixtures/' . $filename; + $generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank'); + $contents = $generator->generate_contentbank_data('contenttype_testable', 1, 0, $systemcontext, true, $filepath); + $content = array_shift($contents); + + // Check the URL is returned OK for a content with file. + $contenttype = new contenttype($systemcontext); + $url = $contenttype->get_download_url($content); + $this->assertNotEmpty($url); + $this->assertContains($filename, $url); + + // Check the URL is empty when the content hasn't any file. + $record = new stdClass(); + $content = $contenttype->create_content($record); + $url = $contenttype->get_download_url($content); + $this->assertEmpty($url); + } } diff --git a/contentbank/view.php b/contentbank/view.php index 5fd66d1addf..a482d8034cc 100644 --- a/contentbank/view.php +++ b/contentbank/view.php @@ -84,6 +84,15 @@ if ($contenttype->can_manage($content)) { $attributes )); } +if ($contenttype->can_download($content)) { + // Add the download content item to the menu. + $actionmenu->add_secondary_action(new action_menu_link( + new moodle_url($contenttype->get_download_url($content)), + new pix_icon('t/download', get_string('download')), + get_string('download'), + false + )); +} if ($contenttype->can_delete($content)) { // Add the delete content item to the menu. $attributes = [ diff --git a/lang/en/role.php b/lang/en/role.php index 06ef6bb976c..625c0c029b8 100644 --- a/lang/en/role.php +++ b/lang/en/role.php @@ -153,6 +153,7 @@ $string['confirmunassignno'] = 'Cancel'; $string['contentbank:access'] = 'Access the content bank'; $string['contentbank:deleteanycontent'] = 'Delete any content from the content bank'; $string['contentbank:deleteowncontent'] = 'Delete content from own content bank'; +$string['contentbank:downloadcontent'] = 'Download a content from the content bank'; $string['contentbank:manageanycontent'] = 'Manage any content from the content bank'; $string['contentbank:manageowncontent'] = 'Manage content from own content bank'; $string['contentbank:upload'] = 'Upload new content to the content bank'; diff --git a/lib/db/access.php b/lib/db/access.php index 3de202d9de2..5078f4d3daf 100644 --- a/lib/db/access.php +++ b/lib/db/access.php @@ -2565,4 +2565,15 @@ $capabilities = array( 'editingteacher' => CAP_ALLOW, ) ], + + // Allow users to download content. + 'moodle/contentbank:downloadcontent' => [ + 'captype' => 'read', + 'contextlevel' => CONTEXT_COURSE, + 'archetypes' => [ + 'manager' => CAP_ALLOW, + 'coursecreator' => CAP_ALLOW, + 'editingteacher' => CAP_ALLOW, + ] + ], ); diff --git a/version.php b/version.php index c819ce207e6..b99ac133f24 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2021052500.07; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2021052500.08; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '4.0dev (Build: 20200827)'; // Human-friendly version name