fix(customer-portal-backend-v2): align response/request types with the frontend and old Ballerina backend - #1393
Conversation
…e frontend and old Ballerina backend Audits and fixes every remaining endpoint's request/response shape against the live frontend TypeScript and the old Ballerina backend, the same pass applied to case-search/call-requests in PR wso2-open-operations#1388: rename total->totalRecords across deployments, deployed products, instances, products, product versions, product vulnerabilities, catalogs, change requests, time cards, conversations, comments, and case activities; translate ServiceNow/Choreo numeric ids <-> entity-service enums for change-request state/impact, conversation state, deployment type, and product-vulnerability severity; scope deployments/deployed-products/catalogs/time-cards/change-requests search and mutation routes under their owning path parameter instead of trusting a client-supplied body field; rewrite case details, change request, time card, and deployed product response shapes to match the frontend's IdLabelRef-based contract; add missing fields (project.startDate, project.account.hasAgent/ hasKbReferences/activationDate, case.watchList/changeRequests, users/me.lastPasswordUpdateTime, registry token name); fix registry token id/expiresAt to decode as numbers; add GET /products and POST /cases/{id}/attachments. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 36 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (7)
📝 WalkthroughWalkthroughThe customer portal backend now uses scoped resource routes, frontend-aligned DTOs, numeric enum mappings, ChangesCustomer portal API alignment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
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 review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
apps/customer-portal/backend-v2/openapi.yaml (1)
6887-6904: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the base64 field name across the two attachment-create contracts.
CreateCaseAttachmentRequestnames the base64 payloadcontent.CreateAttachmentRequest(line 6920) names the same payloadfile. Two names for one concept increase client error risk. If the frontend requirescontenton the case route, keep it and add a note here that the difference is deliberate.🤖 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 6887 - 6904, The attachment-create request schemas use inconsistent names for the base64 payload. Align CreateCaseAttachmentRequest and CreateAttachmentRequest on one field name, preferably content if required by the frontend; if the contracts must differ, retain content for the case route and document the deliberate difference in its schema description.apps/customer-portal/backend-v2/internal/dto/deployed_product.go (1)
112-134: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider omitting empty deployment and product references.
entityRefToIDLabel(&d.Deployment)andentityRefToIDLabel(&d.Product)always receive a non-nil pointer. If the upstream reference is zero-valued, the mapper emits{"label":""}instead of omitting the field, becauseIDLabelRef.Labelhas noomitempty. The frontend then renders an empty label rather than treating the reference as absent. Add a zero-value check if the frontend distinguishes the two cases.🤖 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/deployed_product.go` around lines 112 - 134, Update MapSearchDeployedProducts to detect zero-valued d.Deployment and d.Product references before calling entityRefToIDLabel, passing nil or otherwise leaving the corresponding fields absent when references are empty. Preserve the existing mapping for non-zero references so the frontend distinguishes absent references from empty labels.
🤖 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/deployed_product.go`:
- Around line 156-178: Update BuildEntityCreateDeployedProductRequest so
ProjectID is derived from the path-mounted deployment rather than copied from
DeployedProductCreateRequest.ProjectID. Resolve the deployment’s owning project
before constructing entity.CreateDeployedProductRequest, and use that value
while preserving DeploymentID from the path; do not trust or forward the
body-supplied project ID.
In `@apps/customer-portal/backend-v2/internal/dto/deployment.go`:
- Around line 105-128: Update DeploymentUpdateRequest.Description to use
json.RawMessage so absent and explicit null remain distinguishable, and modify
BuildEntityUpdateDeploymentRequest to copy non-empty raw bytes into
entity.UpdateDeploymentRequest.Description while preserving string values. Add
tests covering an absent description, explicit null, and a string description.
In `@apps/customer-portal/backend-v2/internal/dto/product_vulnerability.go`:
- Around line 91-128: Remove StatusID from SearchProductVulnerabilitiesFilters
and remove the corresponding statusId definition from openapi.yaml, since
BuildEntitySearchProductVulnerabilitiesRequest cannot forward it. Ensure the
portal contract no longer advertises status filtering until entity-service
supports it.
In `@apps/customer-portal/backend-v2/internal/handler/cases.go`:
- Around line 244-253: Update UpdateCaseRequest so WatchList is a pointer to a
string slice, allowing absent and explicitly empty watch lists to be
distinguished. In the handler’s primary-field validation, check WatchList for
non-nil rather than using len, and update BuildEntityUpdateCaseRequest to
dereference it when present; preserve any entity-service restriction on empty
lists and document it in openapi.yaml if applicable.
In `@apps/customer-portal/backend-v2/internal/handler/deployed_products.go`:
- Around line 108-114: The nested deployment operations must enforce project
ownership server-side. In deployed_products.go, update CreateDeployedProduct and
BuildEntityCreateDeployedProductRequest usage so the project comes from the
deployment identified by deploymentID, or reject requests when ownership cannot
be derived instead of trusting req.ProjectID. In deployments.go lines 128-156,
update PatchDeployment to accept and validate a projectId path parameter before
UpdateDeployment, or expose the endpoint only as explicitly unscoped if the
parent project scope is not authoritative.
In `@apps/customer-portal/backend-v2/internal/handler/products.go`:
- Around line 52-86: Update ProductHandler.GetProducts to inspect the class
query parameter instead of silently discarding it; return a 400 response when
class is supplied, until entity-service supports class filtering. Preserve the
existing offset and limit validation and search flow for requests without class.
---
Nitpick comments:
In `@apps/customer-portal/backend-v2/internal/dto/deployed_product.go`:
- Around line 112-134: Update MapSearchDeployedProducts to detect zero-valued
d.Deployment and d.Product references before calling entityRefToIDLabel, passing
nil or otherwise leaving the corresponding fields absent when references are
empty. Preserve the existing mapping for non-zero references so the frontend
distinguishes absent references from empty labels.
In `@apps/customer-portal/backend-v2/openapi.yaml`:
- Around line 6887-6904: The attachment-create request schemas use inconsistent
names for the base64 payload. Align CreateCaseAttachmentRequest and
CreateAttachmentRequest on one field name, preferably content if required by the
frontend; if the contracts must differ, retain content for the case route and
document the deliberate difference in its schema description.
🪄 Autofix
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: d32ba0c6-880b-49f6-8930-4408fef3af4e
📒 Files selected for processing (39)
apps/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/ai_chat.goapps/customer-portal/backend-v2/internal/dto/attachment.goapps/customer-portal/backend-v2/internal/dto/case.goapps/customer-portal/backend-v2/internal/dto/case_time_cards.goapps/customer-portal/backend-v2/internal/dto/catalog.goapps/customer-portal/backend-v2/internal/dto/change_request.goapps/customer-portal/backend-v2/internal/dto/change_request_enum_mapping.goapps/customer-portal/backend-v2/internal/dto/comment.goapps/customer-portal/backend-v2/internal/dto/conversation_enum_mapping.goapps/customer-portal/backend-v2/internal/dto/deployed_product.goapps/customer-portal/backend-v2/internal/dto/deployment.goapps/customer-portal/backend-v2/internal/dto/deployment_enum_mapping.goapps/customer-portal/backend-v2/internal/dto/instance.goapps/customer-portal/backend-v2/internal/dto/product.goapps/customer-portal/backend-v2/internal/dto/product_vulnerability.goapps/customer-portal/backend-v2/internal/dto/product_vulnerability_enum_mapping.goapps/customer-portal/backend-v2/internal/dto/project.goapps/customer-portal/backend-v2/internal/dto/registry.goapps/customer-portal/backend-v2/internal/dto/time_card.goapps/customer-portal/backend-v2/internal/dto/type_alignment_test.goapps/customer-portal/backend-v2/internal/dto/user.goapps/customer-portal/backend-v2/internal/entity/types.goapps/customer-portal/backend-v2/internal/handler/ai_chat.goapps/customer-portal/backend-v2/internal/handler/cases.goapps/customer-portal/backend-v2/internal/handler/catalogs.goapps/customer-portal/backend-v2/internal/handler/change_requests.goapps/customer-portal/backend-v2/internal/handler/deployed_products.goapps/customer-portal/backend-v2/internal/handler/deployments.goapps/customer-portal/backend-v2/internal/handler/instances.goapps/customer-portal/backend-v2/internal/handler/product_vulnerabilities.goapps/customer-portal/backend-v2/internal/handler/products.goapps/customer-portal/backend-v2/internal/handler/projects.goapps/customer-portal/backend-v2/internal/handler/time_cards.goapps/customer-portal/backend-v2/internal/handler/users.goapps/customer-portal/backend-v2/internal/registry/types.goapps/customer-portal/backend-v2/openapi.yaml
…n PR wso2-open-operations#1393 - Preserve explicit description:null on PATCH /projects/{id}/deployments/{id} by switching DeploymentUpdateRequest.Description from *string to json.RawMessage, matching the three-state absent/null/value convention entity-service's own request type already requires. - Apply GET /products' class filter server-side (post-fetch, since entity-service has no class request parameter) instead of silently ignoring it, with documented pagination/data-source caveats. - Remove statusId from the product-vulnerabilities search filter contract instead of accepting and silently dropping it — entity-service's ProductVulnerabilityView has no status data anywhere to filter by. - Document (rather than "fix") the watchList empty-array limitation on PATCH /cases/{id}: entity-service's own ServiceNow implementation only forwards watchList upstream when non-empty, so no layer of this stack can currently honor an explicit "clear all watchers" request. - Two other findings (deployed-product/deployment project-scoping) were reviewed and left as-is: every write in this backend already authorizes through the caller's own forwarded ID token at the ServiceNow/Choreo integration layer, the same pattern used throughout this codebase — see PR review replies for the entity-service source confirming this. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
19da50e
into
wso2-open-operations:dev-app-csm-portal
Summary
Continues the type-alignment audit started in #1388 (case search / call requests) to cover every remaining
customer-portal-backend-v2endpoint, cross-checked against the live frontend TypeScript and the old Ballerina backend (apps/customer-portal/backend) as the reference contract.total→totalRecordsrenamed across deployments, deployed products, instances (search + 2 stats variants), products, product versions, product vulnerabilities, catalogs, change requests, time cards, conversations, comments, and case activities — matching the frontend's sharedPaginationResponsetype.deployments,deployed-products(now under/deployments/{deploymentId}/products),catalogs(now under/deployments/products/{deployedProductId}/catalogs),time-cards, andchange-requestssearch/mutation routes now scope to their owning path parameter instead of trusting a client-supplied body field.IdLabelRef-based contract ({id, label}instead of plain strings or{id, name}refs).project.startDate,project.account.{hasAgent,hasKbReferences,activationDate},case.{watchList,changeRequests},users/me.lastPasswordUpdateTime, registry tokenname.id/expiresAtnow decode as numbers (previously strings, which didn't match either the real upstream response or the frontend's own type).GET /products(frontend's live product-browsing flow, translated to entity-service'sPOST /products/search) andPOST /cases/{id}/attachments.Everywhere a field genuinely doesn't exist on entity-service's own domain type (verified against
cs-tools/entity-service/internal/domain/entity.go, not assumed), it's left out rather than fabricated — those gaps are documented with doc comments at each call site.Test plan
go build ./...go vet ./...gofmt -l— cleango test ./...— all existing tests pass, plus 16 new tests added covering the translation logic introduced in this PR (internal/dto/type_alignment_test.go)gosec -fmt=text ./...— 0 issuesopenapi.yaml,README.md,CLAUDE.mdupdated to match every route/schema change🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
GET /products.Improvements
totalRecords.Documentation