Skip to content

Add plugin upgrade dispatcher (upgradeJoinFlow)#89

Open
conatus wants to merge 4 commits intomasterfrom
feature/upgrade-hook
Open

Add plugin upgrade dispatcher (upgradeJoinFlow)#89
conatus wants to merge 4 commits intomasterfrom
feature/upgrade-hook

Conversation

@conatus
Copy link
Copy Markdown
Member

@conatus conatus commented May 1, 2026

Summary

Introduces a structural fix for the membership-plan slug bug shipped by PR #88. Instead of relying on the runtime fallback in getMembershipPlan() to scan options on every cache miss, this PR adds a proper plugin-upgrade dispatcher that re-keys the options table once after each version bump.

The fallback stays in place as a safety net (multisite sub-sites, transient lock wedges, future slug-format changes between upgrade and lookup) — but on the hot path of normal lookups it now becomes cold.

Why this exists

Per the v1.4.4 incident: WordPress has no native hook that fires post-upgrade with new code loaded. upgrader_process_complete runs the old plugin code, so it can't run new migrations. The canonical pattern across WooCommerce, EDD, and the WordPress developer docs is version-comparison on every request:

  1. Define a version constant in code (CK_JOIN_FLOW_VERSION).
  2. Compare it on init against a stored db-version option.
  3. When older, dispatch any pending per-version migration callbacks.
  4. Stamp the new version after every migration succeeds.

This PR implements that pattern with a clearly-named entry point (Upgrade::upgradeJoinFlow) so future migrations have an obvious home.

Architecture

Modelled on WooCommerce's WC_Install class:

  • Upgrade::check() — hooked to init. Reads ck_join_flow_db_version, compares against CK_JOIN_FLOW_VERSION, short-circuits when equal. Acquires a 60s transient lock to prevent concurrent dispatcher runs across PHP-FPM workers, then calls upgradeJoinFlow.

  • Upgrade::upgradeJoinFlow($from, $migrations = null) — the public entry point. Filters the migration map to versions strictly greater than $from, sorts ascending, runs each callback. On any exception, bails without stamping the version so the next request retries. The $migrations parameter is purely for unit-test injection; production callers omit it.

  • Migration map — declared as a private static array. Adding a new migration is a one-liner:

    private static array $migrations = [
        '1.4.9' => ['rekeyMembershipPlanOptions'],
        '1.5.0' => ['renameLegacyTagOption'],  // future
    ];
  • First migration: rekeyMembershipPlanOptions reads Settings::get('MEMBERSHIP_PLANS') and pipes it through the existing Settings::saveMembershipPlans(), which writes options under the current getMembershipPlanId() slug format.

Hook timing

Hooked to init (priority 10) rather than plugins_loaded because the rekey migration depends on Carbon Fields theme options. Carbon Fields boots on after_setup_theme, so init is the first hook where Settings::get is safe to call.

Test coverage

Nine new tests in UpgradeTest.php covering the dispatcher contract:

  • check() short-circuits when versions match (hot-path assertion: zero update_option calls)
  • check() dispatches when stored < current
  • check() treats missing stored version as 0.0.0 (fresh install)
  • check() is a no-op when the upgrade lock is held
  • upgradeJoinFlow skips migrations <= fromVersion
  • upgradeJoinFlow runs migrations in ascending version order regardless of declaration order
  • upgradeJoinFlow does NOT bump the version when a migration throws (and the exception propagates)
  • upgradeJoinFlow stamps the version exactly once after every migration succeeds
  • The production migration map declares the 1.4.9 rekey entry (sanity check against accidental deletion)

TDD cycle: red, green, reverse (inverted version_compare → 5 of 9 tests fail), restore.

Bump-script changes

scripts/bump-version.sh now also rewrites the CK_JOIN_FLOW_VERSION define(...) line in join.php and the **Current version:** line in the top-level readme.md. The latter was bumped by hand during the 1.4.9 release because the script didn't know about it — folding it in here prevents future drift across version-touching files (now six locations: join.php header + constant, readme.txt stable tag + changelog, readme.md, index.tsx).

Test plan

  • Run composer install && ./vendor/bin/phpunit --filter UpgradeTest --testdox in packages/join-block — all nine new tests pass
  • Run the full test suite — 78 tests, only pre-existing SessionLockTest fails (unrelated)
  • On staging with plans saved under the legacy slug:
    • Confirm wp option get ck_join_flow_db_version returns nothing initially
    • Hit any front-end page once
    • Confirm wp option get ck_join_flow_db_version now returns 1.4.9
    • Confirm new-format ck_join_flow_membership_plan_* options now exist alongside the old ones
    • Hit /wp-json/join/v1/stripe/create-subscription with the new-format planId and confirm a Stripe subscription is created
  • Fresh-install test: on a site with no ck_join_flow_db_version and no plans configured, confirm init doesn't error and the version is stamped to current
  • Bump-script test: run bash scripts/bump-version.sh 1.5.0 and confirm all six version-touching locations are updated:
    • join.php plugin-header Version: line
    • join.php CK_JOIN_FLOW_VERSION define(...) line
    • readme.txt Stable tag: line
    • readme.txt changelog entry
    • readme.md **Current version:** line
    • index.tsx Sentry release: line
  • After deploy: inspect logs for "Running join-flow upgrade from X.Y.Z to A.B.C" appearing once, then absent on subsequent requests

conatus added 4 commits May 1, 2026 16:59
Introduces the Upgrade class as the canonical home for one-shot data
migrations triggered by version bumps. Modelled on WooCommerce's
WC_Install pattern: a static map of plugin-version => migration
callbacks, dispatched by Upgrade::check() on `init` when the stored
ck_join_flow_db_version is older than CK_JOIN_FLOW_VERSION.

There is no native WordPress hook that fires post-upgrade with new
code loaded (upgrader_process_complete runs the old code), so the
canonical pattern is version-comparison on every request. check()
short-circuits on the hot path when versions match.

The first migration registered is rekeyMembershipPlanOptions for v1.4.9
which proactively re-keys legacy-format ck_join_flow_membership_plan_*
options under the new label_frequency_currency slug, eliminating the
need for the runtime fallback shipped in PR #88 to fire on hot lookups.

upgradeJoinFlow accepts an injectable migrations map purely so unit
tests supply test doubles without mutating static state. Production
callers omit the argument.
Defines the version constant alongside the plugin-header version so
PHP code can reference it (the plugin header is parseable only via
get_file_data, which is wasteful to do per-request).

Hooks Upgrade::check to `init` rather than `plugins_loaded` because
the rekey migration calls Settings::get('MEMBERSHIP_PLANS'), which
depends on Carbon Fields theme options — Carbon Fields boots on
after_setup_theme, so init is the first hook where settings are safe
to read.
The script previously touched join.php's header, readme.txt, and
index.tsx but missed the new CK_JOIN_FLOW_VERSION constant and the
top-level readme.md's **Current version:** line. Both now bump
together so the script remains the single source of truth for
version-touching files.

readme.md was bumped by hand during the 1.4.9 release because the
script didn't know about it; folding it in here prevents future drift.
WordPress's WP_Hook calls registered callbacks with accepted_args=1 by
default, even when the action itself fires with no args. For init,
this means the callback receives an empty string positional arg.
Upgrade::check declares ?array $migrations as its first parameter
(for test injection), so PHP's strict type check rejected the empty
string and threw on every WP-CLI invocation, breaking the e2e suite.

Match the existing init handler's pattern by registering a no-arg
closure that calls check() without forwarding WP's spurious arg.
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.

1 participant