Allow to delete group milestonesissue_36138

10 files changed, 54 insertions, 31 deletions

diff --git a/app/assets/javascripts/pages/groups/milestones/show/index.js b/app/assets/javascripts/pages/groups/milestones/show/index.js
index 74cc4ba42c1..ebaea5ef3dc 100644
--- a/app/assets/javascripts/pages/groups/milestones/show/index.js
+++ b/app/assets/javascripts/pages/groups/milestones/show/index.js
@@ -1,8 +1,10 @@
 import initMilestonesShow from '~/pages/milestones/shared/init_milestones_show';
+import initDeleteMilestoneModal from '~/pages/milestones/shared/delete_milestone_modal_init';
+
 import Milestone from '~/milestone';
 
 document.addEventListener('DOMContentLoaded', () => {
   initMilestonesShow();
-
+  initDeleteMilestoneModal();
   Milestone.initDeprecationMessage();
 });
diff --git a/app/assets/javascripts/pages/milestones/shared/components/delete_milestone_modal.vue b/app/assets/javascripts/pages/milestones/shared/components/delete_milestone_modal.vue
index 4061c11ba8f..48668562f09 100644
--- a/app/assets/javascripts/pages/milestones/shared/components/delete_milestone_modal.vue
+++ b/app/assets/javascripts/pages/milestones/shared/components/delete_milestone_modal.vue
@@ -40,8 +40,8 @@
         if (this.issueCount === 0 && this.mergeRequestCount === 0) {
           return sprintf(
             s__(`Milestones|
-You’re about to permanently delete the milestone %{milestoneTitle} from this project.
-%{milestoneTitle} is not currently used in any issues or merge requests.`),
+You’re about to permanently delete the milestone %{milestoneTitle}.
+This milestone is not currently used in any issues or merge requests.`),
             {
               milestoneTitle,
             },
@@ -51,7 +51,7 @@ You’re about to permanently delete the milestone %{milestoneTitle} from this p
 
         return sprintf(
           s__(`Milestones|
-You’re about to permanently delete the milestone %{milestoneTitle} from this project and remove it from %{issuesWithCount} and %{mergeRequestsWithCount}.
+You’re about to permanently delete the milestone %{milestoneTitle} and remove it from %{issuesWithCount} and %{mergeRequestsWithCount}.
 Once deleted, it cannot be undone or recovered.`),
           {
             milestoneTitle,
diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb
index 9bd51de7e97..6bdc0f79ef2 100644
--- a/app/controllers/groups/milestones_controller.rb
+++ b/app/controllers/groups/milestones_controller.rb
@@ -2,8 +2,8 @@ class Groups::MilestonesController < Groups::ApplicationController
   include MilestoneActions
 
   before_action :group_projects
-  before_action :milestone, only: [:edit, :show, :update, :merge_requests, :participants, :labels]
-  before_action :authorize_admin_milestones!, only: [:edit, :new, :create, :update]
+  before_action :milestone, only: [:edit, :show, :update, :merge_requests, :participants, :labels, :destroy]
+  before_action :authorize_admin_milestones!, only: [:edit, :new, :create, :update, :destroy]
 
   def index
     respond_to do |format|
@@ -56,10 +56,21 @@ class Groups::MilestonesController < Groups::ApplicationController
     redirect_to milestone_path
   end
 
+  def destroy
+    return render_404 if @milestone.legacy_group_milestone?
+
+    Milestones::DestroyService.new(group, current_user).execute(@milestone)
+
+    respond_to do |format|
+      format.html { redirect_to group_milestones_path(group), status: :see_other }
+      format.js { head :ok }
+    end
+  end
+
   private
 
   def authorize_admin_milestones!
-    return render_404 unless can?(current_user, :admin_milestones, group)
+    return render_404 unless can?(current_user, :admin_milestone, group)
   end
 
   def milestone_params
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index a8d7a05f509..333643865e9 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -53,7 +53,7 @@ class GroupPolicy < BasePolicy
 
   rule { has_access }.enable :read_namespace
 
-  rule { developer }.enable :admin_milestones
+  rule { developer }.enable :admin_milestone
 
   rule { reporter }.policy do
     enable :admin_label
diff --git a/app/services/milestones/destroy_service.rb b/app/services/milestones/destroy_service.rb
index 15c04525075..7cda802c120 100644
--- a/app/services/milestones/destroy_service.rb
+++ b/app/services/milestones/destroy_service.rb
@@ -3,8 +3,6 @@
 module Milestones
   class DestroyService < Milestones::BaseService
     def execute(milestone)
-      return unless milestone.project_milestone?
-
       Milestone.transaction do
         update_params = { milestone: nil }
 
@@ -16,15 +14,21 @@ module Milestones
           MergeRequests::UpdateService.new(parent, current_user, update_params).execute(merge_request)
         end
 
-        event_service.destroy_milestone(milestone, current_user)
-
-        Event.for_milestone_id(milestone.id).each do |event|
-          event.target_id = nil
-          event.save
-        end
+        log_destroy_event_for(milestone)
 
         milestone.destroy
       end
     end
+
+    def log_destroy_event_for(milestone)
+      return if milestone.group_milestone?
+
+      event_service.destroy_milestone(milestone, current_user)
+
+      Event.for_milestone_id(milestone.id).each do |event|
+        event.target_id = nil
+        event.save
+      end
+    end
   end
 end
diff --git a/app/views/groups/milestones/index.html.haml b/app/views/groups/milestones/index.html.haml
index f5f621507b8..b6424df55cd 100644
--- a/app/views/groups/milestones/index.html.haml
+++ b/app/views/groups/milestones/index.html.haml
@@ -5,7 +5,7 @@
 
   .nav-controls
     = render 'shared/milestones_sort_dropdown'
-    - if can?(current_user, :admin_milestones, @group)
+    - if can?(current_user, :admin_milestone, @group)
       = link_to "New milestone", new_group_milestone_path(@group), class: "btn btn-new"
 
 .milestones
diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml
index 2a9e20c2caa..0a684f9016a 100644
--- a/app/views/projects/milestones/show.html.haml
+++ b/app/views/projects/milestones/show.html.haml
@@ -43,18 +43,7 @@
         - else
           = link_to 'Reopen milestone', project_milestone_path(@project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-nr btn-grouped"
 
-        %button.js-delete-milestone-button.btn.btn-grouped.btn-danger{ data: { toggle: 'modal',
-          target: '#delete-milestone-modal',
-          milestone_id: @milestone.id,
-          milestone_title: markdown_field(@milestone, :title),
-          milestone_url: project_milestone_path(@project, @milestone),
-          milestone_issue_count: @milestone.issues.count,
-          milestone_merge_request_count: @milestone.merge_requests.count },
-          disabled: true }
-          = _('Delete')
-          = icon('spin spinner', class: 'js-loading-icon hidden' )
-
-        #delete-milestone-modal
+        = render 'shared/milestones/delete_button'
 
       %a.btn.btn-default.btn-grouped.float-right.d-block.d-sm-none.js-sidebar-toggle{ href: "#" }
         = icon('angle-double-left')
diff --git a/app/views/shared/milestones/_delete_button.html.haml b/app/views/shared/milestones/_delete_button.html.haml
new file mode 100644
index 00000000000..e236c24b088
--- /dev/null
+++ b/app/views/shared/milestones/_delete_button.html.haml
@@ -0,0 +1,14 @@
+- milestone_url = @milestone.project_milestone? ? project_milestone_path(@project, @milestone) : group_milestone_path(@group, @milestone)
+
+%button.js-delete-milestone-button.btn.btn-grouped.btn-danger{ data: { toggle: 'modal',
+  target: '#delete-milestone-modal',
+  milestone_id: @milestone.id,
+  milestone_title: markdown_field(@milestone, :title),
+  milestone_url: milestone_url,
+  milestone_issue_count: @milestone.issues.count,
+  milestone_merge_request_count: @milestone.merge_requests.count },
+  disabled: true }
+  = _('Delete')
+  = icon('spin spinner', class: 'js-loading-icon hidden' )
+
+#delete-milestone-modal
diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml
index c559945a9c9..b33bf5df707 100644
--- a/app/views/shared/milestones/_milestone.html.haml
+++ b/app/views/shared/milestones/_milestone.html.haml
@@ -49,7 +49,7 @@
           - unless milestone.active?
             = link_to 'Reopen Milestone', project_milestone_path(@project, milestone, {state_event: :activate }), method: :put, class: "btn btn-grouped btn-reopen"
         - if @group
-          - if can?(current_user, :admin_milestones, @group)
+          - if can?(current_user, :admin_milestone, @group)
             - if milestone.closed?
               = link_to 'Reopen Milestone', group_milestone_route(milestone, {state_event: :activate }), method: :put, class: "btn btn-sm btn-grouped btn-reopen"
             - else
diff --git a/app/views/shared/milestones/_top.html.haml b/app/views/shared/milestones/_top.html.haml
index 320e3788a0f..0499b04a482 100644
--- a/app/views/shared/milestones/_top.html.haml
+++ b/app/views/shared/milestones/_top.html.haml
@@ -23,7 +23,7 @@
       = milestone_date_range(milestone)
   - if group
     .float-right
-      - if can?(current_user, :admin_milestones, group)
+      - if can?(current_user, :admin_milestone, group)
         - if milestone.group_milestone?
           = link_to edit_group_milestone_path(group, milestone), class: "btn btn btn-grouped" do
             Edit
@@ -32,6 +32,9 @@
         - else
           = link_to 'Reopen Milestone', group_milestone_route(milestone, {state_event: :activate }), method: :put, class: "btn btn-grouped btn-reopen"
 
+        - unless is_dynamic_milestone
+          = render 'shared/milestones/delete_button'
+
 = render 'shared/milestones/deprecation_message' if is_dynamic_milestone
 
 .detail-page-description.milestone-detail