-
Notifications
You must be signed in to change notification settings - Fork 54
feat(widgets): added-transfer-functionality #423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 📝 WalkthroughWalkthroughThe changes introduce enhancements to the call control functionality in the contact-center project. New React functional components, such as Possibly related PRs
🪧 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
Documentation and Community
|
PR looks big as there are yarn-lock changes. |
@@ -0,0 +1,59 @@ | |||
import React from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the new list item here. Open to any suggestions.
onAgentSelect: (agentId: string) => void; | ||
} | ||
|
||
const CallControlPopoverComponent: React.FC<CallControlPopoverComponentProps> = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Popover for both consult and transfer to be used - ported from the old PR exactly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need two components if the list is only used inside this popover? Can't they be part of this popover component itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible we use the list in the future elsewhere... I also wanted to keep it different ebcause we might add checkbox later on to the list whereas the popover will house the search component.
Keeping them together would clutter up the popover and increase the file size... so I kept it like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checkbox on the list for what?
I anyway agree with the file size. So this is good to resolve. Just need to understand the checkbox part
@@ -234,17 +235,14 @@ class StoreWrapper implements IStoreWrapper { | |||
}); | |||
}; | |||
|
|||
handleTaskEnd = (event) => { | |||
handleTaskEnd = (event, wrapupRequired) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified this, to handle all types of task ends, checked and it works with all scenarios.
This pull request is automatically being deployed by Amplify Hosting (learn more). |
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (20)
packages/contact-center/task/tests/helper.ts (1)
1110-1130
: LGTM on transfer call test.This test effectively validates that the
transferCall
method correctly invokes the transfer functionality with the appropriate parameters.One suggestion: consider adding a test case for error handling in the
transferCall
method, similar to the other error handling tests in this file.+ it('should handle errors when transferCall fails', async () => { + const transferSpy = jest.fn().mockRejectedValue(new Error('Transfer error')); + const currentTaskWithError = {...mockCurrentTask, transfer: transferSpy}; + const {result} = renderHook(() => + useCallControl({ + currentTask: currentTaskWithError, + onHoldResume: mockOnHoldResume, + onEnd: mockOnEnd, + onWrapUp: mockOnWrapUp, + logger: mockLogger, + deviceType: 'BROWSER', + }) + ); + await act(async () => { + await result.current.transferCall('test_id', 'agent'); + }); + expect(transferSpy).toHaveBeenCalledWith({ + to: 'test_id', + destinationType: 'agent', + }); + expect(mockLogger.error).toHaveBeenCalledWith( + 'Error transferring call: Error: Transfer error', + expect.any(Object) + ); + });packages/contact-center/cc-components/src/components/task/CallControl/CallControlCustom/call-control-list-item.tsx (1)
1-60
: Well-structured reusable component for call control functionality.This component is well-designed for displaying agent information with an action button, which aligns with the transfer functionality mentioned in the PR objectives. The code is clean and follows React best practices.
Consider these minor improvements for better maintainability:
const CallControlListItemComponent: React.FC<CallControlListItemComponentProps> = (props) => { const {title, subtitle, buttonIcon, onButtonPress, className} = props; + // Constants for sizing + const AVATAR_SIZE = 32; + const BUTTON_SIZE = 28; + const initials = title .split(' ') .map((word) => word[0]) .join('') .slice(0, 2) .toUpperCase(); return ( <ListItemBase className={classnames('call-control-list-item', className)} size={50} isPadded aria-label={title}> <ListItemBaseSection position="start"> - <AvatarNext size={32} initials={initials} title={title} /> + <AvatarNext size={AVATAR_SIZE} initials={initials} title={title} /> </ListItemBaseSection> <ListItemBaseSection position="middle" style={{ flex: 1, display: 'flex', flexDirection: 'column', marginLeft: '8px', minWidth: 0, overflow: 'hidden', }} > <Text tagName="p" type="body-primary" style={{margin: 0, lineHeight: '1.2'}}> {title} </Text> {subtitle && ( <Text tagName="p" type="body-secondary" style={{margin: 0, lineHeight: '1.2'}}> {subtitle} </Text> )} </ListItemBaseSection> <ListItemBaseSection position="end"> <div className="hover-button"> - <ButtonCircle onPress={onButtonPress} size={28} color="join"> + <ButtonCircle onPress={onButtonPress} size={BUTTON_SIZE} color="join"> <Icon name={buttonIcon} /> </ButtonCircle> </div> </ListItemBaseSection> </ListItemBase> ); };packages/contact-center/cc-components/src/components/task/CallControl/CallControlCustom/call-control-popover.tsx (4)
19-19
: Clarify the purpose of thefilteredAgents
variable.The variable
filteredAgents
is simply assigned tobuddyAgents
without any actual filtering being performed, which could be confusing to other developers.- const filteredAgents = buddyAgents; + // Use buddyAgents directly since no filtering is currently appliedAlternatively, if filtering is planned for the future:
- const filteredAgents = buddyAgents; + // TODO: Implement filtering logic when search functionality is added + const filteredAgents = buddyAgents; // Will be replaced with filtered results in the future
26-36
: Consider enhancing TabList for future extensibility.The TabList component is implemented but currently only contains a single 'Agents' tab. If additional tabs are planned for the future (like "Queues" or "Teams"), consider adding a comment to indicate this intention.
<TabListNext aria-label="Tabs" className="agent-tablist" hasBackground={false} style={{marginTop: '0'}} onTabSelection={(key) => setSelectedTab(key as string)} > <TabNext key="Agents" className="agent-tab" active={selectedTab === 'Agents'}> Agents </TabNext> + {/* Additional tabs like "Queues" or "Teams" will be added in future iterations */} </TabListNext>
41-42
: Add a comment explaining the event propagation control.The
onMouseDown
handler withe.stopPropagation()
is used to prevent the popover from closing when clicking on an agent item. Adding a comment would make this intention clear to future developers.<div key={agent.agentId} - onMouseDown={(e) => e.stopPropagation()} + onMouseDown={(e) => e.stopPropagation()} // Prevent popover from closing when clicking on an agent style={{cursor: 'pointer', pointerEvents: 'auto'}} >
5-10
: Add JSDoc comments to the props interface.Adding JSDoc comments to the interface would improve code documentation and provide better tooltips in IDEs.
+/** + * Props for the CallControlPopoverComponent + */ export interface CallControlPopoverComponentProps { + /** The title displayed at the top of the popover */ heading: string; + /** Icon name for the button displayed next to each agent */ buttonIcon: string; + /** List of available agents that can be selected */ buddyAgents: Array<{agentId: string; agentName: string; dn: string}>; + /** Callback function triggered when an agent is selected */ onAgentSelect: (agentId: string) => void; }packages/contact-center/cc-components/tests/components/task/TaskList/task-list.tsx (1)
18-18
: Update test description to match the component name.The test description still references "TaskListPresentational Component" even though the component has been renamed to "TaskListComponent".
-describe('TaskListPresentational Component', () => { +describe('TaskListComponent', () => {packages/contact-center/store/src/storeEventsWrapper.ts (2)
238-248
: Add TypeScript type annotation forwrapupRequired
parameter.The
wrapupRequired
parameter in thehandleTaskEnd
method doesn't have a type annotation. Adding one would improve type safety and documentation.- handleTaskEnd = (event, wrapupRequired) => { + handleTaskEnd = (event, wrapupRequired: boolean) => {
357-367
: Well-implementedgetBuddyAgents
method with proper error handling.The new
getBuddyAgents
method is implemented correctly with proper error handling using try/catch and Promise rejection.However, consider making the hard-coded filter parameters configurable through optional function parameters:
- getBuddyAgents = async (): Promise<Array<BuddyDetails>> => { + getBuddyAgents = async ( + options: { mediaType?: string; state?: string } = { mediaType: 'telephony', state: 'Available' } + ): Promise<Array<BuddyDetails>> => { try { const response = await this.store.cc.getBuddyAgents({ - mediaType: 'telephony', - state: 'Available', + mediaType: options.mediaType, + state: options.state, }); return response.data.agentList; } catch (error) { return Promise.reject(error); } };This would provide more flexibility for future use cases while maintaining the current default behavior.
packages/contact-center/cc-components/tests/components/task/CallControl/call-control-list-item.tsx (2)
31-37
: Consider a less aggressive approach to error suppression.The test is currently suppressing all console errors globally, which might hide actual issues. Consider a more targeted approach or removing the suppression entirely to catch potential real errors.
-beforeAll(() => { - jest.spyOn(console, 'error').mockImplementation(() => {}); -}); - -afterAll(() => { - (console.error as jest.Mock).mockRestore(); -}); +// Only suppress specific expected errors if needed +const originalConsoleError = console.error; +beforeAll(() => { + console.error = jest.fn((...args) => { + // Only suppress specific expected errors, e.g., prop validation warnings + const errorMessage = args[0]?.toString() || ''; + if (errorMessage.includes('expected ReactPropTypes')) { + return; + } + originalConsoleError(...args); + }); +}); + +afterAll(() => { + console.error = originalConsoleError; +});
39-46
: Consider adding tests for edge cases.The tests cover the basic functionality well, but consider adding tests for edge cases such as:
- Empty title handling
- Very long titles or subtitles
- Missing buttonIcon
- Missing onButtonPress callback
This would ensure the component behaves correctly under all circumstances.
packages/contact-center/cc-components/src/components/task/CallControl/call-control.tsx (4)
13-14
: Clear State Management
Using separate states (showAgentMenu
andagentMenuType
) is a neat approach. You might consider resetting them once the call ends or the component unmounts to prevent stale states in subsequent interactions.
75-81
: Introduce Consult Button
You could optionally disable this button whenbuddyAgents
is empty to avoid user confusion.disabled: false, +// Optionally disable if there are no buddy agents +disabled: buddyAgents.length === 0,
82-88
: Introduce Transfer Button
Similarly, consider disabling the button if no buddy agents are available or if certain conditions (e.g., call state) make a transfer invalid.
113-174
: Popover Logic
This conditional popover flow for consult/transfer is quite verbose. Extracting the popover logic into a dedicated component or custom hook could improve readability and maintainability.packages/contact-center/cc-components/tests/components/task/CallControl/call-control.tsx (1)
109-114
: Toggle Hold Test
Ensures thattoggleHold
is called with the correct bool value when the hold button is clicked. You could optionally check for corresponding UI changes if needed.packages/contact-center/task/src/helper.ts (3)
134-142
: loadBuddyAgents Function
Logs the error and resetsbuddyAgents
if fetching fails. Consider offering user-visible feedback for a better experience if the load is critical to call flow.
272-283
: transferCall Implementation
This asynchronous approach with error logging is good. Adding an in-app notification upon failure could further improve UX.
285-286
: Empty consultCall
The consult logic is not yet implemented. If this is future work, a TODO comment or a simple placeholder would clarify the intention for other contributors.Do you want me to generate a stub implementation or create a follow-up issue to track this?
packages/contact-center/cc-components/src/components/task/CallControl/call-control.styles.scss (1)
170-173
: Consider avoiding !important declarations if possible.While the
.custom-text
class accomplishes its purpose of overriding margin and line-height, the use of!important
should be approached with caution as it can make styles harder to maintain.Consider using more specific selectors or restructuring the CSS hierarchy to achieve the same result without
!important
if possible.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (17)
packages/contact-center/cc-components/src/components/task/CallControl/CallControlCustom/call-control-list-item.tsx
(1 hunks)packages/contact-center/cc-components/src/components/task/CallControl/CallControlCustom/call-control-popover.tsx
(1 hunks)packages/contact-center/cc-components/src/components/task/CallControl/call-control.styles.scss
(2 hunks)packages/contact-center/cc-components/src/components/task/CallControl/call-control.tsx
(5 hunks)packages/contact-center/cc-components/src/components/task/task.types.ts
(3 hunks)packages/contact-center/cc-components/tests/components/StationLogin/station-login.tsx
(3 hunks)packages/contact-center/cc-components/tests/components/task/CallControl/call-control-list-item.tsx
(1 hunks)packages/contact-center/cc-components/tests/components/task/CallControl/call-control-popover.tsx
(1 hunks)packages/contact-center/cc-components/tests/components/task/CallControl/call-control.tsx
(3 hunks)packages/contact-center/cc-components/tests/components/task/IncomingTask/incoming-task.tsx
(3 hunks)packages/contact-center/cc-components/tests/components/task/TaskList/task-list.tsx
(3 hunks)packages/contact-center/store/package.json
(2 hunks)packages/contact-center/store/src/store.types.ts
(2 hunks)packages/contact-center/store/src/storeEventsWrapper.ts
(6 hunks)packages/contact-center/store/tests/storeEventsWrapper.ts
(4 hunks)packages/contact-center/task/src/helper.ts
(4 hunks)packages/contact-center/task/tests/helper.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
packages/contact-center/cc-components/tests/components/StationLogin/station-login.tsx (3)
Learnt from: Kesari3008
PR: webex/widgets#239
File: packages/contact-center/station-login/tests/station-login/station-login.presentational.tsx:0-0
Timestamp: 2025-03-31T08:21:44.298Z
Learning: The file `packages/contact-center/station-login/tests/station-login/station-login.presentational.tsx` is going to change completely, so writing tests for these changes are being ignored.
Learnt from: Kesari3008
PR: webex/widgets#239
File: packages/contact-center/station-login/src/station-login/station-login.presentational.tsx:0-0
Timestamp: 2025-03-31T08:21:44.298Z
Learning: In `packages/contact-center/station-login/src/station-login/station-login.presentational.tsx`, certain code sections are temporary or removable, and comments regarding these sections can be ignored in future reviews.
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/index.tsx:10-10
Timestamp: 2025-03-31T08:21:44.298Z
Learning: In the StationLogin component (packages/contact-center/station-login/src/station-login/index.tsx), `handleContinue` and `showMultipleLoginAlert` props have been intentionally removed from store destructuring as part of the Agent Multi-Login configurability changes.
packages/contact-center/cc-components/tests/components/task/CallControl/call-control-list-item.tsx (1)
Learnt from: Shreyas281299
PR: webex/widgets#362
File: packages/contact-center/task/tests/CallControl/call-control.presentational.tsx:0-0
Timestamp: 2025-03-31T08:21:44.298Z
Learning: UI testing is not mandatory for React components in the webex/widgets repository.
packages/contact-center/cc-components/tests/components/task/CallControl/call-control.tsx (2)
Learnt from: Shreyas281299
PR: webex/widgets#362
File: packages/contact-center/task/tests/CallControl/call-control.presentational.tsx:0-0
Timestamp: 2025-03-31T08:21:44.298Z
Learning: UI testing is not mandatory for React components in the webex/widgets repository.
Learnt from: Shreyas281299
PR: webex/widgets#362
File: packages/contact-center/task/tests/CallControl/index.tsx:29-46
Timestamp: 2025-03-31T08:21:44.298Z
Learning: UI testing is not mandatory for components in the @webex/widgets repository. Focus should be on functional testing rather than UI element verification.
🧬 Code Definitions (6)
packages/contact-center/cc-components/tests/components/StationLogin/station-login.tsx (1)
packages/contact-center/cc-components/src/index.ts (1)
StationLoginComponent
(10-10)
packages/contact-center/cc-components/tests/components/task/IncomingTask/incoming-task.tsx (1)
packages/contact-center/cc-components/src/index.ts (1)
IncomingTaskComponent
(12-12)
packages/contact-center/cc-components/src/components/task/task.types.ts (1)
packages/contact-center/store/src/store.types.ts (2)
BuddyDetails
(134-134)DestinationType
(133-133)
packages/contact-center/cc-components/src/components/task/CallControl/CallControlCustom/call-control-popover.tsx (1)
packages/contact-center/store/src/storeEventsWrapper.ts (1)
agentId
(44-46)
packages/contact-center/store/src/storeEventsWrapper.ts (1)
packages/contact-center/store/src/store.types.ts (2)
TASK_EVENTS
(137-137)BuddyDetails
(134-134)
packages/contact-center/cc-components/tests/components/task/CallControl/call-control.tsx (1)
packages/contact-center/cc-components/src/index.ts (1)
CallControlComponent
(11-11)
🔇 Additional comments (41)
packages/contact-center/store/package.json (1)
26-26
:❓ Verification inconclusive
Verify compatibility with the newer Webex SDK version.
You've updated the Webex dependency from a "-wxcc" suffixed version to a "-next" version, which suggests moving from a custom build to a pre-release version. While this aligns with the PR objective to upgrade the CC SDK, make sure this pre-release version is stable enough for production use.
🏁 Script executed:
#!/bin/bash # Check if other places in the codebase might be using the older version rg -A 2 -B 2 "3.7.0-wxcc.19" --type js --type ts --type json # Check for potential compatibility issues with the new version rg -A 2 -B 2 "@webex/plugin-cc" --type tsLength of output: 2641
Compatibility Check: Validate Webex SDK Pre-release Stability
We've verified that there are no remaining references to the old "-wxcc" version (e.g., "3.7.0-wxcc.19") in the codebase. However, several modules still import from
@webex/plugin-cc
(e.g., inpackages/contact-center/task/src/helper.ts
,packages/contact-center/station-login/src/helper.ts
, andpackages/contact-center/store/*
). Please double-check that these usages remain compatible with the updated dependency"webex": "3.8.0-next.20"
, and confirm through integration tests and a review of the Webex SDK release notes that no breaking API changes will affect production functionality.packages/contact-center/store/src/store.types.ts (2)
1-1
: LGTM!The import statement has been expanded to include the new types
BuddyDetails
andDestinationType
that will be used for the transfer functionality implementation.
133-134
: LGTM!You've correctly exposed the newly imported types
DestinationType
andBuddyDetails
from the module, making them available for other components to use in the transfer and consultation feature implementation.packages/contact-center/cc-components/tests/components/task/CallControl/call-control-popover.tsx (3)
1-35
: LGTM on test setup.The test file is well-structured with appropriate mocks for external dependencies, making the component tests more focused and isolated.
36-63
: LGTM on basic rendering tests.The tests effectively verify the rendering behavior of the component, including checks for proper display of the heading and agent list.
64-74
: LGTM on edge case and interaction tests.Good job testing both the empty state and the interaction behavior. The test verifies that the
onAgentSelect
callback is triggered with the correct agent ID when an agent is selected.packages/contact-center/task/tests/helper.ts (1)
1087-1108
: LGTM on buddy agents loading test.This test properly verifies that the
loadBuddyAgents
method correctly fetches and updates the state with buddy agents from the store.packages/contact-center/cc-components/tests/components/task/IncomingTask/incoming-task.tsx (1)
4-4
: Component naming standardization looks good.The renaming from
IncomingTaskPresentational
toIncomingTaskComponent
is consistent with the broader refactoring pattern seen across the codebase, aligning with the same rename pattern observed in other components.Also applies to: 17-17, 43-43
packages/contact-center/cc-components/tests/components/StationLogin/station-login.tsx (1)
3-3
: Component naming standardization looks good.The renaming from
StationLoginPresentational
toStationLoginComponent
is consistent with the broader refactoring pattern seen across the codebase, maintaining alignment with other component test files.Also applies to: 18-18, 54-54
packages/contact-center/store/tests/storeEventsWrapper.ts (2)
434-444
: Well-structured tests for the new getBuddyAgents functionality.These tests properly verify the successful retrieval of buddy agents and error handling, which aligns with the transfer functionality mentioned in the PR objectives.
628-628
:✅ Verification successful
Verify changes to wrapupRequired behavior.
The test expectations for
setWrapupRequired
have been changed fromtrue
tofalse
(line 628) and fromfalse
totrue
(lines 637, 646). These changes should be verified to ensure they align with the updated business logic for call transfers.Also applies to: 637-637, 646-646
🏁 Script executed:
#!/bin/bash # Check implementation to verify these expectation changes are correct # Look for the handleTaskEnd implementation that sets wrapupRequired rg -A 10 "handleTaskEnd" --type ts packages/contact-center/store/src/Length of output: 4727
Wrapup behavior verified as intended.
The changes in test expectations are aligned with the updated business logic:
- In the task removal flow,
setWrapupRequired(false)
is invoked (line 628).- In the
handleTaskEnd
function, when thewrapupRequired
flag is true,setWrapupRequired(true)
is correctly called (verified by the tests at lines 637 and 646).No further changes are needed.
packages/contact-center/cc-components/tests/components/task/TaskList/task-list.tsx (1)
4-5
: Component and props imports have been correctly updated.The imports have been properly updated to reference the new component name and props type, which is good for maintaining consistency across the codebase.
packages/contact-center/store/src/storeEventsWrapper.ts (3)
213-213
: Updated event handler to correctly pass thewrapupRequired
parameter.The event handler now correctly passes the
wrapupRequired
parameter to thehandleTaskEnd
method, which is consistent with the method signature changes.
275-276
: Consistent update toTASK_END
event handler.The event handler for
TASK_END
has been updated consistently to pass thewrapupRequired
parameter, which is good for maintaining code consistency.
309-309
: Consistent update toTASK_END
event handler inhandleTaskHydrate
.The event handler for
TASK_END
in thehandleTaskHydrate
method has been updated consistently with the other instances, ensuring uniform behavior across the codebase.packages/contact-center/cc-components/tests/components/task/CallControl/call-control-list-item.tsx (3)
53-59
: Good test coverage for component rendering.The test properly verifies that the component renders with the expected title, subtitle, and correct initials derived from the title.
61-67
: Good test for conditional rendering.The test appropriately checks that the subtitle is not rendered when not provided in the props, which is important for verifying conditional rendering behavior.
69-74
: Well-implemented event handler test.The test correctly verifies that the
onButtonPress
callback is called when the button is clicked, ensuring that the component's interactive behavior works as expected.packages/contact-center/cc-components/src/components/task/CallControl/call-control.tsx (3)
8-8
: Import Looks Good
The import path forCallControlPopoverComponent
is correctly set. No issues here.
29-32
: Prop Integration
The newly introduced props (buddyAgents
,loadBuddyAgents
,transferCall
, andconsultCall
) align well with the new feature requirements. Please confirm these are thoroughly tested for various call scenarios, including edge cases.
196-218
: Fallback Button Rendering
Returning standard buttons in the else-branch is straightforward. The tooltips improve usability and accessibility.packages/contact-center/cc-components/tests/components/task/CallControl/call-control.tsx (11)
1-1
: ESLint Rule Disable
Disablingreact/prop-types
is acceptable here, especially when relying on TypeScript for type checking.
3-3
: Testing Library Import
Bringing in the testing library modules is standard practice.
5-5
: Import CallControlComponent
No issues with this import path.
7-33
: Mocking Momentum UI
These mocks ensure the tests are isolated from the actual UI library. For broader coverage, consider adding integration tests with real components if desired.
40-45
: Mocking DestinationType
MockingDestinationType
enables focused testing without depending on the real store module.
47-57
: Mocking CallControlPopover
Simulating agent selection in this mock is a good approach for validating consult/transfer flows in isolation.
68-70
: Mock Functions
mockLoadBuddyAgents
,mockConsultCall
, andmockTransferCall
are essential for testing the new logic without hitting real endpoints.
104-106
: Render Test
Verifies that theCallControlComponent
successfully renders the container. This is a reliable baseline test.
116-121
: End Call Test
Confirming thatendCall
is triggered upon button click is sufficient coverage for this scenario.
123-131
: Consult Call Test
Validates thatconsultCall
is invoked when an agent is selected. This scenario coverage looks solid.
133-141
: Transfer Call Test
CheckstransferCall
is called with the correct agent ID and destination type. Great job ensuring the consult logic isn’t triggered accidentally.packages/contact-center/task/src/helper.ts (3)
5-5
: Importing BuddyDetails & DestinationType
No concerns. This allows typed references for agent data and transfer destinations.
132-132
: buddyAgents State Initialization
Setting the default to an empty array avoids null checks and simplifies usage.
299-302
: Exposing Hook Methods
ReturningbuddyAgents
,loadBuddyAgents
,transferCall
, andconsultCall
completes your new call-control workflow. Great consistency across the codebase.packages/contact-center/cc-components/src/components/task/task.types.ts (3)
1-1
: Import additions look good.You've correctly imported the necessary types
BuddyDetails
andDestinationType
from the@webex/cc-store
package to support the new transfer and consult functionality.
193-214
: Well-structured additions for transfer and consult functionality.The new properties and methods in the
ControlProps
interface are clearly defined and properly documented with JSDoc comments:
buddyAgents
: Array of available buddy agents for consultationloadBuddyAgents
: Async function to fetch buddy agentstransferCall
: Method to handle call transfers with appropriate parametersconsultCall
: Method to initiate consultation with buddy agentsThese additions align well with the PR objective of implementing transfer and consult functionalities.
231-234
: Correctly updated the CallControlComponentProps type.You've properly extended the
CallControlComponentProps
type to include the newly added properties and methods, ensuring they're accessible wherever this type is utilized in the components.packages/contact-center/cc-components/src/components/task/CallControl/call-control.styles.scss (3)
80-80
: Minor format improvement.Changed from
0.50rem
to0.5rem
, which is a cleaner representation of the same value.
118-157
: Well-structured styles for the new agent popover component.The styles for the agent popover are comprehensive and include:
- Proper z-index to ensure visibility
- Flexbox layout for content organization
- Appropriate dimensions and spacing
- Interactive hover effects for agent items
- Consistent margins and padding throughout the component
These styles will provide a good user experience for the agent consultation UI mentioned in the PR objectives.
159-168
: Good pattern for hover interactions.The implementation of hover behavior for buttons is well done:
- Buttons are hidden by default (opacity: 0)
- They appear when hovering over the list item
- Pointer events are disabled when hidden and enabled when visible
This creates a clean, uncluttered UI that reveals actions only when the user is interacting with an item.
packages/contact-center/cc-components/src/components/task/CallControl/call-control.tsx
Outdated
Show resolved
Hide resolved
...r/cc-components/src/components/task/CallControl/CallControlCustom/call-control-list-item.tsx
Outdated
Show resolved
Hide resolved
...r/cc-components/src/components/task/CallControl/CallControlCustom/call-control-list-item.tsx
Outdated
Show resolved
Hide resolved
...r/cc-components/src/components/task/CallControl/CallControlCustom/call-control-list-item.tsx
Outdated
Show resolved
Hide resolved
...ter/cc-components/src/components/task/CallControl/CallControlCustom/call-control-popover.tsx
Outdated
Show resolved
Hide resolved
...ter/cc-components/src/components/task/CallControl/CallControlCustom/call-control-popover.tsx
Outdated
Show resolved
Hide resolved
...ter/cc-components/src/components/task/CallControl/CallControlCustom/call-control-popover.tsx
Outdated
Show resolved
Hide resolved
packages/contact-center/cc-components/src/components/task/CallControl/call-control.styles.scss
Outdated
Show resolved
Hide resolved
onAgentSelect: (agentId: string) => void; | ||
} | ||
|
||
const CallControlPopoverComponent: React.FC<CallControlPopoverComponentProps> = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need two components if the list is only used inside this popover? Can't they be part of this popover component itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have left a couple of comments in the previous messages itself. Please check and respond. Rest looks good to merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good just a few points.
}; | ||
|
||
try { | ||
await currentTask.transfer(transferPayload); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we wanna add a callback that tells the devs that transfer was initiatied, onTransferInitiated? This can be part of the next PR.
Also I dont see any event being handled here that means the multi tab functionality wont work as of now.
packages/contact-center/cc-components/tests/components/StationLogin/station-login.tsx
Show resolved
Hide resolved
...cc-components/src/components/task/CallControl/CallControlCustom/consult-transfer-popover.tsx
Show resolved
Hide resolved
packages/contact-center/cc-components/tests/components/task/IncomingTask/incoming-task.tsx
Show resolved
Hide resolved
Merging this PR, will handle styling changes and event emitting in the next PR (ie consult) |
# [1.28.0-ccwidgets.39](v1.28.0-ccwidgets.38...v1.28.0-ccwidgets.39) (2025-04-02) ### Features * **widgets:** added-transfer-functionality ([#423](#423)) ([b7974a3](b7974a3))
🎉 This PR is included in version 1.28.0-ccwidgets.39 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [1.28.0-ccwidgets.39](webex/widgets@v1.28.0-ccwidgets.38...v1.28.0-ccwidgets.39) (2025-04-02) ### Features * **widgets:** added-transfer-functionality ([webex#423](webex#423)) ([b7974a3](webex@b7974a3))
COMPLETES SPARK-568988
Continues - #422
Description
Vidcast - https://app.vidcast.io/share/2cd220fc-5622-4b61-b97a-6eeb33db2433
Considerations
Tests