Skip to content

feat(payment_terms): REST API write and read - #6201

Open
D1353L wants to merge 15 commits into
feat-payment-terms-templates-changesfrom
feat-payment-terms-api
Open

D1353L wants to merge 15 commits into
feat-payment-terms-templates-changesfrom
feat-payment-terms-api

Conversation

@D1353L

@D1353L D1353L commented Aug 21, 2026 •

Copy link
Copy Markdown
Contributor

Context

Earlier steps in this stack added the structured payment_term to customers and billing entities, with the net_payment_term integer as a compatibility alias. This PR exposes it via the REST API. Before this change, clients could only send the integer.

Description

Write path

  • payment_term is accepted on customer and billing entity create/update.
  • PaymentTerms::ValidateService runs in all five write services before any persistence. Invalid input returns a validation error.
  • The redundant direct net_payment_term column writes are removed. PaymentTerms::AssignService is now the only term write path.
  • A new RawPaymentTermParams controller concern copies the raw payment_term and net_payment_term values into the permitted params. Without this concern, invalid input would be dropped silently instead of returning a validation error.
  • PaymentTerms::ValidateService normalizes its input, so ActionController::Parameters and plain hashes give the same result.
  • Behavior change: when both payment_term and net_payment_term are sent and disagree (e.g. {net, 30} with 60, or end_of_month with 0), the write is rejected with conflicting_net_payment_term on payment_term. Before, payment_term silently won. A null net_payment_term sent with a structured term, and an explicit payment_term: null, are still accepted.

Read path

  • Added payment_term to customer and billing entity serializers.
  • The invoice serializer returns the snapshotted payment_term. For invoices from before this feature, it builds {term_type: "net", days: N} from the invoice's own integer.

Stack created with GitHub Stacks CLI • Give Feedback 💬

@D1353L
D1353L force-pushed the feat-payment-terms-api branch from 58a3a14 to f01ed55 Compare August 21, 2026 14:55
@D1353L
D1353L force-pushed the feat-payment-terms-api branch from f01ed55 to 32a1772 Compare August 21, 2026 16:02
@D1353L
D1353L force-pushed the feat-payment-terms-api branch from 32a1772 to 632a60d Compare August 21, 2026 16:27
@D1353L
D1353L force-pushed the feat-payment-terms-api branch from 632a60d to 28a1c2c Compare August 21, 2026 16:33
@D1353L
D1353L force-pushed the feat-payment-terms-api branch 3 times, most recently from 2484e8b to 62759f4 Compare August 25, 2026 13:09
@D1353L
D1353L force-pushed the feat-payment-terms-api branch from 62759f4 to 8b31eef Compare August 25, 2026 14:21
@D1353L
D1353L force-pushed the feat-payment-terms-api branch from 8b31eef to 411cde5 Compare August 25, 2026 14:27
@D1353L
D1353L force-pushed the feat-payment-terms-api branch from 411cde5 to 614b744 Compare August 25, 2026 14:35
@D1353L
D1353L force-pushed the feat-payment-terms-api branch 2 times, most recently from 4541299 to feceeae Compare August 26, 2026 16:24
@D1353L
D1353L force-pushed the feat-payment-terms-api branch from feceeae to 7d6f42b Compare August 26, 2026 19:38
@D1353L
D1353L marked this pull request as ready for review August 26, 2026 19:57
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — structured payment-term support is incomplete across sibling APIs and still has invalid-input failure paths.

  • Expose payment_term on the customer and billing-entity GraphQL object/input types and cover those paths. A non-net term written through V1 currently cannot be read or edited through the sibling GraphQL API; it appears there only as net_payment_term: null.
  • Make raw-param preservation handle array-shaped wrong types recursively and add request coverage. In CustomersController, payment_term: [{term_type: "net"}] copies unpermitted nested parameters into permitted, so the subsequent to_h raises instead of returning the intended payment_term: ["invalid_format"] response.
  • Apply payment_term precedence consistently during validation. The services validate net_payment_term even when payment_term is present, so an invalid legacy alias rejects a request that AssignService documents as ignoring because the structured term wins.

@D1353L
D1353L force-pushed the feat-payment-terms-api branch from 7d6f42b to 2fbfb0f Compare August 26, 2026 20:29
@D1353L
D1353L force-pushed the feat-payment-terms-api branch from 2fbfb0f to fca7bfe Compare August 26, 2026 20:30
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — Organizations::CreateService ignores the newly possible validation failure returned by BillingEntities::CreateService.

  • Propagate that child failure and add a regression spec proving invalid payment-term input rolls back organization/API-key creation; currently the outer service reports success and commits an organization without its default billing entity.

