Merge branch 'delete-epic' into 'master'

Delete epic

Closes #3552

See merge request gitlab-org/gitlab-ee!3141

app/assets/javascripts/issue_show/components/app.vue

@@ -29,6 +29,11 @@ export default {
       required: false,
       default: false,
     },
+    showDeleteButton: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
     issuableRef: {
       type: String,
       required: true,
@@ -92,6 +97,11 @@ export default {
       type: String,
       required: true,
     },
+    issuableType: {
+      type: String,
+      required: false,
+      default: 'issue',
+    },
   },
   data() {
     const store = new Store({
@@ -157,21 +167,21 @@ export default {
         })
         .catch(() => {
           eventHub.$emit('close.form');
-          window.Flash('Error updating issue');
+          window.Flash(`Error updating ${this.issuableType}`);
         });
     },
     deleteIssuable() {
       this.service.deleteIssuable()
         .then(res => res.json())
         .then((data) => {
-          // Stop the poll so we don't get 404's with the issue not existing
+          // Stop the poll so we don't get 404's with the issuable not existing
           this.poll.stop();
 
           gl.utils.visitUrl(data.web_url);
         })
         .catch(() => {
           eventHub.$emit('close.form');
-          window.Flash('Error deleting issue');
+          window.Flash(`Error deleting ${this.issuableType}`);
         });
     },
   },
@@ -223,6 +233,7 @@ export default {
       :markdown-preview-path="markdownPreviewPath"
       :project-path="projectPath"
       :project-namespace="projectNamespace"
+      :show-delete-button="showDeleteButton"
     />
     <div v-else>
       <title-component

app/assets/javascripts/issue_show/components/edit_actions.vue

@@ -13,6 +13,11 @@
         type: Object,
         required: true,
       },
+      showDeleteButton: {
+        type: Boolean,
+        required: false,
+        default: true,
+      },
     },
     data() {
       return {
@@ -23,6 +28,9 @@
       isSubmitEnabled() {
         return this.formState.title.trim() !== '';
       },
+      shouldShowDeleteButton() {
+        return this.canDestroy && this.showDeleteButton;
+      },
     },
     methods: {
       closeForm() {
@@ -62,7 +70,7 @@
       Cancel
     </button>
     <button
-      v-if="canDestroy"
+      v-if="shouldShowDeleteButton"
       class="btn btn-danger pull-right append-right-default"
       :class="{ disabled: deleteLoading }"
       type="button"

app/assets/javascripts/issue_show/components/form.vue

@@ -36,6 +36,11 @@
         type: String,
         required: true,
       },
+      showDeleteButton: {
+        type: Boolean,
+        required: false,
+        default: true,
+      },
     },
     components: {
       lockedWarning,
@@ -81,6 +86,7 @@
       :markdown-docs-path="markdownDocsPath" />
     <edit-actions
       :form-state="formState"
-      :can-destroy="canDestroy" />
+      :can-destroy="canDestroy"
+      :show-delete-button="showDeleteButton" />
   </form>
 </template>

app/assets/javascripts/vue_shared/components/loading_button.vue

@@ -35,6 +35,11 @@ export default {
       type: String,
       required: false,
     },
+    containerClass: {
+      type: String,
+      required: false,
+      default: 'btn btn-align-content',
+    },
   },
   components: {
     loadingIcon,
@@ -49,9 +54,9 @@ export default {
 
 <template>
   <button
-    class="btn btn-align-content"
     @click="onClick"
     type="button"
+    :class="containerClass"
     :disabled="loading || disabled"
   >
       <transition name="fade">

app/assets/stylesheets/framework/blocks.scss

@@ -353,3 +353,7 @@
   display: -webkit-flex;
   display: flex;
 }
+
+.flex-right {
+  margin-left: auto;
+}

app/services/users/migrate_to_ghost_user_service.rb

@@ -6,6 +6,7 @@
 # when the user is destroyed.
 module Users
   class MigrateToGhostUserService
+    prepend EE::Users::MigrateToGhostUserService
     extend ActiveSupport::Concern
 
     attr_reader :ghost_user, :user

changelogs/unreleased-ee/delete-epic.yml

+---
+title: Add delete epic button
+merge_request:
+author:
+type: added

ee/app/assets/javascripts/epics/epic_show/components/epic_header.vue

@@ -2,6 +2,8 @@
   import userAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue';
   import timeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
   import tooltip from '~/vue_shared/directives/tooltip';
+  import loadingButton from '~/vue_shared/components/loading_button.vue';
+  import { s__ } from '~/locale';
 
   export default {
     name: 'epicHeader',
@@ -15,6 +17,16 @@
         type: String,
         required: true,
       },
+      canDelete: {
+        type: Boolean,
+        required: false,
+        default: false,
+      },
+    },
+    data() {
+      return {
+        deleteLoading: false,
+      };
     },
     directives: {
       tooltip,
@@ -22,6 +34,15 @@
     components: {
       userAvatarLink,
       timeagoTooltip,
+      loadingButton,
+    },
+    methods: {
+      deleteEpic() {
+        if (confirm(s__('Epic will be removed! Are you sure?'))) { // eslint-disable-line no-alert
+          this.deleteLoading = true;
+          this.$emit('deleteEpic');
+        }
+      },
     },
   };
 </script>
@@ -29,21 +50,26 @@
 <template>
   <div class="detail-page-header">
     <div class="issuable-meta">
-      Opened
-      <timeagoTooltip
-        :time="created"
-      />
-      by
+      {{ s__('Opened') }}
+      <timeago-tooltip :time="created" />
+      {{ s__('by') }}
       <strong>
         <user-avatar-link
           :link-href="author.url"
           :img-src="author.src"
           :img-size="24"
-          :tooltipText="author.username"
+          :tooltip-text="author.username"
           :username="author.name"
-          imgCssClasses="avatar-inline"
+          img-css-classes="avatar-inline"
         />
       </strong>
     </div>
+    <loading-button
+      v-if="canDelete"
+      :loading="deleteLoading"
+      @click="deleteEpic"
+      :label="s__('Delete')"
+      container-class="btn btn-remove btn-inverted flex-right"
+    />
   </div>
 </template>

ee/app/assets/javascripts/epics/epic_show/components/epic_show_app.vue

 <script>
   import issuableApp from '~/issue_show/components/app.vue';
+  import issuableAppEventHub from '~/issue_show/event_hub';
   import epicHeader from './epic_header.vue';
   import epicSidebar from '../../sidebar/components/sidebar_app.vue';
 
@@ -65,16 +66,23 @@
         required: false,
       },
     },
