Skip to content

Ing 467 - #6387

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

Ing 467#6387
mariohd wants to merge 2 commits into
mainfrom
ing-467

Conversation

@mariohd

@mariohd mariohd commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

[> Stacked on #ING-466 — base this PR on ing-466, not main. It builds on the shared connection services introduced there, and amends two of them.

Context

ING-467. The GraphQL counterpart of per-object connection routing for wallets (dive-in T19-GQL). ING-466 added the REST write path plus the shared validation and persistence services; the UI drives wallets through GraphQL, so it needs the same routing exposed there.

Description

A new ConnectionsInput carries one optional choice per category — payment, tax, accounting, crm — each either a connection code or a behavior:

input ConnectionsInput {
  payment: ConnectionChoiceInput
  tax: ConnectionChoiceInput
  accounting: ConnectionChoiceInput
  crm: ConnectionChoiceInput
}

input ConnectionChoiceInput {
  code: String
  behavior: ConnectionBehaviorEnum   # inherit | skip
}

specific is deliberately absent from the enum — it's implied by supplying a code. The argument is added to Wallets::Create/UpdateInput and both recurring-rule inputs, and schema.graphql / schema.json are regenerated (both are spec-enforced). The existing paymentMethod input is untouched, and everything stays behind the multi_connection flag.

No new service logic. The mutations already forward their arguments to the same write path as REST, so validation, the flag gate and persistence came for free.

Reviewer note — why this touches two ING-466 files

GraphQL passes nested inputs as GraphQL::Schema::InputObject, not Hash, and the validator guards with is_a?(Hash) — so every GraphQL request would have been rejected as invalid_connections. Both shared services now normalise at their boundary, converting only when the value isn't already a Hash: HashWithIndifferentAccess#to_hash returns string keys, which silently broke choice[:code] on the recurring-rule path. ING-462 and the future invoice surfaces share these services, so the fix benefits them too.

Testing

9 new mutation examples — 4 on create, 5 on update, the latter covering inherit clearing a row, the per-rule path through RecurringTransactionRules::UpdateService, an unresolvable code, and the flag gate. Regression: 691 examples across the shared services, the wallet REST suites, both mutations and the schema-dump spec. 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

ING-466 added the REST write path for wallet connections along with the
shared validation and persistence services. This exposes the same
routing to the UI, which drives wallets through GraphQL (dive-in
T19-GQL).

## Description

A new ConnectionsInput carries one optional choice per category —
payment, tax, accounting and crm — each either a connection code or a
behavior. The behavior enum is limited to inherit and skip, since
specific is implied by supplying a code. The argument is added to the
wallet create and update inputs and to both recurring-rule inputs, and
the dumped schema is regenerated.

No new service logic was needed: the mutations already forward their
arguments to the same write path as REST, so validation, the
multi_connection gate and persistence come with it.

One fix was required. GraphQL passes nested inputs as an InputObject
rather than a Hash, which the validator rejected outright, so both
shared services now normalise at their boundary. The conversion applies
only when the value is not already a Hash, because
HashWithIndifferentAccess#to_hash returns string keys and silently
broke the lookups on the recurring-rule path.
@mariohd
mariohd marked this pull request as ready for review September 11, 2026 17:09
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — recurring-rule creation can silently accept an unresolvable connection.

  • In Wallets::RecurringTransactionRules::CreateService, AttachToResourceService.call! raises with its own failed result, but the BaseService::FailedResult rescue discards that result and returns the still-successful recurring-rule result. Propagate the failure as the update sibling does, and add a create-path regression asserting an unknown nested connection code fails without persisting the wallet, rule, or overrides.

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