Skip to content

Commit

Permalink
MDL-71956 core_h5p: Add more scenarios to can_edit_content
Browse files Browse the repository at this point in the history
The method can_edit_content() now supports more scenarios where the
H5P content can be edited:
- Instead of supporting only mod_h5pactivity, now it supports any
mod or block when the user has the addinstance capability.
- If the component implements the can_edit_content method in the
h5p\canedit class and it returns true. For instance, the mod_forum
implements it and return true when filearea is post, if the user
can edit the post where the H5P is.
  • Loading branch information
sarjona committed Nov 9, 2021
1 parent d135a12 commit eb4e364
Show file tree
Hide file tree
Showing 5 changed files with 392 additions and 12 deletions.
27 changes: 20 additions & 7 deletions h5p/classes/api.php
Expand Up @@ -267,7 +267,8 @@ public static function get_original_content_from_pluginfile_url(string $url, boo
* - The user is the author of the file.
* - The component is different from user (i.e. private files).
* - If the component is contentbank, the user can edit this file (calling the ContentBank API).
* - If the component is mod_h5pactivity, the user has the addinstance capability.
* - If the component is mod_xxx or block_xxx, the user has the addinstance capability.
* - If the component implements the can_edit_content in the h5p\canedit class and the callback to this method returns true.
*
* @param \stored_file $file The H5P file to check.
*
Expand All @@ -277,25 +278,37 @@ public static function get_original_content_from_pluginfile_url(string $url, boo
public static function can_edit_content(\stored_file $file): bool {
global $USER;

list($type, $component) = \core_component::normalize_component($file->get_component());

// Private files.
$currentuserisauthor = $file->get_userid() == $USER->id;
$isuserfile = $file->get_component() === 'user';
$isuserfile = $component === 'user';
if ($currentuserisauthor && $isuserfile) {
// The user can edit the content because it's a private user file and she is the owner.
return true;
}

// For mod_h5pactivity, check whether the user can add/edit them.
if ($file->get_component() === 'mod_h5pactivity') {
// Check if the plugin where the file belongs implements the custom can_edit_content method and call it if that's the case.
$classname = '\\' . $file->get_component() . '\\h5p\\canedit';
$methodname = 'can_edit_content';
if (method_exists($classname, $methodname)) {
return $classname::{$methodname}($file);
}

// For mod/block files, check if the user has the addinstance capability of the component where the file belongs.
if ($type === 'mod' || $type === 'block') {
// For any other component, check whether the user can add/edit them.
$context = \context::instance_by_id($file->get_contextid());
if (has_capability("mod/h5pactivity:addinstance", $context)) {
// The user can edit the content because she has the capability for creating H5P activities where the file belongs.
$plugins = \core_component::get_plugin_list($type);
$isvalid = array_key_exists($component, $plugins);
if ($isvalid && has_capability("$type/$component:addinstance", $context)) {
// The user can edit the content because she has the capability for creating instances where the file belongs.
return true;
}
}

// For contentbank files, use the API to check if the user has access.
if ($file->get_component() == 'contentbank') {
if ($component == 'contentbank') {
$cb = new \core_contentbank\contentbank();
$content = $cb->get_content_from_id($file->get_itemid());
$contenttype = $content->get_content_type_instance();
Expand Down
95 changes: 90 additions & 5 deletions h5p/tests/api_test.php
Expand Up @@ -27,6 +27,8 @@

namespace core_h5p;

use stdClass;

defined('MOODLE_INTERNAL') || die();

/**
Expand Down Expand Up @@ -444,11 +446,13 @@ public function test_get_original_content_from_pluginfile_url(): void {
* @param string $fileauthor Author of the file to check.
* @param string $filecomponent Component of the file to check.
* @param bool $expected Expected result after calling the can_edit_content method.
* @param string $filearea Area of the file to check.
*
* @return void
*/
public function test_can_edit_content(string $currentuser, string $fileauthor, string $filecomponent, bool $expected): void {
global $USER;
public function test_can_edit_content(string $currentuser, string $fileauthor, string $filecomponent, bool $expected,
$filearea = 'unittest'): void {
global $USER, $DB;

$this->setRunTestInSeparateProcess(true);
$this->resetAfterTest();
Expand All @@ -472,6 +476,20 @@ public function test_can_edit_content(string $currentuser, string $fileauthor, s
$this->setUser($users[$currentuser]);
}

$itemid = rand();
if ($filearea === 'post') {
// Create a forum and add a discussion.
$forum = $this->getDataGenerator()->create_module('forum', ['course' => $course->id]);

$record = new stdClass();
$record->course = $course->id;
$record->userid = $users[$fileauthor]->id;
$record->forum = $forum->id;
$discussion = $this->getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record);
$post = $DB->get_record('forum_posts', ['discussion' => $discussion->id]);
$itemid = $post->id;
}

// Create the file.
$filename = 'greeting-card-887.h5p';
$path = __DIR__ . '/fixtures/' . $filename;
Expand All @@ -491,8 +509,8 @@ public function test_can_edit_content(string $currentuser, string $fileauthor, s
$filerecord = [
'contextid' => $context->id,
'component' => $filecomponent,
'filearea' => 'unittest',
'itemid' => rand(),
'filearea' => $filearea,
'itemid' => $itemid,
'filepath' => '/',
'filename' => basename($path),
'userid' => $users[$fileauthor]->id,
Expand Down Expand Up @@ -589,19 +607,80 @@ public function can_edit_content_provider(): array {
'expected' => false,
],

// Component = mod_book.
'mod_book: Admin user is author' => [
'currentuser' => 'admin',
'fileauthor' => 'admin',
'filecomponent' => 'mod_book',
'expected' => true,
],
'mod_book: Admin user, teacher is author' => [
'currentuser' => 'admin',
'fileauthor' => 'teacher',
'filecomponent' => 'mod_book',
'expected' => true,
],

// Component = mod_forum.
'mod_forum: Admin user is author' => [
'currentuser' => 'admin',
'fileauthor' => 'admin',
'filecomponent' => 'mod_forum',
'expected' => false,
'expected' => true,
],
'mod_forum: Admin user, teacher is author' => [
'currentuser' => 'admin',
'fileauthor' => 'teacher',
'filecomponent' => 'mod_forum',
'expected' => true,
],
'mod_forum: Teacher user, admin is author' => [
'currentuser' => 'teacher',
'fileauthor' => 'admin',
'filecomponent' => 'mod_forum',
'expected' => true,
],
'mod_forum: Student user, teacher is author' => [
'currentuser' => 'student',
'fileauthor' => 'teacher',
'filecomponent' => 'mod_forum',
'expected' => false,
],
'mod_forum/post: Admin user is author' => [
'currentuser' => 'admin',
'fileauthor' => 'admin',
'filecomponent' => 'mod_forum',
'expected' => true,
'filearea' => 'post',
],
'mod_forum/post: Teacher user, admin is author' => [
'currentuser' => 'teacher',
'fileauthor' => 'admin',
'filecomponent' => 'mod_forum',
'expected' => true,
'filearea' => 'post',
],
'mod_forum/post: Student user, teacher is author' => [
'currentuser' => 'student',
'fileauthor' => 'teacher',
'filecomponent' => 'mod_forum',
'expected' => false,
'filearea' => 'post',
],

// Component = block_html.
'block_html: Admin user is author' => [
'currentuser' => 'admin',
'fileauthor' => 'admin',
'filecomponent' => 'block_html',
'expected' => true,
],
'block_html: Admin user, teacher is author' => [
'currentuser' => 'admin',
'fileauthor' => 'teacher',
'filecomponent' => 'block_html',
'expected' => true,
],

// Component = contentbank.
'contentbank: Admin user is author' => [
Expand Down Expand Up @@ -654,6 +733,12 @@ public function can_edit_content_provider(): array {
'filecomponent' => 'mod_unexisting',
'expected' => false,
],
'Unexisting block' => [
'currentuser' => 'admin',
'fileauthor' => 'admin',
'filecomponent' => 'block_unexisting',
'expected' => false,
],
];
}

Expand Down
3 changes: 3 additions & 0 deletions h5p/upgrade.txt
Expand Up @@ -6,6 +6,9 @@ information provided here is intended especially for developers.
* Added edit.php and editcontent_form class, for modifying H5P content given an H5P identifier (from the h5p table).
* Added a new parameter to the player::display method, to define whether the edit button should be displayed below the
H5P content or not. Default value for this parameter is false.
* H5P subsystem is allowed to act as an API (level 2) too.
* Plugins can now implement h5p\canedit::can_edit_content method to define, if required, any custom behaviour for deciding
whether an H5P content can be edited or not. The specific plugin check will completely override the generic check.

=== 3.11 ===
* Added $skipcapcheck parameter to H5P constructor, api::create_content_from_pluginfile_url() and
Expand Down
75 changes: 75 additions & 0 deletions mod/forum/classes/h5p/canedit.php
@@ -0,0 +1,75 @@
<?php
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

namespace mod_forum\h5p;

/**
* Class to check if the H5P content can be edited for this plugin.
*
* @package mod_forum
* @copyright 2021 Sara Arjona (sara@moodle.com)
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class canedit {

/**
* Check if the user can edit an H5P file. In that case, this method will return true if the file belongs to mod_forum
* filearea is post and the user can edit the post where the H5P is.
*
* @param \stored_file $file The H5P file to check.
*
* @return boolean Whether the user can edit or not the given file.
* @since Moodle 4.0
*/
public static function can_edit_content(\stored_file $file): bool {
global $USER;

list($type, $component) = \core_component::normalize_component($file->get_component());

if ($type === 'mod' && $component === 'forum') {
// For mod_forum files in posts, check if the user can edit the post where the H5P is.
if ($file->get_filearea() === 'post') {
// Check if the user can edit the forum post.
$vaultfactory = \mod_forum\local\container::get_vault_factory();
$forumvault = $vaultfactory->get_forum_vault();
$discussionvault = $vaultfactory->get_discussion_vault();
$postvault = $vaultfactory->get_post_vault();
$postid = $file->get_itemid();
$postentity = $postvault->get_from_id($postid);
if (!empty($postentity)) {
$discussionentity = $discussionvault->get_from_id($postentity->get_discussion_id());
$managerfactory = \mod_forum\local\container::get_manager_factory();
$forumentity = $forumvault->get_from_id($discussionentity->get_forum_id());
$capabilitymanager = $managerfactory->get_capability_manager($forumentity);
if ($capabilitymanager->can_edit_post($USER, $discussionentity, $postentity)) {
return true;
}
}
} else {
// For any other fileare, check whether the user can add/edit them.
$context = \context::instance_by_id($file->get_contextid());
$plugins = \core_component::get_plugin_list($type);
$isvalid = array_key_exists($component, $plugins);
if ($isvalid && has_capability("$type/$component:addinstance", $context)) {
// The user can edit the content because she has the capability for creating instances where the file belongs.
return true;
}
}
}

return false;
}
}

0 comments on commit eb4e364

Please sign in to comment.