feat: support disabling the built-in updater #219
Conversation
095bf83 to
f34e415
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for disabling the built-in updater via a UserDefaults setting that can be controlled by MDM policies. When disableUpdater is set to true, the updater won't initialize and related UI elements will be hidden.
- Adds
disabledproperty to UpdaterService that checks UserDefaults for updater disable setting - Conditionally initializes the Sparkle updater only when not disabled
- Updates GeneralTab UI to hide updater controls and show explanatory message when disabled
- Refactors some @published properties to private(set) for better encapsulation
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| UpdaterService.swift | Core logic for conditionally disabling updater initialization based on UserDefaults |
| GeneralTab.swift | UI changes to hide updater controls and show disabled message |
| FilePicker.swift | Access control improvements for @published properties |
| VPNService.swift | Access control improvements for @published properties |
| // The auto-updater can be entirely disabled by setting the | ||
| // `disableUpdater` UserDefaults key to `true`. This is designed for use in | ||
| // MDM configurations, where the value can be set to `true` permanently. | ||
| let disabled: Bool = UserDefaults.standard.bool(forKey: Keys.disableUpdater) |
There was a problem hiding this comment.
The UserDefaults.bool(forKey:) method returns false when the key doesn't exist, which means the updater will be enabled by default. However, the comment suggests the intent is to disable the updater when the key is set to true. This logic appears inverted - consider using !UserDefaults.standard.bool(forKey: Keys.disableUpdater) if you want the updater enabled by default.
Copilot uses AI. Check for mistakes.
| Button("Check for updates") { updater.checkForUpdates() }.disabled(!updater.canCheckForUpdates) | ||
| } else { | ||
| Section { | ||
| Text("The app updater has been disabled by a device management policy.") |
There was a problem hiding this comment.
[nitpick] The error message assumes the updater was disabled by device management policy, but the UserDefaults key could be set by other means. Consider making the message more generic like 'The app updater has been disabled.' or 'Automatic updates are not available.'
| Text("The app updater has been disabled by a device management policy.") | |
| Text("The app updater has been disabled.") |
Copilot uses AI. Check for mistakes.
Merge activity
|
Closes #182.
UserDefaults can be forcibly set by MDM admins.
When the
disableUpdaterbool in UserDefaults is set tofalse, the updater won't be initialized on launch, and the UI elements for the updater in settings will be hidden.Related to #220.