From 99c1543aa856dc15b45eeb8a2e4e63852fa4b6d8 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 4 Oct 2023 12:15:38 +0800 Subject: [PATCH] MDL-79581 communication: Context is a mandatory field The API was incorrectly assuming that all uses of the API were for a course, and that the instanceid of the communication instance was a course id. These assumptions are both entirely wrong. The API is intended to support a range of uses including use at the site, user, and activity levels. Furthermore, if a group were to be used, then the instanceid should be of that group's id, and therefore the contextid would need to be fetched or that group's course instead. The only solution here is to add a new contextid field to the table, and implement it all parts of the API. --- communication/classes/api.php | 41 ++++-- communication/classes/form/configure_form.php | 34 +++-- communication/classes/processor.php | 34 +++++ communication/configure.php | 17 ++- .../tests/communication_feature_test.php | 2 + .../tests/communication_feature_test.php | 7 + .../matrix/tests/matrix_test_helper_trait.php | 5 + communication/tests/api_test.php | 84 ++++++----- communication/tests/processor_test.php | 132 +++++++++++------- course/lib.php | 25 ++-- course/view.php | 1 + lib/accesslib.php | 2 + lib/db/install.xml | 3 +- lib/db/upgrade.php | 58 ++++++++ lib/enrollib.php | 22 +-- lib/moodlelib.php | 14 +- lib/navigationlib.php | 8 +- lib/outputrenderers.php | 7 +- user/lib.php | 7 +- version.php | 2 +- 20 files changed, 358 insertions(+), 147 deletions(-) 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/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/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/tests/api_test.php b/communication/tests/api_test.php index a0e92a6558d..3bcf4c7d3e9 100644 --- a/communication/tests/api_test.php +++ b/communication/tests/api_test.php @@ -62,9 +62,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 +87,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 +115,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 +150,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 +166,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 +189,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 +209,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 +240,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 +278,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..a4719935c1d 100644 --- a/communication/tests/processor_test.php +++ b/communication/tests/processor_test.php @@ -56,14 +56,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 +81,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 +89,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 +110,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 +139,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 +147,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 +167,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 +196,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 +213,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 +240,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 +268,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 +290,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 +312,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 +336,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 +360,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 +386,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 +424,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 +452,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 +495,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 +508,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 dc2291ef14e..43dca68acfd 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -3601,5 +3601,63 @@ privatefiles,moodle|/user/files.php'; upgrade_main_savepoint(true, 2023091300.03); } + if ($oldversion < 2023100400.01) { + // 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.01); + } + 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 60d1ff24d0b..8c08a7609d6 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 b7fb1178229..eb8647ce79f 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -4363,7 +4363,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 74339f2f0d0..3368b1aa6f3 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2023100400.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2023100400.01; // 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