Skip to content

Migrate old configs to newest version automatically - #7

Merged
ToothyDev merged 8 commits into
masterfrom
copilot/fix-user-notification-bug
May 12, 2026
Merged

ToothyDev merged 8 commits into
masterfrom
copilot/fix-user-notification-bug

Conversation

Copilot AI commented Mar 31, 2026 •

Copy link
Copy Markdown
Contributor

The 1.2.0→current refactor changed the notify config key from the flat chatguard.notify-user to the nested chatguard.notify.user. Since saveDefaultConfig() never overwrites an existing file, upgraded installs retained the old key. Bukkit's getBoolean() returns false for missing keys, silently suppressing all player notifications. Console logs were unaffected because that code path is unconditional.

Changes

  • ConfigHelper: Added getBoolean(path, defaultValue) overload delegating to Bukkit's FileConfiguration.getBoolean(path, def).
  • ConfigMigrationHelper (new utils class): Contains all config migration logic, keeping PixelChat.java uncluttered and making future migrations easy to add:
    • migrate() — public entry point; reads the stored config version key and skips all work if it already matches the running plugin version. Dispatches to per-version methods for any steps that apply to the detected version range.
    • migrateTo1_3_0() — private method handling the 1.2.0 → 1.3.0 changes: moves chatguard.notify-user (flat) to chatguard.notify.user (nested), preserving an explicit false; adds chatguard.notify.admins: true when absent.
    • isConfigVersionOlderThan() — private semver comparison helper; treats null/empty as older than any target and trailing .0 segments as equal (e.g. "1.3" == "1.3.0").
    • Logs "Configuration migrated to the <version> format." (version read dynamically from the plugin descriptor) when any change is made.
  • PixelChat: Startup migration reduced to a single call: new ConfigMigrationHelper(this).migrate().
  • CHANGELOG.md: Added a 1.3.0 entry noting automatic config migration for upgraded installs.
# Old (1.2.0) config
chatguard:
  notify-user: true

# New (1.3.0) config — written automatically on first startup after upgrade
chatguard:
  notify:
    user: true
    admins: true

This approach correctly handles users who had explicitly set notify-user: false in their 1.2.0 config, skips all migration work on already up-to-date installs, and establishes a clean, scalable pattern for future config restructuring — new migrations only require adding a migrateTo_X_Y_Z() method and a corresponding version guard in migrate().

Copilot AI linked an issue Mar 31, 2026 that may be closed by this pull request
@ToothyDev ToothyDev modified the milestone: 1.3.0 Mar 31, 2026
Copilot AI changed the title [WIP] Fix user notification for blocked messages Fix user notification silently skipped on configs upgraded from 1.2.0 Mar 31, 2026
Copilot AI requested a review from ToothyDev March 31, 2026 23:10
@ToothyDev

Copy link
Copy Markdown
Member

@copilot So does this only apply for upgraded configs from 1.2.0 -> current? If so, do you think this is the best way to tackle this seemingly architectural problem, or should the way this stuff is handled change altogether? Or should there simply be a disclaimer in the changelog of 1.3.0 to make sure the config is changed or made to regenerate, and then changed appropriately? Pick what you think is best here and go ahead.

Copilot AI commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

@copilot So does this only apply for upgraded configs from 1.2.0 -> current? If so, do you think this is the best way to tackle this seemingly architectural problem, or should the way this stuff is ha...

The getBoolean(path, true) approach was a band-aid — it handled the missing key case but would silently override any user who had explicitly set notify-user: false in their 1.2.0 config, and it would need repeating every time a config key changes.

Went with proper config migration instead (commit 45b6e26). On startup, migrateConfig() in PixelChat.java:

  1. Detects the old chatguard.notify-user flat key, reads its actual value (respecting false), writes it to chatguard.notify.user, and removes the old key.
  2. Sets chatguard.notify.admins: true (the new 1.3.0 key) when absent from old configs.
  3. Logs "Configuration migrated to the v1.3.0 format: notification settings updated." so admins know what changed.

The getBoolean(path, true) calls in ChatGuardHelper are reverted back to getBoolean(path) — the migration guarantees both keys are present before any listener reads them. This sets up a clean pattern for future config restructuring.

@ToothyDev ToothyDev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also add this as a changelog entry for 1.3.0 since this is a noteworthy addition

Copilot AI commented Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

Also add this as a changelog entry for 1.3.0 since this is a noteworthy addition

Added in b34664a under the 1.3.0 Changed section:

Automatic config migration on startup: upgraded installs from 1.2.0 will have their configuration keys silently updated to the current format (no manual edits required)

@ToothyDev ToothyDev changed the title Fix user notification silently skipped on configs upgraded from 1.2.0 Migrate old configs to newest version automatically Mar 31, 2026
@Gaming12846 Gaming12846 added the enhancement New feature or request label May 7, 2026
@Gaming12846 Gaming12846 added this to the 1.3.0 milestone May 7, 2026
…on-range each block

Agent-Logs-Url: https://github.com/PixelMindMC/PixelChatGuardian/sessions/137205cc-75f9-4883-930b-50396246491d

Co-authored-by: ToothyDev <55001472+ToothyDev@users.noreply.github.com>
@ToothyDev
ToothyDev marked this pull request as ready for review May 12, 2026 17:36
@ToothyDev

Copy link
Copy Markdown
Member

@copilot Split the entire migration logic into a seperate helper class, and in there, split the different version migrations into their own methods so it remains readable over time, and the main plugin file doesn't get cluttered

Copilot AI commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Split the entire migration logic into a seperate helper class, and in there, split the different version migrations into their own methods so it remains readable over time, and the main plugi...

Done in 183f0fd. All migration logic now lives in utils/ConfigMigrationHelper.java:

  • migrate() — public entry point; reads the stored config version, short-circuits if already up-to-date, and dispatches to version-specific methods
  • migrateTo1_3_0() — private method handling the 1.2.0 → 1.3.0 key changes; returns true if anything was written
  • isConfigVersionOlderThan() — private semver comparison helper

Future migrations just need a new migrateTo_X_Y_Z() method and a corresponding if (isConfigVersionOlderThan(...)) block in migrate(). PixelChat.java is now a single line: new ConfigMigrationHelper(this).migrate();

@ToothyDev
ToothyDev requested a review from Gaming12846 May 12, 2026 17:58
@ToothyDev ToothyDev removed this from the 1.3.0 milestone May 12, 2026
@ToothyDev
ToothyDev merged commit 4dd3b7d into master May 12, 2026
3 checks passed
@ToothyDev
ToothyDev deleted the copilot/fix-user-notification-bug branch May 12, 2026 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] User is not notified of blocks etc.

3 participants