From 44e725fe95ebd4bfede79a4ebf2a145cac02bf07 Mon Sep 17 00:00:00 2001 From: Shane Maglangit Date: Fri, 5 Sep 2025 13:42:53 +0800 Subject: [PATCH 1/3] Assign target_type to new push events Changelog: changed --- app/models/push_event.rb | 5 ----- app/services/event_create_service.rb | 2 +- spec/services/event_create_service_spec.rb | 12 ++++++++++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/models/push_event.rb b/app/models/push_event.rb index 4e18d4840e6da8..1b5082ac20d6be 100644 --- a/app/models/push_event.rb +++ b/app/models/push_event.rb @@ -13,11 +13,6 @@ class PushEvent < Event # should ensure the ID points to a valid project. validates :project_id, presence: true - # These fields are also not used for push events, thus storing them would be a - # waste. - validates :target_id, absence: true - validates :target_type, absence: true - delegate :branch?, to: :push_event_payload delegate :tag?, to: :push_event_payload delegate :commit_from, to: :push_event_payload diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index 3cafd2b2054701..d5d1f2a61f4cc1 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -238,7 +238,7 @@ def create_push_event(service_class, project, current_user, push_data) # when creating push payload data will result in the event creation being # rolled back as well. event = Event.transaction do - new_event = create_event(project, current_user, :pushed) + new_event = create_record_event(project, current_user, :pushed) service_class.new(new_event, push_data).execute diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index 761f7b1e30fcd4..8db8fdea0c3e5b 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -341,7 +341,11 @@ def create_event } end - subject { service.push(project, user, push_data) } + subject(:push) { service.push(project, user, push_data) } + + it 'creates new project event' do + expect { push }.to change { Event.where(target: project).count }.by(1) + end it_behaves_like 'service for creating a push event', PushEventPayloadService @@ -369,7 +373,11 @@ def create_event } end - subject { service.bulk_push(project, user, push_data) } + subject(:bulk_push) { service.bulk_push(project, user, push_data) } + + it 'creates new project event' do + expect { bulk_push }.to change { Event.where(target: project).count }.by(1) + end it_behaves_like 'service for creating a push event', BulkPushEventPayloadService -- GitLab From 3cd5e73bc8a18db2b89da12544e6c9f2163e5f81 Mon Sep 17 00:00:00 2001 From: Shane Maglangit Date: Tue, 9 Sep 2025 20:29:51 +0800 Subject: [PATCH 2/3] Fix pushed event filters --- .../ai/code_suggestion_events_finder.rb | 3 ++- .../postgresql_data_collector.rb | 11 ++++++++--- .../ai/code_suggestion_events_finder_spec.rb | 6 +++--- .../postgresql_data_collector_spec.rb | 7 ++++--- lib/event_filter.rb | 3 ++- spec/lib/event_filter_spec.rb | 19 ++++++++++--------- 6 files changed, 29 insertions(+), 20 deletions(-) diff --git a/ee/app/finders/ai/code_suggestion_events_finder.rb b/ee/app/finders/ai/code_suggestion_events_finder.rb index 24e3b9745fbd8f..55cc59431cee88 100644 --- a/ee/app/finders/ai/code_suggestion_events_finder.rb +++ b/ee/app/finders/ai/code_suggestion_events_finder.rb @@ -71,7 +71,8 @@ def contributors_ids_from_postgresql Event.pushed_action .where('created_at >= ?', 1.week.ago.beginning_of_day) .where(project_id: Project.for_group_and_its_subgroups(resource)) - .where(target_type: nil) # Filter for pushed events without targets to optimize index usage + # TODO: Remove `nil` filter once https://gitlab.com/gitlab-org/gitlab/-/issues/565789 is complete + .where(target_type: [nil, Event::TARGET_TYPES[:project].name]) .select('DISTINCT author_id') end diff --git a/ee/lib/gitlab/contribution_analytics/postgresql_data_collector.rb b/ee/lib/gitlab/contribution_analytics/postgresql_data_collector.rb index e9ccc65e03785e..2b9b3623ee92a7 100644 --- a/ee/lib/gitlab/contribution_analytics/postgresql_data_collector.rb +++ b/ee/lib/gitlab/contribution_analytics/postgresql_data_collector.rb @@ -29,8 +29,12 @@ def base_query .select('source_id AS id')) cte_condition = 'project_id IN (SELECT id FROM project_ids)' - target_type = Arel::Nodes::Case.new.when(Event.arel_table[:target_type].in(%w[Issue WorkItem])) - .then('Issue').else(Event.arel_table[:target_type]).as('target_type') + target_type = Arel::Nodes::Case.new + .when(Event.arel_table[:target_type].in(%w[Issue WorkItem])).then('Issue') + # TODO: Remove once https://gitlab.com/gitlab-org/gitlab/-/issues/565789 is complete + .when(Event.arel_table[:target_type].eq('Project')).then(Arel.sql('NULL')) + .else(Event.arel_table[:target_type]) + .as('target_type') events_from_date = ::Event .where(cte_condition) @@ -40,7 +44,8 @@ def base_query ::Event.with(cte.to_arel).from_union( [ - events_from_date.where(action: :pushed, target_type: nil), + # TODO: Remove `nil` filter once https://gitlab.com/gitlab-org/gitlab/-/issues/565789 is complete + events_from_date.where(action: :pushed, target_type: [nil, Event::TARGET_TYPES[:project].name]), events_from_date.where( action: [:created, :closed, :merged, :approved], target_type: [::MergeRequest.name, ::Issue.name, ::WorkItem.name]) diff --git a/ee/spec/finders/ai/code_suggestion_events_finder_spec.rb b/ee/spec/finders/ai/code_suggestion_events_finder_spec.rb index 0bb0928c77206b..f2ed3e6cff0ba1 100644 --- a/ee/spec/finders/ai/code_suggestion_events_finder_spec.rb +++ b/ee/spec/finders/ai/code_suggestion_events_finder_spec.rb @@ -56,16 +56,16 @@ end let_it_be(:event_2) do - create(:event, :pushed, project: project, author: user_contributor_2, target: nil, created_at: 1.day.ago) + create(:event, :pushed, project: project, author: user_contributor_2, target: project, created_at: 1.day.ago) end let_it_be(:event_3) do - create(:event, :pushed, project: project, author: user_contributor_only_on_ch, target: nil, + create(:event, :pushed, project: project, author: user_contributor_only_on_ch, target: project, created_at: 1.month.ago) end let_it_be(:event_4) do - create(:event, :created, :for_issue, project: project, author: user_not_contributor, target: nil, + create(:event, :created, :for_issue, project: project, author: user_not_contributor, target: project, created_at: 1.day.ago) end diff --git a/ee/spec/lib/gitlab/contribution_analytics/postgresql_data_collector_spec.rb b/ee/spec/lib/gitlab/contribution_analytics/postgresql_data_collector_spec.rb index a927b3fbf9385e..1c75b53916703c 100644 --- a/ee/spec/lib/gitlab/contribution_analytics/postgresql_data_collector_spec.rb +++ b/ee/spec/lib/gitlab/contribution_analytics/postgresql_data_collector_spec.rb @@ -11,11 +11,12 @@ it 'returns correct data' do # before the date range - create(:event, :pushed, project: project1, target: nil, created_at: 4.months.ago, author: user1) + create(:event, :pushed, project: project1, target: project1, created_at: 4.months.ago, author: user1) # after the date range - create(:event, :pushed, project: project1, target: nil, created_at: Date.today, author: user2) + create(:event, :pushed, project: project1, target: project1, created_at: Date.today, author: user2) # in the range range - create(:event, :pushed, project: project2, target: nil, created_at: 2.months.ago, author: user1) + create(:event, :pushed, project: project2, target: project2, created_at: 2.months.ago, author: user1) + # legacy push event create(:event, :pushed, project: project2, target: nil, created_at: 2.months.ago, author: user1) create(:event, :merged, project: project1, target: nil, created_at: 1.month.ago, author: user1, target_type: ::MergeRequest.name) diff --git a/lib/event_filter.rb b/lib/event_filter.rb index 7eea16307103ec..fcc69cf4106e97 100644 --- a/lib/event_filter.rb +++ b/lib/event_filter.rb @@ -70,7 +70,8 @@ def in_operator_query_builder_params(array_data) # > target_type IS NULL AND action = 5 AND author_id = X ORDER BY target_type DESC, id DESC in_operator_params( array_data: array_data, - scope: Event.where(target_type: nil).pushed_action, + # TODO: Remove `nil` filter once https://gitlab.com/gitlab-org/gitlab/-/issues/565789 is complete + scope: Event.where(target_type: [nil, Event::TARGET_TYPES[:project].name]).pushed_action, order_hint_column: :target_type ) when MERGED diff --git a/spec/lib/event_filter_spec.rb b/spec/lib/event_filter_spec.rb index 16612a6288cd03..f69305fab384a6 100644 --- a/spec/lib/event_filter_spec.rb +++ b/spec/lib/event_filter_spec.rb @@ -4,15 +4,16 @@ RSpec.describe EventFilter do let_it_be(:public_project) { create(:project, :public) } - let_it_be(:push_event) { create(:push_event, project: public_project) } - let_it_be(:merged_event) { create(:event, :merged, project: public_project, target: public_project) } - let_it_be(:created_event) { create(:event, :created, project: public_project, target: create(:issue, project: public_project)) } - let_it_be(:updated_event) { create(:event, :updated, project: public_project, target: create(:issue, project: public_project)) } - let_it_be(:closed_event) { create(:event, :closed, project: public_project, target: create(:issue, project: public_project)) } - let_it_be(:reopened_event) { create(:event, :reopened, project: public_project, target: create(:issue, project: public_project)) } + let_it_be(:push_event) { create(:push_event, project: public_project, target: public_project) } + let_it_be(:legacy_push_event) { create(:push_event, project: public_project, target: nil) } + let_it_be(:merged_event) { create(:event, :merged, project: public_project, target: public_project) } + let_it_be(:created_event) { create(:event, :created, project: public_project, target: create(:issue, project: public_project)) } + let_it_be(:updated_event) { create(:event, :updated, project: public_project, target: create(:issue, project: public_project)) } + let_it_be(:closed_event) { create(:event, :closed, project: public_project, target: create(:issue, project: public_project)) } + let_it_be(:reopened_event) { create(:event, :reopened, project: public_project, target: create(:issue, project: public_project)) } let_it_be(:comments_event) { create(:event, :commented, project: public_project, target: public_project) } - let_it_be(:joined_event) { create(:event, :joined, project: public_project, target: public_project) } - let_it_be(:left_event) { create(:event, :left, project: public_project, target: public_project) } + let_it_be(:joined_event) { create(:event, :joined, project: public_project, target: public_project) } + let_it_be(:left_event) { create(:event, :left, project: public_project, target: public_project) } let_it_be(:wiki_page_event) { create(:wiki_page_event) } let_it_be(:wiki_page_update_event) { create(:wiki_page_event, :updated) } let_it_be(:design_event) { create(:design_event) } @@ -47,7 +48,7 @@ let(:filter) { described_class::PUSH } it 'filters push events only' do - expect(filtered_events).to contain_exactly(push_event) + expect(filtered_events).to contain_exactly(push_event, legacy_push_event) end end -- GitLab From e854cbe59adce6e48e8e6f4a99921d13f02f5044 Mon Sep 17 00:00:00 2001 From: Shane Maglangit Date: Tue, 9 Sep 2025 21:18:19 +0800 Subject: [PATCH 3/3] Fix pushed event filters --- .../contribution_analytics/postgresql_data_collector.rb | 5 +++-- ee/spec/finders/ai/code_suggestion_events_finder_spec.rb | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ee/lib/gitlab/contribution_analytics/postgresql_data_collector.rb b/ee/lib/gitlab/contribution_analytics/postgresql_data_collector.rb index 2b9b3623ee92a7..a5621118789026 100644 --- a/ee/lib/gitlab/contribution_analytics/postgresql_data_collector.rb +++ b/ee/lib/gitlab/contribution_analytics/postgresql_data_collector.rb @@ -44,8 +44,9 @@ def base_query ::Event.with(cte.to_arel).from_union( [ - # TODO: Remove `nil` filter once https://gitlab.com/gitlab-org/gitlab/-/issues/565789 is complete - events_from_date.where(action: :pushed, target_type: [nil, Event::TARGET_TYPES[:project].name]), + # TODO: Remove nil target subquery once https://gitlab.com/gitlab-org/gitlab/-/issues/565789 is complete + events_from_date.where(action: :pushed, target_type: nil), + events_from_date.where(action: :pushed, target_type: Event::TARGET_TYPES[:project].name), events_from_date.where( action: [:created, :closed, :merged, :approved], target_type: [::MergeRequest.name, ::Issue.name, ::WorkItem.name]) diff --git a/ee/spec/finders/ai/code_suggestion_events_finder_spec.rb b/ee/spec/finders/ai/code_suggestion_events_finder_spec.rb index f2ed3e6cff0ba1..c36cef91cd4956 100644 --- a/ee/spec/finders/ai/code_suggestion_events_finder_spec.rb +++ b/ee/spec/finders/ai/code_suggestion_events_finder_spec.rb @@ -52,7 +52,7 @@ context 'when user can see code suggestion events' do let_it_be(:user) { create(:user, :with_self_managed_duo_enterprise_seat) } let_it_be(:event_1) do - create(:event, :pushed, project: project, author: user_contributor_1, target: nil, created_at: 3.days.ago) + create(:event, :pushed, project: project, author: user_contributor_1, target: project, created_at: 3.days.ago) end let_it_be(:event_2) do @@ -65,7 +65,7 @@ end let_it_be(:event_4) do - create(:event, :created, :for_issue, project: project, author: user_not_contributor, target: project, + create(:event, :created, :for_issue, project: project, author: user_not_contributor, target: nil, created_at: 1.day.ago) end -- GitLab