Merge pull request opf#18888 from opf/bugfix/63831-error-500-when-bulk-delete-related-work-packages

[63831] Fix error 500 when bulk deleting related work packages

Author: cbliard

app/services/work_packages/delete_service.rb

@@ -50,7 +50,7 @@ def persist(service_result)
 
   def destroy_descendants(descendants, result)
     descendants.each do |descendant|
-      success = destroy(descendant)
+      success = destroy(descendant.reload)
       result.add_dependent!(ServiceResult.new(success:, result: descendant))
     end
   end
@@ -63,6 +63,7 @@ def destroy(work_package)
 
   def find_successors_of_self_and_descendants(work_package)
     WorkPackage.where(id: Relation.follows.of_predecessor(work_package.self_and_descendants).select(:from_id))
+               .where.not(id: work_package.self_and_descendants)
   end
 
   def update_ancestors_and_successors(successors, result)
@@ -94,8 +95,8 @@ def reschedule_related(deleted_work_package, successors)
       .new(user:, work_package: work_packages_to_reschedule)
       .call
 
-    result.dependent_results.map(&:result).each do |dependent_result|
-      dependent_result.save(validate: false)
+    result.dependent_results.map(&:result).each do |rescheduled_work_package|
+      rescheduled_work_package.save(validate: false)
     end
 
     result

spec/controllers/work_packages/bulk_controller_spec.rb

@@ -81,7 +81,7 @@
            principal: user,
            roles: [role])
   end
-  shared_let(:work_package1) do
+  shared_let(:work_package1, refind: true) do
     create(:work_package,
            author: user,
            assigned_to: user,
@@ -91,7 +91,7 @@
            custom_field_values: { custom_field1.id => custom_field_value },
            project: project1)
   end
-  shared_let(:work_package2) do
+  shared_let(:work_package2, refind: true) do
     create(:work_package,
            author: user,
            assigned_to: user,
@@ -101,7 +101,7 @@
            custom_field_values: { custom_field1.id => custom_field_value },
            project: project1)
   end
-  shared_let(:work_package3) do
+  shared_let(:work_package3, refind: true) do
     create(:work_package,
            author: user,
            type:,
@@ -643,5 +643,23 @@ def send_destroy_request
         expect(response).to redirect_to(project_work_packages_path(work_package1.project))
       end
     end
+
+    context "with children work packages following each other" do
+      before_all do
+        work_package1.update(subject: "parent", schedule_manually: false)
+        work_package2.update(subject: "predecessor child", parent: work_package1, schedule_manually: true)
+        work_package3.update(subject: "successor child", parent: work_package1, schedule_manually: false)
+        create(:follows_relation, predecessor: work_package2, successor: work_package3)
+      end
+
+      let(:params) { { "ids" => [work_package1.id, work_package2.id, work_package3.id] } }
+
+      it "deletes them all without errors" do
+        expect { send_destroy_request }.not_to raise_error
+
+        expect(WorkPackage.count).to eq(0)
+        expect(response).to redirect_to(project_work_packages_path(project1))
+      end
+    end
   end
 end

spec/services/work_packages/delete_service_spec.rb

@@ -133,10 +133,14 @@
 
   context "with descendants" do
     let(:child) do
-      build_stubbed(:work_package)
+      build_stubbed(:work_package).tap do |wp|
+        allow(wp).to receive(:reload).and_return(wp)
+      end
     end
     let(:grandchild) do
-      build_stubbed(:work_package)
+      build_stubbed(:work_package).tap do |wp|
+        allow(wp).to receive(:reload).and_return(wp)
+      end
     end
     let(:descendants) do
       [child, grandchild]