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

## Description

Whenever a event associated with PayInAdvance charge is recieved, we
create a Fee for this event.

The validation was performed only after we create the Fee for
`invoiceable: false` charges creating some disparities if the charge is
invoiceable or not.

I realized that there is already a validation on the aggregation in the
Charge model, which I refactored.

~I have removed all the validation in the PostProcess events because,
you're not supposed to be able to create invalid charge (see Charge
model validation) and if you send events without the
`properties[bm.field_name]`, I'm not sure it should fail silently.~ It
creates a fee even without valid properties so I'm keeping the condition
as-is, but earlier in the function.

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?

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?

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:})

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

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 }