FCN-251: oidc configs api call#6492
Conversation
Signed-off-by: David Aznaurov <daznauro@redhat.com>
|
[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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds authenticated backend retrieval for ROSA OIDC configurations filtered by AWS account, frontend response types and API access, and a React Query hook that maps results into selectable options with query-key and proxy support. ChangesROSA OIDC configuration flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Wizard
participant useFetchOIDCConfigs
participant getWizardOIDCConfigs
participant Backend
participant OpenShift
Wizard->>useFetchOIDCConfigs: fetch AWS account configurations
useFetchOIDCConfigs->>getWizardOIDCConfigs: request OIDC configs
getWizardOIDCConfigs->>Backend: POST /oidc-configs
Backend->>OpenShift: query filtered oidc_configs
OpenShift-->>Backend: OIDC configuration response
Backend-->>getWizardOIDCConfigs: JSON response
getWizardOIDCConfigs-->>useFetchOIDCConfigs: response items
useFetchOIDCConfigs-->>Wizard: selectable options
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.test.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. 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 |
|
/cc @kelvah |
|
/cc @KevinFCormier |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.ts (2)
28-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unnecessary
asyncmodifier.The
fetchfunction only performs a synchronous state update. Theasyncmodifier andPromise<void>return type are unnecessary and can be removed.♻️ Proposed refactor
- const fetch = useCallback(async (accountId: string): Promise<void> => { + const fetch = useCallback((accountId: string) => { setAwsAccountId(accountId) }, [])🤖 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/queries/useFetchOIDCConfigs.ts` around lines 28 - 30, Update the fetch callback in useFetchOIDCConfigs to remove the unnecessary async modifier and Promise<void> return type, leaving it as a synchronous callback that only calls setAwsAccountId.
32-37: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winExtract the fallback array to a stable constant.
Returning an inline empty array
[]as a fallback creates a new reference on every render whendatais undefined. This can trigger infinite loops or unnecessary re-renders in child components that depend ondata. Extract it to a stable constant outside the hook.♻️ Proposed refactor
Apply the following diff to extract the constant:
+type OIDCConfigOption = { value: string; label: string; issuer_url: string } +const EMPTY_OIDC_CONFIGS: OIDCConfigOption[] = [] + export const useFetchOIDCConfigs = (selectedSecret: SelectedSecret) => {Then, update the return statement:
return { - data: data ?? [], + data: data ?? EMPTY_OIDC_CONFIGS, isFetching: isLoading, error: isError ? (error instanceof Error ? error.message : 'Unknown error') : null, fetch, }🤖 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/queries/useFetchOIDCConfigs.ts` around lines 32 - 37, Define a stable empty-array constant outside the hook in useFetchOIDCConfigs, then replace the inline data ?? [] fallback with that constant. Keep the existing data behavior unchanged when fetched data is available.frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/queryKeyFactory.ts (1)
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse camelCase for function parameters.
As per coding guidelines, functions and variables must use descriptive camelCase names. Rename
aws_account_idtoawsAccountId.♻️ Proposed refactor
- oidcConfigs: (id: string, aws_account_id: string) => [...rosaWizardKeys.all, id, aws_account_id, 'oidc-configs'], + oidcConfigs: (id: string, awsAccountId: string) => [...rosaWizardKeys.all, id, awsAccountId, 'oidc-configs'],🤖 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/queries/queryKeyFactory.ts` at line 8, Rename the oidcConfigs parameter aws_account_id to awsAccountId in the query key factory, and update its usage in the returned key tuple while preserving the existing key structure.Source: Coding guidelines
frontend/src/lib/rosa-hcp-api.ts (1)
3-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
import typefor type-only imports.
All three files import TypeScript types or interfaces as values; the shared root cause is the missingtypekeyword. As per coding guidelines, useimport typefor type-only imports to avoid generating runtime imports.
frontend/src/lib/rosa-hcp-api.ts#L3-L9: addtypeto the import statement (import type { AwsAccountIdsResponse... }).frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.ts#L4-L4: addtypeto the import statement (import type { SelectedSecret }).frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.test.tsx#L4-L4: addtypeto the import statement (import type { SelectedSecret }).🤖 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 3 - 9, Use type-only imports for the listed symbols: update frontend/src/lib/rosa-hcp-api.ts lines 3-9, frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.ts line 4, and frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.test.tsx line 4 to use import type for AwsAccountIdsResponse, OIDCConfigResponse, OrganizationQuotaResponse, WizardBasePayload, WizardErrorResponse, and SelectedSecret.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 29-32: Update the aws_account_id field in the OIDCConfigReq type
from number to string so leading zeros in 12-digit AWS account IDs are
preserved.
- Around line 108-118: Update the request-body accumulation in the req data/end
handlers to preserve the exact payload: collect chunks as Buffers or explicitly
decode each chunk to strings, then concatenate with an empty separator rather
than the default comma. Ensure JSON.parse in the end handler receives the
original JSON without inserted delimiters or unsafe implicit conversions.
- Line 121: Update the accountPath construction in the rosa wizard request to
URL-encode the entire search query parameter value, preserving the existing
filter expression while encoding its spaces and special characters before
sending it upstream.
In `@backend/test/routes/rosaWizardApi.test.ts`:
- Around line 111-164: Update the OIDC config tests around the POST
/oidc-configs cases to define aws_account_id as a string and replace exact
query-string paths in both nock(API_HOST) handlers with .query() matching the
expected search parameter, so URL-encoded query values are accepted while
preserving the existing responses and assertions.
In
`@frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.ts`:
- Line 12: Update the queryKey construction in useFetchOIDCConfigs to safely
handle a nullish selectedSecret by using its optional client_id with an
empty-string fallback, or remove the enabled null check only if the
SelectedSecret type guarantees it is always defined. Ensure query initialization
cannot access client_id on nullish data before enabled is evaluated.
---
Nitpick comments:
In `@frontend/src/lib/rosa-hcp-api.ts`:
- Around line 3-9: Use type-only imports for the listed symbols: update
frontend/src/lib/rosa-hcp-api.ts lines 3-9,
frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.ts
line 4, and
frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.test.tsx
line 4 to use import type for AwsAccountIdsResponse, OIDCConfigResponse,
OrganizationQuotaResponse, WizardBasePayload, WizardErrorResponse, and
SelectedSecret.
In
`@frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/queryKeyFactory.ts`:
- Line 8: Rename the oidcConfigs parameter aws_account_id to awsAccountId in the
query key factory, and update its usage in the returned key tuple while
preserving the existing key structure.
In
`@frontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.ts`:
- Around line 28-30: Update the fetch callback in useFetchOIDCConfigs to remove
the unnecessary async modifier and Promise<void> return type, leaving it as a
synchronous callback that only calls setAwsAccountId.
- Around line 32-37: Define a stable empty-array constant outside the hook in
useFetchOIDCConfigs, then replace the inline data ?? [] fallback with that
constant. Keep the existing data behavior unchanged when fetched data is
available.
🪄 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: 35da8d01-355c-4203-b911-e458ad4c4585
📒 Files selected for processing (11)
backend/src/app.tsbackend/src/routes/rosaWizardApi.tsbackend/test/routes/rosaWizardApi.test.tsfrontend/src/lib/rosa-hcp-api.test.tsfrontend/src/lib/rosa-hcp-api.tsfrontend/src/resources/rosa-hcp-wizard.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/useFetchOIDCConfigs.test.tsxfrontend/src/routes/Infrastructure/Clusters/ManagedClusters/components/rosahcp/queries/useFetchOIDCConfigs.tsfrontend/webpack.config.ts
Signed-off-by: David Aznaurov <daznauro@redhat.com>
Signed-off-by: David Aznaurov <daznauro@redhat.com>
|
1 participant
📝 Summary
Ticket Summary (Title):
Api call for OIDC configs for 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
POST /oidc-configs./multicloud/oidc-configs).