Skip to content

Fix - Recover paid Stripe renewals in the missed-payment backfill, keep active members while Stripe retries, and keep the backfill window after a Stripe error (themegrill/user-registration-pro#1888) - #1547

Merged
asheshmagar merged 10 commits into
developfrom
fix/1888-stripe-backfill-and-failed-invoice-status
Oct 8, 2026
Merged

asheshmagar merged 10 commits into
developfrom
fix/1888-stripe-backfill-and-failed-invoice-status

Conversation

@bhattaganesh

@bhattaganesh bhattaganesh commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

All Submissions:

Builds on #1515 (merged): it reuses that PR's invoice PaymentIntent lookup, and updates its regression test for the lookup now throwing to its caller.

I merged develop into this branch after #1515 was squash-merged. The only conflicts were #1515's own helper in StripeService.php and its regression test and spec, where this branch carries the updated version (the lookup throws to its caller); I kept this branch's side in each.

Changes proposed in this Pull Request:

Closes themegrill/user-registration-pro#1888

Stripe moved subscription and payment_intent off the invoice in API version 2025-03-31.basil. The Stripe test account I used sends events in 2026-04-22.dahlia, and its invoice events have neither field. This PR fixes the two handlers the issue reports and two more problems in the same flow that I confirmed live.

1. The missed-payment backfill never recovered a paid renewal. run_missed_payment_backfill() read $invoice->subscription and $invoice->payment_intent and skipped the invoice when either was empty, which on current API versions is always. It now:

get_invoice_payment_intent_id() now throws to its caller instead of calling wp_die() itself. The webhook still logs INVOICE_PAYMENT_LOOKUP_FAILED and aborts with an error status, exactly as in #1515.

2. Every invoice.payment_failed reset the member to pending. Stripe sends this event on each retry, so a paying member lost access at the first declined attempt, and any status an admin set was overwritten at the next retry. handle_failed_invoice() now leaves an active subscription alone while the invoice still has next_payment_attempt, in line with the past_due expiry hold from #1445. When Stripe has no retry left, or the member is trial or pending, it still sets pending.

3. One failed backfill run lost its window for good (found while checking the remaining items on the issue). The cron moved urm_last_missed_payment_events_check_sync_time forward even when the Stripe backfill threw, so a single Stripe API error dropped every paid renewal in that window. The Stripe order backfills (payments and one-time) now keep their own sync time, urm_last_stripe_payment_backfill_sync_time, which only moves forward after a fully successful run, as PayPal's already does. A failed invoice lookup or a failed order write also keeps the window. The status, refund and dispute backfills stay on the shared time, because replaying their events re-applies old snapshots (see the note below).

4. Webhooks from an endpoint on a current API version were ignored. handle_webhook() read only the top-level subscription, so an invoice.* event from an endpoint on 2025-03-31.basil or newer stopped at NULL_SUBSCRIPTION_ID. It now also reads parent.subscription_details.subscription. The plugin's own endpoint is pinned to 2023-10-16 and was never affected, so this only matters for an endpoint made by hand.

Bugs reproduced before the fix

I reproduced everything on masteriiyo.local with a real Stripe test account. The member signed up through the real registration form with card 4242 4242 4242 4242, on a £29/month membership (a test copy of an existing plan; its monthly price was created through the plugin's own sync_product_and_price_in_stripe()). Renewals were real scheduled renewals (billing_reason = subscription_cycle), driven by a Stripe test clock attached to the member's customer.

Webhooks were delivered by Stripe itself to the site's real endpoint /wp-json/user-registration/stripe-webhook through a temporary Cloudflare tunnel. I created a temporary test-mode endpoint exactly as create_webhook() does (pinned to 2023-10-16, the plugin's own event list) and stored its secret where the plugin looks for it. Every delivery passed the plugin's signature check. The Stripe CLI couldn't be used: it forwards events in the account's default API version, not in the endpoint's pinned one (docs).

Bug 1, base code (develop + #1515): I disabled the endpoint and advanced the clock one month, so Stripe charged the renewal (invoice paid, £29) and the site received nothing. Running the real cron hook urm_missed_payment_events_check logged "orders_created": 0, "skipped": 2, and Payments still showed only the signup payment.

Bug 1 before: Payments shows only the signup payment

Bug 2, base code: I attached Stripe's "attaches but declines" card (pm_card_chargeCustomerFail) as the subscription's default, enabled the endpoint and advanced the clock one more month. Stripe declined the renewal and scheduled a retry for two days later (subscription past_due). It delivered invoice.payment_failed, and the site logged "Invoice payment failed handled". The member went from Active to Pending after one attempt Stripe was still going to retry.

Bug 2 before: member is Pending after one retried failure

Bug 3, with bugs 1 and 2 fixed but before this change: I reset the backfill window to before that paid renewal. I made one cron run hit a Stripe API error by swapping the test key for an invalid one, then restored it. The failed run still moved the sync time forward (04:49 to 04:55). The next good run started after the renewal and never recovered it.

Bug 4, before this change: I registered a second member and created an endpoint the way a person would in the Dashboard (no api_version, so the account default), with its secret in the legacy option. The plugin's endpoint was disabled. When the clock renewed the subscription, Stripe delivered invoice.payment_succeeded, the signature passed, and the site logged Subscription ID is null in webhook (NULL_SUBSCRIPTION_ID): no renewal order.

Verification I ran (final code, same setup)

  • Bug 1 fixed. The same cron run recovered the missed renewal as a completed order with its PaymentIntent, and the duplicate event for that invoice was skipped. Running it again created nothing.

    Bug 1 after: the scheduled renewal is listed

  • Bug 2 fixed, through every retry. I advanced the clock to each retry Stripe scheduled. Attempts 2 to 8 each failed and were delivered, the site logged "Invoice payment failed, Stripe will retry; subscription status kept" each time, and the member stayed Active. After attempt 9, Stripe stopped retrying (next_payment_attempt null) and cancelled the subscription per the account's retry settings. That delivery set the member to Pending.

    Bug 2 after: member stays Active while Stripe retries

  • Bug 3 fixed. Same sequence: the failing run logged "the Stripe payment sync time is kept" and stored the window start. The next good run recovered the renewal, and a third run added nothing.

  • Bug 4 fixed. The next renewal through the hand-made endpoint created the renewal order ("Subscription renewed successfully"). A declined renewal through it (retry scheduled) kept the member Active.

  • Link instead of duplicate. Against the live database, get_unlinked_order_by_subscription() returned the order when one Stripe order was blank. It returned nothing when two were blank, or when the blank one was a PayPal order. End to end, blanking a member's signup order and running the cron logged "orders_linked": 1, "orders_created": 0.

  • Unit regressions. These run the real method bodies with Stripe and repository doubles.

  • PHPCS: no new findings. Each changed file has the same count as on the base branch: StripeService.php 37, SubscriptionService.php 119, OrdersRepository.php 61. The new test file is clean.

  • Reviews: two /code-review passes and two security reviews (none found anything). All 5 Copilot threads are answered: 3 fixed, 1 already fixed, 1 explained.

Remaining items from the issue, checked

  • Item 3 (a paid invoice paid by a plain charge). Not reachable. Stripe shows a payment as charge only when the charge has no PaymentIntent, and every invoice finalized since March 2019 gets one. I also marked a live renewal invoice paid out of band. Stripe still created a PaymentIntent (left canceled), recorded the paid payment as payment_record, and sent only invoice.paid, which the plugin's endpoint doesn't subscribe to. So a manual "paid out of band" in Stripe records nothing on the site. That's a product question, not a regression, and this PR doesn't change it.
  • The webhook's "existing order" fallback only checks the subscription's latest order. Not reproducible through the normal flow: all four registrations in this session saved their PaymentIntent on the order, so the fallback never ran. The backfill now uses the stricter lookup above.
  • handle_succeeded_invoice() looks up the PaymentIntent after stop_replaced_subscription(). Not a bug, from reading the code: that call cancels the old subscription once, then deletes the user meta that triggers it, so a redelivered event skips it.

Found along the way, not changed here

The Stripe status backfill (run_missed_subscription_backfill(), untouched here) replays customer.subscription.updated events newest first. So the oldest snapshot in the window is the last write. In one live run it set a member's row to past_due (not a value the status column allows) and then back to an older active. It needs its own issue.

How to test the changes in this Pull Request:

You need Stripe test mode, a monthly Stripe subscription membership on a registration form, and a way for Stripe to reach the site (a public site, or a tunnel such as cloudflared tunnel --url https://<site> --http-host-header <site>). Make sure the plugin's webhook endpoint (Stripe settings, Save) points at that URL.

  1. Register a member with card 4242 4242 4242 4242.
  2. Attach a test clock to the member's Stripe customer: POST /v1/test_helpers/test_clocks with customer=<cus_…> and frozen_time a minute from now.
  3. Backfill: disable the endpoint in Stripe, then advance the clock past the period end plus two hours. Run the cron event urm_missed_payment_events_check. Expected: Payments shows a second order with the note "Backfilled order for missed payment event". Running it again adds nothing.
  4. Failed renewal: attach pm_card_chargeCustomerFail, make it the subscription's default payment method, enable the endpoint and advance to the next renewal. Expected: the member stays Active, and the Stripe log says "Stripe will retry; subscription status kept". Keep advancing to each next_payment_attempt. After the last attempt, the member goes to Pending.
  5. Kept window: set urm_last_stripe_payment_backfill_sync_time before a missed renewal, put an invalid Stripe test key in place for one cron run, restore it and run again. Expected: the second run recovers the renewal.

Scope: what this touches

  • Stripe missed-payment backfill (run_missed_payment_backfill, cron urm_missed_payment_events_check). Can now create renewal orders and link signup PaymentIntents it never handled on current API versions.
  • Stripe sync scheduling (SubscriptionService::urm_backfill_missed_payment_events). The Stripe payment and one-time backfills read and write the new urm_last_stripe_payment_backfill_sync_time. Status, refund and dispute backfills, PayPal and other gateways are unchanged.
  • Stripe webhooks (handle_webhook, handle_failed_invoice, handle_succeeded_invoice). The subscription ID fallback, the hold for active members while Stripe retries, and where the lookup error is handled.
  • OrdersRepository. One new read-only query.
  • Not touched: PayPal, bank transfer, registration and checkout, the expiry cron.

Out of scope (noted, not changed)

  • The webhook and the cron could both create an order for the same PaymentIntent if they run at the same moment. That existed before this PR; fixing it needs a unique index on transaction_id.
  • The REST entry point still logs a null subscription for current-version payloads. Only the log line is affected, because handle_webhook() now finds it.
  • A failure that keeps the Stripe payment window also re-runs the one-time backfill for that window, which is safe because both match by PaymentIntent.
  • Two QA-suite checks fail on this PR (captcha-options and the 2FA card heading). This PR doesn't touch either, and captcha-options fails the same way on develop and on this PR's base when I run it locally.

Types of changes:

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (modification of the currently available functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully ran tests with your changes locally?
  • Have you updated the documentation accordingly?

Changelog entry

Fix - Stripe missed-payment recovery now records paid renewals on current Stripe API versions and no longer skips a period after a Stripe error, and a failed renewal no longer sets an active member to Pending while Stripe is still retrying the payment.

…/user-registration-pro#1823)

Keep the PaymentIntent lookup helper and take develop's amount_paid handling. The helper now normalises an expanded payment_intent object via extract_stripe_id, as develop does.
…current API versions, and keep the member's status while Stripe retries a failed invoice (themegrill/user-registration-pro#1888)

- The backfill read invoice.subscription and invoice.payment_intent, which Stripe removed in 2025-03-31.basil, so every paid invoice was skipped. It now reads the subscription from parent.subscription_details and resolves the PaymentIntent with the shared invoice lookup, only for subscriptions this site owns. A failed lookup skips that invoice and the run continues.
- The invoice lookup now throws to its caller; the webhook keeps logging it and aborting with an error so Stripe redelivers.
- invoice.payment_failed no longer moves an active or trial subscription to pending while Stripe still has a retry scheduled (next_payment_attempt). A failure with no retry left still sets pending.
…ill/user-registration-pro#1888)

- Handle each invoice once per run: invoice.paid and invoice.payment_succeeded list the same invoice, so the second costs no extra Stripe call.
- Link the signup invoice to a registration order saved without a PaymentIntent instead of creating a duplicate order, via a new OrdersRepository::get_unlinked_order_by_subscription().
- Keep only active members during Stripe retries; an ended trial whose first charge fails goes to pending as before.
- Read the invoice creation time from the same array as the other fields, and update the delayed-start comment that described the old failed-invoice behaviour.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved issues affect order linking, database-write handling, cross-process deduplication, and PaymentIntent ID normalization.

5 open findings
What changed in this PR

Updates Stripe renewal recovery for newer invoice APIs and preserves active membership access during payment retries.

Changes:

  • Backfills paid invoices and links missing signup PaymentIntents.
  • Prevents active subscriptions from reverting to pending during retries.
  • Adds PHP and Playwright regression coverage.
File Summary
tests/​security/​stripe-invoice-payment-intent.php Updates PaymentIntent lookup assertions.
tests/​security/​stripe-backfill-and-failed-invoice.php Adds backfill and retry regression coverage.
tests/​e2e/​specs/​security-stripe-invoice-payment-intent.spec.ts Updates assertion coverage.
tests/​e2e/​specs/​security-stripe-backfill-and-failed-invoice.spec.ts Runs backfill regression tests.
modules/​membership/​includes/​Admin/​Services/​Stripe/​StripeService.php Implements invoice recovery and retry handling.
modules/​membership/​includes/​Admin/​Repositories/​OrdersRepository.php Adds unlinked subscription-order lookup.

🧠 Review effort: Lite


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread modules/membership/includes/Admin/Services/Stripe/StripeService.php
Comment thread modules/membership/includes/Admin/Services/Stripe/StripeService.php
Comment thread modules/membership/includes/Admin/Services/Stripe/StripeService.php
Comment thread modules/membership/includes/Admin/Services/Stripe/StripeService.php Outdated
Comment thread modules/membership/includes/Admin/Repositories/OrdersRepository.php Outdated
@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

QA suite — failed ❌

2 of 21 checks failed. The rest passed.

18 passed · 2 failed · 1 skipped · 0 flaky · 185s


What failed

1. Limit CAPTCHA and login settings writes to declared fields (isolated PHP)

What should happen: Command failed: php /home/runner/work/user-registration/user-registration/product/tests/security/captcha-options.php
PHP Fatal error: Uncaught ErrorException: stripslashes(): Passing null to parameter #1 ($string) of type string is deprecated in /home/runner/work/user-registration/user-registration/product/tests/security/bootstrap.php:80
Stack trace:
#0 [internal function]: {closure}()
#1 /home/runner/work/user-registration/user-registration/product/tests/security/bootstrap.php(80): stripslashes()
#2 [internal function]: wp_unslash()
#3 /home/runner/work/user-registration/user-registration/product/tests/security/bootstrap.php(80): array_map()
#4 /home/runner/work/user-registration/user-registration/product/tests/security/captcha-options.php(115) : eval()'d code(9): wp_unslash()
#5 /home/runner/work/user-registration/user-registration/product/tests/security/bootstrap.php(71): SecurityLoginAjax::login_settings_save_action()
#6 /home/runner/work/user-registration/user-registration/product/tests/security/captcha-options.php(123): security_response()
#7 /home/runner/work/user-registration/user-registration/product/tests/security/captcha-options.php(167): {closure}()
#8 {main}
thrown in /home/runner/work/user-registration/user-registration/product/tests/security/bootstrap.php on line 80

This was retried 1 time and failed every time.

1 trace of this failure is in the report linked at the bottom.

Technical detail

Location: tests/e2e/specs/security-captcha-options.spec.ts:5

Area: security

Likely cause: this failure does not match a known pattern. Check open the trace in the report — it carries the last action, the DOM at that moment, the network log and the console.

Error: Command failed: php /home/runner/work/user-registration/user-registration/product/tests/security/captcha-options.php
PHP Fatal error:  Uncaught ErrorException: stripslashes(): Passing null to parameter #1 ($string) of type string is deprecated in /home/runner/work/user-registration/user-registration/product/tests/security/bootstrap.php:80
Stack trace:
#0 [internal function]: {closure}()
#1 /home/runner/work/user-registration/user-registration/product/tests/security/bootstrap.php(80): stripslashes()
#2 [internal function]: wp_unslash()
#3 /home/runner/work/user-registration/user-registration/product/tests/security/bootstrap.php(80): array_map()
#4 /home/runner/work/user-registration/user-registration/product/tests/security/captcha-options.php(115) : eval()'d code(9): wp_unslash()
#5 /home/runner/work/user-registration/user-registration/product/tests/security/bootstrap.php(71): SecurityLoginAjax::login_settings_save_action()
… truncated, see the full report

2. the 2FA card heading uses the registered label

What happened: the check did not pass. The technical detail is below.

This was retried 1 time and failed every time.

1 screenshot and 1 trace of this failure are in the report linked at the bottom.

Technical detail

Location: tests/e2e/specs/settings-security.spec.ts:97

Area: security

Likely cause: the element never appeared, so the test gave up waiting for it. Check the selector at tests/e2e/specs/settings-security.spec.ts:97. Either this change altered the markup it looks for, or the page needs a state (a menu, a widget, demo content) that a fresh site never seeds.

Expected: "2FA"
Received: "Two Factor Authentication"
   95 |     // With the 2FA addon active the section renders its real settings, not the upsell card.
   96 |     test.skip((await page.locator(".ur-upgrade--link").count()) === 0, "2FA upsell card not shown");
>  97 |     await expect(page.locator(".user-registration-card__title").first()).toHaveText("2FA");
      |                                                                          ^
   98 |   });
   99 | });
  100 |

