Skip to content

feat(payment_terms): Dual-write to alias and jsonb - #6186

Open
D1353L wants to merge 1 commit into
feat-payment-terms-servicesfrom
feat-payment-terms-alias
Open

feat(payment_terms): Dual-write to alias and jsonb#6186
D1353L wants to merge 1 commit into
feat-payment-terms-servicesfrom
feat-payment-terms-alias

Conversation

@D1353L

@D1353L D1353L commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Context

Payment terms are moving from a single integer (net_payment_term, days) to a structured payment_term jsonb that can express more term types (net N days, due on receipt, day of month, etc.). Both columns already exist on customers and billing entities. This step starts writing them together so they never disagree.

Description

Adds PaymentTerms::AssignService, the single writer for both payment term columns. Every create/update path for customers and billing entities now goes through it instead of assigning net_payment_term inline.

The service applies the equivalence rules between the two fields:

  • payment_term sent (object or null): it wins. The jsonb is assigned, and the integer alias is mirrored from it
    ({net, N} → N, due_on_receipt → 0, other types → null). null clears both.
  • Only net_payment_term sent: the integer is assigned and the jsonb is derived as {term_type: "net", days: N}. null clears both.

The service only does assignment; callers still own validation and persistence. The payment_term branch is not reachable from the public API yet. API params for it ship in a later PR - so behavior for existing callers is unchanged: sending net_payment_term works exactly as before, it just also fills the jsonb.

Also adds PaymentTerm.from_net_payment_term to build a net term from a legacy integer.


Stack created with GitHub Stacks CLIGive Feedback 💬

@D1353L
D1353L force-pushed the feat-payment-terms-alias branch from ca564a1 to 3688bd2 Compare August 19, 2026 11:40
@D1353L
D1353L force-pushed the feat-payment-terms-alias branch from 3688bd2 to a76078d Compare August 19, 2026 11:55
@D1353L
D1353L force-pushed the feat-payment-terms-alias branch from a76078d to 99693cb Compare August 19, 2026 13:46
@D1353L
D1353L force-pushed the feat-payment-terms-alias branch 2 times, most recently from 1fe0cf4 to aae2f47 Compare August 19, 2026 17:16
@D1353L
D1353L force-pushed the feat-payment-terms-alias branch 2 times, most recently from 03b02e2 to e8a9387 Compare August 20, 2026 15:41
@D1353L
D1353L force-pushed the feat-payment-terms-alias branch 2 times, most recently from 46c6335 to 63face6 Compare August 21, 2026 16:02
@D1353L
D1353L marked this pull request as ready for review August 24, 2026 13:09
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — the alias matrix is not safely integrated into the record update paths.

  • Customer and billing-entity updates recalculate draft invoices only for net_payment_term, before payment_term can win. A structured-only update leaves drafts stale, while conflicting keys recalculate them from the losing value. Resolve the winning alias first and cover both cases in the update-service specs.
  • Non-net or null payment_term values map a billing entity's net_payment_term to nil, but that column and its model validation are non-nullable. Reconcile that compatibility representation and add persisted billing-entity create/update coverage; the current unit spec stops before the save that fails.

@D1353L
D1353L force-pushed the feat-payment-terms-alias branch from 63face6 to 3f3cbed Compare August 25, 2026 13:09
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — the structured payment-term path is unsafe at the persistence and update boundaries.

  • AssignService maps non-net billing-entity terms (and payment_term: nil) to net_payment_term = nil, but that column is non-null and the model validates it, so create/update cannot save. Resolve the alias/storage policy and cover persisted billing-entity writes; the current spec stops before saving.
  • Customer and billing-entity updates run legacy draft-invoice due-date updates only from net_payment_term, before payment_term is allowed to win. A structured-only update leaves drafts stale, while sending both can update drafts from the losing value. Apply precedence before side effects and test both cases.
  • None of the new call sites invokes PaymentTerms::ValidateService, so malformed structured terms can be coerced or saved by customer services. Validate before assignment and add failure coverage.

@D1353L
D1353L force-pushed the feat-payment-terms-alias branch from 3f3cbed to 1cc167e Compare August 25, 2026 14:21
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — the payment-term compatibility layer is incomplete.

  • Existing rows with payment_term: nil are not backfilled, and PaymentTerms::ResolveService ignores their legacy net_payment_term, resolving them as due-on-receipt. Add a legacy fallback or backfill, with customer and billing-entity coverage.
  • A payment_term-only update changes net_payment_term after the draft-invoice update guard, leaving draft invoice terms and due dates stale in both customer and billing-entity update paths. Route the derived alias through the existing update services and test the side effects.
  • Non-net billing-entity terms derive a nil alias, but billing_entities.net_payment_term is validated and constrained non-null; the new service spec only checks the unsaved object. Make this persistable or reject it explicitly, and cover the actual create/update service result.

@D1353L
D1353L force-pushed the feat-payment-terms-alias branch from 1cc167e to f8f852e Compare August 26, 2026 16:24
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — structured payment terms are not persisted consistently through the updated callers.

  • Resolve payment_term precedence before the legacy due-date updater: when both fields are sent, customer and billing-entity updates can persist the winning term (for example, due-on-receipt/0) while leaving draft invoices calculated from the losing net_payment_term; add caller-level regression coverage for both services.
  • Make non-aliasable billing-entity terms persistable or reject them: AssignService sets net_payment_term to nil for terms such as day_of_month, but the billing-entity column and model require a value; the current spec only checks the unsaved assignment and misses the 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