feat(plugin-cc): added-werbtc-task-refactor #4359
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 📝 WalkthroughWalkthroughThe changes update logic and tests related to UI controls for telephony tasks in a contact center application. In the sample app, the condition for setting UI controls on incoming telephony calls was broadened by removing a restriction based on the login option. The WebRTC class was updated so that its constructor enables both accept and decline controls by default, and its UI control logic was refined to handle different task event types, specifically managing the mute and wrapup controls' visibility and enabled states. The test suite was expanded to cover these new UI control behaviors and WebRTC event handling. Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
|
@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: 0
🧹 Nitpick comments (1)
packages/@webex/plugin-cc/src/services/task/types.ts (1)
1250-1250: Fix inaccurate JSDoc comment.The comment "Disables the button control" is misleading since the method accepts a boolean parameter and can both enable or disable the control based on the value passed.
- * Disables the button control. + * Sets the enabled state of the button control.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/samples/contact-center/app.js(1 hunks)packages/@webex/plugin-cc/src/services/task/types.ts(1 hunks)packages/@webex/plugin-cc/src/services/task/voice/WebRTC.ts(2 hunks)packages/@webex/plugin-cc/test/unit/spec/services/task/voice/WebRTC.ts(3 hunks)
🔇 Additional comments (7)
packages/@webex/plugin-cc/src/services/task/voice/WebRTC.ts (2)
24-24: LGTM! Good initialization of UI controls.Enabling both accept and decline controls by default in the constructor ensures proper initialization of the WebRTC task UI state.
40-71: Well-structured UI control management with good event-driven logic.The implementation properly calls the superclass method first and then applies WebRTC-specific UI control logic based on task event types. The logic for each case makes sense:
AGENT_CONTACT_ASSIGNED: Enables mute when call is activeAGENT_CONTACT_HELD: Disables mute during hold (logical UX decision)AGENT_CONTACT_UNHELD: Re-enables mute when resuming- Default: Hides mute during wrapup state
The code follows a clear pattern and handles the different interaction states appropriately.
docs/samples/contact-center/app.js (1)
1280-1284: Good simplification of UI control logic.Removing the browser login exclusion makes sense given that the WebRTC class constructor now handles UI control initialization properly. This ensures consistent UI control behavior for all telephony tasks regardless of login method, which aligns with the enhanced UI control management implemented in the WebRTC service.
packages/@webex/plugin-cc/test/unit/spec/services/task/voice/WebRTC.ts (4)
2-5: Good addition of necessary imports for enhanced test coverage.The new imports for
CALL_EVENT_KEYS,TASK_EVENTS, andCC_EVENTSare properly utilized in the expanded test suite to verify event handling and UI control management.
32-32: Good enhancement of test data structure.Adding the
interactionobject withstateproperty provides more realistic test data that aligns with the actual task data structure used in the implementation.
78-96: Excellent test coverage for internal WebRTC methods.These tests properly verify:
- Event listener registration for remote media
- Proper event emission when handling remote media
The tests ensure the WebRTC service correctly handles calling service integration.
98-139: Comprehensive UI controls test suite with excellent coverage.The test suite thoroughly validates the UI control behavior for different task event types:
- Initial state verification (accept/decline enabled)
- Event-specific behavior (assigned, held, unheld states)
- Default case handling (wrapup state)
Each test case directly corresponds to the implementation logic in the
setUIControlsmethod, ensuring the UI control state management works correctly across all task lifecycle events.
| case CC_EVENTS.AGENT_CONTACT_RESERVED: | ||
| this.taskUiControls.accept.setEnabled(true); | ||
| case CC_EVENTS.AGENT_CONTACT_ASSIGNED: | ||
| this.updateTaskUiControls({ |
There was a problem hiding this comment.
We should set accept and decline to false right after the call is answered here and where are we setting them to true when we receive RESERVED
There was a problem hiding this comment.
We don't need RESERVED as during that event we create the new task.
Also we are calling super.updateTaskUiControls wherein we set accept and decline to false(visibility) and set the other call controls.
There was a problem hiding this comment.
Could you please point me towards where we are doing super. updateTaskUiControls
| // hide mute when wrapup is active | ||
| if (this.taskUiControls.wrapup.visible) { | ||
| this.updateTaskUiControls({ | ||
| mute: [false, false], |
There was a problem hiding this comment.
What about the mute handling when consult is active, cancelled, accepted, declined or ended?
There was a problem hiding this comment.
In case of all ended cases we are handling it as seen above.
For consult cases... in agent desktop we are showing mute for all consulting flows... so it should work out of the box.
Declined would anyway have it working as we don't show mute before that itself.
| }); | ||
|
|
||
| it('setUIControls for AGENT_CONTACT_ASSIGNED shows mute', () => { | ||
| const assignedData = { ...data, type: CC_EVENTS.AGENT_CONTACT_ASSIGNED } as any; |
There was a problem hiding this comment.
If it's possible to avoid any, let's try to do that even in test files
|
@Kesari3008 , attaching video showing consult flow - https://app.vidcast.io/share/99684c4a-7dc9-437c-843f-808a766eeb93 |
Kesari3008
left a comment
Kesari3008
left a comment
There was a problem hiding this comment.
Observed few issues while testing:
- After the consult is ended, we resume the customer interaction but mute button is still disabled.
- Customer side audio is not being received on the agent's end. One-way audio also observed for consult case. Please cross-check once
- Task List doesn't have the accept/decline button for comsult and transfer case for receiving agent side.
Approving it but would like to setup a call tomorrow to test the consult flows more thoroughly before we merge this.
| case CC_EVENTS.AGENT_CONTACT_RESERVED: | ||
| this.taskUiControls.accept.setEnabled(true); | ||
| case CC_EVENTS.AGENT_CONTACT_ASSIGNED: | ||
| this.updateTaskUiControls({ |
There was a problem hiding this comment.
Could you please point me towards where we are doing super. updateTaskUiControls
| break; | ||
|
|
||
| // on consult accepted hide accept/decline and show mute | ||
| case CC_EVENTS.AGENT_CONSULTING: |
There was a problem hiding this comment.
What about the controls handling for initiating end for this case and consult end case?
There was a problem hiding this comment.
That is done in the Voice class. It will always be enabled for webRTC case.
|
Merging this PR after chatting with Priya. |
3 participants
COMPLETES #< CAI-6475 >
This pull request addresses
by making the following changes
Vidcast - https://app.vidcast.io/share/be1f9085-f773-4e66-8ecd-7cdf78d23c63
Change Type
The following scenarios where 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.