Merge branch 'add-support-remove-milestone' into 'master'

Add support for destroying project milestones

### What does this MR do?

This MR adds a "Remove" button to the project milestones page and the milestone page itself.

### Why was this MR needed?

Because lots of people talked about needing to clean their toilets. :)

### What are the relevant issue numbers?

Closes https://github.com/gitlabhq/gitlabhq/issues/1504

### Screenshots

![image](https://gitlab.com/gitlab-org/gitlab-ce/uploads/ef8c7a3ea1db7b37cccae3869ac4de0a/image.png)

![image](https://gitlab.com/gitlab-org/gitlab-ce/uploads/33eb01c7bc30fb235de96db5efb8746d/image.png)

See merge request !980

8 files changed, 55 insertions, 2 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 1ae1f57b7a1..001d5350bad 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 7.14.0 (unreleased)
   - Expire Rails cache entries after two weeks to prevent endless Redis growth
+  - Add support for destroying project milestones (Stan Hu)
 
 v 7.13.0 (unreleased)
   - Only enable HSTS header for HTTPS and port 443 (Stan Hu)
diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb
index 61689488d13..9efe9704d1e 100644
--- a/app/controllers/projects/milestones_controller.rb
+++ b/app/controllers/projects/milestones_controller.rb
@@ -64,7 +64,12 @@ class Projects::MilestonesController < Projects::ApplicationController
   end
 
   def destroy
-    return access_denied! unless can?(current_user, :admin_milestone, @milestone)
+    return access_denied! unless can?(current_user, :admin_milestone, @project)
+
+    update_params = { milestone: nil }
+    @milestone.issues.each do |issue|
+      Issues::UpdateService.new(@project, current_user, update_params).execute(issue)
+    end
 
     @milestone.destroy
 
diff --git a/app/views/projects/milestones/_milestone.html.haml b/app/views/projects/milestones/_milestone.html.haml
index 14a0580f966..2ce5358fa74 100644
--- a/app/views/projects/milestones/_milestone.html.haml
+++ b/app/views/projects/milestones/_milestone.html.haml
@@ -5,6 +5,10 @@
         %i.fa.fa-pencil-square-o
         Edit
       = link_to 'Close Milestone', namespace_project_milestone_path(@project.namespace, @project, milestone, milestone: {state_event: :close }), method: :put, remote: true, class: "btn btn-sm btn-close"
+      = link_to namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-sm btn-remove" do
+        %i.fa.fa-trash-o
+        Remove
+
   %h4
     = link_to_gfm truncate(milestone.title, length: 100), namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone)
     - if milestone.expired? and not milestone.closed?
diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml
index 5947498e379..7b1681df336 100644
--- a/app/views/projects/milestones/show.html.haml
+++ b/app/views/projects/milestones/show.html.haml
@@ -19,6 +19,9 @@
         = link_to 'Close Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :close }), method: :put, class: "btn btn-close btn-grouped"
       - else
         = link_to 'Reopen Milestone', namespace_project_milestone_path(@project.namespace, @project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-grouped"
+      = link_to namespace_project_milestone_path(@project.namespace, @project, @milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-grouped btn-remove" do
+        %i.fa.fa-trash-o
+        Remove
 
 %hr
 - if @milestone.issues.any? && @milestone.can_be_closed?
diff --git a/config/routes.rb b/config/routes.rb
index 055d59a0c93..2e16c3ecb39 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -481,7 +481,7 @@ Gitlab::Application.routes.draw do
           end
         end
 
-        resources :milestones, except: [:destroy], constraints: { id: /\d+/ } do
+        resources :milestones, constraints: { id: /\d+/ } do
           member do
             put :sort_issues
             put :sort_merge_requests
diff --git a/features/project/issues/milestones.feature b/features/project/issues/milestones.feature
index 9ac65b1257c..bfbaaec5a35 100644
--- a/features/project/issues/milestones.feature
+++ b/features/project/issues/milestones.feature
@@ -17,6 +17,10 @@ Feature: Project Issues Milestones
     And I submit new milestone "v2.3"
     Then I should see milestone "v2.3"
 
+  Scenario: I delete new milestone
+    Given I click link to remove milestone "v2.2"
+    And I should see no milestones
+
   @javascript
   Scenario: Listing closed issues
     Given the milestone has open and closed issues
diff --git a/features/steps/project/issues/milestones.rb b/features/steps/project/issues/milestones.rb
index 708c5243947..61e62c2adbd 100644
--- a/features/steps/project/issues/milestones.rb
+++ b/features/steps/project/issues/milestones.rb
@@ -56,4 +56,12 @@ class Spinach::Features::ProjectIssuesMilestones < Spinach::FeatureSteps
   step 'I should see 3 issues' do
     expect(page).to have_selector('#tab-issues li.issue-row', count: 4)
   end
+
+  step 'I click link to remove milestone "v2.2"' do
+    click_link 'Remove'
+  end
+
+  step 'I should see no milestones' do
+    expect(page).to have_content('No milestones to show')
+  end
 end
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
new file mode 100644
index 00000000000..d3868c13202
--- /dev/null
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+describe Projects::MilestonesController do
+  let(:project) { create(:project) }
+  let(:user)    { create(:user) }
+  let(:milestone) { create(:milestone, project: project) }
+  let(:issue) { create(:issue, project: project, milestone: milestone) }
+
+  before do
+    sign_in(user)
+    project.team << [user, :master]
+    controller.instance_variable_set(:@project, project)
+  end
+
+  describe "#destroy" do
+    it "should remove milestone" do
+      expect(issue.milestone_id).to eq(milestone.id)
+      delete :destroy, namespace_id: project.namespace.id, project_id: project.id, id: milestone.id, format: :js
+      expect(response).to be_success
+      expect { Milestone.find(milestone.id) }.to raise_exception(ActiveRecord::RecordNotFound)
+      issue.reload
+      expect(issue.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')
+    end
+  end
+end