Skip to content

Closes #8702: Story 4 — Fresh-install Pro detection & resolution - #8754

Open
jeawhanlee wants to merge 44 commits into
enhancement/8693-rocketcdn-free-tiers-refactorfrom
task/8702-story-4-fresh-install-pro-detection-resolution
Open

Closes #8702: Story 4 — Fresh-install Pro detection & resolution#8754
jeawhanlee wants to merge 44 commits into
enhancement/8693-rocketcdn-free-tiers-refactorfrom
task/8702-story-4-fresh-install-pro-detection-resolution

Conversation

@jeawhanlee

@jeawhanlee jeawhanlee commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #8702
On fresh install, RocketCDN's Pro/Free status is unknown until the license API responds. This PR adds a background retry loop that resolves it automatically, with a manual retry option if all attempts fail, so users are not left in an indeterminate CDN state.

Type of change

  • New feature (non-breaking change which adds functionality).
  • Bug fix (non-breaking change which fixes an issue).
  • Enhancement (non-breaking change which improves an existing functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as before).
  • Sub-task of #(issue number)
  • Chore
  • Release

Detailed scenario

What was tested

Automated (PHPUnit integration/unit tests):

  • SubscriptionController::auto_detect_pro_subscription() — the synchronous first-install check, schedules the retry job only when the subscription API doesn't return a conclusive result.
  • SubscriptionController::scheduled_auto_detect_pro_subscription() — retries on an inconclusive API response, cancels/clears state on a conclusive one, and sets a failure transient once attempts are exhausted.
  • DataManagerSubscriber::handle_manual_retry_pro_detection(), fired via admin_post_rocket_retry_pro_detection — nonce check, capability check, transient clear, and a synchronous one-shot detection check (no job scheduling).
  • Render\Controller::add_rocketcdn_paid_section() / add_rocketcdn_free_section() — updated to branch on live subscription data.

How to test

  1. On a fresh WP Rocket install, Check that Subscription stat is checked and resolved immediately. After plugin activation, user should see RocketCDN Pro section instead of RocketCDN Free if there is an active subscription.
  2. If API does not resolve correctly with a 200, confirm rocket_cdn_auto_detect is scheduled via Action Scheduler and runs even outside wp-admin (e.g. a plain WP-Cron request).
  3. Let it run to a conclusive result (or force one via pre_http_request) and confirm the RocketCDN paid/free section rendered reflects the resolved plan.
  4. To see the failure path: force the API response to stay inconclusive until attempts are exhausted, confirm the rocket_cdn_pro_detection_failed transient is set and the admin notice appears on the WP Rocket settings page.
  5. Click "Refresh customer data" on that notice (visible only on the WP Rocket settings page), confirm it redirects back, the failure transient is cleared, and the plan is re-resolved synchronously (no background job is scheduled by this action).

Affected Features & Quality Assurance Scope

RocketCDN: fresh-install activation flow, the WP Rocket settings page admin notices, the CDN driver sections (paid/free), and plugin uninstall (added transient cleanup).

Technical description

Documentation

  • SubscriptionController::auto_detect_pro_subscription() runs synchronously on wp_rocket_first_install; if the subscription API doesn't give a conclusive answer (HTTP 200) right away, it schedules the rocket_cdn_auto_detect Action Scheduler job via Queue::schedule_pro_detection_job().
  • Queue::schedule_pro_detection_job() cancels any already-pending detection job before scheduling a new one, avoiding duplicate pending actions across retries/re-triggers; cancel_pro_detection_job() now matches regardless of the attempt arg the job was scheduled with.
  • SubscriptionController::scheduled_auto_detect_pro_subscription( $attempt ) is the Action Scheduler callback: a conclusive result clears the failure transient and cancels any further pending job; an inconclusive one reschedules with $attempt - 1 until it hits 0, at which point rocket_cdn_pro_detection_failed is set.
  • DataManagerSubscriber (which owns these hooks) is now registered as a common subscriber instead of an admin-only one, so its Action Scheduler callbacks are also registered on genuine WP-Cron requests, not just wp-admin/AJAX ones — fixes "no callbacks are registered" failures when Action Scheduler's async loopback isn't available.
  • Render\Controller::add_rocketcdn_paid_section() / add_rocketcdn_free_section() now branch on SubscriptionController::is_paid() (live/cached subscription data) instead of the DB-persisted Context::get_driver() state.
  • NoticesSubscriber::display_pro_detection_failure() shows the failure as an admin notice with a retry link to admin_post_rocket_retry_pro_detection, handled by DataManagerSubscriber::handle_manual_retry_pro_detection() (nonce + capability checked, then clears the failure transient and re-runs the subscription check synchronously — a one-shot check outside the automatic backoff chain, it does not reschedule the Action Scheduler job).
  • Also fixes a broken composer install: mobiledetect/mobiledetectlib 3.74 declares both a PSR-4 and a classmap autoloader; Mozart's classmap mover failed to match the nested classmap path and deleted the vendor copy before the target existed. Added override_autoload in composer.json so Mozart treats the package as PSR-4-only, matching how it's already vendored in inc/Dependencies/Detection/MobileDetect.php.

