Problem/Motivation
subscription_manager_user_predelete() deletes all of the user's local subscription entities and removes roles, but never contacts the connector. The remote contract (Stripe subscription, Recharge contract, etc.) stays active and keeps charging the customer, while the site has deleted its only record linking that remote contract to a person. The customer is billed for access that no longer exists, and self-healing can't help because the account is gone.
Steps to reproduce
- Subscribe a user via any connector.
- Delete the user account.
- The remote subscription remains active and renews.
Proposed resolution
On user predelete, before deleting each local entity, attempt remote cancellation through the entity's connector. The interface already offers a path: connectors with supportsInlineActions() expose cancellation via executeAction('cancel', $subscription). Proposed behavior:
- If the connector supports a cancel action, execute it, wrapped so a remote failure never blocks the user deletion (log at error severity with the remote subscription and customer IDs).
- If it doesn't (or the plugin is uninstalled), log at critical severity with the remote IDs so an operator can cancel manually, since after deletion the log line is the only remaining linkage.
- Make the behavior a setting (cancel remotely: yes/no, default yes) for sites where user deletion intentionally shouldn't touch billing (e.g. migrations).
Whether cancellation should be immediate or at period end is connector policy; the module requests "cancel" and the connector maps it.
Remaining tasks
Agree on the default; patch; kernel test with a spy connector asserting the cancel action fires and that a throwing connector doesn't abort deletion; change record.
User interface changes
One new checkbox on the settings form ("Cancel remote billing when a user account is deleted", default on); a warning message to the operator when a cancellation could not be completed.
API changes
New optional RemoteCancellingConnectorInterface for connectors; new subscription_manager.remote_cancellation service.
Data model changes
New boolean setting cancel_remote_on_user_delete.
Issue fork subscription_manager-3616934
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
colanComment #3
colanDesign pass before implementation, re-read against the current 1.0.x and every connector in the ecosystem. The proposed resolution needs one correction and one extension, because two things changed since August: the inline-actions API turns out to be unused, and charge-based mode (#3618739: Add a charge-based connector mode with a local billing cycle engine) now exists.
What the code says
SubscriptionManagerHooks::userPredelete()deletes the user's local subscription entities and never touches the connector — confirmed.getAvailableActions()andexecuteAction()have no caller except thefind-duplicates --cancelDrush helper, and every shipped connector returnssupportsInlineActions() = FALSE: Stripe Subscription 2.0.x, Recharge Subscription 1.0.x, and the Chargebee and Shopify sandboxes. Only Stripe can cancel at all — it has a publiccancelSubscription()and honoursexecuteAction('cancel')despite answering FALSE; Recharge, Chargebee and Shopify refuse every action as portal-only. (Of the two other ecosystem entries, the Recharge Commerce sandbox is an empty repository and SamCart Subscription is not a connector — it provisions users from a post-checkout return and does not depend on this module.)supportsInlineActions()— would cancel nothing on any existing connector, including the one that can. The Drush helper already works around that withmethod_exists($connector, 'cancelSubscription')duck typing, which is the smell this issue should remove rather than copy.instrument_id: a GoCardless mandate, a vaulted card) — live at the processor, unattached to anything on our side — and possibly a submitted charge whose webhook lands later. The cycle engine already tolerates a charge whose subscription is gone (resolveCharge(),retryCharge(),rebillReversedCharge()all null-check), so the records survive; a test should pin that.Proposal
1. An explicit opt-in interface instead of the inline-actions gate.
RemoteCancellingConnectorInterface::cancelRemoteSubscription(SubscriptionEntityInterface $subscription): bool. One method, mode-agnostic: a self-scheduling connector cancels the remote contract; a charge-based connector cancels or detaches the stored instrument so nothing remote-side can bill this subscription again. Implementations must be idempotent — an already-cancelled contract or already-revoked mandate is a success, so callers never have to reason about stale local status. Stripe implements it in one line around its existingcancelSubscription(); portal-only connectors simply don't implement it. Optional interfaces are the module's established pattern (ChargeBasedConnectorInterface,InstrumentReplacingConnectorInterface), and this keeps the base interface stable for connectors that cannot cancel by API. This is an API addition, so the issue summary's "API changes: none" is wrong and there will be a change record.2. One resolution strategy in one service. A
subscription_manager.remote_cancellationservice withcancel(SubscriptionEntityInterface $subscription): boolthat tries, in order: the new interface; thenexecuteAction('cancel')when the connector declaressupportsInlineActions()(honouring the existing contract for a connector that has a cancel button); otherwise reports "unsupported". Exceptions are caught and logged with the connector id, remote subscription and customer ids, instrument id and user id. Both the predelete hook and the Drush duplicates helper call it, and themethod_existsduck typing goes.3. Predelete flow. New setting
cancel_remote_on_user_delete(boolean, default TRUE, one checkbox on the settings form, update hook for existing sites). When on, every subscription of the user is offered to the service before deletion — all of them, not onlystatus = TRUE, because a suspended charge-based subscription still has a live mandate and a "cancelled" local status may be stale (the login self-heal exists precisely because it can be). Success is logged at notice. Failure or "unsupported" is logged at critical with every remote identifier so an operator can finish the job by hand, and the same text goes to the acting user via the messenger — a deletion is almost always an administrator in the UI, and a warning on the confirmation result page is worth far more than a log line. Deletion is never blocked. I looked at queueing failed cancellations for retry and recommend against it: the entity is gone after predelete, so the item would have to carry a snapshot and a second code path, for a rare, operator-driven event where the operator is present to read the warning.4. Charge records stay. Charges are financial records already reported through the tax seam (#3618741: Add a pluggable tax/invoice service seam (calculation, evidence, invoice reference), keeping tax engines out of core); they are not deleted with the subscription, and a late webhook must still be able to settle or reverse them. Covered by a test rather than new code, given the engine already null-checks.
5. Out of scope, noted for the record. Blocking an account (the
user_cancel_block*methods) fires no predelete and leaves billing running; that is a policy question (blocked ≠ cancelled) rather than a bug, and I'd leave it alone unless someone wants it. Notifying the customer of the cancellation is the connector's or the processor's job, not ours.Tests
Kernel tests with a test connector implementing the new interface and recording calls in state, with success / FALSE / throw behaviours: cancellation is attempted for every subscription of the deleted user (active and not) and only for that user; the setting off means no remote call; a FALSE result, an exception, and a connector without the interface each log critical with the ids and never prevent the user or the subscriptions from being deleted; a connector with
supportsInlineActions()falls back toexecuteAction('cancel'); a charge-based subscription's charge records survive and can still be resolved afterwards. The Drush helper's strategy is exercised through the shared service.Connector impact
Stripe Subscription gains a one-line
RemoteCancellingConnectorInterfaceimplementation; the others need nothing (they land on the critical-log path, which is today's behaviour minus the silence). Both go into the connector catch-up issue text the roadmap generates after beta7.Decisions to ratify: (1) new optional interface over the inline-actions gate; (2) shared service also replaces the Drush duck typing; (3) attempt for every subscription of the user, default on, critical log + messenger, no retry queue; (4) charge records retained; (5) account blocking out of scope.
Comment #5
colanFixed in the MR, in three commits following the ratified design.
RemoteCancellingConnectorInterface(new, optional) — one method,cancelRemoteSubscription(SubscriptionEntityInterface $subscription): bool, mode-agnostic: a self-scheduling connector cancels the remote contract immediately, a charge-based connector cancels or detaches the stored instrument. The docblock requires idempotence (already cancelled = success), so callers never reason about stale local status.subscription_manager.remote_cancellation(new service,RemoteCancellation) owns the one resolution strategy: the interface when implemented, elseexecuteAction('cancel')when the connector declaressupportsInlineActions(), else "cannot cancel by API".cancel()returns a bool and logs the outcome — notice on success, error on anything else — always with the connector id, remote subscription id, customer id, instrument id, and local ids, and never throws (a connector exception or an uninstalled plugin becomes a logged FALSE). The Drushfind-duplicates --cancelhelper now calls it and themethod_exists('cancelSubscription')duck typing is gone.User deletion.
hook_user_predelete()offers every subscription of the user tocancelBeforeDeletion()before removing it — all of them, not only active ones. When cancellation fails or is unsupported, that method logs at critical ("Subscription 12 of deleted user 34 is gone but its remote billing may continue: cancel it manually at connector … (remote subscription …, customer …, instrument …)") and shows the same identifiers to the operator through the messenger. Deletion is never blocked. New settingcancel_remote_on_user_delete(default TRUE; checkbox on the settings form; update 10025 sets it on existing sites).Charge records are untouched by deletion, as designed; the test pins that a late webhook can still resolve one whose subscription is gone.
Tests.
UserDeletionRemoteCancellationTest(9 cases): every subscription of the deleted user and only theirs is offered; the setting off makes no remote call; a refusal, a thrown exception, a portal-only connector, and a failing inline action each produce the critical log line and the operator warning carrying all four identifiers while the user and subscriptions are still deleted; the inline 'cancel' action is used as the fallback; charge records survive and resolve; and the service reports its bool outcome for Drush without escalating to critical. Supporting test-module additions: acancel_test_connectorimplementing the interface with confirmed/refused/throw behaviours, state-controlled inline-action behaviour ontest_connector, and alogger-tagged recorder so tests can assert on operator-facing log lines (first log assertions in the suite). Suite is at 162 kernel tests.Connectors. Stripe Subscription can implement the interface in one line around its existing
cancelSubscription(); the others need nothing and land on the critical-log path (today's behaviour, minus the silence). Both points go into the connector catch-up text the roadmap generates after beta7. The orphaned portal API this design pass surfaced is filed against the cleanup omnibus.Change record drafted.
Comment #10
colan