diff --git a/app/models/billable_metric.rb b/app/models/billable_metric.rb index 6190d7f60b2..928f803251f 100644 --- a/app/models/billable_metric.rb +++ b/app/models/billable_metric.rb @@ -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 @@ -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? diff --git a/app/models/charge.rb b/app/models/charge.rb index b760bd56b54..3a728c202b6 100644 --- a/app/models/charge.rb +++ b/app/models/charge.rb @@ -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 @@ -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? diff --git a/app/services/events/post_process_service.rb b/app/services/events/post_process_service.rb index 5856973fe13..15199b0d213 100644 --- a/app/services/events/post_process_service.rb +++ b/app/services/events/post_process_service.rb @@ -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 @@ -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? || 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 @@ -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:}) diff --git a/spec/models/billable_metric_spec.rb b/spec/models/billable_metric_spec.rb index c5e89cd6b37..95b2796b464 100644 --- a/spec/models/billable_metric_spec.rb +++ b/spec/models/billable_metric_spec.rb @@ -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 diff --git a/spec/services/events/post_process_service_spec.rb b/spec/services/events/post_process_service_spec.rb index 52bbdbe7d83..f1453a2e269 100644 --- a/spec/services/events/post_process_service_spec.rb +++ b/spec/services/events/post_process_service_spec.rb @@ -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 }