feat: persist due billing segments - #6348
brunomiguelpinto wants to merge 2 commits into
Conversation
|
Automated pre-review (advisory, not a required check) — verdict: HOLD · CI green HOLD —
|
| # after termination shortens it. Select against the unshortened | ||
| # pricing windows, then persist the actual service boundaries. | ||
| full_cycle = cycle.with(ended_at: cycle.calendar.interval_containing(cycle.started_at).end) | ||
| eligible_starts = billable_segments_of([full_cycle]).filter_map do |segment| |
There was a problem hiding this comment.
convert this to a Set .to_set to avoid O(MxN) in the line 35
| class ScheduleService < BaseService | ||
| Result = BaseResult[:billing_segments] | ||
|
|
||
| def initialize(customer:, timestamp: Time.current) |
There was a problem hiding this comment.
better to use a range here, having a single timestamp we don't cover the option to generate segments for past dates/segments.
| def persist_segment(card, segment) | ||
| # Resuming replays the last cycle, which can contain several segments. | ||
| # Keep existing snapshots and statuses intact, including on a stale-clock retry. | ||
| if card.billing_segments.exists?(started_at: segment.started_at) |
There was a problem hiding this comment.
Could we handle the known billing-segment overlap constraint as a structured service failure?
persist_segment only skips existing rows with the same started_at. If an existing segment overlaps the candidate but starts at a different instant, create! raises ActiveRecord::StatementInvalid wrapping PG::ExclusionViolation. Neither rescue in call handles that exception.
The transaction still rolls back correctly, but the caller receives an exception instead of a failed Result. The legacy BillingCycles::ScheduleService explicitly handles its known period conflicts and returns overlapping_periods.
Could we rescue the specific billing_segments_no_overlapping_periods constraint violation, return a structured failure, and add a regression for a different-start overlap? Unrelated database errors should continue to propagate.
| return | ||
| end | ||
|
|
||
| card.billing_segments.create!( |
There was a problem hiding this comment.
I think we can use a import! here and hit the DB into a single shot. Using batches as well
| billing_at: segment.billing_at, | ||
| rate_card_rate: segment.rate, | ||
| rate_override: segment.rate_override, | ||
| rate_properties: (segment.rate_override || segment.rate).properties, |
There was a problem hiding this comment.
move this to segment.
segment.properties
| rate_override: segment.rate_override, | ||
| rate_properties: (segment.rate_override || segment.rate).properties, | ||
| currency: card.rate_card.currency, | ||
| pricing_unit: pricing_unit(card), |
There was a problem hiding this comment.
pricing_unit(card) runs customer.organization.pricing_units.find_by(code:) once per persisted segment, but the result depends only on the card, not the segment. On a catch-up run that persists several segments for the same card (e.g. the delayed-run and phase-transition cases), this fires the same query repeatedly. Consider resolving the pricing unit once per card in schedule_card and passing it into persist_segment, so the lookup (and its pricing_unit_not_found failure) happens a single time per card.
| ContractRateCard.joins(:contract) | ||
| .where(organization_id: customer.organization_id, next_billing_at: ..timestamp) | ||
| .where(contracts: {customer_id: customer.id, status: %w[active terminated], started_at: ..timestamp}) | ||
| .includes(:rate_card, contract: :customer) |
There was a problem hiding this comment.
isn't missing to load the rate_phases here?
| end | ||
|
|
||
| def schedule_card(card) | ||
| schedule = Billing::RateCards::BuildScheduleService.call!(contract_rate_card: card).schedule |
There was a problem hiding this comment.
there is a small issue here. If a customer has multiple contract rate cards with configured rate_card in each of them plus rate card rates. But a single one does not have a single rate_card_rates, the entire schedule will fail due this error https://github.com/getlago/lago-api/blob/billing-segments-scheduler/app/services/billing/rate_cards/build_schedule_service.rb#L22
Basically, it's all or nothing for a single customer. We should evaluate this to ignore that missing one
|
|
||
| attr_reader :customer, :timestamp | ||
|
|
||
| def due_cards |
There was a problem hiding this comment.
From AI:
The scheduler preloads the rate card and contract/customer, but the builder additionally loads rates, phases, overrides, and potentially plan-applied cards. Candidate segments each trigger an existence check; pricing-unit lookup repeats for each new segment.
With C cards, P phases per card, and S candidate segments, these paths can generate O(CP + S) read queries alongside required writes.
Consider preloading the builder's association graph, fetching existing starts in bulk, and resolving pricing units once per distinct code.
af82234 to
9027bf0
Compare
The billing calendar needs a durable scheduling step before its segments can be processed into invoices. Persist due segments and advance contract rate card clocks atomically under a customer lock. Preserve existing snapshots on retries and respect the initial billing period for backdated contracts. Allow exhausted schedules to clear their next billing date and expose that state through REST and GraphQL. Cover scheduling, rollback, pricing references and API serialization with focused tests.
## Context Termination can shorten an arrears period before its saved billing date. Filtering against the shortened end would omit the final segment and clear the clock. ## Description Apply the billing bound in the calendar using the full pricing window while preserving the actual service end and proration. Keep periods before the initial billing bound excluded. Add coverage for shortened contracts and cards, concurrent scheduling, phase transitions and partial periods without proration.
9027bf0 to
c19680a
Compare
The billing calendar can calculate due segments, but it does not persist them or advance the billing clock. Add
BillingSegments::ScheduleServiceto save those segments aspending, ready for a later invoice-processing step.For example, an arrears card attached on January 15 with a monthly January 1 anchor produces a January 15–February 1 segment when run on February 1, stores the calendar's
17/31proration ratio, and movesnext_billing_atto March 1. Repeating the run creates no duplicate segments.Changes
next_billing_at = NULL, including the model, REST serializer and GraphQL schema. Existing dates remain unchanged by the migration; clients must accept null once a schedule completes.This is the segment-persistence slice of #5836, adapted to
ContractRateCardandBillingSegment. It introduces no jobs, clock entry points, fee calculation or invoice creation.Validation