[CSM Portal] stabilize e2e cases-list suite; combine Set Fix ETA dialog; validate call-request case state - #1286
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR consolidates fix ETA updates into one PATCH payload, adds case-state gating for call-request creation and scheduling, and stabilizes case-management end-to-end tests against loading delays and nondeterministic browser storage behavior. ChangesCombined fix ETA submission
Call-request case-state gating
End-to-end test stabilization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SetFixEtaDialog
participant CsmCaseDetailPage
participant CasesAPI
User->>SetFixEtaDialog: enter ETA and sharing fields
SetFixEtaDialog->>CsmCaseDetailPage: submit combined payload
CsmCaseDetailPage->>CasesAPI: PATCH case with ETA fields
CasesAPI-->>CsmCaseDetailPage: update result
sequenceDiagram
participant CsmCaseDetailPage
participant CallRequestsWidget
participant CreateCallRequestDialog
participant callRequestState
CsmCaseDetailPage->>CallRequestsWidget: pass caseState
CallRequestsWidget->>callRequestState: evaluate creation state
callRequestState-->>CallRequestsWidget: block reason or allowed
CallRequestsWidget->>CreateCallRequestDialog: open only when allowed
CreateCallRequestDialog->>callRequestState: validate submission state
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…efect paths - CasesListPage: add rowCountSettled() — wait for the first row (or a genuine empty result) before counting, so a still-loading list isn't read as empty. - cases list: use rowCountSettled() for the initial count in search/filters/ sort/pagination (these were spuriously skipping as "0 cases" on a populated tenant). Sort now self-skips when the list empties and does not repopulate (a backing sort re-query defect), rather than hanging. - recent-nav: skip the QuickNav and recent-views tests — both depend on the localStorage recent-views hook reflecting a same-tab write without a reload, which it does not do deterministically. - watchers: query by the signed-in user's email domain (a one-letter query matched only empty-email accounts and skipped); self-skip if the picked candidate's watcher chip does not appear, instead of hard-failing.
rksk
changed the title
The data source only accepts scheduling a call while the case is in one of 5 states (Work in progress, Awaiting info, Waiting on WSO2, Solution proposed, Reopened); any other state was silently rejected with a generic "Invalid request payload." error only after submit. Add an FE-side gate that surfaces the reason up front and disables both the "Create call request" trigger and the dialog's submit action when the case is in a disallowed state, re-derived live so it tracks the case's current state rather than a snapshot taken when the dialog opened.
rksk
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx (1)
1344-1358: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDialog stays open after a successful combined save, hiding the success banner.
With one combined save there's no reason to keep the dialog mounted; the "Fix ETA updated." feedback renders on the page behind the modal, so the engineer sees no confirmation and may click Save again. Other single-shot dialogs here (autoclose hold, add tag) close in
onSuccess.🐛 Proposed fix
patchCase.mutate(patch as BeCaseUpdatePayload, { onSuccess: () => { + setFixEtaOpen(false); setFeedback({ message: "Fix ETA updated.",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx` around lines 1344 - 1358, Update the onSuccess handler in onSetFixEta to close the Fix ETA dialog before or alongside setting the success feedback, matching the autoclose hold and add-tag save behavior; preserve the existing success message and error handling.
🧹 Nitpick comments (4)
apps/csm-portal/webapp/tests/e2e/specs/shell/recent.spec.ts (1)
133-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrack the unconditional skips for removal.
Both scenarios are skipped on every run, so QuickNav and recent-view regressions are no longer detected. Add a linked issue/owner or expiry and restore the assertions once same-tab reactivity is fixed.
Also applies to: 165-166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/tests/e2e/specs/shell/recent.spec.ts` around lines 133 - 141, Track the unconditional skips in the QuickNav and recently-viewed scenarios by adding a linked issue or owner and an explicit expiry, then restore both assertions once the same-tab reactivity described around useRecentViews is fixed. Update the test.skip cases at “recent nav — QuickNav search + navigate” and the corresponding recently-viewed scenario so they cannot remain indefinitely skipped without ownership or a removal target.apps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.test.tsx (1)
125-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the
isSavingstate.The suite exercises validation and payload shape well, but nothing asserts that Save is disabled/loading and the fields are disabled while
isSavingis true — the main way a double PATCH could slip in given the dialog stays open after save.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.test.tsx` around lines 125 - 151, Extend the SetFixEtaDialog tests to render with isSaving set to true and assert that the Save control is disabled or shows its loading state, while the editable sharing fields such as product and public ticket are disabled. Verify the dialog prevents another save submission during this state.apps/csm-portal/webapp/src/api/backend/types.ts (1)
619-644: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCombined variant accepts
{}, so the "at least one estimate" rule is no longer compile-time enforced.Every member of the new variant is optional, so an empty object (and
{ addPublicComment: true }alone) type-checks even though the doc block right above says at least one estimate must be present and that the contract is "enforced at compile time, not just in docs". A three-way required-one union keeps that guarantee.♻️ Sketch: require at least one estimate
- | (Omit< - BeCaseUpdateNever, - | "bestCaseFixEta" - | "mostLikelyFixEta" - | "worstCaseFixEta" - | "addPublicComment" - | "product" - | "publicTicket" - > & { - bestCaseFixEta?: string; - mostLikelyFixEta?: string; - worstCaseFixEta?: string; - addPublicComment?: boolean; - product?: string; - publicTicket?: string; - }); + | (Omit< + BeCaseUpdateNever, + | "bestCaseFixEta" + | "mostLikelyFixEta" + | "worstCaseFixEta" + | "addPublicComment" + | "product" + | "publicTicket" + > & + BeCaseFixEtaEstimates & { + addPublicComment?: boolean; + product?: string; + publicTicket?: string; + });with, above the union:
type BeCaseFixEtaEstimates = | { bestCaseFixEta: string; mostLikelyFixEta?: string; worstCaseFixEta?: string } | { bestCaseFixEta?: string; mostLikelyFixEta: string; worstCaseFixEta?: string } | { bestCaseFixEta?: string; mostLikelyFixEta?: string; worstCaseFixEta: string };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/src/api/backend/types.ts` around lines 619 - 644, Update the combined variant in the BeCaseUpdate type to require at least one fix-ETA estimate at compile time. Define a three-way BeCaseFixEtaEstimates union where each branch requires one of bestCaseFixEta, mostLikelyFixEta, or worstCaseFixEta while keeping the other two optional, then intersect it with the existing optional addPublicComment, product, and publicTicket fields.apps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.tsx (1)
36-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider deriving this payload type from the backend contract.
FixEtaSavePayloadrestates the combined fix-ETA fields already declared inBeCaseUpdatePayload, and the caller inapps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx(Line 1346) has to cast withas BeCaseUpdatePayload, which suppresses any future drift between the two shapes. Extracting the combined variant's field set intypes.tsand reusing it here removes both the duplication and the cast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.tsx` around lines 36 - 44, The FixEtaSavePayload shape duplicates fields from BeCaseUpdatePayload and requires an unsafe cast at the caller. Extract the shared combined fix-ETA field set in types.ts, derive or reuse it for FixEtaSavePayload, and update CsmCaseDetailPage to pass the payload without casting to BeCaseUpdatePayload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/csm-portal/webapp/src/features/csm-cases/components/CallRequestsWidget.tsx`:
- Around line 108-118: Extend the stateBlockReason restriction beyond create
flows to scheduling and rescheduling: pass stateBlockReason into
ScheduleCallDialog so it displays the reason and disables submission, and update
handleSchedule to defensively reject the scheduled-state PATCH when a block
reason exists. Preserve the existing schedule/reschedule behavior when no block
reason is present.
In
`@apps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.tsx`:
- Around line 96-103: The clearable picker configuration in SetFixEtaDialog
conflicts with handleSave’s truthy-only payload construction, so cleared
estimates are not persisted. Remove clearable behavior from the picker field
slotProps (including the corresponding picker configuration around the second
estimate field), preserving the existing prefilled values and save flow.
In `@apps/csm-portal/webapp/tests/e2e/pages/CasesListPage.ts`:
- Around line 231-238: Update CasesListPage.rowCountSettled so a
rows().first().waitFor timeout is not treated as an empty list. Wait for either
a row or the page’s established empty-state/loading-complete signal, return the
count only after one terminal state is reached, and propagate or fail on timeout
when neither state occurs.
In `@apps/csm-portal/webapp/tests/e2e/specs/cases/detail-lifecycle.spec.ts`:
- Around line 352-356: Update the watcherChip wait flow in the added check to
catch only the expected 10-second visible-state timeout and return false for
that case; inspect the caught error and rethrow unexpected failures such as
closed pages, contexts, or invalid locators instead of silently treating them as
an absent chip.
---
Outside diff comments:
In `@apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx`:
- Around line 1344-1358: Update the onSuccess handler in onSetFixEta to close
the Fix ETA dialog before or alongside setting the success feedback, matching
the autoclose hold and add-tag save behavior; preserve the existing success
message and error handling.
---
Nitpick comments:
In `@apps/csm-portal/webapp/src/api/backend/types.ts`:
- Around line 619-644: Update the combined variant in the BeCaseUpdate type to
require at least one fix-ETA estimate at compile time. Define a three-way
BeCaseFixEtaEstimates union where each branch requires one of bestCaseFixEta,
mostLikelyFixEta, or worstCaseFixEta while keeping the other two optional, then
intersect it with the existing optional addPublicComment, product, and
publicTicket fields.
In
`@apps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.test.tsx`:
- Around line 125-151: Extend the SetFixEtaDialog tests to render with isSaving
set to true and assert that the Save control is disabled or shows its loading
state, while the editable sharing fields such as product and public ticket are
disabled. Verify the dialog prevents another save submission during this state.
In
`@apps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.tsx`:
- Around line 36-44: The FixEtaSavePayload shape duplicates fields from
BeCaseUpdatePayload and requires an unsafe cast at the caller. Extract the
shared combined fix-ETA field set in types.ts, derive or reuse it for
FixEtaSavePayload, and update CsmCaseDetailPage to pass the payload without
casting to BeCaseUpdatePayload.
In `@apps/csm-portal/webapp/tests/e2e/specs/shell/recent.spec.ts`:
- Around line 133-141: Track the unconditional skips in the QuickNav and
recently-viewed scenarios by adding a linked issue or owner and an explicit
expiry, then restore both assertions once the same-tab reactivity described
around useRecentViews is fixed. Update the test.skip cases at “recent nav —
QuickNav search + navigate” and the corresponding recently-viewed scenario so
they cannot remain indefinitely skipped without ownership or a removal target.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5842db14-29d4-4c7d-9290-9dbd2476aeff
📒 Files selected for processing (13)
apps/csm-portal/webapp/src/api/backend/types.tsapps/csm-portal/webapp/src/features/csm-cases/components/CallRequestsWidget.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CaseActionBar.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CreateCallRequestDialog.tsxapps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.test.tsxapps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.tsxapps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsxapps/csm-portal/webapp/src/features/csm-cases/utils/callRequestState.test.tsapps/csm-portal/webapp/src/features/csm-cases/utils/callRequestState.tsapps/csm-portal/webapp/tests/e2e/pages/CasesListPage.tsapps/csm-portal/webapp/tests/e2e/specs/cases/detail-lifecycle.spec.tsapps/csm-portal/webapp/tests/e2e/specs/cases/list.spec.tsapps/csm-portal/webapp/tests/e2e/specs/shell/recent.spec.ts
… case state, drop misleading clearable ETA pickers, fix e2e row-count/watcher-chip error handling - ScheduleCallDialog now accepts stateBlockReason and displays/disables submit on it; CallRequestsWidget passes it through and defensively rejects handleSchedule when the case is in a non-eligible state, mirroring the create-call-request gate. - SetFixEtaDialog's date pickers drop the clearable slot prop: the PATCH contract has no way to null out a single ETA field, so clearing one silently left the old value in place upstream. Removing the affordance avoids the misleading UI state. - CasesListPage.rowCountSettled() now races the first row against the list's own empty-state message instead of treating any wait timeout as "empty"; if neither signal appears before the timeout it throws instead of silently returning 0, so callers can't mistake a slow-loading list for a confirmed empty result. - detail-lifecycle.spec.ts's watcher-chip self-skip now only swallows Playwright's TimeoutError; any other failure (closed page/context, broken locator, etc.) rethrows instead of being downgraded to a skip.
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReviews resumed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/csm-portal/webapp/tests/e2e/pages/CasesListPage.ts`:
- Around line 241-252: Update CasesListPage.rowCountSettled to await the row and
empty-state visibility checks with Promise.race so it resolves immediately when
either settled signal appears. Preserve the boolean outcomes needed to determine
which signal won, and throw only after both signals fail to appear within
timeoutMs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e9e2247e-c64f-4733-90c9-8642bcdcca98
📒 Files selected for processing (5)
apps/csm-portal/webapp/src/features/csm-cases/components/CallRequestsWidget.tsxapps/csm-portal/webapp/src/features/csm-cases/components/ScheduleCallDialog.tsxapps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.tsxapps/csm-portal/webapp/tests/e2e/pages/CasesListPage.tsapps/csm-portal/webapp/tests/e2e/specs/cases/detail-lifecycle.spec.ts
💤 Files with no reviewable changes (1)
- apps/csm-portal/webapp/src/features/csm-cases/components/SetFixEtaDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/csm-portal/webapp/tests/e2e/specs/cases/detail-lifecycle.spec.ts
… Promise.all Promise.all waited for both waitFor calls to settle, so a fast row still blocked for the full timeoutMs on the empty-state branch. Race them instead and only fall back to awaiting both if the race's first settlement was a rejection.
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReviews resumed. |
Rashmika998
approved these changes
Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Three independent CSM portal fixes/improvements bundled on this branch:
cases-listspec suite was flaky: a still-loading list was sometimes read as "0 cases" before the first row settled, causing spurious failures on search/filter/sort/pagination assertions; a few other specs (recent-nav, watchers) had similar non-deterministic dependencies.Goals
Approach
rowCountSettled()toCasesListPageand used it as the initial-count gate in search/filters/sort/pagination specs. The sort spec now self-skips if the list empties and doesn't repopulate (a backing re-query defect, not an e2e bug). Recent-nav specs (QuickNav, recent-views) are skipped — they depend on a same-tab localStorage write being reflected without a reload, which isn't deterministic. The watchers spec now queries by the signed-in user's email domain (a one-letter query previously matched only empty-email accounts) and self-skips if the picked candidate's watcher chip doesn't render.addPublicComment/product/publicTicketfields go out in one combinedPATCH /cases/{id}call. No existing e2e spec referenced the old multi-button UI, so no spec changes were needed for this part.caseAcceptsCallRequests/callRequestCaseStateBlockReasonhelpers (reusing this codebase's existing case-state label constants).CreateCallRequestDialogdisables Save and shows an inline message when the case's current state isn't one of the five SN-allowed states;CallRequestsWidget's trigger button follows the same disabled-with-tooltip pattern already used for a closed case, rather than hiding the action outright.User stories
Release note
Documentation
N/A — internal CSM-portal-only UI/test changes, no externally published documentation covers this workflow.
Training
N/A — no training content exists for this internal tool.
Certification
N/A — internal tool, not covered by any certification exam.
Marketing
N/A — internal tool, not customer/market-facing.
Automation tests
Added/updated unit tests for
SetFixEtaDialog(rewritten for the combined-save UI) andcallRequestState(3 new focused tests for the state-gating logic). Fullpnpm testrun: 479 passed / 9 failed, with the 9 failures reproduced identically on the pre-change base commit (CaseActivitiesFeed.test.tsx,CsmAnnouncementsPage.test.tsx,CaseActionBar.test.tsx) — confirmed pre-existing and unrelated to this PR.e2e: stabilized the existing
cases-listPlaywright/Cypress specs (see Approach above) — no new e2e scenarios added, existing ones made deterministic.Security checks
pnpm lintandtsc/pnpm buildclean instead (one pre-existing, unrelated type error inCreateSecurityReportPage.tsxconfirmed present on the base branch too).Samples
N/A
Related PRs
Builds on cs-tools #1283 (SRA attachments, name display, project filtering, parentCase.type — merged to
main), which this branch was based on/rebased against.Migrations (if applicable)
N/A — no data migration involved.
Test environment
Node/pnpm toolchain per
apps/csm-portal/webapp/package.json; unit tests run viavitest; verified against the ServiceNow-backed dev tenant for the call-request state rule and fix-ETA share-comment behavior described above.Learning
The call-request state rule and the fix-ETA share-comment behavior were both confirmed by inspecting the ServiceNow scripted API's own validation logic directly (not inferred from documentation), to make sure the FE mirrors the actual backend contract rather than a guess.
Summary by CodeRabbit