Merge branch 'handle-failure-when-deleting-tags' into 'master'

Display error message when deleting tag in web UI fails

See merge request !9906

app/controllers/projects/tags_controller.rb

@@ -41,13 +41,27 @@ class Projects::TagsController < Projects::ApplicationController
   end
 
   def destroy
-    Tags::DestroyService.new(project, current_user).execute(params[:id])
+    result = Tags::DestroyService.new(project, current_user).execute(params[:id])
 
     respond_to do |format|
-      format.html do
-        redirect_to namespace_project_tags_path(@project.namespace, @project)
+      if result[:status] == :success
+        format.html do
+          redirect_to namespace_project_tags_path(@project.namespace, @project)
+        end
+
+        format.js
+      else
+        @error = result[:message]
+
+        format.html do
+          redirect_to namespace_project_tags_path(@project.namespace, @project),
+            alert: @error
+        end
+
+        format.js do
+          render status: :unprocessable_entity
+        end
       end
-      format.js
     end
   end
 end

app/services/tags/destroy_service.rb

@@ -21,6 +21,8 @@ module Tags
       else
         error('Failed to remove tag')
       end
+    rescue GitHooksService::PreReceiveError => ex
+      error(ex.message)
     end
 
     def error(message, return_code = 400)

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

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

changelogs/unreleased/handle-failure-when-deleting-tags.yml

+---
+title: Display error message when deleting tag in web UI fails
+merge_request: 9906
+author:

spec/features/tags/master_deletes_tag_spec.rb

@@ -10,16 +10,12 @@ feature 'Master deletes tag', feature: true do
     visit namespace_project_tags_path(project.namespace, project)
   end
 
-  context 'from the tags list page' do
+  context 'from the tags list page', js: true do
     scenario 'deletes the tag' do
       expect(page).to have_content 'v1.1.0'
 
-      page.within('.content') do
-        first('.btn-remove').click
-      end
+      delete_first_tag
 
-      expect(current_path).to eq(
-        namespace_project_tags_path(project.namespace, project))
       expect(page).not_to have_content 'v1.1.0'
     end
   end
@@ -37,4 +33,23 @@ feature 'Master deletes tag', feature: true do
       expect(page).not_to have_content 'v1.0.0'
     end
   end
+
+  context 'when pre-receive hook fails', js: true do
+    before do
+      allow_any_instance_of(GitHooksService).to receive(:execute)
+        .and_raise(GitHooksService::PreReceiveError, 'Do not delete tags')
+    end
+
+    scenario 'shows the error message' do
+      delete_first_tag
+
+      expect(page).to have_content('Do not delete tags')
+    end
+  end
+
+  def delete_first_tag
+    page.within('.content') do
+      first('.btn-remove').click
+    end
+  end
 end