From fa7e469051d6de28b3c91d2b794c0c3f0b6bfa98 Mon Sep 17 00:00:00 2001
From: ulferts <jens.ulferts@googlemail.com>
Date: Thu, 23 Jan 2020 13:02:53 +0100
Subject: [PATCH] also update attributes when parent_id is signaled to be
 updated

---
 .../work_packages/update_ancestors_service.rb |   2 +-
 .../update_ancestors_service_spec.rb          | 111 +++++++++++-------
 2 files changed, 67 insertions(+), 46 deletions(-)

diff --git a/app/services/work_packages/update_ancestors_service.rb b/app/services/work_packages/update_ancestors_service.rb
index eab66517a7..3f9cec1182 100644
--- a/app/services/work_packages/update_ancestors_service.rb
+++ b/app/services/work_packages/update_ancestors_service.rb
@@ -100,7 +100,7 @@ class WorkPackages::UpdateAncestorsService
   end
 
   def inherit?(attributes, attribute)
-    [attribute, :parent].any? { |attr| attributes.include? attr }
+    [attribute, :parent, :parent_id].any? { |attr| attributes.include? attr }
   end
 
   def set_journal_note(work_packages)
diff --git a/spec/services/work_packages/update_ancestors_service_spec.rb b/spec/services/work_packages/update_ancestors_service_spec.rb
index 5e5c05fa4c..7bf5f691e4 100644
--- a/spec/services/work_packages/update_ancestors_service_spec.rb
+++ b/spec/services/work_packages/update_ancestors_service_spec.rb
@@ -67,10 +67,10 @@ describe WorkPackages::UpdateAncestorsService, type: :model do
     let(:children) do
       (statuses.size - 1).downto(0).map do |i|
         FactoryBot.create :work_package,
-                           parent: parent,
-                           status: statuses[i] == :open ? open_status : closed_status,
-                           estimated_hours: estimated_hours[i],
-                           done_ratio: done_ratios[i]
+                          parent: parent,
+                          status: statuses[i] == :open ? open_status : closed_status,
+                          estimated_hours: estimated_hours[i],
+                          done_ratio: done_ratios[i]
       end
     end
     let(:parent) { FactoryBot.create :work_package, status: open_status }
@@ -199,10 +199,10 @@ describe WorkPackages::UpdateAncestorsService, type: :model do
     end
     let!(:sibling) do
       FactoryBot.create :work_package,
-                         parent: parent,
-                         status: sibling_status,
-                         estimated_hours: sibling_estimated_hours,
-                         done_ratio: sibling_done_ratio
+                        parent: parent,
+                        status: sibling_status,
+                        estimated_hours: sibling_estimated_hours,
+                        done_ratio: sibling_done_ratio
     end
 
     let!(:work_package) do
@@ -265,58 +265,79 @@ describe WorkPackages::UpdateAncestorsService, type: :model do
     end
     let!(:parent) do
       FactoryBot.create :work_package,
-                         parent: grandparent
+                        parent: grandparent
     end
     let!(:work_package) do
       FactoryBot.create :work_package,
-                         status: status,
-                         estimated_hours: estimated_hours,
-                         done_ratio: done_ratio
+                        status: status,
+                        estimated_hours: estimated_hours,
+                        done_ratio: done_ratio
     end
 
-    subject do
-      work_package.parent = parent
-      work_package.save!
-      work_package.parent_id_was
+    shared_examples_for 'updates the attributes within the new hierarchy' do
+      before do
+        subject
+      end
 
-      described_class
-        .new(user: user,
-             work_package: work_package)
-        .call(%i(parent))
-    end
+      it 'is successful' do
+        expect(subject)
+          .to be_success
+      end
 
-    before do
-      subject
-    end
+      it 'returns the new ancestors in the dependent results' do
+        expect(subject.dependent_results.map(&:result))
+          .to match_array [parent, grandparent]
+      end
 
-    it 'is successful' do
-      expect(subject)
-        .to be_success
-    end
+      it 'updates the done_ratio of the new parent' do
+        expect(parent.reload(select: :done_ratio).done_ratio)
+          .to eql done_ratio
+      end
 
-    it 'returns the new ancestors in the dependent results' do
-      expect(subject.dependent_results.map(&:result))
-        .to match_array [parent, grandparent]
-    end
+      it 'updates the estimated_hours of the new parent' do
+        expect(parent.reload(select: :derived_estimated_hours).derived_estimated_hours)
+          .to eql estimated_hours
+      end
 
-    it 'updates the done_ratio of the new parent' do
-      expect(parent.reload(select: :done_ratio).done_ratio)
-        .to eql done_ratio
-    end
+      it 'updates the done_ratio of the new grandparent' do
+        expect(grandparent.reload(select: :done_ratio).done_ratio)
+          .to eql done_ratio
+      end
 
-    it 'updates the estimated_hours of the new parent' do
-      expect(parent.reload(select: :derived_estimated_hours).derived_estimated_hours)
-        .to eql estimated_hours
+      it 'updates the estimated_hours of the new grandparent' do
+        expect(grandparent.reload(select: :derived_estimated_hours).derived_estimated_hours)
+          .to eql estimated_hours
+      end
     end
 
-    it 'updates the done_ratio of the new grandparent' do
-      expect(grandparent.reload(select: :done_ratio).done_ratio)
-        .to eql done_ratio
+    context 'if setting the parent' do
+      subject do
+        work_package.parent = parent
+        work_package.save!
+        work_package.parent_id_was
+
+        described_class
+          .new(user: user,
+               work_package: work_package)
+          .call(%i(parent))
+      end
+
+      it_behaves_like 'updates the attributes within the new hierarchy'
     end
 
-    it 'updates the estimated_hours of the new grandparent' do
-      expect(grandparent.reload(select: :derived_estimated_hours).derived_estimated_hours)
-        .to eql estimated_hours
+    context 'if setting the parent_id' do
+      subject do
+        work_package.parent_id = parent.id
+        work_package.save!
+        work_package.parent_id_was
+
+        described_class
+          .new(user: user,
+               work_package: work_package)
+          .call(%i(parent_id))
+      end
+
+      it_behaves_like 'updates the attributes within the new hierarchy'
     end
   end
 end