[Customer Portal][BE] Update updates/search response and improve vulnerability not-found handling - #178
Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughRefactored UpdateResponse and related update-level types: removed jwt/platform fields, renamed productBaseVersion→productVersion and appliedUpdateNumbers→appliedUpdatesNumbers, added open Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/customer-portal/backend/modules/updates/types.bal (1)
112-135:⚠️ Potential issue | 🔴 CriticalMissing
json...;rest descriptor in internalUpdateResponse— will reject upstream fields at runtime.The upstream service likely still sends fields like
jwt,platform-name,platform-version, andproduct-base-versionthat were removed from this closed record. Without ajson...;rest descriptor (which you did add toRecommendedUpdateLevel,ProductUpdateLevel,UpdateLevel, and the publicUpdateResponseintypes/types.bal), Ballerina's closed record binding will fail at runtime when the upstream response contains any field not declared here.🐛 Proposed fix: add `json...;` to accept and discard extra upstream fields
# Applied update numbers int[] applied\-updates\-numbers; + json...; |};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/updates/types.bal` around lines 112 - 135, The internal record type UpdateResponse is a closed record and will reject unexpected upstream fields; add a json rest descriptor to the record body (i.e., include json...; inside the UpdateResponse record) so that extra fields like jwt, platform-name, platform-version, product-base-version are accepted and discarded at runtime—mirror the same change you already applied to RecommendedUpdateLevel, ProductUpdateLevel, UpdateLevel, and the public UpdateResponse in types/types.bal.
🧹 Nitpick comments (1)
apps/customer-portal/backend/service.bal (1)
1323-1330: Include the vulnerabilityidin the log message for easier debugging.The current log message references the user but not which vulnerability was requested. Adding the
idwill make it much easier to trace specific 404 issues.🔧 Proposed fix
if getStatusCode(response) == http:STATUS_NOT_FOUND { - log:printWarn(string `Requested product vulnerability is not found for the user: ${userInfo.userId}`); + log:printWarn(string `Product vulnerability with ID: ${id} is not found for the user: ${userInfo.userId}`); return <http:NotFound>{ body: { message: "Requested product vulnerability is not found!"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/service.bal` around lines 1323 - 1330, The 404 log prints the user but not which vulnerability was requested; update the log inside the getStatusCode(response) == http:STATUS_NOT_FOUND branch to include the vulnerability identifier (the same variable used to fetch the resource—e.g., id or productVulnerabilityId) alongside userInfo.userId by expanding the template string (e.g., `Requested product vulnerability id ${id} for user ${userInfo.userId} is not found`), ensuring you reference the actual identifier variable in scope where getStatusCode(response) is checked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/customer-portal/backend/modules/updates/types.bal`:
- Around line 112-135: The internal record type UpdateResponse is a closed
record and will reject unexpected upstream fields; add a json rest descriptor to
the record body (i.e., include json...; inside the UpdateResponse record) so
that extra fields like jwt, platform-name, platform-version,
product-base-version are accepted and discarded at runtime—mirror the same
change you already applied to RecommendedUpdateLevel, ProductUpdateLevel,
UpdateLevel, and the public UpdateResponse in types/types.bal.
---
Nitpick comments:
In `@apps/customer-portal/backend/service.bal`:
- Around line 1323-1330: The 404 log prints the user but not which vulnerability
was requested; update the log inside the getStatusCode(response) ==
http:STATUS_NOT_FOUND branch to include the vulnerability identifier (the same
variable used to fetch the resource—e.g., id or productVulnerabilityId)
alongside userInfo.userId by expanding the template string (e.g., `Requested
product vulnerability id ${id} for user ${userInfo.userId} is not found`),
ensuring you reference the actual identifier variable in scope where
getStatusCode(response) is checked.
5c37e3b
into
wso2-open-operations:customer-portal-milestone-1
Description
This PR cleans up unused fields in the
updates/searchresponse and improves the not-found handling for the vulnerability get endpoint.Changes
updates/search Response Cleanup
Removed the following unused fields from the updates service response:
Updated related DTOs and mappings to reflect the cleaned response structure.
Vulnerability Not-Found Handling
Reason
Response Cleanup
The fields
jwt,platform-name, andplatform-versionwere present in the response but not used by the system. Keeping them:Removing them simplifies the response contract and keeps the API clean.
Not-Found Improvement
The vulnerability get endpoint previously did not handle missing resources clearly. This update ensures:
Testing
updates/searchresponse structureImpact
Checklist
Summary by CodeRabbit
Bug Fixes
Refactor