From bfbb314e2f7af692da672d9c1396b25ed7371280 Mon Sep 17 00:00:00 2001
From: Michael Hawkins <michaelh@moodle.com>
Date: Thu, 24 Aug 2023 20:04:49 +0800
Subject: [PATCH] MDL-78619 communication: De-couple room & user management and
 API fix

Management fix: Although rooms and room users are implemented as
separate provider interfaces, there were still assumptions that
adding/removing rooms would require user handling. Where room_provider
was implemented but room_user_provider was not, that would then result
in either unnecessary ad-hoc tasks, or tasks failing with errors. Now,
checks are in place to avoid these scenarios.

API fix: Previously save_form_data() was being called on the old
provider instead of the newly enabled one, so when switching between
providers the settings would not be correctly applied and the provider
may not function. Now, the object is reloaded and the data is saved correctly.
---
 communication/classes/api.php                     | 15 ++++++++++-----
 .../task/create_and_configure_room_task.php       |  5 ++---
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/communication/classes/api.php b/communication/classes/api.php
index b9840cd1992..db3f454bec7 100644
--- a/communication/classes/api.php
+++ b/communication/classes/api.php
@@ -451,6 +451,9 @@ class api {
                 );
             }
 
+            // 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);
@@ -514,8 +517,8 @@ class api {
             return;
         }
 
-        // No userids? don't bother doing anything.
-        if (empty($userids)) {
+        // No user IDs or this provider does not manage users? No action required.
+        if (empty($userids) || !$this->communication->supports_user_features()) {
             return;
         }
 
@@ -542,12 +545,14 @@ class api {
             return;
         }
 
-        if ($this->communication->get_provider() === processor::PROVIDER_NONE) {
+        $provider = $this->communication->get_provider();
+
+        if ($provider === processor::PROVIDER_NONE) {
             return;
         }
 
-        // No user ids? don't bother doing anything.
-        if (empty($userids)) {
+        // No user IDs or this provider does not manage users? No action required.
+        if (empty($userids) || !$this->communication->supports_user_features()) {
             return;
         }
 
diff --git a/communication/classes/task/create_and_configure_room_task.php b/communication/classes/task/create_and_configure_room_task.php
index 1327e4e723d..a65dfee5d46 100644
--- a/communication/classes/task/create_and_configure_room_task.php
+++ b/communication/classes/task/create_and_configure_room_task.php
@@ -46,13 +46,12 @@ class create_and_configure_room_task extends adhoc_task {
             return;
         }
 
-        // If the room is created successfully, add members to the room.
-        if ($communication->get_room_provider()->create_chat_room()) {
+        // If the room is created successfully, add members to the room if supported by the provider.
+        if ($communication->get_room_provider()->create_chat_room() && $communication->supports_user_features()) {
             add_members_to_room_task::queue(
                 $communication
             );
         }
-
     }
 
     /**