feat: reactivation flow for cancelled memberships + trial skip#750
Open
kenedytorcatt wants to merge 1 commit intoUltimate-Multisite:mainfrom
Open
feat: reactivation flow for cancelled memberships + trial skip#750kenedytorcatt wants to merge 1 commit intoUltimate-Multisite:mainfrom
kenedytorcatt wants to merge 1 commit intoUltimate-Multisite:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughDetects cancelled memberships and introduces a "reactivation" cart/checkout flow: login redirects to a reactivation checkout, the cart can be rebuilt from a cancelled membership with signup fees disabled, checkout skips site-creation/customization fields, and membership model gains reactivation helpers. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant WP as WordPress (wp_login)
participant PM as Payment Manager
participant Mem as Membership Model
participant CO as Checkout
participant Cart as Cart Model
User->>WP: login
WP->>PM: maybe_redirect_cancelled_membership(user_login,user)
PM->>Mem: load customer's memberships
Mem-->>PM: returns cancelled membership
PM->>PM: build reactivation URL (plan_id,membership_id)
PM->>User: wp_safe_redirect(reactivation URL)
User->>CO: open reactivation checkout
CO->>Cart: build_from_membership(membership, product_ids?)
Cart->>Mem: membership->is_cancelled()
Mem-->>Cart: true
Cart->>Cart: set cart_type='reactivation', copy currency/country, populate products, disable signup fees
Cart-->>CO: configured cart
CO->>CO: setup_checkout() -> remove site fields for reactivation
CO->>CO: maybe_create_site() -> return existing site (skip creation)
User->>Mem: confirm/reactivate
Mem->>Mem: reactivate() (renew, clear cancellation, save, hooks)
Mem-->>User: reactivation result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kenedytorcatt
force-pushed
the
feature/reactivation-flow-upstream
branch
from
e9fac9a to
8e4d6c3
Compare
Collaborator
|
Please do not include the images in the docs folder |
… trial skip
Adds a complete self-service reactivation flow for cancelled memberships
and skips the 15-day trial on reactivation carts to prevent a revenue leak.
Core changes (5 files, additive — gated behind cart_type='reactivation'):
* inc/models/class-membership.php
- Add is_cancelled() helper
- Add reactivate() method (reuses renew() + wu_membership_pre/post_reactivate hooks)
* inc/checkout/class-cart.php
- Detect cancelled membership in build_from_membership() and set cart_type='reactivation'
- Skip trial in get_billing_start_date() when cart_type='reactivation'
(customer already used their trial — prevents infinite free trials via cancel+resub)
* inc/checkout/class-checkout.php
- Skip site_url/site_title/template_selection fields for reactivation carts
- maybe_create_site() returns existing site instead of creating a new one
* inc/managers/class-payment-manager.php
- maybe_redirect_cancelled_membership() on wp_login: redirects users
whose only memberships are cancelled to the reactivation checkout
(breaks the infinite login redirect loop)
* inc/managers/class-site-manager.php
- Replace the wp_die() dead-end in lock_site() with a friendly HTML page
that shows a 'Renew your subscription' button. Filterable via
wu_blocked_site_reactivation_url and wu_blocked_site_template.
New hooks:
* wu_membership_pre_reactivate (action)
* wu_membership_post_reactivate (action)
* wu_blocked_site_reactivation_url (filter)
* wu_blocked_site_template (filter)
* wu_blocked_site_support_url (filter)
* wu_cancelled_membership_redirect_url (filter)
No DB changes. No addon changes. No WooCommerce changes.
Active/pending/expired memberships are not affected — new logic only runs
when cart_type === 'reactivation'.
Version bump: 2.4.13-beta.21
kenedytorcatt
force-pushed
the
feature/reactivation-flow-upstream
branch
from
8e4d6c3 to
85439b4
Compare
Author
|
Done — removed the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/checkout/class-cart.php`:
- Around line 818-847: The reactivation branch returns before validating that
the current customer actually owns the cancelled membership, allowing a
guest/other customer to reuse someone else's membership; modify the logic in the
method containing the shown block (class-cart.php) to perform the ownership
check (compare $this->customer->get_id() / $this->customer with
$membership->get_user_id() or the membership ownership accessor) before setting
cart_type = 'reactivation', calling set_currency(), adding products, or adding
the wu_apply_signup_fee filter, and only proceed with the reactivation setup and
the early return if the ownership check passes; if the check fails, do not treat
it as a reactivation so the later permission guard and maybe_create_membership()
run as normal.
- Around line 829-835: When $this->attributes->products is empty the cart
rebuild only adds $membership->get_plan_id(), skipping any attached add-ons on
the existing membership; change the empty case to pull product IDs from the
membership model and add them all. Replace the current branch so that when
$this->attributes->products is empty you retrieve the membership's product IDs
(e.g. via $membership->get_product_ids() or $membership->get_products()),
iterate those IDs and call $this->add_product(...) for each (making sure the
plan ID is included and duplicates are avoided), otherwise keep the existing
foreach for $this->attributes->products.
In `@inc/checkout/class-checkout.php`:
- Around line 1343-1346: The branch that returns current($sites) on reactivation
can return a pending/stale site because Membership::get_sites() includes pending
metadata; update the logic in the reactivation branch (the code that checks
$this->order->get_cart_type() === 'reactivation' and calls
$this->membership->get_sites()) to scan/filter the returned sites for a
published/live site (e.g., check each site's published status or
is_published()/status === 'published') and return the first published site,
otherwise return false so site creation proceeds. Ensure you replace the direct
current($sites) return with that filtered lookup using the site object
property/method that indicates published state.
- Around line 478-489: The reactivation check runs too late because
request_or_session('cart_type') and $this->order may be 'new' or null on the
initial request; update the logic in the checkout class (around the block using
request_or_session, $this->order, and get_cart_type) to also infer reactivation
when membership_id or plan_id is present in the request (e.g.,
$this->request_or_session or $_REQUEST), and then apply the same filtering of
$this->step['fields'] (removing site_field_types via wu_get_isset) so the
reactivation path is used before process_order() builds the cart; ensure you
still preserve the existing checks against $this->order->get_cart_type() to
cover already-built orders.
In `@inc/managers/class-payment-manager.php`:
- Around line 233-245: The code currently stops on the first cancelled
membership (the $cancelled_membership set inside the foreach that breaks), which
can pick the wrong subscription; instead, collect all cancelled memberships from
get_memberships() by checking method_exists(..., 'is_cancelled') &&
is_cancelled(), then choose the appropriate one deterministically: prefer a
membership whose site/blog id matches the current site (e.g., compare
$membership->get_site_id() or get_blog_details()/get_current_blog_id()), or if
none match and there are multiple cancelled memberships, do not
auto-redirect—return or present a selection to the user; update the logic around
$cancelled_membership and the redirect to use this deterministic selection
strategy rather than breaking on the first match.
In `@inc/managers/class-site-manager.php`:
- Around line 465-489: Initialize and ensure a sensible fallback for
$reactivation_url so the blocked-site template always has a CTA: before the
membership/is_cancelled block set $reactivation_url = '' (or fetch the generic
checkout/register URL) and then, if the cancelled-membership logic builds a
specific URL use that; otherwise keep the generic checkout URL (or a safe
placeholder) and still run it through
apply_filters('wu_blocked_site_reactivation_url', ...). Update the code around
the membership check (the block using $membership->is_cancelled(),
$checkout_pages->get_page_url('register'), and apply_filters) so
$reactivation_url is always defined and contains a usable fallback when not
built for cancelled memberships.
In `@inc/models/class-membership.php`:
- Around line 2434-2449: The reactivation flow does a second save after calling
$this->renew(), which causes listeners to run with the stale cancellation date
and makes reactivate() return false even when renewal already persisted; move
the call to $this->set_date_cancellation(null) so it runs before
$this->renew(false, 'active') and remove the subsequent $this->save() /
is_wp_error() block so reactivation becomes a single write handled by renew();
update reactivate() to rely on renew() to persist and only log/return based on
the renew() result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5232449b-c676-49bd-8643-60d8d259b3b8
⛔ Files ignored due to path filters (6)
docs/reactivation-flow/after-01-renew-button.pngis excluded by!**/*.pngdocs/reactivation-flow/after-02-checkout-page.pngis excluded by!**/*.pngdocs/reactivation-flow/after-03-blocked-page-improved.pngis excluded by!**/*.pngdocs/reactivation-flow/before-01-suspended-page.pngis excluded by!**/*.pngdocs/reactivation-flow/before-02-after-login-loop.pngis excluded by!**/*.pngdocs/reactivation-flow/before-03-wpdie-ugly.pngis excluded by!**/*.png
📒 Files selected for processing (7)
inc/checkout/class-cart.phpinc/checkout/class-checkout.phpinc/class-wp-ultimo.phpinc/managers/class-payment-manager.phpinc/managers/class-site-manager.phpinc/models/class-membership.phpultimate-multisite.php
Comment on lines
+818
to
+847
| if (method_exists($membership, 'is_cancelled') && $membership->is_cancelled()) { | ||
| $plan_matches = ! empty($this->attributes->products) | ||
| && in_array($membership->get_plan_id(), (array) $this->attributes->products, false); | ||
|
|
||
| if ($plan_matches || empty($this->attributes->products)) { | ||
| $this->cart_type = 'reactivation'; | ||
| $this->membership = $membership; | ||
|
|
||
| $this->country = $this->country ?: ($this->customer ? $this->customer->get_country() : ''); | ||
| $this->set_currency($membership->get_currency()); | ||
|
|
||
| if (empty($this->attributes->products)) { | ||
| $this->add_product($membership->get_plan_id()); | ||
| } else { | ||
| foreach ($this->attributes->products as $product_id) { | ||
| $this->add_product($product_id); | ||
| } | ||
| } | ||
|
|
||
| $plan_product = $membership->get_plan(); | ||
|
|
||
| if ($plan_product) { | ||
| $this->duration = $plan_product->get_duration(); | ||
| $this->duration_unit = $plan_product->get_duration_unit(); | ||
| } | ||
|
|
||
| // Skip signup fee for reactivations — they already paid it. | ||
| add_filter('wu_apply_signup_fee', '__return_false'); | ||
|
|
||
| return true; |
Contributor
There was a problem hiding this comment.
Reactivation currently bypasses membership ownership checks.
This early return runs before the permission guard below, so a guest or another customer can pass a cancelled membership_id and checkout will later reuse that membership in maybe_create_membership(). The customer ownership check needs to happen before any reactivation setup.
🛠️ Suggested fix
$membership = wu_get_membership($membership_id);
if ( ! $membership) {
$this->errors->add('membership_not_found', __('The membership in question was not found.', 'ultimate-multisite'));
return true;
}
+
+ if (empty($this->customer) || $this->customer->get_id() !== $membership->get_customer_id()) {
+ $this->errors->add('lacks_permission', __('You are not allowed to modify this membership.', 'ultimate-multisite'));
+
+ return true;
+ }
/*
* Reactivation flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/checkout/class-cart.php` around lines 818 - 847, The reactivation branch
returns before validating that the current customer actually owns the cancelled
membership, allowing a guest/other customer to reuse someone else's membership;
modify the logic in the method containing the shown block (class-cart.php) to
perform the ownership check (compare $this->customer->get_id() / $this->customer
with $membership->get_user_id() or the membership ownership accessor) before
setting cart_type = 'reactivation', calling set_currency(), adding products, or
adding the wu_apply_signup_fee filter, and only proceed with the reactivation
setup and the early return if the ownership check passes; if the check fails, do
not treat it as a reactivation so the later permission guard and
maybe_create_membership() run as normal.
Comment on lines
+1343
to
+1346
| if ($this->order && $this->order->get_cart_type() === 'reactivation') { | ||
| $sites = $this->membership->get_sites(); | ||
|
|
||
| return ! empty($sites) ? current($sites) : false; |
Contributor
There was a problem hiding this comment.
Require a published site before skipping site creation.
Membership::get_sites() includes pending-site metadata. If a cancelled membership only has a stale pending site, this branch returns that object and checkout skips site creation entirely, so the customer can pay for a reactivation without any live site to restore.
🛠️ Suggested fix
- if ($this->order && $this->order->get_cart_type() === 'reactivation') {
- $sites = $this->membership->get_sites();
-
- return ! empty($sites) ? current($sites) : false;
+ if ($this->order && 'reactivation' === $this->order->get_cart_type()) {
+ $sites = $this->membership->get_sites(false);
+
+ if (empty($sites)) {
+ return new \WP_Error(
+ 'reactivation_site_missing',
+ __('We could not find an existing site for this membership. Please contact support.', 'ultimate-multisite')
+ );
+ }
+
+ return current($sites);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ($this->order && $this->order->get_cart_type() === 'reactivation') { | |
| $sites = $this->membership->get_sites(); | |
| return ! empty($sites) ? current($sites) : false; | |
| if ($this->order && 'reactivation' === $this->order->get_cart_type()) { | |
| $sites = $this->membership->get_sites(false); | |
| if (empty($sites)) { | |
| return new \WP_Error( | |
| 'reactivation_site_missing', | |
| __('We could not find an existing site for this membership. Please contact support.', 'ultimate-multisite') | |
| ); | |
| } | |
| return current($sites); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/checkout/class-checkout.php` around lines 1343 - 1346, The branch that
returns current($sites) on reactivation can return a pending/stale site because
Membership::get_sites() includes pending metadata; update the logic in the
reactivation branch (the code that checks $this->order->get_cart_type() ===
'reactivation' and calls $this->membership->get_sites()) to scan/filter the
returned sites for a published/live site (e.g., check each site's published
status or is_published()/status === 'published') and return the first published
site, otherwise return false so site creation proceeds. Ensure you replace the
direct current($sites) return with that filtered lookup using the site object
property/method that indicates published state.
Comment on lines
+233
to
+245
| $cancelled_membership = null; | ||
|
|
||
| foreach ($memberships as $membership) { | ||
| if (method_exists($membership, 'is_cancelled') && $membership->is_cancelled()) { | ||
| $cancelled_membership = $membership; | ||
|
|
||
| break; | ||
| } | ||
| } | ||
|
|
||
| if ( ! $cancelled_membership) { | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Don't auto-redirect to the first cancelled membership.
Customers can have more than one cancelled membership/site. Breaking on the first match makes this redirect depend on get_memberships() ordering and can send the user to reactivate the wrong subscription.
🛠️ Suggested fix
- $cancelled_membership = null;
-
- foreach ($memberships as $membership) {
- if (method_exists($membership, 'is_cancelled') && $membership->is_cancelled()) {
- $cancelled_membership = $membership;
-
- break;
- }
- }
-
- if ( ! $cancelled_membership) {
+ $cancelled_memberships = array_values(
+ array_filter(
+ $memberships,
+ static fn($membership) => $membership->is_cancelled()
+ )
+ );
+
+ if (1 !== count($cancelled_memberships)) {
return;
}
+
+ $cancelled_membership = $cancelled_memberships[0];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/managers/class-payment-manager.php` around lines 233 - 245, The code
currently stops on the first cancelled membership (the $cancelled_membership set
inside the foreach that breaks), which can pick the wrong subscription; instead,
collect all cancelled memberships from get_memberships() by checking
method_exists(..., 'is_cancelled') && is_cancelled(), then choose the
appropriate one deterministically: prefer a membership whose site/blog id
matches the current site (e.g., compare $membership->get_site_id() or
get_blog_details()/get_current_blog_id()), or if none match and there are
multiple cancelled memberships, do not auto-redirect—return or present a
selection to the user; update the logic around $cancelled_membership and the
redirect to use this deterministic selection strategy rather than breaking on
the first match.
Comment on lines
+465
to
+489
| if ($membership && method_exists($membership, 'is_cancelled') && $membership->is_cancelled()) { | ||
| $checkout_pages = \WP_Ultimo\Checkout\Checkout_Pages::get_instance(); | ||
| $checkout_url = $checkout_pages->get_page_url('register'); | ||
|
|
||
| if ($checkout_url) { | ||
| $reactivation_url = add_query_arg( | ||
| [ | ||
| 'plan_id' => $membership->get_plan_id(), | ||
| 'membership_id' => $membership->get_id(), | ||
| ], | ||
| $checkout_url | ||
| ); | ||
|
|
||
| /** | ||
| * Filters the reactivation URL shown on blocked sites. | ||
| * | ||
| * @param string $reactivation_url The reactivation checkout URL. | ||
| * @param \WP_Ultimo\Models\Membership $membership The cancelled membership. | ||
| * @param \WP_Ultimo\Models\Site $site The blocked site. | ||
| * | ||
| * @since 2.4.14 | ||
| */ | ||
| $reactivation_url = apply_filters('wu_blocked_site_reactivation_url', $reactivation_url, $membership, $site); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Keep the fallback page generic when no reactivation URL exists.
$reactivation_url is only built for cancelled memberships, but this template is also used by other false === $can_access paths. When the URL is empty, the page still tells users to renew their subscription and then offers no matching CTA.
Suggested tweak
- $html .= '<h1>' . esc_html__('This site is not available', 'ultimate-multisite') . '</h1>';
- $html .= '<p>' . esc_html__('The subscription for this site has expired or been cancelled. To restore access, please renew your subscription.', 'ultimate-multisite') . '</p>';
-
- if ( ! empty($reactivation_url)) {
- $html .= '<a class="wu-btn wu-btn-primary" href="' . esc_url($reactivation_url) . '">' . esc_html__('Renew your subscription', 'ultimate-multisite') . '</a>';
- }
+ $html .= '<h1>' . esc_html__('This site is not available', 'ultimate-multisite') . '</h1>';
+
+ if ( ! empty($reactivation_url)) {
+ $html .= '<p>' . esc_html__('The subscription for this site has expired or been cancelled. To restore access, please renew your subscription.', 'ultimate-multisite') . '</p>';
+ $html .= '<a class="wu-btn wu-btn-primary" href="' . esc_url($reactivation_url) . '">' . esc_html__('Renew your subscription', 'ultimate-multisite') . '</a>';
+ } else {
+ $html .= '<p>' . esc_html__('This site is currently unavailable. Please log in for more details.', 'ultimate-multisite') . '</p>';
+ }Also applies to: 509-514
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/managers/class-site-manager.php` around lines 465 - 489, Initialize and
ensure a sensible fallback for $reactivation_url so the blocked-site template
always has a CTA: before the membership/is_cancelled block set $reactivation_url
= '' (or fetch the generic checkout/register URL) and then, if the
cancelled-membership logic builds a specific URL use that; otherwise keep the
generic checkout URL (or a safe placeholder) and still run it through
apply_filters('wu_blocked_site_reactivation_url', ...). Update the code around
the membership check (the block using $membership->is_cancelled(),
$checkout_pages->get_page_url('register'), and apply_filters) so
$reactivation_url is always defined and contains a usable fallback when not
built for cancelled memberships.
Comment on lines
+2434
to
+2449
| $renewed = $this->renew(false, 'active'); | ||
|
|
||
| if ( ! $renewed) { | ||
| wu_log_add("membership-{$id}", sprintf('Membership reactivation failed for membership #%d: renewal returned false.', $id)); | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| $this->set_date_cancellation(null); | ||
|
|
||
| $status = $this->save(); | ||
|
|
||
| if (is_wp_error($status)) { | ||
| wu_log_add("membership-{$id}", sprintf('Membership reactivation failed for membership #%d: %s', $id, $status->get_error_message())); | ||
|
|
||
| return false; |
Contributor
There was a problem hiding this comment.
Make reactivation a single write.
renew() already saves the membership and fires wu_membership_post_renew. If the second save() fails, reactivate() returns false even though the membership is already active, and renew listeners already ran with a stale cancellation date.
🛠️ Suggested fix
- $renewed = $this->renew(false, 'active');
+ $this->set_date_cancellation(null);
+ $renewed = $this->renew(false, Membership_Status::ACTIVE);
if ( ! $renewed) {
wu_log_add("membership-{$id}", sprintf('Membership reactivation failed for membership #%d: renewal returned false.', $id));
return false;
}
-
- $this->set_date_cancellation(null);
-
- $status = $this->save();
-
- if (is_wp_error($status)) {
- wu_log_add("membership-{$id}", sprintf('Membership reactivation failed for membership #%d: %s', $id, $status->get_error_message()));
-
- return false;
- }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/models/class-membership.php` around lines 2434 - 2449, The reactivation
flow does a second save after calling $this->renew(), which causes listeners to
run with the stale cancellation date and makes reactivate() return false even
when renewal already persisted; move the call to
$this->set_date_cancellation(null) so it runs before $this->renew(false,
'active') and remove the subsequent $this->save() / is_wp_error() block so
reactivation becomes a single write handled by renew(); update reactivate() to
rely on renew() to persist and only log/return based on the renew() result.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (5)
inc/models/class-membership.php (1)
2434-2449:⚠️ Potential issue | 🟠 MajorMake reactivation a single persistence step.
renew()already saves and fires renewal hooks. Clearingdate_cancellationin a secondsave()means renew listeners still see a cancelled membership, and a failure on that second write returnsfalseafter the membership is already active.Suggested fix
- $renewed = $this->renew(false, 'active'); + $this->set_date_cancellation(null); + $renewed = $this->renew(false, Membership_Status::ACTIVE); if ( ! $renewed) { wu_log_add("membership-{$id}", sprintf('Membership reactivation failed for membership #%d: renewal returned false.', $id)); return false; } - - $this->set_date_cancellation(null); - - $status = $this->save(); - - if (is_wp_error($status)) { - wu_log_add("membership-{$id}", sprintf('Membership reactivation failed for membership #%d: %s', $id, $status->get_error_message())); - - return false; - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/models/class-membership.php` around lines 2434 - 2449, Clear the membership's date_cancellation before invoking renew so the single save inside renew() persists the non-cancelled active state; specifically, call $this->set_date_cancellation(null) prior to $this->renew(false, 'active'), remove the subsequent $this->save() call and its is_wp_error($status) check/log path, and keep the existing logging/false return handling only for $renewed being false (use the symbols renew(), set_date_cancellation(), save(), and the membership reactivation log lines to locate the code to change).inc/managers/class-site-manager.php (1)
463-488:⚠️ Potential issue | 🟡 MinorKeep the blocked page actionable when no specific reactivation URL exists.
wu_blocked_site_reactivation_urlonly runs inside the cancelled-membership branch, and the template still tells the user to renew even when no URL was built. If the register page is missing or the site is blocked for another inactive state, this becomes a dead end.Suggested tweak
- $reactivation_url = ''; + $checkout_pages = \WP_Ultimo\Checkout\Checkout_Pages::get_instance(); + $reactivation_url = $checkout_pages->get_page_url('register') ?: ''; if ($membership && method_exists($membership, 'is_cancelled') && $membership->is_cancelled()) { - $checkout_pages = \WP_Ultimo\Checkout\Checkout_Pages::get_instance(); - $checkout_url = $checkout_pages->get_page_url('register'); + $checkout_url = $reactivation_url; if ($checkout_url) { $reactivation_url = add_query_arg( [ 'plan_id' => $membership->get_plan_id(), 'membership_id' => $membership->get_id(), ], $checkout_url ); - - /** - * Filters the reactivation URL shown on blocked sites. - * - * `@param` string $reactivation_url The reactivation checkout URL. - * `@param` \WP_Ultimo\Models\Membership $membership The cancelled membership. - * `@param` \WP_Ultimo\Models\Site $site The blocked site. - * - * `@since` 2.4.14 - */ - $reactivation_url = apply_filters('wu_blocked_site_reactivation_url', $reactivation_url, $membership, $site); } } + + $reactivation_url = apply_filters('wu_blocked_site_reactivation_url', $reactivation_url, $membership, $site); @@ - $html .= '<p>' . esc_html__('The subscription for this site has expired or been cancelled. To restore access, please renew your subscription.', 'ultimate-multisite') . '</p>'; - - if ( ! empty($reactivation_url)) { + if ( ! empty($reactivation_url)) { + $html .= '<p>' . esc_html__('The subscription for this site has expired or been cancelled. To restore access, please renew your subscription.', 'ultimate-multisite') . '</p>'; $html .= '<a class="wu-btn wu-btn-primary" href="' . esc_url($reactivation_url) . '">' . esc_html__('Renew your subscription', 'ultimate-multisite') . '</a>'; + } else { + $html .= '<p>' . esc_html__('This site is currently unavailable. Please log in for more details.', 'ultimate-multisite') . '</p>'; }Also applies to: 509-514
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/managers/class-site-manager.php` around lines 463 - 488, The template can show a "renew" action even when no reactivation URL is built because apply_filters('wu_blocked_site_reactivation_url') is only invoked inside the cancelled-membership branch; move or duplicate the filter call so $reactivation_url is passed through apply_filters('wu_blocked_site_reactivation_url', $reactivation_url, $membership, $site) in all cases (i.e., after you initialize $reactivation_url = '' and regardless of membership->is_cancelled()), so themes/plugins can supply an alternative URL when the checkout/register page is missing or the site is blocked for other inactive states; update the code paths where $reactivation_url is used (including the similar block around the other branch mentioned) to rely on the filtered value.inc/managers/class-payment-manager.php (1)
221-245:⚠️ Potential issue | 🟠 MajorOnly auto-redirect when there is one unambiguous cancelled membership.
Membership::is_active()does not covertrialing, so a customer with a current trial can still fall through here. Thebreakalso makes the redirect target depend onget_memberships()ordering when more than one cancelled membership exists.Suggested fix
- foreach ($memberships as $membership) { - if ($membership->is_active()) { - return; - } - } + $cancelled_memberships = []; + + foreach ($memberships as $membership) { + if ($membership->is_active() || \WP_Ultimo\Database\Memberships\Membership_Status::TRIALING === $membership->get_status()) { + return; + } + + if (method_exists($membership, 'is_cancelled') && $membership->is_cancelled()) { + $cancelled_memberships[] = $membership; + } + } /* * No active membership found. Look for a cancelled one. */ - $cancelled_membership = null; - - foreach ($memberships as $membership) { - if (method_exists($membership, 'is_cancelled') && $membership->is_cancelled()) { - $cancelled_membership = $membership; - - break; - } - } - - if ( ! $cancelled_membership) { + if (1 !== count($cancelled_memberships)) { return; } + + $cancelled_membership = $cancelled_memberships[0];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/managers/class-payment-manager.php` around lines 221 - 245, The current logic returns only if any Membership::is_active() is true but misses trialing statuses and picks the first cancelled membership found, making behavior order-dependent; update the checks so you treat trialing memberships as active (check method_exists($membership,'is_trialing') && $membership->is_trialing() alongside is_active()) and change the cancelled-membership logic to collect all cancelled entries from $memberships into an array (use method_exists($membership,'is_cancelled') && $membership->is_cancelled()), then only proceed when count($cancelled_memberships) === 1 and set $cancelled_membership = $cancelled_memberships[0]; otherwise return.inc/checkout/class-cart.php (2)
829-841:⚠️ Potential issue | 🟠 MajorRebuild reactivation carts from the membership’s actual products.
The empty-products path only adds
get_plan_id(). That drops attached add-ons and can also re-add the base plan instead of the membership’s current variation/billing period.Suggested fix
- if (empty($this->attributes->products)) { - $this->add_product($membership->get_plan_id()); + if (empty($this->attributes->products)) { + foreach ($membership->get_all_products() as $membership_product) { + $product = $membership_product['product']; + + if ($product) { + $this->add_product($product->get_id(), $membership_product['quantity']); + } + } } else { foreach ($this->attributes->products as $product_id) { $this->add_product($product_id); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/checkout/class-cart.php` around lines 829 - 841, The empty-products branch currently calls $this->add_product($membership->get_plan_id()) which drops add-ons and can add the wrong variation; instead rebuild the cart from the membership’s actual products: replace that single add_product call with iterating over the membership’s stored product list (e.g. $membership->get_products() or $membership->get_product_ids()) and call $this->add_product($product_id) for each entry so attached add-ons and the correct plan variation/billing period are preserved; keep the existing foreach path for when $this->attributes->products is present and ensure you reference $this->attributes->products, $this->add_product(), and $membership->get_plan_id()/get_products() in the fix.
800-847:⚠️ Potential issue | 🔴 CriticalRun the ownership guard before the reactivation early return.
This branch returns before the existing permission check. Passing another customer's cancelled
membership_idis enough to build a reactivation cart against that membership.Suggested fix
if ( ! $membership) { $this->errors->add('membership_not_found', __('The membership in question was not found.', 'ultimate-multisite')); return true; } + + if (empty($this->customer) || $this->customer->get_id() !== $membership->get_customer_id()) { + $this->errors->add('lacks_permission', __('You are not allowed to modify this membership.', 'ultimate-multisite')); + + return true; + } /* * Reactivation flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/checkout/class-cart.php` around lines 800 - 847, Run the ownership/permission check for $membership before performing the reactivation early return: ensure the same guard that verifies the current customer owns the membership (the check that adds errors via $this->errors->add when not permitted) is executed before the "if (method_exists($membership, 'is_cancelled') && $membership->is_cancelled())" branch returns true; either move that existing ownership check above this reactivation block or call it at the top of the block and abort (add the appropriate error and return) if the check fails, so you don't allow building a reactivation cart for another customer's cancelled membership.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@inc/checkout/class-cart.php`:
- Around line 829-841: The empty-products branch currently calls
$this->add_product($membership->get_plan_id()) which drops add-ons and can add
the wrong variation; instead rebuild the cart from the membership’s actual
products: replace that single add_product call with iterating over the
membership’s stored product list (e.g. $membership->get_products() or
$membership->get_product_ids()) and call $this->add_product($product_id) for
each entry so attached add-ons and the correct plan variation/billing period are
preserved; keep the existing foreach path for when $this->attributes->products
is present and ensure you reference $this->attributes->products,
$this->add_product(), and $membership->get_plan_id()/get_products() in the fix.
- Around line 800-847: Run the ownership/permission check for $membership before
performing the reactivation early return: ensure the same guard that verifies
the current customer owns the membership (the check that adds errors via
$this->errors->add when not permitted) is executed before the "if
(method_exists($membership, 'is_cancelled') && $membership->is_cancelled())"
branch returns true; either move that existing ownership check above this
reactivation block or call it at the top of the block and abort (add the
appropriate error and return) if the check fails, so you don't allow building a
reactivation cart for another customer's cancelled membership.
In `@inc/managers/class-payment-manager.php`:
- Around line 221-245: The current logic returns only if any
Membership::is_active() is true but misses trialing statuses and picks the first
cancelled membership found, making behavior order-dependent; update the checks
so you treat trialing memberships as active (check
method_exists($membership,'is_trialing') && $membership->is_trialing() alongside
is_active()) and change the cancelled-membership logic to collect all cancelled
entries from $memberships into an array (use
method_exists($membership,'is_cancelled') && $membership->is_cancelled()), then
only proceed when count($cancelled_memberships) === 1 and set
$cancelled_membership = $cancelled_memberships[0]; otherwise return.
In `@inc/managers/class-site-manager.php`:
- Around line 463-488: The template can show a "renew" action even when no
reactivation URL is built because
apply_filters('wu_blocked_site_reactivation_url') is only invoked inside the
cancelled-membership branch; move or duplicate the filter call so
$reactivation_url is passed through
apply_filters('wu_blocked_site_reactivation_url', $reactivation_url,
$membership, $site) in all cases (i.e., after you initialize $reactivation_url =
'' and regardless of membership->is_cancelled()), so themes/plugins can supply
an alternative URL when the checkout/register page is missing or the site is
blocked for other inactive states; update the code paths where $reactivation_url
is used (including the similar block around the other branch mentioned) to rely
on the filtered value.
In `@inc/models/class-membership.php`:
- Around line 2434-2449: Clear the membership's date_cancellation before
invoking renew so the single save inside renew() persists the non-cancelled
active state; specifically, call $this->set_date_cancellation(null) prior to
$this->renew(false, 'active'), remove the subsequent $this->save() call and its
is_wp_error($status) check/log path, and keep the existing logging/false return
handling only for $renewed being false (use the symbols renew(),
set_date_cancellation(), save(), and the membership reactivation log lines to
locate the code to change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a8989638-357b-402b-b009-c7acf893e1e6
📒 Files selected for processing (7)
inc/checkout/class-cart.phpinc/checkout/class-checkout.phpinc/class-wp-ultimo.phpinc/managers/class-payment-manager.phpinc/managers/class-site-manager.phpinc/models/class-membership.phpultimate-multisite.php
✅ Files skipped from review due to trivial changes (2)
- ultimate-multisite.php
- inc/checkout/class-checkout.php
🚧 Files skipped from review as they are similar to previous changes (1)
- inc/class-wp-ultimo.php
superdav42
added
the
needs-maintainer-review
Collaborator
|
Supervisor review — flagging for maintainer attention before merge. This is a valuable external contribution (reactivation flow + trial-skip revenue fix) but CodeRabbit identified 7 actionable issues including one critical security vulnerability that must be addressed before merge: Critical (security):
Major:
Minor:
Recommended action: The contributor should address the critical ownership check and the major issues before this is merge-ready. The trial-skip logic and the core reactivation flow are sound — this just needs the security and correctness fixes applied. aidevops.sh v3.6.97 plugin for OpenCode v1.3.0 with claude-sonnet-4-6 Overall, 2h 20m since this issue was created. |
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
2 participants
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.
Reactivation Flow for Cancelled Memberships (+ trial skip)
Hi David 👋 — this PR adds a complete self-service reactivation flow for users whose memberships have been cancelled, so they can re-subscribe without admin intervention, and fixes a revenue leak where cancelled users were getting a second free trial.
This branch is rebased directly onto current
upstream/main— all of your recent work (FrankenPHP, SSO GlotPress, password auto-generate, t524, etc.) is preserved untouched. The diff is purely additive: +354 lines / −19 lines across 5 core files.Status: Fully built, tested end-to-end with a real cancelled user, and currently running in production on kursopro.com as
2.4.13-beta.21while we wait for your review.The Problem
When a user's membership is cancelled,
Site_Manager::lock_site()blocks their site frontend. The blocked page tells them to "log in" — but:cart_typesupportsnew,retry,upgrade,downgradebut NOTreactivationreactivate()method on theMembershipmodel — onlyrenew()andcancel()check_pending_payments()ignores cancelled memberships — only checksstatus=pendinglock_site()showswp_die()with no actionable path — no checkout link, no renew button$0today. This is the critical revenue leak and the reason for the trial-skip commit.Redirect loop reproduced with a real cancelled user (membership #256):
Impact: Every user who cancels is permanently locked out. Zero self-service recovery. An admin has to manually reactivate each account. Lost revenue from users who genuinely want to resubscribe.
Why the 15-day trial must be skipped on reactivation
This is the most important part to review — the business reasoning, not just the code:
$0today and get 15 more free days — effectively letting anyone cancel + re-signup on the same day to extend their trial indefinitely.Total a Pagar Hoy: $0to cancelled users reactivating. Confirmed revenue leak in production.The fix enforces this at the core level in 4 lines so the whole stack (core → addon → WC Subscriptions → Stripe) honors it with zero ambiguity.
The trial skip (4 lines in
Cart::get_billing_start_date())Why this is enough — no addon changes needed:
ultimate-multisite-woocommercereads$cart->has_trial()from coreCart::has_trial()depends onCart::get_billing_start_date()null→has_trial() === false→ addon skips the_subscription_trial_lengthmeta → WC Subscriptions shows no trial and charges the full amount immediatelyFull problem chain we traced end-to-end before finding the root cause:
/renovar/→ clicks "Pagar y reactivar"cart_type='reactivation'(new cart type added in this PR)$cart->has_trial()→ internally callsget_billing_start_date()get_billing_start_date()iterated all products and returned the smallest trial → reactivation carts were inheriting the 15-day trial from the plan producthas_trial() === true→ computed$days_to_start = 15ultimate-multisite-woocommerce/inc/gateways/class-woocommerce-gateway.php:736added_subscription_trial_length = 15to the auto-created WC productTotal a Pagar Hoy: $0What Changed (5 core files, all additive)
inc/models/class-membership.phpreactivate()+is_cancelled()inc/checkout/class-cart.phpbuild_from_membership()→cart_type='reactivation'+ trial skip inget_billing_start_date()inc/checkout/class-checkout.phpsite_url/site_title/template_selectionfields for reactivation carts +maybe_create_site()returns existing siteinc/managers/class-payment-manager.phpmaybe_redirect_cancelled_membership()onwp_logininc/managers/class-site-manager.phpwp_die()dead-end inlock_site()with a friendly HTML page showing a "Renew your subscription" buttonwp_die()was a dead end with no path to paymentAll new code is gated behind
cart_type === 'reactivation'ormethod_exists($membership, 'is_cancelled') && $membership->is_cancelled()checks. Normalnew/retry/upgrade/downgradeflows are completely untouched. Thekeep_until_livedemo-mode logic you recently added inlock_site()is preserved exactly as-is.New Hooks (6)
wu_membership_pre_reactivatewu_membership_post_reactivatewu_blocked_site_reactivation_urlwu_blocked_site_templatewu_blocked_site_support_urlwu_cancelled_membership_redirect_urlBackward Compatibility
active/pending/expired/trialingmembershipscart_type === 'reactivation'— normal subscriptions still get their trial as beforemethod_exists()guards used when callingis_cancelled()on the membership object so the code degrades gracefully if applied to older forksEnd-to-End Test Results (real cancelled user, production)
Plan: KURSO INICIAL
$69/month, status:cancelled, user:pierinaferreiro(membership #256)cart_type='reactivation'Total a Pagar Hoyshows full price ($69) — NOT$0$69 → $55first month,$69/moafterphp -lclean on all 5 filesDeployment Status
kursopro.comproduction as2.4.13-beta.21✅kursopro.com 200,/login/ 200,/planes/ 200,/renovar/ 302(correct redirect for unauthenticated) ✅debug.log: clean of any errors from the deployed files ✅We're running our fork in production while we wait for your review. Once you merge and release upstream, we'll realign to your tag.
What I need from you
Cart::get_billing_start_date()is the critical one for the revenue leak.2.4.13-beta.21(or any version scheme you prefer).Zero urgency on your side — everything is working for our users right now. Take the time you need to review. I just needed to ship this because cancelled users were piling up with no self-service path back in.
Thanks David 🙏
Summary by CodeRabbit
New Features
Checkout Improvements
Blocked Site Experience
Version