From d3d3b3bdc046856a33329e0c75ac55b214b16362 Mon Sep 17 00:00:00 2001 From: Sara Arjona Date: Wed, 24 Feb 2021 16:11:12 +0100 Subject: [PATCH] MDL-69331 core_h5p: Add enabled field to libraries The "enabled" field has been added to the H5P libraries to let admins decide whether a library should be used or not in the site. --- h5p/classes/api.php | 21 ++++++ h5p/classes/framework.php | 2 +- h5p/classes/output/libraries.php | 11 ++++ h5p/libraries.php | 10 +++ h5p/templates/h5plibraries.mustache | 23 +++++-- h5p/tests/api_test.php | 95 +++++++++++++++++++++++++++ h5p/tests/behat/h5p_libraries.feature | 13 ++++ h5p/tests/editor_framework_test.php | 13 ++-- h5p/tests/generator/lib.php | 15 +++-- h5p/tests/generator_test.php | 1 + lib/db/install.xml | 3 +- lib/db/upgrade.php | 15 +++++ theme/boost/scss/moodle/core.scss | 4 ++ theme/boost/style/moodle.css | 3 + theme/classic/style/moodle.css | 3 + 15 files changed, 216 insertions(+), 16 deletions(-) diff --git a/h5p/classes/api.php b/h5p/classes/api.php index d864e79f98b..850904c95d0 100644 --- a/h5p/classes/api.php +++ b/h5p/classes/api.php @@ -593,4 +593,25 @@ class api { return null; } + + /** + * Enable or disable a library. + * + * @param int $libraryid The id of the library to enable/disable. + * @param bool $isenabled True if the library should be enabled; false otherwise. + */ + public static function set_library_enabled(int $libraryid, bool $isenabled): void { + global $DB; + + $library = $DB->get_record('h5p_libraries', ['id' => $libraryid], '*', MUST_EXIST); + if ($library->runnable) { + // For now, only runnable libraries can be enabled/disabled. + $record = [ + 'id' => $libraryid, + 'enabled' => $isenabled, + ]; + $DB->update_record('h5p_libraries', $record); + } + } + } diff --git a/h5p/classes/framework.php b/h5p/classes/framework.php index d4b0d6ad1e1..e0d22dbb2e6 100644 --- a/h5p/classes/framework.php +++ b/h5p/classes/framework.php @@ -520,7 +520,7 @@ class framework implements \H5PFrameworkInterface { $results = $DB->get_records('h5p_libraries', [], 'title ASC, majorversion ASC, minorversion ASC', 'id, machinename AS machine_name, majorversion AS major_version, minorversion AS minor_version, - patchversion AS patch_version, runnable, title'); + patchversion AS patch_version, runnable, title, enabled'); $libraries = array(); foreach ($results as $library) { diff --git a/h5p/classes/output/libraries.php b/h5p/classes/output/libraries.php index 07a8fe97c8c..a3aef637c50 100644 --- a/h5p/classes/output/libraries.php +++ b/h5p/classes/output/libraries.php @@ -89,6 +89,17 @@ class libraries implements renderable, templatable { get_string('deletelibraryversion', 'core_h5p') )); $version->actionmenu = $actionmenu->export_for_template($output); + if ($version->enabled) { + $version->toggleenabledurl = new moodle_url('/h5p/libraries.php', [ + 'id' => $version->id, + 'action' => 'disable', + ]); + } else { + $version->toggleenabledurl = new moodle_url('/h5p/libraries.php', [ + 'id' => $version->id, + 'action' => 'enable', + ]); + } $installed[] = $version; } } diff --git a/h5p/libraries.php b/h5p/libraries.php index 8d784f6643f..bb5f3839515 100644 --- a/h5p/libraries.php +++ b/h5p/libraries.php @@ -28,6 +28,7 @@ require_login(null, false); $deletelibrary = optional_param('deletelibrary', null, PARAM_INT); $confirm = optional_param('confirm', false, PARAM_BOOL); +$action = optional_param('action', null, PARAM_ALPHANUMEXT); $context = context_system::instance(); require_capability('moodle/h5p:updatelibraries', $context); @@ -64,6 +65,15 @@ if ($deletelibrary) { die(); } +if (!is_null($action)) { + if ($action == 'enable' || $action == 'disable') { + // If action is enable or disable, library id is required too. + $libraryid = required_param('id', PARAM_INT); + + \core_h5p\api::set_library_enabled($libraryid, ($action == 'enable')); + } +} + echo $OUTPUT->header(); echo $OUTPUT->heading($pagetitle); echo $OUTPUT->box(get_string('librariesmanagerdescription', 'core_h5p')); diff --git a/h5p/templates/h5plibraries.mustache b/h5p/templates/h5plibraries.mustache index cb6e7b06051..f5dba1059e4 100644 --- a/h5p/templates/h5plibraries.mustache +++ b/h5p/templates/h5plibraries.mustache @@ -26,14 +26,18 @@ "minor_version:": 0, "patch_version:": 0, "runnable": 1, - "icon": "icon.svg" + "icon": "icon.svg", + "enabled": true, + "toggleenabledurl": "http://myserver.cat/h5p/libraries.php?id=26&action=disable" }, { "title": "Collage", "major_version": 0, "minor_version:": 3, "patch_version:": 1, - "runnable": 1 + "runnable": 1, + "enabled": true, + "toggleenabledurl": "http://myserver.cat/h5p/libraries.php?id=37&action=disable" }, { "title": "FontAwesome", @@ -41,7 +45,9 @@ "minor_version:": 5, "patch_version:": 0, "runnable": 1, - "icon": "icon.svg" + "icon": "icon.svg", + "enabled": false, + "toggleenabledurl": "http://myserver.cat/h5p/libraries.php?id=54&action=enable" } ] } @@ -67,6 +73,7 @@ + @@ -76,6 +83,14 @@ {{#contenttypes}} {{#runnable}} +
{{#str}}enable, core{{/str}} {{#str}}description, core{{/str}} {{#str}}version, core{{/str}}
+ {{#enabled}} + {{#pix}}t/hide, core,{{#str}}disable{{/str}}{{/pix}} + {{/enabled}} + {{^enabled}} + {{#pix}}t/show, core,{{#str}}enable{{/str}}{{/pix}} + {{/enabled}} + {{#icon}} - \ No newline at end of file + diff --git a/h5p/tests/api_test.php b/h5p/tests/api_test.php index e1ec76e8350..24ea653b1e3 100644 --- a/h5p/tests/api_test.php +++ b/h5p/tests/api_test.php @@ -35,6 +35,7 @@ defined('MOODLE_INTERNAL') || die(); * @package core_h5p * @copyright 2020 Sara Arjona * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @coversDefaultClass \core_h5p\api */ class api_testcase extends \advanced_testcase { @@ -503,4 +504,98 @@ class api_testcase extends \advanced_testcase { \core_h5p\file_storage::EXPORT_FILEAREA); $this->assertNull($exportfile); } + + /** + * Test the behaviour of set_library_enabled(). + * + * @covers ::set_library_enabled + * @dataProvider set_library_enabled_provider + * + * @param string $libraryname Library name to enable/disable. + * @param string $action Action to be done with the library. Supported values: enable, disable. + * @param int $expected Expected value for the enabled library field. -1 will be passed if the library doesn't exist. + */ + public function test_set_library_enabled(string $libraryname, string $action, int $expected): void { + global $DB; + + $this->resetAfterTest(); + + // Create libraries. + $generator = $this->getDataGenerator()->get_plugin_generator('core_h5p'); + $generator->generate_h5p_data(); + + // Check by default the library is enabled. + $library = $DB->get_record('h5p_libraries', ['machinename' => $libraryname]); + if ($expected >= 0) { + $this->assertEquals(1, $library->enabled); + $libraryid = (int) $library->id; + } else { + // Unexisting library. Set libraryid to some unexisting id. + $libraryid = -1; + $this->expectException('dml_missing_record_exception'); + } + + \core_h5p\api::set_library_enabled($libraryid, ($action == 'enable')); + + // Check the value of the "enabled" field after calling enable/disable method. + $libraries = $DB->get_records('h5p_libraries'); + foreach ($libraries as $libraryid => $library) { + if ($library->machinename == $libraryname) { + $this->assertEquals($expected, $library->enabled); + } else { + // Check that only $libraryname has been enabled/disabled. + $this->assertEquals(1, $library->enabled); + } + } + } + + /** + * Data provider for test_set_library_enabled(). + * + * @return array + */ + public function set_library_enabled_provider(): array { + return [ + 'Disable existing library' => [ + 'libraryname' => 'MainLibrary', + 'action' => 'disable', + 'expected' => 0, + ], + 'Enable existing library' => [ + 'libraryname' => 'MainLibrary', + 'action' => 'enable', + 'expected' => 1, + ], + 'Disable existing library (not main)' => [ + 'libraryname' => 'Library1', + 'action' => 'disable', + 'expected' => 0, + ], + 'Enable existing library (not main)' => [ + 'libraryname' => 'Library1', + 'action' => 'enable', + 'expected' => 1, + ], + 'Disable existing library (not runnable)' => [ + 'libraryname' => 'Library3', + 'action' => 'disable', + 'expected' => 1, // Not runnable libraries can't be disabled. + ], + 'Enable existing library (not runnable)' => [ + 'libraryname' => 'Library3', + 'action' => 'enable', + 'expected' => 1, + ], + 'Enable unexisting library' => [ + 'libraryname' => 'Unexisting library', + 'action' => 'enable', + 'expected' => -1, + ], + 'Disable unexisting library' => [ + 'libraryname' => 'Unexisting library', + 'action' => 'disable', + 'expected' => -1, + ], + ]; + } } diff --git a/h5p/tests/behat/h5p_libraries.feature b/h5p/tests/behat/h5p_libraries.feature index 2d990b3e6d4..c5b255ade7e 100644 --- a/h5p/tests/behat/h5p_libraries.feature +++ b/h5p/tests/behat/h5p_libraries.feature @@ -61,3 +61,16 @@ Feature: Upload and list H5P libraries and content types installed And I should not see "H5P.FontIcons" And I should not see "Joubel UI" And I should see "Transition" + + @javascript + Scenario: Enable/disable H5P library + Given I log in as "admin" + And I navigate to "H5P > Manage H5P content types" in site administration + And I upload "h5p/tests/fixtures/filltheblanks.h5p" file to "H5P content type" filemanager + And I click on "Upload H5P content types" "button" in the "#fitem_id_uploadlibraries" "css_element" + When I click on "Disable" "link" in the "Fill in the Blanks" "table_row" + Then "Enable" "icon" should exist in the "Fill in the Blanks" "table_row" + And "Disable" "icon" should not exist in the "Fill in the Blanks" "table_row" + And I click on "Enable" "link" in the "Fill in the Blanks" "table_row" + And "Disable" "icon" should exist in the "Fill in the Blanks" "table_row" + And "Enable" "icon" should not exist in the "Fill in the Blanks" "table_row" diff --git a/h5p/tests/editor_framework_test.php b/h5p/tests/editor_framework_test.php index 8e3e220a8b3..1c93f87beb5 100644 --- a/h5p/tests/editor_framework_test.php +++ b/h5p/tests/editor_framework_test.php @@ -37,19 +37,24 @@ use core_h5p\local\library\autoloader; * * @runTestsInSeparateProcesses */ -class editor_framework_testcase extends \advanced_testcase { +class editor_framework_test extends \advanced_testcase { /** @var editor_framework H5P editor_framework instance */ protected $editorframework; + /** + * Setup to ensure that fixtures are loaded. + */ + public static function setupBeforeClass(): void { + autoloader::register(); + } + /** * Set up function for tests. */ protected function setUp(): void { parent::setUp(); - autoloader::register(); - $this->editorframework = new editor_framework(); } @@ -362,7 +367,7 @@ class editor_framework_testcase extends \advanced_testcase { $expectedlibraries = []; foreach ($data as $key => $value) { - if (isset($value->data)) { + if (isset($value->data) && $value->data->runnable) { $value->data->name = $value->data->machinename; $value->data->majorVersion = $value->data->majorversion; $value->data->minorVersion = $value->data->minorversion; diff --git a/h5p/tests/generator/lib.php b/h5p/tests/generator/lib.php index 78c234be536..15a13fe8a40 100644 --- a/h5p/tests/generator/lib.php +++ b/h5p/tests/generator/lib.php @@ -177,7 +177,7 @@ class core_h5p_generator extends \component_generator_base { 'http://tutorial.org', 'http://example.org'); $lib1 = $libraries[] = $this->create_library_record('Library1', 'Lib1', 2, 0, 1, '', null, null, 'http://example.org'); $lib2 = $libraries[] = $this->create_library_record('Library2', 'Lib2', 2, 1, 1, '', null, 'http://tutorial.org'); - $lib3 = $libraries[] = $this->create_library_record('Library3', 'Lib3', 3, 2); + $lib3 = $libraries[] = $this->create_library_record('Library3', 'Lib3', 3, 2, 1, '', null, null, null, true, 0); $lib4 = $libraries[] = $this->create_library_record('Library4', 'Lib4', 1, 1); $lib5 = $libraries[] = $this->create_library_record('Library5', 'Lib5', 1, 3); @@ -251,20 +251,22 @@ class core_h5p_generator extends \component_generator_base { * @param string $addto The plugin configuration data * @param string $tutorial The tutorial URL * @param string $examlpe The example URL + * @param bool $enabled Whether the library is enabled or not + * @param int $runnable Whether the library is runnable (1) or not (0) * @return stdClass An object representing the added library record */ public function create_library_record(string $machinename, string $title, int $majorversion = 1, int $minorversion = 0, int $patchversion = 1, string $semantics = '', string $addto = null, - string $tutorial = null, string $example = null): stdClass { + string $tutorial = null, string $example = null, bool $enabled = true, int $runnable = 1): stdClass { global $DB; - $content = array( + $content = [ 'machinename' => $machinename, 'title' => $title, 'majorversion' => $majorversion, 'minorversion' => $minorversion, 'patchversion' => $patchversion, - 'runnable' => 1, + 'runnable' => $runnable, 'fullscreen' => 1, 'preloadedjs' => 'js/example.js', 'preloadedcss' => 'css/example.css', @@ -272,8 +274,9 @@ class core_h5p_generator extends \component_generator_base { 'semantics' => $semantics, 'addto' => $addto, 'tutorial' => $tutorial, - 'example' => $example - ); + 'example' => $example, + 'enabled' => $enabled, + ]; $libraryid = $DB->insert_record('h5p_libraries', $content); diff --git a/h5p/tests/generator_test.php b/h5p/tests/generator_test.php index b43df426dbb..8111ad07502 100644 --- a/h5p/tests/generator_test.php +++ b/h5p/tests/generator_test.php @@ -251,6 +251,7 @@ class generator_testcase extends \advanced_testcase { 'coremajor' => null, 'coreminor' => null, 'metadatasettings' => null, + 'enabled' => 1, ]; $this->assertEquals($expected, $data); diff --git a/lib/db/install.xml b/lib/db/install.xml index 53554a6cfe7..fdeba8596f2 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -4203,6 +4203,7 @@ + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 8704835d4b0..cb43328e0e0 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2545,5 +2545,20 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2021052500.75); } + if ($oldversion < 2021052500.78) { + + // Define field enabled to be added to h5p_libraries. + $table = new xmldb_table('h5p_libraries'); + $field = new xmldb_field('enabled', XMLDB_TYPE_INTEGER, '1', null, null, null, '1', 'example'); + + // Conditionally launch add field enabled. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Main savepoint reached. + upgrade_main_savepoint(true, 2021052500.78); + } + return true; } diff --git a/theme/boost/scss/moodle/core.scss b/theme/boost/scss/moodle/core.scss index 8bba6e735f0..e5b97391416 100644 --- a/theme/boost/scss/moodle/core.scss +++ b/theme/boost/scss/moodle/core.scss @@ -2466,6 +2466,10 @@ body.h5p-embed { } } +#h5pcontenttypes td { + vertical-align: middle; +} + .text-decoration-none { text-decoration: none !important; /* stylelint-disable-line declaration-no-important */ } diff --git a/theme/boost/style/moodle.css b/theme/boost/style/moodle.css index 128b5fce001..c5e5a302c6c 100644 --- a/theme/boost/style/moodle.css +++ b/theme/boost/style/moodle.css @@ -11660,6 +11660,9 @@ body.h5p-embed #maincontent { body.h5p-embed .h5pmessages { min-height: 230px; } +#h5pcontenttypes td { + vertical-align: middle; } + .text-decoration-none { text-decoration: none !important; /* stylelint-disable-line declaration-no-important */ } diff --git a/theme/classic/style/moodle.css b/theme/classic/style/moodle.css index a5186ba0e8d..047068c9c2f 100644 --- a/theme/classic/style/moodle.css +++ b/theme/classic/style/moodle.css @@ -11878,6 +11878,9 @@ body.h5p-embed #maincontent { body.h5p-embed .h5pmessages { min-height: 230px; } +#h5pcontenttypes td { + vertical-align: middle; } + .text-decoration-none { text-decoration: none !important; /* stylelint-disable-line declaration-no-important */ }