-
Notifications
You must be signed in to change notification settings - Fork 359
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
feat(wxcc): agent-multi-login-changes #4068
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@@ -2,7 +2,7 @@ import {LOGGER} from '@webex/calling'; | |||
|
|||
export default { | |||
cc: { | |||
allowMultiLogin: true, | |||
allowMultiLogin: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tried to set allowMultiLogin to false in webexConfig object inside CC Widgets but that we not working, So I think we need to set allowMultiLogin to false from SDK side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work. Approving the PR as still we can debug/test this with false also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a TODO to fix this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, why do we need this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkesavan13 , We need this because to tell backend that there should not be multi-login by agent and if there is a multi-login then send AGENT_MULTI_LOGIN event to the client, so that the client know that there is a multi-login.
This will not impact any existing functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the tests enumerated in the PR description as well as a demo vidcast
@@ -2,7 +2,7 @@ import {LOGGER} from '@webex/calling'; | |||
|
|||
export default { | |||
cc: { | |||
allowMultiLogin: true, | |||
allowMultiLogin: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work. Approving the PR as still we can debug/test this with false also.
@mkesavan13 , Updated the PR description and added Vidcast for the same. |
@pagour98 Let's update the SDK samples to show some notification on the UI for multi-login instead of showing in the console. Let's discuss 1-1 if help/clarification is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to know how the flow would look in the Widgets like, after this PR is merged. What SDK method do we call once they confirm? How does the other page get logged out?
@@ -2,7 +2,7 @@ import {LOGGER} from '@webex/calling'; | |||
|
|||
export default { | |||
cc: { | |||
allowMultiLogin: true, | |||
allowMultiLogin: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a TODO to fix this?
@@ -2,7 +2,7 @@ import {LOGGER} from '@webex/calling'; | |||
|
|||
export default { | |||
cc: { | |||
allowMultiLogin: true, | |||
allowMultiLogin: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, why do we need this change?
So when we set multilogin to false in sdk, then the server will send the AGENT_MULTI_LOGIN event. If the agent log in second time then the client will receive the event the above event and if the event is received on the existing session we show the alert dialog box and if the confirm is pressed we do web-socket registration again because in case of AGENT_MULTI_LOGIN event the server close down the existing web socket connection. When the agent sigin in new tab, agent-relogin method is called which silently login the agent in new session and the existing session will receive AGENT_MULTI_LOGIN EVENT |
|
COMPLETES #<SPARK-601180>
This pull request addresses
by making the following changes
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
Tested the agent multi login Websocket event by logging in agent multiple times.
Tested the other functionalities to ensure nothing breaks.
Tested SDK by following the CC SDK test plan suite
Vidcast for the SDK Changes Testing:- https://app.vidcast.io/share/5ad06894-3ead-40db-8441-e4739e4569d3
I certified that
I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly
Make sure to have followed the contributing guidelines before submitting.