misc: remove groups from event post validation (#2280)

## Context The refresh of `last_hour_events_mv` can take a lot of time in our cloud instances. Some OSS users also reported issues with it. In some conditions the refresh could take more than one hour, leading missing validation on some events since the validation is supposed to refresh the view and loop over the event every hour. After investigation it appears that the issue comes mainly from the group validation. ## Description Since `groups` is now legacy and has been replaced by `filters`, it does not make any sense to keep this validation. This PR simply removes all the validation logic related to `groups`
getlago · Jul 11, 2024 · d15b8b9 · d15b8b9
1 parent 7e497f5
commit d15b8b9
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 120 deletions.
diff --git a/app/services/events/post_validation_service.rb b/app/services/events/post_validation_service.rb
@@ -12,13 +12,11 @@ def call
       errors = {
         invalid_code: process_query(invalid_code_query),
         missing_aggregation_property: process_query(missing_aggregation_property_query),
-        missing_group_key: process_query(missing_group_key_query),
         invalid_filter_values: process_query(invalid_filter_values_query)
       }
 
       if errors[:invalid_code].present? ||
           errors[:missing_aggregation_property].present? ||
-          errors[:missing_group_key].present? ||
           errors[:invalid_filter_values].present?
         deliver_webhook(errors)
       end
@@ -58,24 +56,6 @@ def missing_aggregation_property_query
       SQL
     end
 
-    def missing_group_key_query
-      <<-SQL
-        SELECT DISTINCT transaction_id
-        FROM last_hour_events_mv
-        WHERE organization_id = '#{organization.id}'
-          AND (
-            (
-              parent_group_mandatory = 't'
-              AND has_parent_group_key = 'f'
-            )
-            OR (
-              child_group_mandatory = 't'
-              AND has_child_group_key = 'f'
-            )
-          )
-      SQL
-    end
-
     def invalid_filter_values_query
       <<-SQL
         SELECT DISTINCT transaction_id

diff --git a/db/migrate/20240708081356_update_last_hour_events_mv_v04.rb b/db/migrate/20240708081356_update_last_hour_events_mv_v04.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class UpdateLastHourEventsMvV04 < ActiveRecord::Migration[7.1]
+  def change
+    drop_view :last_hour_events_mv, materialized: true
+    create_view :last_hour_events_mv, materialized: true, version: 4
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
diff --git a/db/views/last_hour_events_mv_v04.sql b/db/views/last_hour_events_mv_v04.sql
@@ -0,0 +1,37 @@
+WITH billable_metric_filters as (
+  SELECT
+    billable_metrics.organization_id as bm_organization_id,
+    billable_metrics.id AS bm_id,
+    billable_metrics.code AS bm_code,
+    filters.key AS filter_key,
+    filters.values AS filter_values
+  FROM billable_metrics
+    INNER JOIN billable_metric_filters filters
+ 		  ON filters.billable_metric_id = billable_metrics.id
+  WHERE
+    billable_metrics.deleted_at IS NULL
+    AND filters.deleted_at IS NULL
+)
+
+
+SELECT
+  events.organization_id,
+  events.transaction_id,
+  events.properties,
+  billable_metrics.code AS billable_metric_code,
+  billable_metrics.aggregation_type != 0 AS field_name_mandatory,
+  billable_metrics.aggregation_type IN (1,2,5,6) AS numeric_field_mandatory,
+  events.properties ->> billable_metrics.field_name::text AS field_value,
+  events.properties ->> billable_metrics.field_name::text ~ '^-?\d+(\.\d+)?$' AS is_numeric_field_value,
+  events.properties ? billable_metric_filters.filter_key as has_filter_keys,
+  (events.properties ->> billable_metric_filters.filter_key) = ANY (billable_metric_filters.filter_values) as has_valid_filter_values
+FROM
+  events
+    LEFT JOIN billable_metrics ON billable_metrics.code = events.code
+      AND events.organization_id = billable_metrics.organization_id
+    LEFT JOIN billable_metric_filters ON billable_metrics.id = billable_metric_filters.bm_id
+WHERE
+  events.deleted_at IS NULL
+  AND events.created_at >= date_trunc('hour', NOW()) - INTERVAL '1 hour'
+  AND events.created_at < date_trunc('hour', NOW())
+  AND billable_metrics.deleted_at IS NULL
diff --git a/spec/services/events/post_validation_service_spec.rb b/spec/services/events/post_validation_service_spec.rb
@@ -43,27 +43,6 @@
     )
   end
 
-  let(:billable_metric_with_group) do
-    create(
-      :sum_billable_metric,
-      organization:
-    )
-  end
-
-  let(:parent_group) do
-    create(:group, billable_metric: billable_metric_with_group)
-  end
-
-  let(:child_group) do
-    create(
-      :group,
-      billable_metric: billable_metric_with_group,
-      parent: parent_group,
-      key: 'provider',
-      value: 'aws'
-    )
-  end
-
   let(:billable_metric_with_filter) do
     create(
       :billable_metric,
@@ -80,29 +59,6 @@
     )
   end
 
-  let(:missing_parent_group_key_event) do
-    create(
-      :event,
-      organization:,
-      code: billable_metric_with_group.code,
-      properties: {billable_metric_with_group.field_name => 12},
-      created_at: Time.current.beginning_of_hour - 25.minutes
-    )
-  end
-
-  let(:missing_child_group_key_event) do
-    create(
-      :event,
-      organization:,
-      code: billable_metric_with_group.code,
-      properties: {
-        parent_group.key => parent_group.value,
-        billable_metric_with_group.field_name => 12
-      },
-      created_at: Time.current.beginning_of_hour - 25.minutes
-    )
-  end
-
   let(:invalid_filter_values_event) do
     create(
       :event,
@@ -114,13 +70,9 @@
   end
 
   before do
-    child_group
-
     invalid_code_event
     missing_aggregation_property_event
     negative_aggregation_property_event
-    missing_parent_group_key_event
-    missing_child_group_key_event
     invalid_filter_values_event
 
     Scenic.database.refresh_materialized_view(
@@ -150,11 +102,6 @@
         .to include(missing_aggregation_property_event.transaction_id)
       expect(result.errors[:missing_aggregation_property])
         .not_to include(negative_aggregation_property_event.transaction_id)
-      expect(result.errors[:missing_group_key])
-        .to include(
-          missing_parent_group_key_event.transaction_id,
-          missing_child_group_key_event.transaction_id
-        )
       expect(result.errors[:invalid_filter_values]).to include(invalid_filter_values_event.transaction_id)
     end
 
@@ -168,7 +115,6 @@
           errors: {
             invalid_code: [invalid_code_event.transaction_id],
             missing_aggregation_property: [missing_aggregation_property_event.transaction_id],
-            missing_group_key: Array,
             invalid_filter_values: [invalid_filter_values_event.transaction_id]
           }
         )