[CSM Portal] case/CR detail cleanup: linked SRs to Related tab, remove redundant menu items, hide SLAs tab, CR page tabbed layout - #1267
Conversation
Now that watchers has its own tab, the Details tab's card layout has room; move the Linked service requests card into the Related tab alongside child cases so cross-case links live with other case-relationship content instead of general details.
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR updates CSM account contracts and rendering, adjusts case actions and filters, adds tabbed change-request content, synchronizes recent views with identity resolution, exposes ServiceNow Salesforce IDs, and adds endpoint-specific attachment body limits. ChangesCSM account data and presentation
CSM case UI behavior
Change-request detail tabs
Recent-view identity synchronization
Entity-service account and attachment contracts
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
…ide SLAs tab - Manage watchers, Link to another case, and Request a call are removed from the case detail More-actions menu: each now has a fully equivalent, directly reachable entry point in its own tab (Watchers tab's inline add/remove, Related tab's own "Link to another case" button, Call requests tab's own "Create call request" button), so keeping both was two ways to do one thing. Create task stays in the menu since the Tasks tab is still hidden and has no create affordance of its own. - Removed the now-dead autoOpenCallCreate plumbing that only existed to support the removed "Request a call" menu item. - SLAs tab hidden in TAB_DEFS (data/hooks untouched, same pattern already used for the Tasks tab).
…ching case detail
…ail Issues tab The project detail page's Issues tab locks only the project filter, with no case-type lock, so it should return every issue type for that project. It was only ever showing support cases because an empty caseTypes filter gets omitted from the /cases/search request entirely, and the entity-service defaults an absent types filter to support cases only rather than "no restriction". Fix in CsmIssuesView: when the type filter is unlocked and nothing is selected, send every known case type explicitly instead of omitting the field, so the backend default can't silently narrow the result. Every other CsmIssuesView caller locks caseTypes to a single value already, so this only changes behavior for the one unlocked, multi-type view.
… search request shape
- Rename the Account type's createdAt/updatedAt to createdOn/updatedOn to
match every account response shape (both data sources return the *On
fields, never *At).
- Nest the account search request's searchQuery under filters, matching
the backend's search payload shape and the pattern already used by the
case search builder.
- Tolerate the alternate account response shape's supportTier field (a
plain string on the list view, an {id, label} ref on the detail view)
alongside tier, since the FE has one Account type shared across both
data sources. Adds a resolveAccountTier helper used on both pages.
- Minor: show the existing empty-state placeholder for a blank SF ID on
the accounts list, matching the detail page.
…f the generic 1 MiB default CreateCaseAttachment routed through the generic decodeRequest helper, which enforces a blanket 1 MiB JSON body cap meant for small structured payloads. Any attachment upload over ~1 MiB (base64-encoded) was rejected with "request body too large" even though it is well under every advertised limit -- the FE's 10 MB client-side check and the csm-portal backend's own 15 MiB ceiling never got a chance to matter. Add decodeRequestWithLimit, a variant of decodeRequest that accepts a caller-supplied body size cap and too-large message, and use it for CreateCaseAttachment with a 15 MiB cap -- matching the csm-portal backend's own maxAttachmentBodyBytes ceiling (sized for a 10 MB file inflated by base64 plus JSON overhead). The error message now states the actual limit instead of the generic one. No other endpoint's cap changes.
Adds the field the backing data source already provides but wasn't being forwarded: SNAccountView and SNAccountDetail gain SfID, wired through from the ServiceNow account response's sfId field.
…l reload useRecentViews' bucket-key resolution and the "pending" (pre-identity) migration lived only in a background effect that ran independently of both the record (write) path and the read path, so a visit recorded before this browser's ID-token decode settled could sit unreflected in an already-mounted reader until some unrelated event forced a re-read. Extract the resolve/migrate step into a shared helper and call it synchronously from both useRecordRecentView's write and useRecentViews' own settle-effect, and force an explicit re-read the moment a reader's own identity settles instead of only reacting to a dispatched event from elsewhere. Adds unit coverage for the record-then-read-within-one-session scenario.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
# Conflicts: # entity-service/internal/domain/entity.go # entity-service/internal/service/sn_account_service.go
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.tsx (1)
408-436: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTabs aren't associated with their panels for assistive tech.
The four content blocks below are plain
Box/Cardelements with norole="tabpanel",id, oraria-labelledby, and theTabs have noid/aria-controls. Screen reader users get a tablist that announces no relationship to the rendered content.♿ Suggested wiring
return ( <Tab key={t.id} value={t.id} + id={`cr-tab-${t.id}`} + aria-controls={`cr-tabpanel-${t.id}`} icon={t.icon} iconPosition="start" label={count ? `${t.label} (${count})` : t.label} sx={{ minHeight: 44, textTransform: "none" }} /> );Then give each rendered block
role="tabpanel",id={cr-tabpanel-}andaria-labelledby={cr-tab-}.🤖 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-operations/pages/CsmChangeRequestDetailPage.tsx` around lines 408 - 436, Associate the tabs and their rendered content panels for accessibility. In the Tabs map within CsmChangeRequestDetailPage, add matching ids and aria-controls to each Tab, and update each corresponding content Box/Card block with role="tabpanel", id="cr-tabpanel-<id>", and aria-labelledby="cr-tab-<id>"; keep the existing active-tab rendering behavior unchanged.apps/csm-portal/webapp/src/features/csm-cases/components/CaseActionBar.test.tsx (1)
372-375: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that removed actions are absent from the overflow menu.
The narrowed matrix stops dispatch testing, but does not prevent
manage_watchers,link_case, orrequest_callfrom being reintroduced. Add one menu-level absence test.Suggested test
+ it("does not expose actions now available from case-detail tabs", () => { + render( + <CaseActionBar + caseDetail={caseInState("awaiting_info", ["waiting_on_wso2"])} + onAction={vi.fn()} + />, + ); + + fireEvent.click(screen.getByRole("button", { name: /more/i })); + + expect(screen.queryByRole("menuitem", { name: /manage watchers/i })).not.toBeInTheDocument(); + expect(screen.queryByRole("menuitem", { name: /link to another case/i })).not.toBeInTheDocument(); + expect(screen.queryByRole("menuitem", { name: /request a call/i })).not.toBeInTheDocument(); + });🤖 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/CaseActionBar.test.tsx` around lines 372 - 375, Add a menu-level test in the “CaseActionBar — Hold auto-closure / Edit case details” suite that opens or renders the overflow menu and asserts the removed actions manage_watchers, link_case, and request_call are absent. Keep the existing SECONDARY_ITEMS dispatch matrix unchanged and verify absence through visible menu action labels or identifiers.
🤖 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-recent/hooks/useRecentViews.test.ts`:
- Around line 250-267: Update the test around useRecordRecentView so mockUserid
is reset to undefined before the recorder mounts, while preserving the reader’s
already-resolved shared identity. Then invoke the recorder and assert the entry
lands in the reader’s existing bucket, exercising the resolveActiveUserKey
fallback when the writer’s own userid is unresolved.
In `@entity-service/internal/handler/decode.go`:
- Around line 49-54: Update decodeRequestWithLimit’s trailing-data validation so
a *http.MaxBytesError from the second Decode is reported through the
size-specific tooLargeMsg path rather than the generic invalid-request message.
Add a regression test covering a valid first object followed by oversized
trailing data and assert the size-limit response.
---
Nitpick comments:
In
`@apps/csm-portal/webapp/src/features/csm-cases/components/CaseActionBar.test.tsx`:
- Around line 372-375: Add a menu-level test in the “CaseActionBar — Hold
auto-closure / Edit case details” suite that opens or renders the overflow menu
and asserts the removed actions manage_watchers, link_case, and request_call are
absent. Keep the existing SECONDARY_ITEMS dispatch matrix unchanged and verify
absence through visible menu action labels or identifiers.
In
`@apps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.tsx`:
- Around line 408-436: Associate the tabs and their rendered content panels for
accessibility. In the Tabs map within CsmChangeRequestDetailPage, add matching
ids and aria-controls to each Tab, and update each corresponding content
Box/Card block with role="tabpanel", id="cr-tabpanel-<id>", and
aria-labelledby="cr-tab-<id>"; keep the existing active-tab rendering behavior
unchanged.
🪄 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: e2a319f1-9144-4285-996a-e9ab5c0343bf
📒 Files selected for processing (17)
apps/csm-portal/webapp/src/features/csm-accounts/pages/CsmAccountDetailPage.tsxapps/csm-portal/webapp/src/features/csm-accounts/pages/CsmAccountsPage.tsxapps/csm-portal/webapp/src/features/csm-accounts/types/csmAccounts.tsapps/csm-portal/webapp/src/features/csm-cases/components/CaseActionBar.test.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CaseActionBar.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CsmIssuesView.tsxapps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsxapps/csm-portal/webapp/src/features/csm-operations/pages/CsmChangeRequestDetailPage.tsxapps/csm-portal/webapp/src/features/csm-recent/hooks/useRecentViews.test.tsapps/csm-portal/webapp/src/features/csm-recent/hooks/useRecentViews.tsentity-service/internal/domain/entity.goentity-service/internal/handler/case_handler.goentity-service/internal/handler/case_handler_test.goentity-service/internal/handler/decode.goentity-service/internal/service/sn_account_service.goentity-service/internal/service/sn_account_service_test.goentity-service/openapi.yaml
…ing data - useRecentViews.test.ts: the "unresolved-identity writer" test never actually left the writer's own identity unresolved before it mounted, so the resolveActiveUserKey fallback branch it claims to exercise was never hit. Reset the mock claim before the recorder mounts, then restore it before invoking the record call. - decode.go: decodeRequestWithLimit's trailing-data check treated an oversized-body error from the second Decode() the same as any other trailing data, returning the generic "must contain a single JSON object" message instead of the caller's size-limit message. Handle *http.MaxBytesError explicitly on that path, with a regression test.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@entity-service/internal/handler/decode_test.go`:
- Around line 42-50: Extend the test around decodeRequestWithLimit to assert
that the first decode succeeded by verifying dst.X equals "ok" after the call.
Keep the existing failure, status, and message assertions unchanged so the test
specifically covers oversized trailing data after a successful initial object
decode.
- Around line 32-35: Update the attachment-size error message used by the
handler in case_handler.go and the tooLargeMsg expectation in the test to
consistently state the promised 15 MiB limit, using accurate binary-size units.
Keep the test and caller messages identical.
🪄 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: 0e06f9d6-14d2-441d-8329-6a408473a03e
📒 Files selected for processing (3)
apps/csm-portal/webapp/src/features/csm-recent/hooks/useRecentViews.test.tsentity-service/internal/handler/decode.goentity-service/internal/handler/decode_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/csm-portal/webapp/src/features/csm-recent/hooks/useRecentViews.test.ts
…onger test assertion - Extracted the attachment too-large message into attachmentTooLargeMsg (case_handler.go) so the handler and both test files can no longer drift out of sync — it was previously duplicated as a literal string in each, and CodeRabbit flagged the new decode_test.go copy as inconsistent with the 15 MiB request-body cap. It's intentionally worded around the 10 MB file-size limit callers care about, not the raw request-body cap (which includes base64/JSON overhead the caller never sees) — documented in the new constant's comment. - decode_test.go: assert dst.X == "ok" after the call, proving the test actually exercises the trailing-data check (the second Decode) rather than passing because the first Decode already failed.
hidden: true was reintroduced in wso2-open-operations#1267 with no recorded rationale, three weeks after the tab shipped visible in wso2-open-operations#1049/wso2-open-operations#1079. The underlying data path (useGetCsmCaseSlas -> BFF/entity-service POST /slas/search) works and is already being fetched on page load; only the tab button was unreachable.
Purpose
CSM portal cleanup and bug fixes, landing as a stacked set of related changes on this one PR (kept as a draft while more may be added — will mark ready for review once settled). Spans the CSM webapp (FE) and the Go entity-service.
hiddenpattern already used for the Tasks tab).createdAt/updatedAt, but every backend account response shape returnscreatedOn/updatedOn; the account search request sentsearchQueryat the top level, but the backend expects it nested underfilters(matching the/cases/searchpayload shape); and the Tier value wasn't read at all for accounts coming from the alternate (non-Postgres) response shape, which names the fieldsupportTierinstead oftier(a plain string on the list view, an{id, label}ref on the detail view).Goals
One way to do each thing, not two — remove menu shortcuts once their target tab makes the same action directly reachable. Bring the change request detail page's information architecture in line with the case detail page's, which already went through this same tabbed reorganization. Make the project detail Issues tab actually show everything its filters imply it should. Make the account pages render correctly regardless of which backend response shape an account came from, and show every field the backing data source actually has for an account. Make "Recently viewed" reflect a visit as soon as it happens, in the same tab, with no reload required.
Approach
Each change is a small, independent commit — see commit messages for detail. (1) is a pure JSX relocation (no handler/state logic changes); (2) removes dead menu entries plus their now-unused handlers/state (
autoOpenCallCreateplumbing); (3) is a one-lineTAB_DEFSchange; (4) relocates existing cards into tab-content blocks with no changes to data fetching, mutations, or the recently-fixed request-approval/error-message handling; (5) is scoped to the shared issues-list component's query-filter merge (no change to the request/response contract) — an unlocked, empty type selection now resolves to the full known type list before the search request is built; (6) is FE-only — renames the account type's date fields, nests the search request'ssearchQueryunderfilters, and adds a small helper that resolves the Tier value from whichever oftier/supportTier(string or ref) the loaded account actually carries, since the FE has a singleAccounttype shared across both backend response shapes; (7) adds a scoped size-limit override for the attachment-create handler only, leaving every other endpoint's default untouched; (8) adds the SF ID field to the ServiceNow account wire struct and domain types, wired through the existing SN-to-domain mapping, with round-trip tests; (9) is FE-only, scoped to theuseRecentViewshook — extracts the resolve/migrate step into a shared helper called synchronously from both the record callback and the reader's own settle-effect, with unit coverage for the record-then-read-within-one-session scenario.Release note
Case detail page cleanup: linked service requests moved to the Related tab, three redundant menu shortcuts removed, SLAs tab hidden for now. Change request detail page reorganized into tabs (Approval, Details, Comments, Attachments), matching the case detail page. Project detail page's Issues tab now shows all issue types for the project, not only support cases. Account pages now show correct created/updated dates, the Tier value, and the SF ID for every account, and account search works correctly again. Attachment uploads no longer fail for files over ~1 MiB. "Recently viewed" now reflects a visit immediately after navigating away, without needing a page reload.
Documentation
N/A — internal UI reorganization and bug fixes, no new contract/shape changes beyond the additive SF ID field (documented in
openapi.yaml).Security checks
eslint/tscclean; N/A for Go —go vet ./...cleanRelated PRs
N/A
Summary by CodeRabbit