summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/services/projects/destroy_service.rb2
-rw-r--r--app/views/projects/_deletion_failed.html.haml4
-rw-r--r--app/views/projects/_flash_messages.html.haml5
-rw-r--r--app/views/projects/empty.html.haml5
-rw-r--r--app/views/projects/show.html.haml6
-rw-r--r--app/workers/project_destroy_worker.rb15
-rw-r--r--db/migrate/20170428064307_add_column_delete_error_to_projects.rb24
-rw-r--r--spec/features/projects/show_project_spec.rb30
-rw-r--r--spec/workers/project_destroy_worker_spec.rb21
9 files changed, 63 insertions, 49 deletions
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 682407ac896..7b0a08af290 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -44,7 +44,7 @@ module Projects
system_hook_service.execute_hooks_for(project, :destroy)
- log_info("Project \"#{project.path_with_namespace}\" was removed")
+ log_info("Project \"#{project.full_path}\" was removed")
true
end
diff --git a/app/views/projects/_deletion_failed.html.haml b/app/views/projects/_deletion_failed.html.haml
index cd717760432..028510b5671 100644
--- a/app/views/projects/_deletion_failed.html.haml
+++ b/app/views/projects/_deletion_failed.html.haml
@@ -1,9 +1,9 @@
- if @project.delete_error.present?
.project-deletion-failed-message.alert.alert-warning
- This project was scheduled for deletion, but failed with the message:
+ This project was scheduled for deletion, but failed with the following message:
= @project.delete_error
.alert-link-group
- = link_to "Don't show again", profile_path(user: {hide_no_ssh_key: true}), method: :put, class: 'alert-link'
+ = 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'
diff --git a/app/views/projects/_flash_messages.html.haml b/app/views/projects/_flash_messages.html.haml
new file mode 100644
index 00000000000..6c9d466c761
--- /dev/null
+++ b/app/views/projects/_flash_messages.html.haml
@@ -0,0 +1,5 @@
+= content_for flash_message_container do
+ = render 'deletion_failed'
+ - 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 0f132a68ce1..3d7c72ae61a 100644
--- a/app/views/projects/empty.html.haml
+++ b/app/views/projects/empty.html.haml
@@ -1,10 +1,7 @@
- @no_container = true
- flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
-= content_for flash_message_container do
- - if current_user && can?(current_user, :download_code, @project)
- = render 'shared/no_ssh'
- = render 'shared/no_password'
+= render partial: 'flash_messages', locals: { project: @project }
= render "projects/head"
= render "home_panel"
diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml
index 336bc694ffc..3926149e790 100644
--- a/app/views/projects/show.html.haml
+++ b/app/views/projects/show.html.haml
@@ -6,11 +6,7 @@
= content_for :meta_tags do
= auto_discovery_link_tag(:atom, project_path(@project, rss_url_options), title: "#{@project.name} activity")
-= content_for flash_message_container do
- = render 'deletion_failed'
- - if current_user && can?(current_user, :download_code, @project)
- = render 'shared/no_ssh'
- = render 'shared/no_password'
+= render partial: 'flash_messages', locals: { project: @project }
= render "projects/head"
= render "projects/last_push"
diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb
index 482e1e38cd1..209cf11e893 100644
--- a/app/workers/project_destroy_worker.rb
+++ b/app/workers/project_destroy_worker.rb
@@ -3,17 +3,14 @@ class ProjectDestroyWorker
include DedicatedSidekiqQueue
def perform(project_id, user_id, params)
- begin
- project = Project.unscoped.find(project_id)
- rescue ActiveRecord::RecordNotFound
- return
- end
-
+ project = Project.find(project_id)
user = User.find(user_id)
::Projects::DestroyService.new(project, user, params.symbolize_keys).execute
- rescue StandardError => error
- project.assign_attributes(delete_error: error.message, pending_delete: false)
- project.save!(validate: false)
+ rescue Exception => error # rubocop:disable Lint/RescueException
+ project&.update_attributes(delete_error: error.message, pending_delete: false)
+ Rails.logger.error("Deletion failed on #{project&.full_path} with the following message: #{error.message}")
+
+ raise
end
end
diff --git a/db/migrate/20170428064307_add_column_delete_error_to_projects.rb b/db/migrate/20170428064307_add_column_delete_error_to_projects.rb
index ef5fc2cdea5..09f9d9b5b7a 100644
--- a/db/migrate/20170428064307_add_column_delete_error_to_projects.rb
+++ b/db/migrate/20170428064307_add_column_delete_error_to_projects.rb
@@ -1,30 +1,6 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
class AddColumnDeleteErrorToProjects < ActiveRecord::Migration
- include Gitlab::Database::MigrationHelpers
-
- # Set this constant to true if this migration requires downtime.
DOWNTIME = false
- # When a migration requires downtime you **must** uncomment the following
- # constant and define a short and easy to understand explanation as to why the
- # migration requires downtime.
- # DOWNTIME_REASON = ''
-
- # When using the methods "add_concurrent_index", "remove_concurrent_index" or
- # "add_column_with_default" you must disable the use of transactions
- # as these methods can not run in an existing transaction.
- # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
- # that either of them is the _only_ method called in the migration,
- # any other changes should go in a separate migration.
- # This ensures that upon failure _only_ the index creation or removing fails
- # and can be retried or reverted easily.
- #
- # To disable transactions uncomment the following line and remove these
- # comments:
- # disable_ddl_transaction!
-
def change
add_column :projects, :delete_error, :text
end
diff --git a/spec/features/projects/show_project_spec.rb b/spec/features/projects/show_project_spec.rb
new file mode 100644
index 00000000000..5aa0d8f0026
--- /dev/null
+++ b/spec/features/projects/show_project_spec.rb
@@ -0,0 +1,30 @@
+require 'spec_helper'
+
+describe 'Project show page', feature: true do
+ context 'when project pending delete' do
+ let(:project) { create(:project, :empty_repo, pending_delete: true) }
+ let(:worker) { ProjectDestroyWorker.new }
+
+ before do
+ sign_in(project.owner)
+ end
+
+ it 'shows flash error if deletion for project fails' do
+ error_message = "some error message"
+ project.update_attributes(delete_error: error_message, pending_delete: false)
+
+ visit namespace_project_path(project.namespace, project)
+
+ expect(page).to have_selector('.project-deletion-failed-message')
+ expect(page).to have_content("This project was scheduled for deletion, but failed with the following message: #{error_message}")
+ end
+
+ it 'renders 404 if project was successfully deleted' do
+ worker.perform(project.id, project.owner.id, {})
+
+ visit namespace_project_path(project.namespace, project)
+
+ expect(page).to have_http_status(404)
+ end
+ end
+end
diff --git a/spec/workers/project_destroy_worker_spec.rb b/spec/workers/project_destroy_worker_spec.rb
index 3d135f40c1f..29f0295de42 100644
--- a/spec/workers/project_destroy_worker_spec.rb
+++ b/spec/workers/project_destroy_worker_spec.rb
@@ -1,24 +1,37 @@
require 'spec_helper'
describe ProjectDestroyWorker do
- let(:project) { create(:project, :repository) }
+ let(:project) { create(:project, :repository, pending_delete: true) }
let(:path) { project.repository.path_to_repo }
subject { described_class.new }
- describe "#perform" do
- it "deletes the project" do
+ describe '#perform' do
+ it 'deletes the project' do
subject.perform(project.id, project.owner.id, {})
expect(Project.all).not_to include(project)
expect(Dir.exist?(path)).to be_falsey
end
- it "deletes the project but skips repo deletion" do
+ it 'deletes the project but skips repo deletion' do
subject.perform(project.id, project.owner.id, { "skip_repo" => true })
expect(Project.all).not_to include(project)
expect(Dir.exist?(path)).to be_truthy
end
+
+ describe 'when StandardError is raised' do
+ it 'reverts pending_delete attribute with a error message' do
+ allow_any_instance_of(::Projects::DestroyService).to receive(:execute).and_raise(StandardError, "some error message")
+
+ expect do
+ subject.perform(project.id, project.owner.id, {})
+ end.to change { project.reload.pending_delete }.from(true).to(false)
+
+ expect(Project.all).to include(project)
+ expect(project.delete_error).to eq("some error message")
+ end
+ end
end
end