Add plugin upgrade dispatcher (upgradeJoinFlow)#89
Open
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_completeruns 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:CK_JOIN_FLOW_VERSION).initagainst a stored db-version option.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_Installclass:Upgrade::check()— hooked toinit. Readsck_join_flow_db_version, compares againstCK_JOIN_FLOW_VERSION, short-circuits when equal. Acquires a 60s transient lock to prevent concurrent dispatcher runs across PHP-FPM workers, then callsupgradeJoinFlow.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$migrationsparameter 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:
First migration:
rekeyMembershipPlanOptionsreadsSettings::get('MEMBERSHIP_PLANS')and pipes it through the existingSettings::saveMembershipPlans(), which writes options under the currentgetMembershipPlanId()slug format.Hook timing
Hooked to
init(priority 10) rather thanplugins_loadedbecause the rekey migration depends on Carbon Fields theme options. Carbon Fields boots onafter_setup_theme, soinitis the first hook whereSettings::getis safe to call.Test coverage
Nine new tests in
UpgradeTest.phpcovering the dispatcher contract:update_optioncalls)0.0.0(fresh install)TDD cycle: red, green, reverse (inverted
version_compare→ 5 of 9 tests fail), restore.Bump-script changes
scripts/bump-version.shnow also rewrites theCK_JOIN_FLOW_VERSIONdefine(...)line injoin.phpand the**Current version:**line in the top-levelreadme.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.phpheader + constant,readme.txtstable tag + changelog,readme.md,index.tsx).Test plan
composer install && ./vendor/bin/phpunit --filter UpgradeTest --testdoxinpackages/join-block— all nine new tests passSessionLockTestfails (unrelated)wp option get ck_join_flow_db_versionreturns nothing initiallywp option get ck_join_flow_db_versionnow returns1.4.9ck_join_flow_membership_plan_*options now exist alongside the old ones/wp-json/join/v1/stripe/create-subscriptionwith the new-formatplanIdand confirm a Stripe subscription is createdck_join_flow_db_versionand no plans configured, confirminitdoesn't error and the version is stamped to currentbash scripts/bump-version.sh 1.5.0and confirm all six version-touching locations are updated:join.phpplugin-headerVersion:linejoin.phpCK_JOIN_FLOW_VERSIONdefine(...)linereadme.txtStable tag:linereadme.txtchangelog entryreadme.md**Current version:**lineindex.tsxSentryrelease:line