Skip to content

EDM-4049: Do now show sync status of RS without repositories - #661

Merged
celdrake merged 1 commit into
flightctl:mainfrom
celdrake:EDM-4049-deleted-repository-rs
Sep 3, 2026
Merged

EDM-4049: Do now show sync status of RS without repositories#661
celdrake merged 1 commit into
flightctl:mainfrom
celdrake:EDM-4049-deleted-repository-rs

Conversation

@celdrake

@celdrake celdrake commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

If users delete the repository that a RS is associated with, the UI was still showing the Pending/Error notification about that RS status.

Since eventually the RS will fail with a condition, and the resource sync status won't change while the repository is missing, we'll hide those notifications.

  status:
    conditions:
    - lastTransitionTime: "2026-06-01T07:49:41.454211613Z"
      message: Repository of name "test-repo" not found
      reason: repository resource not found
      status: "False"
      type: Accessible

Summary by CodeRabbit

  • Bug Fixes
    • Updated the shared libs/ui-components/ ResourceSync status component.
    • Hides pending and error notifications when the referenced repository does not exist.
    • Suppresses notifications while repository data is loading or unavailable.
  • The change affects shared UI behavior and can apply to both standalone and OCP plugin consumers.
  • No changes affect libs/types/, libs/i18n/, libs/cypress/, platform-specific app code, the Go auth proxy, container builds, E2E tests, or CI configuration.

@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 233fbfd7-b105-4154-8f5f-066ae0bd39c8

📥 Commits

Reviewing files that changed from the base of the PR and between 64c6a99 and e5d5c82.

📒 Files selected for processing (1)
  • libs/ui-components/src/components/ResourceSync/ResourceSyncImportStatus.tsx

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The PR updates ResourceSyncImportStatus to fetch repository data for pending or errored syncs. It suppresses alerts while repository data is loading or unavailable and excludes syncs that reference missing repositories.

Changes

ResourceSync alert filtering

Layer / File(s) Summary
Repository-aware alert selection
libs/ui-components/src/components/ResourceSync/ResourceSyncImportStatus.tsx
The component uses RepositoryList to filter pending and errored syncs by existing repositories. It shows no alerts while repository data is loading or unavailable.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to e5d5c

The change hides sync notifications for resources whose repositories were deleted, but it can also hide valid Pending/Error notifications when repository loading fails. This is a bounded UI correctness risk requiring owner awareness or follow-up.

