Conversation
…wngrade ## Context The earlier feature work introduced support for binding a subscription to a non-default billing entity on creation, but the downgrade path was not part of that initial scope. As a result, a subscription explicitly bound to a non-default entity loses that binding the moment a downgrade is requested: the pending row created for the next billing period is unbound, and the billing entity parameters accepted on create are silently dropped on downgrade, so operators cannot re-bind the pending subscription in one call. ## Description Apply the same billing-entity resolution rule on the downgrade path that already runs on create: explicit params win; otherwise carry the parent subscription's binding over to the pending row; otherwise persist nil to keep the "inherit from customer at billing time" semantic. The same rule applies when re-downgrading a not-yet-active subscription via update_pending_subscription, which only writes the column when params explicitly carry an entity reference. Carry-over from the parent is not gated by the multi_entity_billing flag. This is a no-op for single-entity customers, whose subscriptions all have NULL bindings today, but it lets multi-entity organizations keep their existing bindings even if the feature is later disabled at the org level.
lovrocolic
reviewed
May 14, 2026
| ending_at: params.key?(:ending_at) ? params[:ending_at] : current_subscription.ending_at, | ||
| progressive_billing_disabled: params[:progressive_billing_disabled] || false | ||
| progressive_billing_disabled: params[:progressive_billing_disabled] || false, | ||
| billing_entity_id: current_subscription.billing_entity_id |
Collaborator
There was a problem hiding this comment.
Can we double check with the product team if this as a fallback is correct? I am not sure if we should set nil as a fallback so that we inherit billing entity from customer 🤔
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
The earlier feature work introduced support for binding a subscription to a non-default billing entity on creation, but the downgrade path was not part of that initial scope. A subscription explicitly bound to a non-default entity therefore loses that binding the moment a downgrade is requested: the pending row created for the next billing period is unbound, and the
billing_entity_id/billing_entity_codeparams that work on the create endpoint are silently dropped on downgrade. Operators can neither preserve an existing binding nor re-bind a pending subscription to a different entity in one call.Description
This is the same rule the create path applies, transplanted onto the downgrade path. The resolution order is: explicit params win; otherwise carry the parent subscription's binding over to the pending row; otherwise persist
nilso the customer's default fires at billing time. The same rule applies when re-downgrading a not-yet-active subscription viaupdate_pending_subscription, which only touches the column when params explicitly carry an entity reference.One nuance worth calling out: the carry-over from the parent subscription is not gated by the
multi_entity_billingflag. That's deliberate. Single-entity customers all haveNULLbindings today, so the carry-over is a no-op for them. For multi-entity organizations, it means an existing binding survives even if someone later disables the flag at the org level.customer.billing_entityat activationbilling_entity_code: Y, parent bound to X404 billing_entity_not_foundstarting_in_the_futuresub withbilling_entity_code: Ymulti_entity_billingOFF, parent bound to XStacked on #5503, which extracted the resolution helper into the shared
Subscriptions::Concerns::BillingEntityResolutionConcernreused here.Two things worth flagging for the reviewer. Invoice numbering at activation (gapless per
(customer, billing_entity)) is satisfied by construction: the pending row'sbilling_entity_idis now correctly set, and the existing invoice generation path readssubscription.billing_entity || customer.billing_entityat billing time, so no new invoice-level assertions were added here. Separately,Subscriptions::PlanUpgradeService#update_pending_subscriptionhas the same gap on the upgrade side: it does not propagatebilling_entitywhen an operator upgrades a not-yet-active pending subscription. That parity gap is out of scope here and worth a follow-up.How to try locally
Enable the
multi_entity_billingfeature flag on your test organization, then:POST /api/v1/billing_entitieswithcode: "eu").billing_entity_code: "eu". Confirm the subscription row hasbilling_entity_idset.billing_entity_*. Confirm the pendingnext_subscriptioncarriesbilling_entity_id = eu.billing_entity_code: "us". Confirm the pending row now points tous(theupdate_pending_subscriptionbranch).404 billing_entity_not_found.The tests for these scenarios live under
describe "billing entity binding on downgrade"inspec/services/subscriptions/create_service_spec.rb: