View in Argo CD action disabled (#10082)#254
Conversation
Signed-off-by: Keith Chong <kykchong@redhat.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR introduces a shared ChangesArgo URL centralization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Component as Application Tab/View
participant useArgoServer
participant RouteModel as Route Model
participant getApplicationArgoUrl
Component->>useArgoServer: useArgoServer(app)
useArgoServer->>RouteModel: getArgoServer(model, app)
RouteModel-->>useArgoServer: ArgoServer (protocol, host)
useArgoServer-->>Component: argoServer state
Component->>getApplicationArgoUrl: getApplicationArgoUrl(argoServer, app)
getApplicationArgoUrl-->>Component: argoUrl string
Component->>Component: render ArgoCDLink / useResourceRowsDV with argoUrl
Related PRs: None found. Suggested labels: refactor, frontend Suggested reviewers: None specified. 🐰 A hop through hooks, a leap through state, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #254 +/- ##
==========================================
+ Coverage 11.92% 11.94% +0.01%
==========================================
Files 154 155 +1
Lines 6272 6305 +33
Branches 2028 2109 +81
==========================================
+ Hits 748 753 +5
- Misses 5524 5552 +28
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/gitops/components/application/History/History.tsx (1)
63-69: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the empty-url fallback before appending query params.
getApplicationArgoUrlreturns''until the Argo server is available, but this concatenation turns it into a relative?resource=...link. Keephrefempty whenargoUrlis empty.Proposed fix
- argoUrl + - '?resource=&node=argoproj.io%2FApplication%2F' + - obj?.metadata?.namespace + - '%2F' + - obj?.metadata?.name + - '%2F' + - '&view=tree&resource=&operation=false&rollback=0' + argoUrl + ? `${argoUrl}?resource=&node=argoproj.io%2FApplication%2F${obj?.metadata?.namespace}%2F${obj?.metadata?.name}%2F&view=tree&resource=&operation=false&rollback=0` + : ''🤖 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 `@src/gitops/components/application/History/History.tsx` around lines 63 - 69, The `History` link builder is turning an empty `getApplicationArgoUrl` result into a relative query-only URL instead of preserving the empty fallback. Update the `href` construction in `History.tsx` so the query params are appended only when `argoUrl` is non-empty, and otherwise keep the link empty. Use the existing `getApplicationArgoUrl`/`argoUrl` logic in `History` to guard the concatenation.src/gitops/components/application/ApplicationSourcesTab.tsx (1)
199-210: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEmpty
argoUrlproduces a malformed relative link, not an empty one.When
getApplicationArgoUrl(argoServer, obj)returns''(e.g., beforeuseArgoServer's async resolution completes), the resultinghrefbecomes'?resource=&node=...&tab=parameters'— a truthy, relative URL rather than an empty/no-op href. Clicking it during that window navigates to a nonsensical path instead of doing nothing.🔧 Proposed fix
+ const argoUrl = getApplicationArgoUrl(argoServer, obj); return ( <> <ArgoCDLink href={ - getApplicationArgoUrl(argoServer, obj) + - '?resource=&node=argoproj.io%2FApplication%2F' + - obj?.metadata?.namespace + - '%2F' + - obj?.metadata?.name + - '%2F' + - '&tab=parameters' + argoUrl + ? `${argoUrl}?resource=&node=argoproj.io%2FApplication%2F${obj?.metadata?.namespace}%2F${obj?.metadata?.name}%2F&tab=parameters` + : '' } />🤖 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 `@src/gitops/components/application/ApplicationSourcesTab.tsx` around lines 199 - 210, The ArgoCD link in ApplicationSourcesTab is being built even when getApplicationArgoUrl(argoServer, obj) returns an empty string, which turns the href into a malformed relative URL. Update the ArgoCDLink href construction so it only appends the resource/tab query when the base URL is present, and otherwise pass an empty/no-op href. Use the ApplicationSourcesTab component and getApplicationArgoUrl call as the key places to fix this conditional link generation.
🤖 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 `@src/gitops/components/application/ApplicationDetailsTab.tsx`:
- Around line 49-50: Gate the Argo CD link rendering in ApplicationDetailsTab so
it only appears once the final argoUrl is available instead of passing an empty
or partial URL to ArgoCDLink. Update the logic around useArgoServer and
getApplicationArgoUrl to conditionally render the link only when argoUrl is
ready, and apply the same pattern in the related ApplicationResourcesTab and
ApplicationSourcesTab link paths so no query-only relative links are shown while
the server is still resolving.
In `@src/gitops/components/application/ApplicationResourcesTab.tsx`:
- Around line 50-51: Guard the resource-level Argo CD action in
ApplicationResourcesTab so it only renders or becomes enabled when
argoBaseURL/argoUrl is available, matching the application-level action
behavior. Update the kebab action path that calls
window.open(getResourceURL(argoBaseURL, resource), '_blank') to check for a
valid server URL first, and hide or disable the action until useArgoServer(obj)
and getApplicationArgoUrl(...) have produced a usable URL.
In `@src/gitops/components/application/graph/ApplicationGraphView.tsx`:
- Around line 343-348: The ref initializer block in ApplicationGraphView is
failing Prettier/ESLint formatting, so normalize the indentation and alignment
of the object/function arguments in that section. Update the ref initializer
near the ApplicationGraphView component so the listed symbols (application,
navigate, launchLabelsModal, launchAnnotationsModal, launchDeleteModal,
setGroupNodeStates) follow the same multiline formatting style as the rest of
the file and pass the lint gate.
In `@src/gitops/hooks/useApplicationActionsProvider.tsx`:
- Around line 154-168: Fix the formatting in useApplicationActionsProvider’s
conditional action array so it matches ESLint/Prettier expectations, and remove
the dead disabled check from the “View in Argo CD” action. Since the array item
is only included when href is truthy, the disabled: href === undefined property
in the object is unreachable; clean up the object in the cta block and keep the
existing accessReview and window.open(href, '_blank') behavior intact.
- Around line 164-166: The CTA in useApplicationActionsProvider currently opens
a new tab with window.open(href, '_blank') and should be hardened against
tabnabbing. Update the cta callback to open the link with noopener and
noreferrer behavior, preserving the existing href/new-tab flow while preventing
the opened page from accessing window.opener.
In `@src/gitops/hooks/useArgoServer.ts`:
- Around line 8-25: The useArgoServer effect can still call setArgoServer after
app or model changes, allowing an older getArgoServer request to overwrite newer
state. Add a cancellation/cleanup guard inside useEffect in useArgoServer so
only the latest request can update state, and ignore resolved results or errors
after the effect has been invalidated; keep the logic tied to getArgoServer,
setArgoServer, and the React.useEffect cleanup.
- Around line 9-22: Guard the useEffect in useArgoServer until the Route model
from useK8sModel is available, since model can be undefined on the first render
and getArgoServer(model, app) may run too early. Add a check alongside the
existing app guard so the effect returns before calling getArgoServer when model
is not ready, and keep the current React.useEffect dependency list and
argoServer state flow unchanged.
In `@src/gitops/utils/gitops.test.ts`:
- Around line 14-24: The gitops tests in getApplicationArgoUrl need prettier
cleanup because the multi-line calls are not formatted consistently. Update the
assertions in gitops.test.ts so the getApplicationArgoUrl invocations follow
standard line breaks and indentation, matching prettier output; using the
getApplicationArgoUrl test cases as the anchor should make the formatting fix
straightforward.
---
Outside diff comments:
In `@src/gitops/components/application/ApplicationSourcesTab.tsx`:
- Around line 199-210: The ArgoCD link in ApplicationSourcesTab is being built
even when getApplicationArgoUrl(argoServer, obj) returns an empty string, which
turns the href into a malformed relative URL. Update the ArgoCDLink href
construction so it only appends the resource/tab query when the base URL is
present, and otherwise pass an empty/no-op href. Use the ApplicationSourcesTab
component and getApplicationArgoUrl call as the key places to fix this
conditional link generation.
In `@src/gitops/components/application/History/History.tsx`:
- Around line 63-69: The `History` link builder is turning an empty
`getApplicationArgoUrl` result into a relative query-only URL instead of
preserving the empty fallback. Update the `href` construction in `History.tsx`
so the query params are appended only when `argoUrl` is non-empty, and otherwise
keep the link empty. Use the existing `getApplicationArgoUrl`/`argoUrl` logic in
`History` to guard the concatenation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 31c220ff-a5c0-455d-809f-03c14b976d7e
📒 Files selected for processing (11)
src/gitops/components/application/ApplicationDetailsTab.tsxsrc/gitops/components/application/ApplicationNavPage.tsxsrc/gitops/components/application/ApplicationResourcesTab.tsxsrc/gitops/components/application/ApplicationSourcesTab.tsxsrc/gitops/components/application/ApplicationSyncStatusTab.tsxsrc/gitops/components/application/History/History.tsxsrc/gitops/components/application/graph/ApplicationGraphView.tsxsrc/gitops/hooks/useApplicationActionsProvider.tsxsrc/gitops/hooks/useArgoServer.tssrc/gitops/utils/gitops.test.tssrc/gitops/utils/gitops.ts
| const argoServer = useArgoServer(obj); | ||
| const argoUrl = getApplicationArgoUrl(argoServer, obj); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- files ---\n'
git ls-files 'src/gitops/components/application/*' 'src/gitops/hooks/*' | sed -n '1,200p'
printf '\n--- relevant symbols ---\n'
rg -n "useArgoServer|getApplicationArgoUrl|ArgoCDLink|View in Argo CD|useApplicationActionsProvider" src/gitops/components src/gitops/hooks -S
printf '\n--- ApplicationDetailsTab excerpt ---\n'
sed -n '1,220p' src/gitops/components/application/ApplicationDetailsTab.tsx
printf '\n--- ApplicationResourcesTab excerpt ---\n'
sed -n '1,220p' src/gitops/components/application/ApplicationResourcesTab.tsx
printf '\n--- ApplicationSourcesTab excerpt ---\n'
sed -n '1,220p' src/gitops/components/application/ApplicationSourcesTab.tsx
printf '\n--- useArgoServer / url helper excerpts ---\n'
rg -n "function useArgoServer|const useArgoServer|export .*useArgoServer|function getApplicationArgoUrl|const getApplicationArgoUrl|export .*getApplicationArgoUrl" src -SRepository: redhat-developer/gitops-console-plugin
Length of output: 267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- files ---'
git ls-files 'src/gitops/components/application/*' 'src/gitops/hooks/*' | sed -n '1,200p'
echo
echo '--- relevant symbols ---'
rg -n "useArgoServer|getApplicationArgoUrl|ArgoCDLink|View in Argo CD|useApplicationActionsProvider" src/gitops/components src/gitops/hooks -S
echo
echo '--- ApplicationDetailsTab excerpt ---'
sed -n '1,220p' src/gitops/components/application/ApplicationDetailsTab.tsx
echo
echo '--- ApplicationResourcesTab excerpt ---'
sed -n '1,220p' src/gitops/components/application/ApplicationResourcesTab.tsx
echo
echo '--- ApplicationSourcesTab excerpt ---'
sed -n '1,220p' src/gitops/components/application/ApplicationSourcesTab.tsx
echo
echo '--- useArgoServer / url helper excerpts ---'
rg -n "function useArgoServer|const useArgoServer|export .*useArgoServer|function getApplicationArgoUrl|const getApplicationArgoUrl|export .*getApplicationArgoUrl" src -SRepository: redhat-developer/gitops-console-plugin
Length of output: 30790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- useArgoServer.ts ---'
cat -n src/gitops/hooks/useArgoServer.ts
echo
echo '--- utils/gitops.ts around getApplicationArgoUrl ---'
sed -n '80,150p' src/gitops/utils/gitops.ts | cat -n
echo
echo '--- ArgoCDLink.tsx ---'
cat -n src/gitops/components/shared/ArgoCDLink/ArgoCDLink.tsx
echo
echo '--- ApplicationDetailsTab around the link ---'
sed -n '120,150p' src/gitops/components/application/ApplicationDetailsTab.tsx | cat -n
echo
echo '--- ApplicationResourcesTab around the link ---'
sed -n '105,125p' src/gitops/components/application/ApplicationResourcesTab.tsx | cat -n
echo
echo '--- ApplicationSourcesTab around the link ---'
sed -n '190,215p' src/gitops/components/application/ApplicationSourcesTab.tsx | cat -nRepository: redhat-developer/gitops-console-plugin
Length of output: 8354
Hide Argo CD links until the URL is ready
ApplicationDetailsTab.tsxandApplicationResourcesTab.tsxpass''toArgoCDLinkwhileuseArgoServeris still resolving.ApplicationSourcesTab.tsxturns that same state into a query-only relative link (?resource=...).
Gate these links on the final argoUrl before rendering.
🤖 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 `@src/gitops/components/application/ApplicationDetailsTab.tsx` around lines 49
- 50, Gate the Argo CD link rendering in ApplicationDetailsTab so it only
appears once the final argoUrl is available instead of passing an empty or
partial URL to ArgoCDLink. Update the logic around useArgoServer and
getApplicationArgoUrl to conditionally render the link only when argoUrl is
ready, and apply the same pattern in the related ApplicationResourcesTab and
ApplicationSourcesTab link paths so no query-only relative links are shown while
the server is still resolving.
| const argoServer = useArgoServer(obj); | ||
| const argoUrl = getApplicationArgoUrl(argoServer, obj); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether useResourceActionsProvider conditionally handles an empty argoBaseURL
# similar to useApplicationActionsProvider's href-truthy check.
fd -e ts -e tsx useResourceActionsProvider
rg -n -A 20 'useResourceActionsProvider' --type=tsx --type=tsRepository: redhat-developer/gitops-console-plugin
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== outline: ApplicationResourcesTab =="
ast-grep outline src/gitops/components/application/ApplicationResourcesTab.tsx --view expanded || true
echo
echo "== outline: useResourceActionsProvider =="
ast-grep outline src/gitops/hooks/useResourceActionsProvider.tsx --view expanded || true
echo
echo "== outline: useApplicationActionsProvider =="
fd -a useApplicationActionsProvider src/gitops/hooks src/gitops/components || true
if [ -f src/gitops/hooks/useApplicationActionsProvider.tsx ]; then
ast-grep outline src/gitops/hooks/useApplicationActionsProvider.tsx --view expanded || true
fi
echo
echo "== relevant matches =="
rg -n -A 30 -B 10 'useResourceActionsProvider|useApplicationActionsProvider|argoBaseURL|argoUrl|View in Argo CD' src/gitops/components/application/ApplicationResourcesTab.tsx src/gitops/hooks/useResourceActionsProvider.tsx src/gitops/hooks/useApplicationActionsProvider.tsx || trueRepository: redhat-developer/gitops-console-plugin
Length of output: 38061
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== useResourceActionsProvider =="
cat -n src/gitops/hooks/useResourceActionsProvider.tsx | sed -n '1,260p'
echo
echo "== useApplicationActionsProvider =="
fd -a useApplicationActionsProvider src/gitops/hooks src/gitops/components || true
if [ -f src/gitops/hooks/useApplicationActionsProvider.tsx ]; then
cat -n src/gitops/hooks/useApplicationActionsProvider.tsx | sed -n '1,260p'
fi
echo
echo "== ApplicationResourcesTab around the caller =="
cat -n src/gitops/components/application/ApplicationResourcesTab.tsx | sed -n '280,340p'Repository: redhat-developer/gitops-console-plugin
Length of output: 13126
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' src/gitops/hooks/useResourceActionsProvider.tsx | cat -n
printf '\n---\n'
sed -n '1,260p' src/gitops/hooks/useApplicationActionsProvider.tsx | cat -n
printf '\n---\n'
sed -n '280,340p' src/gitops/components/application/ApplicationResourcesTab.tsx | cat -nRepository: redhat-developer/gitops-console-plugin
Length of output: 12947
Guard the resource-level Argo CD action on argoBaseURL The kebab action still always renders and calls window.open(getResourceURL(argoBaseURL, resource), '_blank'), so it can still open a broken/empty Argo CD URL when the server URL isn’t ready. Hide or disable this action until the URL exists, like the application action does.
🤖 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 `@src/gitops/components/application/ApplicationResourcesTab.tsx` around lines
50 - 51, Guard the resource-level Argo CD action in ApplicationResourcesTab so
it only renders or becomes enabled when argoBaseURL/argoUrl is available,
matching the application-level action behavior. Update the kebab action path
that calls window.open(getResourceURL(argoBaseURL, resource), '_blank') to check
for a valid server URL first, and hide or disable the action until
useArgoServer(obj) and getApplicationArgoUrl(...) have produced a usable URL.
| application, | ||
| navigate, | ||
| launchLabelsModal, | ||
| launchAnnotationsModal, | ||
| launchDeleteModal, | ||
| setGroupNodeStates, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Prettier error in the ref initializer.
ESLint reports this block as non-compliant; normalize the indentation so the lint gate passes.
Proposed fix
- application,
- navigate,
- launchLabelsModal,
- launchAnnotationsModal,
- launchDeleteModal,
- setGroupNodeStates,
+ application,
+ navigate,
+ launchLabelsModal,
+ launchAnnotationsModal,
+ launchDeleteModal,
+ setGroupNodeStates,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| application, | |
| navigate, | |
| launchLabelsModal, | |
| launchAnnotationsModal, | |
| launchDeleteModal, | |
| setGroupNodeStates, | |
| application, | |
| navigate, | |
| launchLabelsModal, | |
| launchAnnotationsModal, | |
| launchDeleteModal, | |
| setGroupNodeStates, |
🧰 Tools
🪛 ESLint
[error] 343-343: Delete ····
(prettier/prettier)
[error] 344-344: Replace ········ with ····
(prettier/prettier)
[error] 345-345: Delete ····
(prettier/prettier)
[error] 346-346: Delete ····
(prettier/prettier)
[error] 347-347: Replace ········ with ····
(prettier/prettier)
[error] 348-348: Delete ····
(prettier/prettier)
🤖 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 `@src/gitops/components/application/graph/ApplicationGraphView.tsx` around
lines 343 - 348, The ref initializer block in ApplicationGraphView is failing
Prettier/ESLint formatting, so normalize the indentation and alignment of the
object/function arguments in that section. Update the ref initializer near the
ApplicationGraphView component so the listed symbols (application, navigate,
launchLabelsModal, launchAnnotationsModal, launchDeleteModal,
setGroupNodeStates) follow the same multiline formatting style as the rest of
the file and pass the lint gate.
Source: Linters/SAST tools
| ...(href ? [{ | ||
| id: 'gitops-action-view-in-argocd', | ||
| disabled: href === undefined, | ||
| label: t('View in Argo CD'), | ||
| accessReview: { | ||
| group: ApplicationModel.apiGroup, | ||
| verb: 'get' as K8sVerb, | ||
| resource: ApplicationModel.plural, | ||
| namespace: application?.metadata?.namespace, | ||
| }, | ||
| cta: () => { | ||
| window.open(href, '_blank'); | ||
| } | ||
| }] : [] | ||
| ), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix lint formatting and drop the now-dead disabled check.
ESLint/Prettier report formatting violations across this block. Also, since the object is only spread into the array when href is truthy, disabled: href === undefined can never be true — it's dead logic left over from the previous always-present-but-disabled approach.
🧹 Proposed cleanup
- ...(href ? [{
- id: 'gitops-action-view-in-argocd',
- disabled: href === undefined,
- label: t('View in Argo CD'),
- accessReview: {
- group: ApplicationModel.apiGroup,
- verb: 'get' as K8sVerb,
- resource: ApplicationModel.plural,
- namespace: application?.metadata?.namespace,
- },
- cta: () => {
- window.open(href, '_blank');
- }
- }] : []
- ),
+ ...(href
+ ? [
+ {
+ id: 'gitops-action-view-in-argocd',
+ disabled: false,
+ label: t('View in Argo CD'),
+ accessReview: {
+ group: ApplicationModel.apiGroup,
+ verb: 'get' as K8sVerb,
+ resource: ApplicationModel.plural,
+ namespace: application?.metadata?.namespace,
+ },
+ cta: () => {
+ window.open(href, '_blank');
+ },
+ },
+ ]
+ : []),As per static analysis hints (prettier/prettier), these lines require formatting corrections.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ...(href ? [{ | |
| id: 'gitops-action-view-in-argocd', | |
| disabled: href === undefined, | |
| label: t('View in Argo CD'), | |
| accessReview: { | |
| group: ApplicationModel.apiGroup, | |
| verb: 'get' as K8sVerb, | |
| resource: ApplicationModel.plural, | |
| namespace: application?.metadata?.namespace, | |
| }, | |
| cta: () => { | |
| window.open(href, '_blank'); | |
| } | |
| }] : [] | |
| ), | |
| ...(href | |
| ? [ | |
| { | |
| id: 'gitops-action-view-in-argocd', | |
| disabled: false, | |
| label: t('View in Argo CD'), | |
| accessReview: { | |
| group: ApplicationModel.apiGroup, | |
| verb: 'get' as K8sVerb, | |
| resource: ApplicationModel.plural, | |
| namespace: application?.metadata?.namespace, | |
| }, | |
| cta: () => { | |
| window.open(href, '_blank'); | |
| }, | |
| }, | |
| ] | |
| : []), |
🧰 Tools
🪛 ESLint
[error] 154-154: Replace ·?·[ with ⏎········?·[⏎············
(prettier/prettier)
[error] 155-155: Replace ·········· with ··············
(prettier/prettier)
[error] 156-156: Insert ····
(prettier/prettier)
[error] 157-157: Replace ·········· with ··············
(prettier/prettier)
[error] 158-158: Replace ·········· with ··············
(prettier/prettier)
[error] 159-159: Insert ····
(prettier/prettier)
[error] 160-160: Insert ····
(prettier/prettier)
[error] 161-161: Replace ············ with ················
(prettier/prettier)
[error] 162-162: Insert ····
(prettier/prettier)
[error] 163-163: Insert ····
(prettier/prettier)
[error] 164-164: Replace ·········· with ··············
(prettier/prettier)
[error] 165-165: Insert ····
(prettier/prettier)
[error] 166-166: Replace } with ····},
(prettier/prettier)
[error] 167-167: Replace }]·:·[ with ····},⏎··········
(prettier/prettier)
[error] 168-168: Replace ······ with ········:·[]
(prettier/prettier)
🤖 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 `@src/gitops/hooks/useApplicationActionsProvider.tsx` around lines 154 - 168,
Fix the formatting in useApplicationActionsProvider’s conditional action array
so it matches ESLint/Prettier expectations, and remove the dead disabled check
from the “View in Argo CD” action. Since the array item is only included when
href is truthy, the disabled: href === undefined property in the object is
unreachable; clean up the object in the cta block and keep the existing
accessReview and window.open(href, '_blank') behavior intact.
Source: Linters/SAST tools
| cta: () => { | ||
| window.open(href, '_blank'); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Harden window.open against tabnabbing.
Opening a new tab via window.open(href, '_blank') without noopener/noreferrer lets the new page access window.opener and redirect the original tab.
🔒 Proposed fix
- cta: () => {
- window.open(href, '_blank');
- }
+ cta: () => {
+ window.open(href, '_blank', 'noopener,noreferrer');
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cta: () => { | |
| window.open(href, '_blank'); | |
| } | |
| cta: () => { | |
| window.open(href, '_blank', 'noopener,noreferrer'); | |
| }, |
🧰 Tools
🪛 ESLint
[error] 164-164: Replace ·········· with ··············
(prettier/prettier)
[error] 165-165: Insert ····
(prettier/prettier)
[error] 166-166: Replace } with ····},
(prettier/prettier)
🤖 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 `@src/gitops/hooks/useApplicationActionsProvider.tsx` around lines 164 - 166,
The CTA in useApplicationActionsProvider currently opens a new tab with
window.open(href, '_blank') and should be hardened against tabnabbing. Update
the cta callback to open the link with noopener and noreferrer behavior,
preserving the existing href/new-tab flow while preventing the opened page from
accessing window.opener.
| export const useArgoServer = (app?: ApplicationKind): ArgoServer => { | ||
| const [model] = useK8sModel({ group: 'route.openshift.io', version: 'v1', kind: 'Route' }); | ||
| const [argoServer, setArgoServer] = React.useState<ArgoServer>({ host: '', protocol: '' }); | ||
|
|
||
| React.useEffect(() => { | ||
| if (!app) { | ||
| return undefined; | ||
| } | ||
|
|
||
| getArgoServer(model, app) | ||
| .then(setArgoServer) | ||
| .catch((err) => { | ||
| console.error('Error:', err); | ||
| }); | ||
| }, [model, app]); | ||
|
|
||
| return argoServer; | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add cancellation guard to avoid stale state updates.
The effect has no cleanup, so if app (or model) changes before the in-flight getArgoServer promise resolves, a stale response can overwrite state with outdated data after the newer request has already started/resolved.
🔧 Proposed fix
React.useEffect(() => {
if (!app) {
return undefined;
}
-
- getArgoServer(model, app)
- .then(setArgoServer)
- .catch((err) => {
- console.error('Error:', err);
- });
+ let cancelled = false;
+
+ getArgoServer(model, app)
+ .then((server) => {
+ if (!cancelled) setArgoServer(server);
+ })
+ .catch((err) => {
+ if (!cancelled) console.error('Error:', err);
+ });
+
+ return () => {
+ cancelled = true;
+ };
}, [model, app]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useArgoServer = (app?: ApplicationKind): ArgoServer => { | |
| const [model] = useK8sModel({ group: 'route.openshift.io', version: 'v1', kind: 'Route' }); | |
| const [argoServer, setArgoServer] = React.useState<ArgoServer>({ host: '', protocol: '' }); | |
| React.useEffect(() => { | |
| if (!app) { | |
| return undefined; | |
| } | |
| getArgoServer(model, app) | |
| .then(setArgoServer) | |
| .catch((err) => { | |
| console.error('Error:', err); | |
| }); | |
| }, [model, app]); | |
| return argoServer; | |
| }; | |
| export const useArgoServer = (app?: ApplicationKind): ArgoServer => { | |
| const [model] = useK8sModel({ group: 'route.openshift.io', version: 'v1', kind: 'Route' }); | |
| const [argoServer, setArgoServer] = React.useState<ArgoServer>({ host: '', protocol: '' }); | |
| React.useEffect(() => { | |
| if (!app) { | |
| return undefined; | |
| } | |
| let cancelled = false; | |
| getArgoServer(model, app) | |
| .then((server) => { | |
| if (!cancelled) setArgoServer(server); | |
| }) | |
| .catch((err) => { | |
| if (!cancelled) console.error('Error:', err); | |
| }); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [model, app]); | |
| return argoServer; | |
| }; |
🤖 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 `@src/gitops/hooks/useArgoServer.ts` around lines 8 - 25, The useArgoServer
effect can still call setArgoServer after app or model changes, allowing an
older getArgoServer request to overwrite newer state. Add a cancellation/cleanup
guard inside useEffect in useArgoServer so only the latest request can update
state, and ignore resolved results or errors after the effect has been
invalidated; keep the logic tied to getArgoServer, setArgoServer, and the
React.useEffect cleanup.
| const [model] = useK8sModel({ group: 'route.openshift.io', version: 'v1', kind: 'Route' }); | ||
| const [argoServer, setArgoServer] = React.useState<ArgoServer>({ host: '', protocol: '' }); | ||
|
|
||
| React.useEffect(() => { | ||
| if (!app) { | ||
| return undefined; | ||
| } | ||
|
|
||
| getArgoServer(model, app) | ||
| .then(setArgoServer) | ||
| .catch((err) => { | ||
| console.error('Error:', err); | ||
| }); | ||
| }, [model, app]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect getArgoServer to see how it uses the model param and whether an unresolved model causes incorrect host resolution.
rg -n -A 20 'export (const|function) getArgoServer' src/gitops/utils/gitops.tsRepository: redhat-developer/gitops-console-plugin
Length of output: 1663
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== useArgoServer.ts ==\n'
sed -n '1,220p' src/gitops/hooks/useArgoServer.ts
printf '\n== gitops.ts getArgoServer area ==\n'
sed -n '1,180p' src/gitops/utils/gitops.ts
printf '\n== useK8sModel usages ==\n'
rg -n "useK8sModel\\(" src | sed -n '1,120p'Repository: redhat-developer/gitops-console-plugin
Length of output: 7942
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== inFlight usage with useK8sModel ==\n'
rg -n -A 3 -B 3 'useK8sModel\\(' src | sed -n '1,240p'
printf '\n== explicit inFlight references ==\n'
rg -n 'inFlight' src | sed -n '1,240p'
printf '\n== similar Route model hooks/components ==\n'
sed -n '300,380p' src/gitops/components/appset/graph/ApplicationSetGraphView.tsx
sed -n '60,120p' src/gitops/components/project/ProjectRolesTab.tsx
sed -n '60,120p' src/gitops/components/project/ProjectAllowDenyTab.tsx
sed -n '1,80p' src/gitops/components/project/ProjectSyncWindowsTab.tsxRepository: redhat-developer/gitops-console-plugin
Length of output: 294
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== explicit inFlight references ==\n'
rg -n 'inFlight' src | sed -n '1,240p'
printf '\n== useK8sModel call sites around Route model ==\n'
sed -n '320,360p' src/gitops/components/appset/graph/ApplicationSetGraphView.tsx
sed -n '70,110p' src/gitops/components/project/ProjectRolesTab.tsx
sed -n '60,100p' src/gitops/components/project/ProjectAllowDenyTab.tsx
sed -n '1,60p' src/gitops/components/project/ProjectSyncWindowsTab.tsx
printf '\n== useArgoServer consumers ==\n'
rg -n 'useArgoServer\\(' src | sed -n '1,120p'Repository: redhat-developer/gitops-console-plugin
Length of output: 212
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== useArgoServer consumers ==\n'
rg -n 'useArgoServer\(' src | sed -n '1,120p'
printf '\n== relevant consumer files ==\n'
sed -n '1,220p' src/gitops/components/project/ProjectRolesTab.tsx
sed -n '1,220p' src/gitops/components/project/ProjectAllowDenyTab.tsx
sed -n '1,120p' src/gitops/components/project/ProjectSyncWindowsTab.tsx
sed -n '300,380p' src/gitops/components/appset/graph/ApplicationSetGraphView.tsxRepository: redhat-developer/gitops-console-plugin
Length of output: 21995
🌐 Web query:
useK8sModel returns [model, inFlight] dynamic-plugin-sdk semantics and whether model is undefined before the resource is loaded
💡 Result:
The useK8sModel hook in the dynamic-plugin-sdk returns an array containing two elements: [model, inFlight] [1][2]. The model element represents the retrieved Kubernetes model (or undefined) [2][3], and the inFlight element is a boolean indicating the current loading status [1][2]. Regarding the behavior before the resource (model) is loaded: 1. Model State: Before the Kubernetes models are fully loaded or if the requested model cannot be found in the Redux store, the model value is undefined [2][3]. The hook retrieves the model from Redux, and if it is not present, it returns undefined [3]. 2. In-Flight Status: The inFlight boolean indicates whether the application is currently fetching or waiting for Kubernetes model data [1][2]. Because the hook relies on Redux state for model retrieval, it is designed to reactively update. When a component first mounts and the model data has not yet been populated in the global store, the model will typically be undefined until the underlying Redux state is updated with the available models [2][3]. Summary of return values: - model: The K8sModel object (if found) or undefined (if not found or still loading) [2][3]. - inFlight: A boolean true while the models are loading/fetching, and false once the loading process has completed [1][2].
Citations:
- 1: https://github.com/openshift/console/blob/104213c6eedaaf4dc8e07c41195871e53a9cd0c1/frontend/packages/console-dynamic-plugin-sdk/docs/api.md
- 2: https://cdn.jsdelivr.net/npm/@openshift/dynamic-plugin-sdk-utils@5.0.1/dist/index.d.ts
- 3: spadgett/console@8b67319
Guard the effect until the Route model is available. useK8sModel can return undefined on the first render, so getArgoServer(model, app) may run once with an unresolved model and emit an avoidable error before the effect reruns.
🤖 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 `@src/gitops/hooks/useArgoServer.ts` around lines 9 - 22, Guard the useEffect
in useArgoServer until the Route model from useK8sModel is available, since
model can be undefined on the first render and getArgoServer(model, app) may run
too early. Add a check alongside the existing app guard so the effect returns
before calling getArgoServer when model is not ready, and keep the current
React.useEffect dependency list and argoServer state flow unchanged.
| getApplicationArgoUrl({ protocol: 'https', host: 'openshift-gitops-server-openshift-gitops' }, { | ||
| metadata: { name: 'guestbook', namespace: 'openshift-gitops' }, | ||
| } as any), | ||
| ).toBe('https://openshift-gitops-server-openshift-gitops/applications/openshift-gitops/guestbook'); | ||
| }); | ||
|
|
||
| it('returns empty string when server or app metadata is missing', () => { | ||
| expect(getApplicationArgoUrl({ protocol: 'https', host: '' }, {} as any)).toBe(''); | ||
| expect( | ||
| getApplicationArgoUrl({ protocol: 'https', host: 'openshift-gitops-server-openshift-gitops' }, undefined), | ||
| ).toBe(''); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix prettier formatting.
ESLint/prettier flags formatting issues on lines 14-17 and 23 (missing line breaks/indentation in the multi-line getApplicationArgoUrl calls). Run prettier --write on this file.
🧰 Tools
🪛 ESLint
[error] 14-14: Replace {·protocol:·'https',·host:·'openshift-gitops-server-openshift-gitops'·}, with ⏎········{·protocol:·'https',·host:·'openshift-gitops-server-openshift-gitops'·},⏎·······
(prettier/prettier)
[error] 15-15: Insert ··
(prettier/prettier)
[error] 16-16: Replace }·as·any with ··}·as·any,⏎······
(prettier/prettier)
[error] 17-17: Replace 'https://openshift-gitops-server-openshift-gitops/applications/openshift-gitops/guestbook' with ⏎······'https://openshift-gitops-server-openshift-gitops/applications/openshift-gitops/guestbook',⏎····
(prettier/prettier)
[error] 23-23: Replace {·protocol:·'https',·host:·'openshift-gitops-server-openshift-gitops'·},·undefined with ⏎········{·protocol:·'https',·host:·'openshift-gitops-server-openshift-gitops'·},⏎········undefined,⏎······
(prettier/prettier)
🤖 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 `@src/gitops/utils/gitops.test.ts` around lines 14 - 24, The gitops tests in
getApplicationArgoUrl need prettier cleanup because the multi-line calls are not
formatted consistently. Update the assertions in gitops.test.ts so the
getApplicationArgoUrl invocations follow standard line breaks and indentation,
matching prettier output; using the getApplicationArgoUrl test cases as the
anchor should make the formatting fix straightforward.
Source: Linters/SAST tools
2 participants
See GITOPS-10082