@annvelents
annvelents force-pushed the feat-payment-terms-api branch from fca7bfe to 969586f Compare September 8, 2026 10:01
@annvelents
annvelents force-pushed the feat-payment-terms-api branch 2 times, most recently from 70890a9 to 051f7a0 Compare October 30, 2026 14:46
@annvelents
annvelents force-pushed the feat-payment-terms-api branch from 051f7a0 to 5a3f4f2 Compare October 30, 2026 15:18
@annvelents
annvelents force-pushed the feat-payment-terms-api branch from 5a3f4f2 to edb8a95 Compare October 30, 2026 15:22
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — a structured customer term can make quotes announce different payment terms from the resulting invoices.

  • app/services/quote_versions/compute_mention_variables_service.rb:29 still snapshots customer.applicable_net_payment_term. Non-net terms intentionally have a null alias (app/models/payment_term.rb:57), so Customer#applicable_net_payment_term falls back to the billing entity (app/models/customer.rb:269) even though invoice resolution prioritizes the customer's structured term (app/services/payment_terms/resolve_service.rb:14). Resolve and localize the structured term in the quote path, preserve existing integer snapshots, and add a regression spec for an explicit non-net customer term.

@annvelents
annvelents force-pushed the feat-payment-terms-api branch from c2cdb7c to 6ad9cb5 Compare November 3, 2026 14:06
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — the organization payment-term source is inconsistent across REST and GraphQL.

  • app/serializers/v1/organization_serializer.rb:52 now sources net_payment_term from the default billing entity because direct billing-entity updates do not update the organization column, but app/graphql/types/organizations/current_organization_type.rb:25 still resolves Organization#net_payment_term. After such an update, GraphQL returns the stale integer while REST returns the billing entity's value. Resolve the GraphQL field from the default billing entity as well and add a behavioral spec; the current type spec only checks nullability.

@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

PASS — Structured payment terms are validated and synchronized across customer and billing-entity write paths, the legacy alias remains compatible, V1 reads expose the optional field without leaking it into V2 customer payloads, and the new behavior has focused request, service, resolver, and serializer coverage.

D1353L and others added 14 commits November 3, 2026 18:34
When a write sent both payment_term and net_payment_term, payment_term
silently won even if the two disagreed. This was introduced with the
GraphQL change but applies to every customer and billing entity write
path, so it belongs with the REST write PR where reviewers see it.

ValidateService now rejects a payment_term whose integer alias differs
from the net_payment_term sent with it, with conflicting_net_payment_term.
A null alias alongside a structured term, and an explicit null
payment_term, are still accepted. The write services now build the
validator arguments with params.slice.

Specs cover the full alias matrix, the customer update behaviour change,
and the REST customer create rejection.
## Context

RawPaymentTermParams copies the raw payment_term past strong params so a
wrong-type value reaches the validator, but it only permitted a direct
hash. A payment_term sent as an array of objects left unpermitted
parameters behind, and converting the params to a hash raised instead of
returning invalid_format.

## Description

Raw values are permitted recursively, through arrays, before being copied.
A request spec sends an array of objects and expects the invalid_format
validation error.
BIL-805: net_payment_term N with payment_term null (or a term with a null alias) returned 200 and let payment_term win. Any non-equivalent pair now returns 422 conflicting_net_payment_term, as Spec #1 §5.3 says.
…anization

BIL-796: GET /organizations returned the organization's own net_payment_term column, which no longer follows the default billing entity. It now mirrors the default entity: net_payment_term (null for the four non-net types) and payment_term.
## Context

net_payment_term keeps main's validation, so "45" and 30.5 are accepted
again and cast by the integer column. The request specs still expected
invalid_format, and the alias check compared the raw value, so "30"
sent with {term_type: "net", days: 30} was reported as conflicting.

## Description

The alias check compares the payment_term alias with the value cast as
the column casts it. The request specs assert that a digit string and a
decimal are stored as integers, with the matching net term.
## Context

BIL-805: the dive-in says payment_term always wins when a write sends
both fields. The alias check returned 422 conflicting_net_payment_term
for any pair that did not describe the same term, null included.

## Description

ValidateService no longer compares the two fields. AssignService already
assigns payment_term and derives net_payment_term from it, so net 30
with {end_of_month} stores end_of_month with a null alias, and net 30
with a null payment_term clears both.
…default billing entity

The REST organization serializer already reads the default billing entity,
since its term changes no longer write the organization column. GraphQL
still returned the stale organization integer.
…esolver spec

The shared membership is a let_it_be record, so its organization is frozen.
…ganization

The default user is already a member of the default organization.
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — malformed legacy payment-term input can silently clear an existing term.

  • RawPaymentTermParams#with_raw_payment_term copies object/array values for net_payment_term into permitted params (app/controllers/concerns/raw_payment_term_params.rb:10), but PaymentTerms::ValidateService#valid? validates only payment_term (app/services/payment_terms/validate_service.rb:9). Rails casts those non-integer aliases to nil, and PaymentTerms::AssignService then mirrors nil into both columns (app/services/payment_terms/assign_service.rb:24), so customer upserts and billing-entity updates can return success while erasing the prior term. Validate the raw alias shape before assignment and add request coverage proving malformed object/array aliases return a validation error without changing either record.

The raw copy forwarded an object or array net_payment_term, which Rails casts
to nil. allow_nil let it through and AssignService cleared both columns, so a
malformed alias erased an existing term. Strong params drop such values, as
before payment terms, and the validator no longer reads net_payment_term.
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

PASS — The latest fix restores legacy handling for malformed net_payment_term shapes instead of forwarding them for casting, with regression coverage showing customer upserts and billing-entity updates preserve existing terms.

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.

2 participants