+    data() {
+      return {
+        // Epics specific configuration
+        issuableRef: '',
+        projectPath: this.groupPath,
+        projectNamespace: '',
+      };
+    },
     components: {
       epicHeader,
       epicSidebar,
       issuableApp,
     },
-    created() {
-      // Epics specific configuration
-      this.issuableRef = '';
-      this.projectPath = this.groupPath;
-      this.projectNamespace = '';
+    methods: {
+      deleteEpic() {
+        issuableAppEventHub.$emit('delete.issuable');
+      },
     },
   };
 </script>
@@ -84,6 +92,8 @@
     <epic-header
       :author="author"
       :created="created"
+      :canDelete="canDestroy"
+      @deleteEpic="deleteEpic"
     />
     <div class="issuable-details content-block">
       <div class="detail-page-description">
@@ -105,9 +115,21 @@
       </div>
       <epic-sidebar
         :endpoint="endpoint"
+        :issuable-ref="issuableRef"
+        :initial-title-html="initialTitleHtml"
+        :initial-title-text="initialTitleText"
+        :initial-description-html="initialDescriptionHtml"
+        :initial-description-text="initialDescriptionText"
+        :markdown-preview-path="markdownPreviewPath"
+        :markdown-docs-path="markdownDocsPath"
+        :project-path="projectPath"
+        :project-namespace="projectNamespace"
+        :show-inline-edit-button="true"
+        :show-delete-button="false"
+        issuable-type="epic"
         :editable="canUpdate"
-        :initialStartDate="startDate"
-        :initialEndDate="endDate"
+        :initial-start-date="startDate"
+        :initial-end-date="endDate"
       />
     </div>
   </div>

ee/app/assets/javascripts/epics/epic_show/epic_show_bundle.js

