Several code paths assume each user has at most one subscription, but nothing enforces it. SubscriptionManagerService::loadLocalSubscription() returns reset() of whatever loadByProperties() found, in storage order. Callers that inherit this ambiguity include subscribe() (which routes an existing subscriber to "their" connector), getMySubscription(), getPortalUrl(), and subscription_manager_page_attachments() (which exposes exactly one subscription to drupalSettings). Meanwhile subscription_manager_user_presave() and user_predelete() correctly handle multiple subscriptions per user.
With multiple connectors installed the ambiguity becomes user-facing, and more so once the subscribe chooser (#3616769: Add a connector chooser to the subscribe flow when multiple connectors provide plans) makes running several connectors side by side a supported configuration: a user who somehow ends up with subscriptions on two connectors gets nondeterministic behavior on the manage page, the portal redirect, and the upgrade flow.
Decide the invariant explicitly, then enforce it:
SubscriptionEntity::preSave() (reject or deactivate a second active subscription) and in the subscribe flow (an active subscriber attempting to subscribe on a different connector is sent to a switch flow: cancel on the old connector, then subscribe on the new one, since customers and payment instruments do not transfer between services).Either way, add deterministic ordering (e.g. by created) to loadLocalSubscription() so behavior stops depending on storage order.
Note that the decision here gates part of #3616770: Self-healing remote subscription sync only queries the default connector: its proposed multi-connector self-healing sync can stop after the first connector that yields a remote subscription only if Option A's invariant holds; under Option B it must always consult every connector.
Pick the option in the issue discussion; patch; kernel tests for the enforcement point and for deterministic selection; change record.
Option A: a message or switch flow when an active subscriber tries to start a second subscription.
Option A: none. Option B: plural response shapes on the JSON endpoints.
None; this is behavioral.
Show commandsStart 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:
changes
Comments
Comment #2
colanComment #3
colanAlso, "Has a subscription" is ambiguous about status.
userHasLocalSubscriptionToAnyPlan()ignoresstatus, so route access, subscribe routing, and login-redirect all treat lapsed subscribers as subscribed, whileuserIsSubscribedToPlan()and the self-heal short-circuit filter on active. Probably correct for manage-billing access, wrong for redirect logic; it should be an explicit parameter.Comment #4
colanComment #6
colanDecision: Option A — one active subscription per user, site-wide. It matches the current design assumption throughout the code; if multi-subscription support is ever wanted, it should be added holistically as its own feature rather than inherited from ambiguity. MR opened implementing it:
SubscriptionEntity::preSave(), newest save wins: activating a subscription deactivates any other active one the user holds, mirroring the remote reality that the latest contract is the one in force. Deactivation rather than rejection was chosen deliberately: a webhook- or self-heal-driven sync can create a fresh active row before the stale one is closed out, and rejecting would break those flows. The superseded row'spostSave()revokes its snapshotted roles (#3616929: Role revocation depends on a live plan lookup, so users can keep paid roles after expiry or cancellation) as usual, so entitlements follow the supersession.loadLocalSubscription()now orders candidates active-first, then newest-created, then highest-id, instead of returning storage order.userHasLocalSubscriptionToAnyPlan()/getUserLocalSubscriptionToAnyPlan()gainedbool $active_only = FALSE. The default keeps lapsed subscribers counting — correct for manage-billing access and the portals, which serve them for history and reactivation — and the post-login subscribe redirect now passes TRUE, since a lapsed subscriber is exactly who that redirect exists for (previously they were never redirected).subscribe()deterministically routes an existing subscriber to their subscription's connector; the full cancel-then-subscribe switch UX stays in #3616769: Add a connector chooser to the subscribe flow when multiple connectors provide plans's scope as planned, and #3616770: Self-healing remote subscription sync only queries the default connector may now use its first-yielding-connector short-circuit, since the invariant it depends on holds.Kernel coverage: supersession (statuses and roles both correct), re-save stability, deterministic selection (active beats newer-inactive; newest wins among equals), the
$active_onlydistinction, and the redirect fix (RED before). One existing test changed meaningfully:GrantedRolesSnapshotTest::testPresaveUnionUsesSnapshotswas built on two concurrent active subscriptions, a state this issue makes impossible by design, so it now covers what the presave union still means under the invariant — healing a manually removed role from the active snapshot and resurrecting nothing after revocation.Change record drafted covering the invariant, the deterministic selection, and the new parameter.
Comment #8
colan