fix(station-login): doStationLogout prop to logout on signout confirm#473
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 enhanced keyboard accessibility and new sign-out behavior options to the Contact Center Station Login components. A keyboard event handler is added to the sign-out modal to allow closing it with the Escape key. A new optional boolean prop, Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
mkesavan13
added
the
run_e2e
|
@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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/contact-center/cc-components/src/components/StationLogin/station-login.tsx(3 hunks)packages/contact-center/cc-components/src/components/StationLogin/station-login.types.ts(1 hunks)packages/contact-center/cc-components/src/components/StationLogin/station-login.utils.tsx(2 hunks)packages/contact-center/cc-components/tests/components/StationLogin/station-login.tsx(3 hunks)packages/contact-center/cc-components/tests/components/StationLogin/station-login.utils.tsx(2 hunks)packages/contact-center/station-login/src/helper.ts(3 hunks)packages/contact-center/station-login/src/station-login/index.tsx(2 hunks)packages/contact-center/station-login/src/station-login/station-login.types.ts(1 hunks)packages/contact-center/station-login/tests/helper.ts(6 hunks)packages/contact-center/station-login/tests/station-login/index.tsx(1 hunks)widgets-samples/cc/samples-cc-react-app/src/App.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/index.tsx:10-10
Timestamp: 2025-02-13T06:58:01.805Z
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/src/components/StationLogin/station-login.tsx (1)
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/index.tsx:10-10
Timestamp: 2025-02-13T06:58:01.805Z
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.
widgets-samples/cc/samples-cc-react-app/src/App.tsx (1)
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/index.tsx:10-10
Timestamp: 2025-02-13T06:58:01.805Z
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/station-login/src/station-login/station-login.types.ts (1)
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/station-login.types.ts:129-132
Timestamp: 2025-02-13T06:59:01.103Z
Learning: In the station-login component's type definitions, it's acceptable for `UseStationLoginProps` to have a subset of props (`cc`, `onLogin`, `onLogout`, `logger`, `isAgentLoggedIn`) compared to `IStationLoginProps` and `StationLoginPresentationalProps`, even if properties like `handleContinue` and `showMultipleLoginAlert` exist in the latter interfaces.
packages/contact-center/cc-components/tests/components/StationLogin/station-login.utils.tsx (1)
Learnt from: Kesari3008
PR: webex/widgets#239
File: packages/contact-center/station-login/tests/station-login/station-login.presentational.tsx:0-0
Timestamp: 2024-11-19T13:44:14.152Z
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.
packages/contact-center/station-login/tests/station-login/index.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: 2024-11-19T13:44:14.152Z
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: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/station-login.types.ts:129-132
Timestamp: 2025-02-13T06:59:01.103Z
Learning: In the station-login component's type definitions, it's acceptable for `UseStationLoginProps` to have a subset of props (`cc`, `onLogin`, `onLogout`, `logger`, `isAgentLoggedIn`) compared to `IStationLoginProps` and `StationLoginPresentationalProps`, even if properties like `handleContinue` and `showMultipleLoginAlert` exist in the latter interfaces.
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/index.tsx:10-10
Timestamp: 2025-02-13T06:58:01.805Z
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/src/components/StationLogin/station-login.types.ts (1)
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/station-login.types.ts:129-132
Timestamp: 2025-02-13T06:59:01.103Z
Learning: In the station-login component's type definitions, it's acceptable for `UseStationLoginProps` to have a subset of props (`cc`, `onLogin`, `onLogout`, `logger`, `isAgentLoggedIn`) compared to `IStationLoginProps` and `StationLoginPresentationalProps`, even if properties like `handleContinue` and `showMultipleLoginAlert` exist in the latter interfaces.
packages/contact-center/cc-components/tests/components/StationLogin/station-login.tsx (1)
Learnt from: Kesari3008
PR: webex/widgets#239
File: packages/contact-center/station-login/tests/station-login/station-login.presentational.tsx:0-0
Timestamp: 2024-11-19T13:44:14.152Z
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.
packages/contact-center/station-login/src/station-login/index.tsx (2)
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/index.tsx:10-10
Timestamp: 2025-02-13T06:58:01.805Z
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.
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/station-login.types.ts:129-132
Timestamp: 2025-02-13T06:59:01.103Z
Learning: In the station-login component's type definitions, it's acceptable for `UseStationLoginProps` to have a subset of props (`cc`, `onLogin`, `onLogout`, `logger`, `isAgentLoggedIn`) compared to `IStationLoginProps` and `StationLoginPresentationalProps`, even if properties like `handleContinue` and `showMultipleLoginAlert` exist in the latter interfaces.
packages/contact-center/station-login/src/helper.ts (2)
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/index.tsx:10-10
Timestamp: 2025-02-13T06:58:01.805Z
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.
Learnt from: pagour98
PR: webex/widgets#385
File: packages/contact-center/station-login/src/station-login/station-login.types.ts:129-132
Timestamp: 2025-02-13T06:59:01.103Z
Learning: In the station-login component's type definitions, it's acceptable for `UseStationLoginProps` to have a subset of props (`cc`, `onLogin`, `onLogout`, `logger`, `isAgentLoggedIn`) compared to `IStationLoginProps` and `StationLoginPresentationalProps`, even if properties like `handleContinue` and `showMultipleLoginAlert` exist in the latter interfaces.
packages/contact-center/station-login/tests/helper.ts (1)
Learnt from: Kesari3008
PR: webex/widgets#239
File: packages/contact-center/station-login/tests/station-login/station-login.presentational.tsx:0-0
Timestamp: 2024-11-19T13:44:14.152Z
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.
🔇 Additional comments (26)
packages/contact-center/station-login/tests/station-login/index.tsx (1)
81-82: LGTM: Test correctly reflects new props.The test properly validates that the
useStationLoginhook is called with the newonCCSignOutanddoStationLogoutprops, maintaining consistency with the component's extended functionality.packages/contact-center/cc-components/src/components/StationLogin/station-login.utils.tsx (2)
222-229: LGTM: Clean keyboard accessibility implementation.The function properly handles the Escape key to close the sign-out modal, following standard accessibility practices. The implementation is focused and well-typed.
244-244: LGTM: Proper export addition.The new utility function is correctly added to the exports list.
widgets-samples/cc/samples-cc-react-app/src/App.tsx (3)
58-58: LGTM: Proper state initialization.The
doStationLogoutstate is correctly initialized with a sensible default value oftrue.
499-508: LGTM: Consistent UI control implementation.The checkbox implementation follows the same pattern as other toggles in the app and provides a clear way to test the new station logout behavior.
649-649: LGTM: Correct prop usage.The
doStationLogoutprop is appropriately passed only to the non-profile modeStationLogincomponent, which makes sense since this behavior is relevant for the login/logout flow rather than profile editing.packages/contact-center/cc-components/tests/components/StationLogin/station-login.utils.tsx (1)
16-16: LGTM: Comprehensive test coverage.The tests thoroughly cover the new
handleCCSignoutKeyDownfunction with both positive (Escape key) and negative (other keys) test cases. The test implementation follows established patterns and provides clear assertions.Also applies to: 38-52
packages/contact-center/cc-components/src/components/StationLogin/station-login.tsx (3)
17-17: LGTM: Proper import addition.The
handleCCSignoutKeyDownutility function is correctly imported for use in the component.
102-107: LGTM: Enhanced keyboard accessibility.The
onKeyDownevent handler properly integrates keyboard support for the sign-out modal, allowing users to close it with the Escape key. This improves accessibility and follows standard modal behavior patterns.
123-129: LGTM: Improved modal behavior.The sign-out button now immediately closes the modal before proceeding with the sign-out logic. This prevents the modal from remaining open during the sign-out process and provides better user experience.
packages/contact-center/station-login/src/station-login/station-login.types.ts (2)
15-16: LGTM! Type definitions properly extended.The addition of
onCCSignOutanddoStationLogouttoUseStationLoginPropsis consistent with the feature requirements and maintains type safety.
21-21: LGTM! StationLoginProps updated consistently.The
StationLoginPropstype correctly includes the new props, ensuring consistency across the component interface.packages/contact-center/cc-components/src/components/StationLogin/station-login.types.ts (1)
114-118: Excellent documentation and type definition.The
doStationLogoutproperty is well-documented with clear explanation of both true/false behaviors and the default value. The optional nature maintains backward compatibility.packages/contact-center/station-login/src/station-login/index.tsx (2)
10-10: LGTM! Props correctly destructured.The component properly extracts the new
doStationLogoutprop alongside existing props.
36-37: LGTM! Props correctly passed to hook.Both
onCCSignOutanddoStationLogoutare properly passed to theuseStationLoginhook, enabling the new functionality.packages/contact-center/cc-components/tests/components/StationLogin/station-login.tsx (3)
126-126: LGTM! Modal class assertion added.Good addition to verify that the sign-out modal has the expected CSS class for proper styling.
497-497: LGTM! Test suite name updated appropriately.The test suite name now accurately reflects that it covers both modal popup and keyboard handling functionality.
593-596: LGTM! Keyboard event handler test added.The test properly verifies that pressing the Escape key calls the state setter to close the modal. Good direct testing of the utility function.
packages/contact-center/station-login/src/helper.ts (2)
16-17: LGTM! Props extraction and default value handling.The props are correctly extracted and the default value for
doStationLogoutis appropriately set totrue, maintaining backward compatibility.
246-246: LGTM! Conditional function return.The function is correctly returned only when
onCCSignOutis provided, ensuring the API is clean and type-safe.packages/contact-center/station-login/tests/helper.ts (6)
37-37: LGTM! Mock method addition aligns with new functionality.The addition of the
deregistermock method properly supports the newonCCSignOutfunctionality that requires bothstationLogoutandderegistercalls.
558-585: Well-structured test for unchanged login options scenario.The test properly verifies that
saveLoginOptionsdoesn't perform unnecessary operations when no changes are detected, and includes appropriate logging verification.
586-631: Comprehensive test coverage for successful save operation.The test thoroughly validates the save functionality including:
- State changes triggering save operations
- Proper API calls with correct parameters
- State updates after successful save
- Logging verification
632-666: Good error handling test coverage.The test properly verifies error scenarios including error state updates and error logging, ensuring the application gracefully handles API failures.
667-707: Excellent edge case coverage for BROWSER device type.The test correctly verifies that the
dialNumberparameter is omitted when the device type isBROWSER, which is important for API contract compliance.
709-766: Comprehensive test suite for new onCCSignOut functionality.The test suite thoroughly covers both scenarios for the new
doStationLogoutprop:
- When
true: verifies bothstationLogoutandderegisterare called along with the callback- When
false: verifies station logout operations are skipped but callback is still invokedThe setup with proper mock clearing and state management is well-implemented.
Kesari3008
left a comment
Kesari3008
left a comment
There was a problem hiding this comment.
What about a case where IU only want to do station logout but not deregister ?
| }); | ||
|
|
||
| describe('SignOut Modal Popup', () => { | ||
| describe('SignOut Modal Popup and handleCCSignoutKeyDown', () => { |
There was a problem hiding this comment.
We would need test for setShowCCsignoutModal being invoked from cc-logout-button also right
There was a problem hiding this comment.
True. To test that, we had to move the setShowCCsignoutModal into continueClicked as we cannot test it directly by mocking prop method. Updated the test as well to test this case.
We decided on the DSM call that we won't do that as they are logging out of Webex anyway so it won't make sense to not deregister |
| await cc.deregister(); | ||
| } catch (error) { | ||
| logger.error(`CC-Widgets: Error during station logout: ${error}`, { | ||
| module: 'widget-station-login#station-login/helper.ts', |
There was a problem hiding this comment.
we don't need #station-login right, module: 'widget-station-login#helper.ts', is how I observed we are logging eveywhere
There was a problem hiding this comment.
There were other logs in this format and some other format. Now fixed everything to the format you observed
| const continueBtn = screen.getByTestId('ContinueButton'); | ||
| fireEvent.click(continueBtn); | ||
| expect(stationLoginUtils.continueClicked).toHaveBeenCalledWith(mockModal, handleContinue); | ||
| expect(stationLoginUtils.continueClicked).toHaveBeenCalledWith(mockModal, handleContinue, expect.any(Function)); |
There was a problem hiding this comment.
Instead of expect.any(Function), is it possible to make it more accurate which function, use the spy on the function
There was a problem hiding this comment.
We cannot do that because it is a state inside the component and jest doesn't give that much control there.
# [1.28.0-ccwidgets.77](v1.28.0-ccwidgets.76...v1.28.0-ccwidgets.77) (2025-06-24) ### Bug Fixes * **station-login:** doStationLogout prop to logout on signout confirm ([#473](#473)) ([a38cc5b](a38cc5b))
|
🎉 This PR is included in version 1.28.0-ccwidgets.77 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
4 participants
COMPLETES Ad-Hoc
This pull request addresses
by making the following changes
Vidcast
https://app.vidcast.io/share/9f02d056-796a-47bd-b8b8-4968342affc3
Change Type
The following scenarios were tested
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging