From d5bd7f1e842491764405c7a72d1d9ff5f1a9579c Mon Sep 17 00:00:00 2001
From: Nicolas Dular <ndular@gitlab.com>
Date: Fri, 23 Feb 2024 17:19:00 +0100
Subject: [PATCH] Run validation only for synced epics
We can trim down the number of sidekiq jobs spawned by checking if the
epic also has a synced work item.
In addition we can use the new `actor_from_id` method to not query the
group but still check the feature flag.
---
ee/app/models/ee/epic.rb | 1 +
ee/lib/ee/gitlab/event_store.rb | 6 ++++--
ee/spec/models/epic_spec.rb | 9 +++++++++
.../validate_epic_work_item_sync_worker_spec.rb | 4 +++-
4 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 1d9964325d0bc..b1b6ea2413e25 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -112,6 +112,7 @@ module Epic
end
scope :with_work_item, -> { preload(:work_item) }
+ scope :has_work_item, -> { where.not(issue_id: nil) }
scope :within_timeframe, -> (start_date, end_date) do
epics = ::Epic.arel_table
diff --git a/ee/lib/ee/gitlab/event_store.rb b/ee/lib/ee/gitlab/event_store.rb
index 621c78a0262f1..e2caa889043e0 100644
--- a/ee/lib/ee/gitlab/event_store.rb
+++ b/ee/lib/ee/gitlab/event_store.rb
@@ -78,12 +78,14 @@ def subscribe_to_epic_events(store)
store.subscribe ::WorkItems::ValidateEpicWorkItemSyncWorker,
to: ::Epics::EpicCreatedEvent,
if: ->(event) {
- ::Feature.enabled?(:validate_epic_work_item_sync, ::Group.find_by_id(event.data[:group_id]))
+ ::Feature.enabled?(:validate_epic_work_item_sync, ::Group.actor_from_id(event.data[:group_id])) &&
+ ::Epic.has_work_item.id_in(event.data[:id]).exists?
}
store.subscribe ::WorkItems::ValidateEpicWorkItemSyncWorker,
to: ::Epics::EpicUpdatedEvent,
if: ->(event) {
- ::Feature.enabled?(:validate_epic_work_item_sync, ::Group.find_by_id(event.data[:group_id]))
+ ::Feature.enabled?(:validate_epic_work_item_sync, ::Group.actor_from_id(event.data[:group_id])) &&
+ ::Epic.has_work_item.id_in(event.data[:id]).exists?
}
end
end
diff --git a/ee/spec/models/epic_spec.rb b/ee/spec/models/epic_spec.rb
index e52443e86cecb..71ae462c99c13 100644
--- a/ee/spec/models/epic_spec.rb
+++ b/ee/spec/models/epic_spec.rb
@@ -137,6 +137,15 @@
expect(described_class.from_id(epic2.id)).to match_array([epic2, epic3])
end
end
+
+ describe '.has_work_item' do
+ let_it_be(:epic_with_work_item) { create(:epic, :with_synced_work_item) }
+ let_it_be(:epic_without_work_item) { create(:epic) }
+
+ it 'returns only epics with a work item' do
+ expect(described_class.has_work_item).to match_array([epic_with_work_item])
+ end
+ end
end
describe 'validations' do
diff --git a/ee/spec/workers/work_items/validate_epic_work_item_sync_worker_spec.rb b/ee/spec/workers/work_items/validate_epic_work_item_sync_worker_spec.rb
index 4eaddebf6a2aa..f1a03f4dda437 100644
--- a/ee/spec/workers/work_items/validate_epic_work_item_sync_worker_spec.rb
+++ b/ee/spec/workers/work_items/validate_epic_work_item_sync_worker_spec.rb
@@ -4,7 +4,7 @@
RSpec.describe WorkItems::ValidateEpicWorkItemSyncWorker, feature_category: :team_planning do
let_it_be(:group) { create(:group) }
- let_it_be_with_reload(:epic) { create(:epic, group: group) }
+ let_it_be_with_reload(:epic) { create(:epic, :with_synced_work_item, group: group) }
let(:data) { { id: epic.id, group_id: group.id } }
let(:epic_created_event) { Epics::EpicCreatedEvent.new(data: data) }
@@ -39,6 +39,8 @@
end
context 'when epic has no associated work item' do
+ let_it_be_with_reload(:epic) { create(:epic, group: group) }
+
it 'does not log anything or tries to create a diff' do
expect(Gitlab::EpicWorkItemSync::Logger).not_to receive(:warn)
expect(Gitlab::EpicWorkItemSync::Diff).not_to receive(:new)
--
GitLab