diff --git a/ee/app/services/concerns/epics/sync_as_work_item.rb b/ee/app/services/concerns/epics/sync_as_work_item.rb index 9cbad2943f027fc3adedbed9bfae78262088b279..2cb27fad6e520c0f98ca4b325572e0919e760511 100644 --- a/ee/app/services/concerns/epics/sync_as_work_item.rb +++ b/ee/app/services/concerns/epics/sync_as_work_item.rb @@ -3,6 +3,7 @@ module Epics module SyncAsWorkItem extend ActiveSupport::Concern + include ::Gitlab::Utils::StrongMemoize SyncAsWorkItemError = Class.new(StandardError) @@ -23,7 +24,7 @@ def create_work_item_for(epic) end sync_color(epic, work_item) - sync_dates(epic, work_item) + sync_dates_on_epic_creation(epic, work_item) work_item.save!(touch: false) work_item @@ -34,8 +35,8 @@ def create_work_item_for(epic) def update_work_item_for!(epic) return true unless epic.work_item - sync_color(epic, epic.work_item) - sync_dates(epic, epic.work_item) + sync_color(epic, epic.work_item) if changed_attributes(epic).include?(:color) + sync_dates_on_epic_update(epic, epic.work_item) epic.work_item.assign_attributes(update_params(epic)) epic.work_item.save!(touch: false) @@ -47,11 +48,11 @@ def update_work_item_for!(epic) def create_params(epic) ALLOWED_PARAMS.index_with { |attr| epic[attr] } - .merge(work_item_type: WorkItems::Type.default_by_type(:epic), namespace_id: group.id) + .merge(work_item_type: WorkItems::Type.default_by_type(:epic), namespace_id: group.id) end def update_params(epic) - filtered_attributes = epic.previous_changes.keys.map(&:to_sym) + filtered_attributes = changed_attributes(epic) .intersection(ALLOWED_PARAMS + %i[title_html description_html]) .index_with { |attr| epic[attr] } @@ -70,7 +71,7 @@ def sync_color(epic, work_item) end end - def sync_dates(epic, work_item) + def sync_dates_on_epic_creation(epic, work_item) dates_source = work_item.dates_source || work_item.build_dates_source dates_source.start_date = epic.start_date @@ -88,6 +89,32 @@ def sync_dates(epic, work_item) work_item.dates_source = dates_source end + def sync_dates_on_epic_update(epic, work_item) + changed_attributes = changed_date_attributes(epic) + return if changed_attributes.blank? + + dates_source = work_item.dates_source || work_item.build_dates_source + + if changed_attributes.include?(:start_date_sourcing_epic_id) + dates_source.start_date_sourcing_work_item_id = epic.start_date_sourcing_epic&.issue_id + changed_attributes.delete(:start_date_sourcing_epic_id) + end + + if changed_attributes.include?(:due_date_sourcing_epic_id) + dates_source.due_date_sourcing_work_item_id = epic.due_date_sourcing_epic&.issue_id + changed_attributes.delete(:due_date_sourcing_epic_id) + end + + if changed_attributes.include?(:end_date) + dates_source.due_date = epic.end_date + changed_attributes.delete(:end_date) + end + + dates_source.assign_attributes(changed_attributes.index_with { |attr| epic[attr] }) + + work_item.dates_source = dates_source + end + def raise_error!(action, error, epic = nil) log_error(action, error.message, epic) @@ -107,5 +134,20 @@ def log_error(action, error_message, epic) group_id: group.id, epic_id: epic&.id) end + + def changed_date_attributes(epic) + changed_attributes(epic).intersection( + %i[ + start_date start_date_fixed start_date_is_fixed start_date_sourcing_milestone_id start_date_sourcing_epic_id + end_date due_date_fixed due_date_is_fixed due_date_sourcing_milestone_id due_date_sourcing_epic_id + ] + ) + end + + def changed_attributes(epic) + strong_memoize_with(:changed_attributes, epic) do + epic.previous_changes.keys.map(&:to_sym) + end + end end end diff --git a/ee/spec/services/epics/update_service_spec.rb b/ee/spec/services/epics/update_service_spec.rb index 19e6fe73ed5c64d438b19b32fad474f7a347108a..8d6edfd8527d41cf1e4570065f57685c6e1511e2 100644 --- a/ee/spec/services/epics/update_service_spec.rb +++ b/ee/spec/services/epics/update_service_spec.rb @@ -844,6 +844,17 @@ def update_issuable(update_params) create(:parent_link, work_item_parent: epic.work_item, work_item: WorkItem.find(child_issue.id)) end + context 'when updating only due date' do + let(:opts) { { due_date: 2.days.from_now.to_date } } + + it 'syncs due date' do + subject + + expect(work_item.dates_source.due_date).to eq(epic.end_date) + expect(work_item.dates_source.due_date).to eq(opts[:due_date].to_date) + end + end + it 'sets rolledup dated for the work item', :aggregate_failures do subject @@ -992,9 +1003,11 @@ def update_issuable(update_params) end end - context 'when params are different than the epic attributes' do + context 'when params are different than the epic attributes', :aggregate_failures do before do - epic.update!(title: "Outdated title") + epic.work_item.create_dates_source!(start_date: 3.days.ago, due_date: 3.days.from_now) + epic.work_item.create_color!(color: "#123123") + epic.update!(title: "Outdated title", color: "#aabbcc", due_date_fixed: 3.days.ago) end let(:opts) do @@ -1011,6 +1024,8 @@ def update_issuable(update_params) expect(work_item.updated_by).to eq(epic.updated_by) expect(work_item.updated_at).to eq(epic.updated_at) expect(work_item.title).not_to eq('Outdated title') + expect(work_item.color.color).not_to eq(epic.color) + expect(work_item.dates_source.due_date_fixed).not_to eq(epic.due_date_fixed) end end end