diff --git a/SUBSCRIPTION_MODULE_AUDIT.md b/SUBSCRIPTION_MODULE_AUDIT.md new file mode 100644 index 0000000..31fa82c --- /dev/null +++ b/SUBSCRIPTION_MODULE_AUDIT.md @@ -0,0 +1,727 @@ +# 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 `` β€” 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/`). +- 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.** diff --git a/admin-spa/src/routes/Settings/ModuleSettings.tsx b/admin-spa/src/routes/Settings/ModuleSettings.tsx index 912a550..84a3d52 100644 --- a/admin-spa/src/routes/Settings/ModuleSettings.tsx +++ b/admin-spa/src/routes/Settings/ModuleSettings.tsx @@ -10,6 +10,7 @@ import { Button } from '@/components/ui/button'; import { ArrowLeft } from 'lucide-react'; import { __ } from '@/lib/i18n'; import { DynamicComponentLoader } from '@/components/DynamicComponentLoader'; +import { GatewayCapabilityMatrix as SubscriptionGatewayCapabilitiesSection } from './Modules/Subscription/GatewayCapabilityMatrix'; interface Module { id: string; @@ -143,6 +144,8 @@ export default function ModuleSettings() { )} + + {moduleId === 'subscription' && } ); } diff --git a/admin-spa/src/routes/Settings/Modules/Subscription/GatewayCapabilityMatrix.tsx b/admin-spa/src/routes/Settings/Modules/Subscription/GatewayCapabilityMatrix.tsx new file mode 100644 index 0000000..8246424 --- /dev/null +++ b/admin-spa/src/routes/Settings/Modules/Subscription/GatewayCapabilityMatrix.tsx @@ -0,0 +1,173 @@ +import React, { useMemo, useState } from 'react'; +import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; +import { api } from '@/lib/api'; +import { toast } from 'sonner'; +import { SettingsCard } from '../../components/SettingsCard'; +import { ToggleField } from '../../components/ToggleField'; +import { Button } from '@/components/ui/button'; +import { __ } from '@/lib/i18n'; + +interface GatewayRow { + id: string; + title: string; + description: string; + enabled: boolean; + default: boolean; + override: boolean | null; + auto_renew: boolean; + forced_manual: boolean; +} + +interface CapabilityResponse { + gateways: GatewayRow[]; + kill_switch: boolean; +} + +type OverrideMap = Record; + +/** + * Gateway Capability Matrix + * + * Renders one row per WC payment gateway with a per-gateway auto-renew + * toggle. Overrides are persisted via POST /subscriptions/gateway-capabilities. + * The kill switch is a separate field already on the standard settings form + * (force_manual_renewal); when on, every row is rendered as forced-manual + * and the per-gateway toggles are disabled. + */ +export const GatewayCapabilityMatrix: React.FC = () => { + const queryClient = useQueryClient(); + const [overrides, setOverrides] = useState({}); + + const { data, isLoading } = useQuery({ + queryKey: ['subscription', 'gateway-capabilities'], + queryFn: async () => { + const r = await api.get('/subscriptions/gateway-capabilities'); + return r as CapabilityResponse; + }, + }); + + const save = useMutation({ + mutationFn: async (payload: OverrideMap) => { + return api.post('/subscriptions/gateway-capabilities', { overrides: payload }); + }, + onSuccess: () => { + toast.success(__('Gateway capabilities saved')); + setOverrides({}); + queryClient.invalidateQueries({ queryKey: ['subscription', 'gateway-capabilities'] }); + }, + onError: (e: any) => { + toast.error(e?.response?.data?.message ?? __('Failed to save')); + }, + }); + + const rows = useMemo(() => data?.gateways ?? [], [data]); + + const effectiveFor = (row: GatewayRow): boolean => { + if (row.forced_manual) return false; + if (row.id in overrides) return overrides[row.id] === true; + return row.auto_renew; + }; + + const dirty = Object.keys(overrides).length > 0; + + if (isLoading) { + return ( + +
{__('Loading gateways…')}
+ + ); + } + + return ( + + {rows.length === 0 ? ( +
{__('No payment gateways available.')}
+ ) : ( +
+ {rows.map((row) => { + const eff = effectiveFor(row); + const overrideState = row.id in overrides ? overrides[row.id] : row.override; + return ( +
+
+
+ {row.title} + {!row.enabled && ( + + {__('Site disabled')} + + )} + {row.forced_manual && ( + + {__('Forced manual (kill switch)')} + + )} + {overrideState === null && ( + + {__('Default')}: {row.default ? __('Auto-renew') : __('Manual')} + + )} + {overrideState !== null && ( + + {__('Override')} + + )} +
+ {row.description && ( +

{row.description}

+ )} +

{row.id}

+
+
+ { + const currentEffective = row.auto_renew; + if (checked === currentEffective) { + setOverrides((p) => { + const n = { ...p }; + delete n[row.id]; + return n; + }); + } else { + setOverrides((p) => ({ ...p, [row.id]: checked })); + } + }} + label={eff ? __('Auto-renew') : __('Manual')} + /> +
+
+ ); + })} +
+ )} + +
+ + {dirty && ( + + )} +
+ + ); +}; diff --git a/admin-spa/src/routes/Subscriptions/Detail.tsx b/admin-spa/src/routes/Subscriptions/Detail.tsx index 0035804..57470bb 100644 --- a/admin-spa/src/routes/Subscriptions/Detail.tsx +++ b/admin-spa/src/routes/Subscriptions/Detail.tsx @@ -118,8 +118,18 @@ async function fetchSubscription(id: string) { return res; } -async function subscriptionAction(id: number, action: string, reason?: string) { - const res = await api.post(`/subscriptions/${id}/${action}`, { reason }); +async function subscriptionAction(id: number, action: string, reason?: string, extra?: Record) { + let path = `/subscriptions/${id}/${action}`; + if (extra) { + const usp = new URLSearchParams(); + for (const [k, v] of Object.entries(extra)) { + if (v == null) continue; + usp.set(k, String(v)); + } + const qs = usp.toString(); + if (qs) path += `?${qs}`; + } + const res = await api.post(path, { reason }); return res; } @@ -158,11 +168,70 @@ export default function SubscriptionDetail() { }, }); + // H1 β€” Renew is special: the response carries { order_id, status } so the admin + // can see whether a payment URL needs to be sent to the customer. + const renewMutation = useMutation({ + mutationFn: () => subscriptionAction(parseInt(id!), 'renew'), + onSuccess: (res: any) => { + queryClient.invalidateQueries({ queryKey: ['subscription', id] }); + queryClient.invalidateQueries({ queryKey: ['subscriptions'] }); + const orderId = res?.order_id; + const status = res?.status; + if (status === 'complete') { + toast.success(__(`Renewed successfully (order #${orderId}). Payment captured automatically.`)); + } else if (status === 'manual') { + toast.success( + __(`Renewal order #${orderId} created. The customer must complete payment manually β€” open the order to send a payment link.`), + { duration: 8000 } + ); + } else if (status === 'existing') { + toast.info(__(`Order #${orderId} is already pending payment β€” using the existing order.`)); + } else { + toast.success(__(`Renewed (order #${orderId}).`)); + } + }, + onError: (error: Error) => { + toast.error(error.message); + }, + }); + + // M2 β€” "Charge Now" is the same renew endpoint with `?charge_now=true`. It bypasses + // the per-gateway capability gate so the auto-debit path is attempted even on + // normally-manual gateways. On failure the order is marked failed (no manual + // fallback) so the admin sees the charge could not be processed. + const chargeNowMutation = useMutation({ + mutationFn: () => subscriptionAction(parseInt(id!), 'renew', undefined, { charge_now: 'true' }), + onSuccess: (res: any) => { + queryClient.invalidateQueries({ queryKey: ['subscription', id] }); + queryClient.invalidateQueries({ queryKey: ['subscriptions'] }); + const orderId = res?.order_id; + const status = res?.status; + if (status === 'complete') { + toast.success(__(`Charged successfully (order #${orderId}). The subscription has been renewed.`)); + } else if (status === 'existing') { + toast.info(__(`Order #${orderId} is already pending payment β€” using the existing order.`)); + } else { + toast.warning(__(`Charge attempt completed with status "${status || 'unknown'}" (order #${orderId}).`)); + } + }, + onError: (error: Error) => { + toast.error(error.message); + }, + }); + const handleAction = (action: string) => { if (action === 'cancel') { setShowCancelDialog(true); return; } + if (action === 'renew') { + renewMutation.mutate(); + return; + } + if (action === 'charge_now') { + chargeNowMutation.mutate(); + return; + } actionMutation.mutate({ action }); }; @@ -229,10 +298,21 @@ export default function SubscriptionDetail() { + )} + {subscription.status === 'active' && ( + )} {subscription.can_cancel && ( diff --git a/admin-spa/src/routes/Subscriptions/index.tsx b/admin-spa/src/routes/Subscriptions/index.tsx index 6886860..f360f82 100644 --- a/admin-spa/src/routes/Subscriptions/index.tsx +++ b/admin-spa/src/routes/Subscriptions/index.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useCallback } from 'react'; +import React, { useState, useEffect, useCallback, useRef } from 'react'; import { useQuery, useMutation, useQueryClient, keepPreviousData } from '@tanstack/react-query'; import { useNavigate, Link } from 'react-router-dom'; import { Repeat, MoreHorizontal, Play, Pause, XCircle, RefreshCw, Eye, Filter, Package } from 'lucide-react'; @@ -93,23 +93,49 @@ export default function SubscriptionsIndex() { const initial = getQuery(); const [page, setPage] = useState(Number(initial.page ?? 1) || 1); const [status, setStatus] = useState(initial.status || undefined); + // M4 β€” Search input is held in a local "raw" state for snappy typing, then + // committed to `committedSearch` after a 300ms debounce. We key the React + // Query cache on the committed value so the debounce actually coalesces + // requests, not just defers re-renders. + const [rawSearch, setRawSearch] = useState((initial as any).search || ''); + const [committedSearch, setCommittedSearch] = useState((initial as any).search || ''); const [isRefreshing, setIsRefreshing] = useState(false); const [showCancelDialog, setShowCancelDialog] = useState(false); const [cancelTargetId, setCancelTargetId] = useState(null); + const [showBulkCancelDialog, setShowBulkCancelDialog] = useState(false); const perPage = 20; + // Debounce rawSearch β†’ committedSearch. 300ms is the sweet spot for "feels + // instant" vs "don't fire on every keystroke". + const debounceRef = useRef | null>(null); + useEffect(() => { + if (debounceRef.current) clearTimeout(debounceRef.current); + debounceRef.current = setTimeout(() => { + setCommittedSearch(rawSearch.trim()); + setPage(1); + }, 300); + return () => { + if (debounceRef.current) clearTimeout(debounceRef.current); + }; + }, [rawSearch]); + useEffect(() => { setPageHeader(__('Subscriptions')); return () => clearPageHeader(); }, [setPageHeader, clearPageHeader]); useEffect(() => { - setQuery({ page, status }); - }, [page, status]); + setQuery({ page, status, search: committedSearch || undefined }); + }, [page, status, committedSearch]); const q = useQuery({ - queryKey: ['subscriptions', { status, page }], - queryFn: () => api.get('/subscriptions', { status, page, per_page: perPage }), + queryKey: ['subscriptions', { status, page, search: committedSearch }], + queryFn: () => api.get('/subscriptions', { + status, + page, + per_page: perPage, + search: committedSearch || undefined, + }), placeholderData: keepPreviousData, }); @@ -155,6 +181,71 @@ export default function SubscriptionsIndex() { } }; + // M3 β€” Bulk actions. The checkboxes below drive `selectedIds`; this mutation + // posts to /subscriptions/bulk and the toolbar above the table exposes the + // available actions. CSV export uses a hidden form submit so the browser can + // handle the download directly. + const bulkActionMutation = useMutation({ + mutationFn: ({ action, ids }: { action: 'cancel' | 'export_csv'; ids: number[] }) => + api.post('/subscriptions/bulk', { action, ids }), + onSuccess: (res: any, vars) => { + if (vars.action === 'cancel') { + const ok = res?.ok ?? 0; + const failed = Array.isArray(res?.failed) ? res.failed.length : 0; + if (failed === 0) { + toast.success(__(`Cancelled ${ok} subscription${ok === 1 ? '' : 's'}.`)); + } else { + toast.warning(__(`Cancelled ${ok}, failed ${failed}.`)); + } + setSelectedIds([]); + queryClient.invalidateQueries({ queryKey: ['subscriptions'] }); + } + }, + onError: (error: Error) => { + toast.error(error.message); + }, + }); + + const handleBulkCancel = () => { + if (selectedIds.length === 0) return; + setShowBulkCancelDialog(true); + }; + + const confirmBulkCancel = () => { + bulkActionMutation.mutate({ action: 'cancel', ids: selectedIds }); + setShowBulkCancelDialog(false); + }; + + const handleBulkExport = () => { + if (selectedIds.length === 0) return; + // We can't easily stream a CSV through fetch+sonner, so open a POST form + // with a hidden _wpnonce and let the browser download directly. The + // server returns Content-Disposition: attachment. + const form = document.createElement('form'); + form.method = 'POST'; + form.action = (window.WNW_API?.root || '') + '/woonoow/v1/subscriptions/bulk'; + const nonce = document.createElement('input'); + nonce.type = 'hidden'; + nonce.name = '_wpnonce'; + nonce.value = window.WNW_API?.nonce || ''; + form.appendChild(nonce); + const actionInput = document.createElement('input'); + actionInput.type = 'hidden'; + actionInput.name = 'action'; + actionInput.value = 'export_csv'; + form.appendChild(actionInput); + selectedIds.forEach((id) => { + const i = document.createElement('input'); + i.type = 'hidden'; + i.name = 'ids[]'; + i.value = String(id); + form.appendChild(i); + }); + document.body.appendChild(form); + form.submit(); + document.body.removeChild(form); + }; + // Checkbox logic const [selectedIds, setSelectedIds] = useState([]); const allIds = subscriptions.map(s => s.id); @@ -191,7 +282,22 @@ export default function SubscriptionsIndex() { -
+
+ {/* M4 β€” Search input. The input is uncontrolled-looking + (we just track rawSearch in state) so typing feels + instant; the debounce above commits the value to + the React Query cache 300ms after the user stops. */} +
+ setRawSearch(e.target.value)} + placeholder={__('Search by id, email, or name…')} + aria-label={__('Search subscriptions')} + className="border rounded-md pl-3 pr-3 py-2 text-sm w-64 focus:outline-none focus:ring-1 focus:ring-primary" + /> +
+ - {status && ( + {(status || committedSearch) && ( @@ -235,6 +341,14 @@ export default function SubscriptionsIndex() { {/* Mobile: Status filter bar */}
+ setRawSearch(e.target.value)} + placeholder={__('Search…')} + aria-label={__('Search subscriptions')} + className="border rounded-md px-3 py-2 text-sm flex-1 min-w-0 focus:outline-none focus:ring-1 focus:ring-primary" + />