diff options
author | Adam Niedzielski <adamsunday@gmail.com> | 2017-03-13 20:12:32 +0300 |
---|---|---|
committer | Adam Niedzielski <adamsunday@gmail.com> | 2017-03-13 20:12:32 +0300 |
commit | 34bf5d863ec1655e3681997c549d159fea6983a9 (patch) | |
tree | e751c69843b79acc74522d07cd2aac440c252d45 | |
parent | 32da7602686f2b8161175d82b121deb9e01b2db5 (diff) |
Display error message when deleting tag in web UI fails
-rw-r--r-- | app/controllers/projects/tags_controller.rb | 22 | ||||
-rw-r--r-- | app/services/tags/destroy_service.rb | 2 | ||||
-rw-r--r-- | app/views/projects/tags/destroy.js.haml | 4 | ||||
-rw-r--r-- | changelogs/unreleased/handle-failure-when-deleting-tags.yml | 4 | ||||
-rw-r--r-- | spec/features/tags/master_deletes_tag_spec.rb | 27 |
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 |