FCN-247: regions api call#6490
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: davidaznaur The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/cc @KevinFCormier |
|
Warning Review limit reached
Next review available in: 26 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: Enterprise Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe PR adds a backend ChangesROSA wizard regions
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Wizard
participant Backend
participant OCM
Wizard->>Backend: POST /regions with service account credentials
Backend->>OCM: Authenticate and obtain service token
Backend->>OCM: Fetch cloud providers with regions
OCM-->>Backend: Return provider and region data
Backend-->>Wizard: Return JSON response
Wizard->>Wizard: Select AWS hypershift regions
Possibly related PRs
🚥 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/src/lib/rosa-hcp-api.ts (1)
73-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse camelCase for function parameters.
As per coding guidelines, functions and variables must use descriptive camelCase names. Please rename
client_idandclient_secrettoclientIdandclientSecret.♻️ Proposed refactor
export const getWizardRegions = ( - client_id: string, - client_secret: string, + clientId: string, + clientSecret: string, signal?: AbortSignal, additionalData?: Record<string, unknown> ): Promise<CloudProviderResponse> => getWizardData<CloudProviderResponse, Record<string, unknown>>( - client_id, - client_secret, + clientId, + clientSecret, '/regions', signal, additionalData )🤖 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 `@frontend/src/lib/rosa-hcp-api.ts` around lines 73 - 85, Rename the getWizardRegions parameters client_id and client_secret to clientId and clientSecret, and update their corresponding references in the getWizardData call while preserving the existing API behavior.Source: Coding guidelines
frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/constants/types.ts (1)
7-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
interfacefor object shapes.As per coding guidelines, use
interfacefor object shapes that may be extended, andtypefor unions and computed types.♻️ Proposed refactor
-export type DropdownType = { +export interface DropdownType { value: string label: string }🤖 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 `@frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/constants/types.ts` around lines 7 - 10, Replace the DropdownType object-shape alias with an interface named DropdownType, preserving its existing value and label string properties.Source: Coding guidelines
🤖 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 `@backend/src/routes/rosaWizardApi.ts`:
- Around line 100-128: Update getWizardCloudProviders, getAwsAccountIds, and
getAwsBillingAccountIds to explicitly return Promise<void>, join request-body
chunks with an empty separator, and wrap each async req.on('end') callback body
in try/catch so JSON parsing or token-fetch failures log the error and send the
existing internal-server-error response instead of causing an unhandled
rejection or hanging request.
In
`@frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRegions.ts`:
- Around line 12-17: Filter regions in the useFetchRegions query so entries
without an ID are excluded before mapping, while retaining the existing
Hypershift support condition. Update
frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRegions.ts
at lines 12-17 accordingly, and revise
frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRegions.test.tsx
at lines 202-225 to assert that an ID-less region produces an empty result
rather than a fallback option.
---
Nitpick comments:
In `@frontend/src/lib/rosa-hcp-api.ts`:
- Around line 73-85: Rename the getWizardRegions parameters client_id and
client_secret to clientId and clientSecret, and update their corresponding
references in the getWizardData call while preserving the existing API behavior.
In
`@frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/constants/types.ts`:
- Around line 7-10: Replace the DropdownType object-shape alias with an
interface named DropdownType, preserving its existing value and label string
properties.
🪄 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: Enterprise
Run ID: 179f794b-de4b-4592-87a3-578d28e51fdf
📒 Files selected for processing (10)
backend/src/app.tsbackend/src/routes/rosaWizardApi.tsfrontend/src/lib/rosa-hcp-api.tsfrontend/src/resources/rosa-hcp-wizard.tsfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/constants/types.tsfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/queryKeyFactory.test.tsfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/queryKeyFactory.tsfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRegions.test.tsxfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchRegions.tsfrontend/webpack.config.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/routes/rosaWizardApi.ts (1)
1-1: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winUse an empty string separator to join request body chunks.
By default,
chucks.join()uses a comma as the separator, which will corrupt multi-chunk JSON payloads beforeJSON.parseis executed. As highlighted in a previous review, this must be explicitly joined with an empty string.
backend/src/routes/rosaWizardApi.ts#L41-41: Change todata = chucks.join('')ingetAwsAccountIds.backend/src/routes/rosaWizardApi.ts#L82-82: Change todata = chucks.join('')ingetAwsBillingAccountIds.backend/src/routes/rosaWizardApi.ts#L122-122: Change todata = chucks.join('')ingetWizardCloudProviders.🤖 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 `@backend/src/routes/rosaWizardApi.ts` at line 1, Update the request-body assembly in getAwsAccountIds, getAwsBillingAccountIds, and getWizardCloudProviders to join chunks with an empty string explicitly, preserving the JSON payload before JSON.parse.
🤖 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 `@backend/src/routes/rosaWizardApi.ts`:
- Line 122: Update the request-body assembly in the visible data assignment to
join the chucks array with an empty string separator, preserving multi-chunk
JSON before parsing.
- Line 82: Update the request-body assembly in the surrounding handler to join
the chucks array with an empty separator, preserving the original JSON across
multiple chunks before parsing.
- Line 41: Update the request-body chunk assembly at the data assignment to join
the chucks array with an empty string separator, preserving the original JSON
payload across multiple chunks before parsing.
---
Outside diff comments:
In `@backend/src/routes/rosaWizardApi.ts`:
- Line 1: Update the request-body assembly in getAwsAccountIds,
getAwsBillingAccountIds, and getWizardCloudProviders to join chunks with an
empty string explicitly, preserving the JSON payload before JSON.parse.
🪄 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: Enterprise
Run ID: c40522a9-05b5-4de2-ab1d-f0efa1ccbca9
📒 Files selected for processing (3)
backend/src/routes/rosaWizardApi.tsbackend/test/routes/rosaWizardApi.test.tsfrontend/src/lib/rosa-hcp-api.test.ts
|
/test unit-tests-sonarcloud |
1 similar comment
|
/test unit-tests-sonarcloud |
|
/test unit-tests-sonarcloud |
|
/test unit-tests-sonarcloud |
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
6e3aa4f to
ffe0c9c
Compare
|
|
/retest |
3 participants
📝 Summary
Ticket Summary (Title):
Regions api call for the ROSA HCP wizard
Ticket Link:
Type of Change:
✅ Checklist
General
ACM-12340 Fix bug with...)If Feature
If Bugfix
🗒️ Notes for Reviewers
Summary by CodeRabbit
New Features
POST /regions) and corresponding client helper to fetch available cloud provider regions.Bug Fixes
Tests
POST /regions, plus frontend tests for query keys, region filtering/formatting, hook behavior, and API request wiring.