New dependencies

None.

Risks

Low. The detection job only reads subscription status and never modifies the CDN configuration a user has already set; retries are capped, deduplicated, and end in a visible, dismissible-by-action notice rather than a silent failure.

Mandatory Checklist

Code validation

  • I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I implemented built-in tests to cover the new/changed code.

Code style

  • I wrote a self-explanatory code about what it does.
  • I protected entry points against unexpected inputs.
  • I did not introduce unnecessary complexity.
  • Output messages (errors, notices, logs) are explicit enough for users to understand the issue and are actionnable.

Unticked items justification

N/A — all mandatory items apply and are checked.

Additional Checks

  • In the case of complex code, I wrote comments to explain it.
  • When possible, I prepared ways to observe the implemented system (logs, data, etc.).
  • I added error handling logic when using functions that could throw errors (HTTP/API request, filesystem, etc.)

@jeawhanlee jeawhanlee self-assigned this Aug 21, 2026
@jeawhanlee
jeawhanlee changed the base branch from develop to enhancement/8693-rocketcdn-free-tiers-refactor August 21, 2026 10:52
@codacy-production

codacy-production Bot commented Aug 21, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric Results
Complexity 0

View in Codacy

🟢 Coverage 61.54% diff coverage · +0.06% coverage variation

Metric Results
Coverage variation +0.06% coverage variation (-0.10%)
Diff coverage 61.54% diff coverage (50.00%)

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (91a0742) 47853 22625 47.28%
Head commit (d32de6a) 47929 (+76) 22689 (+64) 47.34% (+0.06%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#8754) 78 48 61.54%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@jeawhanlee jeawhanlee linked an issue Aug 21, 2026 that may be closed by this pull request
@jeawhanlee

Copy link
Copy Markdown
Contributor Author
Screenshot 2026-08-25 at 09 10 33

@jeawhanlee
jeawhanlee requested a review from a team August 25, 2026 08:27
@jeawhanlee
jeawhanlee marked this pull request as ready for review August 25, 2026 08:27
…sk/8702-story-4-fresh-install-pro-detection-resolution
…sk/8702-story-4-fresh-install-pro-detection-resolution

@hellofromahmed hellofromahmed left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving with minor requests.

Notes for @wp-media/qa-team

  1. This currently applies to free and paid subscriptions.
  2. We have two attempts and then fail with the notice that @jeawhanlee mentioned here.
  3. This PR will introduce more calls to user details endpoint because we remove the transient with each attempt.

Comment thread composer.json
Comment thread inc/Engine/CDN/RocketCDN/SubscriptionController.php Outdated
Comment thread inc/Engine/CDN/Render/Controller.php Outdated
Comment thread inc/Engine/CDN/Render/Controller.php Outdated
@hellofromahmed

hellofromahmed commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Note

Generated by the AI delivery pipeline (lead-reviewer · Claude Sonnet 5).

Review: ❌ CHANGES REQUESTED

Blockers:

  • [MEDIUM] Render\Controller::add_rocketcdn_paid_section/add_rocketcdn_free_sectionis_paid() replaces Context::get_driver()'s local short-circuit; on a rocketcdn_status cache-miss this can trigger a blocking wp_remote_get() wherever rocket_cdn_driver_sections fires. Still open from the previous review pass.
  • [MEDIUM] inc/Plugin.php:321 (new) — rocketcdn_data_manager_subscriber moved from admin-only to common, so it's now constructed on every front-end pageview for every visitor. Only the rocket_cdn_auto_detect AS callback needed broader-than-admin registration (to fix the wp-cron gap); the other 9 hooks (admin_init, 5x wp_ajax_*, admin_post_*) never fire outside wp-admin regardless. Also broadens when maybe_refresh_rocketcdn_details() (token save / cache flush) can run — undocumented in the PR's Risks section. Fix: split the 3 new hooks into a small dedicated common subscriber, keep the rest admin-only.
  • [MEDIUM] SubscriptionController::flush_caches_and_get_subscription_data() — still relies implicitly on user_client->get_user_data() succeeding (via the set_transient_wp_rocket_customer_data hook chain) to invalidate the rocketcdn_status transient before each retry. If that unrelated user-data call fails (network blip), rocketcdn_status is never flushed and the retry re-reads the same stale cached "inconclusive" result — silently burning one of only 2 retry attempts without a genuine live check. No test exercises a 2-attempt sequence to catch this. Fix: explicitly delete_transient('rocketcdn_status') (or call $this->options_manager->flush_subscription_cache()) unconditionally in this method.
  • No test coverage for Render\Controller::add_rocketcdn_paid_section()'s guard condition (only the free-section test was updated) — still open.

Nice-to-haves:

  • Queue::schedule_pro_detection_job() TOCTOU race under concurrent first-install requests (self-heals, wasteful only).
  • Queue::cancel_pro_detection_job()'s is_scheduled() pre-check before cancel_all() is redundant.
  • composer.json mozart override_autoload fix is unrelated scope creep — author already justified it inline (Slack link); consider a separate PR next time.
  • add_exclusions_fields() / render_cdn_driver_tabs() in the same Render\Controller still branch on the legacy Context::get_driver() / ROCKETCDN_PAID_TYPE instead of the get_applied_cdn_state() + is_paid() pattern this PR introduces two methods above — worth a consistency pass in a follow-up.
  • handle_manual_retry_pro_detection() checks nonce before capability — minor convention inconsistency, not exploitable.

Verified, no new issue found: retry/backoff arithmetic ($attempt - 1 bottoms out at 1, never reschedules at 0 — matches test fixtures); cancel_pro_detection_job()'s is_scheduled($hook, null) / cancel_all($hook, null) do match regardless of the attempt arg (confirmed against as_has_scheduled_action/as_unschedule_all_actionsnull is deliberate and required here, the default [] would NOT have matched); nonce + capability gate present and correctly wired on admin_post_rocket_retry_pro_detection; DI wiring for the new UserClient constructor arg is correct; no unescaped output, no SQL/authz issues introduced.

Test coverage: PASS with the gaps noted above — integration tests cover auto_detect_pro_subscription(), scheduled_auto_detect_pro_subscription() (reschedule/exhaustion/cancel — single-attempt scenarios only), and handle_manual_retry_pro_detection() (nonce/capability/success paths).

@hellofromahmed

hellofromahmed commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Note

Generated by the AI delivery pipeline (qa-engineer · Claude Sonnet 5). Code-level pass only — Strategy C (test suite + source analysis), no live environment or browser this run.

QA: ⚠️ PARTIAL

Acceptance Criterion Method Result Why it failed
1. Fresh install triggers detection automatically, no user action Analysis + test read (Test_AutoDetectProSubscription)
2. Inconclusive result schedules rocket_cdn_auto_detect, retries with bounded attempts, stops instead of retrying forever Analysis + test read (Test_ScheduledAutoDetectProSubscription)
3. Job callback registers/runs outside wp-admin (WP-Cron) Analysis (inc/Plugin.php: rocketcdn_data_manager_subscriber moved into init_common_subscribers(), which always runs)
4. Manual "Refresh customer data" retry: nonce + capability checked, transient cleared, synchronous re-check, no job re-scheduled, redirects back Analysis + test read (Test_HandleManualRetryProDetection)
5. Failure state shown as an admin notice with a refresh button Analysis (code read only) ⚠️ Backend "stop retrying / set failure transient" is tested, but NoticesSubscriber::display_pro_detection_failure_notice() itself has zero test coverage (its sibling activation_failed_notice() has an integration test; this one doesn't). Actual on-screen rendering can't be confirmed without a browser this run.
6. Paid/Free CDN section rendered reflects the resolved subscription plan Analysis (code read only) ⚠️ Controller::add_rocketcdn_paid_section() branches on get_applied_cdn_state() + is_paid() but has no unit test at all (before or after this PR). add_rocketcdn_free_section()'s test only exercises the is_paid() === false branch, not the "paid subscriber → free section suppressed" branch. Rendering itself not verifiable without a browser.
7. Issue #8702 Task 4.3: fresh-install resolution sets an explicit deactivated/toggle-off state (CDNOptionsManager::set_post_install_state(), decoupled rocketcdn_pro_enabled/rocketcdn_free_enabled fields) Analysis (repo-wide search) No set_post_install_state(), rocketcdn_pro_enabled, or rocketcdn_free_enabled exists anywhere in this branch. The PR may have superseded this with the existing get_applied_cdn_state() approach (and simply never calls enable() during detection, leaving CDN off by default) — but nothing in the diff or tests asserts the toggle state explicitly, so this can't be confirmed as intentionally satisfied vs. a gap.

Blockers:

  • Criterion 5: add an integration test for NoticesSubscriber::display_pro_detection_failure_notice() mirroring the existing activationFailedNotice.php test (transient present/absent, screen/capability gating).
  • Criterion 6: add a unit test for Controller::add_rocketcdn_paid_section() (currently untested), and extend Test_AddRocketcdnFreeSection to cover the is_paid() === true early-return branch.
  • Criterion 7: confirm with the ticket owner whether issue Story 4 — Fresh-install Pro detection & resolution #8702's Task 4.3 "deactivated/toggle-off" persistence is intentionally out of scope for this PR or still pending; if pending, it should be tracked before closing Story 4 — Fresh-install Pro detection & resolution #8702.

jeawhanlee and others added 4 commits August 28, 2026 16:34
Co-authored-by: Ahmed Saeed <146129302+hellofromahmed@users.noreply.github.com>
…n' of github.com:wp-media/wp-rocket into task/8702-story-4-fresh-install-pro-detection-resolution
hellofromahmed added a commit that referenced this pull request Aug 30, 2026
…uine

CRITICAL: APIClient::get_remote_subscription_data()'s error/fallback
path (network failure, non-200, empty body, decode failure) returns a
hardcoded plan_type => 'free' default - indistinguishable from a real
free-tier response by the existing empty($value['plan_type']) check. A
transient RocketCDN API outage could therefore spuriously persist/
activate cdn_state and trigger a full cache clear. Predates the
"activate from nothing" change (it could already spuriously downgrade
an already-paid customer during an outage); that change just widened
the blast radius.

Now requires status_code === 200 before trusting plan_type, matching
the same conclusive-result pattern PR #8754 uses. Also adds a
current_user_can('rocket_manage_options') check as defense-in-depth -
not currently exploitable (every caller of get_subscription_data() is
already capability-gated) but the write path had no boundary of its
own.

Test class switched from AdminTestCase to the RocketCDN TestCase base
(which explicitly sets an admin screen) instead of depending on
ambient screen state from whichever test ran previously, and a new
test proves the is_admin() guard is a genuine no-op on the front end.
Comment thread inc/Plugin.php Outdated
@Mai-Saad

Mai-Saad commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@jeawhanlee @hellofromahmed Thanks for the PR, Please check possible risks raised by AI and confirm we are good or need further change /followup GH 🙏

  1. RocketCDN Paid/Free section rendering now depends on live API state, not a DB flag
    Controller.php:126-133 switched from Context::get_driver() (persisted in the DB) to get_applied_cdn_state() + SubscriptionController::is_paid(). is_paid() reads the wp_rocket_customer_data/subscription cache and falls back to false on a miss or API failure. If the subscription API is briefly unreachable and the cache expires, a paying customer could see the Free CDN section instead of Paid — previously this couldn't happen since it was driven by a DB value, not live API availability.

  2. Every detection attempt flushes and rebuilds a plugin-wide cache, not just RocketCDN's
    flush_caches_and_get_subscription_data() (SubscriptionController.php:407-417) deletes and repopulates the wp_rocket_customer_data transient, which is used across the whole plugin (license data), not scoped to RocketCDN. On a fresh install with an inconclusive result, that's up to 3 cycles (1 sync + 2 async retries) × 2 outbound HTTP calls each, and each repopulation re-fires set_transient_wp_rocket_customer_data → maybe_refresh_rocketcdn_details, which can trigger further side effects. Multiplied across every fresh install in the wild, this is real added load on WP Rocket's own API.

  3. Manual "Refresh customer data" retry is synchronous and blocking
    handle_manual_retry_pro_detection() makes two blocking API calls inline before wp_safe_redirect(). If the license/RocketCDN API is slow, the admin-post request (and the user's click) hangs. Acceptable for an explicit user action, but worth noting as UX risk under API degradation.

  4. Non-atomic duplicate-job guard — cancel_pro_detection_job() then reschedule (Queue.php:211-222) isn't atomic across requests, so two near-simultaneous triggers (e.g. manual retry racing the scheduled job) could theoretically produce two pending actions. Low impact — retries are idempotent.

  5. The retry callback is now a "common" (non-admin) subscriber (FrontendSubscriber.php, confirmed against Plugin.php's admin/common subscriber grouping), which is required so it fires under real WP-Cron. On sites without working async loopback, that means the retry's outbound API calls can execute during a live front-end pageview via WP-Cron, adding latency to that request — a known, generally-accepted tradeoff for WP-Cron-based jobs, but worth being aware of.

@jeawhanlee

Copy link
Copy Markdown
Contributor Author

@DahmaniAdame I believe we'll need the correct text for this new notice:

Screenshot 2026-08-25 at 09 10 33

@jeawhanlee

Copy link
Copy Markdown
Contributor Author

@Mai-Saad I believe we are good here.
We'll wait for copy-write here before merging

…ub.com:wp-media/wp-rocket into enhancement/8693-rocketcdn-free-tiers-refactor
…k/8702-story-4-fresh-install-pro-detection-resolution
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.

Story 4 — Fresh-install Pro detection & resolution

3 participants