Merge pull request #7976 from opf/fix/ancestory_attribute_updates

also update attributes when parent_id is signaled to be updated

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)

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,
+                          parent: parent,
-                           status: statuses[i] == :open ? open_status : closed_status,
+                          status: statuses[i] == :open ? open_status : closed_status,
-                           estimated_hours: estimated_hours[i],
+                          estimated_hours: estimated_hours[i],
-                           done_ratio: done_ratios[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,
+                        parent: parent,
-                         status: sibling_status,
+                        status: sibling_status,
-                         estimated_hours: sibling_estimated_hours,
+                        estimated_hours: sibling_estimated_hours,
-                         done_ratio: sibling_done_ratio
+                        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,
+                        status: status,
-                         estimated_hours: estimated_hours,
+                        estimated_hours: estimated_hours,
-                         done_ratio: done_ratio
+                        done_ratio: done_ratio
     end
 
-    subject do
+    shared_examples_for 'updates the attributes within the new hierarchy' do
-      work_package.parent = parent
+      before do
-      work_package.save!
+        subject
-      work_package.parent_id_was
+      end
 
-      described_class
+      it 'is successful' do
-        .new(user: user,
+        expect(subject)
-             work_package: work_package)
+          .to be_success
-        .call(%i(parent))
+      end
-    end
 
-    before do
+      it 'returns the new ancestors in the dependent results' do
-      subject
+        expect(subject.dependent_results.map(&:result))
-    end
+          .to match_array [parent, grandparent]
+      end
 
-    it 'is successful' do
+      it 'updates the done_ratio of the new parent' do
-      expect(subject)
+        expect(parent.reload(select: :done_ratio).done_ratio)
-        .to be_success
+          .to eql done_ratio
-    end
+      end
 
-    it 'returns the new ancestors in the dependent results' do
+      it 'updates the estimated_hours of the new parent' do
-      expect(subject.dependent_results.map(&:result))
+        expect(parent.reload(select: :derived_estimated_hours).derived_estimated_hours)
-        .to match_array [parent, grandparent]
+          .to eql estimated_hours
-    end
+      end
 
-    it 'updates the done_ratio of the new parent' do
+      it 'updates the done_ratio of the new grandparent' do
-      expect(parent.reload(select: :done_ratio).done_ratio)
+        expect(grandparent.reload(select: :done_ratio).done_ratio)
-        .to eql done_ratio
+          .to eql done_ratio
-    end
+      end
 
-    it 'updates the estimated_hours of the new parent' do
+      it 'updates the estimated_hours of the new grandparent' do
-      expect(parent.reload(select: :derived_estimated_hours).derived_estimated_hours)
+        expect(grandparent.reload(select: :derived_estimated_hours).derived_estimated_hours)
-        .to eql estimated_hours
+          .to eql estimated_hours
+      end
     end
 
-    it 'updates the done_ratio of the new grandparent' do
+    context 'if setting the parent' do
-      expect(grandparent.reload(select: :done_ratio).done_ratio)
+      subject do
-        .to eql done_ratio
+        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
+    context 'if setting the parent_id' do
-      expect(grandparent.reload(select: :derived_estimated_hours).derived_estimated_hours)
+      subject do
-        .to eql estimated_hours
+        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