diff --git a/app/events/work_items/work_item_updated_event.rb b/app/events/work_items/work_item_updated_event.rb index 42ad7c92ae923d0a7450dc1402fd86ae389eb7b1..8cd6b0d4c593f47230dcf7bbdd4338c81adcfbe4 100644 --- a/app/events/work_items/work_item_updated_event.rb +++ b/app/events/work_items/work_item_updated_event.rb @@ -9,6 +9,7 @@ def schema 'properties' => { 'id' => { 'type' => 'integer' }, 'namespace_id' => { 'type' => 'integer' }, + 'previous_work_item_parent_id' => { 'type' => 'integer' }, 'updated_attributes' => { 'type' => 'array', 'items' => { diff --git a/app/services/work_items/update_service.rb b/app/services/work_items/update_service.rb index 046e6ebea54ab1054b90f5e5730844d62928abac..b3ad359783968d88a74366f6b01d647f3383611f 100644 --- a/app/services/work_items/update_service.rb +++ b/app/services/work_items/update_service.rb @@ -18,7 +18,6 @@ def execute(work_item) updated_work_item = super if updated_work_item.valid? - publish_event(work_item) success(payload(work_item)) else error(updated_work_item.errors.full_messages, :unprocessable_entity, pass_back: payload(updated_work_item)) @@ -57,6 +56,13 @@ def before_update(work_item, skip_spam_check: false) super end + override :associations_before_update + def associations_before_update(work_item) + super.merge( + work_item_parent_id: work_item.work_item_parent&.id + ) + end + def transaction_update(work_item, opts = {}) execute_widgets(work_item: work_item, callback: :before_update_in_transaction, widget_params: @widget_params) @@ -68,6 +74,7 @@ def after_update(work_item, old_associations) super GraphqlTriggers.issuable_title_updated(work_item) if work_item.previous_changes.key?(:title) + publish_event(work_item, old_associations) end def payload(work_item) @@ -82,10 +89,11 @@ def handle_label_changes(issuable, old_labels) ) end - def publish_event(work_item) + def publish_event(work_item, old_associations) event = WorkItems::WorkItemUpdatedEvent.new(data: { id: work_item.id, namespace_id: work_item.namespace_id, + previous_work_item_parent_id: old_associations[:work_item_parent_id], updated_attributes: work_item.previous_changes&.keys&.map(&:to_s), updated_widgets: @widget_params&.keys&.map(&:to_s) }.tap(&:compact_blank!)) diff --git a/ee/app/services/work_items/widgets/rolledup_dates_service/hierarchy_update_service.rb b/ee/app/services/work_items/widgets/rolledup_dates_service/hierarchy_update_service.rb index 7c17aafad71827e42c4672f404db70932fd64c78..ea0e63cc14b73798828f136c6620484cff31ec3d 100644 --- a/ee/app/services/work_items/widgets/rolledup_dates_service/hierarchy_update_service.rb +++ b/ee/app/services/work_items/widgets/rolledup_dates_service/hierarchy_update_service.rb @@ -4,8 +4,9 @@ module WorkItems module Widgets module RolledupDatesService class HierarchyUpdateService - def initialize(work_item) + def initialize(work_item, previous_work_item_parent_id = nil) @work_item = work_item + @previous_work_item_parent_id = previous_work_item_parent_id end def execute @@ -14,18 +15,7 @@ def execute work_item.build_dates_source if work_item.dates_source.blank? - attributes = {} - - unless work_item.dates_source.due_date_is_fixed? - maximum_due_date_attributes = finder.maximum_due_date.first&.attributes - attributes.merge!(maximum_due_date_attributes) if maximum_due_date_attributes.present? - end - - unless work_item.dates_source.start_date_is_fixed? - minimum_start_date_attributes = finder.minimum_start_date.first&.attributes - attributes.merge!(minimum_start_date_attributes) if minimum_start_date_attributes.present? - end - + attributes = attributes_for(:due_date).merge(attributes_for(:start_date)) work_item.dates_source.update!(attributes.except('issue_id')) if attributes.present? update_parent @@ -35,15 +25,25 @@ def execute attr_reader :work_item + def attributes_for(field) + return {} if work_item.dates_source.read_attribute(:"#{field}_is_fixed") + + finder.attributes_for(field).presence || { + field => nil, + "#{field}_sourcing_milestone_id": nil, + "#{field}_sourcing_work_item_id": nil + } + end + def finder @finder ||= WorkItems::Widgets::RolledupDatesFinder.new(work_item) end def update_parent - parent = work_item.work_item_parent - return if parent.blank? + parent_id = @previous_work_item_parent_id || work_item.work_item_parent&.id + return if parent_id.blank? - ::WorkItems::RolledupDates::UpdateRolledupDatesWorker.perform_async(parent.id) + ::WorkItems::RolledupDates::UpdateRolledupDatesWorker.perform_async(parent_id) end 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 905e9802cca56647d98ba26dc16f76f97b915f50..9f3040e75066b8fcde56fb130ebe9f3faa85fd05 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 @@ -26,7 +26,17 @@ end end - context "when the work_item has an epic parent" do + context "when previous_work_item_parent_id is given" do + specify do + expect(::WorkItems::RolledupDates::UpdateRolledupDatesWorker) + .to receive(:perform_async) + .with(1) + + described_class.new(work_item, 1).execute + end + end + + context "when work_item has a parent" do let_it_be(:parent) do create(:work_item, :epic, namespace: group).tap do |parent| create(:parent_link, work_item: work_item, work_item_parent: parent) @@ -170,5 +180,28 @@ it_behaves_like "when work item already have an associated dates_source" it_behaves_like "when work item does not have an associated dates_source" end + + context "when all children are removed from the hierarchy", :sidekiq_inline, :clean_gitlab_redis_shared_state do + let_it_be(:child_work_item) do + create(:work_item, :issue, namespace: group, start_date: start_date, due_date: due_date) + .tap do |child| + # Simulate that the dates were rolled up at some point + create( + :work_items_dates_source, + work_item: work_item, + start_date: start_date, + start_date_sourcing_work_item_id: child.id, + due_date: due_date, + due_date_sourcing_work_item_id: child.id + ) + end + end + + it "nullify the start/due date" do + expect { described_class.new(child_work_item, work_item.id).execute } + .to change { work_item.reload.dates_source&.due_date }.from(due_date).to(nil) + .and change { work_item.reload.dates_source&.start_date }.from(start_date).to(nil) + end + end end end diff --git a/spec/services/work_items/update_service_spec.rb b/spec/services/work_items/update_service_spec.rb index 739b3668fcc881d8881e5b8961da37da6353fca7..a399804549ca7a4a863a16dbdd8bcde2162cd768 100644 --- a/spec/services/work_items/update_service_spec.rb +++ b/spec/services/work_items/update_service_spec.rb @@ -39,7 +39,7 @@ end shared_examples 'publish WorkItems::WorkItemUpdatedEvent event' do |attributes: nil, widgets: nil| - it do + specify do expect { expect(update_work_item[:status]).to eq(:success) } .to publish_event(WorkItems::WorkItemUpdatedEvent) .with({ @@ -51,18 +51,18 @@ end end - shared_examples 'do not publish WorkItems::WorkItemUpdatedEvent event' do - it do + shared_examples 'does not publish WorkItems::WorkItemUpdatedEvent event' do + specify do expect { update_work_item }.not_to publish_event(WorkItems::WorkItemUpdatedEvent) end end - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event' + include_examples 'does not publish WorkItems::WorkItemUpdatedEvent event' context 'when applying quick actions' do let(:opts) { { description: "/shrug" } } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ description description_html @@ -100,7 +100,7 @@ context 'when title is changed' do let(:opts) { { title: 'changed' } } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ lock_version title @@ -132,7 +132,7 @@ context 'when title is not changed' do let(:opts) { { description: 'changed' } } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ description description_html @@ -159,7 +159,7 @@ context 'when dates are changed' do let(:opts) { { start_date: Date.today } } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ start_date updated_at @@ -180,7 +180,7 @@ context 'when decription is changed' do let(:opts) { { description: 'description changed' } } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ description description_html @@ -216,13 +216,7 @@ context 'when state_event is close' do let(:opts) { { state_event: 'close' } } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', - attributes: %w[ - closed_at - closed_by_id - state_id - updated_at - ] + include_examples 'does not publish WorkItems::WorkItemUpdatedEvent event' it 'closes the work item' do expect do @@ -239,12 +233,7 @@ work_item.close! end - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', - attributes: %w[ - closed_at - state_id - updated_at - ] + include_examples 'does not publish WorkItems::WorkItemUpdatedEvent event' it 'reopens the work item' do expect do @@ -311,7 +300,7 @@ end context 'for the description widget' do - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ description description_html @@ -379,7 +368,7 @@ context 'for start and due date widget' do let(:updated_date) { 1.week.from_now.to_date } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ description description_html @@ -422,7 +411,7 @@ let(:widget_params) { { hierarchy_widget: { children: [child_work_item] } } } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ title title_html @@ -467,7 +456,7 @@ context 'when work item validation fails' do let(:opts) { { title: '' } } - it_behaves_like 'do not publish WorkItems::WorkItemUpdatedEvent event' + include_examples 'does not publish WorkItems::WorkItemUpdatedEvent event' it 'returns validation errors' do expect(update_work_item[:message]).to contain_exactly("Title can't be blank") @@ -488,7 +477,7 @@ let(:widget_params) { { milestone_widget: { milestone_id: milestone.id } } } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ milestone_id updated_at @@ -529,7 +518,7 @@ let_it_be(:user_todo) { create(:todo, target: work_item, user: developer, project: project, state: :pending) } let_it_be(:other_todo) { create(:todo, target: work_item, user: create(:user), project: project, state: :pending) } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ description description_html @@ -546,7 +535,7 @@ context 'when action is mark_as_done' do let(:widget_params) { { current_user_todos_widget: { action: 'mark_as_done' } } } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ updated_at updated_by_id @@ -589,7 +578,7 @@ let(:label) { create(:label, project: project) } let(:opts) { { label_ids: [label1.id] } } - it_behaves_like 'publish WorkItems::WorkItemUpdatedEvent event', + include_examples 'publish WorkItems::WorkItemUpdatedEvent event', attributes: %w[ updated_at updated_by_id