fix(store): pass task object directly to setTaskCallback/removeTaskCallback to prevent duplicate event listeners - #723
fix(store): pass task object directly to setTaskCallback/removeTaskCallback to prevent duplicate event listeners#723brain-frog wants to merge 5 commits into
Conversation
…to prevent duplicate event listeners - Changed setTaskCallback and removeTaskCallback signatures to accept ITask instead of taskId string, eliminating volatile store.taskList lookup that caused orphaned listeners during React 18 StrictMode mount/unmount cycles - Added diagnostic logging with optional chaining on logger - Updated useIncomingTask and useCallControl hooks to pass task objects - Fixed leaking spy in useIncomingTask test (mockRestore) - Added jest.restoreAllMocks() in useCallControl beforeEach - Added regression test for removal when task absent from store.taskList CAI-8283
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 340bf53809
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!callback) return; | ||
| const task = this.store.taskList[taskId]; | ||
| if (!task) return; | ||
| setTaskCallback = (event: TASK_EVENTS, callback, task: ITask) => { |
There was a problem hiding this comment.
Existing @webex/cc-store consumers can still call setTaskCallback(event, cb, interactionId) because this singleton is a published/default package surface, and this change now treats that string as an ITask: the guard passes, the log prints an undefined id, and the call then fails at task.on(...) instead of registering the listener. Since this PR is described as non-breaking and only updates the in-repo callers, keep accepting the old string-id form (or add a separate new API) so already-published widgets/hosts do not crash after upgrading; the same compatibility issue applies to removeTaskCallback.
Useful? React with 👍 / 👎.
…ack signature change - Added STORE-R-022 requirement for ITask-based signatures - Updated test strategy with regression test coverage - Updated task spec overview, data flow, pitfalls, and do's/don'ts - Reflects CAI-8283 fix for duplicate event listeners CAI-8283
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d284c97064
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| store.setTaskCallback(TASK_EVENTS.TASK_WRAPPEDUP, wrapupCallCallback, interactionId); | ||
| store.setTaskCallback(TASK_EVENTS.TASK_RECORDING_PAUSED, pauseRecordingCallback, interactionId); | ||
| store.setTaskCallback(TASK_EVENTS.TASK_RECORDING_RESUMED, resumeRecordingCallback, interactionId); | ||
| store.setTaskCallback(TASK_EVENTS.TASK_HOLD, holdCallback, currentTask); |
There was a problem hiding this comment.
When CallControl is driven by store.currentTask, this passes the store-exposed task into task.on, but setCurrentTask stores a cloned object (Object.assign(Object.create(Object.getPrototypeOf(task)), task)) rather than the SDK task instance held in taskList. The SDK/store task events are registered and emitted on the original task objects, so active-call callbacks such as onHoldResume, onEnd, and recording toggles can stop firing when listeners are attached to the clone; capture the original task reference for registration and reuse that same reference for cleanup.
Useful? React with 👍 / 👎.
- Fix removeTaskCallback guard test to assert on mockTask.off instead of mockTask.on so the assertion actually catches a regression - Verify setTaskCallback receives the correct task object (not just any function) in the useCallControl event-listener test - Update react-patterns.md example to pass the ITask object instead of the stale interactionId string to setTaskCallback/removeTaskCallback
1 participant
COMPLETES # https://jira-eng-sjc12.cisco.com/jira/browse/CAI-8283
https://jira-eng-sjc12.cisco.com/jira/browse/CAI-8283
This pull request addresses
Getting duplicate task acceptances
< DESCRIBE THE CONTEXT OF THE ISSUE >
by making the following changes
< DESCRIBE YOUR CHANGES >
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging
Make sure to have followed the contributing guidelines before submitting.