From d644c3d89dd6f3d946bf150dbc66735ecafccc4d Mon Sep 17 00:00:00 2001 From: Stevani Andolo Date: Wed, 6 Sep 2023 13:40:52 +0800 Subject: [PATCH] MDL-78750 communication_matrix: Dynamic settings --- .../classes/communication_provider.php | 8 +++++++ communication/classes/processor.php | 14 +++++++---- .../classes/communication_feature.php | 4 ++++ .../tests/communication_feature_test.php | 10 ++++++++ .../matrix/classes/communication_feature.php | 23 ++++++++++++++++++- communication/provider/matrix/settings.php | 11 +++------ .../tests/communication_feature_test.php | 20 ++++++++++++++++ communication/provider/matrix/version.php | 2 +- communication/tests/api_test.php | 6 +++++ communication/tests/processor_test.php | 22 ++++++++++++++---- 10 files changed, 100 insertions(+), 20 deletions(-) diff --git a/communication/classes/communication_provider.php b/communication/classes/communication_provider.php index 2023ee3cb5e..c45a6b0be30 100644 --- a/communication/classes/communication_provider.php +++ b/communication/classes/communication_provider.php @@ -34,4 +34,12 @@ interface communication_provider { * @param processor $communication The communication object */ public static function load_for_instance(processor $communication): self; + + /** + * Check if the provider is configured or not. + * + * This method is intended to check if the plugin have got any settings and if all the settings are set properly. + * This checking helps to reduce errors in future when a communication instance is added for the provider and not configured. + */ + public static function is_configured(): bool; } diff --git a/communication/classes/processor.php b/communication/classes/processor.php index 9ced9f1e1f8..396044716ca 100644 --- a/communication/classes/processor.php +++ b/communication/classes/processor.php @@ -347,7 +347,7 @@ class processor { public static function load_by_id(int $id): ?self { global $DB; $record = $DB->get_record('communication', ['id' => $id]); - if ($record && self::is_provider_enabled($record->provider)) { + if ($record && self::is_provider_available($record->provider)) { return new self($record); } @@ -376,7 +376,7 @@ class processor { 'instancetype' => $instancetype, ]); - if ($record && self::is_provider_enabled($record->provider)) { + if ($record && self::is_provider_available($record->provider)) { return new self($record); } @@ -671,12 +671,16 @@ class processor { } /** - * Is communication provider enabled/disabled. + * Is the communication provider enabled and configured, or disabled. * * @param string $provider provider component name * @return bool */ - public static function is_provider_enabled(string $provider): bool { - return \core\plugininfo\communication::is_plugin_enabled($provider); + public static function is_provider_available(string $provider): bool { + if (\core\plugininfo\communication::is_plugin_enabled($provider)) { + $providerclass = "{$provider}\\communication_feature"; + return $providerclass::is_configured(); + } + return false; } } diff --git a/communication/provider/customlink/classes/communication_feature.php b/communication/provider/customlink/classes/communication_feature.php index 3250be4a60a..e2b4c760a05 100644 --- a/communication/provider/customlink/classes/communication_feature.php +++ b/communication/provider/customlink/classes/communication_feature.php @@ -170,4 +170,8 @@ class communication_feature implements 'addcommunicationoptionshere' ), 'addcommunicationoptionshere'); } + + public static function is_configured(): bool { + return true; + } } diff --git a/communication/provider/customlink/tests/communication_feature_test.php b/communication/provider/customlink/tests/communication_feature_test.php index e3957e38c4f..37663c3fd9d 100644 --- a/communication/provider/customlink/tests/communication_feature_test.php +++ b/communication/provider/customlink/tests/communication_feature_test.php @@ -113,4 +113,14 @@ class communication_feature_test extends \advanced_testcase { return $communicationprocessor; } + + /** + * Test if the selected provider is configured. + * + * @return void + */ + public function test_is_configured() { + $communicationprocessor = $this->get_test_communication_processor(); + $this->assertTrue($communicationprocessor->get_form_provider()->is_configured()); + } } diff --git a/communication/provider/matrix/classes/communication_feature.php b/communication/provider/matrix/classes/communication_feature.php index 92cf7a00fd1..5788132bd06 100644 --- a/communication/provider/matrix/classes/communication_feature.php +++ b/communication/provider/matrix/classes/communication_feature.php @@ -92,7 +92,7 @@ class communication_feature implements $this->homeserverurl = get_config('communication_matrix', 'matrixhomeserverurl'); $this->webclienturl = get_config('communication_matrix', 'matrixelementurl'); - if ($this->homeserverurl) { + if ($processor::is_provider_available('communication_matrix')) { // Generate the API instance. $this->matrixapi = matrix_client::instance( serverurl: $this->homeserverurl, @@ -751,4 +751,25 @@ class communication_feature implements return $powerlevel; } + + /* + * Check if matrix settings are configured + * + * @return boolean + */ + public static function is_configured(): bool { + // Matrix communication settings. + $matrixhomeserverurl = get_config('communication_matrix', 'matrixhomeserverurl'); + $matrixaccesstoken = get_config('communication_matrix', 'matrixaccesstoken'); + $matrixelementurl = get_config('communication_matrix', 'matrixelementurl'); + + if ( + !empty($matrixhomeserverurl) && + !empty($matrixaccesstoken) && + (PHPUNIT_TEST || BEHAT_SITE_RUNNING || !empty($matrixelementurl)) + ) { + return true; + } + return false; + } } diff --git a/communication/provider/matrix/settings.php b/communication/provider/matrix/settings.php index 7fb817d31a8..93faf97de59 100644 --- a/communication/provider/matrix/settings.php +++ b/communication/provider/matrix/settings.php @@ -28,20 +28,15 @@ if ($hassiteconfig) { // Home server URL. $name = new lang_string('matrixhomeserverurl', 'communication_matrix'); $desc = new lang_string('matrixhomeserverurl_desc', 'communication_matrix'); - $settings->add(new admin_setting_requiredtext('communication_matrix/matrixhomeserverurl', $name, $desc, '')); + $settings->add(new admin_setting_configtext('communication_matrix/matrixhomeserverurl', $name, $desc, '')); // Access token. $name = new lang_string('matrixaccesstoken', 'communication_matrix'); $desc = new lang_string('matrixaccesstoken_desc', 'communication_matrix'); - $settings->add(new admin_setting_requiredpasswordunmask('communication_matrix/matrixaccesstoken', $name, $desc, '')); - - // Refresh token. - $name = new lang_string('matrixrefreshtoken', 'communication_matrix'); - $desc = new lang_string('matrixrefreshtoken_desc', 'communication_matrix'); - $settings->add(new admin_setting_requiredpasswordunmask('communication_matrix/matrixrefreshtoken', $name, $desc, '')); + $settings->add(new admin_setting_configpasswordunmask('communication_matrix/matrixaccesstoken', $name, $desc, '')); // Element web URL. $name = new lang_string('matrixelementurl', 'communication_matrix'); $desc = new lang_string('matrixelementurl_desc', 'communication_matrix'); - $settings->add(new admin_setting_requiredtext('communication_matrix/matrixelementurl', $name, $desc, '')); + $settings->add(new admin_setting_configtext('communication_matrix/matrixelementurl', $name, $desc, '')); } diff --git a/communication/provider/matrix/tests/communication_feature_test.php b/communication/provider/matrix/tests/communication_feature_test.php index ee75b81d8e8..496bd0e85d1 100644 --- a/communication/provider/matrix/tests/communication_feature_test.php +++ b/communication/provider/matrix/tests/communication_feature_test.php @@ -539,4 +539,24 @@ class communication_feature_test extends \advanced_testcase { $communication->reload(); return $communication; } + + /** + * Test if the selected provider is configured. + * + * @return void + */ + public function test_is_configured() { + $course = $this->get_course(); + $communicationprocessor = processor::load_by_instance( + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id + ); + $this->assertTrue($communicationprocessor->get_room_provider()->is_configured()); + + // Unset communication_matrix settings. + unset_config('matrixhomeserverurl', 'communication_matrix'); + unset_config('matrixaccesstoken', 'communication_matrix'); + $this->assertFalse($communicationprocessor->get_room_provider()->is_configured()); + } } diff --git a/communication/provider/matrix/version.php b/communication/provider/matrix/version.php index 803695f03f7..7c42c5722e0 100644 --- a/communication/provider/matrix/version.php +++ b/communication/provider/matrix/version.php @@ -25,6 +25,6 @@ defined('MOODLE_INTERNAL') || die(); $plugin->component = 'communication_matrix'; -$plugin->version = 2023090600; +$plugin->version = 2023092300; $plugin->requires = 2023011300; $plugin->maturity = MATURITY_ALPHA; diff --git a/communication/tests/api_test.php b/communication/tests/api_test.php index adafe0bd634..a0e92a6558d 100644 --- a/communication/tests/api_test.php +++ b/communication/tests/api_test.php @@ -18,8 +18,11 @@ namespace core_communication; defined('MOODLE_INTERNAL') || die(); +require_once(__DIR__ . '/../provider/matrix/tests/matrix_test_helper_trait.php'); require_once(__DIR__ . '/communication_test_helper_trait.php'); +use \communication_matrix\matrix_test_helper_trait; + /** * Class api_test to test the communication public api and its associated methods. * @@ -30,12 +33,15 @@ require_once(__DIR__ . '/communication_test_helper_trait.php'); * @covers \core_communication\api */ class api_test extends \advanced_testcase { + + use matrix_test_helper_trait; use communication_test_helper_trait; public function setUp(): void { parent::setUp(); $this->resetAfterTest(); $this->setup_communication_configs(); + $this->initialise_mock_server(); } /** diff --git a/communication/tests/processor_test.php b/communication/tests/processor_test.php index 145ea33e041..84c283350ec 100644 --- a/communication/tests/processor_test.php +++ b/communication/tests/processor_test.php @@ -18,8 +18,11 @@ namespace core_communication; defined('MOODLE_INTERNAL') || die(); +require_once(__DIR__ . '/../provider/matrix/tests/matrix_test_helper_trait.php'); require_once(__DIR__ . '/communication_test_helper_trait.php'); +use \communication_matrix\matrix_test_helper_trait; + /** * Class processor_test to test the communication internal api and its associated methods. * @@ -30,8 +33,17 @@ require_once(__DIR__ . '/communication_test_helper_trait.php'); * @coversDefaultClass \core_communication\processor */ class processor_test extends \advanced_testcase { + + use matrix_test_helper_trait; use communication_test_helper_trait; + public function setUp(): void { + parent::setUp(); + $this->resetAfterTest(); + $this->setup_communication_configs(); + $this->initialise_mock_server(); + } + /** * Test create instance. * @@ -417,18 +429,18 @@ class processor_test extends \advanced_testcase { } /** - * Test if the provider is enabled or disabled. + * Test if the provider is enabled and configured, or disabled. * - * @covers ::is_provider_enabled + * @covers ::is_provider_available */ - public function test_is_provider_enabled(): void { + public function test_is_provider_available(): void { $this->resetAfterTest(); $communicationprovider = 'communication_matrix'; - $this->assertTrue(processor::is_provider_enabled($communicationprovider)); + $this->assertTrue(processor::is_provider_available($communicationprovider)); // Now test is disabling the plugin returns false. set_config('disabled', 1, $communicationprovider); - $this->assertFalse(processor::is_provider_enabled($communicationprovider)); + $this->assertFalse(processor::is_provider_available($communicationprovider)); } /**