Send signaling messages when starting and stopping breakout rooms #8477
Conversation
|
As discussed:
|
|
From a quick look you are sending the whole mapping to the room which will be forwarded by the signaling server to each session in the room. Would it be better to add special handling to the signaling server, so that it parses the message and only sends each session an event that it should switch to a different room? It's probably not necessary that each session knows which rooms the other sessions will switch to? |
|
Sorry for the delay.
Done, but note that #8519 is needed to properly handle the event in the WebUI.
Yes, explicit handling of the message in the signaling server would be great so each session only receives its own message, and also to replace the Nextcloud session ID with the signaling session ID. I used a generic message to avoid bothering you, but if you could add it to the signaling server it would be great :-) (and in worst case I could also try to add it myself... but you will be surely quicker ;-) ). |
danxuliu
force-pushed
the
send-signaling-messages-when-starting-and-stopping-breakout-rooms
branch
2 times, most recently
from
eb135dc
to
c03b8e1
Compare
|
Would be good to check for the spreed/lib/Signaling/Manager.php Line 52 in ee85c02 and here: Line 1029 in ee85c02 |
danxuliu
force-pushed
the
send-signaling-messages-when-starting-and-stopping-breakout-rooms
branch
from
52d9112
to
584badc
Compare
|
@nickvergessen When the breakout rooms were stopped the lobby was first enabled and then the breakout rooms were stopped. This caused a I have changed the order to first stop the breakout rooms and, then, do the needed cleanup (enable the lobby and remove the asisstance requests). However, is the previous order needed for some reason? Would that change break something? |
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When asserting that a message was sent to the signaling server all the requests were validated before looking for the expected one. As the requests include the token of the room all the requests were expected to be sent to the same room; otherwise any request sent to other room would make the assert fail. Now the requests to other rooms than the room of the actual message being asserted are ignored, which will make possible to sent messages to different rooms in the same test. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When the breakout room status changes a "switchto" message is sent to all the active sessions in either the parent or the breakout rooms (depending on whether they are being started or stopped) with the token of the room that they have to switch to. When the breakout rooms are started the message is sent only to non moderators, as moderators do not have a single breakout room assigned. On the other hand, when the breakout rooms are stopped the message is also sent to all moderators (who are in a breakout room and not already in the parent room), as all participants need to switch to the parent room. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When the client receives a message to switch to a different room the WebUI joins that room. If the WebUI was already in a call it will automatically join the call in the target room; in that case the call view will be kept shown during the switch, rather than showing the chat while leaving the previous call and joining the new room to then show the call again when joining the call in the target room. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The external signaling server includes some room data in the "roomlist" update event sent when a room is modified. That data is up to date, and it will be the same received when fetching the room data again, so the properties can be already updated in the store. This prevents the lobby from being briefly shown when switching to a breakout room due to the "switchto" message being handled before the updated room data could be fetched from the server. In order to keep the changes to a minimum note that this does was not applied to guest users, as a different event seems to be sent in that case, nor to the Talk sidebar, as the current properties provided in the event should not be relevant to it. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When the breakout rooms are stopped the lobby is enabled again in them. However, as it was first enabled and then the breakout rooms were stopped if the participants updated the room data before switching back to the parent room the lobby was briefly shown. To prevent that the breakout rooms should be stopped and, then, the lobby should be enabled again. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
danxuliu
force-pushed
the
send-signaling-messages-when-starting-and-stopping-breakout-rooms
branch
from
584badc
to
a46fa7e
Compare
No internal reason in my side |
| foreach ($breakoutRooms as $breakoutRoom) { | ||
| $sessionIds = []; | ||
|
|
||
| $breakoutRoomParticipants = $participantService->getParticipantsForRoom($breakoutRoom); |
There was a problem hiding this comment.
#8660 brings in a getSessionsAndParticipantsForRooms so we can get them all with one query instead of a query per room
Maybe something for a follow up
|
Was not really successful in testing as the web totally does not work at the moment. But yoloing for now |
nickvergessen
deleted the
send-signaling-messages-when-starting-and-stopping-breakout-rooms
branch
When the breakout room status changes a generic
roommessage is sent to all the active sessions in either the parent or the breakout rooms (depending on whether they are being started or stopped) with the token of the room that they have to switch to.🚧 TODO
Another possibility would have been to send aThe explicit message will be kept, as it could be used in other scenariosroomModifiedmessage including the breakout room status and then let each client figure which room they need to switch to. This could lead to a potential issue with guests in the future, as if the client has to figure the target room it would need to check all the conversation data to find the room, but guests can only get the data for the room they are in and that is it. However that could be simply fixable by also including the token of the assigned breakout room for the participant in the parent room data, so maybe it is better to let the clients figure where to switch to.roomlistupdate event is sent when the lobby is modified and that event contains the up to date value of some properties, like the lobby state, the conversation data is now updated from the data in the event (besides still fetching again the conversation data from the server like before)🏁 Checklist
docs/has been updated or is not required