Merge branch...

Merge branch 'kassio/work-items-rolledup-dates-on-removing-existing-work_item-to-hierarchy' into 'master' 

WorkItems: Rolledup dates on removing existing child from hierarchy

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148191



Merged-by: Eugenia Grieff <egrieff@gitlab.com>
Approved-by: Alexandru Croitor <acroitor@gitlab.com>
Approved-by: Eugenia Grieff <egrieff@gitlab.com>
Reviewed-by: Kassio Borges <kborges@gitlab.com>
Reviewed-by: Alexandru Croitor <acroitor@gitlab.com>
Co-authored-by: Kassio Borges <kassioborgesm@gmail.com>

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' => {

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!))

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

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

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