From 162188f114afa9c85f5abd93ca2436b05c962e1e Mon Sep 17 00:00:00 2001 From: Andreas Wagner Date: Thu, 14 Nov 2024 14:39:52 +0100 Subject: [PATCH] MBS-9408: prevent manual block creation, add block in subcontext (#7) * MBS-9408: prevent manual block creation, add block in subcontext * MBS-9408: add comment and minor fixes. * MBS-9408: add override annotations --------- Co-authored-by: Andreas --- block_ai_chat.php | 52 ++++-- classes/local/helper.php | 19 +- classes/local/hook_callbacks.php | 15 +- tests/ai_chat_test.php | 300 +++++++++++++++++++++++++++++++ version.php | 2 +- 5 files changed, 353 insertions(+), 35 deletions(-) create mode 100644 tests/ai_chat_test.php diff --git a/block_ai_chat.php b/block_ai_chat.php index c2a8096..a6b5f1a 100644 --- a/block_ai_chat.php +++ b/block_ai_chat.php @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -use block_ai_chat\local\helper; +use local_ai_manager\local\userinfo; /** * Block class for block_ai_chat @@ -41,6 +41,7 @@ public function init(): void { * * @return bool */ + #[\Override] public function has_config(): bool { return true; } @@ -52,6 +53,7 @@ public function has_config(): bool { * @throws coding_exception * @throws moodle_exception */ + #[\Override] public function get_content(): stdClass { global $USER; @@ -64,7 +66,8 @@ public function get_content(): stdClass { $this->content->footer = ''; $context = \context_block::instance($this->instance->id); - if (!has_capability('block/ai_chat:view', $context) || !has_capability('local/ai_manager:use', $context)) { + if (!has_capability('block/ai_chat:view', $context) || + !has_capability('local/ai_manager:use', $context)) { return $this->content; } $tenant = \core\di::get(\local_ai_manager\local\tenant::class); @@ -75,16 +78,12 @@ public function get_content(): stdClass { return $this->content; } if ($tenant->is_tenant_allowed() && !$configmanager->is_tenant_enabled()) { - if ($aiconfig['role'] === - \local_ai_manager\local\userinfo::get_role_as_string(\local_ai_manager\local\userinfo::ROLE_BASIC - )) { + if ($aiconfig['role'] === userinfo::get_role_as_string(userinfo::ROLE_BASIC)) { return $this->content; } } if (!$chatconfig['isconfigured']) { - if ($aiconfig['role'] === - \local_ai_manager\local\userinfo::get_role_as_string(\local_ai_manager\local\userinfo::ROLE_BASIC - )) { + if ($aiconfig['role'] === userinfo::get_role_as_string(userinfo::ROLE_BASIC)) { return $this->content; } } @@ -103,33 +102,48 @@ public function get_content(): stdClass { * * @return bool */ + #[\Override] public function instance_allow_multiple(): bool { return false; } /** - * Returns on which page formats this block can be used. + * Returns on which page formats this block can be added. + * + * We do not want any user to create the block manually. + * But me must add at least one applicable format here otherwise it will lead to an installation error, + * because the block::_self_test fails. + * + * There are only two ways to create block instances: + * - Check "add ai block" in the settings form of a course + * - Admin has set up an automatic create of a block instance using the plugin settings. * * @return array */ + #[\Override] public function applicable_formats(): array { return ['course-view' => true]; } + /** + * We don't want any user to manually create an instance of this block. + * + * @param $page + * @return false + */ #[\Override] public function user_can_addto($page) { - $tenant = \core\di::get(\local_ai_manager\local\tenant::class); - // Add exception for site admin, otherwise we cannot add the block to the dashboard in the website administration. - if (!$tenant->is_tenant_allowed() && !is_siteadmin()) { - return false; - } - if (helper::show_global_block($page)) { - // If a global block is being shown on a page, we do not allow the user to add an own block. - return false; - } - return parent::user_can_addto($page); + return false; } + /** + * Do any additional initialization you may need at the time a new block instance is created + * + * @return boolean + * / + * @return true + * @throws dml_exception + */ #[\Override] public function instance_create() { global $DB; diff --git a/classes/local/helper.php b/classes/local/helper.php index ea7f17d..d2116be 100644 --- a/classes/local/helper.php +++ b/classes/local/helper.php @@ -26,22 +26,21 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class helper { - /** - * Check if a block is present. - * + * Check, if a block is existing in course context. * @param int $courseid - * @return mixed + * @return object|bool + * @throws \dml_exception */ - public static function check_block_present($courseid) { + public static function has_block_in_course_context(int $courseid): object|bool { global $DB; // Check if tenant is enabled for the school. - $sql = "SELECT bi.id - FROM {block_instances} bi - JOIN {context} ctx ON bi.parentcontextid = ctx.id - WHERE bi.blockname = :blockname AND ctx.contextlevel = :contextlevel - AND ctx.instanceid = :courseid"; + $sql = "SELECT bi.* + FROM {block_instances} bi + JOIN {context} ctx ON bi.parentcontextid = ctx.id + WHERE bi.blockname = :blockname AND ctx.contextlevel = :contextlevel + AND ctx.instanceid = :courseid"; $params = [ 'blockname' => 'ai_chat', diff --git a/classes/local/hook_callbacks.php b/classes/local/hook_callbacks.php index fd1e0dd..6c82ee1 100644 --- a/classes/local/hook_callbacks.php +++ b/classes/local/hook_callbacks.php @@ -45,7 +45,7 @@ public static function handle_after_form_definition(\core_course\hook\after_form } /** - * Check for addaichat form setting and add/remove ai-chat blockk. + * Check for addaichat form setting and add/remove ai-chat block. * * @param after_form_submission $hook */ @@ -56,7 +56,7 @@ public static function handle_after_form_submission(\core_course\hook\after_form // Check if block_ai_chat instance is present. $courseid = $data->id; - $blockinstance = \block_ai_chat\local\helper::check_block_present($courseid); + $blockinstance = helper::has_block_in_course_context($courseid); if (!empty($data->addaichat) && $data->addaichat == '1') { if (!$blockinstance) { @@ -64,7 +64,8 @@ public static function handle_after_form_submission(\core_course\hook\after_form $newinstance = new \stdClass; $newinstance->blockname = 'ai_chat'; $newinstance->parentcontextid = \context_course::instance($courseid)->id; - $newinstance->showinsubcontexts = 0; + // We want to make the block usable for single activity courses as well, so display in subcontexts. + $newinstance->showinsubcontexts = 1; $newinstance->pagetypepattern = '*'; $newinstance->subpagepattern = null; $newinstance->defaultregion = 'side-pre'; @@ -75,6 +76,8 @@ public static function handle_after_form_submission(\core_course\hook\after_form $newinstance->id = $DB->insert_record('block_instances', $newinstance); } } else { + // If tenant is not allowed, $data->addaichat will be empty, + // so an existing instance will be deleted by following lines. if ($blockinstance) { // Remove block instance. blocks_delete_instance($blockinstance); @@ -85,7 +88,9 @@ public static function handle_after_form_submission(\core_course\hook\after_form /** * Check if block instance is present and set addaichat form setting. * - * @param after_form_submission $hook + * @param \core_course\hook\after_form_definition_after_data $hook + * @return void + * @throws \dml_exception */ public static function handle_after_form_definition_after_data(\core_course\hook\after_form_definition_after_data $hook): void { // Get form data. @@ -94,7 +99,7 @@ public static function handle_after_form_definition_after_data(\core_course\hook if (!empty($formwrapper->get_course()->id)) { $courseid = $formwrapper->get_course()->id; - $blockinstance = \block_ai_chat\local\helper::check_block_present($courseid); + $blockinstance = helper::has_block_in_course_context($courseid); if ($blockinstance) { // Block present, so set checkbox accordingly. $mform->setDefault('addaichat', "checked"); diff --git a/tests/ai_chat_test.php b/tests/ai_chat_test.php new file mode 100644 index 0000000..f33238d --- /dev/null +++ b/tests/ai_chat_test.php @@ -0,0 +1,300 @@ +. + +/** + * Test class for the block_ai_chat. + * + * @package block_ai_chat + * @copyright 2024 ISB Bayern + * @author Andreas Wagner + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @group local_mbs + * @group mebis + */ + +namespace block_ai_chat; + +defined('MOODLE_INTERNAL') || die(); + +use core\di; +use core\hook\output\before_footer_html_generation; +use moodle_page; + +global $CFG; +require_once($CFG->dirroot . '/lib/blocklib.php'); +require_once($CFG->dirroot . '/course/edit_form.php'); + +/** + * Test class for the block_ai_chat. + * + * @package block_ai_chat + * @copyright 2024 ISB Bayern + * @author Andreas Wagner + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +final class ai_chat_test extends \advanced_testcase { + + /** + * Create a page with regions. + * + * @param array $regions + * @param object $context + * @param object $course + * @param string $pagetype + * @return moodle_page + * @throws \coding_exception + */ + protected function get_moodle_page( + array $regions, + object $context, + object $course, + string $pagetype + ): \moodle_page { + + $page = new \moodle_page(); + $page->set_context($context); + $page->set_course($course); + $page->set_pagetype($pagetype); + $page->set_url(new \moodle_url('/')); + + $page->blocks->add_regions($regions, false); + $page->blocks->set_default_region($regions[0]); + + return $page; + } + + /** + * Tests the method get_addable_blocks + * + * @covers \block_ai_chat::applicable_formats + */ + public function test_get_addable_blocks(): void { + + $this->setAdminUser(); + $this->resetAfterTest(); + + // Create course containing a module. + $course = $this->getDataGenerator()->create_course(); + $coursecontext = \context_course::instance($course->id); + + $options = ['course' => $course->id]; + $forum = $this->getDataGenerator()->create_module('forum', $options); + $contextmodule = \context_module::instance($forum->cmid); + + // Get course page and module page. + $coursepage = $this->get_moodle_page( + ['side-pre'], $coursecontext, $course, 'course-view' + ); + + $modulepage = $this->get_moodle_page( + ['side-pre'], $contextmodule, $course, 'mod-forum-view' + ); + + // Block cannot be added manually on course pages. + $coursepage->blocks->load_blocks(false); + $coursepage->blocks->create_all_block_instances(); + $this->assertFalse(in_array('ai_chat', array_keys($coursepage->blocks->get_addable_blocks()))); + + $modulepage->blocks->load_blocks(false); + $modulepage->blocks->create_all_block_instances(); + $this->assertFalse(in_array('ai_chat', array_keys($modulepage->blocks->get_addable_blocks()))); + } + + /** + * Test hook before_footer. + * + * @return void + * @throws \coding_exception + * @throws \dml_exception + * + * @covers \hook_callbacks::handle_before_footer_html_generation + */ + public function test_before_footer_html_generation_hook(): void { + global $PAGE, $CFG, $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + // Assert there is no general block instance existing. + $aiblockinstances = $DB->get_records('block_instances', ['blockname' => 'ai_chat']); + $this->assertEmpty($aiblockinstances); + + // Replace the version of the manager in the DI container with a phpunit one. + \core\di::set( + \core\hook\manager::class, + \core\hook\manager::phpunit_get_instance([ + 'block_ai_chat' => $CFG->dirroot . '/blocks/ai_chat/db/hooks.php', + ]), + ); + + // Prepare renderer. + $PAGE->blocks->add_region('side-pre'); + $renderer = $PAGE->get_renderer('core', null, RENDERER_TARGET_GENERAL); + + // Dispatch. + $hook = new before_footer_html_generation($renderer); + di::get(\core\hook\manager::class)->dispatch($hook); + + // Assert there is no general block instance existing. + $aiblockinstances = $DB->get_records('block_instances', ['blockname' => 'ai_chat']); + $this->assertEmpty($aiblockinstances); + + // Set proper pagetype. + set_config('showonpagetypes', 'vendor-bin-phpunit', 'block_ai_chat'); + // Dispatch. + ob_start(); + di::get(\core\hook\manager::class)->dispatch($hook); + $output = ob_get_clean(); + + // Assert bock is printed. + $this->assertStringContainsString('id="ai_chat_button"', $output); + // Assert there is one general block instance existing. + $aiblockinstances = $DB->get_records('block_instances', ['blockname' => 'ai_chat']); + $this->assertCount(1, $aiblockinstances); + $aiblockinstance = array_shift($aiblockinstances); + + $this->assertEquals(\context_system::instance()->id, $aiblockinstance->parentcontextid); + $this->assertEquals(0, $aiblockinstance->showinsubcontexts); + $this->assertEquals('', $aiblockinstance->pagetypepattern); + + // Dispatch, while global block exists. + ob_start(); + di::get(\core\hook\manager::class)->dispatch($hook); + $output = ob_get_clean(); + + // Assert there is no additional block instance created. + $aiblockinstances = $DB->get_records('block_instances', ['blockname' => 'ai_chat']); + $this->assertCount(1, $aiblockinstances); + } + + /** + * Test the course settings form hooks. + * + * @return void + * @throws \coding_exception + * + * @covers \hook_callbacks::handle_after_form_definition + * @covers \hook_callbacks::handle_after_form_submission + * @covers \hook_callbacks::handle_after_form_definition_after_data + */ + public function test_course_settings_form_hooks(): void { + global $CFG, $DB, $PAGE; + + $this->resetAfterTest(); + $this->setAdminUser(); + + // Replace the version of the manager in the DI container with a phpunit one. + \core\di::set( + \core\hook\manager::class, + \core\hook\manager::phpunit_get_instance([ + 'block_ai_chat' => $CFG->dirroot . '/blocks/ai_chat/db/hooks.php', + ]), + ); + + $category = $this->getDataGenerator()->create_category(); + $course = $this->getDataGenerator()->create_course(['category' => $category->id]); + $coursecontext = \context_course::instance($course->id); + + $PAGE->set_url('/'); + $editoroptions = [ + 'context' => $coursecontext, + ]; + + // Create an course settings edit form mock. + $course = file_prepare_standard_editor($course, 'summary', $editoroptions, + $coursecontext, 'course', 'summary', 0); + $editform = new mock_course_edit_form(null, ['course' => $course, 'category' => $category, + 'editoroptions' => $editoroptions, 'returnto' => '0', 'returnurl' => '']); + $mform = $editform->get_mform(); + + // Tenants are not restricted per default, so addaichat form element should exist. + $addaichatelement = $mform->getElement('addaichat'); + $this->assertNotEmpty($addaichatelement); + + // Trigger definition_after_data. + $editform->render(); + + // There is no other instance of an ai chat block, so lement should be unchecked. + $this->assertEquals(false, $addaichatelement->getValue()); + + // Assert there is no general block instance existing. + $aiblockinstances = $DB->get_records('block_instances', ['blockname' => 'ai_chat']); + $this->assertEmpty($aiblockinstances); + + // Update with addaichat flag set. + $data = (object) $mform->exportValues(); + $data = file_postupdate_standard_editor($data, 'summary', $editoroptions, + $coursecontext, 'course', 'summary', 0); + $data->addaichat = 1; + // Update course triggers handle_after_form_submission. + update_course($data, $editoroptions); + + // Assert there is one course block instance existing. + $aiblockinstances = $DB->get_records('block_instances', ['blockname' => 'ai_chat']); + $this->assertCount(1, $aiblockinstances); + $aiblockinstance = array_shift($aiblockinstances); + + $this->assertEquals($coursecontext->id, $aiblockinstance->parentcontextid); + $this->assertEquals(1, $aiblockinstance->showinsubcontexts); + $this->assertEquals('*', $aiblockinstance->pagetypepattern); + + // Assert that addaichat element is existing and is checked. + $editform = new mock_course_edit_form(null, ['course' => $course, 'category' => $category, + 'editoroptions' => $editoroptions, 'returnto' => '0', 'returnurl' => '']); + $mform = $editform->get_mform(); + + // Tenants are not restricted per default, so addaichat form element should exist. + $addaichatelement = $mform->getElement('addaichat'); + $this->assertNotEmpty($addaichatelement); + + // Trigger definition_after_data. + $editform->render(); + + // There is no other instance of an ai chat block, so element should be checked. + $this->assertEquals(true, $addaichatelement->getValue()); + + // Update with addaichat flag set. + $data = (object) $mform->exportValues(); + $data = file_postupdate_standard_editor($data, 'summary', $editoroptions, $coursecontext, 'course', 'summary', 0); + $data->addaichat = 0; + // Update course triggers handle_after_form_submission. + update_course($data, $editoroptions); + + // Assert there is no course block instance existing. + $aiblockinstances = $DB->get_records('block_instances', ['blockname' => 'ai_chat']); + $this->assertCount(0, $aiblockinstances); + } +} + +/** + * Class to retrieve MoodleQuickform from course_edit_form. + * + * @package block_ai_chat + * @copyright 2024 Andreas Wagner + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @coversNothing + */ +class mock_course_edit_form extends \course_edit_form { + + /** + * Get the protected MoodleQuickForm. + * + * @return MoodleQuickForm the form used. + */ + public function get_mform(): \MoodleQuickForm { + return $this->_form; + } +} diff --git a/version.php b/version.php index 21494f7..5f9e195 100644 --- a/version.php +++ b/version.php @@ -26,7 +26,7 @@ defined('MOODLE_INTERNAL') || die(); $plugin->release = '1.0'; -$plugin->version = 2024111201; +$plugin->version = 2024111202; $plugin->requires = 2024042200; $plugin->component = 'block_ai_chat'; $plugin->maturity = MATURITY_STABLE;