L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-79573 communication: Support new records for each provider type

Browse Source 
Previously, we replaced information in the same communication record
in the database, now there will be a unique record per provider.
This ensures when switching between different providers, the correct
actions such as room member management can be actioned for the correct
provider.

This also includes the required updates for the instances to no
longer assume a provider must be enabled to be relevant. Specifying a
provider is optional to fetch a specific provider instance, the default
will use the currently enabled provider.
This commit is contained in:
Michael Hawkins
2023-10-06 09:54:32 +08:00
parent f9d75f083f
commit 329a9e97d2
8 changed files with 258 additions and 158 deletions
Download Patch File Download Diff File Expand all files Collapse all files
communication/classes/api.php
+107 -113
View File
@@ -56,6 +56,7 @@ class api {
* @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
* @param string|null $provider The provider type - if null will load for this context's active provider.
*
*/
private function __construct(
@@ -63,12 +64,14 @@ class api {
private string $component,
private string $instancetype,
private int $instanceid,
private ?string $provider = null,
) {
$this->communication = processor::load_by_instance(
context: $context,
component: $component,
instancetype: $instancetype,
instanceid: $instanceid,
provider: $provider,
);
}
@@ -79,6 +82,7 @@ class api {
* @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
* @param string|null $provider The provider type - if null will load for this context's active provider.
* @return api
*/
public static function load_by_instance(
@@ -86,12 +90,14 @@ class api {
string $component,
string $instancetype,
int $instanceid,
?string $provider = null,
): self {
return new self(
context: $context,
component: $component,
instancetype: $instancetype,
instanceid: $instanceid,
provider: $provider,
);
}
@@ -104,6 +110,7 @@ class api {
component: $this->component,
instancetype: $this->instancetype,
instanceid: $this->instanceid,
provider: $this->provider,
);
}
@@ -246,31 +253,33 @@ class api {
* @param string $provider The provider name
*/
public function form_definition_for_provider(\MoodleQuickForm $mform, string $provider = processor::PROVIDER_NONE): void {
if ($provider !== processor::PROVIDER_NONE) {
// Room name for the communication provider.
$mform->insertElementBefore(
$mform->createElement(
'text',
'communicationroomname',
get_string('communicationroomname', 'communication'),
'maxlength="100" size="20"'
),
'addcommunicationoptionshere'
);
$mform->setType('communicationroomname', PARAM_TEXT);
$mform->insertElementBefore(
$mform->createElement(
'static',
'communicationroomnameinfo',
'',
get_string('communicationroomnameinfo', 'communication'),
),
'addcommunicationoptionshere',
);
processor::set_provider_specific_form_definition($provider, $mform);
if ($provider === processor::PROVIDER_NONE) {
return;
}
// Room name for the communication provider.
$mform->insertElementBefore(
$mform->createElement(
'text',
'communicationroomname',
get_string('communicationroomname', 'communication'),
'maxlength="100" size="20"'
),
'addcommunicationoptionshere'
);
$mform->setType('communicationroomname', PARAM_TEXT);
$mform->insertElementBefore(
$mform->createElement(
'static',
'communicationroomnameinfo',
'',
get_string('communicationroomnameinfo', 'communication'),
),
'addcommunicationoptionshere',
);
processor::set_provider_specific_form_definition($provider, $mform);
}
/**
@@ -396,120 +405,105 @@ class api {
* Create a communication ad-hoc task for create operation.
* This method will add a task to the queue to create the room.
*
* @param string $selectedcommunication The selected communication provider
* @param string $communicationroomname The communication room name
* @param null|\stored_file $avatar The stored file for the avatar
* @param \stdClass|null $instance The actual instance object
*/
public function create_and_configure_room(
string $selectedcommunication,
string $communicationroomname,
?\stored_file $avatar = null,
?\stdClass $instance = null,
): void {
if ($selectedcommunication !== processor::PROVIDER_NONE && $selectedcommunication !== '') {
// Create communication record.
$this->communication = processor::create_instance(
context: $this->context,
provider: $selectedcommunication,
instanceid: $this->instanceid,
component: $this->component,
instancetype: $this->instancetype,
roomname: $communicationroomname,
);
// Update provider record from form data.
if ($instance !== null) {
$this->communication->get_form_provider()->save_form_data($instance);
}
// Set the avatar.
if (!empty($avatar)) {
$this->set_avatar($avatar);
}
// Add ad-hoc task to create the provider room.
create_and_configure_room_task::queue(
$this->communication,
);
if ($this->provider === processor::PROVIDER_NONE || $this->provider === '') {
return;
}
// Create communication record.
$this->communication = processor::create_instance(
context: $this->context,
provider: $this->provider,
instanceid: $this->instanceid,
component: $this->component,
instancetype: $this->instancetype,
roomname: $communicationroomname,
);
// Update provider record from form data.
if ($instance !== null) {
$this->communication->get_form_provider()->save_form_data($instance);
}
// Set the avatar.
if (!empty($avatar)) {
$this->set_avatar($avatar);
}
// Add ad-hoc task to create the provider room.
create_and_configure_room_task::queue(
$this->communication,
);
}
/**
* Create a communication ad-hoc task for update operation.
* This method will add a task to the queue to update the room.
*
* @param string $selectedprovider The selected communication provider
* @param string $communicationroomname The communication room name
* @param null|int $active The selected active state of the provider
* @param null|string $communicationroomname The communication room name
* @param null|\stored_file $avatar The stored file for the avatar
* @param \stdClass|null $instance The actual instance object
*/
public function update_room(
?string $selectedprovider = null,
?int $active = null,
?string $communicationroomname = null,
?\stored_file $avatar = null,
?\stdClass $instance = null,
): void {
// Existing object found, let's update the communication record and associated actions.
if ($this->communication !== null) {
// Get the previous data to compare for update.
$previousprovider = $this->communication->get_provider();
if ($previousprovider === $selectedprovider) {
// If the provider is the same, unset it.
$selectedprovider = null;
}
$previousroomname = $this->communication->get_room_name();
if ($previousroomname === $communicationroomname) {
// If the room name is the same, we don't need to update the room.
$communicationroomname = null;
}
if ($selectedprovider !== null || $communicationroomname !== null) {
// Something to update. Update communication record.
$this->communication->update_instance(
provider: $selectedprovider,
roomname: $communicationroomname,
);
}
// Reload so the currently selected provider is used.
$this->reload();
// Update provider record from form data.
if ($instance !== null) {
$this->communication->get_form_provider()->save_form_data($instance);
}
// Update the avatar.
// If the value is `null`, then unset the avatar.
$this->set_avatar($avatar);
// If the provider is none, we don't need to do anything from room point of view.
if ($this->communication->get_provider() === processor::PROVIDER_NONE) {
return;
}
// Add ad-hoc task to update the provider room if the room name changed.
// TODO add efficiency considering dynamic fields.
if (
$previousprovider === $selectedprovider
) {
update_room_task::queue(
$this->communication,
);
} else if (
$previousprovider !== $selectedprovider
) {
// Add ad-hoc task to create the provider room.
create_and_configure_room_task::queue(
$this->communication,
);
}
} else {
// The instance didn't have any communication record, so create one.
$this->create_and_configure_room($selectedprovider, $communicationroomname, $avatar, $instance);
// If the provider is none, we don't need to do anything from room point of view.
if ($this->communication->get_provider() === processor::PROVIDER_NONE) {
return;
}
$roomnamechange = null;
$activestatuschange = null;
// Check if the room name is being changed.
if (
$communicationroomname !== null &&
$communicationroomname !== $this->communication->get_room_name()
) {
$roomnamechange = $communicationroomname;
}
// Check if the active status of the provider is being changed.
if (
$active !== null &&
$active !== $this->communication->is_instance_active()
) {
$activestatuschange = $active;
}
if ($roomnamechange !== null || $activestatuschange !== null) {
$this->communication->update_instance(
active: $active,
roomname: $communicationroomname,
);
}
// Update provider record from form data.
if ($instance !== null) {
$this->communication->get_form_provider()->save_form_data($instance);
}
// Update the avatar.
// If the value is `null`, then unset the avatar.
$this->set_avatar($avatar);
// Always queue a room update, even if none of the above standard fields have changed.
// It is possible for providers to have custom fields that have been updated.
update_room_task::queue(
$this->communication,
);
}
/**
communication/classes/processor.php
+20 -17
View File
@@ -109,22 +109,17 @@ class processor {
/**
* Update the communication instance with any changes.
*
* @param null|string $provider The communication provider
* @param null|int $active Active state of the instance (processor::PROVIDER_ACTIVE or processor::PROVIDER_INACTIVE)
* @param null|string $roomname The room name
*/
public function update_instance(
?string $provider = null,
?string $active = null,
?string $roomname = null,
): void {
global $DB;
if ($provider !== null) {
if ($provider === self::PROVIDER_NONE) {
$this->instancedata->active = self::PROVIDER_INACTIVE;
} else {
$this->instancedata->provider = $provider;
$this->instancedata->active = self::PROVIDER_ACTIVE;
}
if ($active !== null && in_array($active, [self::PROVIDER_ACTIVE, self::PROVIDER_INACTIVE])) {
$this->instancedata->active = $active;
}
if ($roomname !== null) {
@@ -365,24 +360,35 @@ class processor {
* @param string $component The component name
* @param string $instancetype The instance type
* @param int $instanceid The instance id
* @param string|null $provider The provider type - if null will load for this context's active provider.
* @return processor|null
*/
public static function load_by_instance(
context $context,
string $component,
string $instancetype,
int $instanceid
int $instanceid,
?string $provider = null,
): ?self {
global $DB;
$record = $DB->get_record('communication', [
$params = [
'contextid' => $context->id,
'instanceid' => $instanceid,
'component' => $component,
'instancetype' => $instancetype,
]);
];
if ($provider === null) {
// Fetch the active provider in this context.
$params['active'] = 1;
} else {
// Fetch a specific provider in this context (which may be inactive).
$params['provider'] = $provider;
}
$record = $DB->get_record('communication', $params);
if ($record && self::is_provider_available($record->provider)) {
return new self($record);
}
@@ -464,15 +470,12 @@ class processor {
}
/**
* Get communication provider.
* Get communication provider type.
*
* @return string|null
*/
public function get_provider(): ?string {
if ((int)$this->instancedata->active === self::PROVIDER_ACTIVE) {
return $this->instancedata->provider;
}
return self::PROVIDER_NONE;
return $this->instancedata->provider;
}
/**
communication/configure.php
+1
View File
@@ -52,6 +52,7 @@ $communication = \core_communication\api::load_by_instance(
component: $component,
instancetype: $instancetype,
instanceid: $instanceid,
provider: $selectedcommunication,
);
// No communication, no way this form can be used.
communication/provider/matrix/tests/communication_feature_test.php
+2 -2
View File
@@ -60,10 +60,10 @@ class communication_feature_test extends \advanced_testcase {
component: 'communication_matrix',
instancetype: 'example',
instanceid: 1,
provider: 'communication_matrix',
);
$communication->create_and_configure_room(
selectedcommunication: 'communication_matrix',
communicationroomname: 'Room name',
instance: (object) [
'matrixroomtopic' => 'A fun topic',
@@ -526,10 +526,10 @@ class communication_feature_test extends \advanced_testcase {
component: $component,
instancetype: $itemtype,
instanceid: $itemid,
provider: 'communication_matrix',
);
$communication->create_and_configure_room(
selectedcommunication: 'communication_matrix',
communicationroomname: $roomname ?? 'Room name',
avatar: $roomavatar,
instance: (object) [
communication/provider/matrix/tests/matrix_test_helper_trait.php
+2 -2
View File
@@ -253,7 +253,7 @@ trait matrix_test_helper_trait {
* @return api
*/
protected function create_matrix_room(
?string $component = 'communication_matrix',
?string $component = 'core_course',
?string $itemtype = 'example',
?int $itemid = 1,
?string $roomname = null,
@@ -269,10 +269,10 @@ trait matrix_test_helper_trait {
component: $component,
instancetype: $itemtype,
instanceid: $itemid,
provider: 'communication_matrix',
);
$communication->create_and_configure_room(
selectedcommunication: 'communication_matrix',
communicationroomname: $roomname ?? 'Room name',
avatar: $roomavatar,
instance: (object) [
communication/tests/api_test.php
+21 -3
View File
@@ -118,9 +118,10 @@ class api_test extends \advanced_testcase {
component: 'core_course',
instancetype: 'coursecommunication',
instanceid: $course->id,
provider: $selectedcommunication,
);
$communication->create_and_configure_room($selectedcommunication, $communicationroomname, $avatar);
$communication->create_and_configure_room($communicationroomname, $avatar);
// Reload the communication processor.
$communicationprocessor = processor::load_by_instance(
@@ -153,8 +154,9 @@ class api_test extends \advanced_testcase {
component: 'core_course',
instancetype: 'coursecommunication',
instanceid: $course->id,
provider: $selectedcommunication,
);
$communication->create_and_configure_room($selectedcommunication, $communicationroomname);
$communication->create_and_configure_room($communicationroomname);
// Test the tasks added.
$adhoctask = \core\task\manager::get_adhoc_tasks('\\core_communication\\task\\create_and_configure_room_task');
@@ -213,7 +215,7 @@ class api_test extends \advanced_testcase {
instancetype: 'coursecommunication',
instanceid: $course->id,
);
$communication->update_room($selectedcommunication, $communicationroomname);
$communication->update_room(processor::PROVIDER_ACTIVE, $communicationroomname);
// Test the communication record exists.
$communicationprocessor = processor::load_by_instance(
@@ -225,6 +227,22 @@ class api_test extends \advanced_testcase {
$this->assertEquals($communicationroomname, $communicationprocessor->get_room_name());
$this->assertEquals($selectedcommunication, $communicationprocessor->get_provider());
$this->assertTrue($communicationprocessor->is_instance_active());
$communication->update_room(processor::PROVIDER_INACTIVE, $communicationroomname);
// Test updating active state.
$communicationprocessor = processor::load_by_instance(
context: \core\context\course::instance($course->id),
component: 'core_course',
instancetype: 'coursecommunication',
instanceid: $course->id,
provider: $selectedcommunication,
);
$this->assertEquals($communicationroomname, $communicationprocessor->get_room_name());
$this->assertEquals($selectedcommunication, $communicationprocessor->get_provider());
$this->assertFalse($communicationprocessor->is_instance_active());
}
/**
communication/tests/processor_test.php
+29 -4
View File
@@ -127,7 +127,7 @@ class processor_test extends \advanced_testcase {
$selectedcommunication = 'none';
$communicationroomname = 'communicationroomedited';
$communicationprocessor->update_instance($selectedcommunication, $communicationroomname);
$communicationprocessor->update_instance(processor::PROVIDER_INACTIVE, $communicationroomname);
// Now test the record against the database.
$communicationrecord = $DB->get_record('communication', [
@@ -291,7 +291,7 @@ class processor_test extends \advanced_testcase {
$course = $this->get_course();
$context = \core\context\course::instance($course->id);
// Test the communication record exists.
// Test the communication record exists when fetching the active provider.
$communicationprocessor = processor::load_by_instance(
context: $context,
component: 'core_course',
@@ -300,6 +300,29 @@ class processor_test extends \advanced_testcase {
);
$this->assertEquals('communication_matrix', $communicationprocessor->get_provider());
// Test the communication record exists when specifying the provider.
$communicationprocessor = processor::load_by_instance(
context: $context,
component: 'core_course',
instancetype: 'coursecommunication',
instanceid: $course->id,
provider: 'communication_matrix',
);
$this->assertEquals('communication_matrix', $communicationprocessor->get_provider());
// Test the communication record exists when the provider is not active.
$communicationprocessor->update_instance(processor::PROVIDER_INACTIVE);
$communicationprocessor = processor::load_by_instance(
context: $context,
component: 'core_course',
instancetype: 'coursecommunication',
instanceid: $course->id,
provider: 'communication_matrix',
);
$this->assertEquals('communication_matrix', $communicationprocessor->get_provider());
}
/**
@@ -427,8 +450,9 @@ class processor_test extends \advanced_testcase {
component: 'core_course',
instancetype: 'coursecommunication',
instanceid: $course->id,
provider: $selectedcommunication,
);
$communication->create_and_configure_room($selectedcommunication, $communicationroomname, $avatar);
$communication->create_and_configure_room($communicationroomname, $avatar);
$communicationprocessor = processor::load_by_instance(
context: \core\context\course::instance($course->id),
@@ -498,8 +522,9 @@ class processor_test extends \advanced_testcase {
component: $component,
instancetype: $instancetype,
instanceid: $course->id,
provider: $selectedcommunication,
);
$communication->create_and_configure_room($selectedcommunication, $communicationroomname);
$communication->create_and_configure_room($communicationroomname);
$communication->add_members_to_room([$user1, $user2]);
// Now remove user1 from the room.
course/lib.php
+76 -17
View File
@@ -2292,9 +2292,9 @@ function create_course($data, $editoroptions = NULL) {
component: 'core_course',
instancetype: 'coursecommunication',
instanceid: $course->id,
provider: $provider,
);
$communication->create_and_configure_room(
$provider,
$communicationroomname,
$courseimage ?: null,
$data,
@@ -2461,38 +2461,97 @@ function update_course($data, $editoroptions = NULL) {
$enrolledusers[] = $user->id;
}
// Existing communication provider.
$communication = \core_communication\api::load_by_instance(
context: $context,
component: 'core_course',
instancetype: 'coursecommunication',
instanceid: $data->id,
);
$existingprovider = $communication->get_provider();
$addusersrequired = false;
$enablenewprovider = false;
$instanceexists = true;
$addafterupdate = false;
if ($provider !== $communication->get_provider()) {
// If provider set to none, remove all the members.
if ($provider === 'none') {
// Action required changes if provider has changed.
if ($provider !== $existingprovider) {
// Provider changed, flag new one to be enabled.
$enablenewprovider = true;
// If provider set to none, remove all the members from previous provider.
if ($provider === 'none' && $existingprovider !== '') {
$communication->remove_members_from_room($enrolledusers);
} else if (
// If previous provider was not none and current provider is not none, but a different provider, remove members.
$communication->get_provider() !== '' &&
$communication->get_provider() !== 'none' &&
$provider !== $communication->get_provider()
// If previous provider was not none and current provider is not none,
// remove members from previous provider.
$existingprovider !== '' &&
$existingprovider !== 'none'
) {
$communication->remove_members_from_room($enrolledusers);
$addafterupdate = true;
$addusersrequired = true;
} else if (
// If previous provider was none and current provider is not none, but a different provider, remove members.
($communication->get_provider() === '' || $communication->get_provider() === 'none') &&
$provider !== $communication->get_provider()
// If previous provider was none and current provider is not none,
// remove members from previous provider.
($existingprovider === '' || $existingprovider === 'none')
) {
$addafterupdate = true;
$addusersrequired = true;
}
// Disable previous provider, if one was enabled.
if ($existingprovider !== '' && $existingprovider !== 'none') {
$communication->update_room(
active: \core_communication\processor::PROVIDER_INACTIVE,
);
}
// Switch to the newly selected provider so it can be updated.
if ($provider !== 'none') {
$communication = \core_communication\api::load_by_instance(
context: $context,
component: 'core_course',
instancetype: 'coursecommunication',
instanceid: $data->id,
provider: $provider,
);
// Create it if it does not exist.
if ($communication->get_provider() === '') {
$communication->create_and_configure_room(
communicationroomname: $communicationroomname,
avatar: $courseimage,
instance: $data
);
$communication = \core_communication\api::load_by_instance(
context: $context,
component: 'core_course',
instancetype: 'coursecommunication',
instanceid: $data->id,
provider: $provider,
);
$addusersrequired = true;
$instanceexists = false;
}
}
}
$communication->update_room($provider, $communicationroomname, $courseimage, $data);
if ($addafterupdate) {
$communication->add_members_to_room($enrolledusers, false);
if ($provider !== 'none' && $instanceexists) {
// Update the currently enabled provider's room data.
// Newly created providers do not need to run this, the create process handles it.
$communication->update_room(
active: $enablenewprovider ? \core_communication\processor::PROVIDER_ACTIVE : null,
communicationroomname: $communicationroomname,
avatar: $courseimage,
instance: $data,
);
}
// Complete room membership tasks if required.
// Newly created providers complete the user mapping but do not queue the task
// (it will be handled by the room creation task).
if ($addusersrequired) {
$communication->add_members_to_room($enrolledusers, $instanceexists);
}
}