[CSM Portal] show state transitions and field updates in case activity stream - #1064
Conversation
|
Warning Review limit reached
Next review available in: 26 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 Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR adds a new backend "case activities" search API contract with field-change tracking, a React Query hook ( ChangesCase Activities Feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant CsmCaseDetailPage
participant useGetCsmCaseActivities
participant BackendAPI
participant auditEntryFromBeActivity
participant CaseActivitiesFeed
CsmCaseDetailPage->>useGetCsmCaseActivities: call with caseId
useGetCsmCaseActivities->>BackendAPI: POST /cases/{id}/activities/search (includeFieldChanges: true)
BackendAPI-->>useGetCsmCaseActivities: activity entries
useGetCsmCaseActivities->>useGetCsmCaseActivities: filter type === "field_change"
useGetCsmCaseActivities->>auditEntryFromBeActivity: map each entry
auditEntryFromBeActivity-->>useGetCsmCaseActivities: CaseAuditEntry[]
useGetCsmCaseActivities-->>CsmCaseDetailPage: activityAudit
CsmCaseDetailPage->>CaseActivitiesFeed: audit = activityAudit ?? []
CaseActivitiesFeed->>CaseActivitiesFeed: filter redundant timestamp changes
CaseActivitiesFeed-->>CsmCaseDetailPage: rendered field-change lines
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/csm-portal/webapp/src/api/backend/types.ts (1)
577-595: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a discriminated union instead of an optional-properties bag.
changesis documented as "only present ontype === 'field_change'" but is typed as an optional field on the shared interface, so nothing stops constructing e.g.{ type: "comment", changes: [...] }at compile time. A discriminated union ontypewould make invalid states unrepresentable and let TS narrow automatically at call sites (e.g. inauditEntryFromBeActivity).♻️ Example discriminated-union shape
-export interface BeCaseActivityEntry { - id: string; - type: BeCaseActivityType; - content?: string; - createdOn: string; - createdBy?: string; - createdByFirstName?: string; - createdByLastName?: string; - createdByFullName?: string; - /** Only present on `type === "field_change"` entries. */ - changes?: BeFieldChange[]; -} +interface BeCaseActivityEntryBase { + id: string; + content?: string; + createdOn: string; + createdBy?: string; + createdByFirstName?: string; + createdByLastName?: string; + createdByFullName?: string; +} + +export type BeCaseActivityEntry = + | (BeCaseActivityEntryBase & { type: "comment" | "attachment" }) + | (BeCaseActivityEntryBase & { type: "field_change"; changes: BeFieldChange[] });🤖 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 577 - 595, `BeCaseActivityEntry` is modeled as a single optional-properties interface even though `changes` only applies when `type` is `"field_change"`. Refactor this type in `types.ts` into a discriminated union keyed by `type`, so non-field-change entries cannot include `changes` and TypeScript can narrow automatically. Update any consumers such as `auditEntryFromBeActivity` to use the narrowed union members instead of relying on optional checks.apps/csm-portal/webapp/src/features/csm-cases/api/useCsmCaseActivities.ts (1)
42-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMapper doesn't validate
entry.typebefore mapping.
auditEntryFromBeActivityis exported and unconditionally setskind: "field_change"regardless ofentry.type. It's currently only called after the caller filters fortype === "field_change"(line 90), but nothing enforces that contract at the type level. Tying this to a discriminatedBeCaseActivityEntry(see comment ontypes.ts) would make misuse a compile error instead of a runtime footgun for future callers.🤖 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/api/useCsmCaseActivities.ts` around lines 42 - 58, auditEntryFromBeActivity is assuming a field_change entry without enforcing it, since it always returns kind: "field_change" regardless of entry.type. Update the function signature to accept only the discriminated BeCaseActivityEntry variant for field_change (or narrow inside the function before mapping), and align the BeCaseActivityEntry type definitions so misuse becomes a compile-time error. Keep the mapping logic in auditEntryFromBeActivity and the caller filter in useCsmCaseActivities consistent with the discriminated union contract.
🤖 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/CaseActivitiesFeed.tsx`:
- Around line 371-394: The CaseActivitiesFeed rendering currently leaves the
body blank when e.entry.changes exists but visibleChanges is empty after
filtering redundant timestamp updates. Update the conditional around the
changes/description rendering in CaseActivitiesFeed so it falls back to
e.entry.description whenever visibleChanges has no items, not only when
e.entry.changes is missing or empty; use the visibleChanges mapping block and
the FieldChangeLine branch as the place to make the check.
In `@apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx`:
- Around line 270-273: The activities query in CsmCaseDetailPage is missing
load/error handling, so a failed `useGetCsmCaseActivities(caseId)` request is
treated the same as “no state changes.” Update the `activityAudit` section to
destructure and use `isLoading` and `isError` from `useGetCsmCaseActivities`,
gate the skeleton with `isActivityLoading` alongside the existing loading
checks, and add an inline error notice when `isActivityError` is true, matching
the existing comments/chat handling in this file.
---
Nitpick comments:
In `@apps/csm-portal/webapp/src/api/backend/types.ts`:
- Around line 577-595: `BeCaseActivityEntry` is modeled as a single
optional-properties interface even though `changes` only applies when `type` is
`"field_change"`. Refactor this type in `types.ts` into a discriminated union
keyed by `type`, so non-field-change entries cannot include `changes` and
TypeScript can narrow automatically. Update any consumers such as
`auditEntryFromBeActivity` to use the narrowed union members instead of relying
on optional checks.
In `@apps/csm-portal/webapp/src/features/csm-cases/api/useCsmCaseActivities.ts`:
- Around line 42-58: auditEntryFromBeActivity is assuming a field_change entry
without enforcing it, since it always returns kind: "field_change" regardless of
entry.type. Update the function signature to accept only the discriminated
BeCaseActivityEntry variant for field_change (or narrow inside the function
before mapping), and align the BeCaseActivityEntry type definitions so misuse
becomes a compile-time error. Keep the mapping logic in auditEntryFromBeActivity
and the caller filter in useCsmCaseActivities consistent with the discriminated
union contract.
🪄 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
Run ID: 5128b0ca-e2d0-4956-9aab-7b6e642024e6
📒 Files selected for processing (10)
apps/csm-portal/webapp/src/api/backend/types.tsapps/csm-portal/webapp/src/constants/apiConstants.tsapps/csm-portal/webapp/src/features/csm-cases/api/useCsmCaseActivities.test.tsapps/csm-portal/webapp/src/features/csm-cases/api/useCsmCaseActivities.tsapps/csm-portal/webapp/src/features/csm-cases/api/usePatchCsmCase.tsapps/csm-portal/webapp/src/features/csm-cases/components/CaseActivitiesFeed.test.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CaseActivitiesFeed.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CaseDetailWidgets.tsxapps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsxapps/csm-portal/webapp/src/features/csm-cases/types/csmCases.ts
…y stream Wire the case activity feed's lifecycle lane to the new field-change activity endpoint. Previously the lane was always empty because the audit list was hardcoded to an empty array on the client, so state, severity, and assignee changes never appeared in the timeline even though the backend now records them. Comments, work notes, chat, and attachments keep reading from their existing endpoints/hooks — this only adds the field-change lane alongside them.
… header, drop duplicate timestamp, local tz
…ed changes (field curation moved to backend)
26fdc75 to
cc48b13
Compare
Purpose
The case activity feed's lifecycle lane ("State changes") was always empty on the client — the
auditarray was hardcoded to[]. Meanwhile the backend now exposes a dedicated activities endpoint (POST /cases/{id}/activities/search) that returns audited field/state changes for a case. This PR wires that endpoint'sfield_changeentries into the existing lifecycle lane so engineers can see when a case's state, severity, or assignee changed, by whom, and what the old/new values were.Goals
<field>: <new value> was <old value>, with the old value struck through, following the agent-workspace convention for field-change history.Approach
useGetCsmCaseActivities(new hook, mirrors the existinguseGetCsmCaseCommentspattern): callsPOST /cases/{id}/activities/searchwith a single wide page (includeFieldChanges: true), then filters the response down totype === "field_change"entries and maps them onto the existingCaseAuditEntryshape (extended with an optionalchangesarray).CaseActivitiesFeednow renders a field-change entry'schangesarray as one line per changed field (old value struck through), falling back to the legacydescriptionstring for any audit entries that don't carrychanges.CsmCaseDetailPagenow sources the "Activity timeline" audit lane from the new hook instead of the case detail response's always-emptyauditfield.usePatchCsmCase,usePatchCsmCaseById) now also invalidate the activities query on success, since a state/severity/assignee/watcher patch is audited server-side.useGetCsmCaseComments,useGetCsmConversationMessages,useGetCsmCaseAttachments). The new activities endpoint also returnscomment/attachmententries, but this PR ignores them to avoid a second, divergent read path, and because the endpoint excludes work notes entirely.Known limitations (tracked as follow-ups, not fixed here):
User stories
As a CS engineer viewing a case, I want to see when the case's state, severity, or assignee changed (and by whom) in the activity timeline, so I don't have to reconstruct that history from memory or separate audit tooling.
Release note
The case activity timeline now shows state, severity, and assignee changes (with old/new values) alongside comments and attachments, instead of leaving that lane empty.
Documentation
N/A — internal CS-engineer portal UI behavior change with no external-facing documentation.
Automation tests
useCsmCaseActivities.test.tscovering the field-change mapper (author display-name fallback order, default-to-emptychanges), andCaseActivitiesFeed.test.tsxcovering field-change rendering (single change, multiple changes in one entry, cleared value, and fallback todescriptionwhenchangesis absent).pnpm build.Security checks
pnpm lint(eslint) andtscviapnpm buildclean instead.Related PRs
Stacked on top of the CSM case comments/attachments work in #1062 and #1063. Depends on a corresponding backend/entity-service change exposing
POST /cases/{id}/activities/search, tracked separately.Migrations (if applicable)
N/A — no data migrations; purely additive read-path wiring on the frontend.
Test environment
Verified locally with
pnpm build,pnpm test, andpnpm lintagainst Node/pnpm on macOS.Learning
N/A
Summary by CodeRabbit
New Features
Bug Fixes
Tests