Delete conflicting orphaned routes
This commit is contained in:
parent
792e9ed7fa
commit
5d57030e46
3 changed files with 79 additions and 0 deletions
|
@ -1,4 +1,6 @@
|
||||||
class Route < ActiveRecord::Base
|
class Route < ActiveRecord::Base
|
||||||
|
include CaseSensitivity
|
||||||
|
|
||||||
belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
|
belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
|
||||||
|
|
||||||
validates :source, presence: true
|
validates :source, presence: true
|
||||||
|
@ -10,6 +12,7 @@ class Route < ActiveRecord::Base
|
||||||
|
|
||||||
validate :ensure_permanent_paths, if: :path_changed?
|
validate :ensure_permanent_paths, if: :path_changed?
|
||||||
|
|
||||||
|
before_validation :delete_conflicting_orphaned_routes
|
||||||
after_create :delete_conflicting_redirects
|
after_create :delete_conflicting_redirects
|
||||||
after_update :delete_conflicting_redirects, if: :path_changed?
|
after_update :delete_conflicting_redirects, if: :path_changed?
|
||||||
after_update :create_redirect_for_old_path
|
after_update :create_redirect_for_old_path
|
||||||
|
@ -78,4 +81,13 @@ class Route < ActiveRecord::Base
|
||||||
def conflicting_redirect_exists?
|
def conflicting_redirect_exists?
|
||||||
RedirectRoute.permanent.matching_path_and_descendants(path).exists?
|
RedirectRoute.permanent.matching_path_and_descendants(path).exists?
|
||||||
end
|
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
|
end
|
||||||
|
|
|
@ -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
|
|
@ -79,6 +79,13 @@ describe Route do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'callbacks' do
|
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
|
context 'after update' do
|
||||||
it 'calls #create_redirect_for_old_path' do
|
it 'calls #create_redirect_for_old_path' do
|
||||||
expect(route).to receive(:create_redirect_for_old_path)
|
expect(route).to receive(:create_redirect_for_old_path)
|
||||||
|
@ -378,4 +385,58 @@ describe Route do
|
||||||
end
|
end
|
||||||
end
|
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
|
end
|
||||||
|
|
Loading…
Reference in a new issue