Suggested labels: ui-components

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: hide resource sync status when repositories are missing. It contains a typo: “now” should be “not,” but the intended meaning remains clear.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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.
No-Hardcoded-Secrets ✅ Passed No hardcoded secret was introduced. The commit changes only ResourceSyncImportStatus.tsx; its 24 added lines contain a type import, repository-name filtering, a conditional repositories endpoint, …
No-Weak-Crypto ✅ Passed PASS. The pull request changes only ResourceSyncImportStatus.tsx. The added code imports repository types, filters resource syncs by repository name, and controls repository fetching. It introduces …
No-Injection-Vectors ✅ Passed PASS. The pull request changes only ResourceSyncImportStatus.tsx. The added code imports RepositoryList, calls useFetchPeriodically with the static endpoint 'repositories', and compares reposi…
Container-Privileges ✅ Passed PASS. The exact HEAD-versus-parent diff changes only ResourceSyncImportStatus.tsx. The change adds repository fetching and filtering logic. It does not add or modify a container/Kubernetes manifest,…
No-Sensitive-Data-In-Logs ✅ Passed PASS — The pull request adds no logging. The only changed file adds repository fetching and filtering logic; it does not call console, a logger, telemetry, or error-reporting API. The shared `useFet…
Resource-Leaks ✅ Passed PASS — The pull request changes only libs/ui-components/src/components/ResourceSync/ResourceSyncImportStatus.tsx. The diff contains TypeScript/React logic and no changed files under proxy/ or any …
Unchecked-Errors ✅ Passed PASS: The pull-request commit changes only libs/ui-components/src/components/ResourceSync/ResourceSyncImportStatus.tsx. It changes no Go file under proxy/, so the unchecked-errors check is not a…
Ai-Attribution ✅ Passed The pull request commit uses the permitted Made-with: Cursor attribution trailer. The merge-base shows one PR commit, and that commit has no AI Co-Authored-By trailer. An AI Co-Authored-By trail…
Generated-Files-Not-Hand-Edited ✅ Passed PASS: The pull request changes only libs/ui-components/src/components/ResourceSync/ResourceSyncImportStatus.tsx. The commit diff and the diff against main contain no changes under the protected ge…
I18n-Compliance ✅ Passed The commit changes only ResourceSyncImportStatus.tsx and adds no user-facing copy. The added string literals are the API endpoint repositories and an empty endpoint control value, so they are syst…
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

Full details: No-Hardcoded-Secrets

Explanation

No hardcoded secret was introduced. The commit changes only ResourceSyncImportStatus.tsx; its 24 added lines contain a type import, repository-name filtering, a conditional repositories endpoint, and UI state handling. Focused scans found no API key, secret/token/password assignment, embedded URL credentials, private-key material, or base64-like literal over 32 characters. FC_DISMISS_SYNCS is an existing storage-key constant, not a credential.

Full details: No-Weak-Crypto

Explanation

PASS. The pull request changes only ResourceSyncImportStatus.tsx. The added code imports repository types, filters resource syncs by repository name, and controls repository fetching. It introduces no MD5, SHA1, DES, 3DES, RC4, Blowfish, or ECB usage, no custom cryptography, and no comparison of secrets or tokens.

Full details: No-Injection-Vectors

Explanation

PASS. The pull request changes only ResourceSyncImportStatus.tsx. The added code imports RepositoryList, calls useFetchPeriodically with the static endpoint 'repositories', and compares repository names with resource-sync references. The diff introduces no eval, exec, os.system, exec.Command, dangerouslySetInnerHTML, or YAML loading. Existing yaml.load calls elsewhere are unchanged and are not caused by this pull request.

Full details: Container-Privileges

Explanation

PASS. The exact HEAD-versus-parent diff changes only ResourceSyncImportStatus.tsx. The change adds repository fetching and filtering logic. It does not add or modify a container/Kubernetes manifest, and it introduces none of privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation: true.

Full details: No-Sensitive-Data-In-Logs

Explanation

PASS — The pull request adds no logging. The only changed file adds repository fetching and filtering logic; it does not call console, a logger, telemetry, or error-reporting API. The shared useFetchPeriodically/useFetch path also contains no logging of fetched data, and the repository-wide search found no new logging in the affected component area.

Full details: Resource-Leaks

Explanation

PASS — The pull request changes only libs/ui-components/src/components/ResourceSync/ResourceSyncImportStatus.tsx. The diff contains TypeScript/React logic and no changed files under proxy/ or any Go files. It introduces no opened files, HTTP response bodies, network connections, or goroutines in the checked scope. Therefore, the Go resource-leak conditions are not applicable and no failure was introduced.

Full details: Unchecked-Errors

Explanation

PASS: The pull-request commit changes only libs/ui-components/src/components/ResourceSync/ResourceSyncImportStatus.tsx. It changes no Go file under proxy/, so the unchecked-errors check is not applicable.

Full details: Ai-Attribution

Explanation

The pull request commit uses the permitted Made-with: Cursor attribution trailer. The merge-base shows one PR commit, and that commit has no AI Co-Authored-By trailer. An AI Co-Authored-By trailer exists only in an unrelated parent commit outside this pull request range.

Full details: Generated-Files-Not-Hand-Edited

Explanation

PASS: The pull request changes only libs/ui-components/src/components/ResourceSync/ResourceSyncImportStatus.tsx. The commit diff and the diff against main contain no changes under the protected generated paths, and the working tree has no changes in those paths.

Full details: I18n-Compliance

Explanation

The commit changes only ResourceSyncImportStatus.tsx and adds no user-facing copy. The added string literals are the API endpoint repositories and an empty endpoint control value, so they are system/configuration values. Existing user-facing alert text remains wrapped in <Trans t={t}>, and existing titles use hardcoded literal keys such as t('Fleets import failed'); no t() call uses a variable key.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@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: 1

🤖 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 `@libs/ui-components/src/components/ResourceSync/ResourceSyncImportStatus.tsx`:
- Around line 167-180: The current memo treats repoList being undefined as “hide
all alerts”; change the logic so you only filter out RS entries when you
actually have a valid repoList—otherwise preserve the original lists. In the
React.useMemo for pendingRs/errorRs (referencing repoList, isLoadingRepos,
allPendingRs, allErrorRs, removeRsWithMissingRepositories), return { pendingRs:
allPendingRs, errorRs: allErrorRs } when repoList is undefined or loading, and
only call removeRsWithMissingRepositories when repoList is present
(non-undefined) to avoid suppressing alerts on transient fetch failures.
🪄 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: 301ba0e6-2832-41a0-96a9-b624ef6b60a3

📥 Commits

Reviewing files that changed from the base of the PR and between 905a256 and 69bcc42.

📒 Files selected for processing (1)
  • libs/ui-components/src/components/ResourceSync/ResourceSyncImportStatus.tsx

@celdrake
celdrake force-pushed the EDM-4049-deleted-repository-rs branch from 69bcc42 to 5e36bfc Compare August 19, 2026 11:53
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@celdrake
celdrake force-pushed the EDM-4049-deleted-repository-rs branch from 5e36bfc to e5d5c82 Compare September 2, 2026 06:47
@celdrake
celdrake requested a review from asmasarw September 3, 2026 10:26

@asmasarw asmasarw 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

@celdrake
celdrake force-pushed the EDM-4049-deleted-repository-rs branch from e5d5c82 to e57e556 Compare September 3, 2026 14:00
@celdrake
celdrake merged commit f16e00d into flightctl:main Sep 3, 2026
10 checks passed
@celdrake
celdrake deleted the EDM-4049-deleted-repository-rs branch September 4, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants