Skip to content

fix: make admin settings downgrade-safe with legacy write-through mirror - #3308

Open
mrabbani wants to merge 3 commits into
refactor/simplify-settings-to-flat-arrayfrom
fix/settings-downgrade-safe-legacy-mirror
Open

fix: make admin settings downgrade-safe with legacy write-through mirror#3308
mrabbani wants to merge 3 commits into
refactor/simplify-settings-to-flat-arrayfrom
fix/settings-downgrade-safe-legacy-mirror

Conversation

@mrabbani

@mrabbani mrabbani commented Jul 7, 2026

Copy link
Copy Markdown
Member

All Submissions:

  • My code follow the WordPress' coding standards
  • My code satisfies feature requirements
  • My code is tested
  • My code passes the PHPCS tests
  • My code has proper inline documentation

Changes proposed in this Pull Request:

The flat-array settings refactor made dokan_admin_settings the single physical source of truth: new-UI saves never reached the legacy dokan_* rows, legacy-path saves actively stripped mapped keys out of those rows, and nothing detected edits made while the bridge was absent. That breaks the downgrade → edit → re-upgrade round trip both ways:

  1. Downgrade: an old plugin version (no bridge, no overlay filters) reads raw legacy rows → sees stale values, or nothing at all for stripped keys.
  2. Re-upgrade: the stale dokan_admin_settings snapshot unconditionally shadows the fresher legacy rows → the user's downgrade-period edits silently vanish.

This PR keeps the flat option canonical while making the legacy rows a physical, downgrade-safe mirror:

  • LegacyMirror (new Hookable, includes/Admin/Settings/Migration/LegacyMirror.php)
    • Write-through: listens on dokan_admin_settings_changed and fans changed values out to the legacy rows via LegacySettingsBridge::write_new_to_legacy(), stamping a baseline snapshot (dokan_admin_settings_legacy_snapshot).
    • Reconciliation: on admin_init, compares the raw rows against the baseline; keys changed while the bridge was not watching (i.e. by an older plugin version) are adopted back into dokan_admin_settingslast write wins. Keys absent from the baseline are never adopted, protecting newly-mapped schema fields. First run materializes the full mirror, healing historically-stripped rows.
  • Stripping is gated, not removed — single line restores the old strict single-source model (strip on, mirror + reconcile off):
    add_filter( 'dokan_admin_settings_legacy_mirror', '__return_false' );
  • BridgeBootstrap::without_overlay() (new): mirror/reconcile row I/O runs with the overlay suppressed — otherwise update_option()'s internal old-value read is projected from the flat option, a stale row looks current, and the physical write silently no-ops.

How to test the changes in this Pull Request:

  1. On this branch, save a setting in the new admin settings UI (e.g. Vendor Store URL).
  2. Inspect the raw row: wp db query "SELECT option_value FROM wp_options WHERE option_name='dokan_general'" → the mapped key now physically holds the saved value (previously stale/stripped).
  3. Simulate a downgrade edit: wp option patch update dokan_general custom_store_url old-version-edit (or downgrade to a pre-bridge Dokan build and save settings there).
  4. Load any wp-admin page on this branch → dokan_admin_settings adopts the edited value; both the new UI and dokan_get_option() return old-version-edit.
  5. vendor/bin/phpunit -c phpunit.xml --group admin-settings → 174 tests pass, including the new LegacyMirrorTest round-trip coverage.

Changelog entry

fix: Admin settings survive plugin downgrade/re-upgrade round trips

Previously, settings saved in the new admin settings UI were invisible to older plugin versions (legacy option rows were stale or stripped), and settings saved while running an older version were silently discarded on re-upgrade. Legacy rows are now kept as a write-through mirror of the canonical flat option, and edits made on an older version are reconciled back (last write wins) when returning to the new version.

Saving via the new flat dokan_admin_settings option previously left the
legacy dokan_* rows stale (and legacy-path saves stripped mapped keys from
them), so downgrading to a plugin version without the settings bridge lost
the user's edits — and edits made on that old version were silently
shadowed by the stale flat option after re-upgrading.

- Add LegacyMirror (Hookable): mirrors every dokan_admin_settings_changed
  slice into the legacy rows, and reconciles on admin_init by comparing the
  raw rows against a baseline snapshot (dokan_admin_settings_legacy_snapshot),
  adopting values an older plugin version changed — last write wins.
- Gate mapped-key stripping behind LegacySettingsBridge::is_legacy_mirror_enabled();
  single-line opt-out: add_filter( 'dokan_admin_settings_legacy_mirror', '__return_false' ).
- Add BridgeBootstrap::without_overlay() so physical row reads/writes bypass
  the overlay projection (update_option would otherwise silently no-op).
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cad2f6f8-f963-46bc-a9f9-47720b0a1f55

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/settings-downgrade-safe-legacy-mirror

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@mrabbani

mrabbani commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

UAT — Downgrade-safe admin settings (Legacy Mirror)

Environment prep: a site running this branch (with WooCommerce), WP-CLI access. "Old version" below means any released Dokan build without the settings bridge (e.g. the current production release zip).

Verification helper — read the PHYSICAL legacy row (bypasses the overlay filter, i.e. exactly what an old plugin version would see):

wp db query "SELECT option_value FROM $(wp db prefix)options WHERE option_name='dokan_general'" --skip-column-names

Scenario 1 — New-UI save is visible to old plugin versions (write-through)

  1. On this branch, open Dokan → Settings (new UI) and change a mapped field, e.g. Vendor Store URL to store-a. Save.
  2. Run the verification helper.

Expected: the raw dokan_general row physically contains custom_store_url = store-a (not just dokan_admin_settings). Before this PR the row stayed stale or lost the key entirely.

Scenario 2 — Downgrade: old version reads current data

  1. After Scenario 1, replace the plugin with the old version (deactivate → swap → activate, keep the database).
  2. Open Dokan → Settings in the old version's UI.

Expected: Vendor Store URL shows store-a — the value saved through the new UI. No settings appear empty/reset for mapped fields.

Scenario 3 — Edit and save on the old version

  1. Still on the old version, change Vendor Store URL to store-b and save.
  2. Confirm via helper: raw row now holds store-b; dokan_admin_settings still holds the stale store-a:
    wp option get dokan_admin_settings --format=json | grep -o '"vendor_store_url_slug":"[^"]*"'

Scenario 4 — Re-upgrade: old-version edit wins (reconciliation)

  1. Swap back to this branch's build and activate.
  2. Load any wp-admin page once (reconciliation runs on admin_init).
  3. Check both storages and the UI.

Expected:

  • dokan_admin_settings.vendor_store_url_slug = store-b (adopted — last write wins).
  • New settings UI shows store-b.
  • wp eval 'echo dokan_get_option("custom_store_url", "dokan_general");' prints store-b.
  • Before this PR, the stale store-a silently overwrote the user's store-b edit.

Scenario 5 — Regressions (normal operation, no downgrade involved)

  1. Save several fields across pages in the new UI (switches, numbers, multichecks, withdraw methods) → values persist after reload.
  2. Save via a legacy path (e.g. run the Setup Wizard) → values appear in the new UI and in dokan_admin_settings.
  3. Frontend behavior driven by settings (store URL base, catalog mode, withdraw limits) reflects saved values.
  4. vendor/bin/phpunit -c phpunit.xml --group admin-settings174 tests, 0 failures (includes the new LegacyMirrorTest round-trip coverage).

Scenario 6 — Strict-mode opt-out (single-line switch)

  1. Add to a mu-plugin: add_filter( 'dokan_admin_settings_legacy_mirror', '__return_false' );
  2. Save a mapped field in the new UI, then run the verification helper.

Expected: raw legacy row is NOT updated (pre-PR strict single-source behavior restored); the new UI and dokan_get_option() still return the saved value via the read-time overlay. Remove the filter afterwards.


Pass criteria: Scenarios 1–4 demonstrate no data loss in either direction of the downgrade/upgrade round trip; Scenario 5 shows no behavior change during normal operation; Scenario 6 confirms the one-line rollback switch.

mrabbani and others added 2 commits July 7, 2026 19:43
… expectation

test_round_trip_preserves_legacy_shape asserted the input key order
(fixed, percentage) with assertSame, but to_legacy()'s output order is
contractually (percentage, fixed) — locked by its sibling test — so the
test could never pass. It shipped in #3202 without a @group tag, so
group-filtered runs never executed it. Align the expectation with the
locked output contract.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants