Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Follow up for SIP changes #4689

Merged
merged 9 commits into from
Dec 1, 2020
Merged

Follow up for SIP changes #4689

merged 9 commits into from
Dec 1, 2020

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Nov 27, 2020

  • Make sure the key calculation is always unique
    before it was s#0 for all email invites
  • guests should no see the dial-in info panel
  • Always generate PINs for logged in users when SIP is enabled (not only on join)
  • Add capability and docs

Integration tests will be done in a new PR based on #4698

Fix #4677

@nickvergessen nickvergessen added this to the 💚 Next Major (21) milestone Nov 27, 2020
@nickvergessen nickvergessen self-assigned this Nov 27, 2020
@nickvergessen nickvergessen changed the title Make sure the key calculation is always unique Follow up for SIP changes Nov 30, 2020
@PVince81

This comment has been minimized.

@nickvergessen

This comment has been minimized.

before it was s#0 for all email invites

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

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
Member Author

Rebased to solve the TopBar conflict

@nickvergessen nickvergessen mentioned this pull request Nov 30, 2020
2 tasks
@PVince81
Copy link
Member

  • improve panel style, doesn't look that nice (could be done separately)

image

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

See minor comments.

Will approve after I'm done testing.

lib/Controller/RoomController.php Outdated Show resolved Hide resolved
lib/Controller/SignalingController.php Show resolved Hide resolved
@PVince81
Copy link
Member

PVince81 commented Nov 30, 2020

  • idea: use telephone link (<a href="tel:123-456-7890">123-456-7890</a>) so that if someone opened Talk web on mobile, they can call right away. Could be part of the above.

actually, this is part of the dialin info, so would require markdown there or something formattable.
=> enhancement for later

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Copy link
Member Author

idea: use telephone link (123-456-7890) so that if someone opened Talk web on mobile, they can call right away. Could be part of the above.

actually, this is part of the dialin info, so would require markdown there or something formattable.
=> enhancement for later

Yeah Markup doesn't really work with the clients. Also you can have multiple numbers and the user needs to be aware which one to use instead of clicking the first one. Also if clicking, it should be phonnumber,00token,pin which would automatically verify the user already.

@PVince81
Copy link
Member

PVince81 commented Nov 30, 2020

Users that are already in the room need to rejoin as well to get a PIN

In my local test with internal signaling I didn't need to rejoin.
Had admin and bob in the channel, with settings panel open.
After enabling SIP as admin and waiting a bit, the dial-in info + pin appeared there for both users (no page refresh)

@nickvergessen
Copy link
Member Author

Yeah that's why #4677 (comment) is ticked ;)

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 looks fine to me

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen merged commit 475dbec into master Dec 1, 2020
@nickvergessen nickvergessen deleted the bugfix/4677/sip-follow-ups branch December 1, 2020 08:02
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.

Follow up for SIP changes
2 participants