Skip to content

ACM-28831 Disable AppSet auto sync when the option is unchecked#5513

Merged
openshift-merge-bot[bot] merged 3 commits intostolostron:mainfrom
fxiang1:feng-sync
Jan 26, 2026
Merged

ACM-28831 Disable AppSet auto sync when the option is unchecked#5513
openshift-merge-bot[bot] merged 3 commits intostolostron:mainfrom
fxiang1:feng-sync

Conversation

@fxiang1
Copy link
Copy Markdown
Contributor

@fxiang1 fxiang1 commented Jan 21, 2026

📝 Summary

Ticket Summary (Title):
Implement option to disable AppSet autosync completely

  • Grouped all automated sync related checkboxes into a separate section
  • Introduced a new Enable automated sync checkbox which will disable all other settings when unchecked
  • Added tooltip to let user know settings will be ignored when automated sync is disabled
  • Checking Enable automated sync will enabled the other settings again
  • Supports old automated sync behavior when syncPolicy.automated exists but no enabled field. This will add enabled=true by default.
image

Ticket Link:
https://issues.redhat.com/browse/ACM-28831

Type of Change:

  • 🐞 Bug Fix
  • ✨ Feature
  • 🔧 Refactor
  • 💸 Tech Debt
  • 🧪 Test-related
  • 📄 Docs

✅ Checklist

General

  • PR title follows the convention (e.g. ACM-12340 Fix bug with...)
  • Code builds and runs locally without errors
  • No console logs, commented-out code, or unnecessary files
  • All commits are meaningful and well-labeled
  • All new display strings are externalized for localization (English only)
  • (Nice to have) JSDoc comments added for new functions and interfaces

If Feature

  • UI/UX reviewed (if applicable)
  • All acceptance criteria met
  • Unit test coverage added or updated
  • Relevant documentation or comments included

If Bugfix

  • Root cause and fix summary are documented in the ticket (for future reference / errata)
  • Fix tested thoroughly and resolves the issue
  • Test(s) added to prevent regression

🗒️ Notes for Reviewers

Signed-off-by: fxiang1 <fxiang@redhat.com>
@fxiang1
Copy link
Copy Markdown
Contributor Author

fxiang1 commented Jan 22, 2026

/retest

jeswanke
jeswanke previously approved these changes Jan 26, 2026
@KevinFCormier
Copy link
Copy Markdown
Contributor

/hold

Copy link
Copy Markdown
Contributor

@KevinFCormier KevinFCormier left a comment

Choose a reason for hiding this comment

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

Hey Feng. The behaviour seems a little strange to me.

  • "Automatically sync when cluster state changes" corresponds to selfHeal, but when it is not checked, would it be equivalent if we set the other 2 options under automated (prune and allowEmpty) to false as well, rather than nulling it out?
  • Should we re-order the checkboxes to show the relationship between this one and the other two that become disabled?
  • When I loaded an application where the "automated" section had been removed for editing, the 2 checkboxes were not disabled.

There was also a stray macOS file checked in that should be removed.

Comment thread .DS_Store Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please remove this file. I see it is gitignored for frontend, but you could add the ignore at the root level instead.

Signed-off-by: fxiang1 <fxiang@redhat.com>
Signed-off-by: fxiang1 <fxiang@redhat.com>
@fxiang1
Copy link
Copy Markdown
Contributor Author

fxiang1 commented Jan 26, 2026

Hey Feng. The behaviour seems a little strange to me.

* "Automatically sync when cluster state changes" corresponds to selfHeal, but when it is not checked, would it be equivalent if we set the other 2 options under automated (prune and allowEmpty) to false as well, rather than nulling it out?

* Should we re-order the checkboxes to show the relationship between this one and the other two that become disabled?

* When I loaded an application where the "automated" section had been removed for editing, the 2 checkboxes were not disabled.

There was also a stray macOS file checked in that should be removed.

Thanks Kevin! The changes for these comments are done.

@KevinFCormier
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Jan 26, 2026
@KevinFCormier
Copy link
Copy Markdown
Contributor

/unhold

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jan 26, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fxiang1, KevinFCormier

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [KevinFCormier,fxiang1]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sonarqubecloud
Copy link
Copy Markdown

@openshift-merge-bot openshift-merge-bot Bot merged commit cdc15fe into stolostron:main Jan 26, 2026
12 checks passed
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.

3 participants