fix(user-state): receive agent stateChange event #350
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 WalkthroughThe pull request introduces changes to the contact center user state management, focusing on agent state tracking. A new constant Changes
Sequence DiagramsequenceDiagram
participant UserState as User State Hook
participant CC as Contact Center
participant Agent as Agent State
CC ->> UserState: emit 'agent:stateChange' event
UserState ->> UserState: Check event data
alt Valid State Change
UserState ->> Agent: Update auxiliary code
UserState ->> UserState: Reset elapsed time
else Invalid State
UserState ->> UserState: Ignore event
end
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: 2
🧹 Nitpick comments (1)
packages/contact-center/user-state/tests/user-state/index.tsx (1)
22-24: Consider enhancing test coverage for event handling.While the test verifies the basic structure, consider adding test cases to verify:
- Event handler registration with correct event name
- Callback behavior when state changes occur
- Error handling scenarios
Example enhancement:
it('registers agent state change handler correctly', () => { const mockCC = { on: jest.fn() }; render(<UserState />); expect(mockCC.on).toHaveBeenCalledWith( 'agent:stateChange', expect.any(Function) ); // Test callback behavior const callback = mockCC.on.mock.calls[0][1]; callback({ newState: 'Available' }); // Add assertions for state update });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
packages/contact-center/store/package.json(1 hunks)packages/contact-center/user-state/src/constants.ts(1 hunks)packages/contact-center/user-state/src/helper.ts(2 hunks)packages/contact-center/user-state/tests/helper.ts(2 hunks)packages/contact-center/user-state/tests/user-state/index.tsx(2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/contact-center/user-state/src/helper.ts (1)
Learnt from: mkesavan13
PR: webex/widgets#347
File: packages/contact-center/user-state/src/helper.ts:10-19
Timestamp: 2024-12-06T01:07:12.444Z
Learning: In `packages/contact-center/user-state/src/helper.ts`, within the `useUserState` hook, the timer reset in the `useEffect` hook is handled during the state change success. Therefore, modifying the dependency array is unnecessary.
🔇 Additional comments (7)
packages/contact-center/user-state/src/constants.ts (1)
1-1: LGTM! Well-structured event constant.
The constant follows good practices with:
- Clear, descriptive naming
- Proper event namespacing using 'agent:' prefix
- Consistent with the PR objective of handling agent state changes
packages/contact-center/user-state/tests/user-state/index.tsx (1)
9-11: LGTM! Mock implementation matches new functionality.
The mock correctly implements the cc.on event handler needed for state change events.
packages/contact-center/store/package.json (1)
25-25: Verify compatibility with new Webex version.
The upgrade from 3.6.0-wxcc.5 to 3.7.0-wxcc.3 is a minor version bump. Please ensure:
- The new version supports the agent state change events
- No breaking changes are introduced
- All dependent features are tested with this version
packages/contact-center/user-state/src/helper.ts (3)
2-2: LGTM!
The import of the AGENT_STATE_CHANGE constant is properly added and used consistently.
19-28: LGTM: Timer reset integration
The integration with the existing timer reset logic is correct, aligning with the learnings from PR #347 where timer reset is handled during state change success.
20-20: Verify the AgentStateChangeSuccess event type string
Let's verify that 'AgentStateChangeSuccess' is the correct event type string expected by the contact center SDK.
packages/contact-center/user-state/tests/helper.ts (1)
6-7: LGTM!
The mockCC object is properly updated to include the event listener method.
| cc.on(AGENT_STATE_CHANGE, (data) => { | ||
| if (data && typeof data === 'object' && data.type === 'AgentStateChangeSuccess') { | ||
| const DEFAULT_CODE = '0'; // Default code when no aux code is present | ||
| setCurrentState({ | ||
| id: data.auxCodeId?.trim() !== '' ? data.auxCodeId : DEFAULT_CODE | ||
| }); | ||
| setElapsedTime(0); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Add cleanup for the event listener to prevent memory leaks
The event listener is not being cleaned up when the component unmounts, which could lead to memory leaks and stale state updates.
Add the cleanup in the useEffect's return function:
useEffect(() => {
setElapsedTime(0);
const timer = setInterval(() => {
setElapsedTime(prevTime => prevTime + 1);
}, 1000);
- cc.on(AGENT_STATE_CHANGE, (data) => {
+ const handleStateChange = (data) => {
if (data && typeof data === 'object' && data.type === 'AgentStateChangeSuccess') {
const DEFAULT_CODE = '0'; // Default code when no aux code is present
setCurrentState({
id: data.auxCodeId?.trim() !== '' ? data.auxCodeId : DEFAULT_CODE
});
setElapsedTime(0);
}
- });
+ };
+
+ cc.on(AGENT_STATE_CHANGE, handleStateChange);
return () => {
clearInterval(timer);
+ cc.off(AGENT_STATE_CHANGE, handleStateChange);
};
}, []);📝 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.
| cc.on(AGENT_STATE_CHANGE, (data) => { | |
| if (data && typeof data === 'object' && data.type === 'AgentStateChangeSuccess') { | |
| const DEFAULT_CODE = '0'; // Default code when no aux code is present | |
| setCurrentState({ | |
| id: data.auxCodeId?.trim() !== '' ? data.auxCodeId : DEFAULT_CODE | |
| }); | |
| setElapsedTime(0); | |
| } | |
| }); | |
| const handleStateChange = (data) => { | |
| if (data && typeof data === 'object' && data.type === 'AgentStateChangeSuccess') { | |
| const DEFAULT_CODE = '0'; // Default code when no aux code is present | |
| setCurrentState({ | |
| id: data.auxCodeId?.trim() !== '' ? data.auxCodeId : DEFAULT_CODE | |
| }); | |
| setElapsedTime(0); | |
| } | |
| }; | |
| cc.on(AGENT_STATE_CHANGE, handleStateChange); |
| @@ -1,4 +1,6 @@ | |||
| import {useState, useEffect} from "react"; | |||
| // TODO: Export & Import this AGENT_STATE_CHANGE constant from SDK | |||
| import { AGENT_STATE_CHANGE } from './constants'; | |||
There was a problem hiding this comment.
nit. This import statement doesn't seem to follow the same linting as the previous line?
| @@ -15,8 +17,23 @@ export const useUserState = ({idleCodes, agentId, cc}) => { | |||
| setElapsedTime(prevTime => prevTime + 1); | |||
| }, 1000); | |||
|
|
|||
| const handleStateChange = (data) => { | |||
| if (data && typeof data === 'object' && data.type === 'AgentStateChangeSuccess') { | |||
There was a problem hiding this comment.
Why is typeof data required? All data are objects no?
There was a problem hiding this comment.
This is just to ensure that data.type doesn't throw an error just in case the data received is not an object
| // Cleanup the timer on component unmount | ||
| return () => clearInterval(timer); | ||
| return () => { | ||
| clearInterval(timer); |
There was a problem hiding this comment.
nit. It's also beneficial to set timer = null to ensure there is no leak of timers. This is a recommendation I've seen before.
| @@ -17,7 +20,10 @@ describe('UserState Component', () => { | |||
|
|
|||
| render(<UserState/>); | |||
|
|
|||
| expect(useUserStateSpy).toHaveBeenCalledWith({cc: {}, idleCodes: [], agentId: 'testAgentId'}); | |||
| expect(useUserStateSpy).toHaveBeenCalledWith({cc: { | |||
There was a problem hiding this comment.
comment. Can we add a condition for number of times called too? It should only be called once..
|
🎉 This PR is included in version 1.28.0-ccwidgets.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [1.28.0-ccwidgets.5](webex/widgets@v1.28.0-ccwidgets.4...v1.28.0-ccwidgets.5) (2024-12-23) ### Bug Fixes * **user-state:** receive agent stateChange event ([webex#350](webex#350)) ([21d6ce7](webex@21d6ce7))
|
🎉 This PR is included in version 1.28.0-ccconnectors.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [1.28.0-ccconnectors.1](webex/widgets@v1.27.5...v1.28.0-ccconnectors.1) (2025-02-05) ### Bug Fixes * add material to all components ([webex#376](webex#376)) ([de0ca28](webex@de0ca28)) * **bundling:** fixed the multiple React instance and useRef errors ([webex#355](webex#355)) ([473cd4f](webex@473cd4f)) * **call-control:** add-call-control-widget ([webex#362](webex#362)) ([a677f5e](webex@a677f5e)) * **cc-station-login:** material design ui ([webex#377](webex#377)) ([aec7034](webex@aec7034)) * **cc-store:** receive webex on init, add store types ([webex#341](webex#341)) ([9648969](webex@9648969)) * **cc-widgets:** ship-all-widgets-together ([webex#345](webex#345)) ([83d5a37](webex@83d5a37)) * cleanup and using mobx-react-lite ([webex#356](webex#356)) ([0b304c4](webex@0b304c4)) * convert npm to yarn ([webex#234](webex#234)) ([432ed3c](webex@432ed3c)) * **release:** add-publish-step-in-tooling ([webex#334](webex#334)) ([ca32235](webex@ca32235)) * rename agent state to user state ([webex#361](webex#361)) ([fe409db](webex@fe409db)) * **samples:** change samples index html hrefs ([webex#367](webex#367)) ([ff126ab](webex@ff126ab)) * **user-state:** receive agent stateChange event ([webex#350](webex#350)) ([21d6ce7](webex@21d6ce7)) ### Features * **cc-components:** setup and move user state sample ui comp ([webex#359](webex#359)) ([16a44d0](webex@16a44d0)) * **cc-store:** add logger from sdk ([webex#354](webex#354)) ([a62494b](webex@a62494b)) * **cc-widgets:** added Agent-Multi-Login-Alert Feature ([webex#364](webex#364)) ([f7d75ca](webex@f7d75ca)) * **release:** add new branch to circleci ([18f7bec](webex@18f7bec)) * **release:** publish pipeline for wxcc widgets ([webex#324](webex#324)) ([864fb52](webex@864fb52)) * taskList and IncomingTask widgets added ([webex#348](webex#348)) ([ce3a619](webex@ce3a619)) * **user-state:** load and change state, client timer ([webex#347](webex#347)) ([f1ccaeb](webex@f1ccaeb)) * **widget-cc-station-login:** Spark 575845 login widget ([webex#239](webex#239)) ([66b8a20](webex@66b8a20)) * **widgets:** added-relogin-logic ([webex#357](webex#357)) ([94dd415](webex@94dd415)) * **widgets:** shifted-timer-to-worker ([webex#352](webex#352)) ([c06fe9c](webex@c06fe9c))
COMPLETES SPARK-584812
This pull request addresses
Receiving emitted State Change Event for
agent:stateChangeby making the following changes
agent:stateChangeand updating the current State accordinglyThe following scenarios were tested
agent.state.receive.react.sample.mov
agent.state.receive.wc.sample.mov
Summary by CodeRabbit
New Features
AGENT_STATE_CHANGEfor managing agent state changes.Bug Fixes
Documentation
ccobject.