[CSM Portal] Prefill Post Resolution Activity dialog; align resolution-code/cause labels and cause encoding with the backing data source - #1188
Conversation
|
Warning Review limit reached
Next review available in: 40 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 (11)
📝 WalkthroughWalkthroughThe PR standardizes case-cause enums across backend contracts, frontend types, and ServiceNow mappings. It adds resolved-case metadata propagation and enables ChangesCase resolution flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BeCaseView
participant detailFromBeCase
participant CsmCaseDetailPage
participant ResolutionDialog
BeCaseView->>detailFromBeCase: Provide resolution metadata
detailFromBeCase->>CsmCaseDetailPage: Map to CsmCaseDetail.resolution
CsmCaseDetailPage->>ResolutionDialog: Pass initial values
ResolutionDialog->>ResolutionDialog: Prefill resolution code, cause, and notes
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 |
…list wording
Close/Propose-solution now reopens with the case's prior resolution code,
cause, and notes instead of blank fields, using resolutionCode/cause/
resolutionNotes already returned by GET /cases/{id} but previously dropped
by the FE type and mapper.
Also replaces the mechanically title-cased resolution-code and cause labels
with the backing data source's exact picklist wording (punctuation,
capitalization, acronyms), which can't be derived from the enum token, and
aligns the cause enum/labels with the corrected 25-value taxonomy from the
entity-service change in this PR.
…acking data source domain.CaseCause previously had 16 values with no correspondence to the backing data source's real "cause" choice list (25 entries) — a fabricated taxonomy. Replaces it with one enum value per real choice, verified live against the field's picklist definition on the source's production tenant. The wire encoding also needed a fix, not just the value set: the field is a plain sequential integer (1-25) on the production tenant, not the label text the enum's naming might suggest. (The lower/test tenant configures the same field with the label as its stored value instead — a real inconsistency between tenants of the backing data source, not a bug in this mapping; production is the only tenant live traffic reaches, so that's what this now matches.) Encoding follows the same pattern already used for resolutionCode: domain enum -> integer key, sent as the string form since the downstream integration payload's cause field stays a plain string. Resolution codes were already correct end-to-end and are untouched. openapi.yaml enum updated to match in both this service and csm-portal-backend (docs only there — that service passes the request body through unvalidated).
openapi.yaml documented the previous 16-value cause enum; updates it to the corrected 25-value set from the entity-service change in this PR. Docs only — this service passes the request body through to the entity service unvalidated, so there is no behavior change here.
9f342f0 to
7fa5d52
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
entity-service/internal/service/sn_case_service.go (1)
836-882: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a round-trip unit test for
snCauseKey/snCauseByID.The mapping relies on 25 hand-written sequential integers (1-25) with no test in this batch verifying that every
domain.CaseCauseconstant has a unique key and round-trips correctly throughsnCauseByID. A simple table-driven test would catch a duplicate/missing key or off-by-one before it silently corrupts a case's cause on close.🤖 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 `@entity-service/internal/service/sn_case_service.go` around lines 836 - 882, Add a table-driven unit test covering every entry in snCauseKey, asserting each domain.CaseCause has a unique integer key and that converting the key with strconv.Itoa and looking it up in snCauseByID returns the original cause. Include checks for the expected complete mapping so duplicate, missing, or off-by-one values fail.entity-service/openapi.yaml (1)
3077-3101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared
CaseCauseschema instead of inlining the 25-value enum three times.
UpdateCaseRequest.cause,UpdatedCase.cause, andCaseView.causeeach inline the identical 25-value list.apps/csm-portal/backend/openapi.yamlalready factors the same values into a singleCaseCauseschema referenced via$ref. Doing the same here matches the Go domain type name (domain.CaseCause) and avoids the three copies drifting out of sync on the next ServiceNow choice-list change.As per coding guidelines,
entity-service/**/openapi.yamlshould "match schemas to Go domain type names."♻️ Proposed refactor
schemas: + CaseCause: + type: string + description: Root-cause category for a closed or solution-proposed case. + nullable: true + enum: + - SOLUTION_ARCHITECTURE + - DEPLOYMENT_ARCHITECTURE + - USER_ERROR_CONFIGURATION + - USER_ERROR_PRODUCT_CONCEPT + - USER_ERROR_RUNTIME + - USER_ERROR_RECOMMENDATION_BEST_PRACTICES + - CUSTOMIZATION_LIMITATION + - CUSTOMIZATION_BUG + - DOCUMENTATION_GAP + - DOCUMENTATION_ERROR + - PRODUCT_LIMITATION + - PRODUCT_BUG + - PRODUCT_REGRESSION + - PRODUCT_MIGRATION + - INFRASTRUCTURE_DATABASE + - INFRASTRUCTURE_OS + - INFRASTRUCTURE_NETWORK + - INFRASTRUCTURE_JDK + - INFRASTRUCTURE_LDAP + - INFRASTRUCTURE_LOAD_BALANCER + - INFRASTRUCTURE_IAAS + - INFRASTRUCTURE_EXTERNAL_PRODUCT + - INFRASTRUCTURE_PROXY + - INFRASTRUCTURE_OTHER + - UNKNOWNThen replace the three inline enums with
$ref: '#/components/schemas/CaseCause'(wrapped inallOfalongsidenullable/descriptionwhere those are needed, per OAS 3.0$ref-sibling rules).Also applies to: 3174-3198, 3668-3668
🤖 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 `@entity-service/openapi.yaml` around lines 3077 - 3101, Define a shared components schema named CaseCause containing the existing 25-value enum, then replace the inline cause enums in UpdateCaseRequest.cause, UpdatedCase.cause, and CaseView.cause with references to it. Preserve each field’s existing nullable and description metadata by wrapping the reference in allOf where needed, following OpenAPI 3.0 $ref rules.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@entity-service/internal/service/sn_case_service.go`:
- Around line 836-882: Add a table-driven unit test covering every entry in
snCauseKey, asserting each domain.CaseCause has a unique integer key and that
converting the key with strconv.Itoa and looking it up in snCauseByID returns
the original cause. Include checks for the expected complete mapping so
duplicate, missing, or off-by-one values fail.
In `@entity-service/openapi.yaml`:
- Around line 3077-3101: Define a shared components schema named CaseCause
containing the existing 25-value enum, then replace the inline cause enums in
UpdateCaseRequest.cause, UpdatedCase.cause, and CaseView.cause with references
to it. Preserve each field’s existing nullable and description metadata by
wrapping the reference in allOf where needed, following OpenAPI 3.0 $ref rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 26ae2b90-df29-4aae-b4f3-ca856480d850
📒 Files selected for processing (11)
apps/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/ResolutionDialog.test.tsxapps/csm-portal/webapp/src/features/csm-cases/components/ResolutionDialog.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/caseResolution.tsentity-service/internal/domain/entity.goentity-service/internal/service/sn_case_service.goentity-service/openapi.yaml
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Purpose
The CSM portal's "Close case" / "Propose a solution" dialog (
ResolutionDialog) always opened blank, even when a case already carried a prior resolution code, cause, and notes (e.g. a reopened case). Separately, its two dropdowns' displayed text was mechanically generated from the backend enum tokens, which doesn't match the backing data source's real picklist wording, and thecausefield's backend enum/label set didn't correspond to any real choice in that picklist at all — sending it would not have round-tripped correctly in production.Goals
causefield's backend enum and wire encoding actually match the backing data source's real "cause" choice list (verified live against the field's picklist definition), so the value written back is meaningful.Approach
useGetCsmCaseDetailnow maps the case's existingresolutionCode/cause/resolutionNotes(already returned byGET /cases/{id}, previously dropped by the FE type) onto a newresolutionfield;ResolutionDialogaccepts aninitialprop to seed its fields when opening. Added explicit label maps (RESOLUTION_CODE_LABELS,CASE_CAUSE_LABELS) with the verbatim source wording for both dropdowns, replacing mechanical title-casing (which can't reproduce the source's punctuation, capitalization, or acronyms likeIAAS/JDK/LDAP).domain.CaseCauseis replaced with a 25-value enum matching the real "cause" picklist (previously 16 fabricated values). The backing data source encodes this field as a plain sequential integer (1-25) in production rather than a label string — verified live against the field's picklist definition — so the mapping follows the same pattern already used forresolutionCode(domain enum <-> integer, sent as the string form since the downstream integration payload's cause field stays a plain string). Resolution codes were already correct end-to-end and are untouched.openapi.yamlenum updated to match (docs only — this service passes the request body through unvalidated).User stories
As a CS engineer reopening a previously-resolved case, I see its prior resolution code, cause, and notes prefilled when I close it or propose a solution again, instead of having to re-enter them from scratch. As a CS engineer picking a resolution code or cause, I see the backing source's actual wording in the dropdown.
Release note
Fixed: the case resolution dialog (Close/Propose solution) now prefills from the case's existing resolution data, shows the exact picklist wording for resolution codes and causes, and writes cause values the backing data source actually recognizes.
Documentation
N/A — internal contract fix; no customer- or partner-facing documentation covers this field's value set.
Automation tests
ResolutionDialog.test.tsxcovers prefill-and-resubmit, and dropdown selection now targets the picklist-wording labels;go test ./...passes for entity-service.Security checks
go vetandeslintran clean insteadSamples
N/A
Related PRs
None
Migrations (if applicable)
N/A — no schema/table changes. Note for reviewers: this changes the domain
CaseCauseenum's wire values; any already-closed/solution-proposed case whosecausewas previously written using the old fabricated enum will not decode against the new mapping (returnsnil/omitted rather than a wrong value) — it does not corrupt existing data.Test environment
Go 1.26, Node/pnpm (per
apps/csm-portal/webapp/package.json), macOS (host, no devcontainer). Verified the cause mapping locally against the backing data source's live picklist definition.Learning
Traced the actual wire path end-to-end (FE -> csm-portal-backend passthrough -> entity-service -> downstream integration) before changing anything, since the backend openapi enum turned out not to reflect any real choice in the source picklist. Read the source's own picklist definition directly (read-only, via an authenticated session) rather than guessing values from a UI screenshot, and caught a real discrepancy between two environments of the backing data source in how the same field is configured before finalizing the mapping.