Skip to content
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

feat(events): Refactor PayInAdvance Billable Metric validation #2143

Merged
merged 1 commit into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/models/billable_metric.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class BillableMetric < ApplicationRecord
latest_agg: 6,
custom_agg: 7
}.freeze
AGGREGATION_TYPES_PAYABLE_IN_ADVANCE = %i[count_agg sum_agg unique_count_agg custom_agg].freeze

WEIGHTED_INTERVAL = {seconds: 'seconds'}.freeze

Expand Down Expand Up @@ -92,6 +93,10 @@ def active_groups_as_tree
}
end

def payable_in_advance?
AGGREGATION_TYPES_PAYABLE_IN_ADVANCE.include?(aggregation_type.to_sym)
end

private

def should_have_field_name?
Expand Down
13 changes: 5 additions & 8 deletions app/models/charge.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,12 @@ def validate_charge_model(validator)
.each { |code| errors.add(:properties, code) }
end

# NOTE: An pay_in_advance charge cannot be created in the following cases:
# - billable metric aggregation type is max_agg or weighted_sum_agg
# - charge model is volume
def validate_pay_in_advance
return unless pay_in_advance?

return unless %w[max_agg weighted_sum_agg latest_agg].include?(billable_metric.aggregation_type) || volume?

errors.add(:pay_in_advance, :invalid_aggregation_type_or_charge_model)
if volume? || !billable_metric.payable_in_advance?
errors.add(:pay_in_advance, :invalid_aggregation_type_or_charge_model)
end
end

def validate_min_amount_cents
Expand All @@ -108,8 +105,8 @@ def validate_min_amount_cents
end

# NOTE: A prorated charge cannot be created in the following cases:
# - for pay_in_arrear, price model cannot be package, graduated and percentage
# - for pay_in_idvance, price model cannot be package, graduated, percentage and volume
# - for pay_in_arrears, price model cannot be package, graduated and percentage
# - for pay_in_advance, price model cannot be package, graduated, percentage and volume
# - for weighted_sum aggregation as it already apply pro-ration logic
def validate_prorated
return unless prorated?
Expand Down
18 changes: 10 additions & 8 deletions app/services/events/post_process_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ def call
result.event = event
result
rescue ActiveRecord::RecordInvalid => e
delivor_error_webhook(error: e.record.errors.messages)
deliver_error_webhook(error: e.record.errors.messages)

result
rescue ActiveRecord::RecordNotUnique
delivor_error_webhook(error: {transaction_id: ['value_already_exist']})
deliver_error_webhook(error: {transaction_id: ['value_already_exist']})

result
end
Expand Down Expand Up @@ -97,16 +97,18 @@ def expire_cached_charges(subscriptions)
def handle_pay_in_advance
return unless billable_metric

charges.where(invoiceable: false).find_each do |charge|
Fees::CreatePayInAdvanceJob.perform_later(charge:, event:)
end

# NOTE: ensure event is processable
# NOTE: `custom_agg` and `count_agg` are the only 2 aggregations
# that don't require a field set in property.
# For other aggregation, if the field isn't set we shouldn't create a fee/invoice.
processable_event = billable_metric.count_agg? ||
julienbourdeau marked this conversation as resolved.
Show resolved Hide resolved
billable_metric.custom_agg? ||
event.properties[billable_metric.field_name].present?
return unless processable_event

charges.where(invoiceable: false).find_each do |charge|
Fees::CreatePayInAdvanceJob.perform_later(charge:, event:)
end

charges.where(invoiceable: true).find_each do |charge|
Invoices::CreatePayInAdvanceChargeJob.perform_later(charge:, event:, timestamp: event.timestamp)
end
Expand All @@ -124,7 +126,7 @@ def charges
.where(billable_metric: {code: event.code})
end

def delivor_error_webhook(error:)
def deliver_error_webhook(error:)
return unless organization.webhook_endpoints.any?

SendWebhookJob.perform_later('event.error', event, {error:})
Expand Down
12 changes: 12 additions & 0 deletions spec/models/billable_metric_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,16 @@
end
end
end

describe '#payable_in_advance?' do
it do
described_class::AGGREGATION_TYPES_PAYABLE_IN_ADVANCE.each do |agg|
expect(build(:billable_metric, aggregation_type: agg)).to be_payable_in_advance
end

(described_class::AGGREGATION_TYPES.keys - described_class::AGGREGATION_TYPES_PAYABLE_IN_ADVANCE).each do |agg|
expect(build(:billable_metric, aggregation_type: agg)).not_to be_payable_in_advance
end
end
end
end
5 changes: 1 addition & 4 deletions spec/services/events/post_process_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,7 @@

context 'when event matches an pay_in_advance charge that is not invoiceable' do
let(:charge) { create(:standard_charge, :pay_in_advance, plan:, billable_metric:, invoiceable: false) }
let(:billable_metric) do
create(:billable_metric, organization:, aggregation_type: 'sum_agg', field_name: 'item_id')
end

let(:billable_metric) { create(:billable_metric, organization:, aggregation_type: 'sum_agg', field_name: 'item_id') }
let(:event_properties) { {billable_metric.field_name => '12'} }

before { charge }
Expand Down
Loading