Add a reusable activation URL API for PHP and JS#181
Conversation
Product onboarding screens need an Activate button equivalent to the one in Harbor's LicenseProductCard, but the logic for building a portal activation URL lived in two private places: assembled inline inside wp_localize_script() on the Feature Manager page, and in a JS helper shipped only in Harbor's own admin bundle. Neither was reachable from a host plugin. Extract it into an Activation_Url service and expose the JS helper as a shared, leader-gated script handle. Callers pass product, tier and return URL as parameters, so no user-supplied URL reaches the REST layer and the open-redirect surface a pre-baked URL would create never exists. The script handle and window global are deliberately not vendor-prefixed. Strauss rewrites class names but not strings, so every Harbor copy on the site agrees on them, which is what lets a single registration serve all of them. SMTNC-1844 Claude-Session: https://claude.ai/code/session_01BQPHeqkVhHwUUeaem4Rc8M
The default return destination was built as admin.php?page=lw-software-manager, but the Software Manager is registered as a submenu of Settings. WordPress resolves a plugin page by a hook name derived from its parent, so the admin.php form looks up admin_page_lw-software-manager while the page is registered as settings_page_lw-software-manager. The lookup misses and the request ends in wp_die( 'Cannot load lw-software-manager.' ). The effect was that a user who activated a product in the portal was returned to an error page rather than the Software Manager. The rest of the codebase already used the options-general.php form: the Global_Function_Registry accessor, the Feature_Manager_Page docblock, and the JS helper's own test fixture. Only the redirect builder disagreed. Bring it into line and add a regression test. Also document how to pick a correct return URL, since a consumer registering a submenu can hit exactly the same trap, and fix an undefined variable in the guide's enqueue example. SMTNC-1844 Claude-Session: https://claude.ai/code/session_01BQPHeqkVhHwUUeaem4Rc8M
The guide claimed a consumer enqueuing earlier than priority 0 would lose a race against Harbor's registration. That is wrong. WordPress resolves script dependencies in WP_Dependencies::all_deps() when scripts are printed, not when they are enqueued, and admin_enqueue_scripts always runs before admin_print_scripts. Any consumer enqueuing on admin_enqueue_scripts is in time whatever priority it uses. Priority 0 is still worth keeping as a defensive measure for anything that prints scripts by hand, but the documented hazard overstated the risk and would have pushed integrating plugins into unnecessary hook gymnastics. The genuine failure mode is Harbor not being present on the request at all. Describe that instead. SMTNC-1844 Claude-Session: https://claude.ai/code/session_01BQPHeqkVhHwUUeaem4Rc8M
The previous commit claimed admin.php?page=lw-software-manager left the user on a "Cannot load" error page. That is wrong. Verified against a real install: both admin.php and options-general.php return HTTP 200 and render the Software Manager. The earlier analysis stopped at get_plugin_page_hookname() and assumed the parent stayed admin.php. It does not. get_admin_page_parent() scans the registered submenu, matches the plugin page, and returns its real parent, so the hook resolves to settings_page_lw-software-manager and the page loads normally. Keep the options-general.php form, since it is the page's canonical address and matches every other link to it in the codebase, but describe it as the consistency change it is. Rename the test accordingly and drop its assertion that admin.php is absent, which pinned a behaviour that was never broken. SMTNC-1844 Claude-Session: https://claude.ai/code/session_01BQPHeqkVhHwUUeaem4Rc8M
The repository dictionary is US English and the spell check rejected "behaviour". SMTNC-1844 Claude-Session: https://claude.ai/code/session_01BQPHeqkVhHwUUeaem4Rc8M
Licensing data is cached, so a site that has just activated a product in the portal still believes it is unlicensed when the user lands back on it. Anything gated on that data is wrong on arrival: an Activate button that should have disappeared is still there, a feature that should be available is still locked. Harbor's own page already worked around this with a refresh=auto param and a handler bound to its page slug, but a host plugin's return URL got nothing. Generalise it. Activation_Url now tags every return URL it builds, whichever page the caller nominated, and Activation_Return watches for that tag on any admin screen. It refreshes the license products and the catalog, strips the tag, and redirects, all on admin_init so the page renders against current data. Host plugins need no code for this. Sending a user through a URL from Activation_Url is the whole opt-in, which is the point: the alternative was every plugin reimplementing the same handler and any that forgot shipping the stale-state bug. The tag is namespaced rather than called something generic like "refresh", because it rides on a URL owned by the calling plugin and must not collide with their own params. The handler checks manage_options for the same reason: it can land on a screen that does not gate on that capability itself. It also sits behind the usual version leadership check, so four active Liquid Web plugins make one API call between them rather than four. Harbor's page-specific handler and its refresh=auto param are removed rather than left alongside, and its tests move to the new class. SMTNC-1844 Claude-Session: https://claude.ai/code/session_01BQPHeqkVhHwUUeaem4Rc8M
Three things the previous commit broke, all caught by CI: Feature_Manager_Page kept a Catalog_Repository it no longer reads, since the refresh that used it now lives in Activation_Return. PHPStan flagged the write-only property. Drop the dependency rather than leave it dangling; the container autowires the constructor, so only the test needed updating. Activation_Return called the debug trait statically, which phpcs rejects in a final class. Use self:: instead. The new test unset $_SERVER['REQUEST_URI'] in teardown, which left the suite without one and killed the run after the last test. Restore the original value instead. SMTNC-1844 Claude-Session: https://claude.ai/code/session_01BQPHeqkVhHwUUeaem4Rc8M
Removing the Catalog_Repository parameter left the type column padded to the width of a type that is no longer there. SMTNC-1844 Claude-Session: https://claude.ai/code/session_01BQPHeqkVhHwUUeaem4Rc8M
The handler runs on admin_init, so it is reached on every admin page load, and resolving it constructs License_Manager and with it the licensing HTTP client. Almost no admin request is a return trip from the portal, so that construction was wasted on nearly all of them. Check for the tag in the provider first and only resolve when it is there. The handler keeps its own check so it stays correct and testable on its own. SMTNC-1844 Claude-Session: https://claude.ai/code/session_01BQPHeqkVhHwUUeaem4Rc8M
|
@redscar Do we need to test this in a staging/dev environment or are we just merging when the reviewer has approved? I tagged you for a re-review as I added a couple of new commits after battle testing it with the TEC changes. |
That's a great question. I'm not positive how we tested Harbor in the past. Maybe we should reach out to QA to figure out a game plan? |
Consumers were told to resolve Activation_Url from the container, which ties them to whichever Harbor version's class their own copy ships rather than the loaded, highest-version one. Add lw_harbor_get_activation_url() and lw_harbor_get_product_activation_url() -- version-keyed global functions, the stable public API -- wrapping Activation_Url::get_base() and for_product(). Point the activation-URL guide and the API reference at the functions instead of the class. Claude-Session: https://claude.ai/code/session_011uxoDruAaZXzRy3hk8mr4W
redscar
left a comment
There was a problem hiding this comment.
Look's like you have a failing test. Other than that, the code looks good to me.
MD060 (aligned) flagged the two tables added for the activation-URL global functions -- the new rows pushed their column past the header pipe. Re-align both: the README function table and the guide's "From PHP" table. Claude-Session: https://claude.ai/code/session_011uxoDruAaZXzRy3hk8mr4W
|
@redscar test fixed and has passed 👍 |
| /** | ||
| * Refreshes license and catalog data when the portal redirects back with | ||
| * ?refresh=auto (e.g. after a user activates their license). Strips the | ||
| * query param and redirects so a manual reload does not re-trigger the | ||
| * refresh. | ||
| * | ||
| * Hooked on admin_init so headers have not yet been sent, allowing | ||
| * wp_safe_redirect() to issue the Location header successfully. Calling | ||
| * this from render() (the add_submenu_page callback) is too late — WordPress | ||
| * has already begun sending HTML output by that point. | ||
| * | ||
| * @since 1.0.0 | ||
| * | ||
| * @return void | ||
| */ | ||
| public function maybe_redirect_after_refresh(): void { |
There was a problem hiding this comment.
Debatable, but it could be argued we cannot safely remove this method. This was not a final class and it is a public method.
This is an internal library so I'm not going to block the PR over it, but deprecating it would technically be better.
| * @return string | ||
| */ | ||
| private function get_default_redirect_url(): string { | ||
| return admin_url( 'options-general.php?page=' . Feature_Manager_Page::PAGE_SLUG ); |
There was a problem hiding this comment.
It would probably be better to use add_query_arg() for a more simple case like this where space encoding, etc. isn't a concern.
| add_action( 'admin_enqueue_scripts', [ $this, 'register_activation_script' ], 0, 0 ); | ||
| add_action( 'admin_init', [ $this, 'maybe_refresh_after_activation' ], 0, 0 ); |
There was a problem hiding this comment.
Why are we using priority 0? Is it important that we're running this so early? If so, we should add a comment explaining why.
| public function test_get_activation_url_targets_the_subscriptions_screen(): void { | ||
| $url = lw_harbor_get_activation_url(); | ||
|
|
||
| $this->assertStringContainsString( '/subscriptions/', $url ); | ||
| $this->assertStringContainsString( 'portal-referral=plugin', $url ); | ||
| $this->assertStringNotContainsString( 'sku=', $url ); | ||
| } |
There was a problem hiding this comment.
Since you're using http_build_query() with the PHP_QUERY_RFC3986 constant, it would be good to assert that the standards provided by that choice are being followed. Otherwise there's no good reason to not switch to add_query_arg() for them and a dev may change it in the future.
| // The handler redirects and exits on its success path. | ||
| if ( function_exists( 'uopz_allow_exit' ) ) { | ||
| uopz_allow_exit( false ); | ||
| } |
There was a problem hiding this comment.
Nope, this is risky :) Can't do that.
Instead follow a pattern like this:
https://github.com/stellarwp/learndash-core/tree/main/tests#mocking-an-exit-call
| uopz_allow_exit( true ); | ||
| } | ||
|
|
||
| unset( $_GET[ Activation_Url::RETURN_PARAM ] ); |
There was a problem hiding this comment.
$_GET and similar should already be getting cleared between test methods automatically.
Closes SMTNC-1844 · Parent: SMTNC-1833
Summary
Product onboarding screens across our plugins need an "Activate" button equivalent to the one in Harbor's
LicenseProductCard. Today there is no reusable way to build one — the logic lives in two places, neither reachable from a host plugin:wp_localize_script()inFeature_Manager_Page. Not a method, not a service, no filter.buildActivationUrl(), shipped only inside Harbor's own admin bundle.Without this, each plugin would hand-roll the portal's query string, giving us five copies of a contract that drift the moment the portal changes a param.
This PR extracts the logic into an
Activation_Urlservice, exposes it to host plugins through stablelw_harbor_*global functions, and exposes the JS helper as a shared script handle.There was also a latent correctness gap:
redirect_urlwas hardcoded to Harbor's own Software Manager page, so activating from a plugin's onboarding screen would have returned the user to the wrong place. Callers now supply their own destination.What is purely additive
Nothing below changes existing behaviour. All of it is new surface area.
src/Harbor/Portal/Activation_Url.phpget_base( ?string $redirect_url )andfor_product( string $slug, string $tier, ?string $redirect_url ). Autowired withSite\Data.src/Harbor/Portal/Activation_Script.phplw-harbor-activationscript behindVersion::should_handle(). Admin-only.src/Harbor/Portal/Activation_Return.phpresources/js/activation-entry.tswindow.lwHarbor, 525 bytes, zero dependencies.docs/guides/activation-urls.mdtests/wpunit/Portal/Activation_UrlTest.phptests/wpunit/Portal/Activation_ScriptTest.phptests/wpunit/Portal/Activation_ReturnTest.phptests/js/activation-entry.test.tschangelog/…yamlPHP public API —
lw_harbor_*global functionsAdded in response to review: rather than resolving
Activation_Urlfrom the container, host plugins call two stable global functions.lw_harbor_get_activation_url( ?string $redirect_url )Activation_Url::get_base()lw_harbor_get_product_activation_url( string $slug, string $tier, ?string $redirect_url )Activation_Url::for_product()Like the rest of Harbor's
lw_harbor_*surface these are version-keyed — they always resolve to the loaded, highest-version copy, so a consumer never builds a class from its own possibly-stale vendor tree. Both return''when no Harbor instance is active.global-functions.php,Global_Function_Registry.php, andGlobalFunctionsTest.phpgain the shells, registrations, and tests; the guide and API reference now point at these instead of the service.What actually changed
Three files have modified behaviour or signatures. This is the part worth reviewing closely.
1.
Feature_Manager_Page— constructor signature changedThe inline
http_build_query()block that producedharborData.activationUrlis replaced by$this->activation_url->get_base().Same params, same order, same
PHP_QUERY_RFC3986encoding. The container resolves the new dependency by autowiring, so no call-site changes were needed outside tests.One value changed on purpose — see 1a below. The
redirect_urlparam now points atoptions-general.php?page=lw-software-manager&refresh=autoinstead ofadmin.php?page=….1a. Minor: the default redirect now uses the page's canonical URL
redirect_urlchanges fromadmin.php?page=lw-software-manager&refresh=autotooptions-general.php?page=….Both forms resolve to the same page — verified against a real install, each returns HTTP 200 and renders the Software Manager. WordPress resolves an
admin.php?page={slug}URL to a submenu page viaget_admin_page_parent(), which returns the real parent rather thanadmin.php.This is a consistency change, not a fix.
options-general.phpis the page's canonical address and the form used by every other link to it:Global_Function_Registry.php:130, theFeature_Manager_Pagedocblock, and the JS helper's test fixture intests/js/lib/activation-url.test.ts. Only the redirect builder differed.Covered by
test_get_base_uses_the_canonical_page_url.1b. The return trip now refreshes licensing data
Licensing data is cached, so a user who has just activated in the portal lands back on a site that still believes they are unlicensed. Whatever was gated on that state is wrong on arrival — an Activate button that should have gone, a feature that should have unlocked.
Harbor's own page already worked around this with a
refresh=autoparam and a handler bound to its page slug. A host plugin's return URL got nothing, so every plugin would have reimplemented the same handler and any that forgot would ship the stale-state bug.Activation_Urlnow tags every return URL it builds — whichever page the caller nominated — andActivation_Returnwatches for that tag on any admin screen, refreshes, strips it, and redirects. All onadmin_init, so pages render against current data. Host plugins need no code for this.Points worth a reviewer's attention:
lw-harbor-activated), not something generic likerefresh. It rides on a URL owned by the calling plugin and must not collide with their params.manage_optionsfor the same reason: it can land on a screen with no capability check of its own.admin_initfires on every admin page load and resolving the handler builds the licensing HTTP client. The handler repeats the check for its own sake.Feature_Manager_Page's own handler and itsrefresh=autoparam are removed, not left alongside — its tests move toActivation_ReturnTest. That also orphaned itsCatalog_Repositorydependency, which is dropped; the container autowires the constructor, so nothing outside the tests needed changing.2.
Portal\Provider— new hook registeredTwo container registrations, plus:
This is the only new runtime behaviour on existing installs. Every admin page load now runs the leader check. On a non-leading instance it early-returns before touching
wp_register_script. On the leader it registers one dependency-free script — registered, not enqueued, so nothing is served unless a plugin asks for it.Priority
0is defensive rather than required. WordPress resolves script dependencies when scripts are printed, not when they are enqueued, andadmin_enqueue_scriptsalways runs beforeadmin_print_scripts— so any consumer enqueuing on that hook is in time whatever priority it uses. Priority0additionally covers anything that prints scripts by hand.3.
webpack.config.js— second entryAdds an
activationentry withlibrary: { name: 'lwHarbor', type: 'window' }. The existingindexentry is untouched apart from whitespace alignment. Build output gainsbuild/activation.jsandbuild/activation.asset.php.Also modified
tests/wpunit/Admin/Feature_Manager_PageTest.php— two constructor call sites updated for the new parameter. No assertions changed.Explicitly not changed
Worth stating, because an earlier draft of this design did touch them:
License_Responseand its 8 call sites inLicense_ControllerProduct_Collection::to_array()resources/js/lib/activation-url.ts— already acceptedbaseUrlas a parameter, so it works unmodifiedDropping the idea of a server-side pre-baked per-product URL removed all of the above from scope, and with it the open-redirect surface that returning caller-supplied URLs through REST would have created.
Notes for reviewers
The script handle and window global are not vendor-prefixed, on purpose. Strauss rewrites class names but not strings, so
lw-harbor-activationandlwHarborare what every Harbor copy on the site agrees on — that is what allows a single registration to serve all of them. Please don't "fix" this.Consumers must feature-detect. The leader's copy wins, which may be older than the Harbor a given plugin ships:
window.lwHarbor.versionis appended by PHP viawp_add_inline_script( …, 'after' )rather than compiled in, so it reports the version that actually won registration rather than the one that happened to be bundled.Failure mode to be aware of: WordPress silently drops a script whose dependency is still unregistered at print time — no error, no console warning. Because registration happens during
admin_enqueue_scripts, this means Harbor is absent from the request entirely rather than a timing problem. Consumers can check withwp_script_is( Activation_Script::HANDLE, 'registered' ). Documented in the guide.Encoding parity. PHP's
PHP_QUERY_RFC3986and JS'sURLSearchParamsboth emitsku=givewp%3Aelite. There is a test pinning this on each side; they must not drift.QA notes
Regression surface is narrow but specific.
1. Feature Manager page — no visible change expected. The Activate buttons in the license section should behave exactly as before. Activating should still return you to the Software Manager with the product showing as activated.
The
redirect_urlparam now readsoptions-general.php?page=lw-software-manager&refresh=autorather thanadmin.php?page=…. Both resolve to the same page, so this should be invisible — but it is the one value that changed, so worth an eye on the round trip. Confirm the URL still carriesportal-referral=plugin,domain, and a percent-encodedredirect_url.2. The return trip. Activate a product in the portal and come back. The screen you land on must already reflect the activation — no manual refresh, no stale Activate button. The URL should briefly carry
lw-harbor-activated=1and then redirect to your clean URL.Verified in a real install: with the tag, one redirect and the param is stripped; without it, no redirect.
3. Multiple Harbor copies. With two or more Liquid Web plugins active on different Harbor versions, confirm
lw-harbor-activationis registered exactly once and served from the highest version'svendor/path. Checkwindow.lwHarbor.versionmatches that copy.4. Non-admin pages. The script is admin-only and should not appear on the front end.
5. Nothing enqueued by default. On a site with no consumer plugin,
build/activation.jsshould be registered but never printed.Testing
Passing:
main. The 8 failing suites fail identically onmain— missingnode_modulespackages, pre-existing and unrelated.markdownlintandcspellclean;php -lclean on all changed PHP files.window.lwHarbor.buildActivationUrl()works andversionis assignable onto the emitted global.Passing in CI:
build-frontendran on push and committedbuild/activation.jsandbuild-dev/activation.jsautomatically.Local environment note (not a blocker for this PR):
composer installcurrently fails on a clean checkout —lucatume/tdd-helpersandlucatume/wp-utilsboth return "Repository not found" from GitHub. They are transitive dev dependencies of the Codeception tooling and appear to have been deleted or made private upstream. CI is unaffected, but local PHP test runs are blocked until that is sorted. Unrelated to this branch.Integration note
Validated against a real install by injecting the API into The Events Calendar's Strauss-prefixed vendor tree (
TEC\Common\LiquidWeb\Harbor).Activation_Urlresolved from TEC's container and produced correct URLs for all three call shapes.One constraint worth knowing before rolling this out: TEC's Strauss autoloader runs
setClassMapAuthoritative(true), so PSR-4 is bypassed and new Harbor classes do not load until the classmap is regenerated. A normalcomposer update stellarwp/harborhandles it, but dropping files in by hand will not.The
lw_harbor_*global functions added in review are the answer to exactly this: their shells load via Composer'sfilesautoloader rather than the classmap, and they resolveActivation_Urlfrom the leader internally — so a consumer calls a function that is always present and never references the new class, sidestepping the classmap gap entirely.Follow-ups, not in this PR
wp-admin.