diff --git a/communication/classes/api.php b/communication/classes/api.php index 8973d22b4c3..d49c0054327 100644 --- a/communication/classes/api.php +++ b/communication/classes/api.php @@ -16,6 +16,7 @@ namespace core_communication; +use core\context; use core_communication\task\add_members_to_room_task; use core_communication\task\create_and_configure_room_task; use core_communication\task\delete_room_task; @@ -51,37 +52,47 @@ class api { * This class is the entrypoint for all kinda usages. * It will be used by the other api to manage the communication providers. * + * @param context $context The context of the item for the instance * @param string $component The component of the item for the instance * @param string $instancetype The type of the item for the instance * @param int $instanceid The id of the instance * */ private function __construct( + private context $context, private string $component, private string $instancetype, - private int $instanceid + private int $instanceid, ) { $this->communication = processor::load_by_instance( - $this->component, - $this->instancetype, - $this->instanceid, + context: $context, + component: $component, + instancetype: $instancetype, + instanceid: $instanceid, ); } /** * Get the communication processor object. * + * @param context $context The context of the item for the instance * @param string $component The component of the item for the instance * @param string $instancetype The type of the item for the instance * @param int $instanceid The id of the instance * @return api */ public static function load_by_instance( + context $context, string $component, string $instancetype, - int $instanceid + int $instanceid, ): self { - return new self($component, $instancetype, $instanceid); + return new self( + context: $context, + component: $component, + instancetype: $instancetype, + instanceid: $instanceid, + ); } /** @@ -89,9 +100,10 @@ class api { */ public function reload(): void { $this->communication = processor::load_by_instance( - $this->component, - $this->instancetype, - $this->instanceid, + context: $this->context, + component: $this->component, + instancetype: $this->instancetype, + instanceid: $this->instanceid, ); } @@ -398,11 +410,12 @@ class api { if ($selectedcommunication !== processor::PROVIDER_NONE && $selectedcommunication !== '') { // Create communication record. $this->communication = processor::create_instance( - $selectedcommunication, - $this->instanceid, - $this->component, - $this->instancetype, - $communicationroomname, + context: $this->context, + provider: $selectedcommunication, + instanceid: $this->instanceid, + component: $this->component, + instancetype: $this->instancetype, + roomname: $communicationroomname, ); // Update provider record from form data. diff --git a/communication/classes/form/configure_form.php b/communication/classes/form/configure_form.php index d01bf324c8f..fc317909e88 100644 --- a/communication/classes/form/configure_form.php +++ b/communication/classes/form/configure_form.php @@ -24,6 +24,9 @@ namespace core_communication\form; +use core\context; +use stdClass; + defined('MOODLE_INTERNAL') || die(); require_once($CFG->libdir . '/formslib.php'); @@ -40,27 +43,30 @@ class configure_form extends \moodleform { /** * Class constructor * - * @param \stdClass $course Course object + * @param context $context Context object * @param int|null $instanceid Instance ID * @param string|null $instancetype Instance type * @param string|null $component Component name * @param string|null $selectedcommunication Selected communication service (provider) + * @param stdClass|null $instancedata Instance data */ public function __construct( - \stdClass $course, + context $context, ?int $instanceid = null, ?string $instancetype = null, ?string $component = null, ?string $selectedcommunication = null, + ?stdClass $instancedata = null, ) { parent::__construct( null, [ - 'instance' => $course, + 'context' => $context, 'instanceid' => $instanceid, 'instancetype' => $instancetype, 'component' => $component, 'selectedcommunication' => $selectedcommunication, + 'instancedata' => $instancedata, ], ); } @@ -70,19 +76,21 @@ class configure_form extends \moodleform { */ public function definition() { $mform = $this->_form; + $context = $this->_customdata['context']; $instanceid = $this->_customdata['instanceid']; $instancetype = $this->_customdata['instancetype']; $component = $this->_customdata['component']; - $instance = $this->_customdata['instance']; + $instancedata = $this->_customdata['instancedata']; // Add communication plugins to the form. $this->communication = \core_communication\api::load_by_instance( - $component, - $instancetype, - $instanceid + context: $context, + component: $component, + instancetype: $instancetype, + instanceid: $instanceid, ); $this->communication->form_definition($mform); - $this->communication->set_data($instance); + $this->communication->set_data($instancedata); $this->set_form_definition_for_provider(); @@ -94,6 +102,8 @@ class configure_form extends \moodleform { $mform->closeHeaderBefore('buttonar'); // Hidden elements. + $mform->addElement('hidden', 'contextid', $context->id); + $mform->setType('contextid', PARAM_INT); $mform->addElement('hidden', 'instanceid', $instanceid); $mform->setType('instanceid', PARAM_INT); $mform->addElement('hidden', 'instancetype', $instancetype); @@ -102,7 +112,7 @@ class configure_form extends \moodleform { $mform->setType('component', PARAM_TEXT); // Finally set the current form data. - $this->set_data($instance); + $this->set_data($instancedata); } /** @@ -112,13 +122,13 @@ class configure_form extends \moodleform { * and then use it to show the provider form fields. */ private function set_form_definition_for_provider(): void { - $instance = $this->_customdata['instance']; + $instancedata = $this->_customdata['instancedata']; if ($selectedcommunication = $this->_customdata['selectedcommunication']) { // First is to check whether the selected communication was selected from the form. $provider = $selectedcommunication; - } else if (isset($instance->selectedcommunication)) { + } else if (isset($instancedata->selectedcommunication)) { // If the form is not yet submitted, get the value from the DB. - $provider = $instance->selectedcommunication; + $provider = $instancedata->selectedcommunication; } else { // Otherwise, set to PROVIDER_NONE. $provider = \core_communication\processor::PROVIDER_NONE; diff --git a/communication/classes/processor.php b/communication/classes/processor.php index 396044716ca..e1bd735c10e 100644 --- a/communication/classes/processor.php +++ b/communication/classes/processor.php @@ -16,6 +16,7 @@ namespace core_communication; +use core\context; use stdClass; use stored_file; @@ -68,6 +69,7 @@ class processor { /** * Create communication instance. * + * @param context $context The context of the item for the instance * @param string $provider The communication provider * @param int $instanceid The instance id * @param string $component The component name @@ -76,6 +78,7 @@ class processor { * @return processor|null */ public static function create_instance( + context $context, string $provider, int $instanceid, string $component, @@ -88,6 +91,7 @@ class processor { return null; } $record = (object) [ + 'contextid' => $context->id, 'provider' => $provider, 'instanceid' => $instanceid, 'component' => $component, @@ -357,12 +361,14 @@ class processor { /** * Load communication instance by instance id. * + * @param context $context The context of the item for the instance * @param string $component The component name * @param string $instancetype The instance type * @param int $instanceid The instance id * @return processor|null */ public static function load_by_instance( + context $context, string $component, string $instancetype, int $instanceid @@ -371,6 +377,7 @@ class processor { global $DB; $record = $DB->get_record('communication', [ + 'contextid' => $context->id, 'instanceid' => $instanceid, 'component' => $component, 'instancetype' => $instancetype, @@ -411,6 +418,33 @@ class processor { return $this->instancedata->id; } + /** + * Get the context of the communication instance. + * + * @return context + */ + public function get_context(): context { + return context::instance_by_id($this->get_context_id()); + } + + /** + * Get the context id of the communication instance. + * + * @return int + */ + public function get_context_id(): int { + return $this->instancedata->contextid; + } + + /** + * Get communication instance type. + * + * @return string + */ + public function get_instance_type(): string { + return $this->instancedata->instancetype; + } + /** * Get communication instance id. * diff --git a/communication/configure.php b/communication/configure.php index 6043dbe2fa5..9a9f2266850 100644 --- a/communication/configure.php +++ b/communication/configure.php @@ -27,12 +27,15 @@ require_once('lib.php'); require_login(); +$contextid = required_param('contextid', PARAM_INT); $instanceid = required_param('instanceid', PARAM_INT); $instancetype = required_param('instancetype', PARAM_TEXT); $component = required_param('component', PARAM_COMPONENT); $selectedcommunication = optional_param('selectedcommunication', null, PARAM_PLUGIN); +$context = \core\context::instance_by_id($contextid); $instanceinfo = [ + 'contextid' => $context->id, 'instanceid' => $instanceid, 'instancetype' => $instancetype, 'component' => $component, @@ -44,7 +47,12 @@ if (!core_communication\api::is_available()) { } // Attempt to load the communication instance with the provided params. -$communication = \core_communication\api::load_by_instance($component, $instancetype, $instanceid); +$communication = \core_communication\api::load_by_instance( + context: $context, + component: $component, + instancetype: $instancetype, + instanceid: $instanceid, +); // No communication, no way this form can be used. if (!$communication) { @@ -66,15 +74,16 @@ $PAGE->set_heading($heading); $PAGE->add_body_class('limitedwidth'); // Append the instance data before passing to form object. -$instanceinfo['instance'] = $instance; +$instanceinfo['instancedata'] = $instance; // Get our form definitions. $form = new \core_communication\form\configure_form( - course: $instanceinfo['instance'], + context: $context, instanceid: $instanceinfo['instanceid'], instancetype: $instanceinfo['instancetype'], component: $instanceinfo['component'], - selectedcommunication: $selectedcommunication + selectedcommunication: $selectedcommunication, + instancedata: $instanceinfo['instancedata'], ); diff --git a/communication/provider/customlink/tests/communication_feature_test.php b/communication/provider/customlink/tests/communication_feature_test.php index 136da680508..d68141f7b40 100644 --- a/communication/provider/customlink/tests/communication_feature_test.php +++ b/communication/provider/customlink/tests/communication_feature_test.php @@ -98,12 +98,14 @@ class communication_feature_test extends \advanced_testcase { protected function get_test_communication_processor(): processor { $course = $this->getDataGenerator()->create_course(); $instanceid = $course->id; + $context = \core\context\system::instance(); $component = 'core_course'; $instancetype = 'coursecommunication'; $selectedcommunication = 'communication_customlink'; $communicationroomname = 'communicationroom'; $communicationprocessor = processor::create_instance( + $context, $selectedcommunication, $instanceid, $component, diff --git a/communication/provider/matrix/classes/communication_feature.php b/communication/provider/matrix/classes/communication_feature.php index f0bec8b8c0f..1f377f7b6ce 100644 --- a/communication/provider/matrix/classes/communication_feature.php +++ b/communication/provider/matrix/classes/communication_feature.php @@ -737,10 +737,9 @@ class communication_feature implements * @return int */ public function get_user_allowed_power_level(int $userid): int { - $context = \core\context\course::instance($this->processor->get_instance_id()); $powerlevel = matrix_constants::POWER_LEVEL_DEFAULT; - if (has_capability('communication/matrix:moderator', $context, $userid)) { + if (has_capability('communication/matrix:moderator', $this->processor->get_context(), $userid)) { $powerlevel = matrix_constants::POWER_LEVEL_MOODLE_MODERATOR; } diff --git a/communication/provider/matrix/tests/communication_feature_test.php b/communication/provider/matrix/tests/communication_feature_test.php index 20ec6ee9ae0..9aed0670e1c 100644 --- a/communication/provider/matrix/tests/communication_feature_test.php +++ b/communication/provider/matrix/tests/communication_feature_test.php @@ -16,6 +16,7 @@ namespace communication_matrix; +use core\context; use core_communication\api; use core_communication\communication_test_helper_trait; use core_communication\processor; @@ -55,6 +56,7 @@ class communication_feature_test extends \advanced_testcase { public function test_create_chat_room(): void { // Set up the test data first. $communication = \core_communication\api::load_by_instance( + context: \core\context\system::instance(), component: 'communication_matrix', instancetype: 'example', instanceid: 1, @@ -441,6 +443,7 @@ class communication_feature_test extends \advanced_testcase { $provider->update_room_membership([$user->id]); $processor = \core_communication\processor::load_by_instance( + context: \core\context\course::instance($course->id), component: 'core_course', instancetype: 'coursecommunication', instanceid: $course->id, @@ -478,6 +481,7 @@ class communication_feature_test extends \advanced_testcase { role_assign($studentrole->id, $user2->id, $coursecontext->id); $communicationprocessor = processor::load_by_instance( + context: \core\context\course::instance($course->id), component: 'core_course', instancetype: 'coursecommunication', instanceid: $course->id @@ -514,9 +518,11 @@ class communication_feature_test extends \advanced_testcase { ?string $roomtopic = null, ?\stored_file $roomavatar = null, array $members = [], + ?context $context = null, ): \core_communication\api { // Create a new room. $communication = \core_communication\api::load_by_instance( + context: $context ?? \core\context\system::instance(), component: $component, instancetype: $itemtype, instanceid: $itemid, @@ -548,6 +554,7 @@ class communication_feature_test extends \advanced_testcase { public function test_is_configured(): void { $course = $this->get_course(); $communicationprocessor = processor::load_by_instance( + context: \core\context\course::instance($course->id), component: 'core_course', instancetype: 'coursecommunication', instanceid: $course->id diff --git a/communication/provider/matrix/tests/local/command_test.php b/communication/provider/matrix/tests/local/command_test.php index c2a2329a36f..c255f26ed63 100644 --- a/communication/provider/matrix/tests/local/command_test.php +++ b/communication/provider/matrix/tests/local/command_test.php @@ -102,7 +102,7 @@ class command_test extends \advanced_testcase { * * @return array */ - public function url_parsing_provider(): array { + public static function url_parsing_provider(): array { return [ [ 'example/:id/endpoint', @@ -194,7 +194,7 @@ class command_test extends \advanced_testcase { * * @return array */ - public function parameter_and_option_provider(): array { + public static function parameter_and_option_provider(): array { $command = [ 'method' => 'PUT', 'endpoint' => 'example/:id/endpoint', @@ -289,7 +289,7 @@ class command_test extends \advanced_testcase { * Data provider for query parameter tests. * @return array */ - public function query_provider(): array { + public static function query_provider(): array { return [ 'no query' => [ 'query' => [], @@ -350,7 +350,7 @@ class command_test extends \advanced_testcase { * * @return array */ - public function sendasjson_provider(): array { + public static function sendasjson_provider(): array { return [ 'As JSON' => [ 'sendasjon' => true, diff --git a/communication/provider/matrix/tests/matrix_client_test.php b/communication/provider/matrix/tests/matrix_client_test.php index 44072ba9018..00680e242ad 100644 --- a/communication/provider/matrix/tests/matrix_client_test.php +++ b/communication/provider/matrix/tests/matrix_client_test.php @@ -47,7 +47,7 @@ class matrix_client_test extends \advanced_testcase { * Data provider for valid calls to ::instance. * @return array */ - public function instance_provider(): array { + public static function instance_provider(): array { $testcases = [ 'Standard versions' => [ null, @@ -56,7 +56,7 @@ class matrix_client_test extends \advanced_testcase { ]; // Remove a couple of versions. - $versions = $this->get_current_versions(); + $versions = self::get_current_versions(); array_pop($versions); array_pop($versions); @@ -251,7 +251,7 @@ class matrix_client_test extends \advanced_testcase { * * @return array */ - public function implements_feature_provider(): array { + public static function implements_feature_provider(): array { return [ 'Basic supported feature' => [ 'v1.7', @@ -299,12 +299,12 @@ class matrix_client_test extends \advanced_testcase { * * @return array */ - public function require_features_provider(): array { + public static function require_features_provider(): array { // We'll just add to the standard testcases. $testcases = array_map(static function (array $testcase): array { $testcase[1] = [$testcase[1]]; return $testcase; - }, $this->implements_feature_provider()); + }, self::implements_feature_provider()); $testcases['Require many supported features'] = [ 'v1.6', @@ -361,7 +361,7 @@ class matrix_client_test extends \advanced_testcase { * * @return array */ - public function get_version_provider(): array { + public static function get_version_provider(): array { return [ ['v1.1', '1.1'], ['v1.7', '1.7'], @@ -415,7 +415,7 @@ class matrix_client_test extends \advanced_testcase { * * @return array */ - public function meets_version_provider(): array { + public static function meets_version_provider(): array { return [ 'Same version' => ['v1.1', '1.1', true], 'Same version latest' => ['v1.7', '1.7', true], diff --git a/communication/provider/matrix/tests/matrix_client_test_trait.php b/communication/provider/matrix/tests/matrix_client_test_trait.php index 21e6bc0f296..02d33f337c2 100644 --- a/communication/provider/matrix/tests/matrix_client_test_trait.php +++ b/communication/provider/matrix/tests/matrix_client_test_trait.php @@ -102,8 +102,8 @@ trait matrix_client_test_trait { array $unstablefeatures = null, ): Response { $data = (object) [ - "versions" => array_values($this->get_current_versions()), - "unstable_features" => $this->get_current_unstable_features(), + "versions" => array_values(self::get_current_versions()), + "unstable_features" => self::get_current_unstable_features(), ]; if ($versions) { @@ -122,7 +122,7 @@ trait matrix_client_test_trait { * * @return array */ - protected function get_current_versions(): array { + protected static function get_current_versions(): array { return [ v1p1::class => "v1.1", v1p2::class => "v1.2", @@ -138,7 +138,7 @@ trait matrix_client_test_trait { * A helper to get the current unstable features returned by synapse. * @return array */ - protected function get_current_unstable_features(): array { + protected static function get_current_unstable_features(): array { return [ "org.matrix.label_based_filtering" => true, "org.matrix.e2e_cross_signing" => true, diff --git a/communication/provider/matrix/tests/matrix_test_helper_trait.php b/communication/provider/matrix/tests/matrix_test_helper_trait.php index 14b6a2fe7f0..7a09a188592 100644 --- a/communication/provider/matrix/tests/matrix_test_helper_trait.php +++ b/communication/provider/matrix/tests/matrix_test_helper_trait.php @@ -16,6 +16,7 @@ namespace communication_matrix; +use core\context; use GuzzleHttp\Psr7\Response; /** @@ -259,9 +260,12 @@ trait matrix_test_helper_trait { ?string $roomtopic = null, ?\stored_file $roomavatar = null, array $members = [], + ?context $context = null, ): \core_communication\api { + $context = $context ?? \core\context\system::instance(); // Create a new room. $communication = \core_communication\api::load_by_instance( + context: $context, component: $component, instancetype: $itemtype, instanceid: $itemid, @@ -282,6 +286,7 @@ trait matrix_test_helper_trait { $this->run_all_adhoc_tasks(); return \core_communication\api::load_by_instance( + context: $context, component: $component, instancetype: $itemtype, instanceid: $itemid, diff --git a/communication/provider/matrix/tests/matrix_user_manager_test.php b/communication/provider/matrix/tests/matrix_user_manager_test.php index d96abc8751b..860fb10cf5e 100644 --- a/communication/provider/matrix/tests/matrix_user_manager_test.php +++ b/communication/provider/matrix/tests/matrix_user_manager_test.php @@ -16,7 +16,6 @@ namespace communication_matrix; -use core_communication\processor; use moodle_exception; /** @@ -105,7 +104,7 @@ class matrix_user_manager_test extends \advanced_testcase { * * @return array */ - public function get_formatted_matrix_userid_provider(): array { + public static function get_formatted_matrix_userid_provider(): array { return [ 'alphanumeric' => [ 'https://matrix.example.org', @@ -135,12 +134,12 @@ class matrix_user_manager_test extends \advanced_testcase { * * @return array */ - public function set_matrix_userid_in_moodle_provider(): array { + public static function set_matrix_userid_in_moodle_provider(): array { return array_combine( - array_keys($this->get_formatted_matrix_userid_provider()), + array_keys(self::get_formatted_matrix_userid_provider()), array_map( fn($value) => [$value[2]], - $this->get_formatted_matrix_userid_provider(), + self::get_formatted_matrix_userid_provider(), ), ); } @@ -196,7 +195,7 @@ class matrix_user_manager_test extends \advanced_testcase { * * @return array */ - public function get_formatted_matrix_home_server_provider(): array { + public static function get_formatted_matrix_home_server_provider(): array { return [ 'www is removed' => [ 'https://www.example.org', diff --git a/communication/tests/api_test.php b/communication/tests/api_test.php index a0e92a6558d..8f976d6a77f 100644 --- a/communication/tests/api_test.php +++ b/communication/tests/api_test.php @@ -16,13 +16,13 @@ namespace core_communication; +use communication_matrix\matrix_test_helper_trait; + 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. * @@ -33,7 +33,6 @@ use \communication_matrix\matrix_test_helper_trait; * @covers \core_communication\api */ class api_test extends \advanced_testcase { - use matrix_test_helper_trait; use communication_test_helper_trait; @@ -62,9 +61,10 @@ class api_test extends \advanced_testcase { $course = $this->get_course(); $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); // Sample data. @@ -86,9 +86,10 @@ class api_test extends \advanced_testcase { $course = $this->get_course(); $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertEquals('communication_matrix', $communication->get_provider()); @@ -113,18 +114,20 @@ class api_test extends \advanced_testcase { // Create the room, settingthe avatar. $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id, + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $communication->create_and_configure_room($selectedcommunication, $communicationroomname, $avatar); // Reload the communication processor. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id, + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); // Compare result. @@ -146,9 +149,10 @@ class api_test extends \advanced_testcase { $selectedcommunication = 'communication_matrix'; $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $communication->create_and_configure_room($selectedcommunication, $communicationroomname); @@ -161,9 +165,10 @@ class api_test extends \advanced_testcase { // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertEquals($communicationroomname, $communicationprocessor->get_room_name()); @@ -183,9 +188,10 @@ class api_test extends \advanced_testcase { // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertNull($communicationprocessor); @@ -202,17 +208,19 @@ class api_test extends \advanced_testcase { $selectedcommunication = 'communication_matrix'; $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $communication->update_room($selectedcommunication, $communicationroomname); // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertEquals($communicationroomname, $communicationprocessor->get_room_name()); @@ -231,18 +239,20 @@ class api_test extends \advanced_testcase { // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertEquals($communicationroomname, $communicationprocessor->get_room_name()); $this->assertEquals($selectedcommunication, $communicationprocessor->get_provider()); $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $communication->delete_room(); @@ -267,9 +277,10 @@ class api_test extends \advanced_testcase { // First test the adding members to a room. $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $communication->add_members_to_room([$userid]); diff --git a/communication/tests/processor_test.php b/communication/tests/processor_test.php index 84c283350ec..fd427eb680c 100644 --- a/communication/tests/processor_test.php +++ b/communication/tests/processor_test.php @@ -16,13 +16,13 @@ namespace core_communication; +use communication_matrix\matrix_test_helper_trait; + 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. * @@ -33,7 +33,6 @@ use \communication_matrix\matrix_test_helper_trait; * @coversDefaultClass \core_communication\processor */ class processor_test extends \advanced_testcase { - use matrix_test_helper_trait; use communication_test_helper_trait; @@ -56,14 +55,16 @@ class processor_test extends \advanced_testcase { global $DB; $this->resetAfterTest(); - // Sameple test data. + // Sample test data. $instanceid = 10; - $component = 'core_course'; - $instancetype = 'coursecommunication'; + $context = \core\context\system::instance(); + $component = 'core'; + $instancetype = 'mycommunication'; $selectedcommunication = 'communication_matrix'; $communicationroomname = 'communicationroom'; $communicationprocessor = processor::create_instance( + $context, $selectedcommunication, $instanceid, $component, @@ -79,6 +80,7 @@ class processor_test extends \advanced_testcase { // Test against the set data. $this->assertNotEmpty($communicationrecord); + $this->assertEquals($context->id, $communicationrecord->contextid); $this->assertEquals($instanceid, $communicationrecord->instanceid); $this->assertEquals($component, $communicationrecord->component); $this->assertEquals($selectedcommunication, $communicationrecord->provider); @@ -86,6 +88,8 @@ class processor_test extends \advanced_testcase { $this->assertEquals($instancetype, $communicationrecord->instancetype); // Test against the object. + $this->assertEquals($context->id, $communicationprocessor->get_context_id()); + $this->assertEquals($context, $communicationprocessor->get_context()); $this->assertEquals($communicationprocessor->get_id(), $communicationrecord->id); $this->assertEquals($communicationprocessor->get_provider(), $communicationrecord->provider); $this->assertEquals($communicationprocessor->get_room_name(), $communicationrecord->roomname); @@ -105,12 +109,14 @@ class processor_test extends \advanced_testcase { // Sameple test data. $instanceid = 10; - $component = 'core_course'; - $instancetype = 'coursecommunication'; + $context = \core\context\system::instance(); + $component = 'core'; + $instancetype = 'mycommunication'; $selectedcommunication = 'communication_matrix'; $communicationroomname = 'communicationroom'; $communicationprocessor = processor::create_instance( + $context, $selectedcommunication, $instanceid, $component, @@ -132,6 +138,7 @@ class processor_test extends \advanced_testcase { // Test against the set data. $this->assertNotEmpty($communicationrecord); + $this->assertEquals($context->id, $communicationrecord->contextid); $this->assertEquals($instanceid, $communicationrecord->instanceid); $this->assertEquals($component, $communicationrecord->component); $this->assertEquals(processor::PROVIDER_INACTIVE, $communicationrecord->active); @@ -139,6 +146,8 @@ class processor_test extends \advanced_testcase { $this->assertEquals($instancetype, $communicationrecord->instancetype); // Test against the object. + $this->assertEquals($context->id, $communicationprocessor->get_context_id()); + $this->assertEquals($context, $communicationprocessor->get_context()); $this->assertEquals($communicationprocessor->get_id(), $communicationrecord->id); $this->assertEquals($communicationprocessor->is_instance_active(), $communicationrecord->active); $this->assertEquals($communicationprocessor->get_room_name(), $communicationrecord->roomname); @@ -157,12 +166,14 @@ class processor_test extends \advanced_testcase { // Sameple test data. $instanceid = 10; - $component = 'core_course'; - $instancetype = 'coursecommunication'; + $context = \core\context\system::instance(); + $component = 'core'; + $instancetype = 'mycommunication'; $selectedcommunication = 'communication_matrix'; $communicationroomname = 'communicationroom'; $communicationprocessor = processor::create_instance( + $context, $selectedcommunication, $instanceid, $component, @@ -184,9 +195,10 @@ class processor_test extends \advanced_testcase { // Test against the object. $communicationprocessor = processor::load_by_instance( - $component, - $instancetype, - $instanceid + context: $context, + component: $component, + instancetype: $instancetype, + instanceid: $instanceid, ); $this->assertNull($communicationprocessor); } @@ -200,12 +212,14 @@ class processor_test extends \advanced_testcase { public function test_load_by_instance(): void { $this->resetAfterTest(); $course = $this->get_course(); + $context = \core\context\course::instance($course->id); // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertNotNull($communicationprocessor); @@ -225,12 +239,14 @@ class processor_test extends \advanced_testcase { public function test_load_by_id(): void { $this->resetAfterTest(); $course = $this->get_course(); + $context = \core\context\course::instance($course->id); // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $communicationprocessorbyid = processor::load_by_id($communicationprocessor->get_id()); @@ -251,12 +267,14 @@ class processor_test extends \advanced_testcase { public function test_get_component(): void { $this->resetAfterTest(); $course = $this->get_course(); + $context = \core\context\course::instance($course->id); // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertEquals('core_course', $communicationprocessor->get_component()); @@ -271,12 +289,14 @@ class processor_test extends \advanced_testcase { public function test_get_provider(): void { $this->resetAfterTest(); $course = $this->get_course(); + $context = \core\context\course::instance($course->id); // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertEquals('communication_matrix', $communicationprocessor->get_provider()); @@ -291,12 +311,14 @@ class processor_test extends \advanced_testcase { public function test_get_room_name(): void { $this->resetAfterTest(); $course = $this->get_course(); + $context = \core\context\course::instance($course->id); // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertEquals('Sampleroom', $communicationprocessor->get_room_name()); @@ -313,12 +335,14 @@ class processor_test extends \advanced_testcase { public function test_get_room_provider(): void { $this->resetAfterTest(); $course = $this->get_course(); + $context = \core\context\course::instance($course->id); // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertInstanceOf(room_chat_provider::class, $communicationprocessor->get_room_provider()); @@ -335,12 +359,14 @@ class processor_test extends \advanced_testcase { public function test_get_user_provider(): void { $this->resetAfterTest(); $course = $this->get_course(); + $context = \core\context\course::instance($course->id); // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertInstanceOf(user_provider::class, $communicationprocessor->get_room_provider()); @@ -359,12 +385,14 @@ class processor_test extends \advanced_testcase { public function test_get_room_user_provider(): void { $this->resetAfterTest(); $course = $this->get_course(); + $context = \core\context\course::instance($course->id); // Test the communication record exists. $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertInstanceOf(room_user_provider::class, $communicationprocessor->get_room_user_provider()); @@ -395,16 +423,18 @@ class processor_test extends \advanced_testcase { ); $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $communication->create_and_configure_room($selectedcommunication, $communicationroomname, $avatar); $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $avatar = $communicationprocessor->get_avatar(); @@ -421,9 +451,10 @@ class processor_test extends \advanced_testcase { $communicationprocessor->set_avatar_filename('newname.svg'); $communicationprocessor = processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $this->assertEquals($communicationprocessor->get_avatar_filename(), 'newname.svg'); } @@ -463,9 +494,10 @@ class processor_test extends \advanced_testcase { // Load the communication api. $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: $component, + instancetype: $instancetype, + instanceid: $course->id, ); $communication->create_and_configure_room($selectedcommunication, $communicationroomname); $communication->add_members_to_room([$user1, $user2]); @@ -475,9 +507,10 @@ class processor_test extends \advanced_testcase { // Test against the object. $communicationprocessor = processor::load_by_instance( - $component, - $instancetype, - $course->id + context: \core\context\course::instance($course->id), + component: $component, + instancetype: $instancetype, + instanceid: $course->id, ); $this->assertEquals([$user1], $communicationprocessor->get_all_delete_flagged_userids()); diff --git a/course/lib.php b/course/lib.php index 5a55c15747a..3cfb4f6a6d2 100644 --- a/course/lib.php +++ b/course/lib.php @@ -2246,7 +2246,7 @@ function create_course($data, $editoroptions = NULL) { // Trigger a course created event. $event = \core\event\course_created::create(array( 'objectid' => $course->id, - 'context' => context_course::instance($course->id), + 'context' => $context, 'other' => array('shortname' => $course->shortname, 'fullname' => $course->fullname) )); @@ -2273,7 +2273,7 @@ function create_course($data, $editoroptions = NULL) { // Update course tags. if (isset($data->tags)) { - core_tag_tag::set_item_tags('core', 'course', $course->id, context_course::instance($course->id), $data->tags); + core_tag_tag::set_item_tags('core', 'course', $course->id, $context, $data->tags); } // Set up communication. if (core_communication\api::is_available()) { @@ -2288,9 +2288,10 @@ function create_course($data, $editoroptions = NULL) { // Communication api call. $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id, + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $communication->create_and_configure_room( $provider, @@ -2431,7 +2432,12 @@ function update_course($data, $editoroptions = NULL) { // Attempt to get the communication provider if it wasn't provided in the data. if (empty($provider) && core_communication\api::is_available()) { - $provider = \core_communication\api::load_by_instance('core_course', 'coursecommunication', $data->id)->get_provider(); + $provider = \core_communication\api::load_by_instance( + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $data->id, + )->get_provider(); } // Communication api call. @@ -2456,9 +2462,10 @@ function update_course($data, $editoroptions = NULL) { } $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $data->id + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $data->id, ); $addafterupdate = false; diff --git a/course/view.php b/course/view.php index fb8ab4f9b72..d9f1497c417 100644 --- a/course/view.php +++ b/course/view.php @@ -292,6 +292,7 @@ echo $OUTPUT->header(); // Show communication room status notification. if (core_communication\api::is_available() && has_capability('moodle/course:update', $context)) { $communication = \core_communication\api::load_by_instance( + $context, 'core_course', 'coursecommunication', $course->id diff --git a/lib/accesslib.php b/lib/accesslib.php index 276169e34bb..a034f18e0a0 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -1648,6 +1648,7 @@ function role_assign($roleid, $userid, $contextid, $component = '', $itemid = 0, // Update the room membership and power levels when the user role changes. if (\core_communication\api::is_available() && $coursecontext = $context->get_course_context(false)) { $communication = \core_communication\api::load_by_instance( + $coursecontext, 'core_course', 'coursecommunication', $coursecontext->instanceid, @@ -1765,6 +1766,7 @@ function role_unassign_all(array $params, $subcontexts = false, $includemanual = // Update the room membership and power levels when the user role changes. if (\core_communication\api::is_available() && $coursecontext = $context->get_course_context(false)) { $communication = \core_communication\api::load_by_instance( + $coursecontext, 'core_course', 'coursecommunication', $coursecontext->instanceid, diff --git a/lib/db/install.xml b/lib/db/install.xml index fb8b3447c73..490d8368d1e 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1,5 +1,5 @@ - @@ -221,6 +221,7 @@ + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index 81f318e8f77..4f255afcc40 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -3608,5 +3608,63 @@ privatefiles,moodle|/user/files.php'; upgrade_main_savepoint(true, 2023100400.01); } + if ($oldversion < 2023100400.02) { + // Define field id to be added to communication. + $table = new xmldb_table('communication'); + + // Add the field and allow it to be nullable. + // We need to backfill data before setting it to NOT NULL. + $field = new xmldb_field( + name: 'contextid', + type: XMLDB_TYPE_INTEGER, + precision: '10', + notnull: null, + previous: 'id', + ); + + // Conditionally launch add field id. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Fill the existing data. + $sql = <<execute($sql, [ + 'contextcourse' => CONTEXT_COURSE, + 'instancetype' => 'coursecommunication', + ]); + + $systemcontext = \core\context\system::instance(); + $DB->set_field_select( + table: 'communication', + newfield: 'contextid', + newvalue: $systemcontext->id, + select: 'contextid IS NULL', + ); + + // Now make it NOTNULL. + $field = new xmldb_field( + name: 'contextid', + type: XMLDB_TYPE_INTEGER, + precision: '10', + notnull: XMLDB_NOTNULL, + ); + $dbman->change_field_notnull($table, $field); + + // Add the contextid constraint. + $key = new xmldb_key('contextid', XMLDB_KEY_FOREIGN, ['contextid'], 'context', ['id']); + $dbman->add_key($table, $key); + + // Main savepoint reached. + upgrade_main_savepoint(true, 2023100400.02); + } + return true; } diff --git a/lib/enrollib.php b/lib/enrollib.php index 1210b368a6c..124c424e295 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -2162,9 +2162,10 @@ abstract class enrol_plugin { // Add users to a communication room. if (core_communication\api::is_available()) { $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $courseid + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $courseid, ); $communication->add_members_to_room([$userid]); } @@ -2231,10 +2232,12 @@ abstract class enrol_plugin { // Add/remove users to/from communication room. if (core_communication\api::is_available()) { $course = enrol_get_course_by_user_enrolment_id($ue->id); + $context = \core\context\course::instance($course->id); $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); if (($statusmodified && ((int) $ue->status === 1)) || ($timeendmodified && $ue->timeend !== 0 && (time() > $ue->timeend))) { @@ -2350,9 +2353,10 @@ abstract class enrol_plugin { // Remove users from a communication room. if (core_communication\api::is_available()) { $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $courseid + context: $context, + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $courseid, ); $communication->remove_members_from_room([$userid]); } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index cff4f375da3..6d2128bf0e2 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -4108,9 +4108,10 @@ function delete_user(stdClass $user) { if (core_communication\api::is_available()) { foreach (enrol_get_users_courses($user->id) as $course) { $communication = \core_communication\processor::load_by_instance( - 'core_course', - 'coursecommunication', - $course->id + context: \core\context\course::instance($course->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $course->id, ); $communication->get_room_user_provider()->remove_members_from_room([$user->id]); $communication->delete_instance_user_mapping([$user->id]); @@ -5148,16 +5149,17 @@ function delete_course($courseorid, $showfeedback = true) { // Make the course completely empty. remove_course_contents($courseid, $showfeedback); - // Delete the course and related context instance. - context_helper::delete_instance(CONTEXT_COURSE, $courseid); - // Communication provider delete associated information. $communication = \core_communication\api::load_by_instance( + $context, 'core_course', 'coursecommunication', $course->id ); + // Delete the course and related context instance. + context_helper::delete_instance(CONTEXT_COURSE, $courseid); + // Update communication room membership of enrolled users. require_once($CFG->libdir . '/enrollib.php'); $courseusers = enrol_get_course_users($courseid); diff --git a/lib/navigationlib.php b/lib/navigationlib.php index 8d23e8c2be2..4aed6e06bca 100644 --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -2986,8 +2986,12 @@ class global_navigation extends navigation_node { // Add link for configuring communication. if ($navoptions->communication) { - $url = new moodle_url('/communication/configure.php', ['instanceid' => $course->id, - 'instancetype' => 'coursecommunication', 'component' => 'core_course']); + $url = new moodle_url('/communication/configure.php', [ + 'contextid' => \core\context\course::instance($course->id)->id, + 'instanceid' => $course->id, + 'instancetype' => 'coursecommunication', + 'component' => 'core_course', + ]); $coursenode->add(get_string('communication', 'communication'), $url, navigation_node::TYPE_SETTING, null, 'communication'); } diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index 776d9a70c06..8a69c9d4d2e 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -4384,7 +4384,12 @@ EOD; global $COURSE; $url = ''; if ($COURSE->id !== SITEID) { - $comm = \core_communication\api::load_by_instance('core_course', 'coursecommunication', $COURSE->id); + $comm = \core_communication\api::load_by_instance( + context: \core\context\course::instance($COURSE->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $COURSE->id, + ); $url = $comm->get_communication_room_url(); } diff --git a/user/lib.php b/user/lib.php index 992c7a290b5..1010394e435 100644 --- a/user/lib.php +++ b/user/lib.php @@ -164,9 +164,10 @@ function user_update_user($user, $updatepassword = true, $triggerevent = true) { if (!empty($currentrecord) && isset($user->suspended) && $currentrecord->suspended !== $user->suspended) { foreach ($usercourses as $usercourse) { $communication = \core_communication\api::load_by_instance( - 'core_course', - 'coursecommunication', - $usercourse->id + context: \core\context\course::instance($usercourse->id), + component: 'core_course', + instancetype: 'coursecommunication', + instanceid: $usercourse->id ); // If the record updated the suspended for a user. if ($user->suspended === 0) { diff --git a/version.php b/version.php index 3368b1aa6f3..7cfb61b0914 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2023100400.01; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2023100400.02; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes. $release = '4.3rc1 (Build: 20231004)'; // Human-friendly version name