Merge branch 'mk-delete-orphaned-routes-before-validation' into 'master'

Delete conflicting orphaned routes before validation

Closes #39551

See merge request gitlab-org/gitlab-ce!16242
This commit is contained in:
Stan Hu 2018-01-18 19:16:54 +00:00
commit 56958ecdac
3 changed files with 79 additions and 0 deletions

View File

@ -1,4 +1,6 @@
class Route < ActiveRecord::Base
include CaseSensitivity
belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
validates :source, presence: true
@ -10,6 +12,7 @@ class Route < ActiveRecord::Base
validate :ensure_permanent_paths, if: :path_changed?
before_validation :delete_conflicting_orphaned_routes
after_create :delete_conflicting_redirects
after_update :delete_conflicting_redirects, if: :path_changed?
after_update :create_redirect_for_old_path
@ -78,4 +81,13 @@ class Route < ActiveRecord::Base
def conflicting_redirect_exists?
RedirectRoute.permanent.matching_path_and_descendants(path).exists?
end
def delete_conflicting_orphaned_routes
conflicting = self.class.iwhere(path: path)
conflicting_orphaned_routes = conflicting.select do |route|
route.source.nil?
end
conflicting_orphaned_routes.each(&:destroy)
end
end

View File

@ -0,0 +1,6 @@
---
title: Ensure that users can reclaim a namespace or project path that is blocked by
an orphaned route
merge_request: 16242
author:
type: fixed

View File

@ -79,6 +79,13 @@ describe Route do
end
describe 'callbacks' do
context 'before validation' do
it 'calls #delete_conflicting_orphaned_routes' do
expect(route).to receive(:delete_conflicting_orphaned_routes)
route.valid?
end
end
context 'after update' do
it 'calls #create_redirect_for_old_path' do
expect(route).to receive(:create_redirect_for_old_path)
@ -378,4 +385,58 @@ describe Route do
end
end
end
describe '#delete_conflicting_orphaned_routes' do
context 'when there is a conflicting route' do
let!(:conflicting_group) { create(:group, path: 'foo') }
before do
route.path = conflicting_group.route.path
end
context 'when the route is orphaned' do
let!(:offending_route) { conflicting_group.route }
before do
Group.delete(conflicting_group) # Orphan the route
end
it 'deletes the orphaned route' do
expect do
route.valid?
end.to change { described_class.count }.from(2).to(1)
end
it 'passes validation, as usual' do
expect(route.valid?).to be_truthy
end
end
context 'when the route is not orphaned' do
it 'does not delete the conflicting route' do
expect do
route.valid?
end.not_to change { described_class.count }
end
it 'fails validation, as usual' do
expect(route.valid?).to be_falsey
end
end
end
context 'when there are no conflicting routes' do
it 'does not delete any routes' do
route
expect do
route.valid?
end.not_to change { described_class.count }
end
it 'passes validation, as usual' do
expect(route.valid?).to be_truthy
end
end
end
end