Merge branch 'fix-milestone-destroy-in-mr' into 'master'

Remove milestones from merge requests when milestones are deleted

Destroying a milestone does not remove it from existing merge requests. This MR fixes that.

Requires f4816372 to pull in as well.

Reported by https://github.com/gitlabhq/gitlabhq/issues/9643

See merge request !1348

3 files changed, 12 insertions, 0 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 31b2a759d40..ceebdc12d0f 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.0.0 (unreleased)
   - Fix Markdown links not showing up in dashboard activity feed (Stan Hu)
+  - Remove milestones from merge requests when milestones are deleted (Stan Hu)
   - Fix HTML link that was improperly escaped in new user e-mail (Stan Hu)
   - Fix broken sort in merge request API (Stan Hu)
   - Bump rouge to 1.10.1 to remove warning noise and fix other syntax highlighting bugs (Stan Hu)
diff --git a/app/services/milestones/destroy_service.rb b/app/services/milestones/destroy_service.rb
index 7ce7d259d0b..2414966505b 100644
--- a/app/services/milestones/destroy_service.rb
+++ b/app/services/milestones/destroy_service.rb
@@ -4,10 +4,15 @@ module Milestones
 
       Milestone.transaction do
         update_params = { milestone: nil }
+
         milestone.issues.each do |issue|
           Issues::UpdateService.new(project, 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)
+        end
+
         event_service.destroy_milestone(milestone, current_user)
 
         Event.for_milestone_id(milestone.id).each do |event|
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
index 35446640929..8127efabe6e 100644
--- a/spec/controllers/projects/milestones_controller_spec.rb
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -5,6 +5,7 @@ describe Projects::MilestonesController do
   let(:user)    { create(:user) }
   let(:milestone) { create(:milestone, project: project) }
   let(:issue) { create(:issue, project: project, milestone: milestone) }
+  let(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) }
 
   before do
     sign_in(user)
@@ -14,6 +15,7 @@ describe Projects::MilestonesController do
 
   describe "#destroy" do
     it "should remove milestone" do
+      merge_request.reload
       expect(issue.milestone_id).to eq(milestone.id)
 
       delete :destroy, namespace_id: project.namespace.id, project_id: project.id, id: milestone.id, format: :js
@@ -24,6 +26,10 @@ describe Projects::MilestonesController do
       expect { Milestone.find(milestone.id) }.to raise_exception(ActiveRecord::RecordNotFound)
       issue.reload
       expect(issue.milestone_id).to eq(nil)
+
+      merge_request.reload
+      expect(merge_request.milestone_id).to eq(nil)
+
       # Check system note left for milestone removal
       last_note = project.issues.find(issue.id).notes[-1].note
       expect(last_note).to eq('Milestone removed')