See it for yourself

Download the full report (qa-suite-user-registration-1547.zip). Unzip it and open qa-report.html in any browser — it shows each failure with screenshots of the page at the moment it broke, and what the run did and did not check.

Replaying a failure step by step (developers)

The archive also carries a Playwright trace — every click, the page at each step, network and console. From the unzipped folder:

npx playwright show-report playwright-report

It needs that command rather than opening the file directly: a trace viewer cannot start from a file:// page.

Automated check — no AI involved. It runs the tests in this branch.

…bhook subscription from parent on current API versions (themegrill/user-registration-pro#1888)

- The missed-payment scheduler moved its shared sync time forward even when the Stripe backfill threw, so one failed run dropped that window's paid renewals for good. Stripe now keeps its own sync time (urm_last_stripe_backfill_sync_time), advanced only after a fully successful run, like PayPal's. A skipped invoice lookup also keeps the window.
- handle_webhook() now falls back to parent.subscription_details.subscription, so invoice events from an endpoint on API 2025-03-31.basil or newer no longer stop at NULL_SUBSCRIPTION_ID.
…ion-pro#1888)

- Keep a failed window only for the order backfills (payments, one-time), which match by PaymentIntent. Status, refund and dispute backfills stay on the shared sync time, because replaying their events re-applies stale snapshots.
- Also keep the window when a backfilled order cannot be saved or linked, not only when Stripe throws.
- Normalize the webhook's subscription ID through extract_stripe_id().
…1888)

