ACM-30326 Implement private repo support for AppSet wizard#5789
Open
fxiang1 wants to merge 6 commits intostolostron:mainfrom
Open
ACM-30326 Implement private repo support for AppSet wizard#5789fxiang1 wants to merge 6 commits intostolostron:mainfrom
fxiang1 wants to merge 6 commits intostolostron:mainfrom
Conversation
Signed-off-by: fxiang1 <fxiang@redhat.com>
Signed-off-by: fxiang1 <fxiang@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fxiang1 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughAdds private repository credential support: backend Secret watcher for Argo CD repository secrets, UI translations and informational alert, Recoil-provided secrets passed into ArgoWizard, secret-aware Git selectors using secret credentials for branch/path lookups, and Argo CD URL helper extended to target specific subpaths. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as ArgoWizard UI
participant State as Recoil (secretsState)
participant Selector as GitRevision/GitPath Selector
participant API as Backend/API Client
participant Argo as Argo CD
User->>UI: open repository step / click "Configure repository credentials"
UI->>State: read repoSecrets
UI->>Selector: render with secrets prop
Selector->>State: find matching repo-type Secret
alt Secret found
Selector->>API: getGitChannelBranches/Paths (with credentials)
API->>Argo: request branches/paths (auth)
Argo-->>API: return branches/paths
API-->>Selector: return results
else No secret
Selector->>API: getGitBranchList/getGitPathList (no auth)
API->>Argo: request branches/paths (no auth)
Argo-->>API: return branches/paths
API-->>Selector: return results
end
UI->>User: display branch/path options
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
fxiang1
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
frontend/src/wizards/Argo/ArgoWizard.tsx (1)
200-232:⚠️ Potential issue | 🟠 MajorScope
repoSecretsto the currently selected Argo namespace before reusing them.These new paths consume the raw
props.repoSecretslist, but Argo CD repository credentials are namespace-scoped to the selected Argo server/ApplicationSet namespace. Right now the wizard can offer private repo URLs from another namespace and resolve revisions/paths with the wrong secret, even though the resulting ApplicationSet cannot use that credential.Also applies to: 544-585
🧹 Nitpick comments (1)
frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.test.ts (1)
95-108: Consider adding test coverage for the newtargetPathparameter.The existing tests correctly use the renamed
openArgoCDURLfunction, but there's no test coverage for the newtargetPathparameter functionality. Consider adding a test case that verifies the URL construction whentargetPathis provided (e.g.,'/settings/repos').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.test.ts` around lines 95 - 108, Add a unit test in topologyAppSet.test.ts that covers the new targetPath behavior of openArgoCDURL: call openArgoCDURL with appropriate args (e.g., cluster 'local-cluster', namespace 'openshift-gitops', app name 'test-app', a toggleLoading mock, i18n mock t, and a targetPath like '/settings/repos') and assert that window.open (mockWindowOpen) is invoked with the expected constructed URL including the targetPath; reset mockWindowOpen in beforeEach and verify call count and URL string to ensure path is appended correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/GitOpsPrivateRepoAlert.tsx`:
- Around line 32-38: The CTA always passes cluster: hubClusterName to
processResourceActionLink, which forces opening Argo repo settings on the hub
(push model) even for pull-model users; update the onClick handler to detect the
deployment model and pass the appropriate cluster identifier instead of always
hubClusterName: if the app is in push mode keep cluster: hubClusterName, but for
pull mode pass the target/managed cluster (e.g., the current managed cluster
variable or iterate through managed clusters) so the action
'open_argo_repo_settings' opens settings on each target cluster where
credentials are required; change the call site around
processResourceActionLink({ action: 'open_argo_repo_settings', namespace,
cluster: ... }) to select the correct cluster id based on the deployment model
flag or props.
In `@frontend/src/wizards/Argo/ArgoWizard.tsx`:
- Around line 566-570: The namespace prop for GitOpsPrivateRepoAlert is coming
from the stale applicationSet built from props.resources; change it to read the
live ApplicationSet from the wizard's state (the in-memory draft the user is
editing) instead of applicationSet from props. Locate the GitOpsPrivateRepoAlert
usage and replace applicationSet?.metadata?.namespace ?? '' with the live wizard
state value (e.g., the draft/ApplicationSet object the wizard updates—often
named something like wizardState.applicationSet, applicationSetDraft, or
state.applicationSet) so the alert and its "Configure repository credentials"
action always use the current namespace selected by the user.
In `@frontend/src/wizards/Argo/common/GitPathSelect.tsx`:
- Around line 28-56: In gitPathsAsyncCallback, the secret-backed branch calls
getGitChannelPaths even when revision (targetRevision) is empty; add the same
empty-revision guard used in the public flow so that if revision is falsy/empty
the function returns [] early instead of invoking getGitChannelPaths for the
found secret; update the logic around secret, revision, repoURL and
getGitChannelPaths to return [] when revision is not set.
---
Nitpick comments:
In
`@frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.test.ts`:
- Around line 95-108: Add a unit test in topologyAppSet.test.ts that covers the
new targetPath behavior of openArgoCDURL: call openArgoCDURL with appropriate
args (e.g., cluster 'local-cluster', namespace 'openshift-gitops', app name
'test-app', a toggleLoading mock, i18n mock t, and a targetPath like
'/settings/repos') and assert that window.open (mockWindowOpen) is invoked with
the expected constructed URL including the targetPath; reset mockWindowOpen in
beforeEach and verify call count and URL string to ensure path is appended
correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3821949d-b25c-4c2d-a0d2-37ec5631f550
📒 Files selected for processing (17)
backend/src/routes/events.tsfrontend/public/locales/en/translation.jsonfrontend/src/components/GitOpsPrivateRepoAlert.tsxfrontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/helpers/diagram-helpers.tsfrontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.test.tsfrontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.tsfrontend/src/routes/Applications/CreateArgoApplication/CreateApplicationArgo.tsxfrontend/src/routes/Applications/CreateArgoApplication/CreateApplicationArgoPullModel.tsxfrontend/src/routes/Applications/CreateArgoApplication/EditArgoApplicationSet.tsxfrontend/src/wizards/Argo/ArgoWizard.tsxfrontend/src/wizards/Argo/MultipleGeneratorSelector.tsxfrontend/src/wizards/Argo/MultipleSourcesSelector.tsxfrontend/src/wizards/Argo/SourceSelector.tsxfrontend/src/wizards/Argo/common/GitPathSelect.tsxfrontend/src/wizards/Argo/common/GitRevisionSelect.tsxfrontend/src/wizards/Argo/common/GitSelectors.test.tsxfrontend/src/wizards/Argo/common/index.tsx
Signed-off-by: fxiang1 <fxiang@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/wizards/Argo/common/GitSelectors.test.tsx (1)
23-29:⚠️ Potential issue | 🟡 MinorMirror the real
getGitBranchList/getGitPathListcontracts in this mock.The real helpers forward
channel.spec.pathname,secretArgs, theurlfallback, and the empty-branch short-circuit. This mock drops all of that and callsgetGitChannelBranches/Pathswith the wholechannelobject instead, so the fallback tests will still pass even if the component stops wiring the helper correctly.Suggested fix
jest.mock('../ArgoWizard', () => ({ getGitBranchList: jest.fn((channel: any, getGitChannelBranches: any) => - getGitChannelBranches(channel).then((branches: string[]) => branches) + getGitChannelBranches(channel.spec.pathname, { + secretRef: channel.spec?.secretRef?.name, + namespace: channel.metadata?.namespace, + }).then((branches: string[]) => branches) ), - getGitPathList: jest.fn((channel: any, revision: any, getGitChannelPaths: any) => - getGitChannelPaths(channel, revision).then((paths: string[]) => paths) + getGitPathList: jest.fn((channel: any, revision: any, getGitChannelPaths: any, url?: string) => + !revision + ? Promise.resolve([]) + : getGitChannelPaths(channel?.spec?.pathname || url, revision, { + secretRef: channel?.spec?.secretRef?.name, + namespace: channel.metadata?.namespace, + }).then((paths: string[]) => paths) ), }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/wizards/Argo/common/GitSelectors.test.tsx` around lines 23 - 29, The mock for ../ArgoWizard does not mirror the real getGitBranchList/getGitPathList contracts — it passes the whole channel to getGitChannelBranches/getGitChannelPaths and omits forwarding channel.spec.pathname, secretArgs, the url fallback and the empty-branch short-circuit; update the mock implementations of getGitBranchList and getGitPathList to accept the same parameters as the real helpers and forward channel.spec.pathname (or pathname fallback to url), pass through secretArgs, implement the empty-branch short-circuit, and then call getGitChannelBranches/getGitChannelPaths with the exact forwarded args so tests exercise the same wiring as production.frontend/src/wizards/Argo/ArgoWizard.tsx (1)
195-232:⚠️ Potential issue | 🟠 MajorScope
repoSecretsto the selected Argo server namespace before using them.
CreateApplicationArgo.tsxpasses the fullsecretsStateintorepoSecrets, and this code folds that unfiltered list intogitGeneratorReposand forwards it to the repository selectors. The Git selector components then use the matched secret'smetadata.namespacefor branch/path fetches, so a repository secret from a different GitOps namespace can show up in the dropdowns and drive lookups with the wrong credentials for the selected Argo server.Derive a namespace-scoped secret list from the live
ApplicationSet.metadata.namespaceand reuse that list here.Also applies to: 552-580
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/wizards/Argo/ArgoWizard.tsx` around lines 195 - 232, Scope the repoSecrets to the selected Argo server namespace before building gitGeneratorRepos: derive a filtered list (e.g., scopedRepoSecrets) by filtering props.repoSecrets where secret.metadata.namespace matches the live ApplicationSet.metadata.namespace (or the currently selected Argo namespace from CreateApplicationArgo), then use scopedRepoSecrets instead of props.repoSecrets in the gitGeneratorRepos computation and the similar block at 552-580; keep using findGeneratorPathWithGenType, get, and the same repo parsing logic but operate on the namespace-scoped list so selectors only receive credentials from the correct namespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/wizards/Argo/SourceSelector.test.tsx`:
- Around line 37-49: The test doubles for WizAsyncSelect, WizSelect, WizHidden,
WizTiles, and WizTextInput render only id attributes but the tests query by
data-testid; update each mock component to include a data-testid attribute
(e.g., data-testid={`async-select-${label}`} for WizAsyncSelect,
data-testid={`select-${label}`} and data-testid={`option-${opt}`} for options
rendered in WizSelect, data-testid for WizHidden children wrapper, WizTiles and
WizTextInput) so the test queries (getByTestId/queryByTestId) can locate the
elements.
---
Outside diff comments:
In `@frontend/src/wizards/Argo/ArgoWizard.tsx`:
- Around line 195-232: Scope the repoSecrets to the selected Argo server
namespace before building gitGeneratorRepos: derive a filtered list (e.g.,
scopedRepoSecrets) by filtering props.repoSecrets where
secret.metadata.namespace matches the live ApplicationSet.metadata.namespace (or
the currently selected Argo namespace from CreateApplicationArgo), then use
scopedRepoSecrets instead of props.repoSecrets in the gitGeneratorRepos
computation and the similar block at 552-580; keep using
findGeneratorPathWithGenType, get, and the same repo parsing logic but operate
on the namespace-scoped list so selectors only receive credentials from the
correct namespace.
In `@frontend/src/wizards/Argo/common/GitSelectors.test.tsx`:
- Around line 23-29: The mock for ../ArgoWizard does not mirror the real
getGitBranchList/getGitPathList contracts — it passes the whole channel to
getGitChannelBranches/getGitChannelPaths and omits forwarding
channel.spec.pathname, secretArgs, the url fallback and the empty-branch
short-circuit; update the mock implementations of getGitBranchList and
getGitPathList to accept the same parameters as the real helpers and forward
channel.spec.pathname (or pathname fallback to url), pass through secretArgs,
implement the empty-branch short-circuit, and then call
getGitChannelBranches/getGitChannelPaths with the exact forwarded args so tests
exercise the same wiring as production.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f2583f68-d932-4e11-83b1-81368146396e
📒 Files selected for processing (5)
frontend/src/components/GitOpsPrivateRepoAlert.tsxfrontend/src/wizards/Argo/ArgoWizard.tsxfrontend/src/wizards/Argo/SourceSelector.test.tsxfrontend/src/wizards/Argo/common/GitPathSelect.tsxfrontend/src/wizards/Argo/common/GitSelectors.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/wizards/Argo/common/GitPathSelect.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/src/wizards/Argo/SourceSelector.test.tsx (1)
36-48:⚠️ Potential issue | 🔴 CriticalMock components use
idbut tests query bydata-testid.The mock components render with
id={...}attributes, but the tests usegetByTestId(),getAllByTestId(), andqueryByTestId()queries which search for thedata-testidattribute. These tests will fail to locate the mocked elements.,
🐛 Proposed fix to use data-testid
- WizAsyncSelect: ({ label }: any) => <div id={`async-select-${label}`} />, + WizAsyncSelect: ({ label }: any) => <div data-testid={`async-select-${label}`} />, WizSelect: ({ label, options }: any) => ( - <div id={`select-${label}`}> + <div data-testid={`select-${label}`}> {options?.map((opt: string) => ( - <span key={opt} id={`option-${opt}`}> + <span key={opt} data-testid={`option-${opt}`}> {opt} </span> ))} </div> ), WizHidden: ({ children }: any) => <>{children}</>, - WizTiles: ({ label }: any) => <div id={`tiles-${label}`} />, - WizTextInput: ({ label }: any) => <div id={`text-input-${label}`} />, + WizTiles: ({ label }: any) => <div data-testid={`tiles-${label}`} />, + WizTextInput: ({ label }: any) => <div data-testid={`text-input-${label}`} />,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/wizards/Argo/SourceSelector.test.tsx` around lines 36 - 48, The mocked wizard components (WizAsyncSelect, WizSelect, WizHidden, WizTiles, WizTextInput) currently render elements with id attributes but tests use getByTestId/getAllByTestId/queryByTestId, so update each mock to add matching data-testid attributes (e.g., data-testid={`async-select-${label}`} for WizAsyncSelect, data-testid={`select-${label}`} and on each option data-testid={`option-${opt}`} for WizSelect, data-testid for WizHidden children wrapper, WizTiles, and WizTextInput) so the test queries can find them; keep existing ids if desired but ensure data-testid values mirror the ids used in tests.
🧹 Nitpick comments (3)
frontend/src/routes/Search/SearchPage.tsx (1)
222-222: Consider using a more specific type instead ofany.Using
anybypasses TypeScript's type checking. IfsearchCompletereturns(string | null)[], consider typing the parameter explicitly to preserve type safety.♻️ Suggested improvement
- (_.get(searchCompleteData || [], 'searchComplete') ?? []).filter((s: any) => s !== null), + (_.get(searchCompleteData || [], 'searchComplete') ?? []).filter((s: string | null): s is string => s !== null),This uses a type predicate to narrow the array type from
(string | null)[]tostring[]after filtering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/routes/Search/SearchPage.tsx` at line 222, The filter callback currently uses (s: any) which defeats TypeScript checks; update the filter on (_.get(searchCompleteData || [], 'searchComplete') ?? []) to use a specific type like (s: string | null): s is string and implement a type predicate that returns true for non-null strings so the resulting array is inferred as string[] instead of any[]; look for the filter invocation in SearchPage (the expression referencing searchCompleteData and 'searchComplete') and replace the any-typed parameter with the typed predicate.frontend/src/wizards/Argo/SourceSelector.test.tsx (2)
276-279: SamequerySelectorinconsistency as in Git tests.This uses the same CSS ID selector pattern. Update along with line 190 for consistency.
♻️ Suggested refactor
- expect(helmSelect.querySelector('#option-https\\:\\/\\/github\\.com\\/private\\/repo')).toBeNull() + expect(screen.queryByTestId('option-https://github.com/private/repo')).not.toBeInTheDocument()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/wizards/Argo/SourceSelector.test.tsx` around lines 276 - 279, Replace the brittle CSS ID selector that escapes colons with a stable data-testid lookup: change the assertion that uses helmSelect.querySelector('#option-https\\:\\/\\/github\\.com\\/private\\/repo') to check for helmSelect.querySelector('[data-testid="option-https://github.com/private/repo"]') (or use within(helmSelect).queryByTestId('option-https://github.com/private/repo') if you prefer RTL helpers) and update the matching test at the other occurrence (the similar selector earlier in this file) to use the same data-testid-based approach so both tests are consistent.
188-191: Inconsistent query method:querySelectorwith CSS ID selector.This test uses
.querySelector('#option-...')with a CSS ID selector, which differs from other tests that usegetByTestId. After fixing the mocks to usedata-testid, this query will also need to be updated for consistency.♻️ Suggested refactor for consistency after fixing mocks
- expect(gitSelect.querySelector('#option-https\\:\\/\\/charts\\.example\\.com\\/stable')).toBeNull() + expect(screen.queryByTestId('option-https://charts.example.com/stable')).not.toBeInTheDocument()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/wizards/Argo/SourceSelector.test.tsx` around lines 188 - 191, The test is using gitSelect.querySelector('#option-https\\:\\/\\/charts\\.example\\.com\\/stable') which mixes CSS ID selectors with the rest of the tests that use data-testid; change this to use the testing-library query APIs: use within(gitSelect).queryByTestId('option-https://charts.example.com/stable') (or the sanitized data-testid you added in the mocks) and assert it is null, updating the test to reference the same data-testid naming convention used elsewhere; keep the variables selects and gitSelect as-is to locate the correct element.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@frontend/src/wizards/Argo/SourceSelector.test.tsx`:
- Around line 36-48: The mocked wizard components (WizAsyncSelect, WizSelect,
WizHidden, WizTiles, WizTextInput) currently render elements with id attributes
but tests use getByTestId/getAllByTestId/queryByTestId, so update each mock to
add matching data-testid attributes (e.g., data-testid={`async-select-${label}`}
for WizAsyncSelect, data-testid={`select-${label}`} and on each option
data-testid={`option-${opt}`} for WizSelect, data-testid for WizHidden children
wrapper, WizTiles, and WizTextInput) so the test queries can find them; keep
existing ids if desired but ensure data-testid values mirror the ids used in
tests.
---
Nitpick comments:
In `@frontend/src/routes/Search/SearchPage.tsx`:
- Line 222: The filter callback currently uses (s: any) which defeats TypeScript
checks; update the filter on (_.get(searchCompleteData || [], 'searchComplete')
?? []) to use a specific type like (s: string | null): s is string and implement
a type predicate that returns true for non-null strings so the resulting array
is inferred as string[] instead of any[]; look for the filter invocation in
SearchPage (the expression referencing searchCompleteData and 'searchComplete')
and replace the any-typed parameter with the typed predicate.
In `@frontend/src/wizards/Argo/SourceSelector.test.tsx`:
- Around line 276-279: Replace the brittle CSS ID selector that escapes colons
with a stable data-testid lookup: change the assertion that uses
helmSelect.querySelector('#option-https\\:\\/\\/github\\.com\\/private\\/repo')
to check for
helmSelect.querySelector('[data-testid="option-https://github.com/private/repo"]')
(or use
within(helmSelect).queryByTestId('option-https://github.com/private/repo') if
you prefer RTL helpers) and update the matching test at the other occurrence
(the similar selector earlier in this file) to use the same data-testid-based
approach so both tests are consistent.
- Around line 188-191: The test is using
gitSelect.querySelector('#option-https\\:\\/\\/charts\\.example\\.com\\/stable')
which mixes CSS ID selectors with the rest of the tests that use data-testid;
change this to use the testing-library query APIs: use
within(gitSelect).queryByTestId('option-https://charts.example.com/stable') (or
the sanitized data-testid you added in the mocks) and assert it is null,
updating the test to reference the same data-testid naming convention used
elsewhere; keep the variables selects and gitSelect as-is to locate the correct
element.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e8257e3-2738-4e8b-a306-3dfccae71b6c
📒 Files selected for processing (3)
frontend/src/routes/Applications/CreateArgoApplication/EditArgoApplicationSet.tsxfrontend/src/routes/Search/SearchPage.tsxfrontend/src/wizards/Argo/SourceSelector.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/routes/Applications/CreateArgoApplication/EditArgoApplicationSet.tsx
Signed-off-by: fxiang1 <fxiang@redhat.com>
Signed-off-by: fxiang1 <fxiang@redhat.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/wizards/Argo/ArgoWizard.tsx (1)
180-180: Consider defining a type forsourceinstead of usingany.Using explicit
anybypasses TypeScript's type safety. Sincesourcesshould have a known shape (matchingRepositoryProps), consider using that type or creating a dedicated interface.♻️ Suggested improvement
- sources.forEach((source: any) => { + sources.forEach((source: RepositoryProps) => {Also applies to: 250-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/wizards/Argo/ArgoWizard.tsx` at line 180, The code uses an untyped parameter in sources.forEach((source: any) => ...) which defeats TypeScript's checks; replace the any with the correct shape (e.g., RepositoryProps or a new interface like ArgoRepository) and update all occurrences (including the other instance around line 250) so functions/loops that iterate sources use that type — locate the sources.forEach callback in ArgoWizard (and any other similar loops) and change source: any to source: RepositoryProps (or the new interface) and adjust imports/definitions if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/wizards/Argo/ArgoWizard.tsx`:
- Line 180: The code uses an untyped parameter in sources.forEach((source: any)
=> ...) which defeats TypeScript's checks; replace the any with the correct
shape (e.g., RepositoryProps or a new interface like ArgoRepository) and update
all occurrences (including the other instance around line 250) so
functions/loops that iterate sources use that type — locate the sources.forEach
callback in ArgoWizard (and any other similar loops) and change source: any to
source: RepositoryProps (or the new interface) and adjust imports/definitions if
needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a5dd7b5-1196-4911-a4d3-a6e43bf124f8
📒 Files selected for processing (2)
frontend/src/routes/Search/SearchPage.tsxfrontend/src/wizards/Argo/ArgoWizard.tsx
Contributor
Author
|
/assign @jeswanke |
fxiang1
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Summary
Ticket Summary (Title):
Implement private repo support for AppSet wizard
Ticket Link:
https://issues.redhat.com/browse/ACM-30326
Type of Change:
✅ Checklist
General
ACM-12340 Fix bug with...)If Feature
If Bugfix
🗒️ Notes for Reviewers
Summary by CodeRabbit
New Features
Backend
Localization
Tests