feat(plugin-cc): refactored-event-emissions #4252
Conversation
📝 Walkthrough""" WalkthroughThis change introduces new event types and corresponding event emission logic for consult and recording state transitions within a contact center application. In the sample app, consult-related task event listeners are split into two distinct events ( Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 (For a CapTP with native promises, see @endo/eventual-send and @endo/captp) Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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[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/cc.ts (1)
561-565: Consider adding type safety for event handling.The current implementation emits all non-keepalive events with their original type. While the switch statement handles specific event mappings, adding type checking for
eventData.data.typewould prevent potential issues with undefined properties.- if (!eventData.keepalive) { + if (!eventData.keepalive && eventData.data && eventData.data.type) { // @ts-ignore this.emit(eventData.data.type, eventData.data); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/samples/contact-center/app.js(1 hunks)packages/@webex/plugin-cc/src/cc.ts(3 hunks)packages/@webex/plugin-cc/src/constants.ts(1 hunks)packages/@webex/plugin-cc/src/services/task/TaskManager.ts(2 hunks)packages/@webex/plugin-cc/src/services/task/types.ts(1 hunks)packages/@webex/plugin-cc/test/unit/spec/cc.ts(2 hunks)packages/@webex/plugin-cc/test/unit/spec/services/task/TaskManager.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Initialize Project
- GitHub Check: AWS Amplify Console Web Preview
🔇 Additional comments (15)
packages/@webex/plugin-cc/src/constants.ts (1)
18-22: Well-structured event constants added for agent session operations.These new constants follow the established naming pattern for agent events and will help standardize the WebSocket event handling across the codebase as intended in the PR.
packages/@webex/plugin-cc/src/services/task/types.ts (2)
51-52: New consult event types follow established naming conventions.These event types properly replace the previously used
task:consultOfferCreatedwith more specific and distinct events (task:consultCreatedandtask:offerConsult), improving the event handling granularity.
57-61: Recording state events added with consistent naming patterns.These new event types for recording state transitions follow the existing naming conventions and properly capture success and failure states for pause and resume actions.
docs/samples/contact-center/app.js (2)
613-619: Updated event listeners to match new event model.The sample application now correctly uses the new granular consult events, replacing the previous single event with two distinct events (
task:consultCreatedandtask:offerConsult). This change is consistent with the updates to the event constants intypes.ts.
678-681: Simplified task rejection handling.The special case handling for
RONA_TIMER_EXPIREDhas been removed, simplifying the event handler logic. The handler now consistently logs the rejection reason and shows the agent state popup regardless of the specific reason.packages/@webex/plugin-cc/test/unit/spec/cc.ts (1)
1522-1546: Comprehensive test coverage for new WebSocket event handling.Great job adding tests that verify each new agent event is correctly emitted when the corresponding WebSocket message is received. The parameterized test approach is maintainable and scales well for future event additions.
packages/@webex/plugin-cc/src/cc.ts (1)
567-598: Well-structured event handling implementation.The switch statement approach for handling agent events is more explicit and maintainable than the previous indiscriminate re-emission. This implementation successfully maps internal event types to standardized SDK event names, which aligns with the PR objective of renaming widget-related events to follow SDK naming conventions.
packages/@webex/plugin-cc/test/unit/spec/services/task/TaskManager.ts (4)
492-494: Good test refactoring to improve readability.Storing the task reference once and reusing it improves test clarity and reduces redundant method calls. The task-level emit spy is a good approach to verify the correct events are being emitted.
557-562: Well-structured test for consult offer handling.The mock implementation for
updateTaskDataeffectively tests both the data update and the resulting state. This approach ensures that the task's internal state is correctly modified before testing event emissions.
758-760: Good test pattern for wrap-up event.Capturing the task reference and setting up the spy before the event emission ensures proper verification of the task event flow.
1191-1204: Excellent use of parameterized tests for recording events.The parameterized test approach efficiently tests all recording event variants while appropriately differentiating between success and failure cases. This pattern reduces code duplication while maintaining thorough test coverage.
packages/@webex/plugin-cc/src/services/task/TaskManager.ts (4)
181-181: Added appropriate task event emission for consult creation.This implementation correctly emits the TASK_CONSULT_CREATED event after updating the task data, ensuring event handlers receive the current task state.
189-190: Added proper task event emission for consult offers.This implementation correctly emits the TASK_OFFER_CONSULT event after updating the task data with isConsulted flag, ensuring the task state accurately reflects its participation in the consult.
225-227: Added task wrap-up event emission before removal.This change ensures that listeners can respond to the wrap-up event before the task is removed from the collection, providing a complete event lifecycle.
228-243: Well-implemented recording state event handling.The implementation correctly handles all recording state transitions with a consistent pattern:
- Update task data first
- Emit appropriate event with correct payload (task object for success, raw data for failures)
This pattern provides optimal information to event handlers based on the event type.
adhmenon
left a comment
adhmenon
left a comment
There was a problem hiding this comment.
NOTE
I have not filtered the event data anywhere and just renamed the events and emitted only the required ones as of now.
As mentioned in the comments, there are areas where I am open to discussion. Regarding the filtering of events, I can do that as well in this ticket.
| // Consult flows | ||
| task.on('task:consultOfferCreated', (task) => { | ||
| console.log('Consult offer created'); | ||
| task.on('task:consultCreated', (task) => { |
There was a problem hiding this comment.
Was not present before added this here so we can see the logs, open to removing them if we feel logs clutter.
| } | ||
| }); | ||
|
|
||
| task.on('task:rejected', (reason) => { |
There was a problem hiding this comment.
Duplicate event here... was an older one and not being used. Hence removed it.
packages/@webex/plugin-cc/src/cc.ts
Outdated
| // Re-emit the events related to agent | ||
| if (Object.values(CC_AGENT_EVENTS).includes(eventData.data?.type)) { | ||
| // Re-emit all the events related to agent except keep-alives | ||
| if (!eventData.keepalive) { |
There was a problem hiding this comment.
Here is where I would like a discussion.
So, right now I am emitting ALL events coming from websocket except the keep-alives. My reasoning is that because agent desktop MIGHT consume our SDK in the future, we need to do this for them.
Whatever events we want to use in widgets HAVE been refactored as per SDK standards and are added below.
Open to suggestions - should we add this change later on? I am alright with it as I'm not sure if agent desktop team would consume our SDK anytime soon. Based on the decision, will change the code and documentation.
cc @rsarika
| if (eventData.type === CC_EVENTS.AGENT_MULTI_LOGIN) { | ||
| // @ts-ignore | ||
| this.emit(AGENT_MULTI_LOGIN, eventData.data); | ||
| switch (eventData.type) { |
There was a problem hiding this comment.
Cleaned up switch case of important events we want to send in SDK format for widgets alone.
| export const TASK_MANAGER_FILE = 'TaskManager'; | ||
| export const AGENT_STATE_CHANGE = 'agent:stateChange'; | ||
| export const AGENT_MULTI_LOGIN = 'agent:multiLogin'; | ||
| export const AGENT_STATION_LOGIN_SUCCESS = 'agent:stationLoginSuccess'; |
There was a problem hiding this comment.
Added these new events which we will consume in widgets soon.
| isConsulted: false, // This ensures that the task consult status is always reset | ||
| }); | ||
| // Do not emit anything since this be received only as a result of an API invocation(handled by a promise) | ||
| task.emit(TASK_EVENTS.TASK_CONSULT_CREATED, task); |
There was a problem hiding this comment.
We need to emit these events as these are now being consumed in widgets (ie the older websocket ones are being consumed), hence we need to emit this so that we can refactor.
This is backward compatible also.
| task.emit(TASK_EVENTS.TASK_WRAPPEDUP, task); | ||
| this.removeTaskFromCollection(task); | ||
| break; | ||
| case CC_EVENTS.CONTACT_RECORDING_PAUSED: |
There was a problem hiding this comment.
Added these extra events as on widgets we were listening to the websocket events directly which is incorrect.
Better to listen via SDK generated events.
Note that
| TASK_CONSULT_QUEUE_FAILED: 'task:consultQueueFailed', | ||
| TASK_CONSULT_ACCEPTED: 'task:consultAccepted', | ||
| TASK_CONSULTING: 'task:consulting', | ||
| TASK_CONSULT_CREATED: 'task:consultCreated', |
There was a problem hiding this comment.
All these events are needed for widgets, hence added them here in our standard format.
| CC_FILE, | ||
| AGENT_STATE_CHANGE, | ||
| AGENT_MULTI_LOGIN, | ||
| AGENT_STATION_LOGIN_SUCCESS, |
There was a problem hiding this comment.
Added tests for these events as they were not present before.
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
adhmenon
changed the title
| } as const; | ||
|
|
||
| export type TASK_EVENTS = Enum<typeof TASK_EVENTS>; | ||
| export enum TASK_EVENTS { |
There was a problem hiding this comment.
Made it an enum as of now.. felt it was easiest as I could easily then import it in the widgets.
@Shreyas281299 @rsarika
| agentList: Array<BuddyDetails>; | ||
| }>; | ||
|
|
||
| export enum AGENT_EVENTS { |
There was a problem hiding this comment.
Again, as mentioned below, felt it would be much better as an enum so we can use it in SDK and externally. Unlike type which can only be used in runtime and was causing major confusion. Tested it with widgets and it is working well.
There was a problem hiding this comment.
Thanks for this. This will make using these events in widgets and other apps really easy
| this.emit(AGENT_EVENTS.AGENT_STATION_LOGIN_SUCCESS, stationLoginData); | ||
| break; | ||
| } | ||
| case CC_EVENTS.AGENT_RELOGIN_SUCCESS: |
There was a problem hiding this comment.
Added this case and tested it both manually and in unit tests.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/@webex/plugin-cc/src/cc.ts (1)
33-620: 💡 Verification agent🧩 Analysis chain
Verify we're not missing any agent events in the refactored switch statement.
The current implementation handles the following event types:
- AGENT_STATION_LOGIN_SUCCESS
- AGENT_RELOGIN_SUCCESS
- AGENT_STATION_LOGIN_FAILED
- AGENT_LOGOUT_SUCCESS
- AGENT_LOGOUT_FAILED
- AGENT_STATE_CHANGE
- AGENT_MULTI_LOGIN
- AGENT_DN_REGISTERED
Please check whether there are any other agent-specific events from
CC_EVENTSthat should be explicitly handled in the switch statement to ensure complete coverage of all widget-related events.
🏁 Script executed:
#!/bin/bash # List all CC_EVENTS constants to verify we're covering all agent-related events # Search for all CC_AGENT_EVENTS definitions rg -A 20 -B 5 "export const CC_AGENT_EVENTS" packages/@webex/plugin-cc/src/Length of output: 2528
To verify all agent-specific events, let’s list the
CC_EVENTSconstants as well:
🏁 Script executed:
#!/bin/bash # Show all CC_EVENTS definitions to compare against handled cases rg -A 30 -B 5 "export const CC_EVENTS" packages/@webex/plugin-cc/src/services/config/types.tsLength of output: 890
Add missing agent events in
handleWebSocketMessageThe
switch(eventData.type)currently only handles eight of theCC_AGENT_EVENTS, but the full set includes these additional agent‐specific events that aren’t explicitly covered:
- CC_EVENTS.WELCOME
- CC_EVENTS.AGENT_RELOGIN_FAILED
- CC_EVENTS.AGENT_LOGOUT
- CC_EVENTS.AGENT_STATION_LOGIN
- CC_EVENTS.AGENT_STATE_CHANGE_SUCCESS
- CC_EVENTS.AGENT_STATE_CHANGE_FAILED
- CC_EVENTS.AGENT_BUDDY_AGENTS
- CC_EVENTS.AGENT_BUDDY_AGENTS_SUCCESS
- CC_EVENTS.AGENT_BUDDY_AGENTS_RETRIEVE_FAILED
- CC_EVENTS.AGENT_CONTACT_RESERVED
Please review whether any of these should be mapped to a corresponding
AGENT_EVENTS.*emit (especially those carrying payloads that need reshaping) and add cases for them. If certain events aren’t relevant to the widget, add a comment explaining why they’re intentionally omitted.🧰 Tools
🪛 Biome (1.9.4)
[error] 316-316: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🧹 Nitpick comments (1)
packages/@webex/plugin-cc/src/cc.ts (1)
560-618: Well-structured implementation of standardized event emission.The switch statement provides a clear and explicit handling of each agent event type, ensuring they're properly emitted with standardized names from the
AGENT_EVENTSenum. For login events, the code correctly computes themmProfileobject and includes the proper tracking ID.The code successfully implements the PR objective of renaming widget-related events to align with SDK naming standards.
Consider extracting the duplicate logic for handling
AGENT_STATION_LOGIN_SUCCESSandAGENT_RELOGIN_SUCCESSinto a helper function to reduce code duplication:- case CC_EVENTS.AGENT_STATION_LOGIN_SUCCESS: { - const {channelsMap, ...loginData} = eventData.data; - const stationLoginData = { - ...loginData, - mmProfile: { - chat: channelsMap.chat?.length, - email: channelsMap.email?.length, - social: channelsMap.social?.length, - telephony: channelsMap.telephony?.length, - }, - notifsTrackingId: eventData.trackingId, - }; - // @ts-ignore - this.emit(AGENT_EVENTS.AGENT_STATION_LOGIN_SUCCESS, stationLoginData); - break; - } - case CC_EVENTS.AGENT_RELOGIN_SUCCESS: - { - const {channelsMap, ...loginData} = eventData.data; - const stationReLoginData = { - ...loginData, - mmProfile: { - chat: channelsMap.chat?.length, - email: channelsMap.email?.length, - social: channelsMap.social?.length, - telephony: channelsMap.telephony?.length, - }, - notifsTrackingId: eventData.trackingId, - }; - // @ts-ignore - this.emit(AGENT_EVENTS.AGENT_RELOGIN_SUCCESS, stationReLoginData); - } - break; + case CC_EVENTS.AGENT_STATION_LOGIN_SUCCESS: + case CC_EVENTS.AGENT_RELOGIN_SUCCESS: { + const {channelsMap, ...loginData} = eventData.data; + const loginEventData = { + ...loginData, + mmProfile: { + chat: channelsMap.chat?.length, + email: channelsMap.email?.length, + social: channelsMap.social?.length, + telephony: channelsMap.telephony?.length, + }, + notifsTrackingId: eventData.trackingId, + }; + // @ts-ignore + const eventType = eventData.type === CC_EVENTS.AGENT_STATION_LOGIN_SUCCESS ? + AGENT_EVENTS.AGENT_STATION_LOGIN_SUCCESS : AGENT_EVENTS.AGENT_RELOGIN_SUCCESS; + this.emit(eventType, loginEventData); + break; + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/@webex/plugin-cc/src/cc.ts(2 hunks)packages/@webex/plugin-cc/src/constants.ts(0 hunks)packages/@webex/plugin-cc/src/index.ts(1 hunks)packages/@webex/plugin-cc/src/services/agent/types.ts(1 hunks)packages/@webex/plugin-cc/src/services/task/TaskManager.ts(3 hunks)packages/@webex/plugin-cc/src/services/task/types.ts(1 hunks)packages/@webex/plugin-cc/test/unit/spec/cc.ts(3 hunks)
💤 Files with no reviewable changes (1)
- packages/@webex/plugin-cc/src/constants.ts
✅ Files skipped from review due to trivial changes (2)
- packages/@webex/plugin-cc/src/index.ts
- packages/@webex/plugin-cc/src/services/agent/types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/@webex/plugin-cc/src/services/task/types.ts
- packages/@webex/plugin-cc/src/services/task/TaskManager.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/@webex/plugin-cc/src/cc.ts (2)
packages/@webex/plugin-cc/src/services/config/types.ts (2)
CC_EVENTS(71-74)CC_EVENTS(82-82)packages/@webex/plugin-cc/src/index.ts (1)
AGENT_EVENTS(7-7)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Packages
- GitHub Check: AWS Amplify Console Web Preview
🔇 Additional comments (5)
packages/@webex/plugin-cc/test/unit/spec/cc.ts (3)
11-11: Good addition of the AGENT_EVENTS import.The import of the
AGENT_EVENTSenum aligns with the refactoring of event emissions in the main code. This change helps maintain consistency between the implementation and tests.
576-576: Well updated event emission assertions.The test now properly verifies that the ContactCenter emits standardized event names from the
AGENT_EVENTSenum rather than the previous direct string constants.Also applies to: 581-581
1515-1606: Great test coverage for the refactored event handling!The new test suite comprehensively verifies that the
handleWebSocketMessagemethod correctly emits standardized events for all agent-related event types. The tests cover:
- Computing
mmProfilefrom channel arrays for login events- Including
notifsTrackingIdfrom the original tracking ID- Direct re-emission of data for other agent events
This ensures the refactored event emission logic is working as expected.
packages/@webex/plugin-cc/src/cc.ts (2)
33-33: Good update to imports for the centralized event handling approach.The import of
AGENT_EVENTSenum in place of removed constants supports the refactored event handling approach, improving maintainability by centralizing event type definitions.Also applies to: 35-35
554-558: Consistent implementation of universal event emission.The code now re-emits all non-keepalive agent-related events, which aligns with the PR objective of continuing to emit all websocket events to support Agent Desktop usage. The filtering logic is clear and maintains backward compatibility.
|
Checked with amplify link and locally linking widgets (without any changes to ensure backward compatibility) and everything was working fine. |
3 participants
COMPLETES #< CAI-6349 >
This pull request addresses
by making the following changes
Vidcast - https://app.vidcast.io/share/2364a3bd-7f9d-430b-9e2e-e6a1292eb8d3
UPDATE Vidcast after making event name changes and locally linking with widgets - https://app.vidcast.io/share/9bf70bbb-6bb4-46c1-b068-49cc08fc25e3
Change Type
The following scenarios were tested
The GAI Coding Policy And Copyright Annotation Best Practices
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.