Merge branch 'ek-spike-remove-tags-destory-js-haml' into 'master'

Replace tags/destroy.js.haml file

See merge request gitlab-org/gitlab!42941

app/assets/javascripts/confirm_modal.js

 import Vue from 'vue';
 import ConfirmModal from '~/vue_shared/components/confirm_modal.vue';
 
-const mountConfirmModal = () => {
-  return new Vue({
+const mountConfirmModal = optionalProps =>
+  new Vue({
     render(h) {
       return h(ConfirmModal, {
-        props: { selector: '.js-confirm-modal-button' },
+        props: {
+          selector: '.js-confirm-modal-button',
+          ...optionalProps,
+        },
       });
     },
   }).$mount();
-};
 
-export default () => mountConfirmModal();
+export default (optionalProps = {}) => mountConfirmModal(optionalProps);

app/assets/javascripts/lib/utils/url_utility.js

@@ -255,6 +255,15 @@ export function getBaseURL() {
   return `${protocol}//${host}`;
 }
 
+/**
+ * Takes a URL and returns content from the start until the final '/'
+ *
+ * @param {String} url - full url, including protocol and host
+ */
+export function stripFinalUrlSegment(url) {
+  return new URL('.', url).href;
+}
+
 /**
  * Returns true if url is an absolute URL
  *

app/assets/javascripts/pages/projects/tags/index/index.js

+import { initRemoveTag } from '../remove_tag';
+
+document.addEventListener('DOMContentLoaded', () => {
+  initRemoveTag({
+    onDelete: path => {
+      document
+        .querySelector(`[data-path="${path}"]`)
+        .closest('.js-tag-list')
+        .remove();
+    },
+  });
+});

app/assets/javascripts/pages/projects/tags/remove_tag.js

+import createFlash from '~/flash';
+import axios from '~/lib/utils/axios_utils';
+import initConfirmModal from '~/confirm_modal';
+
+export const initRemoveTag = ({ onDelete = () => {} }) => {
+  return initConfirmModal({
+    handleSubmit: (path = '') =>
+      axios
+        .delete(path)
+        .then(() => onDelete(path))
+        .catch(({ response: { data } }) => {
+          const { message } = data;
+          createFlash({ message });
+        }),
+  });
+};

app/assets/javascripts/pages/projects/tags/show/index.js

+import { redirectTo, getBaseURL, stripFinalUrlSegment } from '~/lib/utils/url_utility';
+import { initRemoveTag } from '../remove_tag';
+
+document.addEventListener('DOMContentLoaded', () => {
+  initRemoveTag({
+    onDelete: (path = '') => {
+      redirectTo(stripFinalUrlSegment([getBaseURL(), path].join('')));
+    },
+  });
+});

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

@@ -12,6 +12,11 @@ export default {
       type: String,
       required: true,
     },
+    handleSubmit: {
+      type: Function,
+      required: false,
+      default: null,
+    },
   },
   data() {
     return {
@@ -41,7 +46,11 @@ export default {
       this.$refs.modal.hide();
     },
     submitModal() {
-      this.$refs.form.submit();
+      if (this.handleSubmit) {
+        this.handleSubmit(this.path);
+      } else {
+        this.$refs.form.submit();
+      }
     },
   },
   csrf,

app/controllers/projects/tags_controller.rb

@@ -76,25 +76,10 @@ class Projects::TagsController < Projects::ApplicationController
   def destroy
     result = ::Tags::DestroyService.new(project, current_user).execute(params[:id])
 
-    respond_to do |format|
-      if result[:status] == :success
-        format.html do
-          redirect_to project_tags_path(@project), status: :see_other
-        end
-
-        format.js
-      else
-        @error = result[:message]
-
-        format.html do
-          redirect_to project_tags_path(@project),
-            alert: @error, status: :see_other
-        end
-
-        format.js do
-          render status: :ok
-        end
-      end
+    if result[:status] == :success
+      render json: result
+    else
+      render json: { message: result[:message] }, status: result[:return_code]
     end
   end
 

app/helpers/tags_helper.rb

@@ -38,4 +38,13 @@ module TagsHelper
 
     text.html_safe
   end
+
+  def delete_tag_modal_attributes(tag_name)
+    {
+      title: s_('TagsPage|Delete tag'),
+      message: s_('TagsPage|Deleting the %{tag_name} tag cannot be undone. Are you sure?') % { tag_name: tag_name },
+      okVariant: 'danger',
+      okTitle: s_('TagsPage|Delete tag')
+    }.to_json
+  end
 end

app/views/projects/buttons/_remove_tag.html.haml

+- project = local_assigns.fetch(:project, nil)
+- tag = local_assigns.fetch(:tag, nil)
+- return unless project && tag
+
+%button{ type: "button", class: "js-remove-tag js-confirm-modal-button gl-button btn btn-remove remove-row has-tooltip gl-ml-3 #{protected_tag?(project, tag) ? 'disabled' : ''}", title: s_('TagsPage|Delete tag'), data: { container: 'body', path: project_tag_path(@project, tag.name), modal_attributes: delete_tag_modal_attributes(tag.name) } }
+  = sprite_icon("remove")

app/views/projects/tags/_tag.html.haml

 - commit = @repository.commit(tag.dereferenced_target)
 - release = @releases.find { |release| release.tag == tag.name }
-%li.flex-row.allow-wrap
+
+%li.flex-row.allow-wrap.js-tag-list
   .row-main-content
     = sprite_icon('tag')
     = link_to tag.name, project_tag_path(@project, tag.name), class: 'item-title ref-name'
@@ -38,5 +39,4 @@
     - if can?(current_user, :admin_tag, @project)
       = link_to edit_project_tag_release_path(@project, tag.name), class: 'btn btn-edit has-tooltip', title: s_('TagsPage|Edit release notes'), data: { container: "body" } do
         = sprite_icon("pencil")
-      = link_to project_tag_path(@project, tag.name), class: "btn btn-remove remove-row has-tooltip gl-ml-3 #{protected_tag?(@project, tag) ? 'disabled' : ''}", title: s_('TagsPage|Delete tag'), method: :delete, data: { confirm: s_('TagsPage|Deleting the %{tag_name} tag cannot be undone. Are you sure?') % { tag_name: tag.name }, container: 'body' }, remote: true do
-        = sprite_icon("remove")
+      = render 'projects/buttons/remove_tag', project: @project, tag: tag

app/views/projects/tags/destroy.js.haml

-- if @error.present?
-  new Flash({ message: '#{escape_javascript(@error)}', type: 'alert' });
-- elsif @repository.tags.empty?
-  $('.tags').load(document.URL + ' .nothing-here-block').hide().fadeIn(1000)

app/views/projects/tags/show.html.haml

@@ -52,8 +52,7 @@
       = render 'projects/buttons/download', project: @project, ref: @tag.name
     - if can?(current_user, :admin_tag, @project)
       .btn-container.controls-item-full
-        = link_to project_tag_path(@project, @tag.name), class: "btn btn-icon btn-danger gl-button remove-row has-tooltip #{protected_tag?(@project, @tag) ? 'disabled' : ''}", title: s_('TagsPage|Delete tag'), method: :delete, data: { confirm: s_('TagsPage|Deleting the %{tag_name} tag cannot be undone. Are you sure?') % { tag_name: @tag.name } } do
-          = sprite_icon('remove', css_class: 'gl-icon')
+        = render 'projects/buttons/remove_tag', project: @project, tag: @tag
 
   - if @tag.message.present?
     %pre.wrap{ data: { qa_selector: 'tag_message_content' } }

spec/controllers/projects/tags_controller_spec.rb

@@ -131,4 +131,25 @@ RSpec.describe Projects::TagsController do
       end
     end
   end
+
+  describe 'DELETE #destroy' do
+    let(:tag) { project.repository.add_tag(user, 'fake-tag', 'master') }
+    let(:request) do
+      delete(:destroy, params: { id: tag.name, namespace_id: project.namespace.to_param, project_id: project })
+    end
+
+    before do
+      project.add_developer(user)
+      sign_in(user)
+    end
+
+    it 'deletes tag' do
+      request
+
+      expect(response).to be_successful
+      expect(response.body).to include("Tag was removed")
+
+      expect(project.repository.find_tag(tag.name)).not_to be_present
+    end
+  end
 end

spec/features/tags/developer_deletes_tag_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Developer deletes tag' do
+RSpec.describe 'Developer deletes tag', :js do
   let(:user) { create(:user) }
   let(:group) { create(:group) }
   let(:project) { create(:project, :repository, namespace: group) }
@@ -13,11 +13,12 @@ RSpec.describe 'Developer deletes tag' do
     visit project_tags_path(project)
   end
 
-  context 'from the tags list page', :js do
+  context 'from the tags list page' do
     it 'deletes the tag' do
       expect(page).to have_content 'v1.1.0'
 
-      delete_tag 'v1.1.0'
+      container = page.find('.content .flex-row', text: 'v1.1.0')
+      delete_tag container
 
       expect(page).not_to have_content 'v1.1.0'
     end
@@ -29,15 +30,15 @@ RSpec.describe 'Developer deletes tag' do
       expect(current_path).to eq(
         project_tag_path(project, 'v1.0.0'))
 
-      click_on 'Delete tag'
+      container = page.find('.nav-controls')
+      delete_tag container
 
-      expect(current_path).to eq(
-        project_tags_path(project))
+      expect(current_path).to eq("#{project_tags_path(project)}/")
       expect(page).not_to have_content 'v1.0.0'
     end
   end
 
-  context 'when pre-receive hook fails', :js do
+  context 'when pre-receive hook fails' do
     before do
       allow_next_instance_of(Gitlab::GitalyClient::OperationService) do |instance|
         allow(instance).to receive(:rm_tag)
@@ -46,15 +47,17 @@ RSpec.describe 'Developer deletes tag' do
     end
 
     it 'shows the error message' do
-      delete_tag 'v1.1.0'
+      container = page.find('.content .flex-row', text: 'v1.1.0')
+      delete_tag container
 
       expect(page).to have_content('Do not delete tags')
     end
   end
 
-  def delete_tag(tag)
-    page.within('.content') do
-      accept_confirm { find("li > .row-fixed-content.controls a.btn-remove[href='/#{project.full_path}/-/tags/#{tag}']").click }
-    end
+  def delete_tag(container)
+    container.find('.js-remove-tag').click
+
+    page.within('.modal') { click_button('Delete tag') }
+    wait_for_requests
   end
 end

spec/frontend/lib/utils/url_utility_spec.js

@@ -701,6 +701,18 @@ describe('URL utility', () => {
     });
   });
 
+  describe('stripFinalUrlSegment', () => {
+    it.each`
+      path                                                        | expected
+      ${'http://fake.domain/twitter/typeahead-js/-/tags/v0.11.0'} | ${'http://fake.domain/twitter/typeahead-js/-/tags/'}
+      ${'http://fake.domain/bar/cool/-/nested/content'}           | ${'http://fake.domain/bar/cool/-/nested/'}
+      ${'http://fake.domain/bar/cool?q="search"'}                 | ${'http://fake.domain/bar/'}
+      ${'http://fake.domain/bar/cool#link-to-something'}          | ${'http://fake.domain/bar/'}
+    `('stripFinalUrlSegment $path => $expected', ({ path, expected }) => {
+      expect(urlUtils.stripFinalUrlSegment(path)).toBe(expected);
+    });
+  });
+
   describe('escapeFileUrl', () => {
     it('encodes URL excluding the slashes', () => {
       expect(urlUtils.escapeFileUrl('/foo-bar/file.md')).toBe('/foo-bar/file.md');

spec/frontend/vue_shared/components/confirm_modal_spec.js

@@ -86,6 +86,22 @@ describe('vue_shared/components/confirm_modal', () => {
         expect(findForm().element.submit).not.toHaveBeenCalled();
       });
 
+      describe('with handleSubmit prop', () => {
+        const handleSubmit = jest.fn();
+        beforeEach(() => {
+          createComponent({ handleSubmit });
+          findModal().vm.$emit('primary');
+        });
+
+        it('will call handleSubmit', () => {
+          expect(handleSubmit).toHaveBeenCalled();
+        });
+
+        it('does not submit the form', () => {
+          expect(findForm().element.submit).not.toHaveBeenCalled();
+        });
+      });
+
       describe('when modal submitted', () => {
         beforeEach(() => {
           findModal().vm.$emit('primary');