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

Display error message when deleting tag in web UI fails

See merge request !9906

5 files changed, 48 insertions, 11 deletions

diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb
index 33379659d73..ea7e4d9f663 100644
--- a/app/controllers/projects/tags_controller.rb
+++ b/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
diff --git a/app/services/tags/destroy_service.rb b/app/services/tags/destroy_service.rb
index 910b4f5e361..a368f4f5b61 100644
--- a/app/services/tags/destroy_service.rb
+++ b/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)
diff --git a/app/views/projects/tags/destroy.js.haml b/app/views/projects/tags/destroy.js.haml
index e4a78fadbeb..cde23e03d54 100644
--- a/app/views/projects/tags/destroy.js.haml
+++ b/app/views/projects/tags/destroy.js.haml
@@ -1,2 +1,4 @@
-- 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)
diff --git a/changelogs/unreleased/handle-failure-when-deleting-tags.yml b/changelogs/unreleased/handle-failure-when-deleting-tags.yml
new file mode 100644
index 00000000000..99b07c5fb5f
--- /dev/null
+++ b/changelogs/unreleased/handle-failure-when-deleting-tags.yml
@@ -0,0 +1,4 @@
+---
+title: Display error message when deleting tag in web UI fails
+merge_request: 9906
+author:
diff --git a/spec/features/tags/master_deletes_tag_spec.rb b/spec/features/tags/master_deletes_tag_spec.rb
index 0f30f562539..ccfafe6db7d 100644
--- a/spec/features/tags/master_deletes_tag_spec.rb
+++ b/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