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

@@ -48,31 +48,20 @@
 // 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();

lib/classes/plugininfo/mod.php

@@ -51,6 +51,14 @@ public static function enable_plugin(string $pluginname, int $enabled): bool {
 
         // 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;

mod/bigbluebuttonbn/classes/settings.php

@@ -136,6 +136,7 @@ protected function add_conditional_element(string $name, admin_setting $item, ad
      * @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 @@ protected function add_general_settings(): admin_settingpage {
             // 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'),

mod/bigbluebuttonbn/lang/en/bigbluebuttonbn.php

@@ -49,6 +49,7 @@
 $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 <a href="{$a}" target="_blank">Blindside Networks data processing agreement</a>. 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 <a href="{$a}">Blindside Networks data processing agreement</a>.';
 $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';

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.

mod/bigbluebuttonbn/tests/lib_test.php

@@ -698,4 +698,72 @@ public function test_mod_bigbluebuttonbn_core_calendar_is_event_visible() {
         $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,
+            ],
+        ];
+    }
 }