[Customer Entity] Expand cases search - add case/engagement types, unified filter model, and SN enum mapping - #919
Conversation
…and engagements endpoints
… and response models
…enum-based filters
…unified response shape
… and response schema
|
Warning Review limit reached
More reviews will be available in 27 minutes and 31 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughReplaces the ChangesPriority-to-Severity Migration and Specialized Search Endpoint Removal
Sequence Diagram(s)sequenceDiagram
participant Client
participant CaseHandler
participant case_service
participant snCaseService
participant ServiceNow
rect rgba(135, 206, 235, 0.5)
note over Client,ServiceNow: SearchCases (post-migration)
Client->>CaseHandler: POST /cases/search {severityKeys, engagementTypeKeys}
CaseHandler->>case_service: SearchCases(req)
case_service->>case_service: validate severityKeys, engagementTypeKeys, limit≤50
case_service->>snCaseService: SearchCases(req)
snCaseService->>snCaseService: domainTypeKeysToSN, domainSeveritiesToSNIDs, domainEngagementTypesToSNIDs
snCaseService->>ServiceNow: POST search {caseTypes, severityKeys, engagementTypeKeys}
ServiceNow-->>snCaseService: snCase[] with severity/engagementType labels
snCaseService->>snCaseService: map labels via snSeverityLabelStr, snLabelStr
snCaseService-->>case_service: SearchCasesResponse
case_service-->>Client: 200 {cases, totalRecords, offset, limit}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
entity-service/internal/repository/case_repo.go (1)
338-350: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the deployed-product filter.
SearchCasesstill filters by project and deployment, but the deployed-product filter block is missing, so requests scoped todeployedProductIDswill return cases from every deployed product in the selected deployment/project scope.Proposed fix
if len(req.Filters.DeploymentIDs) > 0 { where += fmt.Sprintf(" AND c.deployment_id = ANY($%d::uuid[])", argIdx) filterArgs = append(filterArgs, req.Filters.DeploymentIDs) argIdx++ } + + if len(req.Filters.DeployedProductIDs) > 0 { + where += fmt.Sprintf(" AND c.deployed_product_id = ANY($%d::uuid[])", argIdx) + filterArgs = append(filterArgs, req.Filters.DeployedProductIDs) + argIdx++ + } if len(req.Filters.StateKeys) > 0 {🤖 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/repository/case_repo.go` around lines 338 - 350, The deployed-product filter block is missing from the SearchCases function's filter construction logic. After the DeploymentIDs filter block (which checks if len(req.Filters.DeploymentIDs) > 0), add a similar filter block for deployedProductIDs that checks if len(req.Filters.DeployedProductIDs) > 0, appends the appropriate SQL WHERE clause using the ANY operator with a placeholder parameter (incrementing argIdx), adds the filter argument to filterArgs, and increments argIdx. This will ensure that when deployedProductIDs are provided in the request, only cases from those deployed products are returned.entity-service/internal/service/sn_case_service.go (2)
223-230: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject unmapped enum filters instead of dropping them.
The severity and engagement converters silently omit unknown values, and
SearchCasessends the shortened slices. An unsupportedseverityKeys/engagementTypeKeysvalue can therefore broaden results instead of returning a validation error.🐛 Proposed fix
-func domainSeveritiesToSNIDs(severities []domain.CaseSeverity) []int { +func domainSeveritiesToSNIDs(severities []domain.CaseSeverity) ([]int, error) { ids := make([]int, 0, len(severities)) for _, s := range severities { - if id, ok := snSeverityIDMap[s]; ok { - ids = append(ids, id) + id, ok := snSeverityIDMap[s] + if !ok { + return nil, &apierror.ValidationError{Msg: "severityKeys contains invalid value: " + string(s)} } + ids = append(ids, id) } - return ids + return ids, nil }Apply the same pattern to
domainEngagementTypesToSNIDs, then build the payload from validated values:+ severityKeys, err := domainSeveritiesToSNIDs(req.Filters.SeverityKeys) + if err != nil { + return domain.SearchCasesResponse{}, err + } + engagementTypeKeys, err := domainEngagementTypesToSNIDs(req.Filters.EngagementTypeKeys) + if err != nil { + return domain.SearchCasesResponse{}, err + } + payload := snCaseSearchPayload{ Filters: snCaseFilters{ CaseTypes: snCaseTypes, SearchQuery: req.Filters.SearchQuery, ProjectIDs: uuidsToSysids(req.Filters.ProjectIDs), DeploymentIDs: uuidsToSysids(req.Filters.DeploymentIDs), StateKeys: domainStatesToSNIDs(req.Filters.StateKeys), - SeverityKeys: domainSeveritiesToSNIDs(req.Filters.SeverityKeys), + SeverityKeys: severityKeys, IssueTypeKeys: domainIssueTypesToSNIDs(req.Filters.IssueTypeKeys), - EngagementTypeKeys: domainEngagementTypesToSNIDs(req.Filters.EngagementTypeKeys), + EngagementTypeKeys: engagementTypeKeys,Also applies to: 243-250, 1025-1028
🤖 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 223 - 230, The functions domainSeveritiesToSNIDs and domainEngagementTypesToSNIDs silently omit unmapped enum values instead of returning an error, which causes shortened filter slices to be sent to SearchCases and potentially broadens results. Modify both functions to return an error as a second return value and return early if an unmapped value is encountered in the iteration. Update the callers of these functions (around lines 243-250 and 1025-1028) to check the error and only build the search payload with validated values, ensuring that unsupported severity or engagement type values trigger a validation error instead of being silently dropped.
175-183: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMap reopened responses back to
CaseStateReopened.Line 180 adds outbound SN support for
CaseStateReopened, but the inbound label map still decodes"reopened"asCaseStateWaitingOnWSO2, soGetCaseByID/UpdateCasecan return the wrong state for reopened cases.🐛 Proposed fix
var snCaseStateMap = map[string]domain.CaseState{ "open": domain.CaseStateOpen, "work in progress": domain.CaseStateWorkInProgress, "waiting on wso2": domain.CaseStateWaitingOnWSO2, "awaiting info": domain.CaseStateAwaitingInfo, - "reopened": domain.CaseStateWaitingOnWSO2, + "reopened": domain.CaseStateReopened, "solution proposed": domain.CaseStateSolutionProposed, "closed": domain.CaseStateClosed, }🤖 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 175 - 183, The outbound map snStateIDMap correctly maps CaseStateReopened to state ID 1006, but there is an inbound label map that converts ServiceNow state IDs back to domain states which is incorrectly mapping the reopened state (ID 1006) to CaseStateWaitingOnWSO2 instead of CaseStateReopened. This causes GetCaseByID and UpdateCase to return the wrong state for reopened cases. Find the inbound state mapping (likely the reverse mapping function or map that decodes ServiceNow state responses) and update it to correctly map state ID 1006 or the "reopened" label to CaseStateReopened instead of CaseStateWaitingOnWSO2.
🧹 Nitpick comments (2)
entity-service/internal/repository/case_repo.go (1)
317-323: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard against an unmapped sort field at the repository boundary.
If validation is bypassed or a new
CaseSortFieldis added without updatingpgSortColMap,sortColbecomes empty and the generatedORDER BYis invalid. Default to a safe column or return a validation error here too.Proposed fix
sortCol := pgSortColMap[req.SortBy.Field] + if sortCol == "" { + sortCol = pgSortColMap[domain.CaseSortFieldCreatedOn] + } sortDir := string(req.SortBy.Order)Also applies to: 435-436
🤖 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/repository/case_repo.go` around lines 317 - 323, Add validation logic after looking up the sort field in pgSortColMap to guard against unmapped CaseSortField values. When retrieving sortCol from pgSortColMap, check if the result is empty (indicating an unmapped field). If empty, either return a validation error to the caller or default to a safe column like "c.created_at" to ensure the ORDER BY clause is always valid. This validation should occur at the repository boundary where pgSortColMap is used, not just rely on external validation.entity-service/internal/domain/entity.go (1)
577-578: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix the stale filter contract comment.
These filters are domain enum strings now; ServiceNow translates them internally. The current “integer IDs” wording contradicts the request contract and can mislead downstream implementers.
Suggested wording
-// StateKeys, SeverityKeys, IssueTypeKeys, and EngagementTypeKeys use the same -// integer IDs as the ServiceNow integration layer. +// StateKeys, SeverityKeys, IssueTypeKeys, and EngagementTypeKeys use domain +// enum string values; ServiceNow translates them to its internal IDs.🤖 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/domain/entity.go` around lines 577 - 578, The comment for StateKeys, SeverityKeys, IssueTypeKeys, and EngagementTypeKeys incorrectly states that these use the same integer IDs as the ServiceNow integration layer. Update the comment to accurately reflect that these are domain enum strings and that ServiceNow handles the translation of these enum strings internally, removing the outdated reference to integer IDs.
🤖 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/case_handler.go`:
- Line 67: The comment for the PatchCase handler at line 67 is missing
workStateKey from the list of accepted fields. Update the comment to include
workStateKey alongside state and severity as an accepted field from both data
sources, ensuring the documentation accurately reflects all parameters that the
handler actually accepts.
In `@entity-service/internal/service/case_service.go`:
- Around line 308-327: The SearchCases function is missing validation for
typeKeys in the request filters, while it properly validates stateKeys,
severityKeys, issueTypeKeys, and engagementTypeKeys. Add a validation loop for
req.Filters.TypeKeys that checks each value against a validCaseType map (similar
to the existing validCaseState, validCaseSeverity, validCaseIssueType, and
validEngagementType maps) and returns a ValidationError with an appropriate
error message if any invalid type key is found, following the exact same pattern
as the four existing validation loops.
- Around line 295-297: The SearchCases service method validates that the
pagination limit cannot exceed 50, but the shared OpenAPI Pagination schema
specification still allows up to 100, creating a contract mismatch that will
cause client errors. Update the OpenAPI Pagination schema definition to set the
maximum limit cap to 50 to match the actual validation constraint in the
SearchCases method, ensuring the contract accurately reflects what the service
will accept.
In `@entity-service/internal/service/sn_case_service.go`:
- Around line 136-143: The domainTypeKeysToSN function currently returns an
empty slice for both empty typeKeys input and invalid typeKeys that don't exist
in snCaseTypeMap, making it impossible to distinguish between "no filter
provided" and "invalid filter values provided". Modify domainTypeKeysToSN to
separate these two cases by returning a different signal (such as a nil slice,
an error return value, or using a different return type) when typeKeys contains
unsupported values that don't exist in snCaseTypeMap. Then update SearchCases
(and the related code around lines 1014-1017) to check this signal and properly
validate that invalid typeKeys values cause validation to fail rather than
silently falling back to default_case behavior.
- Around line 1019-1028: The snCaseFilters struct has a deployedProductIds field
that is not being populated when constructing the snCaseSearchPayload around the
Filters assignment. Add a line to set the DeployedProductIDs field in the
snCaseFilters, using the uuidsToSysids conversion function on
req.Filters.DeployedProductIDs just like the other ID filter fields (ProjectIDs,
DeploymentIDs, etc.) are being handled in the payload construction.
In `@entity-service/openapi.yaml`:
- Around line 321-322: Update the PATCH endpoint description in the OpenAPI
schema to use the correct property names. Replace the field names `state`,
`severity`, and `workState` with their actual schema equivalents `stateKey`,
`severityKey`, and `workStateKey` in the prose description that explains which
fields can be updated. This ensures the documentation accurately reflects the
actual PATCH request properties that clients should use.
- Around line 1316-1320: The stateKeys enum at the specified location includes
the reopened state, but this state is missing from the corresponding state enums
in the UpdateCaseRequest, UpdatedCase, Case, and CaseView schema definitions.
Add reopened to the enum list in all four of these case state schemas to ensure
consistency with stateKeys and prevent generated clients from rejecting valid
updates or responses. Apply the same enum values across all case state
definitions throughout the OpenAPI spec.
---
Outside diff comments:
In `@entity-service/internal/repository/case_repo.go`:
- Around line 338-350: The deployed-product filter block is missing from the
SearchCases function's filter construction logic. After the DeploymentIDs filter
block (which checks if len(req.Filters.DeploymentIDs) > 0), add a similar filter
block for deployedProductIDs that checks if len(req.Filters.DeployedProductIDs)
> 0, appends the appropriate SQL WHERE clause using the ANY operator with a
placeholder parameter (incrementing argIdx), adds the filter argument to
filterArgs, and increments argIdx. This will ensure that when deployedProductIDs
are provided in the request, only cases from those deployed products are
returned.
In `@entity-service/internal/service/sn_case_service.go`:
- Around line 223-230: The functions domainSeveritiesToSNIDs and
domainEngagementTypesToSNIDs silently omit unmapped enum values instead of
returning an error, which causes shortened filter slices to be sent to
SearchCases and potentially broadens results. Modify both functions to return an
error as a second return value and return early if an unmapped value is
encountered in the iteration. Update the callers of these functions (around
lines 243-250 and 1025-1028) to check the error and only build the search
payload with validated values, ensuring that unsupported severity or engagement
type values trigger a validation error instead of being silently dropped.
- Around line 175-183: The outbound map snStateIDMap correctly maps
CaseStateReopened to state ID 1006, but there is an inbound label map that
converts ServiceNow state IDs back to domain states which is incorrectly mapping
the reopened state (ID 1006) to CaseStateWaitingOnWSO2 instead of
CaseStateReopened. This causes GetCaseByID and UpdateCase to return the wrong
state for reopened cases. Find the inbound state mapping (likely the reverse
mapping function or map that decodes ServiceNow state responses) and update it
to correctly map state ID 1006 or the "reopened" label to CaseStateReopened
instead of CaseStateWaitingOnWSO2.
---
Nitpick comments:
In `@entity-service/internal/domain/entity.go`:
- Around line 577-578: The comment for StateKeys, SeverityKeys, IssueTypeKeys,
and EngagementTypeKeys incorrectly states that these use the same integer IDs as
the ServiceNow integration layer. Update the comment to accurately reflect that
these are domain enum strings and that ServiceNow handles the translation of
these enum strings internally, removing the outdated reference to integer IDs.
In `@entity-service/internal/repository/case_repo.go`:
- Around line 317-323: Add validation logic after looking up the sort field in
pgSortColMap to guard against unmapped CaseSortField values. When retrieving
sortCol from pgSortColMap, check if the result is empty (indicating an unmapped
field). If empty, either return a validation error to the caller or default to a
safe column like "c.created_at" to ensure the ORDER BY clause is always valid.
This validation should occur at the repository boundary where pgSortColMap is
used, not just rely on external validation.
🪄 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: 8e2ccb6c-9cbc-43e7-a52e-8963fa8b421f
📒 Files selected for processing (10)
entity-service/internal/domain/entity.goentity-service/internal/handler/case_handler.goentity-service/internal/repository/case_repo.goentity-service/internal/server/routes.goentity-service/internal/service/case_service.goentity-service/internal/service/interfaces.goentity-service/internal/service/sn_case_service.goentity-service/migrations/000008_create_cases.down.sqlentity-service/migrations/000008_create_cases.up.sqlentity-service/openapi.yaml
…yKey, workStateKey
…to case state enums
…ervice PR wso2-open-operations#919 - Add typeKeys filter (support, service_request, security_report_analysis, announcement, engagement) - Add engagementTypeKeys filter (migration, consultancy, new_feature_improvement, follow_up, onboarding) - Remove deployedProductIds filter (removed from entity service SearchCasesFilters) - Update sortBy.field enum: created_at/updated_at/closed_at -> createdOn/updatedOn/severity/state Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove SearchServiceRequests, SearchSecurityReportAnalyses, SearchEngagements endpoints (deleted upstream in entity service) - Rename priorityKey -> severityKey throughout (request bodies, filters, openapi spec) - Add reopened case state with transitions: closed -> reopened -> work_in_progress - Remove orphaned openapi.yaml component schemas for the three removed endpoints - Update README and CLAUDE.md to reflect severity rename and removed endpoints Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ignment [CSM Portal][BE] Align with entity service PR #919 breaking changes
Summary
Migration: Add
case_type_enum(support/service_request/security_report_analysis/announcement/engagement) andengagement_type_enum(migration/consultancy/new_feature_improvement/follow_up/onboarding) to the cases table. Makeseveritynullable with CHECK constraints (non-null for support cases only),engagement_typenon-null for engagement cases only, and restrict announcement cases toopen/closedstates.Rename priority → severity: Rename
prioritycolumn andcase_priority_enumtoseverity/case_severity_enumthroughout migration, domain, repository, service, and OpenAPI spec. AdoptKey/Keyssuffix convention on request fields (severityKey,stateKey, etc.).Remove SN-only endpoints: Drop
/service-requests/search,/security-report-analyses/search, and/engagements/search— handlers, service interface methods, Postgres stubs, SN implementations, routes, domain types, and OpenAPI paths/schemas all removed.Unified cases search: Expand
POST /cases/searchto cover all case types. Filters now accept enum strings (stateKeys,severityKeys,issueTypeKeys,engagementTypeKeys,typeKeys) rather than integers — Postgres passes them directly to SQL, ServiceNow maps them to integer IDs. Response adopts a unifiedCaseSearchViewwith nullable fields for SN-only data (catalog,catalogItem,assignedTeam,conversation). Response shape changes tototalRecords(dropshasMore).Summary by CodeRabbit
New Features
Breaking Changes
createdOn,updatedOn,severity,state)./service-requests/search,/security-report-analyses/search, and/engagements/searchendpoints.totalRecords,offset,limit).