diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 47461dae0cdc2a60f116ef95a7083fecea254d62..2233e6ba322814b5306b566299b758271395b716 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -815,6 +815,8 @@ - 1 - - work_items_rolledup_dates_update_rolledup_dates - 1 +- - work_items_rolledup_dates_update_rolledup_dates_event_handler + - 1 - - work_items_update_parent_objectives_progress - 1 - - work_items_validate_epic_work_item_sync diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index af3bacc29b0151625c7633e259b4917c2cda4098..58cb2426768587f0fa42559c897c945ab815864c 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -2334,6 +2334,15 @@ :weight: 1 :idempotent: true :tags: [] +- :name: work_items_rolledup_dates_update_rolledup_dates_event_handler + :worker_name: WorkItems::RolledupDates::UpdateRolledupDatesEventHandler + :feature_category: :portfolio_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: work_items_update_parent_objectives_progress :worker_name: WorkItems::UpdateParentObjectivesProgressWorker :feature_category: :team_planning diff --git a/ee/app/workers/work_items/rolledup_dates/update_parent_rolledup_dates_event_handler.rb b/ee/app/workers/work_items/rolledup_dates/update_parent_rolledup_dates_event_handler.rb index 53dd1b3a528c25ad888c2e5ec6c3d8b8af9a7e0c..a39f0ded511092ceb753a000fefb06b04e64aab6 100644 --- a/ee/app/workers/work_items/rolledup_dates/update_parent_rolledup_dates_event_handler.rb +++ b/ee/app/workers/work_items/rolledup_dates/update_parent_rolledup_dates_event_handler.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +# TODO: to be removed at 17.0 module WorkItems module RolledupDates class UpdateParentRolledupDatesEventHandler @@ -9,31 +10,7 @@ class UpdateParentRolledupDatesEventHandler feature_category :portfolio_management idempotent! - UPDATE_TRIGGER_ATTRIBUTES = %w[ - start_date - due_date - milestone - milestone_id - ].freeze - - UPDATE_TRIGGER_WIDGETS = %w[ - start_and_due_date_widget - rolledup_dates_widget - ].freeze - - def self.can_handle_update?(event) - UPDATE_TRIGGER_WIDGETS.any? { |widget| event.data.fetch(:updated_widgets, []).include?(widget) } || - UPDATE_TRIGGER_ATTRIBUTES.any? { |attribute| event.data.fetch(:updated_attributes, []).include?(attribute) } - end - - def handle_event(event) - parent = ::WorkItem.find_by_id(event.data[:id])&.work_item_parent - return if parent.blank? - - ::WorkItems::Widgets::RolledupDatesService::HierarchyUpdateService - .new(parent) - .execute - end + def handle_event(event); end end end end diff --git a/ee/app/workers/work_items/rolledup_dates/update_rolledup_dates_event_handler.rb b/ee/app/workers/work_items/rolledup_dates/update_rolledup_dates_event_handler.rb new file mode 100644 index 0000000000000000000000000000000000000000..8587a3326a0bcadf0c8d8238323ee0091d3848f5 --- /dev/null +++ b/ee/app/workers/work_items/rolledup_dates/update_rolledup_dates_event_handler.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module WorkItems + module RolledupDates + class UpdateRolledupDatesEventHandler + include Gitlab::EventStore::Subscriber + + data_consistency :always + feature_category :portfolio_management + idempotent! + + UPDATE_TRIGGER_ATTRIBUTES = %w[ + start_date + due_date + milestone + milestone_id + ].freeze + + UPDATE_TRIGGER_WIDGETS = %w[ + start_and_due_date_widget + rolledup_dates_widget + hierarchy_widget + ].freeze + + def self.can_handle_update?(event) + UPDATE_TRIGGER_WIDGETS.any? { |widget| event.data.fetch(:updated_widgets, []).include?(widget) } || + UPDATE_TRIGGER_ATTRIBUTES.any? { |attribute| event.data.fetch(:updated_attributes, []).include?(attribute) } + end + + def handle_event(event) + id = event.data[:work_item_parent_id].presence || event.data[:id] + work_item = ::WorkItem.find_by_id(id) + return if work_item.blank? + + ::WorkItems::Widgets::RolledupDatesService::HierarchyUpdateService + .new(work_item) + .execute + end + end + end +end diff --git a/ee/lib/ee/gitlab/event_store.rb b/ee/lib/ee/gitlab/event_store.rb index e4977fa595a0aec876daa56ceeb6d8baeb6032a7..fac65de8b502f062aeef81a82ba9569f1cd6a1e7 100644 --- a/ee/lib/ee/gitlab/event_store.rb +++ b/ee/lib/ee/gitlab/event_store.rb @@ -99,13 +99,13 @@ def subscribe_to_external_issue_links_events(store) end def subscribe_to_work_item_events(store) - store.subscribe ::WorkItems::RolledupDates::UpdateParentRolledupDatesEventHandler, + store.subscribe ::WorkItems::RolledupDates::UpdateRolledupDatesEventHandler, to: ::WorkItems::WorkItemCreatedEvent - store.subscribe ::WorkItems::RolledupDates::UpdateParentRolledupDatesEventHandler, + store.subscribe ::WorkItems::RolledupDates::UpdateRolledupDatesEventHandler, to: ::WorkItems::WorkItemDeletedEvent - store.subscribe ::WorkItems::RolledupDates::UpdateParentRolledupDatesEventHandler, + store.subscribe ::WorkItems::RolledupDates::UpdateRolledupDatesEventHandler, to: ::WorkItems::WorkItemUpdatedEvent, if: ->(event) { - ::WorkItems::RolledupDates::UpdateParentRolledupDatesEventHandler.can_handle_update?(event) + ::WorkItems::RolledupDates::UpdateRolledupDatesEventHandler.can_handle_update?(event) } end end diff --git a/ee/spec/services/work_items/widgets/rolledup_dates_service/hierarchy_update_service_spec.rb b/ee/spec/services/work_items/widgets/rolledup_dates_service/hierarchy_update_service_spec.rb index 51b4fecd08d8ea32898648018d7f069cd260062c..1dd6ecd6e64cc3b1aeb5890d1eb3a882453d7552 100644 --- a/ee/spec/services/work_items/widgets/rolledup_dates_service/hierarchy_update_service_spec.rb +++ b/ee/spec/services/work_items/widgets/rolledup_dates_service/hierarchy_update_service_spec.rb @@ -7,18 +7,12 @@ feature_category: :portfolio_management do let_it_be(:group) { create(:group) } let_it_be_with_reload(:work_item) { create(:work_item, :epic, namespace: group) } - let_it_be_with_reload(:child_work_item) do - now = Time.now.utc - create(:work_item, :issue, namespace: group, start_date: now, due_date: now).tap do |child| - create(:parent_link, work_item: child, work_item_parent: work_item) - end - end shared_examples "does not update work_item's date_source" do specify do expect { described_class.new(work_item).execute } .to not_change { work_item&.reload&.dates_source&.start_date } - .and not_change { work_item&.reload&.dates_source&.due_date } + .and not_change { work_item&.reload&.dates_source&.due_date } end end @@ -42,7 +36,7 @@ specify do expect(::WorkItems::RolledupDates::UpdateRolledupDatesWorker) .to receive(:perform_async) - .with(parent.id) + .with(parent.id) described_class.new(work_item).execute end @@ -54,6 +48,19 @@ work_item.dates_source.delete end + context "when rolling up start and due date" do + before do + create(:work_items_dates_source, work_item: work_item) + end + + it "does not update due date" do + expect { described_class.new(work_item).execute } + .to not_change { WorkItems::DatesSource.count } + .and change { work_item.reload.dates_source&.due_date }.from(nil).to(due_date) + .and change { work_item.reload.dates_source&.start_date }.from(nil).to(start_date) + end + end + context "when due date is fixed" do before do create(:work_items_dates_source, work_item: work_item, due_date_is_fixed: true) @@ -117,7 +124,7 @@ let_it_be(:due_date) { 1.day.from_now.to_date } context "when rolling up from child work_item dates fields" do - before do + before_all do create(:work_item, :issue, namespace: group, start_date: start_date, due_date: due_date).tap do |child| create(:parent_link, work_item: child, work_item_parent: work_item) end @@ -129,7 +136,7 @@ end context "when rolling up from from a child work_item dates_source fields" do - before do + before_all do create(:work_item, :issue, namespace: group).tap do |child| create(:parent_link, work_item: child, work_item_parent: work_item) create(:work_items_dates_source, :fixed, work_item: child, start_date: start_date, due_date: due_date) @@ -146,7 +153,7 @@ create(:milestone, group: group, start_date: start_date, due_date: due_date) end - before do + before_all do create(:work_item, :issue, namespace: group, milestone: milestone).tap do |child| create(:parent_link, work_item: child, work_item_parent: work_item) create( diff --git a/ee/spec/workers/work_items/rolledup_dates/update_parent_rolledup_dates_event_handler_spec.rb b/ee/spec/workers/work_items/rolledup_dates/update_parent_rolledup_dates_event_handler_spec.rb deleted file mode 100644 index 1c7cb769576314bc9ab38b5342b700829166571f..0000000000000000000000000000000000000000 --- a/ee/spec/workers/work_items/rolledup_dates/update_parent_rolledup_dates_event_handler_spec.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe WorkItems::RolledupDates::UpdateParentRolledupDatesEventHandler, feature_category: :portfolio_management do - describe '.can_handle_update?', :aggregate_failures do - it 'returns false if no expected widget or attribute changed' do - event = ::WorkItems::WorkItemCreatedEvent.new(data: { id: 1, namespace_id: 2 }) - expect(described_class.can_handle_update?(event)).to eq(false) - end - - it 'returns true when expected attribute changed' do - described_class::UPDATE_TRIGGER_ATTRIBUTES.each do |attribute| - event = ::WorkItems::WorkItemCreatedEvent.new(data: { - id: 1, - namespace_id: 2, - updated_attributes: [attribute] - }) - - expect(described_class.can_handle_update?(event)).to eq(true) - end - end - - it 'returns true when expected widget changed' do - described_class::UPDATE_TRIGGER_WIDGETS.each do |widget| - event = ::WorkItems::WorkItemCreatedEvent.new(data: { - id: 1, - namespace_id: 2, - updated_widgets: [widget] - }) - - expect(described_class.can_handle_update?(event)).to eq(true) - end - end - end - - describe "handle_event" do - context "when work item has a parent" do - it "updates the work_item hierarchy" do - parent = instance_double(::WorkItem) - - expect(::WorkItem) - .to receive(:find_by_id) - .with(1) - .and_return(instance_double(::WorkItem, work_item_parent: parent)) - - expect_next_instance_of(::WorkItems::Widgets::RolledupDatesService::HierarchyUpdateService, parent) do |service| - expect(service).to receive(:execute) - end - - event = ::WorkItems::WorkItemCreatedEvent.new(data: { id: 1, namespace_id: 2 }) - - handler = described_class.new - handler.handle_event(event) - end - end - - context "when work item does not have a parent" do - it "updates the work_item hierarchy" do - expect(::WorkItem) - .to receive(:find_by_id) - .with(1) - .and_return(instance_double(::WorkItem, work_item_parent: nil)) - - expect(::WorkItems::Widgets::RolledupDatesService::HierarchyUpdateService) - .not_to receive(:new) - - event = ::WorkItems::WorkItemCreatedEvent.new(data: { id: 1, namespace_id: 2 }) - - handler = described_class.new - handler.handle_event(event) - end - end - - context 'when the work item no longer exists' do - it 'does not raise an error' do - event = ::WorkItems::WorkItemCreatedEvent.new(data: { id: non_existing_record_id, namespace_id: 2 }) - - handler = described_class.new - - expect { handler.handle_event(event) }.not_to raise_error - end - end - end -end diff --git a/ee/spec/workers/work_items/rolledup_dates/update_rolledup_dates_event_handler_spec.rb b/ee/spec/workers/work_items/rolledup_dates/update_rolledup_dates_event_handler_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..450ea7a058917ea71c194abd258bb9cec8a9a967 --- /dev/null +++ b/ee/spec/workers/work_items/rolledup_dates/update_rolledup_dates_event_handler_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe WorkItems::RolledupDates::UpdateRolledupDatesEventHandler, feature_category: :portfolio_management do + describe ".can_handle_update?", :aggregate_failures do + it "returns false if no expected widget or attribute changed" do + event = ::WorkItems::WorkItemCreatedEvent.new(data: { id: 1, namespace_id: 2 }) + expect(described_class.can_handle_update?(event)).to eq(false) + end + + it "returns true when expected attribute changed" do + described_class::UPDATE_TRIGGER_ATTRIBUTES.each do |attribute| + event = ::WorkItems::WorkItemCreatedEvent.new(data: { + id: 1, + namespace_id: 2, + updated_attributes: [attribute] + }) + + expect(described_class.can_handle_update?(event)).to eq(true) + end + end + + it "returns true when expected widget changed" do + described_class::UPDATE_TRIGGER_WIDGETS.each do |widget| + event = ::WorkItems::WorkItemCreatedEvent.new(data: { + id: 1, + namespace_id: 2, + updated_widgets: [widget] + }) + + expect(described_class.can_handle_update?(event)).to eq(true) + end + end + end + + describe "handle_event" do + let_it_be(:service_class) { ::WorkItems::Widgets::RolledupDatesService::HierarchyUpdateService } + + it "does nothing when the work item no longer exists" do + expect(service_class).not_to receive(:new) + + event = ::WorkItems::WorkItemCreatedEvent.new(data: { + id: non_existing_record_id, + namespace_id: 1 + }) + + expect { described_class.new.handle_event(event) }.not_to raise_error + end + + shared_examples "updates the work_item hierarchy" do |event_data:| + specify do + work_item = instance_double(::WorkItem) + + expect(::WorkItem) + .to receive(:find_by_id) + .and_return(work_item) + + expect_next_instance_of(service_class, work_item) do |service| + expect(service).to receive(:execute) + end + + event = ::WorkItems::WorkItemCreatedEvent.new(data: event_data) + + described_class.new.handle_event(event) + end + end + + it_behaves_like "updates the work_item hierarchy", event_data: { + id: 1, + namespace_id: 3 + } + + it_behaves_like "updates the work_item hierarchy", event_data: { + id: 1, + work_item_parent_id: 2, + namespace_id: 3 + } + end +end