Move exception handling to execute

6 files changed, 47 insertions, 32 deletions

diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 7e38aacc91a..f6e8b6655f2 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -15,29 +15,48 @@ module Projects
     def execute
       return false unless can?(current_user, :remove_project, project)
 
-      repo_path = project.path_with_namespace
-      wiki_path = repo_path + '.wiki'
-
       # Flush the cache for both repositories. This has to be done _before_
       # removing the physical repositories as some expiration code depends on
       # Git data (e.g. a list of branch names).
-      flush_caches(project, wiki_path)
+      flush_caches(project)
 
       Projects::UnlinkForkService.new(project, current_user).execute
 
-      attempt_destroy_transaction(project, repo_path, wiki_path)
+      attempt_destroy_transaction(project)
 
       system_hook_service.execute_hooks_for(project, :destroy)
-
       log_info("Project \"#{project.full_path}\" was removed")
+
       true
-    rescue Projects::DestroyService::DestroyError => error
-      Rails.logger.error("Deletion failed on #{project.full_path} with the following message: #{error.message}")
+    rescue => error
+      attempt_rollback(project, error.message)
       false
+    rescue Exception => error # rubocop:disable Lint/RescueException
+      # Project.transaction can raise Exception
+      attempt_rollback(project, error.message)
+      raise
     end
 
     private
 
+    def repo_path
+      project.path_with_namespace
+    end
+
+    def wiki_path
+      repo_path + '.wiki'
+    end
+
+    def trash_repositories!
+      unless remove_repository(repo_path)
+        raise_error('Failed to remove project repository. Please try again or contact administrator.')
+      end
+
+      unless remove_repository(wiki_path)
+        raise_error('Failed to remove wiki repository. Please try again or contact administrator.')
+      end
+    end
+
     def remove_repository(path)
       # Skip repository removal. We use this flag when remove user or group
       return true if params[:skip_repo] == true
@@ -59,26 +78,24 @@ module Projects
       end
     end
 
-    def attempt_destroy_transaction(project, repo_path, wiki_path)
+    def attempt_rollback(project, message)
+      return unless project
+
+      project.update_attributes(delete_error: message, pending_delete: false)
+      log_error("Deletion failed on #{project.full_path} with the following message: #{message}")
+    end
+
+    def attempt_destroy_transaction(project)
       Project.transaction do
         unless remove_legacy_registry_tags
           raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.')
         end
 
-        unless remove_repository(repo_path)
-          raise_error('Failed to remove project repository. Please try again or contact administrator.')
-        end
-
-        unless remove_repository(wiki_path)
-          raise_error('Failed to remove wiki repository. Please try again or contact administrator.')
-        end
+        trash_repositories!
 
         project.team.truncate
         project.destroy!
       end
-    rescue Exception => error # rubocop:disable Lint/RescueException
-      project.update_attributes(delete_error: error.message, pending_delete: false)
-      raise
     end
 
     ##
@@ -107,7 +124,7 @@ module Projects
       "#{path}+#{project.id}#{DELETED_FLAG}"
     end
 
-    def flush_caches(project, wiki_path)
+    def flush_caches(project)
       project.repository.before_delete
 
       Repository.new(wiki_path, project).before_delete
diff --git a/app/views/projects/_deletion_failed.html.haml b/app/views/projects/_deletion_failed.html.haml
index 028510b5671..4f3698f91e6 100644
--- a/app/views/projects/_deletion_failed.html.haml
+++ b/app/views/projects/_deletion_failed.html.haml
@@ -1,9 +1,6 @@
-- if @project.delete_error.present?
-  .project-deletion-failed-message.alert.alert-warning
-    This project was scheduled for deletion, but failed with the following message:
-    = @project.delete_error
+- project = local_assigns.fetch(:project)
+- return unless project.delete_error.present?
 
-    .alert-link-group
-      = link_to "Don't show again", profile_path(user: { hide_no_ssh_key: true }), method: :put, class: 'alert-link'
-      |
-      = link_to 'Remind later', '#', class: 'hide-no-ssh-message alert-link'
+.project-deletion-failed-message.alert.alert-warning
+  This project was scheduled for deletion, but failed with the following message:
+  = project.delete_error
diff --git a/app/views/projects/_flash_messages.html.haml b/app/views/projects/_flash_messages.html.haml
index 6c9d466c761..f47d84ef755 100644
--- a/app/views/projects/_flash_messages.html.haml
+++ b/app/views/projects/_flash_messages.html.haml
@@ -1,5 +1,8 @@
+- project = local_assigns.fetch(:project)
+- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
+
 = content_for flash_message_container do
-  = render 'deletion_failed'
+  = render partial: 'deletion_failed', locals: { project: project }
   - if current_user && can?(current_user, :download_code, project)
     = render 'shared/no_ssh'
     = render 'shared/no_password'
diff --git a/app/views/projects/empty.html.haml b/app/views/projects/empty.html.haml
index 3d7c72ae61a..d17709380d5 100644
--- a/app/views/projects/empty.html.haml
+++ b/app/views/projects/empty.html.haml
@@ -1,5 +1,4 @@
 - @no_container = true
-- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
 
 = render partial: 'flash_messages', locals: { project: @project }
 
diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml
index 3926149e790..a9b39cedb1d 100644
--- a/app/views/projects/show.html.haml
+++ b/app/views/projects/show.html.haml
@@ -1,7 +1,6 @@
 - @no_container = true
 - breadcrumb_title "Project"
 - @content_class = "limit-container-width" unless fluid_layout
-- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
 
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, project_path(@project, rss_url_options), title: "#{@project.name} activity")
diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb
index e695ec060f0..a9188b78460 100644
--- a/app/workers/project_destroy_worker.rb
+++ b/app/workers/project_destroy_worker.rb
@@ -8,6 +8,6 @@ class ProjectDestroyWorker
 
     ::Projects::DestroyService.new(project, user, params.symbolize_keys).execute
   rescue ActiveRecord::RecordNotFound => error
-    logger.error("Failed to delete project #{project.path_with_namespace} (#{project.id}): #{error.message}")
+    logger.error("Failed to delete project (#{project_id}): #{error.message}")
   end
 end