[CSM Portal] Lossless dashboard click-through, cases-list advanced filters, and case acknowledgement - #1333
Conversation
The preview URL encoded only each filter entry's field and values, dropping the op, and the parser hardcoded op:"in" on the way back. Two consequences, both silent: - notIn was INVERTED. A widget filtering `tag notIn [s_dip]` serialised to `tag=s_dip` and decoded as `tag in [s_dip]`, so the "View more" page showed exactly the cases the tile excludes. Reproduced live: a tile showing 5 rows opened a preview showing 0. - Value-less ops were dropped entirely, because the serialiser skipped any entry with no values. `assignedUserId isEmpty` disappeared, widening "Unassigned Cases" into all cases; likewise `escalation isNotEmpty`. The op is now encoded in the query param as `field~op`. The default `in` keeps the bare `field=values` form, so links shared before this change still resolve. Value-less ops round-trip with an empty value rather than being skipped. The doc comment asserting "every dashboard case-filter widget uses op:in only" was true when written and is not anymore -- tag exclusions, unassigned and escalation filters all ship today. Corrected rather than left to license the same bug again. Found by live click-through testing against real data, not by any unit test; added four round-trip tests so it stays fixed.
…s already filter by Widgets filtering integrationCsTeam/tag/projectOnboardingStatus/ taskSLABusinessElapsedPercent/escalation/escalationLevel/projectType/date ranges click through to the cases list, which had no way to represent any of them -- the entire condition set was silently dropped. Verified live three times: a tile reading 2 landed on a list of 30, the org-wide figure. This is the data layer only: CasesFilters gains the fields (CasesFilterBar.tsx), the URL codec reads/writes them (casesFiltersUrl.ts), the search payload builder emits the field/op/values DSL entries for them (caseSearchPayload.ts), and the dashboard's translateCaseDashboardFilters passes every one of them through instead of documenting the drop as an accepted limitation. Filter-bar UI controls are separate scope. Field names/ops are taken directly from entity-service's caseFilterFieldSet/ParseCaseFieldFilters (case_filters.go), not guessed -- a mismatch there is silently accepted by SN and returns wrong counts. Op-awareness (the hard requirement): the dashboard preview URL shipped this exact bug once already (6a90597) -- it encoded field+values only, so `tag notIn [x]` decoded as `tag in [x]` (an exclusion became a filter) and value-less ops (isEmpty/isNotEmpty) were dropped for having no values to serialize. That fix needed a field~op query-param encoding because it serializes an opaque, arbitrary field/op array. This codec deliberately does NOT reuse field~op. CasesFilters is a fixed, named-field struct, not a generic array, so every op that would otherwise collide on one field name already gets its own dedicated field instead: tags (op:in) vs. excludeTags (op:notIn) as two arrays; slaElapsedPctGte/Lte and the createdOn/updatedOn/closedOn Gte/Lte pairs as one param per bound; hasEscalation as an explicit true/false/null tri-state rather than an op name a caller could typo. There is no default op to silently fall back to here, because every op already has its own field -- the field~op failure mode is structurally unreachable. Reuse field~op only if CasesFilters ever grows a generic filter escape hatch. Tests added (casesFiltersUrl.test.ts, caseSearchPayload.test.ts, widgetResourceConfig.test.ts) specifically target the shipped-bug shape: tag notIn survives without becoming tag in; escalation's value-less ops survive instead of being dropped; a gte+lte range round-trips both bounds; and a full dashboard filter set (team + tag notIn + state + onboarding + escalation + SLA% + date range) survives translateCaseDashboardFilters -> casesHref -> readCasesFiltersFromUrl unchanged. Confirmed the notIn/range tests do NOT compile against the pre-change CasesFilters type (the fields didn't exist at all), so they're proven to test something real, not restate already-passing behavior. Existing URL params (states, severities, caseTypes, assignees, workStates, projects, engagementTypes, productNames, search) are untouched; a prior `tags` no-op test is updated since `tags` is now a live, wired-through param rather than a stale removed one.
…filter chips for the rest CasesFilters grew ten fields so dashboard click-through can be lossless (see e4283d5), but the bar itself only surfaces the two broadly-useful ones: CS team (a MultiSelectField backed by the existing useTeams hook, showing team display names rather than the raw groupId) and tags, split into two TagsMultiSelect instances (Tags / Exclude tags) rather than one control with an include/exclude toggle -- the two fields are independent and both may be set at once, which a single toggle couldn't represent. The remaining eight fields (onboarding status, SLA % bounds, escalation presence/level, project type, the three date ranges) get no bar control, but now render as removable chips whenever present in the URL, visible regardless of whether the filter grid is expanded -- otherwise a user landing on a dashboard-filtered list has no way to see or undo why it's filtered.
…ust slices Every pie/donut tile's own surface was inert -- only a slice wedge or its legend row navigated anywhere, each straight to that slice's own filtered list. Clicking the tile itself (its header, padding, or an empty-state chart) went nowhere, unlike a "count" tile with the same base filters. DashboardWidgetTile's pie/bar branch now attaches the same click-through a "count" tile would produce for the widget's own base filters directly to the Card (role="button", keyboard-activatable), while every nested interactive element -- the slice wedge/bar, the legend row, the refresh button -- stops click propagation so it doesn't also re-trigger the tile-level navigation underneath it. Legend rows also pick up proper role/tabIndex/keyboard handling they were missing (mouse-only before). Also verified `abt_sla_at_risk` (>=80% elapsed) and `abt_sla_violations` (>=100% elapsed) now produce distinct hrefs now that `taskSLABusinessElapsedPercent` survives translation -- they used to be byte-identical destinations for two different-looking tiles.
…em as chips Per the product owner: the CS team, Tags and Exclude tags controls are advanced and optional, and three extra always-visible selects cluttered the filter bar. A better home for advanced filters is still to be designed. Removing the controls alone would have regressed the thing they were added for: buildActiveFilterChips deliberately skipped csTeams/tags/excludeTags precisely BECAUSE they had bar controls, so without them a dashboard click-through would land on a filtered list with those filters invisible and unclearable. They now render as chips like every other URL-only filter. The team registry is still fetched to label the team chip, but only when a csTeams filter is actually set, so the cases page does not pay for it on every load. An unresolved team falls back to its group id rather than hiding the chip. Nothing in the URL codec, search payload or dashboard translator changed, so click-through stays lossless.
Acknowledgement is a first-write-wins claim that an engineer has seen a case and picked it up, distinct from assignment. It already exists in the backing data source and is already driven by an out-of-band one-click link; this exposes it through the platform API so the internal portal can drive it too. - CaseView.acknowledgedBy on the case detail response - UpdateCaseRequest.acknowledge, mutually exclusive with every other field and rejected outright when false (there is no unacknowledge path) - UpdatedCase.number/alreadyAcknowledged/acknowledgedBy echoed back on the acknowledge path only - openapi.yaml updated for all three, so the operation is reachable through the gateway and not just locally Already-acknowledged is a success, not an error: the caller has to be able to render "already acknowledged by X". Tests cover both mapping directions, the null-acknowledgement case (must stay nil, not an empty reference), the already-acknowledged response, and the three rejection paths.
PATCH /cases/{id} forwards its body verbatim to the entity service, so
`acknowledge` needs no handler code — but it does need to be in the spec, or the
operation is unreachable through the gateway even though it works locally.
- openapi.yaml: `acknowledge` on UpdateCaseRequest (including the oneOf, which
enforces the exactly-one-field contract), and `number` /
`alreadyAcknowledged` / `acknowledgedBy` on the echoed UpdatedCase
- openapi.yaml: `acknowledgedBy` on CaseView
- PatchCase doc comment: spell out why fields like this need no local handling,
since only state and workState are pre-validated here (their guards depend on
the case's current state)
Adds an Acknowledge button to the case action bar, immediately left of the state control. It appears only when nobody has acknowledged the case yet and the severity is S0-S3 — S4 is excluded to match the severities that raise an acknowledgement notification in the first place, so the button shows up on exactly the cases an engineer could already have acknowledged out of band. Acknowledgement is first-write-wins, so the button disappears once the case is claimed rather than becoming a no-op. Losing the race is not an error: the response reports who claimed it first, and that name goes into the feedback banner. - BeCaseView.acknowledgedBy, BeCaseUpdatePayload's `acknowledge: true` variant (typed as the literal true, since there is no unacknowledge), and the echoed number/alreadyAcknowledged/acknowledgedBy on BeUpdatedCase - CsmCaseDetail.acknowledgedBy, mapped in useGetCsmCaseDetail with an email fallback for blank display names - Acknowledge in-flight state is tracked separately from the lifecycle patch, so the two buttons do not spin each other Outlined rather than contained: claiming a case is a lighter act than moving it through its lifecycle and must not out-shout the primary transition. Tests cover the S0-S3 gate, the S4 exclusion, the already-acknowledged case, the no-handler case (no dead button), the click, and the in-flight disabled state.
|
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 (9)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThe PR adds first-write-wins case acknowledgement across the entity service and CSM portal. It also adds advanced case-filter persistence and dashboard translation, active-filter chips, and keyboard-accessible dashboard chart interactions. ChangesCase acknowledgement
Advanced case filters
Dashboard chart interactions
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
apps/csm-portal/webapp/src/features/csm-cases/components/CasesFilterBar.test.tsx (1)
143-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSet a resolved value on
postMockin this suite.The first suite calls
postMock.mockResolvedValue({ teams: [] }). This suite only callsmockReset(), sopostMockreturnsundefined. The test at line 154 setscsTeams: ["g1"], which enables theuseTeamsquery, and the query function then operates onundefined. The assertion still passes through the id fallback, but the test depends on a rejected query rather than on the intended empty-teams path.Mirror the first suite's
beforeEach.♻️ Proposed change
beforeEach(() => { postMock.mockReset(); + postMock.mockResolvedValue({ teams: [] }); });🤖 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/CasesFilterBar.test.tsx` around lines 143 - 145, Update the beforeEach setup for this test suite to reset postMock and set its resolved value to an empty teams response, matching the first suite. Ensure the useTeams query exercised by the csTeams configuration follows the intended successful empty-teams path rather than receiving undefined.apps/csm-portal/webapp/src/features/csm-dashboard/config/widgetResourceConfig.ts (1)
216-257: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMatch the remaining array fields on op as well as field.
caseFilterValuesmatches onfieldonly and ignoresop.csTeams,onboardingStatuses,escalationLevels, andprojectTypestherefore translate any op into an inclusion. A widget that setsintegrationCsTeam notIn [...]orprojectType notIn [...]would decode as an inclusion — the same inversion this PR fixes fortag.No current widget appears to use those ops, so this is a latent risk rather than a live bug. Use
caseFilterEntry(fieldFilters, <field>, "in")for these fields so a non-inop is skipped instead of silently inverted.♻️ Proposed change (same pattern for the other three fields)
- const csTeams = caseFilterValues(fieldFilters, "integrationCsTeam"); + const csTeams = caseFilterEntry(fieldFilters, "integrationCsTeam", "in")?.values; if (csTeams && csTeams.length > 0) out.csTeams = csTeams;Run the following script to check whether any shipped widget config uses a non-
inop on these fields:#!/bin/bash # Description: Find dashboard widget filter entries using ops other than `in`. fd -e json -e yaml -e go . --exec rg -n -C3 '"(integrationCsTeam|projectOnboardingStatus|escalationLevel|projectType)"' {} \; rg -n -C3 '"op"\s*:\s*"(notIn|isEmpty|isNotEmpty|gt|lt)"' --glob '!**/node_modules/**'🤖 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-dashboard/config/widgetResourceConfig.ts` around lines 216 - 257, Update the array-field extraction in the widget filter decoding flow to match both field and operation: replace caseFilterValues usage for csTeams, onboardingStatuses, escalationLevels, and projectTypes with caseFilterEntry(..., "in")?.values. Preserve the existing non-empty checks and output assignments so non-in operations are skipped rather than treated as inclusions.
🤖 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/api/backend/types.ts`:
- Around line 802-805: The backend OpenAPI CaseFieldFilter.field schema must
match the frontend BeCaseFieldFilterField additions. Update the CaseFieldFilter
definition in apps/csm-portal/backend/openapi.yaml to add
taskSLABusinessElapsedPercent, escalationLevel, and escalation to its enum and
include corresponding entries in the description block.
In
`@apps/csm-portal/webapp/src/features/csm-cases/components/CasesFilterBar.test.tsx`:
- Around line 29-31: Mock `@config/apiConfig` alongside `@api/backend/client` at the
top of CasesFilterBar.test.tsx, before any imports or test code that can
transitively load CasesFilterBar and useTeams. Ensure the mock prevents
module-load access to undefined window.config while preserving the existing
backend client mock.
In `@apps/csm-portal/webapp/src/features/csm-cases/components/CasesFilterBar.tsx`:
- Around line 222-228: Update formatDateBound so YYYY-MM-DD inputs are parsed as
local calendar dates before calling toLocaleDateString, preventing the displayed
date from shifting by timezone. Preserve the existing parsing and raw-string
fallback behavior for RFC3339 and malformed values.
In `@apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx`:
- Around line 690-717: Shared patchCase pending state leaks between acknowledge
and lifecycle actions. In CsmCaseDetailPage.tsx lines 690-717, retain
isAcknowledging for the acknowledge flow and use it to derive the lifecycle
action pending prop at lines 1721-1722 as patchCase.isPending &&
!isAcknowledging; in CaseActionBar.tsx lines 465-485, disable the Acknowledge
button when either isAcknowledging or isPending is true.
In `@apps/csm-portal/webapp/src/features/csm-cases/utils/casesFiltersUrl.ts`:
- Around line 54-59: Update the explanatory comment near the tags/excludeTags
fields in casesFiltersUrl.ts to remove the stale reference to the deleted
TagsMultiSelect import comment and instead direct readers to
buildActiveFilterChips as the remaining filter surface.
In
`@apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.test.tsx`:
- Around line 659-687: Add separate Space-key activation tests for the focused
tile and legend row, alongside the existing Enter coverage. Use the relevant
DashboardWidgetTile test cases and dispatch a Space key event, then verify
navigation reaches the expected destination with the same filters (including the
tile’s “open” state filter).
In
`@apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.tsx`:
- Around line 268-274: Restructure DashboardWidgetTile so the Card’s
role="button" navigation target is a separate sibling from the refresh
IconButton and pie legend rows with role="button". Keep the tile click and
keyboard navigation behavior on the standalone target, while rendering the
refresh and chart controls outside it so their interactive semantics remain
exposed.
---
Nitpick comments:
In
`@apps/csm-portal/webapp/src/features/csm-cases/components/CasesFilterBar.test.tsx`:
- Around line 143-145: Update the beforeEach setup for this test suite to reset
postMock and set its resolved value to an empty teams response, matching the
first suite. Ensure the useTeams query exercised by the csTeams configuration
follows the intended successful empty-teams path rather than receiving
undefined.
In
`@apps/csm-portal/webapp/src/features/csm-dashboard/config/widgetResourceConfig.ts`:
- Around line 216-257: Update the array-field extraction in the widget filter
decoding flow to match both field and operation: replace caseFilterValues usage
for csTeams, onboardingStatuses, escalationLevels, and projectTypes with
caseFilterEntry(..., "in")?.values. Preserve the existing non-empty checks and
output assignments so non-in operations are skipped rather than treated as
inclusions.
🪄 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: 90d94d72-89ec-4463-90d7-2d50f8f4e894
📒 Files selected for processing (26)
apps/csm-portal/backend/internal/handler/cases.goapps/csm-portal/backend/openapi.yamlapps/csm-portal/webapp/src/api/backend/types.tsapps/csm-portal/webapp/src/features/csm-cases/api/useGetCsmCaseDetail.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/CasesFilterBar.test.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CasesFilterBar.tsxapps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsxapps/csm-portal/webapp/src/features/csm-cases/types/csmCases.tsapps/csm-portal/webapp/src/features/csm-cases/utils/caseSearchPayload.test.tsapps/csm-portal/webapp/src/features/csm-cases/utils/caseSearchPayload.tsapps/csm-portal/webapp/src/features/csm-cases/utils/casesFiltersUrl.test.tsapps/csm-portal/webapp/src/features/csm-cases/utils/casesFiltersUrl.tsapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardBarChart.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardPieChart.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.tsxapps/csm-portal/webapp/src/features/csm-dashboard/config/widgetResourceConfig.test.tsapps/csm-portal/webapp/src/features/csm-dashboard/config/widgetResourceConfig.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/widgetPreviewUrl.test.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/widgetPreviewUrl.tsentity-service/internal/domain/entity.goentity-service/internal/service/sn_case_acknowledgement_test.goentity-service/internal/service/sn_case_service.goentity-service/openapi.yaml
…nLevel, escalation to CaseFieldFilter enum The portal backend's own CaseFieldFilter.field enum stopped at parentId and never picked up the three values the frontend already sends and the entity-service already accepts, so the portal's documented request contract had drifted from what actually works. Bring the enum and its per-field description entries in line with entity-service/openapi.yaml.
…ight
`new Date("2026-07-27")` is parsed as UTC midnight; toLocaleDateString() then
renders the previous day for any user behind UTC. Pin a bare YYYY-MM-DD bound
to local midnight before formatting instead. Also mock @config/apiConfig in
the filter-bar test alongside @api/backend/client -- both read window.config
at module load, and this suite's CasesFilterBar -> useTeams chain can reach
the API layer.
…atchCase.isPending Acknowledge and every other case-update action reuse the same patchCase mutation, so acknowledging a case also flipped isPending true and spun the primary lifecycle button, and the Acknowledge button itself never respected isPending -- both could end up in flight at once. Pass patchCase.isPending && !isAcknowledging down as the lifecycle isPending prop, and disable Acknowledge on isAcknowledging || isPending.
Pointed at "the comment above TagsMultiSelect's import in CasesFilterBar.tsx", which this branch already removed. Point at buildActiveFilterChips instead, the surface these filters actually render through now.
…ng, not an ancestor, of its nested controls
Both the pie/bar tile's role="button" Card and the count tile's
component={RouterLink} Card wrapped the refresh IconButton (and, for
pie, the chart's own role="button" legend rows) inside an element that
itself carries an interactive role. That demotes every nested control's
role to presentational for assistive tech, so screen readers can fail to
expose them even though they're still clickable.
Restructure both shapes so the tile-level click target is an absolutely
positioned sibling behind (zIndex: 0) a pointerEvents: "none" content
layer, with pointer events switched back on for the refresh button and
the chart specifically. The pie/bar target keeps its role="button" plus
Enter/Space keydown handling (a real anchor only activates on Enter);
the count target stays a real RouterLink as before, now with its own
aria-label since its visible content no longer lives inside it.
Also add the Space-key activation tests CodeRabbit flagged as missing
(test names claimed Enter/Space coverage but only exercised Enter) and a
regression test asserting the tile-level target never contains the
refresh button or a legend row as a DOM descendant.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Purpose
Goals
Approach
Cases-list advanced filters.
translateCaseDashboardFilterspassed only 7 filter fields and silently droppedtag,integrationCsTeam,projectOnboardingStatus,taskSLABusinessElapsedPercent,escalation/escalationLevel,projectTypeand all date ranges — and every widget uses at least one.CasesFilters, the URL codec (casesFiltersUrl.ts), the search payload (caseSearchPayload.ts) and the translator now carry them. No backend change was needed: the BFF, entity-service, Ballerina and SN already supported every one of these filters, which is why the widgets themselves query correctly — this was purely a frontend representation gap.The measured symptom, confirmed three times: a tile reading 2 opened a list of 30, a tile reading 0 opened 16, another 0 opened 11 — each destination total landing exactly on the org-wide figure.
Encoding.
CasesFiltersis a fixed named-field struct, so rather than encoding an op per field, each op that could collide gets its own field:tags(in) vsexcludeTags(notIn),slaElapsedPctGte/Lte, andhasEscalationas atrue/false/nulltri-state. There is no default op to fall back to, so the inversion failure mode below is structurally unreachable here rather than merely tested against.Filter-op bug in the widget preview URL (real, shipped). The preview URL encoded field + values but dropped the
op, and the parser hardcodedop:"in". Sotag notIn [s_dip]round-tripped astag in [s_dip]— the "View more" page showed precisely the cases the tile excludes (reproduced: tile 5 rows → preview 0 rows). Value-less ops (isEmpty/isNotEmpty) were dropped entirely, silently widening "Unassigned Cases" into all cases. Now encoded asfield~op; the defaultinkeeps the barefield=valuesform so links shared before this change still resolve. The doc comment asserting "every dashboard widget usesop:inonly" was true when written and is not anymore — corrected rather than left to license the same bug again.Filter-bar controls. CS-team and tag controls were added, then removed as UI clutter (advanced, rarely hand-picked; a better home for advanced filters is still to be designed). Removing them alone would have regressed the thing they were added for, since the chip builder deliberately skipped those fields because they had controls — they now render as removable active-filter chips like every other URL-only filter, which is what makes a dashboard-filtered arrival self-explanatory.
Pie/bar tiles previously had no click-through at all; only legend rows navigated, carrying
statealone. The tile now navigates to its own filters, and slices merge the tile's filters with the slice's.Case acknowledgement (cherry-picked): entity-service read/write, CSM-portal API documentation, and the acknowledge action on the case detail page. Companion to the Ballerina layer in a corresponding entity-service change, tracked separately. Depends on SN update set
1264-S1224-T92-CST-SajithE.User stories
Release note
Documentation
Automation tests
Security checks
Related PRs
Migrations (if applicable)
Test environment
Summary by CodeRabbit
New Features
Bug Fixes