feat(wc-subscriptions): recover switch proration when no amount paid#4745
feat(wc-subscriptions): recover switch proration when no amount paid#4745miguelpeixe wants to merge 16 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses incorrect proration behavior when switching WooCommerce Subscriptions plans in cases where WCS computes a non-positive “total paid for current period” (e.g., migrated subscriptions, fully-discounted/comped orders, or broken switch chains). It introduces a fallback baseline based on the subscription line item’s recurring total so downgrades are not misclassified as upgrades and don’t incur an erroneous prorated sign-up fee.
Changes:
- Hook
wcs_switch_total_paid_for_current_periodand, when WCS returns a non-positive amount, fall back to the subscription line item’s recurring total. - Add unit tests covering zero baseline recovery, pass-through for positive values, genuinely free subscriptions, and unexpected item types.
- Extend the WooCommerce test mock
WC_Order_Item_Productto supportget_total().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php | Adds unit tests for the new “recover total paid” switch proration fallback behavior. |
| tests/mocks/wc-mocks.php | Adds WC_Order_Item_Product::get_total() to support the new logic and tests. |
| includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php | Adds the wcs_switch_total_paid_for_current_period filter handler that restores a proration baseline when WCS reports zero/non-positive paid amount. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
miguelpeixe
changed the title
miguelpeixe
force-pushed
the
nppm-2837-switching-between-variations-in-separate-products
branch
from
4171f68 to
2cd7405
Compare
miguelpeixe
added
the
[Status] Needs Review
There was a problem hiding this comment.
I think it's worth outlining exactly what I saw during manual testing as there's math involved and I want to make sure that the numbers look correct to you. Here's what I saw. Some other minor comments inline, but can be deferred if necessary.
Setup: test products created
- PR4745 Membership: a variable subscription with two tiers:
- Regular - $5 / month, $3 sign-up fee, 1 month free trial
- Pro - $10 / month, $6 sign-up fee, 1 month free trial
- PR4745 Annual Pass: a simple subscription, $50 / month with no sign-up fee or free trial period
- PR4745 Free Trial: a simple subscription, $10 / month with no sign-up fee, 1 month free trial period
Step 2: Switch to a lower-price variation ("downgrade") with a migrated subscription
- Created a migrated subscription (no order history, stamped with
_stripe_subscription_idmeta value) with the Pro product - Logged in as the subscription owner, clicked "Change subscription" in the My Account page, selected the Regular product
- Transaction details were:
| Product | Subtotal |
|---|---|
| PR4745 Membership - Regular × 1 | $5.00 / month and a $3.00 sign-up fee |
| Subtotal | $3.00 |
| Total | $3.00 |
| Recurring totals | |
| Subtotal | $5.00 / month |
| Recurring total | $5.00 / month First renewal: July 30, 2026 |
Verdict: ✅ Passes. The $3.00 sign-up fee is expected. From the PR description: "Active free trials are skipped (a migrated sub still in a trial has paid nothing)"
Step 3: Switch to a lower-price variation with a regular / non-migrated subscription
- As a reader, purchased a Pro subscription
- Backdated the start date to the same start date as the migrated subscription in step 2
- In the My Account subscription page, started the switch flow via "Change subscription"
- Transaction details were:
| Product | Subtotal |
|---|---|
| PR4745 Membership - Regular × 1 | $5.00 / month and a $3.00 sign-up fee |
| Subtotal | $0.00 |
| Total | $0.00 |
| Recurring totals | |
| Subtotal | $5.00 / month |
| Recurring total | $5.00 / month First renewal: June 29, 2026 |
Verdict: ✅ Passes. The $0.00 sign-up fee is expected. From the PR description: "Switch ends the first-period discount on both sides; charge the new plan at its regular recurring price for the first cycle, crediting any unconsumed portion of what the reader paid." The sign-fee for the Pro plan is $6, the fee for Regular is $3, so the user is credited $3 for a total sign-up fee of $0.
Step 4: Paid-trial, opt-in enabled, immediate switch (matching trials)
- Purchased a Regular subscription
- In My Account, switched to Pro
- Transaction details:
| Product | Subtotal |
|---|---|
| PR4745 Membership - Pro × 1 | $10.00 / month and a $7.00 sign-up fee |
| Subtotal | $7.00 |
| Total | $7.00 |
| Recurring totals | |
| Subtotal | $10.00 / month |
| Recurring total | $10.00 / month First renewal: June 29, 2026 |
Verdict: ✅ Passes. Total = new_recurring − unconsumed_credit — for $10 recurring with $3 unconsumed, that is $7 (plus transaction fee, if any).
Step 5: Paid-trial, opt-in enabled, mid-trial switch (matching trials)
- Adjusted the trial end, next payment, start, and last payment dates for the subscription purchased in step 4 to 14 days earlier.
- In My Account, switched to Pro
- Transaction details:
| Product | Subtotal |
|---|---|
| PR4745 Membership - Pro × 1 | $10.00 / month and a $8.41 sign-up fee |
| Subtotal | $8.41 |
| Total | $8.41 |
| Recurring totals | |
| Subtotal | $10.00 / month |
| Recurring total | $10.00 / month First renewal: June 29, 2026 |
Verdict: ✅ Passes, with a caveat: the important date here is apparently the date of the last order, not the trial end or next payment dates. Until I backdated the last payment date, the total was still showing $7. Functionally identical in a real-world scenario without manually-edited dates.
Step 6: Paid-trial, opt-in enabled, switch into a non-matching plan
- Switched the same subscription from steps 4 and 5 to the PR4745 Annual Pass product
- Transaction details:
| Product | Subtotal |
|---|---|
| PR4745 Annual Pass × 1 | $50.00 / year and a $0.74 sign-up fee |
| Subtotal | $0.74 |
| Total | $0.74 |
| Recurring totals | |
| Subtotal | $50.00 / year |
| Recurring total | $50.00 / year First renewal: June 15, 2026 |
Verdict: ✅ Passes. This one threw me for a bit of a loop, so I had to break it down to make sure it was correct:
The formula: days_until_next × (new_price_per_day − old_price_per_day), with old_price_per_day = paid_sign_up_fee / days_in_old_cycle.
days_in_old_cycle= 32 →old_price_per_day= $3 / 32 = $0.09375new_price_per_day= $50 / 365.25 = $0.13689days_until_next= 17- charge = 17 × ($0.13689 − $0.09375) = $0.733… → $0.73–0.74
Step 7: Regression — paid-trial, opt-in disabled
- Deleted the
NEWSPACK_WC_SUBS_SWITCH_INCLUDE_SIGNUP_FEEconstant - Repeated the switch in step 5
- Transaction details:
| Product | Subtotal |
|---|---|
| PR4745 Membership - Pro × 1 | $10.00 / month and a $3.00 sign-up fee |
| Subtotal | $3.00 |
| Total | $3.00 |
| Recurring totals | |
| Subtotal | $10.00 / month |
| Recurring total | $10.00 / month First renewal: June 15, 2026 |
Verdict: ✅ Passes. No change from release behavior. From PR description: "the matching-trials path forces new_price_per_day to 0 and routes the switch through extend_prepaid_term, so the reader is upgraded to a more expensive plan for free (or for just the sign-up-fee delta, depending on store settings)."
Step 8: Regression — comped / 100%-discount
- Restored the
NEWSPACK_WC_SUBS_SWITCH_INCLUDE_SIGNUP_FEEconstant - As a new reader, purchased a Regular subscription with a 100% coupon code
- In My Account, switched to Pro
- Transaction details:
| Product | Subtotal |
|---|---|
| PR4745 Membership - Pro × 1 | $10.00 / month and a $6.00 sign-up fee |
| Subtotal | $6.00 |
| Total | $6.00 |
| Recurring totals | |
| Subtotal | $10.00 / month |
| Recurring total | $10.00 / month First renewal: June 29, 2026 |
Verdict: ✅ Passes. No change from release; filter is a no-op.
Step 9: Regression — genuine free trial
- As a new reader, purchased a PR4745 Free Trial subscription
- In My Account, switched to Pro
- Transaction details:
| Product | Subtotal |
|---|---|
| PR4745 Membership - Pro × 1 | $10.00 / month and a $6.00 sign-up fee |
| Subtotal | $6.00 |
| Total | $6.00 |
| Recurring totals | |
| Subtotal | $10.00 / month |
| Recurring total | $10.00 / month First renewal: June 29, 2026 |
Verdict: ✅ Passes. No change from release; filter is a no-op.
| return $value; | ||
| } | ||
|
|
||
| $unconsumed_credit = $total_paid * ( $days_until_next / $days_in_old_cycle ); |
There was a problem hiding this comment.
🟡 Unbounded proration ratio → reader under-charged
$days_until_next (WCS ceil()) can exceed $days_in_old_cycle (WCS round()) by ~1 day near a cycle boundary, making unconsumed_credit > total_paid and the ratio > 1.0. max(…, 0.0) prevents a negative charge but the reader is under-charged. The two day-counts also come from different sources for migrated subs (only days_in_old_cycle flows through the clamp filter), compounding the skew.
Suggested fix: clamp the ratio — min( 1.0, max( 0.0, $days_until_next / $days_in_old_cycle ) ).
| // paid, so a switch during the trial sees $0. When the publisher has | ||
| // opted in, count the sign-up fee the reader actually paid. A free | ||
| // trial with no sign-up fee, or a comp, yields nothing and no-ops. | ||
| if ( self::should_count_signup_fee_on_switch( $subscription, $existing_item ) ) { |
There was a problem hiding this comment.
🟡 recover_total_paid_for_switch branch 2 isn't gated like the override is
The recovery branch fires whenever total_paid ≤ 0 and the opt-in is on — it does not also require an active trial or a paid sign-up fee, unlike apply_stepped_pricing_switch_charge() which checks both. The actual financial exposure is bounded (it returns max(actual-amount-paid, total_paid), so 100%-discount/comp cases self-limit to ~0), but an out-of-trial sub with a zero current period and historical payment could still get a baseline the PR's stated scope says it shouldn't.
Suggested fix/decision: mirror the active-trial + paid-sign-up-fee preconditions into the recovery branch for symmetry, or document why the looser gate is intentional. Add the missing no-op tests (opt-in on + out-of-trial / genuine free trial / normal-history zero period).
| // The stepped-pricing signature: the old line item actually carries | ||
| // a paid sign-up fee. A real free trial (sign-up fee = 0) is left | ||
| // alone so we never invent a charge for a reader who paid nothing. | ||
| $paid_sign_up_fee = (float) $subscription->get_items_sign_up_fee( $existing_item ); |
There was a problem hiding this comment.
🟡 Tax-mode mismatch on tax-inclusive stores
The PR calls get_items_sign_up_fee($existing_item) with the default (exclusive_of_tax), while WCS's own apportion_sign_up_fees passes $this->prices_include_tax ? 'inclusive_of_tax' : 'exclusive_of_tax' (calculator:312). On a tax-inclusive store the recovered baseline / >0 signature check can be tax-mismatched against new_recurring from WC_Subscriptions_Product::get_price().
Suggested fix: mirror WCS's tax-mode selection. Consider adding tests to cover a tax-inclusive case.
| // credit -- the right answer for switches that inherit the existing | ||
| // next-payment date. Pass through here so WCS's default applies and | ||
| // we do not double-charge by also overriding the sign-up fee. | ||
| if ( method_exists( $switch_item, 'trial_periods_match' ) && ! $switch_item->trial_periods_match() ) { |
There was a problem hiding this comment.
🟡 trial_periods_match guard fails unsafe
The "don't double-charge on non-matching trials" guard is gated on method_exists(...) && ! trial_periods_match(). If the method is ever absent, the condition short-circuits to false and the override applies — the dangerous path. (It exists in 8.6.1, so this only bites on other WCS versions, but every other method_exists guard here fails safe.)
Suggested fix:
| if ( method_exists( $switch_item, 'trial_periods_match' ) && ! $switch_item->trial_periods_match() ) { | |
| if ( ! method_exists( $switch_item, 'trial_periods_match' ) || ! $switch_item->trial_periods_match() ) { |
| * We hook wcs_switch_sign_up_fee (not extra_to_pay) because WCS | ||
| * classifies a matching-trials switch as a downgrade -- it forces | ||
| * new_price_per_day to 0, then sees old_pp > new_pp and routes through | ||
| * extend_prepaid_term, which never calls calculate_upgrade_cost or |
There was a problem hiding this comment.
🟡 One-payment-subscription edge
The docstring claims a matching-trials switch is always routed through extend_prepaid_term so the override is the only fee applied. For a switch into a length-1 (one-payment) subscription, WCS's set_upgrade_cost() does sign_up_fee = existing_fee + extra_to_pay, adding the PR's overridden fee on top. Narrow (requires matching trials + one-payment target + recurring proration), but the docstring's absolute claim isn't strictly true.
Suggested fix: bail when is_switch_to_one_payment_subscription(), or soften the docstring and confirm stepped-pricing products are never length-1.
github-actions
Bot
added
[Status] Approved
All Submissions:
Changes proposed in this Pull Request:
When a reader switches subscription plans, WooCommerce Subscriptions prorates the change from the amount paid for the current billing period — which it derives by summing the matching line item across the subscription's related orders. For two populations that sum is
$0, which makes WCS treat the old subscription as$0/day, misclassify a downgrade as an upgrade, and charge the full prorated price of the new plan as a sign-up fee instead of crediting the remaining term.This PR corrects the switch behavior for those two cases via WCS filters:
1. Migrated subscriptions. Subscriptions migrated into WooCommerce from another platform (Piano, Stripe) have no Woo order history, so WCS sees
$0. Newspack already opts these into switching viaallow_migrated_subscription_switch(). Thewcs_switch_total_paid_for_current_periodfilter falls back to the subscription line item's recurring total — one billing period's recurring charge, dimensionally what WCS divides by the billing-cycle length. Active free trials are skipped (a migrated sub still in a trial has paid nothing). This applies on all migrated subscriptions.A companion
wcs_switch_proration_days_in_old_cyclefilter clamps the denominator to one billing cycle for migrated subscriptions: without it, WCS's(next_payment - subscription_start) / DAY_IN_SECONDScalculation spans the entire time since the original platform sign-up (often years), which would makeold_price_per_dayartificially low and still misclassify a downgrade as an upgrade even after the amount-paid recovery above. Non-migrated subscriptions are left to WCS's default behavior.2. Paid-trial subscriptions (opt-in). Some publishers express stepped pricing as a one-time sign-up fee plus a free trial — the sign-up fee is functionally a first-period discount, not a real one-time fee. Switching between two such products at WCS's defaults is broken: the matching-trials path forces
new_price_per_dayto0and routes the switch throughextend_prepaid_term, so the reader is upgraded to a more expensive plan for free (or for just the sign-up-fee delta, depending on store settings).When the publisher opts in by defining
NEWSPACK_WC_SUBS_SWITCH_INCLUDE_SIGNUP_FEEinwp-config.php, the integration:wcs_switch_total_paid_for_current_period(so the oldprice_per_dayreflects the discount the reader actually paid). This applies on every stepped-pricing switch and feeds the rest of WCS's calculations a correct baseline.wcs_switch_sign_up_feetomax(new_recurring − unconsumed_credit, 0)only when the old and new products share a trial period — the model is "switch ends the first-period discount on both sides; charge the new plan at its regular recurring price for the first cycle, crediting any unconsumed portion of what the reader paid." Matching trials forcenew_price_per_dayto0, so WCS routes the switch throughextend_prepaid_termand never callswcs_switch_proration_extra_to_pay— the sign-up fee is the only hook that fires.calculate_upgrade_cost, which — given the correctedtotal_paidbaseline above — already equals the prorated remaining-term price minus the prorated unconsumed credit. That is the right charge for switches that inherit the existing next-payment date.Off by default; only opted-in publishers are affected. The opt-in is also exposed as the
newspack_wc_subs_switch_include_signup_feefilter for finer-grained control (e.g. per-subscription or per-product) — callbacks receive the subscription and line item alongside the enabled flag. The override only intervenes during an active trial when the existing line item carries a paid sign-up fee (the stepped-pricing signature); real free trials, comps, and out-of-trial switches all pass through to WCS's default behavior. Works regardless of WC's store-wideapportion_sign_up_feesetting.Scope is intentionally narrow. Every other zero-paid case — 100%-discount purchases, comps, ordinary unpaid states — is left to WCS's default switching behavior on purpose: we do not carry discounts or comps across switches. Subscriptions with normal order history are untouched (WCS already returns a positive value and the filter no-ops).
Closes NPPM-2837.
How to test the changes in this Pull Request:
$5/month + $3 sign-up fee + 1 month trial, Pro at$10/month + $6 sign-up fee + 1 month trial._piano_subscription_idor_stripe_subscription_id). Backdate the subscription start so it predates the next payment by more than one billing cycle (this surfaces thedays_in_old_cycleclamp). Switch it to a cheaper plan and expand the transaction details. Verify the switch is correctly classified as a downgrade — the term should extend / credit the remaining period rather than charging a full prorated sign-up fee.define( 'NEWSPACK_WC_SUBS_SWITCH_INCLUDE_SIGNUP_FEE', true );towp-config.php. Purchase Regular (reader pays the$3sign-up fee, enters the trial). Immediately switch to Pro and expand the transaction details. Verify the charge equalsnew_recurring − unconsumed_credit— for$10recurring with$3unconsumed, that is$7(plus transaction fee, if any).trial_endby 15 days using WP-CLI). Switch to Pro. Verify the charge scales with consumed time — for half-consumed, the charge is roughly$10 − $1.50 = $8.50.$10 / 4 weeks, no sign-up fee, no trialproduct, or$50/year, no trial. Verify the upfront charge equals WCS's standard prorated upgrade cost:days_until_next × (new_price_per_day − old_price_per_day), whereold_price_per_day = paid_sign_up_fee / days_in_old_cycle. Confirm the next-payment date is inherited from the old subscription (no reset). This exercises the pass-through path where WCS's ownextra_to_payis correct.define()(or set itfalse) and repeat step 5. Confirm the switch behaves like release does today — the integration does not intervene and WCS's default switching cost applies.Other information: