-
Notifications
You must be signed in to change notification settings - Fork 346
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(cc-sdk): added-device-type-for-agent-config #4006
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 (
|
@@ -130,14 +130,15 @@ function register() { | |||
}); | |||
const loginVoiceOptions = agentProfile.loginVoiceOptions; | |||
agentLogin.innerHTML = '<option value="" selected>Choose Agent Login ...</option>'; // Clear previously selected option on agentLogin. | |||
dialNumber.value = ''; | |||
dialNumber.disabled = true; | |||
dialNumber.value = agentProfile.defaultDn ? agentProfile.defaultDn : ''; |
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.
Here, we simply update the dial number if it exists (ie for EXTENSION
and AGENT_DN
relogin case).
Note that we disable it if the defaultDn is undefined.
I personally felt this approach was better (ie using the defaultDn of the agent config), however if there are some issues here, I am open to changing.
@@ -207,6 +208,7 @@ function logoutAgent() { | |||
|
|||
setTimeout(() => { | |||
logoutAgentElm.classList.add('hidden'); | |||
agentLogin.selectedIndex = 0; |
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.
Needed in order tor reset the login dropdown status as in the register we set one of the options, if the agent is already logged.
packages/@webex/plugin-cc/src/cc.ts
Outdated
@@ -310,7 +310,8 @@ export default class ContactCenter extends WebexPlugin implements IContactCenter | |||
}; | |||
await this.setAgentState(stateChangeData); | |||
} | |||
// Updating isAgentLoggedIn as true to indicate to the end user | |||
|
|||
await this.handleDeviceType(deviceType, dn); |
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.
Have abstracted the logic to another method so we can handle errors there (if at all) more easily.
packages/@webex/plugin-cc/src/cc.ts
Outdated
/** | ||
* Handles the device type specific logic | ||
*/ | ||
private async handleDeviceType(deviceType: string, dn: string): Promise<void> { |
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.
In this method, we simply run a switch case for all possible device types and based on them, we assign the config object deviceType
.
Improtant thing to note is that for BROWSER
we NEED to do a WebexCallingService re-register as otherwise we have no events sent to mobius (has missed this before).
this.agentConfig.defaultDn = dn; | ||
break; | ||
default: | ||
LoggerProxy.error(`Unsupported device type: ${deviceType}`, { |
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 ideally never reach here, however we must account for all possibilites.
@@ -183,6 +183,8 @@ describe('webex.cc', () => { | |||
data: { | |||
auxCodeId: 'auxCodeId', | |||
agentId: 'agentId', | |||
deviceType: LoginOption.EXTENSION, |
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.
Need to mock this so we do not see any errors.
@@ -714,5 +719,38 @@ describe('webex.cc', () => { | |||
{module: CC_FILE, method: 'silentRelogin'} | |||
); | |||
}); | |||
|
|||
it('should handle errors during silent relogin', async () => { |
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.
Added few more tests so coverage is improved.
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.
Approving it with 2 comments. Please address them before merge
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Updated the PR. Tested all manual test cases as mentioned above and everything is working fine. |
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.
Looks good.
COMPLETES #< SPARK-586282 >
This pull request addresses
isAgentLoggedIn
flag.by making the following changes
reLogin
method.BROWSER
device type and if it is present, we initiate a re-registration ofWebexCallingService
, ie create the lines again and send request to mobius,Vidcast showing EXTENSION device type
https://app.vidcast.io/share/c9ae1f0e-53fc-4c85-ae5b-395cb5b163d3
Vidcast showing BROWSER device type
https://app.vidcast.io/share/b5142b97-df0a-46cc-a814-9ab2554d6243
Vidcast showing internet disconnect flow (same behaviour as before, not broken anything)
https://app.vidcast.io/share/3201f886-2536-4021-a366-6d65fee7bd93
Change Type
The following scenarios were tested
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.