feat(customer-portal): add accounts/updates/scim modules and next 5 endpoints - #1302
Conversation
…ndpoints
Adds PATCH /users/me, POST /accounts/search, GET /accounts/{id},
GET /updates/product-update-levels, and POST /updates/levels/search to
backend-v2, along with two new upstream service clients modeled on
apps/csm-portal/backend's conventions:
- internal/updates: WSO2 Updates service client. Its own types are
already portal-shaped camelCase (mapper.go translates the upstream
snake_case), so handlers write its results directly with no further
dto layer.
- internal/scim: SCIM operations client, used for phone number
read/update on GET/PATCH /users/me (name/timezone/roles still come
from entity-service).
Account endpoints normalize entity-service's two data-source-dependent
response shapes (Postgres vs ServiceNow) into one consistent portal DTO
via internal/dto/account.go, rather than exposing an OpenAPI oneOf like
apps/csm-portal/backend does for the same ambiguity.
Also adds .choreo/component.yaml and openapi.yaml (covering all 11
routes implemented so far), matching apps/csm-portal/backend's
Choreo/OpenAPI conventions.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 38 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 Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdded Customer Portal API v2 contracts and wiring for 11 routes. The changes add account operations, user PATCH support, SCIM phone integration, Updates service operations, OAuth2 configuration, DTO mappings, and documentation. ChangesCustomer Portal API v2
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant UserHandler
participant EntityService
participant SCIMService
Client->>UserHandler: PATCH /users/me
UserHandler->>SCIMService: Update phone number
UserHandler->>EntityService: Update timezone
SCIMService-->>UserHandler: Updated phone number
EntityService-->>UserHandler: Updated timezone
UserHandler-->>Client: UserUpdateResponse
sequenceDiagram
participant Client
participant UpdatesHandler
participant UpdatesService
Client->>UpdatesHandler: Search update descriptions
UpdatesHandler->>UpdatesService: POST update search request
UpdatesService-->>UpdatesHandler: Update descriptions
UpdatesHandler-->>Client: Grouped update-level response
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 |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
apps/customer-portal/backend-v2/openapi.yaml (1)
588-598: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueReconsider exposing
podandclassificationas public search filters.The account DTO guideline treats
Podas an internal routing field and preferstieroverclassification.AccountSummaryandAccountDetailsfollow that rule. The request schema still acceptspodandclassification. This lets a portal client filter on internal fields that it cannot read back.If entity-service requires these filter names, keep them and add a description that explains the internal origin. If not, rename
classificationtotierand droppod.As per coding guidelines: "preferring populated equivalent fields such as
tieroverclassification; never exposeArrTodayorPod".🤖 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/customer-portal/backend-v2/openapi.yaml` around lines 588 - 598, Update the filters schema to stop exposing internal pod and classification fields: remove pod and rename classification to tier, unless entity-service requires those exact names; in that case, retain them and add descriptions identifying their internal origin. Keep the schema aligned with AccountSummary and AccountDetails, which use tier and do not expose Pod.Source: Coding guidelines
apps/customer-portal/backend-v2/internal/dto/account.go (1)
57-123: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the shared account-field mapping.
MapSearchAccounts(Line 60 through Line 73) andMapAccountDetails(Line 109 through Line 121) duplicate the same field-by-field normalization:firstNonNil(a.Tier, a.Classification),mapAccountReffor both owner fields,firstNonNilBoolfor both boolean flags, and the shared date fields. OnlySupportTierhandling and the ID/Name/UpdatedOn fields differ. Extracting a small helper that takes the common inputs (tier, classification, region, owner refs/IDs, dates, flags) and returns the shared subset would reduce the risk of the two mappers drifting apart when one is updated without the other.🤖 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/customer-portal/backend-v2/internal/dto/account.go` around lines 57 - 123, Extract the duplicated normalization logic from MapSearchAccounts and MapAccountDetails into a shared helper for tier, region, owner references, activation/deactivation dates, boolean flags, and created date. Have both mappers reuse that helper while retaining their distinct SupportTier, ID/Name, and UpdatedOn handling, preserving the existing output behavior.apps/customer-portal/backend-v2/internal/entity/users.go (1)
31-41: 🗄️ Data Integrity & Integration | 🔵 TrivialClarify deployment requirements for time-zone updates.
The endpoint is valid, but
PATCH /users/meonly exists whenentity-serviceruns withDATA_SOURCE=servicenow; Postgres defaults to 404. Document the requiredDATA_SOURCE=servicenowrequirement for this endpoint and state that deployments using the default Postgres source cannot updatetimeZonehere.🤖 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/customer-portal/backend-v2/internal/entity/users.go` around lines 31 - 41, Update the documentation comment for Client.PatchMe to explicitly state that PATCH /users/me requires entity-service to run with DATA_SOURCE=servicenow, and that default Postgres deployments return 404 and cannot update timeZone through this endpoint.Source: Path instructions
apps/customer-portal/backend-v2/internal/handler/users.go (1)
131-138: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueReturn an explicit status from PATCH /users/me.
PatchUserMeResponseonly containsMessageandUser, andPatchUserMeUpdateddoes not includeTimeZone. Echoingpayload.TimeZoneis safe here, but thePatchMeclient and handler ignore the upstream message. Store or surface a service message/fallback so the response contract is explicit.🤖 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/customer-portal/backend-v2/internal/handler/users.go` around lines 131 - 138, Update the PATCH /users/me flow around PatchMe and PatchUserMeResponse to preserve the upstream service message and expose it through the response’s Message field, using an explicit fallback when no message is returned. Keep the existing TimeZone echo behavior, but do not rely on PatchUserMeUpdated to represent it; ensure the handler returns a response with a clear success status/message.Source: Path instructions
🤖 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/customer-portal/backend-v2/internal/dto/account.go`:
- Around line 21-45: Add UpdatedOn to dto.AccountSummary using the same JSON
representation as AccountDetails, and update MapSearchAccounts to populate it
from entity.AccountSummary for every data source. Keep the existing deliberate
exclusions unchanged and ensure the mapped value is preserved in the search
response.
- Around line 125-136: Update mapAccountRef so it returns nil when ref is absent
and only bareID is available; do not construct a Ref from bareID. Preserve the
existing ServiceNow ref mapping and nil behavior, ensuring Owner and
TechnicalOwner do not expose nameless internal IDs.
In `@apps/customer-portal/backend-v2/internal/handler/updates.go`:
- Around line 68-94: Validate the unmarshaled `updates.SearchPayload` in
`SearchUpdatesBetweenUpdateLevels` before invoking the Updates service: reject
empty `ProductName` or `ProductVersion`, and reject payloads where
`StartingUpdateLevel` exceeds `EndingUpdateLevel`, using `writeError` with HTTP
400. Return immediately for invalid input so
`h.updates.SearchUpdatesBetweenUpdateLevels` is not called.
In `@apps/customer-portal/backend-v2/internal/scim/client.go`:
- Around line 98-139: Update the SCIM request construction in Client.do to read
the correlation ID with middleware.CorrelationIDFromContext(ctx) and set it on
the outgoing request before c.http.Do(req). Preserve the existing request
headers and send the header only according to the middleware helper’s returned
value.
In `@apps/customer-portal/backend-v2/internal/scim/scim.go`:
- Around line 48-53: Update the SCIM filter construction in the
scimSearchRequest request body to encode email as a quoted SCIM string literal,
escaping backslashes and double quotes before interpolation. Preserve the
existing userName equality filter and other request fields.
In `@apps/customer-portal/backend-v2/openapi.yaml`:
- Around line 39-63: Update getUsersMe at
apps/customer-portal/backend-v2/openapi.yaml lines 39-63 and
getUpdatesProductUpdateLevels at lines 417-443 to add a "400" response with the
existing ErrorPayload schema reference, ensuring both operations document 200,
400, 401, 403, and 500 responses.
---
Nitpick comments:
In `@apps/customer-portal/backend-v2/internal/dto/account.go`:
- Around line 57-123: Extract the duplicated normalization logic from
MapSearchAccounts and MapAccountDetails into a shared helper for tier, region,
owner references, activation/deactivation dates, boolean flags, and created
date. Have both mappers reuse that helper while retaining their distinct
SupportTier, ID/Name, and UpdatedOn handling, preserving the existing output
behavior.
In `@apps/customer-portal/backend-v2/internal/entity/users.go`:
- Around line 31-41: Update the documentation comment for Client.PatchMe to
explicitly state that PATCH /users/me requires entity-service to run with
DATA_SOURCE=servicenow, and that default Postgres deployments return 404 and
cannot update timeZone through this endpoint.
In `@apps/customer-portal/backend-v2/internal/handler/users.go`:
- Around line 131-138: Update the PATCH /users/me flow around PatchMe and
PatchUserMeResponse to preserve the upstream service message and expose it
through the response’s Message field, using an explicit fallback when no message
is returned. Keep the existing TimeZone echo behavior, but do not rely on
PatchUserMeUpdated to represent it; ensure the handler returns a response with a
clear success status/message.
In `@apps/customer-portal/backend-v2/openapi.yaml`:
- Around line 588-598: Update the filters schema to stop exposing internal pod
and classification fields: remove pod and rename classification to tier, unless
entity-service requires those exact names; in that case, retain them and add
descriptions identifying their internal origin. Keep the schema aligned with
AccountSummary and AccountDetails, which use tier and do not expose Pod.
🪄 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 Plus
Run ID: ec5eedf0-2029-4f64-bd8a-e6f7d9948caf
📒 Files selected for processing (22)
apps/customer-portal/backend-v2/.choreo/component.yamlapps/customer-portal/backend-v2/.env.exampleapps/customer-portal/backend-v2/CLAUDE.mdapps/customer-portal/backend-v2/README.mdapps/customer-portal/backend-v2/cmd/server/main.goapps/customer-portal/backend-v2/internal/dto/account.goapps/customer-portal/backend-v2/internal/dto/user.goapps/customer-portal/backend-v2/internal/entity/accounts.goapps/customer-portal/backend-v2/internal/entity/client.goapps/customer-portal/backend-v2/internal/entity/types.goapps/customer-portal/backend-v2/internal/entity/users.goapps/customer-portal/backend-v2/internal/handler/accounts.goapps/customer-portal/backend-v2/internal/handler/updates.goapps/customer-portal/backend-v2/internal/handler/users.goapps/customer-portal/backend-v2/internal/scim/client.goapps/customer-portal/backend-v2/internal/scim/scim.goapps/customer-portal/backend-v2/internal/scim/types.goapps/customer-portal/backend-v2/internal/updates/client.goapps/customer-portal/backend-v2/internal/updates/mapper.goapps/customer-portal/backend-v2/internal/updates/types.goapps/customer-portal/backend-v2/internal/updates/updates.goapps/customer-portal/backend-v2/openapi.yaml
…/scim PR - Add missing UpdatedOn to dto.AccountSummary (was dropped from the search response but kept in the detail response with no explanation). - mapAccountRef no longer surfaces a nameless Ref for Postgres-only bare owner/technicalOwner IDs, matching the DTO's own documented intent to exclude them. - Validate SearchPayload (non-empty productName/productVersion, level ordering) before calling the updates service, instead of forwarding unchecked input. - Forward X-CSM-Correlation-ID on SCIM and updates-service requests too (previously only entity-service calls carried it), via the same WithCorrelationID/context-key pattern already used for entity. - Quote/escape the email value in the SCIM userName filter per RFC 7644's string-literal requirement. - openapi.yaml: add the missing 400 response to getUsersMe and getUpdatesProductUpdateLevels. - gofmt a few pre-existing struct-tag alignment issues flagged along the way (no behavior change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
04ddb0f
into
wso2-open-operations:dev-app-csm-portal
Summary
apps/customer-portal/backend-v2:PATCH /users/me,POST /accounts/search,GET /accounts/{id},GET /updates/product-update-levels,POST /updates/levels/search(11 total now).apps/csm-portal/backend's conventions:internal/updates— WSO2 Updates service client. Its own types are already portal-shaped camelCase (mapper.gotranslates the upstream snake_case), so handlers write its results directly — the one deliberate exception to this backend's "always map through dto" rule.internal/scim— SCIM operations client, used for phone number read/update onGET/PATCH /users/me(name/timezone/roles still come from entity-service).internal/dto/account.go), instead of exposing an OpenAPIoneOffor the same ambiguity likeapps/csm-portal/backenddoes..choreo/component.yamlandopenapi.yaml(covering all 11 routes implemented so far), matchingapps/csm-portal/backend's Choreo/OpenAPI conventions — these were missing from the previous PR.README.md/CLAUDE.mdto document the new modules, the account-normalization pattern, and the OpenAPI-spec maintenance step in the "Adding a new endpoint" recipe.Test plan
go build ./...andgo vet ./...passgosec -fmt=text ./...reports 0 issuesopenapi.yaml/.choreo/component.yamlvalidated as well-formed YAMLPATCH /users/mewith empty body → 400;GET /accounts/{id}with invalid UUID → 400; all new endpoints map an unreachable upstream to a clean 500 (no leakage); correlation IDs carry thecp-prefixDATA_SOURCE=servicenow), Updates service, and SCIM instancesLinked issues
Closes wso2-enterprise/wso2-digital-team-project-management#842
Closes wso2-enterprise/wso2-digital-team-project-management#843
Closes wso2-enterprise/wso2-digital-team-project-management#844
Closes wso2-enterprise/wso2-digital-team-project-management#845
Closes wso2-enterprise/wso2-digital-team-project-management#846
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation