[Customer Portal][BE] Enhance updates/levels/search response to return grouped update levels by type - #231
Conversation
📝 WalkthroughWalkthroughRestructures update types and processing to produce a grouped map of updates by update level (map), adds update-type/updateLevel fields and optional description details, introduces update-related constants, and refactors the search processing to classify and group updates before returning them. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Service
participant Utils as "Updates Utils"
participant API as "External API"
Client->>Service: GET /updates/search (payload)
Service->>Utils: processSearchUpdatesBetweenUpdateLevels(email, payload)
Utils->>API: Request updates (channel = "full")
API-->>Utils: Return raw update items
Utils->>Utils: Enrich items (description, files, securityAdvisories, dependantReleases, timestamp)
Utils->>Utils: For each item: isSecurityUpdate() → set updateType
Utils->>Utils: groupByUpdateLevel() → map<UpdateLevelGroup>
Utils-->>Service: Return grouped map<UpdateLevelGroup>
Service-->>Client: HTTP 200 with grouped updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 1
🧹 Nitpick comments (2)
apps/customer-portal/backend/modules/types/types.bal (1)
914-920: NewUpdateLevelGrouptype looks good.Clean grouping structure. One observation: the field name
updateDescriptionLevelsis somewhat verbose — consider whetherupdateDescriptionswould be clearer, since each entry is anUpdateDescriptionrather than a "level." This is a minor naming nit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/types/types.bal` around lines 914 - 920, The field name updateDescriptionLevels on the UpdateLevelGroup record is overly verbose—rename it to updateDescriptions to better reflect that it holds an array of UpdateDescription items; update the type definition for UpdateLevelGroup and then update every reference to UpdateLevelGroup.updateDescriptionLevels (constructors, destructuring, JSON (de)serialization, test fixtures, and any usages in functions or modules) to use updateDescriptions so compilation and runtime behavior remain consistent.apps/customer-portal/backend/modules/updates/utils.bal (1)
100-147: Use non-optional field access for required field on required value for clarity.Line 128:
description?.update-typeuses optional field access (?.) on a required field. Sinceupdate-typeis a required field in theUpdateDescriptiontype (line 202 inupdates/types.bal), anddescriptionis guaranteed to be non-null in this context, use direct access for clarity.Suggested change
- updateType: description?.update\-type, + updateType: description.update\-type,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/updates/utils.bal` around lines 100 - 147, The code in processSearchUpdatesBetweenUpdateLevels maps UpdateDescription entries but uses optional access description?.update-type for a required field; since UpdateDescription items are non-null here, change all occurrences of description?.update-type (and any other required fields accessed with ?. like description?.update-type) to direct field access (description.update-type) in the mapping to reflect the type guarantees and improve clarity; locate this in the mapping of UpdateDescription -> types:UpdateDescription within the processSearchUpdatesBetweenUpdateLevels function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/customer-portal/backend/modules/updates/utils.bal`:
- Around line 69-93: The map access at groupByUpdateLevel uses
groupedUpdateLevels[levelKey].updateType without nil-guarding; retrieve the
entry into a nilable temp (e.g., types:UpdateLevelGroup? levelGroup =
groupedUpdateLevels[levelKey]), check it with an is-type guard (if levelGroup is
types:UpdateLevelGroup) then set levelGroup.updateType = UPDATE_TYPE_SECURITY
and write it back (groupedUpdateLevels[levelKey] = levelGroup); do this instead
of directly mutating groupedUpdateLevels[levelKey] and keep the existing logic
that uses isSecurityUpdate(description), levelKey, UPDATE_TYPE_SECURITY and
UPDATE_TYPE_REGULAR.
---
Nitpick comments:
In `@apps/customer-portal/backend/modules/types/types.bal`:
- Around line 914-920: The field name updateDescriptionLevels on the
UpdateLevelGroup record is overly verbose—rename it to updateDescriptions to
better reflect that it holds an array of UpdateDescription items; update the
type definition for UpdateLevelGroup and then update every reference to
UpdateLevelGroup.updateDescriptionLevels (constructors, destructuring, JSON
(de)serialization, test fixtures, and any usages in functions or modules) to use
updateDescriptions so compilation and runtime behavior remain consistent.
In `@apps/customer-portal/backend/modules/updates/utils.bal`:
- Around line 100-147: The code in processSearchUpdatesBetweenUpdateLevels maps
UpdateDescription entries but uses optional access description?.update-type for
a required field; since UpdateDescription items are non-null here, change all
occurrences of description?.update-type (and any other required fields accessed
with ?. like description?.update-type) to direct field access
(description.update-type) in the mapping to reflect the type guarantees and
improve clarity; locate this in the mapping of UpdateDescription ->
types:UpdateDescription within the processSearchUpdatesBetweenUpdateLevels
function.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/customer-portal/backend/modules/types/types.balapps/customer-portal/backend/modules/updates/constants.balapps/customer-portal/backend/modules/updates/types.balapps/customer-portal/backend/modules/updates/utils.balapps/customer-portal/backend/service.bal
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/customer-portal/backend/modules/updates/utils.bal (1)
87-92: Nil-safety fix is correctly implemented — no write-back needed.In Ballerina, mutation is tied to storage identity, and when a value stored in some storage location is mutated, the change is visible through all variables referring to the value. Structured values (records, arrays) are stored by reference in maps, so
group.updateType = UPDATE_TYPE_SECURITYcorrectly mutates the map entry in-place; the write-back proposed in the previous review comment is unnecessary. The implementation is correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/updates/utils.bal` around lines 87 - 92, The existing nil-safety handling is correct: when isSecurityUpdate(description) is true, the local variable group (types:UpdateLevelGroup?) obtained from groupedUpdateLevels[levelKey] is a reference to the stored record, so assigning group.updateType = UPDATE_TYPE_SECURITY mutates the map entry in-place; no additional write-back or reassignment is needed—leave the code in the isSecurityUpdate(...) block as-is and remove any previous suggestion to write group back into groupedUpdateLevels.
🧹 Nitpick comments (1)
apps/customer-portal/backend/modules/updates/utils.bal (1)
76-92: RedundantupdateTypeassignment when the group is newly created for a security update.When the group is first initialized at lines 76–81 for a SECURITY description,
updateTypeis already set toUPDATE_TYPE_SECURITY. Lines 87–92 then unconditionally set it toUPDATE_TYPE_SECURITYa second time. The check on lines 87–92 is only semantically necessary for subsequent descriptions of the same key that are SECURITY after the group was initialized as REGULAR. While not a bug, collapsing the two paths would eliminate the redundancy:♻️ Proposed simplification
if !groupedUpdateLevels.hasKey(levelKey) { groupedUpdateLevels[levelKey] = { - updateType: isSecurityUpdate(description) ? UPDATE_TYPE_SECURITY : UPDATE_TYPE_REGULAR, + updateType: UPDATE_TYPE_REGULAR, updateDescriptionLevels: [] }; } types:UpdateDescription[]? existingLevels = groupedUpdateLevels[levelKey]?.updateDescriptionLevels; if existingLevels is types:UpdateDescription[] { existingLevels.push(description); } if isSecurityUpdate(description) { types:UpdateLevelGroup? group = groupedUpdateLevels[levelKey]; if group is types:UpdateLevelGroup { group.updateType = UPDATE_TYPE_SECURITY; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/updates/utils.bal` around lines 76 - 92, The group initialization already sets updateType based on isSecurityUpdate(description), so remove the redundant unconditional assignment that re-sets UPDATE_TYPE_SECURITY for newly created groups; instead only upgrade an existing group to UPDATE_TYPE_SECURITY when a subsequent description is security by guarding the assignment with a check that the group already existed (e.g., only run the block setting group.updateType = UPDATE_TYPE_SECURITY when groupedUpdateLevels.hasKey(levelKey) was true or when you detect the group was not just created); reference groupedUpdateLevels, levelKey, isSecurityUpdate, UPDATE_TYPE_SECURITY, UPDATE_TYPE_REGULAR, types:UpdateLevelGroup and updateDescriptionLevels when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/customer-portal/backend/modules/updates/utils.bal`:
- Around line 87-92: The existing nil-safety handling is correct: when
isSecurityUpdate(description) is true, the local variable group
(types:UpdateLevelGroup?) obtained from groupedUpdateLevels[levelKey] is a
reference to the stored record, so assigning group.updateType =
UPDATE_TYPE_SECURITY mutates the map entry in-place; no additional write-back or
reassignment is needed—leave the code in the isSecurityUpdate(...) block as-is
and remove any previous suggestion to write group back into groupedUpdateLevels.
---
Nitpick comments:
In `@apps/customer-portal/backend/modules/updates/utils.bal`:
- Around line 76-92: The group initialization already sets updateType based on
isSecurityUpdate(description), so remove the redundant unconditional assignment
that re-sets UPDATE_TYPE_SECURITY for newly created groups; instead only upgrade
an existing group to UPDATE_TYPE_SECURITY when a subsequent description is
security by guarding the assignment with a check that the group already existed
(e.g., only run the block setting group.updateType = UPDATE_TYPE_SECURITY when
groupedUpdateLevels.hasKey(levelKey) was true or when you detect the group was
not just created); reference groupedUpdateLevels, levelKey, isSecurityUpdate,
UPDATE_TYPE_SECURITY, UPDATE_TYPE_REGULAR, types:UpdateLevelGroup and
updateDescriptionLevels when making this change.
3340268
into
wso2-open-operations:customer-portal-milestone-1
Description
This PR updates the
updates/levels/searchresponse structure to return a map grouped by update type, including their respective update levels.Changes
updates/levels/searchNew Response Structure
The endpoint now returns a map structured as:
{
"": {
updatesType: "",
updateLevels: [ ... ]
}
}
This allows consumers to easily access update levels categorized by their update type.
Reason
Previously, the endpoint returned a flat list of update levels.
The UI and consumer services require update levels grouped by update type for:
Grouping at the backend reduces frontend transformation logic and improves API clarity.
Impact
⚠ Response structure changed (potential breaking change if consumers rely on the previous flat structure).
Consumers must adjust to the grouped map format.
Testing
Summary by CodeRabbit
New Features
Changes