- Link a signup invoice only to a Stripe order of the subscription, and only when exactly one unlinked non-trial order exists, so a payment is never written onto a PayPal/bank order or onto a guessed order when an upgrade left a second unlinked one.
- Normalize the PaymentIntent read from invoice.payments through extract_stripe_id(), in case Stripe returns it expanded.
@bhattaganesh bhattaganesh changed the title Fix - Recover paid Stripe renewals in the missed-payment backfill and keep active members while Stripe retries a failed invoice (themegrill/user-registration-pro#1888) Fix - Recover paid Stripe renewals in the missed-payment backfill, keep active members while Stripe retries, and keep the backfill window after a Stripe error (themegrill/user-registration-pro#1888) Oct 8, 2026
@bhattaganesh

Copy link
Copy Markdown
Contributor Author

@claudegrill suite

…themegrill/user-registration-pro#1888)

# Conflicts:
#	modules/membership/includes/Admin/Services/Stripe/StripeService.php
#	tests/e2e/specs/security-stripe-invoice-payment-intent.spec.ts
#	tests/security/stripe-invoice-payment-intent.php
@bhattaganesh
bhattaganesh changed the base branch from fix/1823-stripe-renewal-payment-intent to develop October 8, 2026 05:30
@bhattaganesh

Copy link
Copy Markdown
Contributor Author

@claudegrill suite

@bhattaganesh
bhattaganesh marked this pull request as ready for review October 8, 2026 05:32
@bhattaganesh
bhattaganesh requested a review from y000yal October 8, 2026 05:32
@tg-autopilot

Copy link
Copy Markdown
Contributor

Build for af80c80d is ready 🛎️

⬇️ Download user-registration-5.2.8.zip (8.2M)

Installs directly via Plugins → Add New → Upload Plugin.
Link expires in 30 days · updated Oct 8, 2026 11:18 AM +0545

A later push only rebuilds this if its commit message includes #build-zip.

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

@y000yal y000yal 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.

Verdict: Approve

Closes the pro#1888 gaps correctly:

  1. Basil invoice parent — subscription_details.subscription + payments.data[].payment.payment_intent with extract_stripe_id() / get_invoice_payment_intent_id() fallbacks; legacy subscription / top-level payment_intent still work.
  2. Failed-invoice hold — handle_failed_invoice keeps active while next_payment_attempt is set; only demotes when Stripe is done retrying.
  3. Backfill window — urm_last_stripe_payment_backfill_sync_time advances only when the run has no failures (has_backfill_failure); failed create/update/PI lookup keep the window for the next cron.
  4. Signup link guard — get_unlinked_order_by_subscription requires exactly one unlinked non-trial Stripe order (LIMIT 2 + count check) and payment_method = stripe.

Copilot threads look resolved (PI return type, stripe filter, linking guard, update/create failure → keep window). Race on duplicate orders for the same PI is correctly deferred to a unique index.

Non-blocking notes

  • handle_failed_invoice only preserves active during retries; trial (and other statuses) still fall through to pending if a retryable invoice.payment_failed lands. Confirm that matches product intent for trial first-charge failures.
  • subscription_create linking only runs when exactly one unlinked Stripe order exists — good for upgrade/reg collisions; worth a short comment in the helper so future changes don’t drop the guard.

Security tests + e2e specs cover the main paths. CI suite/codesniff noise is outside this card for triage.

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

@asheshmagar
asheshmagar merged commit e570428 into develop Oct 8, 2026
2 of 6 checks passed
@asheshmagar
asheshmagar deleted the fix/1888-stripe-backfill-and-failed-invoice-status branch October 8, 2026 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants