feat: Returns an error on join request with no display name. #13546
Conversation
| @@ -400,6 +400,17 @@ process_host_module(main_muc_component_config, function(host_module, host) | |||
| end | |||
| end | |||
|
|
|||
| -- Check for display name if missing return an error | |||
There was a problem hiding this comment.
Shouldn't this be a separate module? This functionality is not exclusive to lobby is it?
There was a problem hiding this comment.
This is for lobby, we are reporting this through jiconop when lobby is enabled so we can make the display name required.
Now when dropping initial connection on opening pre-join we are adding this if someone tries to join without a display name when lobby is enabled to fail and set the UI that display name is required, so the participant enters a dispalyname and retries.
There was a problem hiding this comment.
But this does not work if prejoin is not enabled ... the tests catch that failing case ... need to look into
|
@damencho What is the idea of the feature? Is it that only for lobby we require always a display name? Or do we want to always requre it? I was wondering if we should add a config for enabling or disabling this? |
|
@hristoterezov this is to implement a feature which we already have, but we will lose after merging the PR where we no longer create a connection on pre-join screen. |
|
The PR covers now the prejoin screen and not Lobby screen (this is when prejoin is disabled), so I'm working on adding that, which will require changes in both ljm and this PR. But I'll push everything on top of this, as a separate commits. |
|
Got it! Thanks for the explanation! |
|
I have pushed the changes about the lobby screen when preJoin is disabled, which should fix the tests. But I think we should not merge this till we merge #13538 as there is logic here that assumes the connection is established when opening the prejoinscreen ... and we need to sync the changes from the other PR with this change and test it. |
damencho
force-pushed
the
req-displayname
branch
2 times, most recently
from
296b221
to
e2c2614
Compare
|
I force pushed as it is rebased now on top of #13538 and is working with that change with no connection established while on prejoin screen. |
damencho
force-pushed
the
req-displayname
branch
5 times, most recently
from
80aa55d
to
b44fe90
Compare
When someone tries to join a room with lobby enabled and display name is not set returns an error.
…s isDisplayNameRequiredError.
damencho
force-pushed
the
req-displayname
branch
from
b44fe90
to
13189db
Compare
This was used on showing prejoin when connection was established on showing prejoin. We no longer establish it at that time, so it is not possible to hit it and act on it.
|
Jenkins test this please. |
When someone tries to join a room with lobby enabled and display name is not set returns an error.