Skip to content

feat(multi-billing-entity): move subscription#5498

Open
aquinofb wants to merge 4 commits into
mainfrom
feat/move-subscription-billing-entity
Open

feat(multi-billing-entity): move subscription#5498
aquinofb wants to merge 4 commits into
mainfrom
feat/move-subscription-billing-entity

Conversation

@aquinofb
Copy link
Copy Markdown
Contributor

@aquinofb aquinofb commented May 12, 2026

Summary

Operators can now move an active subscription to a different billing entity through a normal PATCH /api/v1/subscriptions/:external_id request, instead of having to terminate and recreate it. From the next billing cycle on, invoices for that subscription land under the new entity (using its number prefix and sequence); invoices already generated stay where they were.

The change is gated behind the multi_entity_billing feature flag, so single-entity organizations see no behavior change at all.

What changed

Concern Before After
Moving a subscription to another billing entity Terminate and recreate PATCH with billing_entity_id or billing_entity_code
Where invoice rows read their billing entity from customer.billing_entity subscription.billing_entity (falls back to customer's)
Where fees read their billing entity from subscription.customer.billing_entity_id subscription.billing_entity_id (falls back to customer's)
Subscriptions with NULL billing_entity_id (every row in prod today) Stamped with customer's entity Same; the column fallback keeps stamping byte-identical
Customer with multiple subs on different entities billed together Not reachable Returns a mixed_billing_entities validation failure; never produces a mixed invoice
Feature flag off Param wasn't accepted Param is silently ignored, matching the create-side convention

How the resolver works

Subscriptions::UpdateService resolves the entity inline. It accepts either an id or a code on the same request; if both are sent, the id wins. Unknown values return a 404 with billing_entity_not_found. Cross-organization references resolve to "not found" because the lookup is scoped to the subscription's organization.

For the read side, every fee and invoice writer now prefers subscription.billing_entity_id || subscription.customer.billing_entity_id. Because every existing subscription row has a NULL override, the fallback path is taken and the behavior is identical to today. Once an operator flips the column, the next invoice and its fees follow.

The one read site that intentionally stays on subscription.customer.billing_entity is the previous-period proration lookup in Fees::FixedChargeService, because legacy paid fees were written against the customer's entity and the query needs to match them.

Out of scope

The subscription.updated webhook payload doesn't surface the new billing_entity_id yet; that lives with the serializer and OpenAPI work. Batched billing across mixed-entity subscriptions returns a validation error today; the proper fix is to split the batch upstream in OrganizationBillingService and is deferred.

Test plan

  • Seed an organization with multi_entity_billing in feature_flags, plus two billing entities us (default) and eu. Seed a customer under us and an active subscription on a monthly recurring plan.
  • PATCH /api/v1/subscriptions/:external_id with { subscription: { billing_entity_code: "eu" } }. Expect 200 and verify the subscriptions row's billing_entity_id now points at eu; the customers row is unchanged.
  • Force the next billing cycle (move the clock to the calendar boundary or invoke BillSubscriptionJob directly for the subscription). Verify the resulting invoice's billing_entity_id is eu, its number draws from eu's sequence and prefix, and any previously-generated invoices are untouched.
  • Toggle multi_entity_billing off on the organization. PATCH the subscription again with billing_entity_code: "us". Expect 200 and no change to the row.
  • Re-enable the flag. PATCH with billing_entity_id set to a random UUID. Expect 404 with code: "billing_entity_not_found". PATCH with billing_entity_id referencing an entity from a different organization. Expect the same.
  • Create a second subscription on the same customer and PATCH it to eu, leaving the original on us. Trigger their next billing day. Expect a validation failure with billing_entity: ["mixed_billing_entities"] rather than a mixed invoice.

## Context

Multi-entity billing (Decision 5.6) makes subscription.billing_entity_id
mutable. Operators must be able to move an active subscription between
billing entities without recreating it. Future invoices follow the new
entity; past invoices remain stamped with their original entity.

## Description

Extends Subscriptions::UpdateService to accept billing_entity_id and
billing_entity_code, gated behind the multi_entity_billing feature flag.
Resolution mirrors the create path and lives inside the service: by-id
takes precedence over by-code, unknown values fail with
not_found_failure!, and the param is silently ignored when the flag is
off.

Migrates the invoice and fee write sites to prefer
subscription.billing_entity over customer.billing_entity, falling back
through the column to preserve backwards compatibility for the NULL rows
that exist today. This makes the "next billing cycle uses the new
entity" guarantee hold end-to-end: the generated invoice row, its
sequential id, and the fees on it now all draw from the subscription's
binding when set. The one read site that looks up a previous
subscription's already-paid fixed-charge fee continues to scope by
customer.billing_entity, because legacy rows were written that way and
the lookup must match them.

Adds a guard in Invoices::SubscriptionService that refuses to invoice a
batch whose subscriptions resolve to different billing entities, rather
than silently producing an invoice whose header and fees disagree. The
proper batching split lives upstream in OrganizationBillingService and
is out of scope here.
@aquinofb aquinofb marked this pull request as draft May 12, 2026 15:46
## Context

The acceptance scenario is "active subscription under US, update to EU,
next invoice numbered under EU, past invoice unchanged." Existing
coverage hit each clause separately (UpdateService spec for the write,
SubscriptionService spec for invoice stamping) but no single test
proved the seams compose: that the column written by UpdateService is
the same column read by the next invoice generation, and that the
existing invoice's stamp survives the mutation.

## Description

Adds one end-to-end example in subscription_service_spec.rb that
generates a billing-cycle invoice, calls Subscriptions::UpdateService
to move the subscription to a new entity, generates the next cycle's
invoice, and asserts: the new invoice (header and fees) carries the
new entity, the past invoice's billing_entity_id is unchanged.
@aquinofb aquinofb force-pushed the feat/move-subscription-billing-entity branch from 267498d to 1610823 Compare May 13, 2026 11:13
@aquinofb aquinofb changed the title feat(billing-entity): allow moving subscription feat(multi-billing-entity): move subscription May 13, 2026
## Context

The first pass added a 7-line comment above `mixed_billing_entities?`
that narrated migration history and predicted future work. Those
notes belong in the PR description, not in code where they rot.

## Description

Drops the comment and inlines the one-call helper
`resolved_billing_entity_ids` into the predicate. Also collapses a
redundant `subscription.organization.reload` plus its multiline
before-block in the UpdateService spec; `organization.update!` mutates
the same instance held by the factory, so the reload was load-bearing
only as a workaround for a confusion the simpler one-liner avoids.
@aquinofb aquinofb force-pushed the feat/move-subscription-billing-entity branch from 1610823 to e33178a Compare May 13, 2026 11:20
@aquinofb aquinofb marked this pull request as ready for review May 13, 2026 11:30
@aquinofb aquinofb requested review from a team, lovrocolic, mariohd, rinasergeeva and toommz May 13, 2026 11:30
Copy link
Copy Markdown
Contributor

@mariohd mariohd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Comment thread app/services/fees/build_pay_in_advance_fixed_charge_service.rb Outdated
Comment thread app/services/subscriptions/update_service.rb Outdated
Comment thread app/services/subscriptions/update_service.rb Outdated
Comment thread spec/services/invoices/subscription_service_spec.rb
Comment thread spec/services/subscriptions/update_service_spec.rb
## Context

PR feedback from @toommz on PR #5498. Three changes plus a missing
test assertion.

## Description

Extracts `Subscription#applicable_billing_entity_id`, mirroring the
`applicable_*` convention from Customer. The five call sites (four fee
writers and the `mixed_billing_entities?` predicate in the invoice
service) now read at a glance instead of repeating
`billing_entity_id || customer.billing_entity_id` inline.

Simplifies `Subscriptions::UpdateService#resolve_billing_entity` by
dropping the `attrs` intermediate and calling `find` / `find_by!`
directly per branch; both raise the same `ActiveRecord::RecordNotFound`
that the rescue handles. Renames the local `resolved` to
`new_billing_entity` at the call site for clarity.

Adds a negative assertion in the unknown-billing_entity_id context that
the `subscription.updated` webhook does not fire when the resolver
raises. This locks in that `not_found_failure!` short-circuits before
the transaction reaches `subscription.save!`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants