-
Notifications
You must be signed in to change notification settings - Fork 190
feat: persist due billing segments #6348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module BillingSegments | ||
| # Persists the calendar's due slices. The clock is both the lower billing | ||
| # bound (backdated contracts can join the current period) and the next wake-up. | ||
| class ScheduleService < BaseService | ||
| Result = BaseResult[:billing_segments] | ||
|
|
||
| def initialize(customer:, timestamp: Time.current) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better to use a range here, having a single timestamp we don't cover the option to generate segments for past dates/segments. |
||
| @customer = customer | ||
| @timestamp = timestamp | ||
| super | ||
| end | ||
|
|
||
| def call | ||
| result.billing_segments = [] | ||
|
|
||
| if customer.nil? | ||
| return result.not_found_failure!(resource: "customer") | ||
| end | ||
|
|
||
| scheduled = [] | ||
| ActiveRecord::Base.transaction do | ||
| Customers::LockService.call!(customer:, scope: :billing_schedule) do | ||
| due_cards.find_each do |card| | ||
| scheduled.concat(schedule_card(card)) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| result.billing_segments = scheduled | ||
| result | ||
| rescue ActiveRecord::RecordInvalid => error | ||
| result.record_validation_failure!(record: error.record) | ||
| rescue BaseService::FailedResult => error | ||
| result.fail_with_error!(error) | ||
| end | ||
|
|
||
| private | ||
|
|
||
| attr_reader :customer, :timestamp | ||
|
|
||
| def due_cards | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From AI: |
||
| 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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't missing to load the rate_phases here? |
||
| end | ||
|
|
||
| def schedule_card(card) | ||
| schedule = Billing::RateCards::BuildScheduleService.call!(contract_rate_card: card).schedule | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| scheduled = schedule.segments_due_by(timestamp, billing_from: card.next_billing_at).filter_map do |segment| | ||
| persist_segment(card, segment) | ||
| end | ||
|
|
||
| # nil means the schedule is exhausted, so this card is no longer due. | ||
| card.update!(next_billing_at: schedule.next_billing_at(after: timestamp)) | ||
| scheduled | ||
| end | ||
|
|
||
| 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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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!( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can use a import! here and hit the DB into a single shot. Using batches as well |
||
| organization: customer.organization, | ||
| customer:, | ||
| contract: card.contract, | ||
| cycle_started_at: segment.cycle_started_at, | ||
| started_at: segment.started_at, | ||
| ended_at: BillingSegment.inclusive_end(segment.ended_at), | ||
| billing_at: segment.billing_at, | ||
| rate_card_rate: segment.rate, | ||
| rate_override: segment.rate_override, | ||
| rate_properties: (segment.rate_override || segment.rate).properties, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move this to segment. |
||
| currency: card.rate_card.currency, | ||
| pricing_unit: pricing_unit(card), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| proration_ratio: segment.proration_ratio, | ||
| status: :pending | ||
| ) | ||
| end | ||
|
|
||
| def pricing_unit(card) | ||
| code = card.rate_card.applied_pricing_unit_code | ||
|
|
||
| if code.present? | ||
| unit = customer.organization.pricing_units.find_by(code:) | ||
|
|
||
| if unit.nil? | ||
| result.not_found_failure!(resource: "pricing_unit").raise_if_error! | ||
| end | ||
|
|
||
| unit | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| class AllowExhaustedContractRateCardSchedules < ActiveRecord::Migration[8.0] | ||
| def change | ||
| change_column_null :contract_rate_cards, :next_billing_at, true | ||
| end | ||
| end |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
convert this to a Set
.to_setto avoid O(MxN) in the line 35