@@ -6,11 +6,7 @@ document.addEventListener('DOMContentLoaded', () => {
   const metaData = JSON.parse(el.dataset.meta);
   const initialData = JSON.parse(el.dataset.initial);
 
-  const props = Object.assign({}, initialData, metaData, {
-    // Current iteration does not enable users
-    // to delete epics
-    canDestroy: false,
-  });
+  const props = Object.assign({}, initialData, metaData);
 
   // Convert backend casing to match frontend style guide
   props.startDate = props.start_date;

ee/app/policies/ee/group_policy.rb

@@ -27,13 +27,15 @@ module EE
         enable :create_epic
         enable :admin_epic
         enable :update_epic
-        enable :destroy_epic
       end
 
+      rule { owner }.enable :destroy_epic
+
       rule { auditor }.policy do
         enable  :read_group
         enable  :read_epic
       end
+
       rule { admin }.enable :read_epic
       rule { has_projects }.enable :read_epic
 

ee/app/services/ee/users/migrate_to_ghost_user_service.rb

+module EE
+  module Users
+    module MigrateToGhostUserService
+      private
+
+      def migrate_records
+        migrate_epics
+        super
+      end
+
+      def migrate_epics
+        user.epics.update_all(author_id: ghost_user.id)
+        ::Epic.where(last_edited_by_id: user.id).update_all(last_edited_by_id: ghost_user.id)
+      end
+    end
+  end
+end

spec/ee/spec/controllers/groups/epics_controller_spec.rb

@@ -91,7 +91,7 @@ describe Groups::EpicsController do
 
   describe 'PUT #update' do
     before do
-      group.add_user(user, :developer)
+      group.add_developer(user)
       put :update, group_id: group, id: epic.to_param, epic: { title: 'New title' }, format: :json
     end
 
@@ -107,7 +107,7 @@ describe Groups::EpicsController do
   describe 'GET #realtime_changes' do
     subject { get :realtime_changes, group_id: group, id: epic.to_param }
     it 'returns epic' do
-      group.add_user(user, :developer)
+      group.add_developer(user)
       subject
 
       expect(response.content_type).to eq 'application/json'
@@ -122,4 +122,25 @@ describe Groups::EpicsController do
       end
     end
   end
+
+  describe "DELETE #destroy" do
+    before do
+      sign_in(user)
+    end
+
+    it "rejects a developer to destroy an epic" do
+      group.add_developer(user)
+      delete :destroy, group_id: group, id: epic.to_param
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    it "deletes the epic" do
+      group.add_owner(user)
+      delete :destroy, group_id: group, id: epic.to_param
+
+      expect(response).to have_gitlab_http_status(302)
+      expect(controller).to set_flash[:notice].to(/The epic was successfully deleted\./)
+    end
+  end
 end

spec/ee/spec/features/epics/delete_epic_spec.rb

+require 'spec_helper'
+
+feature 'Delete Epic', :js do
+  let(:user) { create(:user) }
+  let(:group) { create(:group, :public) }
+  let(:epic) { create(:epic, group: group) }
+  let!(:epic2) { create(:epic, group: group) }
+
+  before do
+    sign_in(user)
+  end
+
+  context 'when user who is not a group member displays the epic' do
+    it 'does not show the Delete button' do
+      visit group_epic_path(group, epic)
+
+      expect(page).not_to have_selector('.detail-page-header button')
+    end
+  end
+
+  context 'when user with owner access displays the epic' do
+    before do
+      group.add_owner(user)
+      visit group_epic_path(group, epic)
+      wait_for_requests
+    end
+
+    it 'deletes the issue and redirect to epic list' do
+      page.accept_alert 'Epic will be removed! Are you sure?' do
+        find('.detail-page-header button').click
+      end
+
+      wait_for_requests
+
+      expect(find('.issuable-list')).not_to have_content(epic.title)
+      expect(find('.issuable-list')).to have_content(epic2.title)
+    end
+  end
+end

spec/ee/spec/policies/epic_policy_spec.rb

@@ -32,6 +32,14 @@ describe EpicPolicy do
     it 'reporter group member can manage epics' do
       group.add_reporter(user)
 
+      expect(permissions(user, group)).to be_disallowed(:destroy_epic)
+      expect(permissions(user, group))
+        .to be_allowed(:read_epic, :update_epic, :admin_epic, :create_epic)
+    end
+
+    it 'only group owner can destroy epics' do
+      group.add_owner(user)
+
       expect(permissions(user, group))
         .to be_allowed(:read_epic, :update_epic, :destroy_epic, :admin_epic, :create_epic)
     end
@@ -60,6 +68,14 @@ describe EpicPolicy do
     it 'reporter group member can manage epics' do
       group.add_reporter(user)
 
+      expect(permissions(user, group)).to be_disallowed(:destroy_epic)
+      expect(permissions(user, group))
+        .to be_allowed(:read_epic, :update_epic, :admin_epic, :create_epic)
+    end
+
+    it 'only group owner can destroy epics' do
+      group.add_owner(user)
+
       expect(permissions(user, group))
         .to be_allowed(:read_epic, :update_epic, :destroy_epic, :admin_epic, :create_epic)
     end
@@ -88,6 +104,14 @@ describe EpicPolicy do
     it 'reporter group member can manage epics' do
       group.add_reporter(user)
 
+      expect(permissions(user, group)).to be_disallowed(:destroy_epic)
+      expect(permissions(user, group))
+        .to be_allowed(:read_epic, :update_epic, :admin_epic, :create_epic)
+    end
+
+    it 'only group owner can destroy epics' do
+      group.add_owner(user)
+
       expect(permissions(user, group))
         .to be_allowed(:read_epic, :update_epic, :destroy_epic, :admin_epic, :create_epic)
     end

spec/ee/spec/services/ee/users/migrate_to_ghost_user_service_spec.rb

+require 'spec_helper'
+
+describe Users::MigrateToGhostUserService do
+  context 'epics'  do
+    let!(:user)      { create(:user) }
+    let(:service)    { described_class.new(user) }
+
+    context 'deleted user is present as both author and edited_user' do
+      include_examples "migrating a deleted user's associated records to the ghost user", Epic, [:author, :last_edited_by] do
+        let(:created_record) do
+          create(:epic, group: create(:group), author: user, last_edited_by: user)
+        end
+      end
+    end
+
+    context 'deleted user is present only as edited_user' do
+      include_examples "migrating a deleted user's associated records to the ghost user", Epic, [:last_edited_by] do
+        let(:created_record) { create(:epic, group: create(:group), author: create(:user), last_edited_by: user) }
+      end
+    end
+  end
+end

spec/javascripts/epics/epic_show/components/epic_header_spec.js

@@ -31,4 +31,42 @@ describe('epicHeader', () => {
   it('should render username tooltip', () => {
     expect(vm.$el.querySelector('.user-avatar-link span').dataset.originalTitle).toEqual(author.username);
   });
+
+  describe('canDelete', () => {
+    it('should not show loading button by default', () => {
+      expect(vm.$el.querySelector('.btn-remove')).toBeNull();
+    });
+
+    it('should show loading button if canDelete', (done) => {
+      vm.canDelete = true;
+      Vue.nextTick(() => {
+        expect(vm.$el.querySelector('.btn-remove')).toBeDefined();
+        done();
+      });
+    });
+  });
+
+  describe('delete epic', () => {
+    let deleteEpic;
+
+    beforeEach((done) => {
+      deleteEpic = jasmine.createSpy();
+      spyOn(window, 'confirm').and.returnValue(true);
+      vm.canDelete = true;
+      vm.$on('deleteEpic', deleteEpic);
+
+      Vue.nextTick(() => {
+        vm.$el.querySelector('.btn-remove').click();
+        done();
+      });
+    });
+
+    it('should set deleteLoading', () => {
+      expect(vm.deleteLoading).toEqual(true);
+    });
+
+    it('should emit deleteEpic event', () => {
+      expect(deleteEpic).toHaveBeenCalled();
+    });
+  });
 });

spec/javascripts/epics/epic_show/components/epic_show_app_spec.js

@@ -3,6 +3,8 @@ import epicShowApp from 'ee/epics/epic_show/components/epic_show_app.vue';
 import epicHeader from 'ee/epics/epic_show/components/epic_header.vue';
 import epicSidebar from 'ee/epics/sidebar/components/sidebar_app.vue';
 import issuableApp from '~/issue_show/components/app.vue';
+import issuableAppEventHub from '~/issue_show/event_hub';
+import '~/lib/utils/url_utility';
 import mountComponent from '../../../helpers/vue_mount_component_helper';
 import { props } from '../mock_data';
 import issueShowData from '../../../issue_show/mock_data';
@@ -43,6 +45,7 @@ describe('EpicShowApp', () => {
     headerVm = mountComponent(EpicHeader, {
       author,
       created,
+      canDelete: canDestroy,
     });
 
     const IssuableApp = Vue.extend(issuableApp);
@@ -86,4 +89,14 @@ describe('EpicShowApp', () => {
   it('should render epic-sidebar', () => {
     expect(vm.$el.innerHTML.indexOf(sidebarVm.$el.innerHTML) !== -1).toEqual(true);
   });
+
+  it('should emit delete.issuable when epic is deleted', () => {
+    const deleteIssuable = jasmine.createSpy();
+    issuableAppEventHub.$on('delete.issuable', deleteIssuable);
+    spyOn(window, 'confirm').and.returnValue(true);
+    spyOn(gl.utils, 'visitUrl').and.callFake(() => {});
+
+    vm.$el.querySelector('.detail-page-header .btn-remove').click();
+    expect(deleteIssuable).toHaveBeenCalled();
+  });
 });

spec/javascripts/epics/epic_show/mock_data.js

 export const contentProps = {
   endpoint: '',
   canUpdate: true,
-  canDestroy: false,
+  canDestroy: true,
   markdownPreviewPath: '',
   markdownDocsPath: '',
   groupPath: '',

spec/javascripts/issue_show/components/app_spec.js

@@ -223,23 +223,46 @@ describe('Issuable output', () => {
       });
     });
 
-    it('closes form on error', (done) => {
-      spyOn(window, 'Flash').and.callThrough();
-      spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve, reject) => {
-        reject();
-      }));
+    describe('error when updating', () => {
+      beforeEach(() => {
+        spyOn(window, 'Flash').and.callThrough();
+        spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve, reject) => {
+          reject();
+        }));
+      });
 
-      vm.updateIssuable();
+      it('closes form on error', (done) => {
+        vm.updateIssuable();
 
-      setTimeout(() => {
-        expect(
-          eventHub.$emit,
-        ).toHaveBeenCalledWith('close.form');
-        expect(
-          window.Flash,
-        ).toHaveBeenCalledWith('Error updating issue');
+        setTimeout(() => {
+          expect(
+            eventHub.$emit,
+          ).toHaveBeenCalledWith('close.form');
+          expect(
+            window.Flash,
+          ).toHaveBeenCalledWith('Error updating issue');
 
-        done();
+          done();
+        });
+      });
+
+      it('returns the correct error message for issuableType', (done) => {
+        vm.issuableType = 'merge request';
+
+        Vue.nextTick(() => {
+          vm.updateIssuable();
+
+          setTimeout(() => {
+            expect(
+              eventHub.$emit,
+            ).toHaveBeenCalledWith('close.form');
+            expect(
+              window.Flash,
+            ).toHaveBeenCalledWith('Error updating merge request');
+
+            done();
+          });
+        });
       });
     });
   });

spec/javascripts/issue_show/components/edit_actions_spec.js

@@ -61,6 +61,15 @@ describe('Edit Actions components', () => {
     });
   });
 
+  it('should not show delete button if showDeleteButton is false', (done) => {
+    vm.showDeleteButton = false;
+
+    Vue.nextTick(() => {
+      expect(vm.$el.querySelector('.btn-danger')).toBeNull();
+      done();
+    });
+  });
+
   describe('updateIssuable', () => {
     it('sends update.issauble event when clicking save button', () => {
       vm.$el.querySelector('.btn-save').click();

spec/javascripts/vue_shared/components/loading_button_spec.js

@@ -66,6 +66,23 @@ describe('LoadingButton', function () {
     });
   });
 
+  describe('container class', () => {
+    it('should default to btn btn-align-content', () => {
+      vm = mountComponent(LoadingButton, {});
+      expect(vm.$el.classList.contains('btn')).toEqual(true);
+      expect(vm.$el.classList.contains('btn-align-content')).toEqual(true);
+    });
+
+    it('should be configurable through props', () => {
+      vm = mountComponent(LoadingButton, {
+        containerClass: 'test-class',
+      });
+      expect(vm.$el.classList.contains('btn')).toEqual(false);
+      expect(vm.$el.classList.contains('btn-align-content')).toEqual(false);
+      expect(vm.$el.classList.contains('test-class')).toEqual(true);
+    });
+  });
+
   describe('click callback prop', () => {
     it('calls given callback when normal', () => {
       vm = mountComponent(LoadingButton, {

spec/policies/group_policy_spec.rb

@@ -26,6 +26,7 @@ describe GroupPolicy do
       :admin_namespace,
       :admin_group_member,
       :change_visibility_level,
+      :destroy_epic,
       (Gitlab::Database.postgresql? ? :create_subgroup : nil)
     ].compact
   end