Skip to content

Commit

Permalink
MDL-69331 core_h5p: Display error when main library is disabled
Browse files Browse the repository at this point in the history 
H5P contents with the main library disabled won't be deployed; a
message error will be displayed instead of.
  • Loading branch information
@sarjona
sarjona committed Apr 15, 2021
1 parent dc77f13 commit be71433
Show file tree
Hide file tree
Showing 5 changed files with 252 additions and 6 deletions.
50 changes: 50 additions & 0 deletions filter/displayh5p/tests/behat/h5p_filter.feature
View file
Expand Up @@ -181,3 +181,53 @@ Feature: Render H5P content using filters
And I switch to "h5p-iframe" class iframe
Then I should not see "missing-required-library"
And I should see "Lorum ipsum"

@javascript
Scenario: Render local H5P file with a disabled main library
Given I log in as "admin"
# Upload H5P file to private files.
And I follow "Private files"
And I upload "h5p/tests/fixtures/ipsums.h5p" file to "Files" filemanager
And I click on "Save changes" "button"
# Upload manually the H5P content-type library and disable it.
And I navigate to "H5P > Manage H5P content types" in site administration
And I upload "h5p/tests/fixtures/ipsums.h5p" file to "H5P content type" filemanager
And I click on "Upload H5P content types" "button" in the "#fitem_id_uploadlibraries" "css_element"
And I click on "Disable" "link" in the "Accordion" "table_row"
# Add H5P content to the page.
And I am on "Course 1" course homepage with editing mode on
And I follow "PageName1"
And I navigate to "Edit settings" in current page administration
When I click on "Insert H5P" "button" in the "#fitem_id_page" "css_element"
And I click on "Browse repositories..." "button" in the "Insert H5P" "dialogue"
And I click on "Private files" "link" in the ".fp-repo-area" "css_element"
And I click on "ipsums.h5p" "link"
And I click on "Select this file" "button"
And I click on "Insert H5P" "button" in the "Insert H5P" "dialogue"
And I click on "Save and display" "button"
And I switch to "h5p-iframe" class iframe
# Library is disabled, so an error should be displayed.
Then I should see "This file can't be displayed because its content-type is disabled."
And I should not see "Lorum ipsum"
And I switch to the main frame
And I navigate to "H5P > Manage H5P content types" in site administration
And I click on "Enable" "link" in the "Accordion" "table_row"
And I am on "Course 1" course homepage
# Content should be deployed now that main library is enabled.
And I follow "PageName1"
# Switch to iframe created by filter.
And I switch to "h5p-iframe" class iframe
# Switch to iframe created by embed.php page.
And I switch to "h5p-iframe" class iframe
And I should see "Lorum ipsum"
And I should not see "This file can't be displayed because its content-type is disabled."
And I switch to the main frame
And I navigate to "H5P > Manage H5P content types" in site administration
And I click on "Disable" "link" in the "Accordion" "table_row"
And I am on "Course 1" course homepage
# Library is disabled again, so an error should be displayed.
And I follow "PageName1"
And I switch to "h5p-iframe" class iframe
And I should see "This file can't be displayed because its content-type is disabled."
And I should not see "Lorum ipsum"
And I switch to the main frame
55 changes: 49 additions & 6 deletions h5p/classes/api.php
View file
Expand Up @@ -251,13 +251,21 @@ public static function create_content_from_pluginfile_url(string $url, \stdClass
$context = \context::instance_by_id($file->get_contextid());
if ($h5p) {
// The H5P content has been deployed previously.
$displayoptions = helper::get_display_options($core, $config);
// Check if the user can set the displayoptions.
if ($displayoptions != $h5p->displayoptions && has_capability('moodle/h5p:setdisplayoptions', $context)) {
// If the displayoptions has changed and the user has permission to modify it, update this information in the DB.
$core->h5pF->updateContentFields($h5p->id, ['displayoptions' => $displayoptions]);

// If the main library for this H5P content is disabled, the content won't be displayed.
$mainlibrary = (object) ['id' => $h5p->mainlibraryid];
if (!self::is_library_enabled($mainlibrary)) {
$core->h5pF->setErrorMessage(get_string('mainlibrarydisabled', 'core_h5p'));
return [$file, false];
} else {
$displayoptions = helper::get_display_options($core, $config);
// Check if the user can set the displayoptions.
if ($displayoptions != $h5p->displayoptions && has_capability('moodle/h5p:setdisplayoptions', $context)) {
// If displayoptions has changed and user has permission to modify it, update this information in DB.
$core->h5pF->updateContentFields($h5p->id, ['displayoptions' => $displayoptions]);
}
return [$file, $h5p->id];
}
return [$file, $h5p->id];
} else {
// The H5P content hasn't been deployed previously.

Expand Down Expand Up @@ -614,4 +622,39 @@ public static function set_library_enabled(int $libraryid, bool $isenabled): voi
}
}

/**
* Check whether a library is enabled or not. When machinename is passed, it will return false if any of the versions
* for this machinename is disabled.
* If the library doesn't exist, it will return true.
*
* @param \stdClass $librarydata Supported fields for library: 'id' and 'machichename'.
* @return bool
* @throws \moodle_exception
*/
public static function is_library_enabled(\stdClass $librarydata): bool {
global $DB;

$params = [];
if (property_exists($librarydata, 'machinename')) {
$params['machinename'] = $librarydata->machinename;
}
if (property_exists($librarydata, 'id')) {
$params['id'] = $librarydata->id;
}

if (empty($params)) {
throw new \moodle_exception("Missing 'machinename' or 'id' in librarydata parameter");
}

$libraries = $DB->get_records('h5p_libraries', $params);

// If any of the libraries with these values have been disabled, return false.
foreach ($libraries as $id => $library) {
if (!$library->enabled) {
return false;
}
}

return true;
}
}
7 changes: 7 additions & 0 deletions h5p/classes/helper.php
View file
Expand Up @@ -67,6 +67,13 @@ public static function save_h5p(factory $factory, \stored_file $file, \stdClass
// Check if the h5p file is valid before saving it.
$h5pvalidator = $factory->get_validator();
if ($h5pvalidator->isValidPackage($skipcontent, $onlyupdatelibs)) {
// If the main library of the package is disabled, the H5P content won't be saved.
$mainlibrary = (object) ['machinename' => $h5pvalidator->h5pC->mainJsonData['mainLibrary']];
if (!api::is_library_enabled($mainlibrary)) {
$core->h5pF->setErrorMessage(get_string('mainlibrarydisabled', 'core_h5p'));
return false;
}

$h5pstorage = $factory->get_storage();

$content = [
Expand Down
144 changes: 144 additions & 0 deletions h5p/tests/api_test.php
View file
Expand Up @@ -598,4 +598,148 @@ public function set_library_enabled_provider(): array {
],
];
}

/**
* Test the behaviour of is_library_enabled().
*
* @covers ::is_library_enabled
* @dataProvider is_library_enabled_provider
*
* @param string $libraryname Library name to check.
* @param bool $expected Expected result after calling the method.
* @param bool $exception Exception expected or not.
* @param bool $useid Whether to use id for calling is_library_enabled method.
* @param bool $uselibraryname Whether to use libraryname for calling is_library_enabled method.
*/
public function test_is_library_enabled(string $libraryname, bool $expected, bool $exception = false,
bool $useid = false, bool $uselibraryname = true): void {
global $DB;

$this->resetAfterTest();

// Create the following libraries:
// - H5P.Lib1: 1 version enabled, 1 version disabled.
// - H5P.Lib2: 2 versions enabled.
// - H5P.Lib3: 2 versions disabled.
// - H5P.Lib4: 1 version disabled.
// - H5P.Lib5: 1 version enabled.
$generator = $this->getDataGenerator()->get_plugin_generator('core_h5p');
$libraries = [
'H5P.Lib1.1' => $generator->create_library_record('H5P.Lib1', 'Lib1', 1, 1, 0, '', null, null, null, false),
'H5P.Lib1.2' => $generator->create_library_record('H5P.Lib1', 'Lib1', 1, 2),
'H5P.Lib2.1' => $generator->create_library_record('H5P.Lib2', 'Lib2', 2, 1),
'H5P.Lib2.2' => $generator->create_library_record('H5P.Lib2', 'Lib2', 2, 2),
'H5P.Lib3.1' => $generator->create_library_record('H5P.Lib3', 'Lib3', 3, 1, 0, '', null, null, null, false),
'H5P.Lib3.2' => $generator->create_library_record('H5P.Lib3', 'Lib3', 3, 2, 0, '', null, null, null, false),
'H5P.Lib4.1' => $generator->create_library_record('H5P.Lib4', 'Lib4', 4, 1, 0, '', null, null, null, false),
'H5P.Lib5.1' => $generator->create_library_record('H5P.Lib5', 'Lib5', 5, 1),
];

$countenabledlibraries = $DB->count_records('h5p_libraries', ['enabled' => 1]);
$this->assertEquals(4, $countenabledlibraries);

if ($useid) {
$librarydata = ['id' => $libraries[$libraryname]->id];
} else if ($uselibraryname) {
$librarydata = ['machinename' => $libraryname];
} else {
$librarydata = ['invalid' => true];
}

if ($exception) {
$this->expectException(\moodle_exception::class);
}

$result = api::is_library_enabled((object) $librarydata);
$this->assertEquals($expected, $result);
}

/**
* Data provider for test_is_library_enabled().
*
* @return array
*/
public function is_library_enabled_provider(): array {
return [
'Library with 2 versions, one of them disabled' => [
'libraryname' => 'H5P.Lib1',
'expected' => false,
],
'Library with 2 versions, all enabled' => [
'libraryname' => 'H5P.Lib2',
'expected' => true,
],
'Library with 2 versions, all disabled' => [
'libraryname' => 'H5P.Lib3',
'expected' => false,
],
'Library with only one version, disabled' => [
'libraryname' => 'H5P.Lib4',
'expected' => false,
],
'Library with only one version, enabled' => [
'libraryname' => 'H5P.Lib5',
'expected' => true,
],
'Library with 2 versions, one of them disabled (using id) - 1.1 (disabled)' => [
'libraryname' => 'H5P.Lib1.1',
'expected' => false,
'exception' => false,
'useid' => true,
],
'Library with 2 versions, one of them disabled (using id) - 1.2 (enabled)' => [
'libraryname' => 'H5P.Lib1.2',
'expected' => true,
'exception' => false,
'useid' => true,
],
'Library with 2 versions, all enabled (using id) - 2.1' => [
'libraryname' => 'H5P.Lib2.1',
'expected' => true,
'exception' => false,
'useid' => true,
],
'Library with 2 versions, all enabled (using id) - 2.2' => [
'libraryname' => 'H5P.Lib2.2',
'expected' => true,
'exception' => false,
'useid' => true,
],
'Library with 2 versions, all disabled (using id) - 3.1' => [
'libraryname' => 'H5P.Lib3.1',
'expected' => false,
'exception' => false,
'useid' => true,
],
'Library with 2 versions, all disabled (using id) - 3.2' => [
'libraryname' => 'H5P.Lib3.2',
'expected' => false,
'exception' => false,
'useid' => true,
],
'Library with only one version, disabled (using id)' => [
'libraryname' => 'H5P.Lib4.1',
'expected' => false,
'exception' => false,
'useid' => true,
],
'Library with only one version, enabled (using id)' => [
'libraryname' => 'H5P.Lib5.1',
'expected' => true,
'exception' => false,
'useid' => true,
],
'Unexisting library' => [
'libraryname' => 'H5P.Unexisting',
'expected' => true,
],
'Missing required parameters' => [
'libraryname' => 'H5P.Unexisting',
'expected' => false,
'exception' => true,
'useid' => false,
'uselibraryname' => false,
],
];
}
}
2 changes: 2 additions & 0 deletions lang/en/h5p.php
View file
Expand Up @@ -138,6 +138,8 @@
$string['licenseextras'] = 'Licence extras';
$string['licenseversion'] = 'Licence version';
$string['lockh5pdeploy'] = 'This H5P content cannot be accessed because it is being deployed. Please try again later.';
$string['mainlibrarydisabled'] = 'This file can\'t be displayed because its content-type is disabled. Please contact your administrator to ask for the content type to
be enabled.';
$string['missingcontentfolder'] = 'A valid content folder is missing';
$string['missingcoreversion'] = 'The system was unable to install the {$a->%component} component from the package, as it requires a newer version of the H5P plugin. This site is currently running version {$a->%current}, whereas the required version is {$a->%required} or higher. Please upgrade and then try again.';
$string['missingdependency'] = 'Missing dependency {$a->@dep} required by {$a->@lib}.';
Expand Down

0 comments on commit be71433

Please sign in to comment.