Skip to content

Ing 462 - #6367

Open
mariohd wants to merge 2 commits into
mainfrom
ing-462
Open

Ing 462#6367
mariohd wants to merge 2 commits into
mainfrom
ing-462

Conversation

@mariohd

@mariohd mariohd commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Stacked on #ING-466 — base this PR on ing-466, not main. It calls BillingObjectConnections::AttachToResourceService, which only exists there, so merging it first would break main.

Context

ING-462. The subscription half of per-object connection routing (dive-in T18b). ING-466 added the write path for wallets plus the shared validation and persistence services; this reuses them unchanged.

Description

POST/PATCH /subscriptions accept a connections object keyed by category, with the same semantics as the wallet endpoints:

"connections": {
  "payment":    { "behavior": "skip" },
  "accounting": { "code": "netsuite_sub2" },
  "crm":        { "behavior": "inherit" }
}

A code pins that connection, skip short-circuits the category so it never falls back, inherit drops the override, and an omitted category changes nothing.

No new services. ING-466's two are polymorphic in the owner, so this PR is only params, one validator method, and two write paths — 261 lines, most of it specs.

Behind the multi_connection flag. Connections sent while the flag is off are refused rather than ignored; an unresolvable code is a 422. The existing top-level payment_method is untouched.

Reviewer note — the downgrade case

Subscriptions::CreateService attaches invoice_custom_section with unless downgrade?. I deliberately did not copy that guard. Connections are routing config in the same family as payment_method, which both PlanUpgradeService and PlanDowngradeService already carry onto the new subscription — so skipping them on a downgrade would silently discard the operator's choice. Upgrades and downgrades are each pinned by a spec.

Testing

12 new examples: 7 request (4 POST, 3 PUT) and 5 service, the latter covering plain create, unresolvable code, flag off, and the upgrade/downgrade behaviour above. Regression: 2001 examples across the subscription request and service suites, plus 156 for the non-REST callers — GraphQL mutations, Orders::SubscriptionCreation::ExecuteService, Orders::SubscriptionAmendment::ExecuteService. All green, Rubocop clean.

## Context

Billing objects need to route themselves to one of a customer's
connections. The table, the resolution cascade and the connection
codes already shipped; nothing wrote the override rows yet. This adds
the first write path, behind the multi_connection flag.

## Description

The wallet endpoints and each nested recurring transaction rule accept
a connections object keyed by category (payment, tax, accounting,
crm). An entry either names a connection code, which pins it, or a
behavior: skip never falls back, inherit drops the choice. An omitted
category changes nothing.

Inherit is never persisted, since resolution reads a missing row as
inheritance. An unknown code is a validation error, and connections
sent while the flag is off are refused rather than ignored.

Validation and persistence are polymorphic in the owner, so the
subscription and invoice surfaces can reuse them.
## Context

Billing objects need to route themselves to one of a customer's
connections. ING-466 added the write path for wallets along with the
shared validation and persistence services; this is the subscription
half of the same work (dive-in T18b).

## Description

`POST/PATCH /subscriptions` accept a `connections` object keyed by
category, with the same semantics as the wallet endpoints: a `code`
pins that connection, `skip` short-circuits the category, `inherit`
drops the override, and an omitted category changes nothing.

No new services — the polymorphic ones from ING-466 are reused as-is,
so only the params, the validator and the two write paths change.

Behind the `multi_connection` flag. Connections sent while the flag is
off are refused rather than ignored, and an unresolvable code is a 422.

Unlike `invoice_custom_section`, connections are attached on a
downgrade too. They are routing config in the same family as
`payment_method`, which the upgrade and downgrade paths already carry
onto the new subscription, so skipping them would silently discard the
choice.
@mariohd
mariohd marked this pull request as ready for review September 10, 2026 14:04
@mariohd
mariohd requested a review from lovrocolic September 10, 2026 14:04
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

Automated pre-review (advisory, not a required check) — verdict: HOLD · CI green

HOLD — Two call-context failures can misroute or silently drop requested connection overrides.

  • Subscriptions::CreateService attaches downgrade connections to PlanDowngradeService's returned current subscription, not the newly created pending subscription. Attach them to the pending subscription and make the spec assert both owners explicitly.
  • Wallets::RecurringTransactionRules::CreateService rescues an AttachToResourceService.call! failure but returns its untouched successful result, so an unresolved nested code can persist the wallet/rule without its requested override. Propagate the failed result so the outer transaction rolls back, and add endpoint coverage for this nested failure.

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.

1 participant