Merge branch 'fix_milestone_destroy_service' into 'master'

Fix milestones destroy service

See merge request !12762

2 files changed, 55 insertions, 2 deletions

diff --git a/app/services/milestones/destroy_service.rb b/app/services/milestones/destroy_service.rb
index e457212508f..600ebcfbecb 100644
--- a/app/services/milestones/destroy_service.rb
+++ b/app/services/milestones/destroy_service.rb
@@ -1,15 +1,17 @@
 module Milestones
   class DestroyService < Milestones::BaseService
     def execute(milestone)
+      return unless milestone.is_project_milestone?
+
       Milestone.transaction do
         update_params = { milestone: nil }
 
         milestone.issues.each do |issue|
-          Issues::UpdateService.new(project, current_user, update_params).execute(issue)
+          Issues::UpdateService.new(parent, current_user, update_params).execute(issue)
         end
 
         milestone.merge_requests.each do |merge_request|
-          MergeRequests::UpdateService.new(project, current_user, update_params).execute(merge_request)
+          MergeRequests::UpdateService.new(parent, current_user, update_params).execute(merge_request)
         end
 
         event_service.destroy_milestone(milestone, current_user)
diff --git a/spec/services/milestones/destroy_service_spec.rb b/spec/services/milestones/destroy_service_spec.rb
new file mode 100644
index 00000000000..8d1fe3ae2c1
--- /dev/null
+++ b/spec/services/milestones/destroy_service_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe Milestones::DestroyService, services: true do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:milestone) { create(:milestone, title: 'Milestone v1.0', project: project) }
+  let(:issue) { create(:issue, project: project, milestone: milestone) }
+  let(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) }
+
+  before do
+    project.team << [user, :master]
+  end
+
+  def service
+    described_class.new(project, user, {})
+  end
+
+  describe '#execute' do
+    it 'deletes milestone' do
+      service.execute(milestone)
+
+      expect { milestone.reload }.to raise_error ActiveRecord::RecordNotFound
+    end
+
+    it 'deletes milestone id from issuables' do
+      service.execute(milestone)
+
+      expect(issue.reload.milestone).to be_nil
+      expect(merge_request.reload.milestone).to be_nil
+    end
+
+    context 'group milestones' do
+      let(:group) { create(:group) }
+      let(:group_milestone) { create(:milestone, group: group) }
+
+      before do
+        project.update(namespace: group)
+        group.add_developer(user)
+      end
+
+      it { expect(service.execute(group_milestone)).to be_nil }
+
+      it 'does not update milestone issuables' do
+        expect(MergeRequests::UpdateService).not_to receive(:new)
+        expect(Issues::UpdateService).not_to receive(:new)
+
+        service.execute(group_milestone)
+      end
+    end
+  end
+end