Merge pull request #5048 from nextcloud/bugfix/3462/allow-adding-self-joined

Allow converting self-joined users participants to regular participants

6 files changed, 161 insertions, 33 deletions

diff --git a/lib/Chat/SystemMessage/Listener.php b/lib/Chat/SystemMessage/Listener.php
index b06aa26aa..26601225e 100644
--- a/lib/Chat/SystemMessage/Listener.php
+++ b/lib/Chat/SystemMessage/Listener.php
@@ -274,9 +274,15 @@ class Listener {
 				$listener = \OC::$server->query(self::class);
 				$listener->sendSystemMessage($room, 'moderator_promoted', ['user' => $attendee->getActorId()]);
 			} elseif ($event->getNewValue() === Participant::USER) {
-				/** @var self $listener */
-				$listener = \OC::$server->query(self::class);
-				$listener->sendSystemMessage($room, 'moderator_demoted', ['user' => $attendee->getActorId()]);
+				if ($event->getOldValue() === Participant::USER_SELF_JOINED) {
+					/** @var self $listener */
+					$listener = \OC::$server->query(self::class);
+					$listener->sendSystemMessage($room, 'user_added', ['user' => $attendee->getActorId()]);
+				} else {
+					/** @var self $listener */
+					$listener = \OC::$server->query(self::class);
+					$listener->sendSystemMessage($room, 'moderator_demoted', ['user' => $attendee->getActorId()]);
+				}
 			} elseif ($event->getNewValue() === Participant::GUEST_MODERATOR) {
 				/** @var self $listener */
 				$listener = \OC::$server->query(self::class);
diff --git a/lib/Collaboration/Collaborators/Listener.php b/lib/Collaboration/Collaborators/Listener.php
index b5b2136ef..a697675c7 100644
--- a/lib/Collaboration/Collaborators/Listener.php
+++ b/lib/Collaboration/Collaborators/Listener.php
@@ -27,6 +27,7 @@ use OCA\Talk\Config;
 use OCA\Talk\Exceptions\ParticipantNotFoundException;
 use OCA\Talk\Exceptions\RoomNotFoundException;
 use OCA\Talk\Manager;
+use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCP\Collaboration\AutoComplete\AutoCompleteEvent;
 use OCP\Collaboration\AutoComplete\IManager;
@@ -127,7 +128,11 @@ class Listener {
 		$userId = $result['value']['shareWith'];
 
 		try {
-			$this->room->getParticipant($userId);
+			$participant = $this->room->getParticipant($userId);
+			if ($participant->getAttendee()->getParticipantType() === Participant::USER_SELF_JOINED) {
+				// do list self-joined users so they can be added as permanent participants by moderators
+				return true;
+			}
 			return false;
 		} catch (ParticipantNotFoundException $e) {
 			return true;
diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php
index 56973e318..92a5d1855 100644
--- a/lib/Controller/RoomController.php
+++ b/lib/Controller/RoomController.php
@@ -1222,23 +1222,28 @@ class RoomController extends AEnvironmentAwareController {
 			return new DataResponse([], Http::STATUS_BAD_REQUEST);
 		}
 
-		$participants = $this->participantService->getParticipantUserIds($this->room);
+		$participants = $this->participantService->getParticipantsForRoom($this->room);
+		$participantsByUserId = [];
+		foreach ($participants as $participant) {
+			if ($participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS) {
+				$participantsByUserId[$participant->getAttendee()->getActorId()] = $participant;
+			}
+		}
 
+		// list of participants to attempt adding,
+		// existing ones will be filtered later below
 		$participantsToAdd = [];
+
 		if ($source === 'users') {
 			$newUser = $this->userManager->get($newParticipant);
 			if (!$newUser instanceof IUser) {
 				return new DataResponse([], Http::STATUS_NOT_FOUND);
 			}
 
-			if (\in_array($newParticipant, $participants, true)) {
-				return new DataResponse([]);
-			}
-
-			$this->participantService->addUsers($this->room, [[
+			$participantsToAdd[] = [
 				'actorType' => Attendee::ACTOR_USERS,
 				'actorId' => $newUser->getUID(),
-			]]);
+			];
 		} elseif ($source === 'groups') {
 			$group = $this->groupManager->get($newParticipant);
 			if (!$group instanceof IGroup) {
@@ -1247,21 +1252,11 @@ class RoomController extends AEnvironmentAwareController {
 
 			$usersInGroup = $group->getUsers();
 			foreach ($usersInGroup as $user) {
-				if (\in_array($user->getUID(), $participants, true)) {
-					continue;
-				}
-
 				$participantsToAdd[] = [
 					'actorType' => Attendee::ACTOR_USERS,
 					'actorId' => $user->getUID(),
 				];
 			}
-
-			if (empty($participantsToAdd)) {
-				return new DataResponse([]);
-			}
-
-			$this->participantService->addUsers($this->room, $participantsToAdd);
 		} elseif ($source === 'circles') {
 			if (!$this->appManager->isEnabledForUser('circles')) {
 				return new DataResponse([], Http::STATUS_BAD_REQUEST);
@@ -1281,10 +1276,6 @@ class RoomController extends AEnvironmentAwareController {
 					continue;
 				}
 
-				if (\in_array($member->getUserId(), $participants, true)) {
-					continue;
-				}
-
 				if ($member->getStatus() !== Member::STATUS_INVITED && $member->getStatus() !== Member::STATUS_MEMBER) {
 					// Only allow invited and regular members
 					continue;
@@ -1295,12 +1286,6 @@ class RoomController extends AEnvironmentAwareController {
 					'actorId' => $member->getUserId(),
 				];
 			}
-
-			if (empty($participantsToAdd)) {
-				return new DataResponse([]);
-			}
-
-			$this->participantService->addUsers($this->room, $participantsToAdd);
 		} elseif ($source === 'emails') {
 			$data = [];
 			if ($this->room->setType(Room::PUBLIC_CALL)) {
@@ -1316,7 +1301,25 @@ class RoomController extends AEnvironmentAwareController {
 			return new DataResponse([], Http::STATUS_BAD_REQUEST);
 		}
 
-		return new DataResponse();
+		// attempt adding the listed users to the room
+		// existing users with USER_SELF_JOINED will get converted to regular USER participants
+		foreach ($participantsToAdd as $index => $participantToAdd) {
+			$existingParticipant = $participantsByUserId[$participantToAdd['actorId']] ?? null;
+
+			if ($existingParticipant !== null) {
+				unset($participantsToAdd[$index]);
+				if ($existingParticipant->getAttendee()->getParticipantType() !== Participant::USER_SELF_JOINED) {
+					// user is already a regular participant, skip
+					continue;
+				}
+				$this->participantService->updateParticipantType($this->room, $existingParticipant, Participant::USER);
+			}
+		}
+
+		// add the remaining users in batch
+		$this->participantService->addUsers($this->room, $participantsToAdd);
+
+		return new DataResponse([]);
 	}
 
 	/**
diff --git a/lib/Service/ParticipantService.php b/lib/Service/ParticipantService.php
index 4ad4206d6..f2267096a 100644
--- a/lib/Service/ParticipantService.php
+++ b/lib/Service/ParticipantService.php
@@ -258,6 +258,9 @@ class ParticipantService {
 	 * @param array $participants
 	 */
 	public function addUsers(Room $room, array $participants): void {
+		if (empty($participants)) {
+			return;
+		}
 		$event = new AddParticipantsEvent($room, $participants);
 		$this->dispatcher->dispatch(Room::EVENT_BEFORE_USERS_ADD, $event);
 
diff --git a/tests/integration/features/conversation/add-participant.feature b/tests/integration/features/conversation/add-participant.feature
index 01be02975..7c83bc869 100644
--- a/tests/integration/features/conversation/add-participant.feature
+++ b/tests/integration/features/conversation/add-participant.feature
@@ -62,6 +62,34 @@ Feature: public
       | id   | type | participantType | participants |
       | room | 3    | 3               | participant1-displayname, participant2-displayname, participant3-displayname |
 
+  Scenario: Moderator invites a user who self-joined
+    Given user "participant1" creates room "room"
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant2" joins room "room" with 200
+    When user "participant1" adds "participant2" to room "room" with 200
+    Then user "participant2" is participant of the following rooms
+      | id   | type | participantType | participants |
+      | room | 3    | 3               | participant1-displayname, participant2-displayname |
+
+  Scenario: Moderator invites a group containing a self-joined user
+    Given group "group1" exists
+    And user "participant2" is member of group "group1"
+    And user "participant3" is member of group "group1"
+    And user "participant1" creates room "room"
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant2" joins room "room" with 200
+    # participant3 already present, so it will be skipped
+    And user "participant1" adds "participant3" to room "room" with 200
+    When user "participant1" adds group "group1" to room "room" with 200
+    Then user "participant2" is participant of the following rooms
+      | id   | type | participantType | participants |
+      | room | 3    | 3               | participant1-displayname, participant2-displayname, participant3-displayname |
+    And user "participant3" is participant of the following rooms
+      | id   | type | participantType | participants |
+      | room | 3    | 3               | participant1-displayname, participant2-displayname, participant3-displayname |
+
   Scenario: Stranger invites a user
     Given user "participant1" creates room "room"
       | roomType | 3 |
diff --git a/tests/php/Chat/SystemMessage/ListenerTest.php b/tests/php/Chat/SystemMessage/ListenerTest.php
index 754914bbc..7b83b5e2a 100644
--- a/tests/php/Chat/SystemMessage/ListenerTest.php
+++ b/tests/php/Chat/SystemMessage/ListenerTest.php
@@ -24,6 +24,8 @@ namespace OCA\Talk\Tests\php\Chat\SystemMessage;
 use OCA\Talk\Chat\ChatManager;
 use OCA\Talk\Chat\SystemMessage\Listener;
 use OCA\Talk\Events\AddParticipantsEvent;
+use OCA\Talk\Events\ModifyParticipantEvent;
+use OCA\Talk\Model\Attendee;
 use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\TalkSession;
@@ -107,7 +109,7 @@ class ListenerTest extends TestCase {
 	}
 
 	private function dispatch($eventName, $event) {
-		$handlers = $this->handlers[Room::EVENT_AFTER_USERS_ADD];
+		$handlers = $this->handlers[$eventName];
 		$this->assertCount(1, $handlers);
 
 		$handlers[0]($event);
@@ -226,4 +228,85 @@ class ListenerTest extends TestCase {
 
 		$this->dispatch(Room::EVENT_AFTER_USERS_ADD, $event);
 	}
+
+	public function participantTypeChangeProvider() {
+		return [
+			[
+				Attendee::ACTOR_EMAILS,
+				Participant::USER,
+				Participant::MODERATOR,
+				[],
+			],
+			[
+				Attendee::ACTOR_USERS,
+				Participant::USER,
+				Participant::MODERATOR,
+				[['message' => 'moderator_promoted', 'parameters' => ['user' => 'bob_participant']]],
+			],
+			[
+				Attendee::ACTOR_USERS,
+				Participant::MODERATOR,
+				Participant::USER,
+				[['message' => 'moderator_demoted', 'parameters' => ['user' => 'bob_participant']]],
+			],
+			[
+				Attendee::ACTOR_GUESTS,
+				Participant::GUEST,
+				Participant::GUEST_MODERATOR,
+				[['message' => 'guest_moderator_promoted', 'parameters' => ['session' => 'bob_participant']]],
+			],
+			[
+				Attendee::ACTOR_GUESTS,
+				Participant::GUEST_MODERATOR,
+				Participant::GUEST,
+				[['message' => 'guest_moderator_demoted', 'parameters' => ['session' => 'bob_participant']]],
+			],
+			[
+				Attendee::ACTOR_USERS,
+				Participant::USER_SELF_JOINED,
+				Participant::USER,
+				[['message' => 'user_added', 'parameters' => ['user' => 'bob_participant']]],
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider participantTypeChangeProvider
+	 *
+	 * @param int $actorType
+	 * @param int $oldParticipantType
+	 * @param int $newParticipantType
+	 * @param array $expectedMessages
+	 */
+	public function testAfterParticipantTypeSet(string $actorType, int $oldParticipantType, int $newParticipantType, array $expectedMessages): void {
+		$this->mockLoggedInUser('alice_actor');
+
+		$room = $this->createMock(Room::class);
+		$room->method('getType')->willReturn(Room::GROUP_CALL);
+
+		$attendee = new Attendee();
+		$attendee->setActorId('bob_participant');
+		$attendee->setActorType($actorType);
+
+		$participant = $this->createMock(Participant::class);
+		$participant->method('getAttendee')->willReturn($attendee);
+
+		$event = new ModifyParticipantEvent($room, $participant, '', $newParticipantType, $oldParticipantType);
+
+		foreach ($expectedMessages as $index => $expectedMessage) {
+			$this->chatManager->expects($this->at($index))
+				->method('addSystemMessage')
+				->with(
+					$room,
+					Attendee::ACTOR_USERS,
+					'alice_actor',
+					json_encode($expectedMessage),
+					$this->dummyTime,
+					false,
+					self::DUMMY_REFERENCE_ID,
+				);
+		}
+
+		$this->dispatch(Room::EVENT_AFTER_PARTICIPANT_TYPE_SET, $event);
+	}
 }