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

  1. Subscribe a user via any connector.
  2. Delete the user account.
  3. 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.

Command icon 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

colan created an issue. See original summary.

colan’s picture

colan’s picture

Design 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.
  • Nothing in the module calls the inline-actions API. getAvailableActions() and executeAction() have no caller except the find-duplicates --cancel Drush helper, and every shipped connector returns supportsInlineActions() = 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 public cancelSubscription() and honours executeAction('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.)
  • So the resolution as written — cancel when supportsInlineActions() — would cancel nothing on any existing connector, including the one that can. The Drush helper already works around that with method_exists($connector, 'cancelSubscription') duck typing, which is the smell this issue should remove rather than copy.
  • Charge-based subscriptions have no remote contract: deleting the entity stops the cycle. What they do leave behind is the stored instrument (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 existing cancelSubscription(); 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_cancellation service with cancel(SubscriptionEntityInterface $subscription): bool that tries, in order: the new interface; then executeAction('cancel') when the connector declares supportsInlineActions() (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 the method_exists duck 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 only status = 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 to executeAction('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 RemoteCancellingConnectorInterface implementation; 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.

colan’s picture

Issue summary: View changes
Status: Active » Needs review

Fixed 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, else executeAction('cancel') when the connector declares supportsInlineActions(), 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 Drush find-duplicates --cancel helper now calls it and the method_exists('cancelSubscription') duck typing is gone.

User deletion. hook_user_predelete() offers every subscription of the user to cancelBeforeDeletion() 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 setting cancel_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: a cancel_test_connector implementing the interface with confirmed/refused/throw behaviours, state-controlled inline-action behaviour on test_connector, and a logger-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.

  • colan committed ac93606c on 1.0.x
    Merge branch '3616934-remote-cancel-on-user-delete' into '1.0.x'...

  • colan committed 8ac0c960 on 1.0.x
    Issue #3616934: Cover remote cancellation on user deletion
    
    Kernel tests...

  • colan committed f6e1f51d on 1.0.x
    Issue #3616934: Cancel remote billing before deleting a deleted user's...

  • colan committed f6318740 on 1.0.x
    Issue #3616934: Add RemoteCancellingConnectorInterface and a shared...
colan’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.