Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions packages/contact-center/task/src/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@
agentId,
} = props;
const [isRecording, setIsRecording] = useState(true);
const [isHeld, setIsHeld] = useState(false);
const [buddyAgents, setBuddyAgents] = useState<BuddyDetails[]>([]);
const [loadingBuddyAgents, setLoadingBuddyAgents] = useState(false);
const [consultAgentName, setConsultAgentName] = useState<string>('Consult Agent');
Expand Down Expand Up @@ -445,6 +446,13 @@
}
}, [currentTask, logger, lastTargetType, consultAgentName, setConsultAgentName]);

// 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(() => {
setIsHeld(controlVisibilityBase?.isHeld ?? false);
// eslint-disable-next-line react-hooks/exhaustive-deps

Check failure on line 453 in packages/contact-center/task/src/helper.ts

View workflow job for this annotation

GitHub Actions / linter

Definition for rule 'react-hooks/exhaustive-deps' was not found
}, [currentTask?.data?.interactionId, agentId]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-run hold-state sync when task payload changes

The new hold sync effect only depends on currentTask?.data?.interactionId, so it runs once per interaction and never retries when the same interaction is refreshed with newer data. In the exact incomplete-payload case this block already anticipates (the findHoldStatus try/catch), a later refreshTaskList update for the same interaction ID will not re-sync isHeld, and the stale local value will keep overriding getControlsVisibility. Please include hold-relevant task data (or currentTask) in the dependencies so local state can recover when the task object is hydrated.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentionally keeping interactionId as the dependency rather than currentTask. The root cause of this bug is that the SDK fires TASK_HOLD/TASK_RESUME events before updating its internal task data, so refreshTaskListgetAllTasks() returns stale isHold: false. Adding currentTask back to the deps would re-run the sync on every refreshTaskList and overwrite the event-driven state — reintroducing the original bug. The event callbacks (holdCallback/resumeCallback) are the source of truth during a call; this effect only seeds the initial value on task switch.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-sync held state when task payload is refreshed

This effect only depends on currentTask?.data?.interactionId and agentId, so it never runs when the same interaction is refreshed with newer task data (for example, after refreshTaskList hydration), leaving local isHeld stale while it continues to override getControlsVisibility. I re-checked the current head and confirmed controlVisibilityBase is recomputed from currentTask each render, but this sync path ignores those updates because exhaustive deps are explicitly suppressed.

Useful? React with 👍 / 👎.


// Extract main call timestamp whenever currentTask changes
useEffect(() => {
extractConsultingAgent();
Expand Down Expand Up @@ -530,6 +538,7 @@

const holdCallback = () => {
try {
setIsHeld(true);
if (onHoldResume) {
onHoldResume({
isHeld: true,
Expand All @@ -546,6 +555,7 @@

const resumeCallback = () => {
try {
setIsHeld(false);
if (onHoldResume) {
onHoldResume({
isHeld: false,
Expand Down Expand Up @@ -595,10 +605,12 @@
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',
Expand All @@ -610,10 +622,12 @@
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',
Expand Down Expand Up @@ -648,8 +662,8 @@
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]);

Expand Down Expand Up @@ -927,11 +941,14 @@
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]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Recompute dependent controls using local hold state

Overriding only controlVisibility.isHeld creates inconsistent UI state whenever local isHeld diverges from task-derived hold status (the stale-payload race this patch is handling). getControlsVisibility(...) already computed holdResume.isEnabled/end.isEnabled from the old hold value, while the button icon/action uses the overridden isHeld in cc-components (call-control.utils.ts uses controlVisibility.isHeld for icon/tooltip but controlVisibility.holdResume.isEnabled for disabled). In conference+consult-completed flows this can render a “Resume” hold button that stays disabled, blocking recovery.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Recompute hold-dependent controls after overriding isHeld

Overriding only controlVisibility.isHeld leaves all other hold-derived flags (holdResume.isEnabled, end.isEnabled, etc.) based on stale task data, so the UI can become internally inconsistent when local hold state diverges from task-derived state. Fresh evidence on current head: in cc-components/src/components/task/CallControl/call-control.utils.ts, the hold icon/tooltip use controlVisibility.isHeld while disabled state still uses controlVisibility.holdResume.isEnabled, which can render a “Resume” button that remains disabled in consult/conference paths.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Recompute hold-dependent controls after overriding isHeld

The new controlVisibility override only replaces isHeld ({...controlVisibilityBase, isHeld}), but holdResume.isEnabled, end.isEnabled, and other flags in controlVisibilityBase were already computed from the old task-derived hold value. In consult/conference states where enablement depends on isHeld, this creates internally inconsistent UI (e.g., the button renders "Resume" from isHeld but stays disabled from stale holdResume.isEnabled), which can block recovery actions after hold/resume events.

Useful? React with 👍 / 👎.


// Add useEffect for auto wrap-up timer
useEffect(() => {
let timerId: NodeJS.Timeout;
Expand Down
167 changes: 167 additions & 0 deletions packages/contact-center/task/tests/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,115 @@ 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 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},
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',
})
);

// 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);

// 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);
});

it('should log an error if hold fails', async () => {
mockCurrentTask.hold.mockRejectedValueOnce(new Error('Hold error'));

Expand Down Expand Up @@ -5730,6 +5839,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,
Expand Down
Loading