From c8c0a2847dbdb0055c2ef298213d0e66bcc5122f Mon Sep 17 00:00:00 2001 From: Bharath Balan <62698609+bhabalan@users.noreply.github.com> Date: Mon, 6 Apr 2026 11:40:39 +0530 Subject: [PATCH 1/4] fix(cc-task): guard optional onRecordingToggle callback and fix event name mismatch in cleanup Recording pause/resume callbacks called onRecordingToggle without a null check, throwing TypeError when the optional prop wasn't provided. This crashed the SDK event emitter, preventing subsequent hold/resume events from firing. Additionally, cleanup used CONTACT_RECORDING_PAUSED/RESUMED (different string values) instead of TASK_RECORDING_PAUSED/RESUMED, so old callbacks were never removed on re-renders. --- packages/contact-center/task/src/helper.ts | 24 ++++---- packages/contact-center/task/tests/helper.ts | 58 ++++++++++++++++++++ 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/packages/contact-center/task/src/helper.ts b/packages/contact-center/task/src/helper.ts index c3e2b6129..9da1b84f7 100644 --- a/packages/contact-center/task/src/helper.ts +++ b/packages/contact-center/task/src/helper.ts @@ -595,10 +595,12 @@ export const useCallControl = (props: useCallControlProps) => { const pauseRecordingCallback = () => { try { setIsRecording(false); - onRecordingToggle({ - isRecording: false, - task: currentTask, - }); + if (onRecordingToggle) { + onRecordingToggle({ + isRecording: false, + task: currentTask, + }); + } } catch (error) { logger?.error(`CC-Widgets: Task: Error in pauseRecordingCallback - ${error.message}`, { module: 'useCallControl', @@ -610,10 +612,12 @@ export const useCallControl = (props: useCallControlProps) => { const resumeRecordingCallback = () => { try { setIsRecording(true); - onRecordingToggle({ - isRecording: true, - task: currentTask, - }); + if (onRecordingToggle) { + onRecordingToggle({ + isRecording: true, + task: currentTask, + }); + } } catch (error) { logger?.error(`CC-Widgets: Task: Error in resumeRecordingCallback - ${error.message}`, { module: 'useCallControl', @@ -648,8 +652,8 @@ export const useCallControl = (props: useCallControlProps) => { store.removeTaskCallback(TASK_EVENTS.TASK_RESUME, resumeCallback, interactionId); store.removeTaskCallback(TASK_EVENTS.TASK_END, endCallCallback, interactionId); store.removeTaskCallback(TASK_EVENTS.AGENT_WRAPPEDUP, wrapupCallCallback, interactionId); - store.removeTaskCallback(TASK_EVENTS.CONTACT_RECORDING_PAUSED, pauseRecordingCallback, interactionId); - store.removeTaskCallback(TASK_EVENTS.CONTACT_RECORDING_RESUMED, resumeRecordingCallback, interactionId); + store.removeTaskCallback(TASK_EVENTS.TASK_RECORDING_PAUSED, pauseRecordingCallback, interactionId); + store.removeTaskCallback(TASK_EVENTS.TASK_RECORDING_RESUMED, resumeRecordingCallback, interactionId); }; }, [currentTask]); diff --git a/packages/contact-center/task/tests/helper.ts b/packages/contact-center/task/tests/helper.ts index a0c63d574..9c60ce55a 100644 --- a/packages/contact-center/task/tests/helper.ts +++ b/packages/contact-center/task/tests/helper.ts @@ -5730,6 +5730,64 @@ describe('Task Hook Error Handling and Logging', () => { ); }); + it('should not throw when onRecordingToggle is not provided and pauseRecordingCallback fires', () => { + const setTaskCallbackSpy = jest.spyOn(store, 'setTaskCallback'); + + renderHook(() => + useCallControl({ + currentTask: mockTaskWithInteraction, + logger, + deviceType: 'BROWSER', + featureFlags: {}, + isMuted: false, + conferenceEnabled: false, + agentId: 'agent1', + }) + ); + + const pauseCallback = setTaskCallbackSpy.mock.calls.find( + (call) => call[0] === TASK_EVENTS.TASK_RECORDING_PAUSED + )?.[1]; + + act(() => { + pauseCallback(); + }); + + expect(logger.error).not.toHaveBeenCalledWith( + expect.stringContaining('pauseRecordingCallback'), + expect.any(Object) + ); + }); + + it('should not throw when onRecordingToggle is not provided and resumeRecordingCallback fires', () => { + const setTaskCallbackSpy = jest.spyOn(store, 'setTaskCallback'); + + renderHook(() => + useCallControl({ + currentTask: mockTaskWithInteraction, + logger, + deviceType: 'BROWSER', + featureFlags: {}, + isMuted: false, + conferenceEnabled: false, + agentId: 'agent1', + }) + ); + + const resumeCallback = setTaskCallbackSpy.mock.calls.find( + (call) => call[0] === TASK_EVENTS.TASK_RECORDING_RESUMED + )?.[1]; + + act(() => { + resumeCallback(); + }); + + expect(logger.error).not.toHaveBeenCalledWith( + expect.stringContaining('resumeRecordingCallback'), + expect.any(Object) + ); + }); + it('should handle synchronous errors in toggleRecording', () => { const errorTask = { ...mockTaskWithInteraction, From ae62a5ca976b92b20d538f64ff2368f49eb3ab0d Mon Sep 17 00:00:00 2001 From: Bharath Balan <62698609+bhabalan@users.noreply.github.com> Date: Tue, 7 Apr 2026 09:48:55 +0530 Subject: [PATCH 2/4] fix(cc-task): track hold state locally via TASK_HOLD/TASK_RESUME events The SDK fires TASK_HOLD events before updating internal task data, so getAllTasks() returns stale isHold values. This caused the hold/resume UI to not update after recording pause/resume. Track isHeld locally in useCallControl (same pattern as isRecording), driven by holdCallback/resumeCallback events, synced from task data on currentTask changes, and override controlVisibility.isHeld with this event-driven state. --- packages/contact-center/task/src/helper.ts | 23 +++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/contact-center/task/src/helper.ts b/packages/contact-center/task/src/helper.ts index 9da1b84f7..74290953c 100644 --- a/packages/contact-center/task/src/helper.ts +++ b/packages/contact-center/task/src/helper.ts @@ -16,6 +16,7 @@ import store, { getConferenceParticipants, Participant, findMediaResourceId, + findHoldStatus, MEDIA_TYPE_TELEPHONY_LOWER, } from '@webex/cc-store'; import {getControlsVisibility} from './Utils/task-util'; @@ -297,6 +298,7 @@ export const useCallControl = (props: useCallControlProps) => { agentId, } = props; const [isRecording, setIsRecording] = useState(true); + const [isHeld, setIsHeld] = useState(false); const [buddyAgents, setBuddyAgents] = useState([]); const [loadingBuddyAgents, setLoadingBuddyAgents] = useState(false); const [consultAgentName, setConsultAgentName] = useState('Consult Agent'); @@ -445,6 +447,17 @@ export const useCallControl = (props: useCallControlProps) => { } }, [currentTask, logger, lastTargetType, consultAgentName, setConsultAgentName]); + // Sync local hold state from task data when currentTask changes + useEffect(() => { + if (currentTask?.data?.interaction) { + try { + setIsHeld(findHoldStatus(currentTask, 'mainCall', agentId)); + } catch { + // findHoldStatus may fail if task data is incomplete + } + } + }, [currentTask, agentId]); + // Extract main call timestamp whenever currentTask changes useEffect(() => { extractConsultingAgent(); @@ -530,6 +543,7 @@ export const useCallControl = (props: useCallControlProps) => { const holdCallback = () => { try { + setIsHeld(true); if (onHoldResume) { onHoldResume({ isHeld: true, @@ -546,6 +560,7 @@ export const useCallControl = (props: useCallControlProps) => { const resumeCallback = () => { try { + setIsHeld(false); if (onHoldResume) { onHoldResume({ isHeld: false, @@ -931,11 +946,17 @@ export const useCallControl = (props: useCallControlProps) => { currentTask.cancelAutoWrapupTimer(); }; - const controlVisibility = useMemo( + const controlVisibilityBase = useMemo( () => getControlsVisibility(deviceType, featureFlags, currentTask, agentId, conferenceEnabled, logger), [deviceType, featureFlags, currentTask, agentId, conferenceEnabled, logger] ); + // Override isHeld with event-driven local state to avoid stale task data from getAllTasks() + const controlVisibility = useMemo( + () => ({...controlVisibilityBase, isHeld}), + [controlVisibilityBase, isHeld] + ); + // Add useEffect for auto wrap-up timer useEffect(() => { let timerId: NodeJS.Timeout; From 28233f59af17eb0a42ceae82f281b4b20e0ff2e9 Mon Sep 17 00:00:00 2001 From: Bharath Balan <62698609+bhabalan@users.noreply.github.com> Date: Tue, 7 Apr 2026 10:20:30 +0530 Subject: [PATCH 3/4] fix(cc-task): add tests for event-driven hold state and fix eslint concerns - Add 3 tests verifying controlVisibility.isHeld is driven by hold/resume callbacks rather than stale task data from getAllTasks() - Fix sync useEffect dependency to use interactionId (not currentTask ref) to prevent overwriting event-driven state on refreshTaskList - Replace empty catch block with logger.warn for better observability --- packages/contact-center/task/src/helper.ts | 17 +-- packages/contact-center/task/tests/helper.ts | 113 +++++++++++++++++++ 2 files changed, 122 insertions(+), 8 deletions(-) diff --git a/packages/contact-center/task/src/helper.ts b/packages/contact-center/task/src/helper.ts index 74290953c..f8edffd22 100644 --- a/packages/contact-center/task/src/helper.ts +++ b/packages/contact-center/task/src/helper.ts @@ -447,16 +447,20 @@ export const useCallControl = (props: useCallControlProps) => { } }, [currentTask, logger, lastTargetType, consultAgentName, setConsultAgentName]); - // Sync local hold state from task data when currentTask changes + // Sync local hold state from task data only when a different task is selected + // (not on every currentTask reference change, which would overwrite event-driven state) useEffect(() => { if (currentTask?.data?.interaction) { try { setIsHeld(findHoldStatus(currentTask, 'mainCall', agentId)); - } catch { - // findHoldStatus may fail if task data is incomplete + } catch (error) { + logger?.warn(`CC-Widgets: Task: Error syncing hold state - ${error?.message || error}`, { + module: 'useCallControl', + method: 'syncHoldState', + }); } } - }, [currentTask, agentId]); + }, [currentTask?.data?.interactionId, agentId]); // Extract main call timestamp whenever currentTask changes useEffect(() => { @@ -952,10 +956,7 @@ export const useCallControl = (props: useCallControlProps) => { ); // Override isHeld with event-driven local state to avoid stale task data from getAllTasks() - const controlVisibility = useMemo( - () => ({...controlVisibilityBase, isHeld}), - [controlVisibilityBase, isHeld] - ); + const controlVisibility = useMemo(() => ({...controlVisibilityBase, isHeld}), [controlVisibilityBase, isHeld]); // Add useEffect for auto wrap-up timer useEffect(() => { diff --git a/packages/contact-center/task/tests/helper.ts b/packages/contact-center/task/tests/helper.ts index 9c60ce55a..580e4d3f8 100644 --- a/packages/contact-center/task/tests/helper.ts +++ b/packages/contact-center/task/tests/helper.ts @@ -957,6 +957,119 @@ describe('useCallControl', () => { expect(mockOnHoldResume).toHaveBeenCalledWith({isHeld: false, task: mockCurrentTask}); }); + it('should set controlVisibility.isHeld to true when holdCallback fires', async () => { + const setTaskCallbackSpy = jest.spyOn(store, 'setTaskCallback'); + + const {result} = renderHook(() => + useCallControl({ + currentTask: mockCurrentTask, + onHoldResume: mockOnHoldResume, + onEnd: mockOnEnd, + onWrapUp: mockOnWrapUp, + logger: mockLogger, + featureFlags: store.featureFlags, + deviceType: store.deviceType, + isMuted: false, + conferenceEnabled: true, + agentId: 'test-agent-id', + }) + ); + + // Initially isHeld should be false + expect(result.current.controlVisibility.isHeld).toBe(false); + + // Find the hold callback registered via setTaskCallback + const holdCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_HOLD)?.[1]; + + act(() => { + holdCallback(); + }); + + // After holdCallback fires, isHeld should be true + expect(result.current.controlVisibility.isHeld).toBe(true); + }); + + it('should set controlVisibility.isHeld to false when resumeCallback fires', async () => { + const setTaskCallbackSpy = jest.spyOn(store, 'setTaskCallback'); + + const {result} = renderHook(() => + useCallControl({ + currentTask: mockCurrentTask, + onHoldResume: mockOnHoldResume, + onEnd: mockOnEnd, + onWrapUp: mockOnWrapUp, + logger: mockLogger, + featureFlags: store.featureFlags, + deviceType: store.deviceType, + isMuted: false, + conferenceEnabled: true, + agentId: 'test-agent-id', + }) + ); + + // First hold the call + const holdCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_HOLD)?.[1]; + act(() => { + holdCallback(); + }); + expect(result.current.controlVisibility.isHeld).toBe(true); + + // Then resume — isHeld should go back to false + const resumeCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_RESUME)?.[1]; + act(() => { + resumeCallback(); + }); + + expect(result.current.controlVisibility.isHeld).toBe(false); + }); + + it('should override getControlsVisibility isHeld with local event-driven state', async () => { + // Mock getControlsVisibility to return isHeld: true (simulating stale task data) + mockGetControlsVisibility.mockReturnValue({ + ...mockGetControlsVisibility.mock.results[0]?.value, + muteUnmute: {isVisible: true, isEnabled: true}, + holdResume: {isVisible: true, isEnabled: true}, + end: {isVisible: true, isEnabled: true}, + isHeld: true, + wrapup: {isVisible: false, isEnabled: false}, + }); + + const setTaskCallbackSpy = jest.spyOn(store, 'setTaskCallback'); + + const {result} = renderHook(() => + useCallControl({ + currentTask: mockCurrentTask, + onHoldResume: mockOnHoldResume, + onEnd: mockOnEnd, + onWrapUp: mockOnWrapUp, + logger: mockLogger, + featureFlags: store.featureFlags, + deviceType: store.deviceType, + isMuted: false, + conferenceEnabled: true, + agentId: 'test-agent-id', + }) + ); + + // Even though getControlsVisibility returns isHeld: true, + // the local state starts as false, so controlVisibility.isHeld should be false + expect(result.current.controlVisibility.isHeld).toBe(false); + + // After holdCallback, it should be true (event-driven, not from getControlsVisibility) + const holdCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_HOLD)?.[1]; + act(() => { + holdCallback(); + }); + expect(result.current.controlVisibility.isHeld).toBe(true); + + // After resumeCallback, it should be false again + const resumeCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_RESUME)?.[1]; + act(() => { + resumeCallback(); + }); + expect(result.current.controlVisibility.isHeld).toBe(false); + }); + it('should log an error if hold fails', async () => { mockCurrentTask.hold.mockRejectedValueOnce(new Error('Hold error')); From 3bcf6afb82f6c562d8331962d38ecd6d44220e27 Mon Sep 17 00:00:00 2001 From: Bharath Balan <62698609+bhabalan@users.noreply.github.com> Date: Thu, 9 Apr 2026 01:22:24 +0530 Subject: [PATCH 4/4] fix(cc-task): address PR review - use controlVisibilityBase instead of findHoldStatus - Remove findHoldStatus import per reviewer feedback; reuse value already computed by getControlsVisibility via controlVisibilityBase - Add null safety (optional chaining + fallback) for controlVisibilityBase - Update override test to verify event-driven callbacks override base value --- packages/contact-center/task/src/helper.ts | 15 +++------------ packages/contact-center/task/tests/helper.ts | 20 ++++++++------------ 2 files changed, 11 insertions(+), 24 deletions(-) diff --git a/packages/contact-center/task/src/helper.ts b/packages/contact-center/task/src/helper.ts index f8edffd22..64d14b62f 100644 --- a/packages/contact-center/task/src/helper.ts +++ b/packages/contact-center/task/src/helper.ts @@ -16,7 +16,6 @@ import store, { getConferenceParticipants, Participant, findMediaResourceId, - findHoldStatus, MEDIA_TYPE_TELEPHONY_LOWER, } from '@webex/cc-store'; import {getControlsVisibility} from './Utils/task-util'; @@ -447,19 +446,11 @@ export const useCallControl = (props: useCallControlProps) => { } }, [currentTask, logger, lastTargetType, consultAgentName, setConsultAgentName]); - // Sync local hold state from task data only when a different task is selected + // Sync local hold state from controlVisibilityBase only when a different task is selected // (not on every currentTask reference change, which would overwrite event-driven state) useEffect(() => { - if (currentTask?.data?.interaction) { - try { - setIsHeld(findHoldStatus(currentTask, 'mainCall', agentId)); - } catch (error) { - logger?.warn(`CC-Widgets: Task: Error syncing hold state - ${error?.message || error}`, { - module: 'useCallControl', - method: 'syncHoldState', - }); - } - } + setIsHeld(controlVisibilityBase?.isHeld ?? false); + // eslint-disable-next-line react-hooks/exhaustive-deps }, [currentTask?.data?.interactionId, agentId]); // Extract main call timestamp whenever currentTask changes diff --git a/packages/contact-center/task/tests/helper.ts b/packages/contact-center/task/tests/helper.ts index 580e4d3f8..9d9d81099 100644 --- a/packages/contact-center/task/tests/helper.ts +++ b/packages/contact-center/task/tests/helper.ts @@ -1023,8 +1023,8 @@ describe('useCallControl', () => { expect(result.current.controlVisibility.isHeld).toBe(false); }); - it('should override getControlsVisibility isHeld with local event-driven state', async () => { - // Mock getControlsVisibility to return isHeld: true (simulating stale task data) + it('should override getControlsVisibility isHeld with event-driven state from callbacks', async () => { + // Mock getControlsVisibility to return isHeld: true (simulating stale task data after refreshTaskList) mockGetControlsVisibility.mockReturnValue({ ...mockGetControlsVisibility.mock.results[0]?.value, muteUnmute: {isVisible: true, isEnabled: true}, @@ -1051,23 +1051,19 @@ describe('useCallControl', () => { }) ); - // Even though getControlsVisibility returns isHeld: true, - // the local state starts as false, so controlVisibility.isHeld should be false + // resumeCallback should override getControlsVisibility isHeld: true with false + const resumeCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_RESUME)?.[1]; + act(() => { + resumeCallback(); + }); expect(result.current.controlVisibility.isHeld).toBe(false); - // After holdCallback, it should be true (event-driven, not from getControlsVisibility) + // holdCallback should set it back to true const holdCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_HOLD)?.[1]; act(() => { holdCallback(); }); expect(result.current.controlVisibility.isHeld).toBe(true); - - // After resumeCallback, it should be false again - const resumeCallback = setTaskCallbackSpy.mock.calls.find((call) => call[0] === TASK_EVENTS.TASK_RESUME)?.[1]; - act(() => { - resumeCallback(); - }); - expect(result.current.controlVisibility.isHeld).toBe(false); }); it('should log an error if hold fails', async () => {