feat(station-login): e911 Modal on Desktop Station Login#719
feat(station-login): e911 Modal on Desktop Station Login#719brain-frog wants to merge 14 commits into
Conversation
- Add E911 modal state and methods to store (showE911Modal, isEmergencyModalAlreadyDisplayed) - Add User Preferences API types and interfaces for fetching/updating preferences - Implement fetchUserPreferences and updateEmergencyModalAcknowledgment in store wrapper - Create E911Modal presentational component with checkbox acknowledgment - Integrate E911Modal into StationLogin widget - Trigger E911 check after successful BROWSER login - Add unit tests for E911Modal component and store methods - Fix isEndTaskEnabled -> isEndCallEnabled typo in test fixtures The modal displays for BROWSER login if user hasn't previously acknowledged. User preferences are persisted via SDK userPreference service. Note: User Preferences API requires SDK update (pending publish of 3.12.0-next.133) Refs: CAI-7905
…xports - Rename E911Modal.tsx to e911-modal.tsx for consistency with other files - Remove index.ts barrel file, import component directly - Update cc-components index.ts to use direct imports
![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: 2f023d32e1
ℹ️ 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".
- Fix import path in e911-modal.test.tsx to match renamed component file - Update yarn.lock
github-actions
Bot
removed
the
validated
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e205c18ad
ℹ️ 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".
- Wire Momentum Checkbox via 'onchange' prop instead of React 'onChange' (custom element does not fire React's synthetic change event, leaving Save & Continue permanently disabled) - Sync native dialog 'cancel' event (Escape key) back to onCancel so store state (showE911Modal) stays consistent with dialog visibility - Reset isEmergencyModalAlreadyDisplayed to false whenever desktopPreference is missing/unparsable, instead of leaving the singleton's stale value, which could cause the modal to be skipped for a new agent/session - Throw instead of silently resolving in fetchUserPreferences and updateEmergencyModalAcknowledgment when userPreference service is unavailable, so failures are surfaced instead of leaving the modal stuck open with no way to persist acknowledgment - Add regression tests for native cancel handling and preference reset Addresses review comments on PR webex#719
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 440fa9aef6
ℹ️ 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".
cmullenx
left a comment
cmullenx
left a comment
There was a problem hiding this comment.
left some comments and some of codex reviews need to be addressed as well
akulakum
left a comment
akulakum
left a comment
There was a problem hiding this comment.
station-login-spec.md — Spec not updated
Per repo spec-currency rules, the module spec should be updated in the same change.
Please add E911 modal requirements as new STATION-LOGIN-R-* entries in packages/contact-center/station-login/ai-docs/station-login-spec.md, including:
- Trigger conditions
- Persistence via
desktopPreference - Cancel/dismiss behavior
…and map them to the new tests.
- Merge E911 acknowledgment flag into existing desktopPreference instead of overwriting it (was deleting sibling preference fields) - Use rem units and Momentum theme color tokens in e911-modal.style.scss instead of hardcoded px/hex; drop redundant font-size/weight overrides already handled by the Text component's type prop - Extract 'BROWSER' string literal to the shared DEVICE_TYPE_BROWSER constant from @webex/cc-store - Remove placeholder help link (href="#") until a real destination exists - Add unit tests for checkE911ModalDisplay() covering BROWSER/non-BROWSER device types, already-acknowledged preference, and fetchUserPreferences failure handling - Add regression tests for desktopPreference merge behavior
github-actions
Bot
removed
the
validated
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0d949d355
ℹ️ 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".
- Replace native <dialog> with Momentum Dialog component; disable the built-in close button and Escape-key dismissal so Cancel is the only way to dismiss the modal - Add missing triggers for checkE911ModalDisplay(): on mount when already logged in with BROWSER, on AGENT_STATION_LOGIN_SUCCESS/relogin events, on profile save switching to BROWSER, and on multi-login takeover (handleContinue) - Fix updateEmergencyModalAcknowledgment() to persist the acknowledgment under the preference service's own userId (from getUserPreference()) instead of the CC agentId, since the two identifiers can differ and using the wrong one could cause the modal to reappear erroneously - Merge existing desktopPreference fields when updating the acknowledgment flag instead of overwriting sibling preferences - Add a manual debug button in the samples app to force-open the E911 modal for testing - Add/update unit tests covering the new triggers, dismissal behavior, and the userId-vs-agentId fix
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 419bdb6813
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94c8372b75
ℹ️ 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".
cleanUpStore() reset store.isAgentLoggedIn/deviceType/etc on logout but never cleared showE911Modal/isEmergencyModalAlreadyDisplayed. If the SDK logged the agent out while the E911 dialog was open (e.g. an external agent:logoutSuccess), the modal/acknowledgment state leaked into the logged-out view or the next session. Reset both fields alongside the rest of the logout state.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 348060aaf3
ℹ️ 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".
A single BROWSER login can trigger checkE911ModalDisplay() from two paths at once - login()'s own success handler and the AGENT_STATION_LOGIN_SUCCESS callback (handleLogin) - each starting an independent fetchUserPreferences() call. If one of those reads resolved after the user already saved/canceled the modal, it could overwrite isEmergencyModalAlreadyDisplayed back to false and reopen the modal. Guard checkE911ModalDisplay with an in-flight promise ref so concurrent callers join the same fetchUserPreferences() call instead of racing.
… check
getUserPreference() 404s for a first-time user who has no preference row
yet. fetchUserPreferences() was rethrowing that error, which
checkE911ModalDisplay() then swallowed - so first-time BROWSER agents
never saw the E911 modal. updateEmergencyModalAcknowledgment() then
called updateUserPreference() (update-only) with no userId, which would
also fail for that same user on Save.
- fetchUserPreferences(): treat a 404 from getUserPreference() as
isEmergencyModalAlreadyDisplayed=false instead of rethrowing.
- updateEmergencyModalAcknowledgment(): on a 404, call
createUserPreference({userId: agentId, desktopPreference}) instead of
updateUserPreference(), since there is no existing record (and no
preference-service userId) to update.
- Add createUserPreference to the local IUserPreferenceService shim
type (store.types.ts) pending the real SDK types (CAI-7906).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d221975490
ℹ️ 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".
getUserPreference() returns the persisted desktopPreference JSON string nested under response.preferences.desktopPreference, not as a top-level response.desktopPreference field. fetchUserPreferences() and updateEmergencyModalAcknowledgment() were reading the nonexistent top-level field, so desktopPrefString was always undefined for an existing record - resetting isEmergencyModalAlreadyDisplayed to false and re-showing the E911 modal on every BROWSER login even after the agent had already acknowledged it. Read from response.preferences.desktopPreference in both methods, and correct the local UserPreferenceResponse shim type (pending the real SDK types - CAI-7906) to match.
…preference record createUserPreference()'s userId must be the CI user id - the same identity the SDK's own update path already keys off of via response.userId (see the earlier 'preference service userId, not the CC agentId' test). The first-time-user create path introduced for the 404 case was passing store.agentId (the CC agent identifier) instead, which can differ from the CI id and would create the record under the wrong user or fail outright, leaving Save & Continue unable to persist the E911 acknowledgment for those agents. Source the CI user id from the underlying webex SDK (webex.internal.device.userId) instead of guessing with store.agentId.
github-actions
Bot
removed
the
validated
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d2ef4c8da
ℹ️ 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".
…nce unmounts The single-owner guard for E911Modal previously only attempted to claim ownership once, on mount. If the owning StationLogin instance unmounted while a sibling instance stayed mounted, the sibling never re-checked and the E911 modal would never render again for the rest of the session. Fix: re-run the ownership-claim effect whenever store.showE911Modal changes, not just on mount, so a surviving instance can take over after the previous owner releases ownership. Also included: - E911Modal disables Save & Continue/Cancel while onSaveAndContinue is in flight and shows a user-facing error on failure, keeping the modal open for retry. - station-login rethrows save errors from handleE911SaveAndContinue so the modal can surface them. - storeEventsWrapper: type JSON.parse results for desktopPreference as DesktopPreference instead of implicit any. - fix(store): rename stale isEndCallEnabled to isEndTaskEnabled in getFeatureFlags()'s key list and the shared mockProfile test fixture - the SDK's Profile field was renamed during the task-refactor migration but these were never updated, causing a tsc failure and a silently-dropped feature flag. Pre-existing, unrelated to E911 work; fixed because it was blocking the pre-commit hook's test:unit run. - docs: update cc-components-spec.md and station-login-spec.md for spec-currency (E911Modal public surface/requirements, single-owner guard, and correcting a stale requirement claiming station-login never renders E911Modal).
|
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: c9c37aaee1
ℹ️ 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".
| {ownsE911Modal && ( | ||
| <E911Modal isOpen={showE911Modal} onSaveAndContinue={handleE911SaveAndContinue} onCancel={handleE911Cancel} /> |
There was a problem hiding this comment.
When a host uses the documented onLogin callback to switch away from or unmount the login widget after a successful login, the only component that can render store.showE911Modal is removed before the async E911 preference check can display anything. Fresh evidence is that this fix still renders the modal inside StationLoginInternal, while handleLogin invokes the host callback before starting checkE911ModalDisplay; BROWSER agents in that common host flow can therefore skip the notification entirely. Move the modal to a store/root owner that remains mounted, or complete the E911 gate before notifying the host.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed - handleLogin/the mount-refresh effect in helper.ts call the host's onLogin before checkE911ModalDisplay() resolves (lines 151-153, 180-184), so a host that unmounts/navigates away from the login widget inside onLogin (a common pattern) can leave no StationLogin instance mounted by the time store.showE911Modal flips true. My single-owner "reclaim" fix only covers the case where some instance stays mounted - it doesn't help here.
Agreed this is worth fixing, but the real fix is an architecture decision rather than a quick patch: either (a) render E911Modal from something not tied to the login widget's own mount lifecycle, or (b) hold off calling onLogin until the E911 gate settles (changes the onLogin contract). Filing this as a follow-up rather than fixing inline on this PR - want to align on the approach first. Tracking in CAI-7905 follow-up.
| // Single-flight guard for checkE911ModalDisplay - see its usage below | ||
| const e911CheckInFlightRef = useRef<Promise<void> | null>(null); |
There was a problem hiding this comment.
This ref is created per useStationLogin instance, so it only dedupes races within one widget. I checked the sample flow that can mount the normal and profile-mode StationLogin widgets together (widgets-samples/cc/samples-cc-react-app/src/App.tsx lines 896 and 950); on refresh/relogin both instances can run checkE911ModalDisplay with separate refs and issue independent preference reads, allowing a slower stale read to reset isEmergencyModalAlreadyDisplayed after Save & Continue and reopen the modal. Keep the in-flight guard in the shared store/module instead of per hook instance.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed - e911CheckInFlightRef (line 58) is a useRef scoped per useStationLogin call, so it only dedupes within one mounted StationLogin/profileMode instance, not across the two instances the single-owner guard elsewhere explicitly supports mounting together. Agreed this undermines the guard's own stated purpose.
This one's a tractable, scoped fix (move the in-flight-promise tracking to module/store scope, same pattern as e911ModalOwner) - will follow up with that change.
…tead of per-hook e911CheckInFlightRef was a useRef, so it only deduped concurrent checkE911ModalDisplay() calls within a single useStationLogin instance. Multiple StationLogin instances can be mounted against the same store singleton (e.g. the login widget plus a profileMode settings widget - see the single-owner E911Modal guard), so each instance had its own guard and could race an independent fetchUserPreferences() read, potentially resetting isEmergencyModalAlreadyDisplayed from a stale read and reopening the modal after the user already acknowledged it. Fix: move the in-flight promise to module scope (same pattern as e911ModalOwner), shared by every mounted instance. Addresses Codex review feedback on PR webex#719.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27d544189d
ℹ️ 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".
| await store.fetchUserPreferences(); | ||
|
|
||
| if (!store.isEmergencyModalAlreadyDisplayed) { | ||
| store.setShowE911Modal(true); |
There was a problem hiding this comment.
When a BROWSER login starts this check and the agent logs out, switches to a non-BROWSER device, or a different user logs in before fetchUserPreferences() resolves, this continuation still applies the stale request result. cleanUpStore() can reset showE911Modal to false, but the old promise can flip it back to true for a signed-out/non-BROWSER view, and a quick next login can also join the stale module-scoped in-flight promise and use the previous user's preference. Recheck the current logged-in/device/session identity after the await, or cancel/clear stale E911 checks on cleanup.
Useful? React with 👍 / 👎.
3 participants
COMPLETES # https://jira-eng-sjc12.cisco.com/jira/browse/CAI-7905
This pull request addresses
When a user does a "Desktop" station login in agent desktop there is supposed to be a modal asking them to fill out their emergency numbers and a value in user preferences that tracks if the modal has been acknowledged
< DESCRIBE THE CONTEXT OF THE ISSUE >
by making the following changes
Adding the modal and having it pop up in that use case
< DESCRIBE YOUR CHANGES >
https://app.vidcast.io/share/86714212-0e4c-4b4d-9173-6df14d7bf273?playerMode=vidcast
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.