Allow to apply a new attendee map later on

Signed-off-by: Joas Schilling <coding@schilljs.com>

appinfo/routes/routesBreakoutRoomController.php

@@ -36,6 +36,8 @@ return [
 		['name' => 'BreakoutRoom#removeBreakoutRooms', 'url' => '/api/{apiVersion}/breakout-rooms/{token}', 'verb' => 'DELETE', 'requirements' => $requirements],
 		/** @see \OCA\Talk\Controller\BreakoutRoomController::broadcastChatMessage() */
 		['name' => 'BreakoutRoom#broadcastChatMessage', 'url' => '/api/{apiVersion}/breakout-rooms/{token}/broadcast', 'verb' => 'POST', 'requirements' => $requirements],
+		/** @see \OCA\Talk\Controller\BreakoutRoomController::applyAttendeeMap() */
+		['name' => 'BreakoutRoom#applyAttendeeMap', 'url' => '/api/{apiVersion}/breakout-rooms/{token}/attendees', 'verb' => 'POST', 'requirements' => $requirements],
 		/** @see \OCA\Talk\Controller\BreakoutRoomController::requestAssistance() */
 		['name' => 'BreakoutRoom#requestAssistance', 'url' => '/api/{apiVersion}/breakout-rooms/{token}/request-assistance', 'verb' => 'POST', 'requirements' => $requirements],
 		/** @see \OCA\Talk\Controller\BreakoutRoomController::resetRequestForAssistance() */

docs/breakout-rooms.md

@@ -32,7 +32,7 @@ Group and public conversations can be used to host breakout rooms.
         + `400 Bad Request` When the conversation is a breakout room itself
         + `400 Bad Request` When the mode is invalid
         + `400 Bad Request` When the amount is below the minimum or above the maximum
-        + `400 Bad Request` When the attendee map contains an invalid room number
+        + `400 Bad Request` When the attendee map contains an invalid room number or moderator
         + `403 Forbidden` When the current user is not a moderator/owner
         + `404 Not Found` When the conversation could not be found for the participant
 
@@ -94,6 +94,26 @@ Group and public conversations can be used to host breakout rooms.
 		+ `404 Not Found` When the conversation could not be found for the participant
 		+ `413 Payload Too Large` When the message was longer than the allowed limit of 32000 characters (check the `spreed => config => chat => max-length` capability for the limit)
 
+## Configure breakout rooms
+
+* Required capability: `breakout-rooms-v1`
+* Method: `POST`
+* Endpoint: `/breakout-rooms/{token}/attendees`
+* Data:
+
+| field         | type   | Description                                                                                          |
+|---------------|--------|------------------------------------------------------------------------------------------------------|
+| `attendeeMap` | string | A json encoded Map of attendeeId => room number (0 based) (Only considered when the mode is "manual" |
+
+* Response:
+	- Status code:
+		+ `200 OK`
+		+ `400 Bad Request` When breakout rooms are disabled on the server
+		+ `400 Bad Request` When breakout rooms are not configured
+		+ `400 Bad Request` When the attendee map contains an invalid room number or moderator
+		+ `403 Forbidden` When the current user is not a moderator/owner
+		+ `404 Not Found` When the conversation could not be found for the participant
+
 ## Request assistance
 
 This endpoint allows participants to raise their hand (token is the breakout room) and moderators will see it in any of the breakout rooms as well as the parent room.

lib/Controller/BreakoutRoomController.php

@@ -86,6 +86,19 @@ class BreakoutRoomController extends AEnvironmentAwareController {
 		return new DataResponse([], Http::STATUS_CREATED);
 	}
 
+	/**
+	 * @NoAdminRequired
+	 * @RequireLoggedInModeratorParticipant
+	 */
+	public function applyAttendeeMap(string $attendeeMap): DataResponse {
+		try {
+			$this->breakoutRoomService->applyAttendeeMap($this->room, $attendeeMap);
+		} catch (InvalidArgumentException $e) {
+			return new DataResponse(['error' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
+		}
+		return new DataResponse([], Http::STATUS_OK);
+	}
+
 	/**
 	 * @NoAdminRequired
 	 * @RequireLoggedInParticipant

lib/Service/BreakoutRoomService.php

@@ -67,6 +67,55 @@ class BreakoutRoomService {
 		$this->l = $l;
 	}
 
+	/**
+	 * @param string $map
+	 * @param int $max
+	 * @return array
+	 */
+	protected function parseAttendeeMap(string $map, int $max): array {
+		if ($map === '') {
+			return [];
+		}
+
+		try {
+			$attendeeMap = json_decode($map, true, 2, JSON_THROW_ON_ERROR);
+		} catch (\JsonException) {
+			throw new InvalidArgumentException('attendeeMap');
+		}
+
+		if (!is_array($attendeeMap)) {
+			throw new InvalidArgumentException('attendeeMap');
+		}
+
+		if (empty($attendeeMap)) {
+			return [];
+		}
+
+		try {
+			$attendeeMap = array_filter($attendeeMap, static fn (int $roomNumber, int $attendeeId) => true, ARRAY_FILTER_USE_BOTH);
+		} catch (\Throwable) {
+			throw new InvalidArgumentException('attendeeMap');
+		}
+
+		if (empty($attendeeMap)) {
+			return [];
+		}
+
+		if (max($attendeeMap) >= $max) {
+			throw new InvalidArgumentException('attendeeMap');
+		}
+
+		if (min($attendeeMap) < 0) {
+			throw new InvalidArgumentException('attendeeMap');
+		}
+
+		if (min(array_keys($attendeeMap)) <= 0) {
+			throw new InvalidArgumentException('attendeeMap');
+		}
+
+		return $attendeeMap;
+	}
+
 	/**
 	 * @param Room $parent
 	 * @param int $mode
@@ -108,21 +157,7 @@ class BreakoutRoomService {
 		}
 
 		if ($mode === BreakoutRoom::MODE_MANUAL) {
-			try {
-				$attendeeMap = json_decode($attendeeMap, true, 2, JSON_THROW_ON_ERROR);
-			} catch (\JsonException $e) {
-				throw new InvalidArgumentException('attendeeMap');
-			}
-
-			if (!empty($attendeeMap)) {
-				if (max($attendeeMap) >= $amount) {
-					throw new InvalidArgumentException('attendeeMap');
-				}
-
-				if (min($attendeeMap) < 0) {
-					throw new InvalidArgumentException('attendeeMap');
-				}
-			}
+			$cleanedMap = $this->parseAttendeeMap($attendeeMap, $amount);
 		}
 
 		$breakoutRooms = $this->createBreakoutRooms($parent, $amount);
@@ -149,12 +184,11 @@ class BreakoutRoomService {
 		} elseif ($mode === BreakoutRoom::MODE_MANUAL) {
 			$map = [];
 			foreach ($others as $participant) {
-				$roomNumber = $attendeeMap[$participant->getAttendee()->getId()] ?? null;
-				if ($roomNumber === null) {
+				if (!isset($cleanedMap[$participant->getAttendee()->getId()])) {
 					continue;
 				}
 
-				$roomNumber = (int) $roomNumber;
+				$roomNumber = (int) $cleanedMap[$participant->getAttendee()->getId()];
 
 				$map[$roomNumber] ??= [];
 				$map[$roomNumber][] = $participant;
@@ -167,6 +201,72 @@ class BreakoutRoomService {
 		return $breakoutRooms;
 	}
 
+	/**
+	 * @param Room $parent
+	 * @param string $attendeeMap
+	 * @throws InvalidArgumentException When the map was invalid
+	 */
+	public function applyAttendeeMap(Room $parent, string $attendeeMap): void {
+		if (!$this->config->isBreakoutRoomsEnabled()) {
+			throw new InvalidArgumentException('config');
+		}
+
+		if ($parent->getBreakoutRoomMode() === BreakoutRoom::MODE_NOT_CONFIGURED) {
+			throw new InvalidArgumentException('mode');
+		}
+
+		$breakoutRooms = $this->manager->getMultipleRoomsByObject(BreakoutRoom::PARENT_OBJECT_TYPE, $parent->getToken());
+		$amount = count($breakoutRooms);
+
+		$cleanedMap = $this->parseAttendeeMap($attendeeMap, $amount);
+		$attendeeIds = array_keys($cleanedMap);
+
+		$participants = $this->participantService->getParticipantsForRoom($parent);
+		$participants = array_filter($participants, static fn (Participant $participant) => in_array($participant->getAttendee()->getId(), $attendeeIds, true));
+		// TODO Removing any non-users here as breakout rooms only support logged in users in version 1
+		$participants = array_filter($participants, static fn (Participant $participant) => $participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS);
+
+		$userIds = array_map(static fn (Participant $participant) => $participant->getAttendee()->getActorId(), $participants);
+
+		$removals = [];
+		foreach ($breakoutRooms as $breakoutRoom) {
+			$breakoutRoomParticipants = $this->participantService->getParticipantsForRoom($breakoutRoom);
+
+			foreach ($breakoutRoomParticipants as $participant) {
+				$attendee = $participant->getAttendee();
+				if ($attendee->getActorType() === Attendee::ACTOR_USERS && in_array($attendee->getActorId(), $userIds, true)) {
+					if ($participant->hasModeratorPermissions()) {
+						// Can not remove moderators with this method
+						throw new \InvalidArgumentException('moderator');
+					}
+
+					$removals[] = [
+						'room' => $breakoutRoom,
+						'participant' => $participant,
+					];
+				}
+			}
+		}
+
+		foreach ($removals as $removal) {
+			$this->participantService->removeAttendee($removal['room'], $removal['participant'], Room::PARTICIPANT_REMOVED);
+		}
+
+		$map = [];
+		foreach ($participants as $participant) {
+			if (!isset($cleanedMap[$participant->getAttendee()->getId()])) {
+				continue;
+			}
+
+			$roomNumber = (int) $cleanedMap[$participant->getAttendee()->getId()];
+
+			$map[$roomNumber] ??= [];
+			$map[$roomNumber][] = $participant;
+		}
+
+		$this->addOthersToBreakoutRooms($breakoutRooms, $map);
+	}
+
 	/**
 	 * @param Room[] $rooms
 	 * @param Participant[] $moderators

tests/integration/features/bootstrap/FeatureContext.php

@@ -2419,6 +2419,32 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 		$this->assertStatusCode($this->response, $status);
 	}
 
+	/**
+	 * @Then /^user "([^"]*)" moves participants into different breakout rooms for "([^"]*)" with (\d+) \((v1)\)$/
+	 *
+	 * @param string $user
+	 * @param string $identifier
+	 * @param int $status
+	 * @param string $apiVersion
+	 * @param TableNode|null $formData
+	 */
+	public function userMovesParticipantsInsideBreakoutRooms(string $user, string $identifier, int $status, string $apiVersion, TableNode $formData = null): void {
+		$data = [];
+		if ($formData instanceof TableNode) {
+			$mapArray = [];
+			foreach ($formData->getRowsHash() as $attendee => $roomNumber) {
+				[$type, $id] = explode('::', $attendee);
+				$attendeeId = $this->getAttendeeId($type, $id, $identifier);
+				$mapArray[$attendeeId] = (int) $roomNumber;
+			}
+			$data['attendeeMap'] = json_encode($mapArray, JSON_THROW_ON_ERROR);
+		}
+
+		$this->setCurrentUser($user);
+		$this->sendRequest('POST', '/apps/spreed/api/' . $apiVersion . '/breakout-rooms/' . self::$identifierToToken[$identifier] . '/attendees', $data);
+		$this->assertStatusCode($this->response, $status);
+	}
+
 	/**
 	 * @Then /^user "([^"]*)" broadcasts message "([^"]*)" to room "([^"]*)" with (\d+)(?: \((v1)\))?$/
 	 *

tests/integration/features/conversation/breakout-rooms.feature

@@ -701,3 +701,56 @@ Feature: conversation/breakout-rooms
     When user "participant1" adds user "participant2" to room "Room 2" with 400 (v4)
     # Can not "add" groups
     When user "participant1" adds group "group1" to room "Room 2" with 400 (v4)
+
+  Scenario: Teacher applies a new attendee map
+    Given user "participant1" creates room "class room" (v4)
+      | roomType | 2 |
+      | roomName | class room |
+    And user "participant1" adds user "participant2" to room "class room" with 200 (v4)
+    And user "participant1" adds user "participant3" to room "class room" with 200 (v4)
+    And user "participant1" adds user "participant4" to room "class room" with 200 (v4)
+    And user "participant1" sees the following attendees in room "class room" with 200 (v4)
+      | actorType  | actorId      | participantType |
+      | users      | participant1 | 1               |
+      | users      | participant2 | 3               |
+      | users      | participant3 | 3               |
+      | users      | participant4 | 3               |
+    And user "participant1" promotes "participant2" in room "class room" with 200 (v4)
+    And user "participant1" sees the following attendees in room "class room" with 200 (v4)
+      | actorType  | actorId      | participantType |
+      | users      | participant1 | 1               |
+      | users      | participant2 | 2               |
+      | users      | participant3 | 3               |
+      | users      | participant4 | 3               |
+    When user "participant1" creates 3 manual breakout rooms for "class room" with 200 (v1)
+      | users::participant3 | 0 |
+      | users::participant4 | 1 |
+    Then user "participant3" is participant of the following rooms (v4)
+      | type | name       |
+      | 2    | class room |
+      | 2    | Room 1     |
+    Then user "participant4" is participant of the following rooms (v4)
+      | type | name       |
+      | 2    | class room |
+      | 2    | Room 2     |
+    When user "participant1" moves participants into different breakout rooms for "class room" with 400 (v1)
+      | users::participant2 | 0 |
+      | users::participant3 | 2 |
+      | users::participant4 | 1 |
+    When user "participant1" moves participants into different breakout rooms for "class room" with 400 (v1)
+      | users::participant3 | -2 |
+      | users::participant4 | 1 |
+    When user "participant1" moves participants into different breakout rooms for "class room" with 400 (v1)
+      | users::participant3 | 3 |
+      | users::participant4 | 1 |
+    When user "participant1" moves participants into different breakout rooms for "class room" with 200 (v1)
+      | users::participant3 | 2 |
+      | users::participant4 | 1 |
+    Then user "participant3" is participant of the following rooms (v4)
+      | type | name       |
+      | 2    | class room |
+      | 2    | Room 3     |
+    Then user "participant4" is participant of the following rooms (v4)
+      | type | name       |
+      | 2    | class room |
+      | 2    | Room 2     |

tests/php/Service/BreakoutRoomServiceTest.php

@@ -0,0 +1,108 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023, Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Talk\Tests\php\Service;
+
+use OCA\Talk\Chat\ChatManager;
+use OCA\Talk\Config;
+use OCA\Talk\Manager;
+use OCA\Talk\Service\BreakoutRoomService;
+use OCA\Talk\Service\ParticipantService;
+use OCA\Talk\Service\RoomService;
+use OCP\EventDispatcher\IEventDispatcher;
+use OCP\IL10N;
+use OCP\Notification\IManager as INotificationManager;
+use PHPUnit\Framework\MockObject\MockObject;
+use Test\TestCase;
+
+class BreakoutRoomServiceTest extends TestCase {
+	private BreakoutRoomService $service;
+
+	/** @var Config|MockObject */
+	private $config;
+	/** @var Manager|MockObject */
+	private $manager;
+	/** @var RoomService|MockObject */
+	private $roomService;
+	/** @var ParticipantService|MockObject */
+	private $participantService;
+	/** @var ChatManager|MockObject */
+	private $chatManager;
+	/** @var IEventDispatcher|MockObject */
+	private $dispatcher;
+	/** @var IL10N|MockObject */
+	private $l;
+
+	public function setUp(): void {
+		parent::setUp();
+
+		$this->config = $this->createMock(Config::class);
+		$this->manager = $this->createMock(Manager::class);
+		$this->roomService = $this->createMock(RoomService::class);
+		$this->participantService = $this->createMock(ParticipantService::class);
+		$this->chatManager = $this->createMock(ChatManager::class);
+		$this->notificationManager = $this->createMock(INotificationManager::class);
+		$this->dispatcher = $this->createMock(IEventDispatcher::class);
+		$this->l = $this->createMock(IL10N::class);
+		$this->service = new BreakoutRoomService(
+			$this->config,
+			$this->manager,
+			$this->roomService,
+			$this->participantService,
+			$this->chatManager,
+			$this->notificationManager,
+			$this->dispatcher,
+			$this->l
+		);
+	}
+	public function dataParseAttendeeMap(): array {
+		return [
+			'Empty string means no map' => ['', 3, [], false],
+			'Empty array means no map' => ['[]', 3, [], false],
+			'OK' => [json_encode([1 => 1, 13 => 0, 42 => 2]), 3, [1 => 1, 13 => 0, 42 => 2], false],
+			'Not an array' => ['"hello"', 3, null, true],
+			'Room above max' => [json_encode([1 => 0, 13 => 1, 42 => 2]), 2, null, true],
+			'Room below min' => [json_encode([1 => 0, 13 => -1, 42 => 2]), 3, null, true],
+			'Room not int' => [json_encode([1 => 0, 13 => 'foo', 42 => 2]), 3, null, true],
+			'Room null' => [json_encode([1 => 0, 13 => null, 42 => 2]), 3, null, true],
+			'Attendee not int' => [json_encode([1 => 0, 'foo' => 1, 42 => 2]), 3, null, true],
+			'Attendee negative' => [json_encode([1 => 0, -13 => 1, 42 => 2]), 3, null, true],
+			'Attendee zero' => [json_encode([1 => 0, 0 => 1, 42 => 2]), 3, null, true],
+		];
+	}
+
+	/**
+	 * @dataProvider dataParseAttendeeMap
+	 */
+	public function testParseAttendeeMap(string $json, int $max, ?array $expected, bool $throws): void {
+		if ($throws) {
+			$this->expectException(\InvalidArgumentException::class);
+		}
+
+		$actual = self::invokePrivate($this->service, 'parseAttendeeMap', [$json, $max]);
+		$this->assertEquals($expected, $actual);
+	}
+}