From c0ff257ff3cbf2b124974856f2a9ced1a757d2c3 Mon Sep 17 00:00:00 2001 From: Vincent Pochet Date: Fri, 19 Jan 2024 11:22:27 +0100 Subject: [PATCH] feat(group-by): add grouped_by_value logic to event stores --- app/services/events/stores/base_store.rb | 4 ++++ .../events/stores/clickhouse_store.rb | 6 +++++ app/services/events/stores/postgres_store.rb | 6 +++++ .../events/stores/clickhouse_store_spec.rb | 19 +++++++++++++-- .../events/stores/postgres_store_spec.rb | 23 +++++++++++++++---- 5 files changed, 51 insertions(+), 7 deletions(-) diff --git a/app/services/events/stores/base_store.rb b/app/services/events/stores/base_store.rb index eeccf8c8328a..63c406e2261e 100644 --- a/app/services/events/stores/base_store.rb +++ b/app/services/events/stores/base_store.rb @@ -17,6 +17,10 @@ def initialize(code:, subscription:, boundaries:, filters: {}) @use_from_boundary = true end + def grouped_by_value? + grouped_by.present? && grouped_by_value.present? + end + def events(force_from: false) raise NotImplementedError end diff --git a/app/services/events/stores/clickhouse_store.rb b/app/services/events/stores/clickhouse_store.rb index bbbdb6520762..3651f1934592 100644 --- a/app/services/events/stores/clickhouse_store.rb +++ b/app/services/events/stores/clickhouse_store.rb @@ -18,6 +18,8 @@ def events(force_from: false) scope = scope.where('events_raw.timestamp <= ?', to_datetime) if to_datetime scope = scope.where(numeric_condition) if numeric_property + scope = with_grouped_by_value(scope) if grouped_by_value? + return scope unless group group_scope(scope) @@ -176,6 +178,10 @@ def group_scope(scope) scope.where('events_raw.properties[?] = ?', group.parent.key.to_s => group.parent.value.to_s) end + def with_grouped_by_value(scope) + scope.where('events_raw.properties[?] = ?', grouped_by.to_s, grouped_by_value.to_s) + end + def numeric_condition ActiveRecord::Base.sanitize_sql_for_conditions( [ diff --git a/app/services/events/stores/postgres_store.rb b/app/services/events/stores/postgres_store.rb index bb8a0db36498..7b5a9b45a706 100644 --- a/app/services/events/stores/postgres_store.rb +++ b/app/services/events/stores/postgres_store.rb @@ -16,6 +16,8 @@ def events(force_from: false) .where(numeric_condition) end + scope = with_grouped_by_value(scope) if grouped_by_value? + return scope unless group group_scope(scope) @@ -126,6 +128,10 @@ def group_scope(scope) scope.where('events.properties @> ?', { group.parent.key.to_s => group.parent.value }.to_json) end + def with_grouped_by_value(scope) + scope.where('events.properties @> ?', { grouped_by.to_s => grouped_by_value.to_s }.to_json) + end + def sanitized_propery_name ActiveRecord::Base.sanitize_sql_for_conditions( ['events.properties->>?', aggregation_property], diff --git a/spec/services/events/stores/clickhouse_store_spec.rb b/spec/services/events/stores/clickhouse_store_spec.rb index c7c5a74d89de..3e4cb90df1b0 100644 --- a/spec/services/events/stores/clickhouse_store_spec.rb +++ b/spec/services/events/stores/clickhouse_store_spec.rb @@ -8,7 +8,7 @@ code:, subscription:, boundaries:, - filters: { group: }, + filters: { group:, grouped_by:, grouped_by_value: }, ) end @@ -30,13 +30,19 @@ end let(:group) { nil } + let(:grouped_by) { nil } + let(:grouped_by_value) { nil } let(:events) do events = [] 5.times do |i| properties = { billable_metric.field_name => i + 1 } - properties[group.key.to_s] = group.value.to_s if group && i.even? + + if i.even? + properties[group.key.to_s] = group.value.to_s if group + properties[grouped_by] = grouped_by_value if grouped_by_value + end events << Clickhouse::EventsRaw.create!( transaction_id: SecureRandom.uuid, @@ -80,6 +86,15 @@ expect(event_store.events.count).to eq(3) end end + + context 'with grouped_by_value' do + let(:grouped_by) { 'region' } + let(:grouped_by_value) { 'europe' } + + it 'returns a list of events' do + expect(event_store.events.count).to eq(3) + end + end end describe '.count' do diff --git a/spec/services/events/stores/postgres_store_spec.rb b/spec/services/events/stores/postgres_store_spec.rb index e0203315795a..81994fae9f2b 100644 --- a/spec/services/events/stores/postgres_store_spec.rb +++ b/spec/services/events/stores/postgres_store_spec.rb @@ -8,7 +8,7 @@ code:, subscription:, boundaries:, - filters: { group: }, + filters: { group:, grouped_by:, grouped_by_value: }, ) end @@ -30,12 +30,14 @@ end let(:group) { nil } + let(:grouped_by) { nil } + let(:grouped_by_value) { nil } let(:events) do events = [] 5.times do |i| - event = create( + event = build( :event, organization_id: organization.id, external_subscription_id: subscription.external_id, @@ -47,11 +49,13 @@ }, ) - if group && i.even? - event.properties[group.key] = group.value - event.save! + if i.even? + event.properties[group.key] = group.value if group + event.properties[grouped_by] = grouped_by_value if grouped_by_value end + event.save! + events << event end @@ -72,6 +76,15 @@ expect(event_store.events.count).to eq(3) end end + + context 'with grouped_by_value' do + let(:grouped_by) { 'region' } + let(:grouped_by_value) { 'europe' } + + it 'returns a list of events' do + expect(event_store.events.count).to eq(3) + end + end end describe '.count' do