728 lines
57 KiB
Markdown
728 lines
57 KiB
Markdown
# Subscription Module — Audit Report
|
|
|
|
**Date:** 2026-06-01
|
|
**Scope:** WooNooW plugin — Product Subscriptions module
|
|
**Auditor:** AI-assisted code review (full trace of `woonoow/` folder)
|
|
**Reference:** [Prior audit 2026-01-29](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/.agent/reports/subscription-flow-audit-2026-01-29.md) — 2 critical / 5 warning / 4 info issues, all critical & warning fixed.
|
|
**Re-audit pass (2026-06-01, same day):** verified each finding against the actual implementation. Result below.
|
|
|
|
---
|
|
|
|
## 1. Executive Summary
|
|
|
|
The subscription module is **functionally complete** and follows the documented pattern from the prior audit. This deeper audit (UI/UX, cron, notifications, settings, product, order, payments) surfaces findings the prior audit did not cover — most are **gaps & opportunities**, several are **defects** that will break under realistic usage. **Re-verification on 2026-06-01 confirmed most findings as real and corrected one (C3) that misrepresented the implementation state.**
|
|
|
|
| Severity | Count |
|
|
|---|---|
|
|
| 🔴 Critical (defects that break flows) | 1 |
|
|
| 🟠 High (UX/data integrity issues) | 6 |
|
|
| 🟡 Medium (gaps, missing features) | 4 |
|
|
| 🔵 Low / opportunity | 2 |
|
|
| ❌ Resolved on re-verification (already implemented, finding withdrawn) | 1 (C3) |
|
|
| **Withdrawn on review** | 1 (C2) |
|
|
| **Total** | **14 active + 2 withdrawn** |
|
|
|
|
### Top issues to fix first
|
|
|
|
1. **Per-gateway capability declaration is unimplemented** — §9 is currently *aspirational only*. The system uses `method_exists($gateway, 'process_subscription_renewal_payment')` PHP introspection. There is no capability table, no admin UI, no kill switch. A working schema/settings path exists (C3 resolved — see below), so §9 should be built on that same pattern. Effort: M.
|
|
2. **Customer `early renew` doesn't explain the consequence** (C1) — paying early moves the next billing date forward, but the order-pay page only shows a static timeline snapshot, not the new projected next-payment-date. Effort: S.
|
|
3. **Failed renewal orders are not dedup-protected** (H3) — `renew()` IN clause at `SubscriptionManager.php:527` excludes `failed`/`wc-failed`. If a renewal failed, clicking "Renew Early" creates a second order. Effort: XS.
|
|
4. **Guest checkout silently drops subscriptions** (H6) — `subscription_add_to_cart_text` doesn't check `is_user_logged_in()`; `create_from_order` returns false silently for guests. Effort: S.
|
|
5. **`max_pause_count` is enforced in PHP but not surfaced in the response** (H2) — `enrich_subscription` never includes it, so the customer sees "Times Paused: 3" with no warning before hitting the limit and getting a 500. Effort: S.
|
|
|
|
### C3 is resolved — was a false alarm
|
|
|
|
The original C3 finding stated there was no Settings page for the subscription module. **This is wrong.** A generic `Settings/ModuleSettings.tsx` already exists, the route `/settings/modules/:moduleId` is wired, `useModuleSettings` is implemented, the `/modules/{id}/schema` endpoint serves the registered schema, and `SchemaForm` renders the fields. `SubscriptionSettings::init()` is called from `SubscriptionModule::init()`, and the 11 fields (default_status, button_text_subscribe, button_text_renew, allow_customer_cancel, allow_customer_pause, max_pause_count, renewal_retry_enabled, renewal_retry_days, expire_after_failed_attempts, send_renewal_reminder, reminder_days_before) are all visible and editable. The runtime works, the merchant can change values, the API persists them. C3 is removed from the critical list.
|
|
|
|
---
|
|
|
|
## 2. Module Architecture Map
|
|
|
|
### 2.1 Files in scope
|
|
|
|
| Layer | File | Role |
|
|
|---|---|---|
|
|
| **PHP core** | [SubscriptionManager.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionManager.php) | CRUD, renewals, lifecycle |
|
|
| | [SubscriptionModule.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionModule.php) | Bootstrap, product meta, hooks, notifications |
|
|
| | [SubscriptionScheduler.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionScheduler.php) | Cron (renewals, expiry, reminders) |
|
|
| | [SubscriptionSettings.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/SubscriptionSettings.php) | Settings schema (defaults only) |
|
|
| | [ModuleRegistry.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Core/ModuleRegistry.php) | Module registration |
|
|
| **API** | [SubscriptionsController.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Api/SubscriptionsController.php) | Admin + customer REST endpoints |
|
|
| | [ModuleSettingsController.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Api/ModuleSettingsController.php) | Generic `/modules/{id}/schema` and `/modules/{id}/settings` (used by C3 resolution) |
|
|
| | [OrdersController.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Api/OrdersController.php) | Embeds `related_subscription` in order detail |
|
|
| | [CheckoutController.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Api/CheckoutController.php) | Embeds `subscription` in `order-pay` response |
|
|
| | [ProductsController.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Api/ProductsController.php) | Persists subscription product meta |
|
|
| **Admin SPA** | [Subscriptions/index.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/routes/Subscriptions/index.tsx) | List page (table + mobile cards) |
|
|
| | [Subscriptions/Detail.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/routes/Subscriptions/Detail.tsx) | Admin detail view |
|
|
| | [Orders/Detail.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/routes/Orders/Detail.tsx) | Renders "Related Subscription" block |
|
|
| | [Products/.../GeneralTab.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/routes/Products/partials/tabs/GeneralTab.tsx) | Subscription checkbox + period/interval/trial/signup-fee inputs |
|
|
| | [Settings/ModuleSettings.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/routes/Settings/ModuleSettings.tsx) | **Generic schema-driven settings page (resolves C3)** |
|
|
| | [hooks/useModuleSettings.ts](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/hooks/useModuleSettings.ts) | Settings read/write hook (resolves C3) |
|
|
| **Customer SPA** | [Account/Subscriptions.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/customer-spa/src/pages/Account/Subscriptions.tsx) | List page |
|
|
| | [Account/SubscriptionDetail.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/customer-spa/src/pages/Account/SubscriptionDetail.tsx) | Customer detail (pause/resume/cancel/early renew) |
|
|
| | [components/SubscriptionTimeline.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/customer-spa/src/components/SubscriptionTimeline.tsx) | Visual timeline component |
|
|
| | [OrderPay/index.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/customer-spa/src/pages/OrderPay/index.tsx) | Manual renewal payment page |
|
|
| | [Account/components/AccountLayout.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/customer-spa/src/pages/Account/components/AccountLayout.tsx) | Sidebar with subscription link |
|
|
| **Notifications** | [Notifications/EmailRenderer.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Core/Notifications/EmailRenderer.php) | Resolves subscription variables in emails |
|
|
| | [Notifications/TemplateProvider.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Core/Notifications/TemplateProvider.php) | Lists available subscription email variables |
|
|
| **Cross-module** | [Modules/Licensing/LicenseManager.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Licensing/LicenseManager.php) | License validation gates on subscription status |
|
|
| | [Compat/NavigationRegistry.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Compat/NavigationRegistry.php) | Admin sidebar |
|
|
|
|
### 2.2 Data model
|
|
|
|
- **`wp_woonoow_subscriptions`** — main table (id, user_id, order_id, product_id, variation_id, status, billing_period, billing_interval, recurring_amount, start_date, trial_end_date, next_payment_date, end_date, last_payment_date, payment_method, payment_meta, cancel_reason, pause_count, failed_payment_count, reminder_sent_at).
|
|
- **`wp_woonoow_subscription_orders`** — join table (id, subscription_id, order_id, order_type ENUM 'parent'|'renewal'|'switch'|'resubscribe').
|
|
|
|
### 2.3 Status flow
|
|
|
|
```
|
|
pending ──► active ──► pending-cancel ──► cancelled
|
|
│
|
|
├──► on-hold (paused, payment_failed) ──► resumed ──► active
|
|
└──► expired (end_date_reached, max_failed)
|
|
```
|
|
|
|
### 2.4 Notification events (8 customer + 2 staff)
|
|
|
|
`pending_cancel`, `cancelled`, `expired`, `paused`, `resumed`, `renewal_failed`, `renewal_payment_due`, `renewal_reminder`, plus `cancelled_admin` and `renewal_failed_admin`.
|
|
|
|
---
|
|
|
|
## 3. Critical Defects (🔴)
|
|
|
|
### C1. Customer "early renew" silently resets the billing cycle
|
|
|
|
**File:** [SubscriptionManager.php:706-718](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionManager.php#L706-L718)
|
|
|
|
**Flow:**
|
|
|
|
1. Customer is on a monthly plan, paid May 1, next renewal June 1.
|
|
2. On May 20, customer clicks "Renew Early" — order is created for $X.
|
|
3. After payment, `handle_renewal_success()` is called.
|
|
4. Code computes `next_payment = calculate_next_payment_date($base_date, ...)` where `$base_date = $now` (May 20) **unless** `next_payment_date > $now`, in which case `$base_date = next_payment_date` (June 1).
|
|
|
|
```php
|
|
$base_date = $now;
|
|
if ($subscription->next_payment_date && $subscription->next_payment_date > $now) {
|
|
$base_date = $subscription->next_payment_date; // OK path
|
|
}
|
|
$next_payment = self::calculate_next_payment_date($base_date, ...);
|
|
```
|
|
|
|
**Defect:** This *partially* handles the case, but the renewal order is added to `subscription_orders` with `order_type='renewal'`, AND the next billing is shifted by the early-renew amount. Customer now has:
|
|
- A new renewal order (5/20) for the *upcoming* period
|
|
- A second scheduled next-payment-date 6/1 that **will** bill again immediately
|
|
|
|
Two outcomes depending on `now`:
|
|
- **If `now >= next_payment_date`** (e.g., late payment): no early issue — uses `now` correctly.
|
|
- **If `now < next_payment_date`** (early renew): `$base_date = next_payment_date` and `next_payment` is bumped forward correctly. ✅ Actually OK.
|
|
|
|
**Re-read the code carefully:** the conditional `next_payment_date > $now` correctly uses the future date as the base. The defect is subtler:
|
|
|
|
When `next_payment_date <= $now` (a **late** renewal via early renew button — possible if customer waits past their cycle start), the code uses `$now` as base, so `next_payment` = `now + 1 month` — which **shortens** the cycle (the customer paid 5/20-6/20 but the next charge is 5/20+1mo = 6/20, with `now` being e.g. 5/22). Acceptable.
|
|
|
|
**Real defect:** `handle_renewal_success` resets `last_payment_date = current_time('mysql')` regardless of whether the actual *gateway* charge completed. If the gateway returns `true` from `process_subscription_renewal_payment` but the renewal was flagged as `manual`, the cron later calls `handle_renewal_success` again, double-counting. The early-renew path goes:
|
|
|
|
```
|
|
$payment_result === 'manual' → status 'manual' → return early (no handle_renewal_success)
|
|
```
|
|
|
|
So manual early renew does NOT call `handle_renewal_success` — the schedule shift is deferred to manual payment confirmation via `on_order_status_changed`. ✅ The original concern was unfounded; the path is actually correct.
|
|
|
|
**However**, there is a **real defect** with the "early renew" customer flow:
|
|
|
|
The `customer_renew` REST endpoint at [SubscriptionsController.php:407-434](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Api/SubscriptionsController.php#L407-L434) calls `SubscriptionManager::renew()` which creates a renewal order, then redirects the customer to `/order-pay/{id}`. Once they pay, the renewal is treated as a normal renewal, so:
|
|
|
|
- If the customer pays the early renewal order, `next_payment_date` is set to `current + 1 month` (via the conditional above), giving them **two paid periods back-to-back** with the next charge 1 month from "now" (which is the early renew date) — this is the **expected** SaaS behavior.
|
|
- BUT the customer is *not informed* anywhere in the UI that paying early **moves their next billing date forward** by the early amount. The order-pay page ([OrderPay/index.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/customer-spa/src/pages/OrderPay/index.tsx)) only shows the static `SubscriptionTimeline` snapshot, not what the new next date will be.
|
|
|
|
**Severity: 🔴 Critical — UX expectation violation.** Even if the math is right, customers will be surprised and request refunds.
|
|
|
|
**Fix:** Show the *projected* next billing date on the order-pay page when the order is a renewal. Compute it client-side as `next_payment_date` if in future, else `now + period`.
|
|
|
|
---
|
|
|
|
### C2. Removed — was a misframed finding
|
|
|
|
**Status: Withdrawn on review.** This finding is not a subscription-module defect in the current state of the system.
|
|
|
|
The original C2 argued that the renewal `set_address` from parent ([SubscriptionManager.php:609-614](file:///Users/dwindown/Local%20Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionManager.php#L609-L614)) silently copies a stale address. On review, that framing was wrong.
|
|
|
|
**What the system actually does today:**
|
|
|
|
- The checkout does not ask for an address. Every order is created without one.
|
|
- For **virtual / downloadable** subscription products: no address is needed. The renewal `set_address` call writes an empty/default address, but no shipping line is generated, no merchant ever sees an issue, and the customer never interacts with an address. ✅ Correct by virtue of the product type.
|
|
- For **physical** subscription products: physical subscriptions **cannot be sold** through the current checkout, because the checkout never asks for a shipping address. The renewal `set_address` code is **unreachable** for physical subscriptions in the current state.
|
|
|
|
**What the renewal flow should do once the checkout supports physical subscriptions:**
|
|
|
|
1. At the original order's checkout, the customer fills in the address. The parent order stores it.
|
|
2. On renewal, copy the address from the parent order to the renewal order. This is the correct default — the customer chose to renew, and the parent order is the most recent customer-confirmed address.
|
|
3. On the renewal order-pay page, surface the address with a clear "Is your address still the same? [Change]" prompt. The customer can correct it before paying.
|
|
4. If the customer changes the address on the renewal, persist the change to the renewal order. (Optional: also write it back to the parent order or to the customer's default address, but that is a checkout-level concern, not subscription-level.)
|
|
|
|
**Why this is not a defect today:**
|
|
|
|
- The renewal `set_address` from parent is not "wrong" — it is the only sensible default in the absence of a current customer-confirmed address on the renewal. Pulling from `WC_Customer::get_default_address()` would actually be **worse** in the renewal context: the customer may not have a default address, and the parent order is by definition the most recent address the customer provided for *this* subscription.
|
|
- The "stale address" risk on renewal is real but is already handled by the natural UX: the customer sees the address on the order-pay page and can change it before paying. This is the same trust model as checkout itself.
|
|
|
|
**What to do with this finding:**
|
|
|
|
- Remove C2 from the critical list.
|
|
- Keep the existing `set_address` code unchanged.
|
|
- File a **forward-looking note** in `docs/SUBSCRIPTION_ADDRESS_POLICY.md` (to be written when the checkout address step is added) describing the contract: parent-order address is the default; customer can change on the renewal order-pay page; physical products require the checkout to collect an address in the first place.
|
|
- The only subscription-side code change that may be useful when the checkout supports physical products: surface the address on the renewal order-pay page with the "is your address still the same?" prompt. That is a UX change, not a backend defect fix.
|
|
|
|
**The address question is the checkout's responsibility, not the subscription module's.** The subscription module's `create_renewal_order` is doing the right thing — copying from the parent, which is the only signal it has.
|
|
|
|
---
|
|
|
|
### C3. Resolved — was a false alarm (settings page IS implemented)
|
|
|
|
**Status: Resolved on re-verification (2026-06-01).** The original C3 finding stated there was no Settings page for the subscription module. That claim is wrong.
|
|
|
|
**What is actually implemented today:**
|
|
|
|
- **Generic page exists:** [admin-spa/src/routes/Settings/ModuleSettings.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/routes/Settings/ModuleSettings.tsx) handles ANY module with a schema. It is mounted at `/settings/modules/:moduleId` in `AppRoutes.tsx:230`.
|
|
- **Hook exists:** [admin-spa/src/hooks/useModuleSettings.ts](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/hooks/useModuleSettings.ts) reads `/modules/{id}/settings` and posts to `/modules/{id}/settings`.
|
|
- **Schema endpoint exists:** `ModuleSettingsController::get_schema` (registers `GET /woonoow/v1/modules/{module_id}/schema` at [ModuleSettingsController.php:67-71](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Api/ModuleSettingsController.php#L67-L71)) serves the schema registered via the `woonoow/module_settings_schema` filter.
|
|
- **Form renderer exists:** `SchemaForm` renders text / number / toggle / select fields from the schema declaratively.
|
|
- **Schema is registered:** [SubscriptionSettings.php:18](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/SubscriptionSettings.php#L18) registers the filter; [SubscriptionModule.php:25](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionModule.php#L25) calls `SubscriptionSettings::init()` on bootstrap.
|
|
|
|
**What the merchant sees:** navigate to Settings → Modules → Subscription, and all 11 fields (`default_status`, `button_text_subscribe`, `button_text_renew`, `allow_customer_cancel`, `allow_customer_pause`, `max_pause_count`, `renewal_retry_enabled`, `renewal_retry_days`, `expire_after_failed_attempts`, `send_renewal_reminder`, `reminder_days_before`) are visible, editable, and persisted.
|
|
|
|
**Why this finding is wrong:** the original audit looked for a per-module page (`Settings/Subscription.tsx`) and didn't find one. But the system uses a *generic* schema-driven page that works for any module — including the subscription module. The capability is there; the audit missed it.
|
|
|
|
**Lesson for future audits:** the per-module page pattern is no longer the convention. Look for `ModuleSettings.tsx` + `SchemaForm` + `/modules/{id}/schema` first.
|
|
|
|
---
|
|
|
|
## 4. High-Impact UX/Data Issues (🟠)
|
|
|
|
### H1. "Renew Now" admin button does not consider gateway availability
|
|
|
|
**File:** [admin-spa/src/routes/Subscriptions/Detail.tsx:228-237](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/routes/Subscriptions/Detail.tsx#L228-L237)
|
|
|
|
The admin "Renew Now" button always calls `SubscriptionManager::renew()` which falls through to manual payment if no auto-debit gateway. The admin is not shown a confirmation that the renewal will produce a *pending* order requiring customer payment. They will click the button, see "Renewed" (because `handle_renewal_success` was called on auto-debit success OR no clear feedback on manual), and not realize the customer now has a pending order to pay.
|
|
|
|
**Fix:** After clicking "Renew Now", show the resulting order's payment URL / status to the admin.
|
|
|
|
---
|
|
|
|
### H2. Customer detail: "Times Paused" displayed but `max_pause_count` is not shown, and no warning when reached
|
|
|
|
**File:** [customer-spa/src/pages/Account/SubscriptionDetail.tsx:257-259](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/customer-spa/src/pages/Account/SubscriptionDetail.tsx#L257-L259)
|
|
|
|
The customer can see they've paused N times, but not how many pauses they have left. When the limit is hit, the API returns a generic 500 `pause_failed` — the customer sees a confusing error. The button should be **disabled** with a tooltip when the limit is reached.
|
|
|
|
**Fix:** Add `max_pause_count` and `pauses_remaining` to the enriched response; show on the detail page; disable button when 0 remaining.
|
|
|
|
---
|
|
|
|
### H3. `on-hold` subscription can be "renewed" creating a duplicate order
|
|
|
|
**File:** [SubscriptionManager.php:512-533](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionManager.php#L512-L533)
|
|
|
|
The duplicate prevention check uses `p.post_status IN ('wc-pending', 'pending', 'wc-on-hold', 'on-hold')`. But `failed` and `wc-failed` orders are *not* excluded — and the prior failed order still counts as a "renewal" link. The customer detail page (line 137-140) looks for `pending`, `wc-pending`, `on-hold`, `wc-on-hold`, `failed`, `wc-failed` to surface the "Pay Now" button — but the backend `renew()` only prevents duplicates for pending/on-hold. If a customer's renewal order failed last night, and they click "Renew Early" today, a *second* order is created.
|
|
|
|
**Severity: 🟠 High** — duplicate orders on retry, leading to confused customers and double-billing risk.
|
|
|
|
**Fix:** Add `'wc-failed', 'failed'` to the duplicate-prevention IN clause.
|
|
|
|
---
|
|
|
|
### H4. Renewal order product line uses the *current* product price, not the subscription's stored recurring amount
|
|
|
|
**File:** [SubscriptionManager.php:600-606](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionManager.php#L600-L606)
|
|
|
|
```php
|
|
$product = wc_get_product($subscription->variation_id ?: $subscription->product_id);
|
|
if ($product) {
|
|
$renewal_order->add_product($product, 1, [
|
|
'total' => $subscription->recurring_amount, // ✅ uses stored amount
|
|
'subtotal' => $subscription->recurring_amount,
|
|
]);
|
|
}
|
|
```
|
|
|
|
This *is* correct — it uses the stored `recurring_amount`, not the current product price. ✅
|
|
|
|
**However**, `recalculate_next_payment_date` does not consider **product-level price changes mid-cycle**. The customer's stored `recurring_amount` is the original price. If the admin changes the product price, the customer is grandfathered — which is probably intended, but **not documented in any setting**. No toggle to "re-sync with product price" or "always bill current price."
|
|
|
|
**Severity: 🟠 High (UX clarity)** — admin changes the product price and the next renewal silently uses the old price; surprised admin.
|
|
|
|
**Fix:** Add a setting `price_sync_on_renewal` with options: 'use_stored' (default), 'use_current_product_price'. Document in product meta tooltip.
|
|
|
|
---
|
|
|
|
### H5. Manual renewal email is sent on every cron tick, not gated by `pending` order existence
|
|
|
|
**File:** [SubscriptionManager.php:684-689](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionManager.php#L684-L689)
|
|
|
|
When a renewal returns `'manual'`, the system fires `woonoow/subscription/renewal_payment_due` once per call. If the same due subscription is processed again (e.g., a future hourly tick after `next_payment_date` falls behind), and there's still a pending order, the same notification fires again, **because `process_renewal_payment` doesn't check for an existing pending order first** — `renew()` does (line 521-533), but that's before the existing-pending check returns `'existing'` status.
|
|
|
|
Wait — re-reading: `renew()` returns the existing order for `'existing'` status and does **not** call `process_renewal_payment` or fire the notification. ✅ So this is actually safe.
|
|
|
|
**However**, if a `manual` renewal was created and never paid, the next cron tick sees the subscription as `on-hold` (not `active`) — so `get_due_renewals()` excludes it. The customer gets no reminder that they have an outstanding order. The `pending-cancel` path is similar — when `next_payment_date <= now`, `check_expirations` flips it to `cancelled`, but no email is queued with a "your pending order was cancelled" notice.
|
|
|
|
**Severity: 🟠 High (revenue leakage)** — abandoned-cart-like behavior on unpaid renewals.
|
|
|
|
**Fix:** Add a daily cron that finds `on-hold` subscriptions with pending renewal orders older than 24h and re-sends the `renewal_payment_due` email (or auto-cancels after N days).
|
|
|
|
---
|
|
|
|
### H6. `create_from_order` rejects guest orders silently
|
|
|
|
**File:** [SubscriptionManager.php:107-110](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionManager.php#L107-L110)
|
|
|
|
```php
|
|
if (!$user_id) {
|
|
// Guest orders not supported for subscriptions
|
|
return false;
|
|
}
|
|
```
|
|
|
|
But the **product page still shows "Subscribe Now"** to guests, who can complete checkout as guest, after which **no subscription is created and no error is shown**. The customer thinks they have a subscription, the order is normal.
|
|
|
|
**Severity: 🟠 High — broken expectation for guest purchases.** The "Subscribe" button should be hidden for guests, or guest checkout should be blocked for subscription products, or the customer should be auto-converted to a user.
|
|
|
|
**Fix:** Add a check in the `subscription_add_to_cart_text` filter to *not* change button text for guest users, or force a login redirect for subscription products in cart.
|
|
|
|
---
|
|
|
|
## 5. Medium Gaps (🟡)
|
|
|
|
### M1. Variable products: subscription meta is on parent only
|
|
|
|
**File:** [SubscriptionModule.php:120-177](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionModule.php#L120-L177)
|
|
|
|
The admin SPA form ([GeneralTab.tsx:546-630](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/routes/Products/partials/tabs/GeneralTab.tsx#L546-L630)) saves `_woonoow_subscription_*` on the parent product. When `create_from_order` is called, it reads:
|
|
|
|
```php
|
|
$billing_period = get_post_meta($product_id, '_woonoow_subscription_period', true) ?: 'month';
|
|
```
|
|
|
|
This is the *parent* product ID, not the variation. All variations of a variable subscription product share the same period. The admin cannot have e.g. "Small = monthly, Large = yearly" or "License 1-year vs 5-year" as variations.
|
|
|
|
**Fix:** Add variation-level meta overrides; `create_from_order` should look up variation meta first, then fall back to parent.
|
|
|
|
---
|
|
|
|
### M2. `customer_renew` endpoint has no `force_immediate` flag for ad-hoc admin actions
|
|
|
|
**File:** [SubscriptionsController.php:407-434](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Api/SubscriptionsController.php#L407-L434)
|
|
|
|
The customer "early renew" creates a new order. The admin "Renew Now" does the same. Neither can be used to **trigger an immediate charge against the customer's saved payment method** (i.e., a real "charge now and renew" button). The current behavior is "create a new order that the customer then pays manually" — confusing.
|
|
|
|
**Fix:** Add `?charge_now=true` param to admin renew that calls `process_renewal_payment` with `force = true`, skips the manual fallback.
|
|
|
|
---
|
|
|
|
### M3. No bulk actions on admin subscription list
|
|
|
|
**File:** [admin-spa/src/routes/Subscriptions/index.tsx:158-176](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/routes/Subscriptions/index.tsx#L158-L176)
|
|
|
|
The list page has a checkbox column with `selectedIds` state — but **the checkboxes don't trigger any bulk action**. There's no bulk cancel, bulk export (CSV), or bulk remind button. The select-all UI is dead.
|
|
|
|
**Fix:** Add a bulk-action toolbar (e.g., "Cancel selected", "Send renewal reminder", "Export CSV") and a corresponding REST endpoint.
|
|
|
|
---
|
|
|
|
### M4. No search field on admin list
|
|
|
|
**File:** [admin-spa/src/routes/Subscriptions/index.tsx](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/admin-spa/src/routes/Subscriptions/index.tsx)
|
|
|
|
The `SubscriptionManager::get_all` accepts a `search` parameter ([SubscriptionManager.php:282](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionManager.php#L282)) — but the admin list never passes it. With hundreds of subscriptions, an admin cannot search by customer name/email/product.
|
|
|
|
**Fix:** Wire the search input (currently missing) to the `search` query param.
|
|
|
|
---
|
|
|
|
## 6. Low / Opportunities (🔵)
|
|
|
|
### O1. Notification variables: missing `payment_method_title`, `billing_schedule`, `customer_id`
|
|
|
|
**File:** [TemplateProvider.php:328-345](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Core/Notifications/TemplateProvider.php#L328-L345)
|
|
|
|
The schema lists variables, but **the `EmailRenderer` mapping at lines 343-353** does not include:
|
|
- `payment_method_title` (customer-facing)
|
|
- `billing_schedule` (e.g., "Every 1 month")
|
|
- `customer_id` (admin-facing)
|
|
- `store_email` (declared in schema but not mapped)
|
|
- `my_account_url` (declared in schema but not mapped)
|
|
|
|
**Fix:** Add these to the `EmailRenderer` mapping so email templates can use them.
|
|
|
|
---
|
|
|
|
### O2. Two `.bak.php` files in production code path
|
|
|
|
**File:** [TemplateProvider.bak.php](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Core/Notifications/TemplateProvider.bak.php)
|
|
|
|
The `.bak` file is in the `Notifications/` directory — depending on autoloader, this may be auto-loaded or skipped. The subscription variables in the `.bak` (line 339-348) reference different keys than the live one. Should be removed or moved to `tests/fixtures/`.
|
|
|
|
**Fix:** Delete or relocate.
|
|
|
|
---
|
|
|
|
## 7. UI/UX Observations (not classified as defects)
|
|
|
|
| Observation | Location |
|
|
|---|---|
|
|
| Status badge uses `bg-amber-100` for `pending` in admin, `bg-yellow-100` in customer — inconsistent. | admin/customer SPAs |
|
|
| Mobile card view (admin) shows product name truncated without a "view product" link. | Subscriptions/index.tsx:300-321 |
|
|
| `SubscriptionTimeline` component has hard-coded English labels (`Started`, `Payment Due`, `Every X months`) — no `__()` calls. Breaks i18n for the 2 languages supported per `I18N_IMPLEMENTATION_GUIDE.md`. | SubscriptionTimeline.tsx |
|
|
| Customer detail page has no SEO head except `<SEOHead title="Subscription #X">` — OK but no `og:image` from `product_image`. | SubscriptionDetail.tsx:164 |
|
|
| Admin "Renew Now" doesn't ask for confirmation, but "Cancel" does. | Subscriptions/Detail.tsx:228-247 |
|
|
| `failed_payment_count` is shown in admin detail (good), but not in the customer detail — they don't know they have a retry coming. | admin vs customer |
|
|
| `last_payment_date` is shown in admin only, not customer. | admin vs customer |
|
|
| Order pay page always shows `Loading order details...` if `key` is missing — no helpful 403. | OrderPay/index.tsx:133 |
|
|
|
|
---
|
|
|
|
## 8. Cron Behavior Audit
|
|
|
|
| Hook | Schedule | Purpose | Issues |
|
|
|---|---|---|---|
|
|
| `woonoow_process_subscription_renewals` | hourly | Auto-process due renewals | No batch limit; no admin notice on failure; no lock guard against overlapping runs |
|
|
| `woonoow_check_expired_subscriptions` | daily | Mark end-date-reached as `expired`; finalize `pending-cancel` | No email to customer when their pending-cancel finally cancels |
|
|
| `woonoow_send_renewal_reminders` | daily | Send reminder N days before `next_payment_date` | Uses `last_payment_date` to gate, but `last_payment_date` only updates on **success** — if all retries fail, the gate fails open and may re-send for the same cycle |
|
|
|
|
**Cron-related risks:**
|
|
1. **No `wp_remote_get` lock** to prevent concurrent runs (WP-Cron can double-fire on slow sites).
|
|
2. **`send_renewal_reminders`** query at [SubscriptionScheduler.php:183-192](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionScheduler.php#L183-L192) uses a complex OR clause that may not be correct when `last_payment_date` IS NULL (initial trial): the `(last_payment_date IS NULL AND reminder_sent_at < start_date)` branch. New trial subscriptions may get reminders *before* trial ends.
|
|
3. **`schedule_retry`** at line 246-262 updates `next_payment_date` to the retry date — but the `get_due_renewals()` query compares `next_payment_date <= now`, so the retried subscription will be picked up on the next hourly tick. ✅
|
|
|
|
---
|
|
|
|
## 9. Payment Gateway Integration — Revised Direction (per-gateway capability)
|
|
|
|
> **Status as of 2026-06-01 (re-verified):** this section is **still aspirational**. No code, no schema, no settings entry, no UI, no capability helper exists. Zero references to `subscription_auto_renew`, `GatewayCapabilities`, `gateway_capability`, or `woonoow_gateway_subscription_capabilities` were found anywhere in the plugin. The current system still relies entirely on `method_exists($gateway, 'process_subscription_renewal_payment')` PHP introspection at `SubscriptionManager.php:667-674`.
|
|
>
|
|
> However, **the settings infrastructure to build this is now in place** (see C3 resolution): a generic `ModuleSettings` page reads a registered schema and persists via `/modules/{id}/settings`. §9 can be built on that same pattern, which is why this is still the most important long-term fix — but it is a feature to build, not a bug to remove.
|
|
|
|
### 9.1 Current state (problem)
|
|
|
|
The current code at [SubscriptionManager.php:667-674](file:///Users/dwindown/Local%20Sites/woonoow/app/public/wp-content/plugins/woonoow/includes/Modules/Subscription/SubscriptionManager.php#L667-L674) detects auto-debit capability via PHP introspection:
|
|
|
|
```php
|
|
if (method_exists($gateway, 'process_subscription_renewal_payment')) {
|
|
$result = $gateway->process_subscription_renewal_payment($order, $subscription);
|
|
...
|
|
}
|
|
```
|
|
|
|
This has three problems:
|
|
|
|
1. **Capability is invisible to the merchant.** The admin has no way to see which gateways are declared to support subscription auto-renew, no way to override that declaration, and no way to know that "Stripe is enabled" does not mean "Stripe will charge renewals."
|
|
2. **The default is unsafe.** A gateway without the method silently falls through to manual payment. The system "works," but the merchant believes auto-debit is happening and customers are surprised.
|
|
3. **No per-gateway override is possible.** A merchant who has a custom Stripe wrapper that *does* support auto-debit cannot declare it; a merchant using stock Stripe with no wrapper cannot override the assumption that it works.
|
|
|
|
### 9.2 Recommended direction: per-gateway capability declaration
|
|
|
|
Instead of inferring capability from PHP method existence, store a **gateway capability table** that the merchant (or WooNooW defaults) controls explicitly. The system then computes effective behavior at renewal time.
|
|
|
|
#### Shape of the declaration
|
|
|
|
A per-gateway record, with safe defaults:
|
|
|
|
```php
|
|
// Conceptual. Storage could be wp_option, custom table, or gateway registration API.
|
|
$capabilities = [
|
|
'paypal' => ['subscription_auto_renew' => true],
|
|
'stripe' => ['subscription_auto_renew' => true], // only with WooNooW Stripe adapter
|
|
'dodo' => ['subscription_auto_renew' => true], // only with WooNooW Dodo adapter
|
|
'tripay' => ['subscription_auto_renew' => false], // VA/QRIS — no recurring
|
|
'midtrans' => ['subscription_auto_renew' => false], // VA/QRIS/e-wallet — no recurring
|
|
'xendit' => ['subscription_auto_renew' => false], // even CC re-auth required
|
|
];
|
|
```
|
|
|
|
**Default for any unknown gateway: `false`.** This is the safe default — Indonesian-style and global-style gateways without a WooNooW adapter are treated as manual-renewal-only.
|
|
|
|
#### Why per-gateway, not a site-level "billing mode" toggle
|
|
|
|
A site-level "manual vs auto" toggle asks the merchant to understand a concept ("billing mode") that does not actually exist in their head. The merchant thinks in terms of **payment gateways**. A checkbox next to each gateway in WooCommerce → Settings → Payments is data the merchant already knows.
|
|
|
|
Additionally:
|
|
|
|
- Different merchants use different gateways. A site-level toggle forces a single behavior even when the merchant runs two gateways (one auto-capable, one not) for different products.
|
|
- The capability is a property of the **integration**, not of the **store**. The merchant did not choose "manual mode" — they chose Tripay, and Tripay is a manual gateway.
|
|
- The capability can change as WooNooW ships new adapters. A site-level toggle would be set once and forgotten; a per-gateway table updates as adapters ship.
|
|
|
|
#### What the system does with the declaration
|
|
|
|
At renewal time, the system checks: **the active gateway for this subscription** (stored in `payment_method`) has `subscription_auto_renew = true`.
|
|
|
|
- **Yes, supported** → attempt auto-debit via the gateway's `process_subscription_renewal_payment` (the contract is unchanged, just gated by a positive declaration). On success, `handle_renewal_success`. On failure, fall through to manual renewal and notify the customer.
|
|
- **No, not supported** → skip auto-debit, create a manual renewal order, send `renewal_payment_due` email. No silent failure.
|
|
|
|
If the gateway is unknown to the capability table, the default is `false` — same as explicit "not supported."
|
|
|
|
#### Optional site-level override (default off)
|
|
|
|
Add one secondary toggle for the rare merchant who wants to force manual even when the gateway supports auto (e.g., legal, regulatory, or business reasons):
|
|
|
|
- `force_manual_renewal`: off by default. When on, all renewals become manual regardless of gateway capability. Useful as a "kill switch."
|
|
|
|
This is **not** the primary control. It is an override.
|
|
|
|
### 9.3 What the merchant sees
|
|
|
|
In WooCommerce → Settings → Payments, each gateway row should show a "Supports subscription auto-renewal" indicator. For example:
|
|
|
|
| Gateway | Status |
|
|
|---|---|
|
|
| PayPal (WooCommerce addon) | ✅ Supports auto-renew |
|
|
| Stripe (WooCommerce addon) | ✅ Supports auto-renew |
|
|
| Dodo (WooCommerce addon) | ✅ Supports auto-renew |
|
|
| Tripay Payment (WooCommerce addon) | ❌ Manual renewal only |
|
|
| Midtrans (WooCommerce addon) | ❌ Manual renewal only |
|
|
| Xendit (WooCommerce addon) | ❌ Manual renewal only (even credit card) |
|
|
|
|
A gateway with no entry in the capability table shows ❌ by default and the merchant can flip it on if they have a custom adapter (advanced setting, off by default).
|
|
|
|
### 9.4 What the customer sees (on the order-pay page and renewal emails)
|
|
|
|
The renewal messaging must match the actual behavior, not the marketing claim:
|
|
|
|
- For an auto-renew gateway: "Your subscription will renew automatically on [date] using your saved payment method."
|
|
- For a manual gateway: "Your subscription is up for renewal. Please complete the payment to continue." with a clear CTA to pay.
|
|
- The order-pay page should also show the **next payment date** after this payment completes (the audit's C1 finding applies here too).
|
|
|
|
### 9.5 The Indonesian gateway reality
|
|
|
|
For Indonesian payment gateways (Tripay, Midtrans, Xendit, Doku, and the VA/QRIS/e-wallet channels of any gateway), the default `subscription_auto_renew` must be **`false`**. Specifically:
|
|
|
|
- VA (virtual account): no recurring charge capability.
|
|
- QRIS: typically a one-time merchant-presented QR, no customer-mandate.
|
|
- E-wallets (GoPay, OVO, DANA, ShopeePay): require customer re-authentication for each charge.
|
|
- Credit card on Indonesian gateways: even when the card is tokenized, recurring charges typically require the customer to re-authenticate (BI/PCI-DSS regulatory constraint). The merchant cannot assume the stored token can be charged without the customer's active consent.
|
|
|
|
If a merchant has a special integration (e.g., a specific subscription product on Xendit with a tokenized card and a recurring billing add-on), they can flip the toggle for that gateway. But the default must be manual.
|
|
|
|
### 9.6 Implementation outline (for the AI agent)
|
|
|
|
1. **Capability storage.** A new `wp_option('woonoow_gateway_subscription_capabilities', [...])` keyed by gateway ID, with the safe defaults above. Add a filter `woonoow_gateway_subscription_capabilities` so adapters can self-register.
|
|
2. **Capability lookup helper.** `WooNooW\Modules\Subscription\GatewayCapabilities::supports_auto_renew(string $gateway_id): bool` — single source of truth.
|
|
3. **Renewal flow integration.** In `SubscriptionManager::process_renewal_payment`, before checking `method_exists`, call the capability helper. If `false`, skip the auto-debit attempt entirely and go straight to manual.
|
|
4. **Admin UI.** Render the capability indicator in the gateway list (admin SPA) and on the subscription detail page next to "Payment Method" (e.g., "Stripe — auto-renew enabled" vs "Tripay — manual renewal only").
|
|
5. **Customer messaging.** Pass a boolean `gateway_supports_auto_renew` to the order-pay response and the renewal emails, so the UI can choose the right wording.
|
|
6. **Kill switch.** A single `force_manual_renewal` site setting. Default off.
|
|
7. **Documentation.** A new `docs/SUBSCRIPTION_GATEWAY_CAPABILITIES.md` listing the default capabilities and explaining how to add custom adapters.
|
|
|
|
### 9.7 Migration / no migration
|
|
|
|
This is a behavioral improvement, not a schema change. Existing subscriptions keep their `payment_method` value. The capability table is consulted at renewal time, not retroactively.
|
|
|
|
### 9.8 Recommendation (replaces prior "ship a Stripe adapter" suggestion)
|
|
|
|
Do **not** ship a Stripe adapter as a first-class example. The value of a generic example adapter is low because each gateway's recurring billing is different. Instead:
|
|
|
|
- Ship the **capability table** with safe defaults.
|
|
- Ship a **gateway-integration guide** (`docs/SUBSCRIPTION_GATEWAY_CAPABILITIES.md`) that documents the `process_subscription_renewal_payment` contract.
|
|
- Let third-party gateway authors (or the merchant's developer) implement adapters per gateway.
|
|
- The merchant-visible UX works correctly for any gateway, supported or not, because the fallback to manual is explicit.
|
|
|
|
---
|
|
|
|
## 10. Cross-Module Integration
|
|
|
|
### Licensing
|
|
|
|
- `LicenseManager::validate_license` calls `get_order_subscription_status($license['order_id'])` at line 340-343 — if the subscription is not `active` or `pending-cancel`, the license is rejected with `subscription_inactive`.
|
|
- ✅ Works correctly; the licensing flow depends on the subscription status being accurate.
|
|
- ⚠️ But: when a subscription is **cancelled by customer at period end** (pending-cancel → cancelled), the license is still valid until the cancellation date. This is good UX, but admin should be able to see the relationship in the license detail.
|
|
|
|
### Affiliate
|
|
|
|
- No integration. An affiliate who referred a customer who later subscribes does **not** receive renewal commissions. The `subscription_renewal` order is not tagged with the referral.
|
|
- This is a known gap (Affiliate Module report doesn't mention subscriptions).
|
|
|
|
---
|
|
|
|
## 11. Documentation Drift
|
|
|
|
| File | Says | Reality |
|
|
|---|---|---|
|
|
| [FEATURE_ROADMAP.md:299-363](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/FEATURE_ROADMAP.md#L299-L363) | "Status: Planning" | Module is **fully implemented**; update to "Shipped" |
|
|
| FEATURE_ROADMAP.md lists `customer_id` column | actually `user_id` in schema | Drift |
|
|
| `.agent/plans/subscription-module.md` | Original design | Now stale (e.g., `reminder_sent_at` was added later) |
|
|
|
|
---
|
|
|
|
## 12. Test Coverage
|
|
|
|
**No subscription-specific tests** found in [tests/](file:///Users/dwindown/Local Sites/woonoow/app/public/wp-content/plugins/woonoow/tests/). Only generic schema/parity/page tests exist. The subscription module's complex state machine (status transitions, retry logic, renewal math) is untested.
|
|
|
|
**Recommendation:** Add `tests/Subscription/` with at minimum:
|
|
- `SubscriptionManagerTest.php`: state machine coverage (pending → active → on-hold → resumed, etc.)
|
|
- `SubscriptionSchedulerTest.php`: cron logic with frozen time
|
|
- `SubscriptionsControllerTest.php`: REST endpoint auth and validation
|
|
- `e2e/subscription-checkout.test.ts`: full buy → renew → cancel flow
|
|
|
|
---
|
|
|
|
## 13. Recommended Fix Priority
|
|
|
|
| # | Issue | Severity | Effort | Priority |
|
|
|---|---|---|---|---|
|
|
| 1 | C1 — Early renew UX clarity (show projected next-payment-date on order-pay page) | 🔴 | S | **P0 — this week** |
|
|
| 2 | H3 — Failed orders bypass dedup (add `'wc-failed', 'failed'` to IN clause at `SubscriptionManager.php:527`) | 🟠 | XS | **P0 — this week** |
|
|
| 3 | H6 — Guest checkout silently drops subscription (gate `subscription_add_to_cart_text` on `is_user_logged_in`) | 🟠 | S | **P0 — this week** |
|
|
| 4 | H2 — `max_pause_count` not surfaced in enriched response (add to `enrich_subscription`, disable button in customer detail) | 🟠 | S | **P0 — this week** |
|
|
| 5 | §9 — Per-gateway capability declaration (NEW FEATURE: capability table + settings UI + filter) | 🔴 (architectural) | M | **P1 — this sprint** |
|
|
| 6 | H1 — Admin "Renew Now" feedback (show resulting order URL) | 🟠 | S | P2 |
|
|
| 7 | H4 — Price sync on renewal (add `price_sync_on_renewal` setting) | 🟠 | M | P2 |
|
|
| 8 | H5 — Unpaid renewal recovery (daily cron for `on-hold` with pending order > 24h) | 🟠 | M | P2 |
|
|
| 9 | M1 — Variation-level subscription meta | 🟡 | M | P3 |
|
|
| 10 | M2 — `?charge_now=true` for admin renew | 🟡 | S | P3 |
|
|
| 11 | M3 — Bulk actions on admin subscription list | 🟡 | M | P3 |
|
|
| 12 | M4 — Search field on admin subscription list | 🟡 | S | P3 |
|
|
| 13 | O1 — Missing email variables (`payment_method_title`, `billing_schedule` in subscription block) | 🔵 | XS | P4 |
|
|
| 14 | O2 — Delete `TemplateProvider.bak.php` | 🔵 | XS | P4 |
|
|
| 15 | Doc drift — `FEATURE_ROADMAP.md` (status planning→shipped; column `customer_id`→`user_id`) | 🔵 | XS | P4 |
|
|
| 16 | Test coverage — `tests/Subscription/` | — | L | P4 |
|
|
| — | C2 — **Withdrawn** (not a subscription-module concern) | — | — | Defer to checkout work |
|
|
| — | C3 — **Resolved.** Settings page is implemented (generic schema-driven). | — | — | Done |
|
|
|
|
**Effort key:** XS < 1h, S = 1-4h, M = 1-2 days, L = 3+ days
|
|
|
|
### Why P0 is now 4 small items, not 3 big ones
|
|
|
|
The original P0 was "build §9, build the settings page, fix the order-pay UX." The settings page is **done** — that work is reusable for §9. The remaining P0 work is four small UX-correctness fixes that can be shipped in a single afternoon:
|
|
|
|
- C1: ~2-4h to add a projected-date line to `OrderPay/index.tsx` (compute client-side from `subscription.billing_period`/`billing_interval`).
|
|
- H3: ~30min — add two strings to an IN clause.
|
|
- H6: ~1-2h — early-return in `subscription_add_to_cart_text` for guests, plus a checkout-side guard or friendly error.
|
|
- H2: ~2-3h — add `max_pause_count` and `pauses_remaining` to `enrich_subscription`, conditional disable in `SubscriptionDetail.tsx`.
|
|
|
|
§9 itself is the largest remaining work, but it is now in P1 because the settings infrastructure is reusable. §9.6 (the implementation outline) is unchanged; it now correctly reads as "do this on top of the existing module settings pattern."
|
|
|
|
### 10.2 Address policy — explicitly not a subscription-module concern
|
|
|
|
Earlier drafts of this audit proposed two address-related recommendations: (1) a `needs_shipping()` gate inside `create_renewal_order`, and (2) a site-level address collection mode. **Both are withdrawn.**
|
|
|
|
- The `needs_shipping()` gate is unnecessary. For virtual products, the empty/default address on the renewal is harmless by virtue of the product type. The existing code is already correct.
|
|
- A site-level address collection mode is the wrong layer. The address question is a **checkout-level** concern. Once the checkout collects an address for physical products, the renewal should copy it from the parent and let the customer change it on the order-pay page.
|
|
|
|
**What the subscription module should do today:** nothing. The renewal `set_address` from parent is correct given the current checkout.
|
|
|
|
**What the subscription module should do when the checkout supports physical products:** surface the parent order's address on the renewal order-pay page with a "Is your address still the same? [Change]" prompt. This is a UX change, not a backend defect fix.
|
|
|
|
The address question does not belong in the subscription module's settings. It belongs in the checkout.
|
|
|
|
---
|
|
|
|
## 14. What's Working Well (positive findings)
|
|
|
|
- The state machine handles edge cases (trial, signup-fee, fixed-length, failed payments).
|
|
- The duplicate-prevention check in `renew()` correctly returns existing pending orders.
|
|
- The reminder `reminder_sent_at` DB column (prior audit fix) is properly indexed and used.
|
|
- `handle_renewal_success` correctly sets status to `active` (prior audit fix).
|
|
- The notification event registry cleanly separates customer and admin events.
|
|
- The product form conditionally shows the subscription block only when the module is enabled.
|
|
- The customer account sidebar hides the subscription link when the module is disabled.
|
|
- The order-pay page shows the `SubscriptionTimeline` for renewal orders — nice UX touch.
|
|
- The "Pay Now (#id)" button on customer detail for unpaid renewals is a great recovery path.
|
|
- Hooks are well-named and consistent (`woonoow/subscription/<event>`).
|
|
- The renewal flow **already has the right hook points** (`woonoow_pre_process_subscription_payment` and `woonoow_process_subscription_payment`) to plug the per-gateway capability check into. No architectural rewrite needed.
|
|
|
|
---
|
|
|
|
## 15. Summary of Revised Direction
|
|
|
|
The two material changes from the original draft of this audit:
|
|
|
|
### Payment: per-gateway capability, not a site-level billing mode
|
|
|
|
The original draft of §9 recommended documenting the gateway contract and shipping a sample adapter. The revised direction (§9) is to introduce an **explicit, merchant-visible capability declaration per gateway** with safe defaults (`false` for any gateway without a known adapter, `false` for all Indonesian-style VA/QRIS/e-wallet gateways, `true` for gateways with a working WooNooW adapter).
|
|
|
|
The system then computes effective behavior at renewal time from the **intersection** of (the gateway's declared capability) and (the gateway's actual `process_subscription_renewal_payment` implementation). A site-level "force manual" kill switch exists as a secondary override, default off.
|
|
|
|
The merchant thinks in payment gateways, not in "billing modes." A per-gateway checkbox next to each payment method in WooCommerce → Settings → Payments is data the merchant already has. A site-level toggle is a concept they have to learn.
|
|
|
|
### Address: not a subscription-module concern
|
|
|
|
**Withdrawn entirely.** On re-review, the renewal `set_address` from parent is the correct default for any product type. The original C2 finding, the `needs_shipping()` gate, and the site-level address-mode proposal are all withdrawn.
|
|
|
|
The right model is:
|
|
|
|
- **Virtual product** → checkout shows no address fields → no address anywhere → no problem.
|
|
- **Physical product** → checkout requires address at first order → parent order stores it → on renewal, copy from parent and surface "Is your address still the same? [Change]" on the order-pay page.
|
|
|
|
The address question is a **checkout-level** concern, not a subscription-module concern. The subscription module's `create_renewal_order` is doing the right thing — copying from the parent, which is the only signal it has. The only forward-looking UX change is to surface the address on the renewal order-pay page once the checkout supports it.
|
|
|
|
### What this means for the audit's existing findings
|
|
|
|
- **C2 is withdrawn** (see finding body). The renewal `set_address` is correct.
|
|
- **C1** (early renew UX) is unchanged in spirit; the order-pay page needs to show the projected next-payment-date.
|
|
- **C3** (settings page missing) is unchanged; the settings page is still the most visible P0.
|
|
- **H1** (admin "Renew Now" feedback) becomes more important once the gateway capability is explicit — the admin needs to see "this renewal will be a manual order because the gateway is Tripay."
|
|
|
|
---
|
|
|
|
## 16. Appendix: Full File Inventory
|
|
|
|
### PHP backend
|
|
- `includes/Modules/Subscription/SubscriptionManager.php` (894 lines)
|
|
- `includes/Modules/Subscription/SubscriptionModule.php` (564 lines)
|
|
- `includes/Modules/Subscription/SubscriptionScheduler.php` (264 lines)
|
|
- `includes/Modules/SubscriptionSettings.php` (110 lines)
|
|
- `includes/Api/SubscriptionsController.php` (502 lines)
|
|
- `includes/Api/ModuleSettingsController.php` (210+ lines, generic settings read/write/schema)
|
|
- `includes/Core/ModuleRegistry.php` (subscription block at line 68-82)
|
|
- `includes/Compat/NavigationRegistry.php` (lines 262-284)
|
|
- `includes/Core/Notifications/EmailRenderer.php` (lines 339-376)
|
|
- `includes/Core/Notifications/TemplateProvider.php` (lines 240-345)
|
|
- `includes/Core/Notifications/TemplateProvider.bak.php` ⚠️ **still present, should be removed** (O2)
|
|
|
|
### Frontend (TSX)
|
|
- `admin-spa/src/routes/Subscriptions/index.tsx` (505 lines)
|
|
- `admin-spa/src/routes/Subscriptions/Detail.tsx` (456 lines)
|
|
- `admin-spa/src/routes/Orders/Detail.tsx` (related_subscription block 320-345)
|
|
- `admin-spa/src/routes/Products/partials/tabs/GeneralTab.tsx` (subscription block 546-630)
|
|
- `admin-spa/src/routes/Settings/ModuleSettings.tsx` (149 lines, **generic schema-driven settings page**)
|
|
- `admin-spa/src/hooks/useModuleSettings.ts` (46 lines, settings read/write)
|
|
- `customer-spa/src/pages/Account/Subscriptions.tsx` (244 lines)
|
|
- `customer-spa/src/pages/Account/SubscriptionDetail.tsx` (377 lines)
|
|
- `customer-spa/src/components/SubscriptionTimeline.tsx` (86 lines)
|
|
- `customer-spa/src/pages/OrderPay/index.tsx` (subscription block 35-44, 142-162)
|
|
- `customer-spa/src/pages/Account/components/AccountLayout.tsx` (line 53, 66)
|
|
|
|
### Docs
|
|
- `.agent/reports/subscription-flow-audit-2026-01-29.md` (prior audit)
|
|
- `.agent/plans/subscription-module.md` (original design)
|
|
- `FEATURE_ROADMAP.md` (stale)
|
|
- `HOOKS_REGISTRY.md` (subsections at 104-117, 215, 222, 261, 285-289, 658)
|
|
- `MODULE_SYSTEM_IMPLEMENTATION.md`
|
|
|
|
---
|
|
|
|
## 17. Re-verification matrix (2026-06-01)
|
|
|
|
Each finding in this audit was re-checked against the actual implementation today. The table is the source of truth for "documented vs. implemented."
|
|
|
|
| # | Finding | Original severity | Re-verified status | Notes |
|
|
|---|---|---|---|---|
|
|
| C1 | Early-renew UX lacks projected next-payment-date | 🔴 | ✅ **Confirmed** | `OrderPay/index.tsx` only renders static `SubscriptionTimeline` snapshot; no projection |
|
|
| C2 | Stale address on renewal | 🔴 | ✅ **Withdrawn** | Address is checkout's responsibility, not subscription's. Already resolved in this document. |
|
|
| C3 | Settings page missing | 🔴 | ❌ **Withdrawn — false alarm** | Generic `ModuleSettings.tsx` + `/modules/{id}/schema` + `useModuleSettings` + `SchemaForm` all exist and work. See C3 resolution section. |
|
|
| H1 | Admin "Renew Now" lacks feedback | 🟠 | ✅ Confirmed | Real |
|
|
| H2 | `max_pause_count` not surfaced | 🟠 | ✅ Confirmed | `enrich_subscription()` (lines 439-500) does not add `max_pause_count` or `pauses_remaining` |
|
|
| H3 | Failed orders bypass dedup | 🟠 | ✅ Confirmed | Line 527 IN clause: `('wc-pending', 'pending', 'wc-on-hold', 'on-hold')` — no `failed` |
|
|
| H4 | Renewal uses stored price; no "use current" toggle | 🟠 | ✅ Confirmed (mixed) | Code is correct (uses stored); the missing toggle is the actual fix |
|
|
| H5 | Unpaid renewal not re-notified | 🟠 | ✅ Confirmed | Real revenue-leakage path |
|
|
| H6 | Guest checkout silently drops | 🟠 | ✅ Confirmed | `subscription_add_to_cart_text` does not check `is_user_logged_in`; `create_from_order` returns false silently |
|
|
| M1 | Variable product meta on parent only | 🟡 | ✅ Confirmed | `create_from_order` reads parent product meta; GeneralTab has no variation UI |
|
|
| M2 | No `force_immediate` flag | 🟡 | ✅ Confirmed | Neither endpoint has it |
|
|
| M3 | No bulk actions on admin list | 🟡 | ✅ Confirmed | `selectedIds` state exists but no toolbar |
|
|
| M4 | No search field on admin list | 🟡 | ✅ Confirmed | No search input element |
|
|
| O1 | Missing email variables | 🔵 | ⚠️ **Partially confirmed** | `payment_method_title`, `billing_schedule` not in subscription block (lines 343-353). `my_account_url` is mapped in a different branch. |
|
|
| O2 | `.bak.php` in production | 🔵 | ✅ Confirmed | `TemplateProvider.bak.php` 11464 bytes, present |
|
|
| §9 | Per-gateway capability | 🔴 (architectural) | ⚠️ **Confirmed aspirational, no work done** | Zero references in codebase. Settings infrastructure now reusable, so it can be built on existing patterns. |
|
|
|
|
---
|
|
|
|
**End of report.**
|