feat(contact-center): emit AgentStateChange event #4030
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 WalkthroughThis pull request introduces enhancements to the contact center application's agent state management and event handling. The changes focus on improving wrap-up code functionality in the Changes
Sequence DiagramsequenceDiagram
participant Agent
participant WebApp
participant WebSocket
participant ContactCenter
Agent->>WebApp: Interact with UI
WebApp->>ContactCenter: Register Agent
ContactCenter->>WebSocket: Establish Connection
WebSocket-->>ContactCenter: Agent State Change Event
ContactCenter->>WebApp: Update UI Elements
WebApp->>Agent: Display Updated Agent Status
Possibly related PRs
Suggested reviewers
Poem
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 (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/@webex/plugin-cc/src/cc.ts (1)
287-294:⚠️ Potential issueAdd error handling and type safety checks
The WebSocket message handler needs improvements in error handling and type safety:
- Add try-catch for JSON.parse
- Add type checking for eventData
- Clean up the listener during station logout
Apply this diff to improve the implementation:
this.services.webSocketManager.on('message', (event) => { - const eventData = JSON.parse(event); - - if (eventData.type === CC_EVENTS.AGENT_STATE_CHANGE) { - // @ts-ignore - this.emit(CC_EVENTS.AGENT_STATE_CHANGE, eventData.data); - } + try { + const eventData = JSON.parse(event); + + if (eventData && typeof eventData === 'object' && + eventData.type === CC_EVENTS.AGENT_STATE_CHANGE && + eventData.data) { + this.emit(CC_EVENTS.AGENT_STATE_CHANGE, eventData.data); + } + } catch (error) { + LoggerProxy.error('Failed to parse WebSocket message', { + module: CC_FILE, + method: 'setupEventListeners', + error + }); + } });Additionally, store the message handler reference and remove the listener in stationLogout:
+ private messageHandler = (event: string) => { + try { + const eventData = JSON.parse(event); + + if (eventData && typeof eventData === 'object' && + eventData.type === CC_EVENTS.AGENT_STATE_CHANGE && + eventData.data) { + this.emit(CC_EVENTS.AGENT_STATE_CHANGE, eventData.data); + } + } catch (error) { + LoggerProxy.error('Failed to parse WebSocket message', { + module: CC_FILE, + method: 'setupEventListeners', + error + }); + } + }; private setupEventListeners() { this.services.connectionService.on('connectionLost', this.handleConnectionLost.bind(this)); - this.services.webSocketManager.on('message', (event) => { - // ... existing code - }); + this.services.webSocketManager.on('message', this.messageHandler); } public async stationLogout(data: Logout): Promise<StationLogoutResponse> { try { const logoutResponse = this.services.agent.logout({ data, }); await logoutResponse; + this.services.webSocketManager.off('message', this.messageHandler); if (this.webCallingService) { this.webCallingService.deregisterWebCallingLine(); }
🧹 Nitpick comments (1)
packages/@webex/plugin-cc/test/unit/spec/cc.ts (1)
840-879: Add negative test cases for error scenariosThe test suite looks good but could be enhanced with additional test cases:
Consider adding these test cases:
it('should handle JSON parse error gracefully', () => { webex.cc.setupEventListeners(); const messageCallback = mockWebSocketManager.on.mock.calls[0][1]; // Simulate receiving invalid JSON messageCallback('invalid json'); expect(LoggerProxy.error).toHaveBeenCalledWith( 'Failed to parse WebSocket message', expect.any(Object) ); expect(cCEmitSpy).not.toHaveBeenCalled(); }); it('should not emit event for non-AGENT_STATE_CHANGE message type', () => { webex.cc.setupEventListeners(); const messageCallback = mockWebSocketManager.on.mock.calls[0][1]; const eventData = { type: 'OTHER_EVENT', data: { some: 'data' } }; messageCallback(JSON.stringify(eventData)); expect(cCEmitSpy).not.toHaveBeenCalled(); }); it('should cleanup message listener on stationLogout', async () => { webex.cc.setupEventListeners(); const offSpy = jest.spyOn(mockWebSocketManager, 'off'); await webex.cc.stationLogout({ logoutReason: 'test' }); expect(offSpy).toHaveBeenCalledWith( 'message', expect.any(Function) ); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/samples/contact-center/app.js(2 hunks)packages/@webex/plugin-cc/src/cc.ts(2 hunks)packages/@webex/plugin-cc/test/unit/spec/cc.ts(5 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
docs/samples/contact-center/app.js
[error] 282-282: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 282-282: Unexpected constant condition.
(lint/correctness/noConstantCondition)
🔇 Additional comments (1)
docs/samples/contact-center/app.js (1)
323-323: LGTM!
The implementation correctly retrieves the selected text from the dropdown.
docs/samples/contact-center/app.js
Outdated
| webex.cc.on('AgentStateChange', (data) => { | ||
| if(data.type = 'AgentStateChangeSuccess') { | ||
| idleCodesDropdown.value = data.auxCodeId.trim() !== '' ? data.auxCodeId : 0; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Fix the condition operator and add error handling
The event handler has several issues that need to be addressed:
Apply this diff to fix the implementation:
- webex.cc.on('AgentStateChange', (data) => {
- if(data.type = 'AgentStateChangeSuccess') {
- idleCodesDropdown.value = data.auxCodeId.trim() !== '' ? data.auxCodeId : 0;
- }
- });
+ webex.cc.on('AgentStateChange', (data) => {
+ if (data && typeof data === 'object' && data.type === 'AgentStateChangeSuccess') {
+ const DEFAULT_CODE = '0'; // Default code when no aux code is present
+ idleCodesDropdown.value = data.auxCodeId?.trim() !== '' ? data.auxCodeId : DEFAULT_CODE;
+ }
+ });Key fixes:
- Fixed assignment operator (=) to comparison operator (===)
- Added null checks for data object
- Added optional chaining for auxCodeId
- Replaced magic number with named constant
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| webex.cc.on('AgentStateChange', (data) => { | |
| if(data.type = 'AgentStateChangeSuccess') { | |
| idleCodesDropdown.value = data.auxCodeId.trim() !== '' ? data.auxCodeId : 0; | |
| } | |
| }); | |
| webex.cc.on('AgentStateChange', (data) => { | |
| if (data && typeof data === 'object' && data.type === 'AgentStateChangeSuccess') { | |
| const DEFAULT_CODE = '0'; // Default code when no aux code is present | |
| idleCodesDropdown.value = data.auxCodeId?.trim() !== '' ? data.auxCodeId : DEFAULT_CODE; | |
| } | |
| }); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 282-282: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 282-282: Unexpected constant condition.
(lint/correctness/noConstantCondition)
There was a problem hiding this comment.
@mkesavan13, thanks for confirming that you've addressed the changes! Let me know if there's anything else needed.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
COMPLETES SPARK-584812
This pull request addresses
Emitting Event for
AgentStateChangeby making the following changes
AgentStateChangeevent with the same nameChange Type
The following scenarios were tested
agent.state.emit.mov
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.
Summary by CodeRabbit
New Features
Bug Fixes
Tests