From e6fe30187f8aed0246e0a49587bd7549af010d7d Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Mon, 13 Feb 2023 22:11:43 +0800 Subject: [PATCH] MDL-77248 core: Move pre_enable_plugin_actions callback to enable_plugin Note: The original callback was incorrectly using the $PAGE output, which cannot be relied upon in this callback. The best we can do here is to add a notification to explain the situation. --- admin/modules.php | 25 ++----- lib/classes/plugininfo/mod.php | 8 +++ mod/bigbluebuttonbn/classes/settings.php | 15 +++- .../lang/en/bigbluebuttonbn.php | 1 + mod/bigbluebuttonbn/lib.php | 6 +- mod/bigbluebuttonbn/tests/lib_test.php | 68 +++++++++++++++++++ 6 files changed, 102 insertions(+), 21 deletions(-) diff --git a/admin/modules.php b/admin/modules.php index 8fd9be196e8..a8a04b365f2 100644 --- a/admin/modules.php +++ b/admin/modules.php @@ -48,31 +48,20 @@ $strshowmodulecourse = get_string('showmodulecourse'); // If data submitted, then process and store. if (!empty($hide) && confirm_sesskey()) { $class = \core_plugin_manager::resolve_plugininfo_class('mod'); - $class::enable_plugin($hide, false); - - // Settings not required - only pages. - admin_get_root(true, false); + if ($class::enable_plugin($hide, false)) { + // Settings not required - only pages. + admin_get_root(true, false); + } redirect(new moodle_url('/admin/modules.php')); } if (!empty($show) && confirm_sesskey()) { - $canenablemodule = true; - $modulename = $show; - - // Invoking a callback function that enables plugins to force additional actions (e.g. displaying notifications, - // modals, etc.) and also specify through its returned value (bool) whether the process of enabling the plugin - // should continue after these actions or not. - if (component_callback_exists("mod_{$modulename}", 'pre_enable_plugin_actions')) { - $canenablemodule = component_callback("mod_{$modulename}", 'pre_enable_plugin_actions'); - } - - if ($canenablemodule) { - $class = \core_plugin_manager::resolve_plugininfo_class('mod'); - $class::enable_plugin($show, true); + $class = \core_plugin_manager::resolve_plugininfo_class('mod'); + if ($class::enable_plugin($show, true)) { // Settings not required - only pages. admin_get_root(true, false); - redirect(new moodle_url('/admin/modules.php')); } + redirect(new moodle_url('/admin/modules.php')); } echo $OUTPUT->header(); diff --git a/lib/classes/plugininfo/mod.php b/lib/classes/plugininfo/mod.php index ce8ebccf1dd..8e67f415b72 100644 --- a/lib/classes/plugininfo/mod.php +++ b/lib/classes/plugininfo/mod.php @@ -51,6 +51,14 @@ class mod extends base { // Only set visibility if it's different from the current value. if ($module->visible != $enabled) { + if ($enabled && component_callback_exists("mod_{$pluginname}", 'pre_enable_plugin_actions')) { + // Invoking a callback function that enables plugins to force additional actions (e.g. displaying notifications, + // modals, etc.) and also specify through its returned value (bool) whether the process of enabling the plugin + // should continue after these actions or not. + if (!component_callback("mod_{$pluginname}", 'pre_enable_plugin_actions')) { + return false; + } + } // Set module visibility. $DB->set_field('modules', 'visible', $enabled, ['id' => $module->id]); $haschanged = true; diff --git a/mod/bigbluebuttonbn/classes/settings.php b/mod/bigbluebuttonbn/classes/settings.php index 588dd5b6003..8b71a87a0c7 100644 --- a/mod/bigbluebuttonbn/classes/settings.php +++ b/mod/bigbluebuttonbn/classes/settings.php @@ -136,6 +136,7 @@ class settings { * @throws \coding_exception */ protected function add_general_settings(): admin_settingpage { + global $CFG; $settingsgeneral = new admin_settingpage( $this->section, get_string('config_general', 'bigbluebuttonbn'), @@ -146,9 +147,19 @@ class settings { // Configuration for BigBlueButton. $item = new admin_setting_heading('bigbluebuttonbn_config_general', '', - get_string('config_general_description', 'bigbluebuttonbn')); - + get_string('config_general_description', 'bigbluebuttonbn') + ); $settingsgeneral->add($item); + + if (empty($CFG->bigbluebuttonbn_default_dpa_accepted)) { + $settingsgeneral->add(new admin_setting_configcheckbox( + 'bigbluebuttonbn_default_dpa_accepted', + get_string('acceptdpa', 'mod_bigbluebuttonbn'), + get_string('enablingbigbluebuttondpainfo', 'mod_bigbluebuttonbn', config::DEFAULT_DPA_URL), + 0 + )); + } + $item = new admin_setting_configtext( 'bigbluebuttonbn_server_url', get_string('config_server_url', 'bigbluebuttonbn'), diff --git a/mod/bigbluebuttonbn/lang/en/bigbluebuttonbn.php b/mod/bigbluebuttonbn/lang/en/bigbluebuttonbn.php index 3ec2201f921..00b13420ec2 100644 --- a/mod/bigbluebuttonbn/lang/en/bigbluebuttonbn.php +++ b/mod/bigbluebuttonbn/lang/en/bigbluebuttonbn.php @@ -49,6 +49,7 @@ $string['bigbluebuttondisablednotification'] = 'The BigBlueButton activity modul $string['cannotperformaction'] = 'Cannot perform action {$a} on this recording'; $string['enablingbigbluebutton'] = 'Enabling BigBlueButton activity'; $string['enablingbigbluebuttondpainfo'] = 'In order to meet your data protection obligations, prior to enabling this plugin, you may need to ensure that you have read and accepted the Blindside Networks data processing agreement. Please consult with your own privacy professionals for advice.'; +$string['dpainfonotsigned'] = 'Before enabling this plugin, you must confirm that you have read and accepted the Blindside Networks data processing agreement.'; $string['indicator:cognitivedepth'] = 'BigBlueButton cognitive'; $string['indicator:cognitivedepth_help'] = 'This indicator is based on the cognitive depth reached by the student in a BigBlueButton activity.'; $string['indicator:socialbreadth'] = 'BigBlueButton social'; diff --git a/mod/bigbluebuttonbn/lib.php b/mod/bigbluebuttonbn/lib.php index f5800c3daae..5f7187a2177 100644 --- a/mod/bigbluebuttonbn/lib.php +++ b/mod/bigbluebuttonbn/lib.php @@ -730,7 +730,11 @@ function bigbluebuttonbn_pre_enable_plugin_actions(): bool { // agreement, do not enable the plugin. Instead, display a dynamic form where the administrator can confirm that he // accepts the DPA prior to enabling the plugin. if (config::get('server_url') === config::DEFAULT_SERVER_URL && !config::get('default_dpa_accepted')) { - $PAGE->requires->js_call_amd('mod_bigbluebuttonbn/accept_dpa', 'init', []); + $url = new moodle_url('/admin/category.php', ['category' => 'modbigbluebuttonbnfolder']); + \core\notification::add( + get_string('dpainfonotsigned', 'mod_bigbluebuttonbn', $url->out(false)), + \core\notification::ERROR + ); return false; } // Otherwise, continue and enable the plugin. diff --git a/mod/bigbluebuttonbn/tests/lib_test.php b/mod/bigbluebuttonbn/tests/lib_test.php index f4d60610b90..b54be9fee76 100644 --- a/mod/bigbluebuttonbn/tests/lib_test.php +++ b/mod/bigbluebuttonbn/tests/lib_test.php @@ -698,4 +698,72 @@ class lib_test extends \advanced_testcase { $event->instance = 0; $this->assertFalse(mod_bigbluebuttonbn_core_calendar_is_event_visible($event)); } + + /** + * Check the bigbluebuttonbn_pre_enable_plugin_actions function. + * + * @covers ::bigbluebuttonbn_pre_enable_plugin_actions + * @dataProvider bigbluebuttonbn_pre_enable_plugin_actions_provider + * @param bool $initialstate + * @param bool $expected + * @param int $notificationcount + */ + public function test_bigbluebuttonbn_pre_enable_plugin_actions( + ?bool $initialstate, + bool $expected, + int $notificationcount + ): void { + $this->resetAfterTest(true); + + set_config('bigbluebuttonbn_default_dpa_accepted', $initialstate); + + $this->assertEquals($expected, bigbluebuttonbn_pre_enable_plugin_actions()); + $this->assertCount($notificationcount, \core\notification::fetch()); + } + + /** + * Check the bigbluebuttonbn_pre_enable_plugin_actions function. + * + * @covers ::bigbluebuttonbn_pre_enable_plugin_actions + * @dataProvider bigbluebuttonbn_pre_enable_plugin_actions_provider + * @param bool $initialstate + * @param bool $expected + * @param int $notificationcount + */ + public function test_enable_plugin( + ?bool $initialstate, + bool $expected, + int $notificationcount + ): void { + $this->resetAfterTest(true); + + set_config('bigbluebuttonbn_default_dpa_accepted', $initialstate); + $this->assertEquals($expected, \core\plugininfo\mod::enable_plugin('bigbluebuttonbn', 1)); + $this->assertCount($notificationcount, \core\notification::fetch()); + } + + /** + * Data provider for bigbluebuttonbn_pre_enable_plugin_actions tests. + * + * @return array + */ + public function bigbluebuttonbn_pre_enable_plugin_actions_provider(): array { + return [ + 'Initially unset' => [ + null, + false, + 1, + ], + 'Set to false' => [ + false, + false, + 1, + ], + 'Initially set' => [ + true, + true, + 0, + ], + ]; + } }