Skip to content

Conversation

@BornToBeRoot
Copy link
Owner

Changes proposed in this pull request

  • Backup retention for settings config

Related issue(s)

To-Do

Contributing

By submitting this pull request, I confirm the following:

@BornToBeRoot BornToBeRoot changed the title Feature: Configure backup retention Feature: Automatic Profile & Settings backup Jan 12, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds configurable backup retention for both application settings and profile files. Users can now enable/disable daily automatic backups and configure the maximum number of backups to retain (1-365, default 10). The PR implements a significant refactoring of the ProfileManager to support per-profile-file backup tracking through a new ProfileFileData wrapper class.

Changes:

  • Added UI controls (toggle switch and numeric input) to configure backup settings for both Settings and Profiles in the application's settings views
  • Refactored ProfileManager from using a static Groups list to a ProfileFileData wrapper that includes metadata (version, last backup date) alongside the groups
  • Updated all ViewModel references from ProfileManager.Groups to ProfileManager.LoadedProfileFileData.Groups
  • Enhanced backup logic in both SettingsManager and ProfileManager to check if backups are enabled before creating them
  • Updated documentation to describe the new configuration options

Reviewed changes

Copilot reviewed 119 out of 121 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
Website/docs/settings/settings.md Documents new backup configuration options for settings
Website/docs/settings/profiles.md Documents new backup configuration options for profiles
Website/docs/changelog/next-release.md Updates changelog to reference this PR
Source/NETworkManager/Views/SettingsSettingsView.xaml Adds UI controls for backup configuration
Source/NETworkManager/Views/SettingsProfilesView.xaml Adds UI controls for backup configuration
Source/NETworkManager/ViewModels/SettingsSettingsViewModel.cs Implements backup settings properties with loading guard
Source/NETworkManager/ViewModels/SettingsProfilesViewModel.cs Implements backup settings properties with loading guard
Source/NETworkManager/ViewModels/*HostViewModel.cs (multiple) Updates references from ProfileManager.Groups to LoadedProfileFileData.Groups
Source/NETworkManager/ViewModels/ProfilesViewModel.cs Updates profile group access to use new data structure
Source/NETworkManager.Settings/SettingsInfo.cs Adds properties for backup enable/disable and max count
Source/NETworkManager.Settings/GlobalStaticConfiguration.cs Moves backup defaults to profile/settings-specific constants
Source/NETworkManager.Settings/SettingsManager.cs Adds backup enable check and configurable retention
Source/NETworkManager.Profiles/ProfileManager.cs Major refactoring to use ProfileFileData wrapper with backup tracking
Source/NETworkManager.Profiles/ProfileFileData.cs New class wrapping groups with version and backup metadata
Source/NETworkManager.Profiles/ProfileInfoSerializable.cs Adds documentation comments
Source/NETworkManager.Profiles/GroupInfoSerializable.cs Minor whitespace cleanup
Source/NETworkManager.Profiles/GroupInfo.cs Whitespace cleanup
Source/NETworkManager.Utilities/TimestampHelper.cs Modernizes string slicing syntax
Source/NETworkManager/App.xaml.cs Updates save logic to handle new ProfileFileData structure
Source/NETworkManager.Localization/Resources/Strings.resx Adds localization strings for new UI elements
Source/NETworkManager.Localization/Resources/Strings.Designer.cs Auto-generated localization code
Multiple ViewModels (60+ files) Removes BOM character from file start
Files not reviewed (1)
  • Source/NETworkManager.Localization/Resources/Strings.Designer.cs: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 321 to 331
var backupFiles = Directory.GetFiles(backupFolderPath)
.Where(f => (f.EndsWith(settingsFileName) || f.EndsWith(GetLegacySettingsFileName())) && TimestampHelper.IsTimestampedFilename(Path.GetFileName(f)))
.Where(f =>
{
var fileName = Path.GetFileName(f);

// Check if it's a timestamped backup and contains the settings name
return TimestampHelper.IsTimestampedFilename(fileName) &&
fileName.Contains($"_{settingsNameWithoutExtension}.");
})
.OrderByDescending(f => TimestampHelper.ExtractTimestampFromFilename(Path.GetFileName(f)))
.ToList();
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

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

The CleanupBackups method in SettingsManager does not check if the backup directory exists before attempting to get files from it. This could result in a DirectoryNotFoundException if the directory doesn't exist. The ProfileManager implementation at line 1280-1284 includes this check and logs an error if the directory is missing. Consider adding the same check here for consistency and robustness.

Copilot uses AI. Check for mistakes.
<TextBlock Text="{x:Static localization:Strings.MaximumNumberOfBackups}"
Style="{StaticResource CenterTextBlock}"
Margin="0,0,0,10" />
<StackPanel Orientation="Horizontal" Margin="0,0,0,20">
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

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

This line has trailing whitespace after "Horizontal" and before the closing angle bracket. Consider removing it for consistency.

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

Comment on lines 34 to 55
<Button Content="{x:Static localization:Strings.Reset}" Command="{Binding ResetSettingsCommand}"
IsEnabled="{Binding SettingsExists}" Style="{StaticResource DefaultButton}" HorizontalAlignment="Left" />
Style="{StaticResource DefaultButton}" HorizontalAlignment="Left"
/>
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

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

This closing tag has trailing whitespace before the closing angle bracket and is split across two lines unnecessarily. Consider placing it on a single line for consistency with the rest of the codebase.

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

BornToBeRoot and others added 2 commits February 1, 2026 02:03
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI commented Feb 1, 2026

@BornToBeRoot I've opened a new pull request, #3307, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link
Contributor

Copilot AI commented Feb 1, 2026

@BornToBeRoot I've opened a new pull request, #3308, to work on those changes. Once the pull request is ready, I'll request review from you.

BornToBeRoot and others added 3 commits February 1, 2026 02:15
…3308)

* Initial plan

* Fix: Consolidate Button closing tag to single line

Co-authored-by: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com>
* Initial plan

* Remove trailing whitespace from SettingsSettingsView.xaml line 40

Co-authored-by: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com>
@mergify mergify bot merged commit 561deb8 into main Feb 1, 2026
4 checks passed
@mergify mergify bot deleted the feature/configure_backup_retention branch February 1, 2026 02:05
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