Commit fe0ffcc7 authored by Sean McGivern's avatar Sean McGivern

Merge branch '35072-fix-pages-delete' into 'master'

Resolve "Deleting of a GitLab Pages project results in "`PagesWorker.perform_in` cannot be called inside a transaction""

Closes #35072

See merge request !13631
parents 7e13bb9a c9856e53
...@@ -60,7 +60,7 @@ class Project < ActiveRecord::Base ...@@ -60,7 +60,7 @@ class Project < ActiveRecord::Base
end end
before_destroy :remove_private_deploy_keys before_destroy :remove_private_deploy_keys
after_destroy :remove_pages after_destroy -> { run_after_commit { remove_pages } }
# update visibility_level of forks # update visibility_level of forks
after_update :update_forks_visibility_level after_update :update_forks_visibility_level
...@@ -1224,6 +1224,9 @@ class Project < ActiveRecord::Base ...@@ -1224,6 +1224,9 @@ class Project < ActiveRecord::Base
# TODO: what to do here when not using Legacy Storage? Do we still need to rename and delay removal? # TODO: what to do here when not using Legacy Storage? Do we still need to rename and delay removal?
def remove_pages def remove_pages
# Projects with a missing namespace cannot have their pages removed
return unless namespace
::Projects::UpdatePagesConfigurationService.new(self).execute ::Projects::UpdatePagesConfigurationService.new(self).execute
# 1. We rename pages to temporary directory # 1. We rename pages to temporary directory
......
...@@ -24,10 +24,6 @@ class NamespacelessProjectDestroyWorker ...@@ -24,10 +24,6 @@ class NamespacelessProjectDestroyWorker
unlink_fork(project) if project.forked? unlink_fork(project) if project.forked?
# Override Project#remove_pages for this instance so it doesn't do anything
def project.remove_pages
end
project.destroy! project.destroy!
end end
......
---
title: Fix deleting GitLab Pages files when a project is removed
merge_request: 13631
author:
type: fixed
...@@ -2311,6 +2311,44 @@ describe Project do ...@@ -2311,6 +2311,44 @@ describe Project do
end end
end end
describe '#remove_pages' do
let(:project) { create(:project) }
let(:namespace) { project.namespace }
let(:pages_path) { project.pages_path }
around do |example|
FileUtils.mkdir_p(pages_path)
begin
example.run
ensure
FileUtils.rm_rf(pages_path)
end
end
it 'removes the pages directory' do
expect_any_instance_of(Projects::UpdatePagesConfigurationService).to receive(:execute)
expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return(true)
expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, namespace.full_path, anything)
project.remove_pages
end
it 'is a no-op when there is no namespace' do
project.update_column(:namespace_id, nil)
expect_any_instance_of(Projects::UpdatePagesConfigurationService).not_to receive(:execute)
expect_any_instance_of(Gitlab::PagesTransfer).not_to receive(:rename_project)
project.remove_pages
end
it 'is run when the project is destroyed' do
expect(project).to receive(:remove_pages).and_call_original
project.destroy
end
end
describe '#forks_count' do describe '#forks_count' do
it 'returns the number of forks' do it 'returns the number of forks' do
project = build(:project) project = build(:project)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment