Skip to content

feat(contracts): attach rate cards over REST - #6279

Merged
rsempe merged 12 commits into
mainfrom
bil-624
Sep 4, 2026
Merged

feat(contracts): attach rate cards over REST#6279
rsempe merged 12 commits into
mainfrom
bil-624

Conversation

@rsempe

@rsempe rsempe commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Context

The contract runtime shipped its object and read/create surface, but a plan-less contract had no way to carry pricing — rate cards could only be attached to plans. This is the applied-rate-card surface of the contract runtime, reworking #5871's subscription attach flow onto the dedicated contract_rate_cards storage.

What this adds

  • POST/GET/PUT/DELETE /api/v2/contracts/:external_id/applied_rate_cards + nested rate_phases, addressed by contract external id and card code, mirroring the plan_rate_cards surface.
  • Pending-only authoring: a pending contract's cards are freely editable; once the contract is active/terminated/canceled every write returns contract_locked (via Contract#locked?).
  • Attach rules ported from feat(products): attach rate cards to subscriptions #5871 onto the new storage: one card per pricing slice (product_already_priced / product_filter_already_priced), currency match against the contract's currency (its plan's, or the customer's for a plan-less contract), day-grained window / anchor / billing clock seeded from the contract.
  • Rate phase services generalized: RatePhases::{Replace,Update,Destroy}Service now accept a contract_rate_card parent (Create already did), with a parent-aware lock that reports plan_locked or contract_locked per parent.
  • contract_rate_card ApiKey permission resource.

## Context

The contract runtime shipped its object and read/create surface
(BIL-601/602/603) but a plan-less contract had no way to carry pricing:
rate cards could only be attached to plans. This is the applied-rate-card
surface of the contract runtime, reworking the subscription attach flow
(#5871) onto the dedicated `contract_rate_cards` storage.

## Description

Expose `POST/GET/PUT/DELETE /api/v2/contracts/:external_id/applied_rate_cards`
and the nested `rate_phases`, addressed by contract external id and card
code. Authoring is pending-only: a pending contract's cards are freely
editable, and once the contract is active every write returns
`contract_locked`. Attaching enforces the one-card-per-pricing-slice rule,
matches the card currency against the contract's (its plan's, or the
customer's for a plan-less contract), and seeds the day-grained window,
anchor and billing clock from the contract.

The rate phase services already accepted a contract rate card on create;
generalize replace/update/destroy the same way, with a parent-aware lock
that reports `plan_locked` or `contract_locked` per parent. Add the
`contract_rate_card` ApiKey permission resource.

Active-contract unit versioning (day-grained, last-write-wins per day)
stays with the units-override port, out of this surface.
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD: Immediate-start plan-less contracts cannot be given any pricing: contract creation makes them active by default, while the new applied-rate-card create service rejects every active contract and contract creation has no nested card input. Provide an atomic initial-pricing path (while preserving post-activation immutability) and cover this flow.

Add request coverage for the new contract rate-phase PUT action, including successful update and the active-contract lock; this newly exposed path is currently untested.

## Context

The nested contract rate-phase surface exposed an update (PUT) action
that had request coverage for index, create and destroy but not update.

## Description

Add request specs for updating a contract rate phase: a successful edit
while the contract is pending, and the contract_locked rejection once the
contract is active.
@lago-claude-ai-agent

Copy link
Copy Markdown
Contributor

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

HOLD — The nested contract lookup is ambiguous when an external ID has lifecycle history.

  • ContractRateCardsController#find_contract and the rate-phases controller use an unordered find_by(external_id:). External IDs can be reused after termination and the schema permits pending/active siblings, so these endpoints can read or mutate the wrong contract (or incorrectly report it locked/not found). Select the intended status deterministically, consistent with ContractsController#show, and add request coverage for reused IDs in both controller surfaces.

## Context

An external id is unique only among live (pending/active) contracts; the
partial index lets terminated and canceled siblings reuse it as history.
The applied-rate-card and rate-phase controllers looked a contract up with
an unordered, status-blind `find_by(external_id:)`, and Contract has no
default scope — so those write endpoints could resolve to a terminated
sibling and wrongly report `contract_locked` or act on the wrong row.

## Description

Add `Contract.live` and `Contract.live_by_external_id`, which resolve an
external id to the live contract deterministically (pending/active, newest
first), and use it in both v2 attach controllers. Cover the model resolver
and add reused-external-id request coverage to both surfaces.
## Context

Two lifecycle states the model allows made the attach lookups ambiguous.
A pending replacement can coexist with the active contract under one
external id (the partial unique index permits one of each status), and an
ended rate-card attachment can share a rate-card code with the open one.
Ordering by started_at and an unscoped card find_by would each pick
arbitrary history once those states occur.

## Description

Select the contract status explicitly in Contract.live_by_external_id —
prefer the pending contract (the authoring target), fall back to the
active one — instead of an implicit newest-first ordering. Scope both the
member and rate-phase card lookups to current_and_scheduled so an ended
attachment can no longer shadow the open one. Cover the coexistence and
ended-card cases.
@getlago getlago deleted a comment from lago-claude-ai-agent Bot Sep 3, 2026
## Context

The explicit pending-then-active resolution added earlier only diverged
from the ordered lookup when a pending and active contract coexist under
one external id — a state no current flow creates, and one the ordered
lookup already resolves correctly (a pending contract is future-dated, so
newest-first prefers it). The extra branching was not needed.

## Description

Restore `Contract.live_by_external_id` to `live.order(started_at: :desc)`
and drop the coexistence specs it was carrying. The card-lookup scoping to
`current_and_scheduled` stays — that ambiguity (an ended card sharing an
open card's rate-card code) is real and reachable.
Comment thread app/services/contract_rate_cards/create_service.rb Outdated
Comment thread app/controllers/api/v2/contract_rate_cards/rate_phases_controller.rb Outdated
Comment thread app/services/contract_rate_cards/destroy_service.rb Outdated
Comment thread app/services/contract_rate_cards/update_service.rb Outdated
Comment thread app/services/rate_phases/create_service.rb Outdated
Comment thread app/models/contract.rb Outdated
Comment thread app/services/contract_rate_cards/create_service.rb Outdated
Comment thread app/queries/contract_rate_cards_query.rb Outdated
## Context

Review of the contract applied-rate-card surface surfaced a few overlaps
and clarity issues.

## Description

- Rename `Contract#locked?` to `#editable?` (positive predicate) so it no
  longer reads like ActiveRecord pessimistic locking.
- Move the fee-currency fallback onto `Contract#currency`, and extract the
  card lifecycle seeding into `Contract#default_rate_card_lifecycle`, shared
  by the attach and materialize paths so the timezone rule lives once.
- Extract the duplicated rate-phase lock check into `RatePhases::ParentLock`.
- Scope the contract rate cards list query to `current_and_scheduled` so an
  ended card no longer appears in the index yet 404s when fetched by code.
- Drop the redundant `order(:position)` (the association already orders).
- Give the ordered live-contract lookup an id tie-breaker and correct the
  `live` scope comment (a pending replacement can coexist with the active).
## Context

The rate-phase services shared a `lock_error_code` mixin that switched on
the class of a generically named `parent` (a plan or contract rate card).
The type-switch and the "parent" name were both unclear.

## Description

Rename the services' `parent` to `applied_rate_card` — the term the API and
serializers already use — and move the frozen-state check onto the cards
themselves as `PlanRateCard#edit_error_code` and
`ContractRateCard#edit_error_code`, each returning its own lock error code
or nil. Drop the `RatePhases::ParentLock` module. Each service now reads
`applied_rate_card.edit_error_code`.
## Context

A cold review found the attach endpoints serialized contract rate cards
through a new V1::ContractRateCardSerializer while the same cards embedded
in GET /contracts/:external_id already went through
V2::ContractAppliedRateCardSerializer — so one card returned two JSON
shapes. Separately, the one-card-per-slice guard counted ended cards, which
would block re-pricing a product forever once card-ending exists.

## Description

Reuse V2::ContractAppliedRateCardSerializer on the applied-rate-card
endpoints and delete the duplicate V1 serializer, so a card serializes the
same everywhere. Scope the slice check to current_and_scheduled, matching
the open-card uniqueness the partial index enforces.
## Context

GET /api/v2/contracts/:external_id filtered by status defaulting to
active, so a pending contract returned 404 on its own detail URL even
though its nested applied-rate-card endpoints operated on it — the two
surfaces disagreed on what an external id names.

## Description

With no status filter, resolve the external id to the live contract
(pending or active), consistent with the applied-rate-card endpoints. An
explicit status still reads a specific one, including terminated and
canceled history.
Comment thread app/controllers/api/v2/contract_rate_cards_controller.rb Outdated

@tiagolupepic tiagolupepic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

one last comment and it's good to go

:shipit:

@rsempe
rsempe merged commit 1105f04 into main Sep 4, 2026
12 checks passed
@rsempe
rsempe deleted the bil-624 branch September 4, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants