[Customer Entity] Add search endpoint and standardise enum field naming - #929
Conversation
…oss domain, service, repository, and handler layers
|
Warning Review limit reached
More reviews will be available in 3 minutes and 39 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 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 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 (3)
📝 WalkthroughWalkthroughThe PR removes all ChangesField Rename Cleanup (cases, comments, deployments)
New Change-Request Search Feature
Sequence DiagramsequenceDiagram
participant Client
participant ChangeRequestHandler
participant snChangeRequestService
participant Context
participant IntegrationClient
Client->>ChangeRequestHandler: POST /change-requests/search
ChangeRequestHandler->>ChangeRequestHandler: decode SearchChangeRequestsRequest
ChangeRequestHandler->>snChangeRequestService: SearchChangeRequests(ctx, req)
snChangeRequestService->>Context: extract x-user-id-token
Context-->>snChangeRequestService: userToken (or UnauthorizedError)
snChangeRequestService->>snChangeRequestService: normalize pagination, validate filters/sort/dates
snChangeRequestService->>snChangeRequestService: map domain states/impacts → SN numeric keys
snChangeRequestService->>IntegrationClient: POST /change-requests/search (snPayload)
IntegrationClient-->>snChangeRequestService: raw JSON response
snChangeRequestService->>snChangeRequestService: unmarshal SN DTOs, normalize labels, build SearchChangeRequestView[]
snChangeRequestService-->>ChangeRequestHandler: SearchChangeRequestsResponse{Total, Limit, Offset, ChangeRequests}
ChangeRequestHandler-->>Client: 200 application/json
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
entity-service/internal/service/sn_case_service.go (2)
1019-1038: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject unsupported enum filters before building the SN payload.
domainStatesToSNIDs,domainSeveritiesToSNIDs,domainIssueTypesToSNIDs, anddomainEngagementTypesToSNIDssilently skip unmapped values, so an invalid filter can be dropped and broaden the search. Validate each list before converting, likeTypesalready does. As per coding guidelines: “UsevalidXxxmaps ... to validate enum fields.”Proposed validation
for _, t := range req.Filters.Types { if _, ok := snCaseTypeMap[t]; !ok { return domain.SearchCasesResponse{}, &apierror.ValidationError{Msg: "types contains invalid value: " + t} } } + for _, state := range req.Filters.States { + if _, ok := snStateIDMap[state]; !ok { + return domain.SearchCasesResponse{}, &apierror.ValidationError{Msg: "states contains invalid value or unsupported ServiceNow value: " + string(state)} + } + } + for _, severity := range req.Filters.Severities { + if _, ok := snSeverityIDMap[severity]; !ok { + return domain.SearchCasesResponse{}, &apierror.ValidationError{Msg: "severities contains invalid value or unsupported ServiceNow value: " + string(severity)} + } + } + for _, issueType := range req.Filters.IssueTypes { + if _, ok := snIssueTypeIDMap[issueType]; !ok { + return domain.SearchCasesResponse{}, &apierror.ValidationError{Msg: "issueTypes contains invalid value or unsupported ServiceNow value: " + string(issueType)} + } + } + for _, engagementType := range req.Filters.EngagementTypes { + if _, ok := snEngagementTypeIDMap[engagementType]; !ok { + return domain.SearchCasesResponse{}, &apierror.ValidationError{Msg: "engagementTypes contains invalid value or unsupported ServiceNow value: " + string(engagementType)} + } + } snCaseTypes := domainTypeKeysToSN(req.Filters.Types)🤖 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 1019 - 1038, Reject unsupported enum filter values before constructing the snCaseSearchPayload in the SN case search flow. In sn_case_service.go, add explicit validation for req.Filters.States, req.Filters.Severities, req.Filters.IssueTypes, and req.Filters.EngagementTypes using the corresponding validXxx maps, similar to the existing Types check, and return a ValidationError on any invalid value. This should happen before calling domainStatesToSNIDs, domainSeveritiesToSNIDs, domainIssueTypesToSNIDs, and domainEngagementTypesToSNIDs so unmapped values cannot be silently dropped.Source: Coding guidelines
317-330: 🎯 Functional Correctness | 🟡 MinorUpdate the case-type validation message The error still says
typeKey; change it totype must be "support" for case creation.🤖 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 317 - 330, The case-type validation in snCaseService create flow still uses the old field name in its error text. Update the validation message in the req.Type check inside the case creation path to say type must be "support" for case creation, and keep the change localized around the snCaseTypeMap / snCreateCasePayload setup so the response matches the current API terminology.
🧹 Nitpick comments (1)
entity-service/internal/service/sn_change_request_service.go (1)
91-99: 📐 Maintainability & Code Quality | 🔵 TrivialRemove the unused change-request type map
snCRTypeIDMapisn’t referenced anywhere in this service.snChangeRequestFiltershas no type field, and the search payload never sends one, so this map is dead code.If a type filter is planned, wire it into the payload and add
validChangeRequestTypevalidation; otherwise, drop the map.🤖 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_change_request_service.go` around lines 91 - 99, Remove the dead code by deleting snCRTypeIDMap from sn_change_request_service.go since it is not referenced by snChangeRequestFilters or any payload-building path. If you intend to support a change-request type filter later, wire that through the service methods that build the search payload and add validation via validChangeRequestType; otherwise keep the service focused on the existing fields only.
🤖 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/domain/entity.go`:
- Around line 334-340: Restore the request enum field naming contract in
SearchDeploymentsRequest and the related request types by keeping the Key/Keys
suffix in both Go field names and JSON/OpenAPI tags, since decodeRequest will
reject documented enum inputs otherwise. Update the affected domain structs and
any matching service/repo/OpenAPI references so fields like stateKey and
typeKeys remain accepted, and only proceed with the rename if the PR explicitly
includes the approved compatibility plan and contract update. Use the existing
request type symbols (such as SearchDeploymentsRequest and the other affected
request structs) to align all layers consistently.
In `@entity-service/internal/service/case_service.go`:
- Around line 135-136: The validation error in case creation still references
the old field name, so update the message in the case creation path to mention
the accepted `type` field instead of `typeKey`. Fix the error returned from the
`CreateCase`/case validation branch in `case_service.go` so callers are told to
set `type` to "support" when `req.Type` is invalid.
In `@entity-service/internal/service/sn_change_request_service.go`:
- Around line 244-287: SearchChangeRequests currently validates states, impacts,
and sort fields but skips req.Filters.ProjectIDs, so malformed project IDs can
reach uuidsToSysids unchecked. Add validateUUIDs("projectIds",
req.Filters.ProjectIDs) near the other filter validations in
sn_change_request_service.go before building snChangeRequestSearchPayload, so
invalid IDs return an apierror.ValidationError at the service boundary.
In `@entity-service/openapi.yaml`:
- Around line 678-696: The POST /change-requests/search OpenAPI operation is
missing the required 404 response for the ServiceNow-only route, so update the
change-requests search operation in openapi.yaml to include a standard
ErrorResponse 404 entry alongside the existing 200 and 400 responses. Keep the
response block consistent with the other writable endpoints and ensure the
SearchChangeRequestsResponse operation contract matches the conditional
registration behavior.
---
Outside diff comments:
In `@entity-service/internal/service/sn_case_service.go`:
- Around line 1019-1038: Reject unsupported enum filter values before
constructing the snCaseSearchPayload in the SN case search flow. In
sn_case_service.go, add explicit validation for req.Filters.States,
req.Filters.Severities, req.Filters.IssueTypes, and req.Filters.EngagementTypes
using the corresponding validXxx maps, similar to the existing Types check, and
return a ValidationError on any invalid value. This should happen before calling
domainStatesToSNIDs, domainSeveritiesToSNIDs, domainIssueTypesToSNIDs, and
domainEngagementTypesToSNIDs so unmapped values cannot be silently dropped.
- Around line 317-330: The case-type validation in snCaseService create flow
still uses the old field name in its error text. Update the validation message
in the req.Type check inside the case creation path to say type must be
"support" for case creation, and keep the change localized around the
snCaseTypeMap / snCreateCasePayload setup so the response matches the current
API terminology.
---
Nitpick comments:
In `@entity-service/internal/service/sn_change_request_service.go`:
- Around line 91-99: Remove the dead code by deleting snCRTypeIDMap from
sn_change_request_service.go since it is not referenced by
snChangeRequestFilters or any payload-building path. If you intend to support a
change-request type filter later, wire that through the service methods that
build the search payload and add validation via validChangeRequestType;
otherwise keep the service focused on the existing fields only.
🪄 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: f3a468f8-76e8-484a-a90f-66305620c6f6
📒 Files selected for processing (11)
entity-service/CLAUDE.mdentity-service/internal/domain/entity.goentity-service/internal/handler/change_request_handler.goentity-service/internal/repository/case_repo.goentity-service/internal/repository/deployment_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/internal/service/sn_change_request_service.goentity-service/openapi.yaml
…ion, and document 404 response
…fix, add change-requests search - Remove Key/Keys suffix from all enum request fields to match entity service: stateKey→state, severityKey→severity, workStateKey→workState (PATCH case), typeKey→type, issueTypeKey→issueType (create case/comment), typeKeys→types, stateKeys→states, severityKeys→severities, engagementTypeKeys→engagementTypes, issueTypeKeys→issueTypes (case search), deploymentTypeKeys→deploymentTypes (deployment search) - Update PatchCase handler to read state/workState from new JSON field names - Update CreateCaseComment handler to read type from new JSON field name - Add POST /change-requests/search endpoint: entity client method, handler, interface, mock, tests, route registration, and openapi spec - Update openapi.yaml, README.md, and CLAUDE.md throughout Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
[CSM Portal][BE] align with entity service PR #929: drop Key/Keys suffix, add change-requests search
Summary
POST /change-requests/searchendpoint backed by ServiceNow, following the same pattern as case search (domain types → service interface → SN service implementation → handler → route → OpenAPI spec)totalfield across all data sources (SN internaltotalRecordsmapped to domaintotal)Key/Keyssuffix from enum fields in request structs across domain, services, repositories, and OpenAPI spec (e.g.StateKeys→States,TypeKey→Type)Summary by CodeRabbit
New Features
Bug Fixes
totalacross responses.state,severity,type, andissueTypeinstead of legacy field names.