Skip to content

View in Argo CD action disabled (#10082)#254

Merged
aali309 merged 2 commits into
redhat-developer:mainfrom
keithchong:10082-ViewInArgoCD
Jul 7, 2026
Merged

View in Argo CD action disabled (#10082)#254
aali309 merged 2 commits into
redhat-developer:mainfrom
keithchong:10082-ViewInArgoCD

Conversation

@keithchong

Copy link
Copy Markdown
Collaborator

@openshift-ci openshift-ci Bot requested a review from wtam2018 July 4, 2026 04:20
@keithchong keithchong requested review from aali309 and removed request for wtam2018 July 4, 2026 04:20
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added shared helper for generating Argo CD application URLs and a new hook to resolve the Argo CD server endpoint for application views.
  • Bug Fixes
    • Argo CD links are now conditionally displayed only when a valid destination URL can be computed, preventing empty/broken navigation targets.
    • “View in Argo CD” actions and related UI entries are omitted when their target URL isn’t available.
  • Tests
    • Added unit tests covering the new Argo CD application URL helper, including failure cases.

Walkthrough

This PR adds shared Argo server and application URL helpers, then updates application tabs and views to derive Argo CD links from those shared utilities. Navigation actions now receive the derived URL, and the Argo CD action is only included when a URL exists.

Changes

Argo URL centralization

Layer / File(s) Summary
getApplicationArgoUrl utility and tests
src/gitops/utils/gitops.ts, src/gitops/utils/gitops.test.ts
Adds getApplicationArgoUrl(argoServer, app?) and tests for URL construction and missing-input handling.
useArgoServer hook
src/gitops/hooks/useArgoServer.ts
Adds useArgoServer(app?), which resolves an ArgoServer through the Route model and getArgoServer.
Application tab URL migration
src/gitops/components/application/ApplicationDetailsTab.tsx, ApplicationResourcesTab.tsx, ApplicationSourcesTab.tsx, ApplicationSyncStatusTab.tsx
Replaces per-component Argo server fetching and inline application URL assembly with useArgoServer plus getApplicationArgoUrl, and updates link rendering and resource-row inputs to use the derived URL.
History and graph view migration
src/gitops/components/application/History/History.tsx, src/gitops/components/application/graph/ApplicationGraphView.tsx
Switches History and ApplicationGraphView to derive Argo URLs with the shared hook/helper; ApplicationGraphView also replaces the Route-specific K8s model hook with useK8sModels().
Navigation wiring and action filtering
src/gitops/components/application/ApplicationNavPage.tsx, src/gitops/hooks/useApplicationActionsProvider.tsx, src/gitops/hooks/useResourceActionsProvider.tsx
ApplicationNavPage now computes argoUrl and passes it to useApplicationActionsProvider; both action providers now include the Argo CD action only when a URL is present.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and stays on-topic, summarizing the View in Argo CD behavior change.
Description check ✅ Passed The description links to the related Jira ticket, so it is relevant to the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

codecov-commenter commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 5.88235% with 64 lines in your changes missing coverage. Please review.
✅ Project coverage is 11.92%. Comparing base (01e9db1) to head (5461244).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
src/gitops/hooks/useArgoServer.ts 0.00% 12 Missing and 1 partial ⚠️
.../gitops/components/application/History/History.tsx 0.00% 7 Missing ⚠️
...s/components/application/ApplicationDetailsTab.tsx 0.00% 6 Missing ⚠️
...components/application/ApplicationResourcesTab.tsx 0.00% 6 Missing ⚠️
...s/components/application/ApplicationSourcesTab.tsx 0.00% 6 Missing ⚠️
...ponents/application/graph/ApplicationGraphView.tsx 0.00% 6 Missing ⚠️
...tops/components/application/ApplicationNavPage.tsx 0.00% 5 Missing ⚠️
...omponents/application/ApplicationSyncStatusTab.tsx 0.00% 5 Missing ⚠️
src/gitops/hooks/useApplicationActionsProvider.tsx 0.00% 5 Missing ⚠️
src/gitops/hooks/useResourceActionsProvider.tsx 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #254      +/-   ##
==========================================
- Coverage   11.92%   11.92%   -0.01%     
==========================================
  Files         154      155       +1     
  Lines        6272     6317      +45     
  Branches     2028     2176     +148     
==========================================
+ Hits          748      753       +5     
+ Misses       5524     5341     -183     
- Partials        0      223     +223     
Flag Coverage Δ
unit-tests 11.92% <5.88%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Preserve the empty-url fallback before appending query params.

getApplicationArgoUrl returns '' until the Argo server is available, but this concatenation turns it into a relative ?resource=... link. Keep href empty when argoUrl is 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 win

Empty argoUrl produces a malformed relative link, not an empty one.

When getApplicationArgoUrl(argoServer, obj) returns '' (e.g., before useArgoServer's async resolution completes), the resulting href becomes '?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

📥 Commits

Reviewing files that changed from the base of the PR and between 9086cb7 and 0d49285.

📒 Files selected for processing (11)
  • src/gitops/components/application/ApplicationDetailsTab.tsx
  • src/gitops/components/application/ApplicationNavPage.tsx
  • src/gitops/components/application/ApplicationResourcesTab.tsx
  • src/gitops/components/application/ApplicationSourcesTab.tsx
  • src/gitops/components/application/ApplicationSyncStatusTab.tsx
  • src/gitops/components/application/History/History.tsx
  • src/gitops/components/application/graph/ApplicationGraphView.tsx
  • src/gitops/hooks/useApplicationActionsProvider.tsx
  • src/gitops/hooks/useArgoServer.ts
  • src/gitops/utils/gitops.test.ts
  • src/gitops/utils/gitops.ts

Comment thread src/gitops/components/application/ApplicationDetailsTab.tsx
Comment thread src/gitops/components/application/ApplicationResourcesTab.tsx
Comment thread src/gitops/components/application/graph/ApplicationGraphView.tsx Outdated
Comment thread src/gitops/hooks/useApplicationActionsProvider.tsx Outdated
Comment on lines +164 to +166
cta: () => {
window.open(href, '_blank');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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.

Suggested change
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.

Comment on lines +8 to +25
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;
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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.

Suggested change
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.

Comment on lines +9 to +22
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]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.ts

Repository: 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.tsx

Repository: 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.tsx

Repository: 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:


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.

Comment thread src/gitops/utils/gitops.test.ts Outdated
Signed-off-by: Keith Chong <kykchong@redhat.com>
@keithchong keithchong force-pushed the 10082-ViewInArgoCD branch from 0d49285 to 2d5c6a2 Compare July 6, 2026 18:44
@@ -71,13 +60,7 @@ const HistoryList: React.FC<HistoryListProps> = ({ history, obj }) => {
<div>
<ArgoCDLink

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with ApplicationDetailsTab and ApplicationResourcesTab, WDYT gating the Argo CD link in History and Sources tabs with
argoUrl ? <ArgoCDLink ... /> : null?

Signed-off-by: Keith Chong <kykchong@redhat.com>
@keithchong

keithchong commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

@aali309 I've updated the PR with another commit based on your comments. I changed the return value to undefined, and updated all the references. All actions and links should NOT appear if that URL is undefined. So, you will see that the link appears shortly after visiting the page.

I ran lint on the code and only committed the files I changed for this fix. There are some other files (eg. tests) that still need to be 'linted' but I think we can do that in another PR.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/gitops/hooks/useResourceActionsProvider.tsx (1)

64-66: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

window.open without noopener on external navigation.

Opening the Argo CD resource URL via window.open(url, '_blank') leaves window.opener accessible to the new page, allowing it to navigate the parent tab (reverse tabnabbing). Add noopener,noreferrer.

Proposed fix
-              cta: () => {
-                window.open(getResourceURL(argoBaseURL, resource), '_blank');
-              },
+              cta: () => {
+                window.open(getResourceURL(argoBaseURL, resource), '_blank', 'noopener,noreferrer');
+              },
🤖 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/useResourceActionsProvider.tsx` around lines 64 - 66, The
CTA in useResourceActionsProvider’s window.open call opens an external Argo CD
URL in a new tab without protecting the opener. Update the cta handler that
calls getResourceURL so the new tab is opened with noopener and noreferrer
behavior, preventing access to window.opener and avoiding reverse tabnabbing.
🤖 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.

Nitpick comments:
In `@src/gitops/hooks/useResourceActionsProvider.tsx`:
- Around line 64-66: The CTA in useResourceActionsProvider’s window.open call
opens an external Argo CD URL in a new tab without protecting the opener. Update
the cta handler that calls getResourceURL so the new tab is opened with noopener
and noreferrer behavior, preventing access to window.opener and avoiding reverse
tabnabbing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 72bd53ba-e0ae-4eb1-bc11-21cf9cf4b7a2

📥 Commits

Reviewing files that changed from the base of the PR and between 2d5c6a2 and 5461244.

📒 Files selected for processing (10)
  • src/gitops/components/application/ApplicationDetailsTab.tsx
  • src/gitops/components/application/ApplicationResourcesTab.tsx
  • src/gitops/components/application/ApplicationSourcesTab.tsx
  • src/gitops/components/application/ApplicationSyncStatusTab.tsx
  • src/gitops/components/application/History/History.tsx
  • src/gitops/components/application/graph/ApplicationGraphView.tsx
  • src/gitops/hooks/useApplicationActionsProvider.tsx
  • src/gitops/hooks/useResourceActionsProvider.tsx
  • src/gitops/utils/gitops.test.ts
  • src/gitops/utils/gitops.ts
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/gitops/utils/gitops.ts
  • src/gitops/hooks/useApplicationActionsProvider.tsx
  • src/gitops/components/application/History/History.tsx
  • src/gitops/components/application/ApplicationDetailsTab.tsx
  • src/gitops/components/application/ApplicationSourcesTab.tsx
  • src/gitops/components/application/ApplicationResourcesTab.tsx
  • src/gitops/components/application/graph/ApplicationGraphView.tsx
  • src/gitops/components/application/ApplicationSyncStatusTab.tsx

@aali309 aali309 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @keithchong

@aali309 aali309 merged commit 5505134 into redhat-developer:main Jul 7, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants