Skip to content

Fix split "being in a room" from "calling"#467

Merged
nickvergessen merged 5 commits into
masterfrom
fix-split-being-in-a-room-from-calling
Nov 14, 2017
Merged

Fix split "being in a room" from "calling"#467
nickvergessen merged 5 commits into
masterfrom
fix-split-being-in-a-room-from-calling

Conversation

@danxuliu

@danxuliu danxuliu commented Nov 9, 2017

Copy link
Copy Markdown
Member

This is a follow up for #459.

This pull request fixes the arguments passed to Participant constructor (which fixes joining a room as a guest from the UI) and the integration tests for setting a password and chatting, as they should join a room instead of a call since the changes introduced in the previous pull request.

The integration tests are not fully fixed yet, though. It seems that it is no longer possible to join a call without joining the room first, not even for the creator or someone who was invited to the room. The question is, should directly joining a call automatically make the participant to join the room too? Or should it be kept like now, in which the participant must join the room explicitly? @Ivansss @nickvergessen Please fix the code and/or the integration tests as needed.

Fix #465

@danxuliu danxuliu added this to the 3.0 (Nextcloud 13) milestone Nov 9, 2017
@nickvergessen nickvergessen self-requested a review November 10, 2017 09:47
@nickvergessen

Copy link
Copy Markdown
Member

First commit is now in #471

@nickvergessen nickvergessen mentioned this pull request Nov 10, 2017
@nickvergessen

Copy link
Copy Markdown
Member

I will take this over

Since commit eae71a3 a participant
(user or guest) can be in a room without joining a call; since then
sessions are no longer set and passwords are no longer checked when
joining a call, but when joining a room. Due to this some integration
tests had to be modified to join a room instead of a call.

Note, however, that in the modified tests there is no need to join a
call too once the participant has joined the room, as they either test
joining the room itself (the password tests) or the tested actions
simply need the participant to be in a room (the chat tests). Moreover,
in this later case, note that a participant is in a room if she created
a room, was invited to it or explicitly joined it, so the chat tests
involving a participant creating a room or inviting another one to it
did not need to be modified.

Finally, neither the call tests nor the "userJoinsCall" method from
FeatureContext were modified yet, as it has to be decided first whether
directly joining a call also joins the room automatically or not.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@nickvergessen nickvergessen force-pushed the fix-split-being-in-a-room-from-calling branch from ebc2568 to 41ab1d7 Compare November 10, 2017 10:48
@nickvergessen nickvergessen self-assigned this Nov 10, 2017
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen

Copy link
Copy Markdown
Member

Fixed, also guest handling is fixed now and they can join and leave as they want.

Sometimes it takes a while until the first connection works, but afterwards it's always instant.

@nickvergessen nickvergessen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving the part of Daniel

@nickvergessen

Copy link
Copy Markdown
Member

Also fixes #465

We shouldn't have ended up in the place at all... it was a copy mistake from !== instead of ===


Just to protocol the answer to your question @danxuliu
Yes, we require joining a room before joining a call.

@nickvergessen nickvergessen merged commit a773c8a into master Nov 14, 2017
@nickvergessen nickvergessen deleted the fix-split-being-in-a-room-from-calling branch November 14, 2017 12:20
marcoambrosini pushed a commit that referenced this pull request Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Guest can't join calls

2 participants