[CSM Portal] case work-state actions, claim flow, and v2 contract realignment - #923
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (16)
📝 WalkthroughWalkthroughThe PR aligns the CSM portal frontend with a new backend OpenAPI contract: renames ChangesCases Domain Upgrade
Sequence Diagram(s)sequenceDiagram
participant Engineer
participant CsmCaseDetailPage
participant useFindMyOngoingCases
participant BackendApi
participant usePatchCsmCaseById
participant usePatchCsmCase
Engineer->>CsmCaseDetailPage: action: start work / resume work
CsmCaseDetailPage->>usePatchCsmCase: PATCH stateKey=work_in_progress
CsmCaseDetailPage->>useFindMyOngoingCases: findMyOngoingCases(currentCaseId)
useFindMyOngoingCases->>BackendApi: POST /cases/search stateKeys:[work_in_progress]
BackendApi-->>useFindMyOngoingCases: BeCaseSearchView[]
useFindMyOngoingCases->>useFindMyOngoingCases: filter workState=ongoing, assignee email, excludeId
useFindMyOngoingCases-->>CsmCaseDetailPage: MyOngoingCase[]
alt No conflicts
CsmCaseDetailPage->>usePatchCsmCase: PATCH workStateKey=ongoing
CsmCaseDetailPage->>Engineer: success feedback
else Has conflicts
CsmCaseDetailPage->>Engineer: show pauseConflict dialog
Engineer->>CsmCaseDetailPage: onConfirmStartWork
CsmCaseDetailPage->>usePatchCsmCaseById: PATCH workStateKey=paused (each conflict case)
CsmCaseDetailPage->>usePatchCsmCase: PATCH workStateKey=ongoing
CsmCaseDetailPage->>Engineer: success feedback
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 6
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/api/useGetCsmCases.ts (1)
214-235: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMap the assigned engineer returned by search.
BeCaseSearchViewnow exposesassignedEngineer, but the list mapper still renders every row asUnassigned, so assigned cases show incorrect assignee data in the list.Suggested mapping fix
const cases: CsmCaseRow[] = (casesResponse.cases ?? []).map((c) => { const projectId = c.project?.id ?? ""; const accountId = projectAccount.get(projectId) ?? ""; + const assignee = + c.assignedEngineer?.name?.trim() || + c.assignedEngineer?.email || + "Unassigned"; return { @@ - // No assignee field on the backend yet; surfaced as "Unassigned". - assignee: "Unassigned", + assignee, assigneeIsMe: false,🤖 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/useGetCsmCases.ts` around lines 214 - 235, The case mapper in useGetCsmCases is hardcoding assignee as "Unassigned" and assigneeIsMe as false, but the backend response now includes the assignedEngineer field. Update the map function that transforms casesResponse.cases into CsmCaseRow objects to extract and use the actual assignedEngineer data from the case object c instead of these hardcoded values, determining the assignee name and whether it matches the current user based on the assignedEngineer field returned by the backend.
🧹 Nitpick comments (2)
apps/csm-portal/webapp/src/features/csm-cases/components/CsmCaseCommentInput.tsx (1)
37-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid effect-driven state correction for the locked reply mode.
setInternal(true)insideuseEffectconflicts with the repo’sreact-hooks/set-state-in-effectconvention. Move the correction to render-time, or derive aneffectiveInternal = publicReplyLocked || internalvalue and use that for UI/submission reads.♻️ Proposed localized fix
- useEffect, useMemo,const publicReplyLocked = !!publicCommentDisabledReason; - useEffect(() => { - if (publicReplyLocked && !internal) setInternal(true); - }, [publicReplyLocked, internal]); + if (publicReplyLocked && !internal) { + setInternal(true); + }Based on learnings, apps/csm-portal/webapp follows the
react-hooks/set-state-in-effectrule: do not callsetStateinsideuseEffect; use React’s render-time adjustment pattern instead.Also applies to: 209-211
🤖 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/CsmCaseCommentInput.tsx` at line 37, The CsmCaseCommentInput component violates the react-hooks/set-state-in-effect convention by calling setInternal(true) inside a useEffect hook around line 37 (and again around lines 209-211). Instead of modifying state within the effect, create a derived value at render-time by computing effectiveInternal = publicReplyLocked || internal, and use this derived value throughout the component for UI rendering and form submission logic rather than relying on the state mutation within the effect. This ensures the locked reply mode correction happens during the render phase rather than as an effect side-effect.Source: Learnings
apps/csm-portal/webapp/src/api/backend/mappers.ts (1)
27-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFinish the severity terminology in the mapper API.
The backend type is now
BeCaseSeverity, but the exported helper names still sayPriority, which leaks the removed contract vocabulary into new severity-key call sites.Suggested rename
-export function severityFromPriority( - priority: BeCaseSeverity | undefined, +export function severityFromBeSeverity( + severity: BeCaseSeverity | undefined, ): Severity { - switch (priority) { + switch (severity) { @@ -export function priorityFromSeverity(severity: Severity): BeCaseSeverity { +export function beSeverityFromUiSeverity(severity: Severity): BeCaseSeverity {Then update imports/call sites that currently call
severityFromPriorityorpriorityFromSeverity.🤖 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/mappers.ts` around lines 27 - 68, The function names severityFromPriority and priorityFromSeverity use outdated terminology since the backend type is now BeCaseSeverity instead of BeCasePriority. Rename severityFromPriority to severityFromBeSeverity and priorityFromSeverity to beSeverityFromSeverity to maintain consistent severity terminology throughout the mapper API. Then update all call sites throughout the codebase that reference these old function names with the new ones.
🤖 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 186-195: The typeKey field in the BeCaseCreatePayload interface
currently accepts any BeCaseType value, but the comment indicates the portal
only creates support cases. Narrow the type of the typeKey field in
BeCaseCreatePayload from BeCaseType to a literal type constraint that only
allows the support case type value, preventing callers from accidentally passing
non-creatable case types while still compiling.
In `@apps/csm-portal/webapp/src/features/csm-cases/api/useProjectSearch.ts`:
- Around line 109-114: The getNextPageParam function in useProjectSearch.ts has
a pagination guard issue where if hasMore is true but the backend returns an
empty projects page, the offset calculation using allPages.reduce will not
advance (remaining at 0), causing subsequent scroll-triggered fetches to
repeatedly hit the same page. To fix this, add logic to ensure the next offset
advances even when a page returns no projects. Consider either: keeping track of
a minimum offset increment when hasMore is true, or adding a safety check that
advances the offset by at least 1 when the current page is empty but hasMore
indicates more data exists. This prevents infinite loops of requests fetching
the same empty page.
In
`@apps/csm-portal/webapp/src/features/csm-cases/components/AsyncProjectMultiSelect.tsx`:
- Around line 68-74: The useInfiniteProjectSearch hook destructuring in
AsyncProjectMultiSelect is missing an error state that should be captured
alongside the existing properties (projects, isFetching, isFetchingNextPage,
hasNextPage, fetchNextPage). Add the error property to the destructured values
from useInfiniteProjectSearch, then at line 148 where the empty state message is
rendered, add a conditional check to display an explicit error message when the
error state exists instead of defaulting all non-fetching empty states to "No
projects found".
In `@apps/csm-portal/webapp/src/features/csm-cases/components/CaseActionBar.tsx`:
- Around line 207-218: The condition in the CaseActionBar component currently
requires caseDetail.workState to be truthy before showing the toggle_work_state
action, but this hides the action for work_in_progress cases where workState is
null. Remove the caseDetail.workState check from the condition so that the
action is shown for all work_in_progress cases where assigneeIsMe is true. Then
update the paused variable logic to treat anything other than "ongoing"
(including null) as a paused state, so that null workState cases show "Resume
work" as the label.
In `@apps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseDetailPage.tsx`:
- Around line 477-481: In the toggle_work_state action handler, split the logic
based on the direction of the transition. When transitioning from "paused" to
"ongoing" (resuming work), route through the same conflict detection flow that
is used when starting work (using findMyOngoingCases and the confirmation
dialog), rather than directly calling patchCase.mutate(). Keep the direct patch
for the "ongoing" to "paused" transition. This ensures that resuming a paused
case checks for existing ongoing cases before allowing the state change,
preventing engineers from having multiple concurrent active cases.
- Around line 393-398: The try-catch block around the findMyOngoingCases
function call is silently ignoring lookup failures and continuing with an empty
others array, which allows the case to be marked as ongoing without verifying if
another case is already active. Instead of catching and continuing with the
empty array, remove the catch block or re-throw the error so that the transition
is blocked when the ongoing-case lookup fails, ensuring the error is properly
surfaced rather than allowing the operation to proceed with incomplete
information.
---
Outside diff comments:
In `@apps/csm-portal/webapp/src/features/csm-cases/api/useGetCsmCases.ts`:
- Around line 214-235: The case mapper in useGetCsmCases is hardcoding assignee
as "Unassigned" and assigneeIsMe as false, but the backend response now includes
the assignedEngineer field. Update the map function that transforms
casesResponse.cases into CsmCaseRow objects to extract and use the actual
assignedEngineer data from the case object c instead of these hardcoded values,
determining the assignee name and whether it matches the current user based on
the assignedEngineer field returned by the backend.
---
Nitpick comments:
In `@apps/csm-portal/webapp/src/api/backend/mappers.ts`:
- Around line 27-68: The function names severityFromPriority and
priorityFromSeverity use outdated terminology since the backend type is now
BeCaseSeverity instead of BeCasePriority. Rename severityFromPriority to
severityFromBeSeverity and priorityFromSeverity to beSeverityFromSeverity to
maintain consistent severity terminology throughout the mapper API. Then update
all call sites throughout the codebase that reference these old function names
with the new ones.
In
`@apps/csm-portal/webapp/src/features/csm-cases/components/CsmCaseCommentInput.tsx`:
- Line 37: The CsmCaseCommentInput component violates the
react-hooks/set-state-in-effect convention by calling setInternal(true) inside a
useEffect hook around line 37 (and again around lines 209-211). Instead of
modifying state within the effect, create a derived value at render-time by
computing effectiveInternal = publicReplyLocked || internal, and use this
derived value throughout the component for UI rendering and form submission
logic rather than relying on the state mutation within the effect. This ensures
the locked reply mode correction happens during the render phase rather than as
an effect side-effect.
🪄 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: 01e8fac4-ad14-4fbc-b2d4-1a9a2c6bf9c1
📒 Files selected for processing (29)
apps/csm-portal/webapp/src/api/backend/client.tsapps/csm-portal/webapp/src/api/backend/mappers.tsapps/csm-portal/webapp/src/api/backend/types.tsapps/csm-portal/webapp/src/components/StateChip.tsxapps/csm-portal/webapp/src/config/csmNavItems.tsapps/csm-portal/webapp/src/features/csm-admin/pages/CsmAdminLayout.tsxapps/csm-portal/webapp/src/features/csm-cases/api/mocks/casesMocks.tsapps/csm-portal/webapp/src/features/csm-cases/api/useFindMyOngoingCases.tsapps/csm-portal/webapp/src/features/csm-cases/api/useGetCsmCaseDetail.tsapps/csm-portal/webapp/src/features/csm-cases/api/useGetCsmCases.tsapps/csm-portal/webapp/src/features/csm-cases/api/usePatchCsmCase.tsapps/csm-portal/webapp/src/features/csm-cases/api/useProjectSearch.tsapps/csm-portal/webapp/src/features/csm-cases/api/useQuickCaseSearch.tsapps/csm-portal/webapp/src/features/csm-cases/components/AssignEngineerDialog.tsxapps/csm-portal/webapp/src/features/csm-cases/components/AsyncProjectMultiSelect.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CaseActionBar.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CasesFilterBar.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CasesList.tsxapps/csm-portal/webapp/src/features/csm-cases/components/CsmCaseCommentInput.tsxapps/csm-portal/webapp/src/features/csm-cases/pages/CsmCaseCreatePage.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/caseWorkState.test.tsapps/csm-portal/webapp/src/features/csm-cases/utils/caseWorkState.tsapps/csm-portal/webapp/src/features/csm-dashboard/api/useCaseComposition.tsapps/csm-portal/webapp/src/features/csm-dashboard/api/useCaseCountsMatrix.tsapps/csm-portal/webapp/src/features/csm-dashboard/types/abtDashboard.tsapps/csm-portal/webapp/src/utils/correlationId.test.tsapps/csm-portal/webapp/src/utils/correlationId.ts
…e-case on resume, list assignee mapping, project-search guards
|
@coderabbitai review |
✅ Action performedReview finished.
|
The merge-base changed after approval.
…lignment
Builds on the case-write-contract work with the case work-state lifecycle,
the start-work claim flow, identity-aware gating, list/nav UX polish, and a
realignment of the FE contract layer to the latest entity/BFF changes.
Work state + claim flow
- Surface the work sub-state (ongoing/paused) on the case detail header.
- Pause/Resume the work sub-state from the case action-bar "More" menu,
shown only when the case is in progress and assigned to the current user.
- Starting work on a case (open/waiting -> in progress) enforces a single
active case: look up the engineer's other ongoing cases, move this case to
in progress, then either mark it ongoing (no conflict) or prompt to pause
the other ongoing case(s) and make this one active.
- Resolve assignee == current user from assignedEngineer.email vs the JWT
email (the search has no assignee filter, so "my ongoing cases" is found by
state on the server + assignee/ongoing match on the client; null work state
is never treated as ongoing).
UX
- Render case lifecycle state as a solid status chip (shared StateChip),
consistent on the list and detail; left-align all case-list columns.
- Rename the "Administration" nav item and page heading to "Settings".
- Cases project filter loads the first page on open and lazy-loads more on
scroll (and narrows as you type).
- Rename the FE correlation header to X-CSM-Correlation-ID to match the
gateway-safe backend header.
v2 contract realignment
- PATCH /cases/{id}: stateKey / severityKey / workStateKey (was state /
priority / workState).
- Case create: typeKey + severityKey + issueTypeKey.
- Case search filters: severityKeys (was priorityKeys); drop unsupported
fields. Responses: severity (was priority); add reopened state and case type.
…ect); fix search deployedProduct ref shape
…e-case on resume, list assignee mapping, project-search guards
556ffcf to
91fe568
Compare
Depends on PR #893 - merge that first please.
Work state + claim flow
assignedEngineer.emailvs the JWT email. Search has no assignee/work-state filter, so "my ongoing cases" is narrowed bystateKeys:[work_in_progress]server-side and matched on assignee +workState === "ongoing"client-side (null work state is never ongoing).UX
StateChip), consistent on list + detail; all case-list columns left-aligned.X-CSM-Correlation-IDto match the gateway-safe backend header.v2 contract realignment
PATCH /cases/{id}:stateKey/severityKey/workStateKey.typeKey+severityKey+issueTypeKey.severityKeys(waspriorityKeys); responses:severity(waspriority); addedreopenedstate and casetype.Notes
Testing
pnpm buildgreen ·pnpm test90 passed ·pnpm lintcleanSummary by CodeRabbit
Release Notes
New Features
Improvements