[CSM Portal] Add project/account contact search, closure fields, and project update endpoint - #1191
Merged
Merged
Conversation
…project update endpoint
Facilitates digiops-cs#2349/#2350 at the Go layer, on top of the
underlying Ballerina entity-service changes (separate PR).
entity-service:
- Add ClosureState + 4 closure fields to ProjectDetailsView/ProjectView
(ClosureState itself was missing from this Go layer before this change).
- Add flat closureStatus/endDateFrom/endDateTo/sortBy/sortOrder fields to
SearchProjectsRequest.
- Add dedicated SN-only ProjectContactService/AccountContactService for
the two new contacts-search endpoints.
- Add a new PATCH /projects/{id} endpoint end-to-end (ProjectUpdateService) --
this Go layer had no project-update capability at all before this change,
not even for the pre-existing hasAgent/hasKbReferences fields. Carries
those two plus the three writable closure sub-states; the overall
closureState is derived by SN and is response-only.
csm-portal-backend (BFF):
- Add opaque pass-through endpoints for the two new contacts-search routes,
matching this service's existing pass-through philosophy exactly. No
changes needed for the closure fields/filters (existing project/account
endpoints are already opaque byte pass-throughs) or for the new
PATCH /projects/{id} (deliberately left for a follow-up).
Contributor
📝 WalkthroughWalkthroughAdds ServiceNow-backed account and project contact search APIs, project closure-state filtering and mapping, and project update support. The changes span domain contracts, service implementations, conditional routing, portal proxy handlers, OpenAPI schemas, and handler tests. ChangesServiceNow API expansion
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AccountContactHandler
participant AccountContactService
participant ServiceNow
Client->>AccountContactHandler: POST account contacts search
AccountContactHandler->>AccountContactService: SearchAccountContacts(id, request)
AccountContactService->>ServiceNow: POST /accounts/{id}/contacts/search
ServiceNow-->>AccountContactService: Contact response
AccountContactService-->>AccountContactHandler: Mapped contacts and pagination
AccountContactHandler-->>Client: JSON response
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (1)
entity-service/internal/service/sn_project_service.go (1)
100-123: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUnvalidated filter/sort values forwarded straight to ServiceNow.
req.ClosureStatus,req.SortBy,req.SortOrder(and theEndDateFrom/EndDateTodate strings) are copied into the outbound payload with no validation. Unlike other enum-like fields in this service, there's novalidXxxmap check, so arbitrary strings reach the SN/projects/searchendpoint unfiltered. Since the doc comment onSearchProjectsRequest.SortByin the domain file states "Currently onlyendDateis meaningful," an invalid value should be rejected with aValidationErrorrather than silently forwarded.
As per coding guidelines, "UsevalidXxxmaps to validate enum fields and add a map entry for every new enum constant," and "Validate and reject unexpected input at the boundary... before forwarding requests to upstream services."🤖 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_project_service.go` around lines 100 - 123, Validate SearchProjects request filters before constructing snSearchProjectsPayload: add or reuse validXxx maps for ClosureStatus, SortBy, and SortOrder, enforce that SortBy accepts the documented endDate value, and validate EndDateFrom/EndDateTo using the service’s existing date validation conventions. Return a ValidationError for any invalid value, then forward only validated fields from SearchProjects.Source: Path instructions
🧹 Nitpick comments (1)
entity-service/internal/domain/entity.go (1)
350-364: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated closure-state field block across structs.
The same 5-field closure-state block (
ClosureState,EndDateClosureState,InvoiceDueDateClosureState,ComplianceViolationClosureState,ComplianceViolationDate) is now repeated verbatim inProjectDetailsViewandProjectView, and mirrored again in the SN-facingsnProject/snProjectDetailsResponsestructs insn_project_service.go. Consider extracting a shared embedded struct (e.g.ProjectClosureFields) to avoid drift when a closure field is added/renamed later.Also applies to: 428-441
🤖 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 350 - 364, The five closure-state fields are duplicated across ProjectDetailsView, ProjectView, snProject, and snProjectDetailsResponse. Extract them into a shared ProjectClosureFields struct and embed or reuse it in each affected struct, preserving the existing JSON field names and ServiceNow behavior while removing the repeated declarations.
🤖 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/backend/internal/handler/projects.go`:
- Around line 118-122: Update the project ID validation in the handler around
the PathValue("id") lookup to validate non-empty IDs against the package-level
uuidRe regex. Return HTTP 400 with ErrMsgInvalidUUID when the value does not
match, while preserving the existing ErrMsgBadRequest response for a missing ID.
In `@apps/csm-portal/backend/openapi.yaml`:
- Around line 4409-4430: The contact-search contract uses total, not
totalRecords. In apps/csm-portal/backend/openapi.yaml lines 4409-4430 and
4638-4665, rename the AccountContactSearchResponse and
ProjectContactSearchResponse fields to total; in
apps/csm-portal/backend/internal/handler/projects_test.go lines 217-248, update
the mocked payload and response assertion to use total.
- Around line 994-1047: Update the /projects/{id}/contacts/search operation: add
format: uuid to the id path parameter schema, matching the equivalent accounts
search operation, and add a 403 response using the existing ErrorPayload schema
alongside the other error responses.
In `@entity-service/internal/service/sn_project_service.go`:
- Around line 309-326: Update UpdateProject to validate each non-nil closure
state field—EndDateClosureState, InvoiceDueDateClosureState, and
ComplianceViolationClosureState—against the appropriate validXxx map before
constructing or sending the SN PATCH payload. Return the established validation
error for any unexpected value while preserving valid values and the existing
not-all-nil check.
---
Outside diff comments:
In `@entity-service/internal/service/sn_project_service.go`:
- Around line 100-123: Validate SearchProjects request filters before
constructing snSearchProjectsPayload: add or reuse validXxx maps for
ClosureStatus, SortBy, and SortOrder, enforce that SortBy accepts the documented
endDate value, and validate EndDateFrom/EndDateTo using the service’s existing
date validation conventions. Return a ValidationError for any invalid value,
then forward only validated fields from SearchProjects.
---
Nitpick comments:
In `@entity-service/internal/domain/entity.go`:
- Around line 350-364: The five closure-state fields are duplicated across
ProjectDetailsView, ProjectView, snProject, and snProjectDetailsResponse.
Extract them into a shared ProjectClosureFields struct and embed or reuse it in
each affected struct, preserving the existing JSON field names and ServiceNow
behavior while removing the repeated declarations.
🪄 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: 2d798a4d-09ab-4f52-b1a9-0a53c4ce269d
📒 Files selected for processing (15)
apps/csm-portal/backend/cmd/server/main.goapps/csm-portal/backend/internal/entity/entity.goapps/csm-portal/backend/internal/handler/accounts.goapps/csm-portal/backend/internal/handler/accounts_test.goapps/csm-portal/backend/internal/handler/helpers_test.goapps/csm-portal/backend/internal/handler/projects.goapps/csm-portal/backend/internal/handler/projects_test.goapps/csm-portal/backend/openapi.yamlentity-service/internal/domain/entity.goentity-service/internal/handler/account_handler.goentity-service/internal/handler/project_handler.goentity-service/internal/server/routes.goentity-service/internal/service/interfaces.goentity-service/internal/service/sn_account_service.goentity-service/internal/service/sn_project_service.go
…upe closure fields - Validate closureStatus/sortBy/sortOrder/endDateFrom/endDateTo in SearchProjects before forwarding to ServiceNow. closureStatus is checked against the 3-value set this service's own business rule computes; sortBy/sortOrder against the documented small set. Deliberately did NOT add enum validation to the closure sub-state write fields in UpdateProject -- unlike the overall status, those are set by an evolving SN process with a value set not fully known here (live data already showed "Pending Notified", beyond the small observed set), so a map would risk rejecting legitimate values. - Fix SearchProjectContacts (BFF) to validate the project ID against uuidRe, matching GetAccount/SearchAccountContacts' existing convention instead of the weaker non-empty-only check it mirrored from GetProject. Updated its tests to use UUID-shaped IDs and added a non-UUID rejection case. - Fix contact-search response field name mismatch: entity-service returns "total" (matching the pre-existing SearchProjectsResponse convention), but the BFF's openapi.yaml and tests said "totalRecords" -- a real contract/doc bug, not just a nitpick, since the BFF is a byte pass-through. Fixed the schema and both handler tests. - Added format:uuid + a 403 response to the project-contacts-search OpenAPI path, matching the equivalent account-contacts operation. - Extracted ProjectClosureFields (domain) / snProjectClosureFields (SN-facing) shared structs to stop the 5-field closure block from drifting across ProjectDetailsView/ProjectView/snProject/ snProjectDetailsResponse.
Contributor
Author
|
Addressed the nitpick from the review (no inline thread to reply to — it wasn't anchored to a single location): extracted |
Contributor
✅ Action performedReviews resumed. |
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReviews resumed. |
Rashmika998
approved these changes
Jul 21, 2026
5 tasks
4 tasks
This was referenced Jul 22, 2026
This was referenced Jul 22, 2026
[CSM Portal] Document account/project contacts-search endpoints in entity-service OpenAPI spec
#1213
Merged
This was referenced Jul 31, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Facilitates the Go-layer half of a broader ACP (Account Closure Process) API-facilitation effort — a corresponding Ballerina entity-service change (separate PR, tracked separately) exposes the underlying ServiceNow data; this PR carries it through the CSM entity-service and BFF.
Goals
hasAgent/hasKbReferencestoggles) are actually reachable through this layer — they weren't before this change.Approach
entity-service:
ClosureState+ 4 new closure fields (EndDateClosureState,InvoiceDueDateClosureState,ComplianceViolationClosureState,ComplianceViolationDate) added toProjectDetailsView/ProjectView. Note:ClosureStateitself was missing from this Go layer entirely before this change, despite existing upstream.SearchProjectsRequestgains flatClosureStatus/EndDateFrom/EndDateTo/SortBy/SortOrderfields — kept flat rather than nested under a newFilters/SortBystruct (unlikeSearchCasesRequest's convention) to avoid a breaking shape change to the existing flat{pagination, searchQuery}request.ProjectContactService/AccountContactServiceinterfaces (SN-only, no Postgres equivalent) backing the two newPOST .../{id}/contacts/searchendpoints — not folded into the sharedProjectService/AccountService, since those are also implemented by the Postgres backend and contacts have no PG path.PATCH /projects/{id}end-to-end (ProjectUpdateService) — this Go layer had no project-update capability at all before this change, not even for the pre-existinghasAgent/hasKbReferencesfields (upstream SN/Ballerina only). Carries those two plus the three writable closure sub-states. The overallclosureStateis derived by an SN business rule from those sub-states and is intentionally response-only, never a request field — a direct write to it would be silently overwritten by that rule, so exposing it as writable here would have been misleading.csm-portal-backend (BFF):
SearchProjectContacts/SearchAccountContacts) matching this service's existing byte-pass-through philosophy for project/account endpoints exactly — no typed decode/validation added, consistent with the sibling endpoints.PATCH /projects/{id}pass-through is deliberately left for a follow-up — out of scope for this PR.User stories
As the ACP closure-process automation (or any CSM consumer), I can read a project's/account's contacts and closure state through the CSM entity-service, filter/sort the project list by closure status and end date, and (once the BFF follow-up lands) eventually write the closure sub-states through the full stack.
Release note
CSM entity-service gains project/account contact search, additional project closure fields, closure-status filtering/sorting on project search, and a project-update endpoint. BFF gains pass-through contact-search endpoints.
Documentation
N/A — internal API additions, documented in each service's own
openapi.yaml.Security checks
go vetclean on both servicesSamples
N/A
Training / Certification / Marketing
N/A — internal API change, no user-facing UI or training material impact.
Related PRs
Migrations
N/A
Test environment
Local (macOS), Go 1.x, real ServiceNow DEV tenant via a local Ballerina instance (real Asgardeo token). Both
entity-service(go build/go vet/go test) andapps/csm-portal/backend(make vet/make test/make build) pass clean.Learning
N/A
Summary by CodeRabbit