From 5d57030e46bffcf21f1ea763d031f38330506061 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 4 Jan 2018 14:16:13 -0800 Subject: [PATCH] Delete conflicting orphaned routes --- app/models/route.rb | 12 ++++ ...lete-orphaned-routes-before-validation.yml | 6 ++ spec/models/route_spec.rb | 61 +++++++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 changelogs/unreleased/mk-delete-orphaned-routes-before-validation.yml diff --git a/app/models/route.rb b/app/models/route.rb index 412f5fb45a5..3d4b5a8b5ee 100644 --- a/app/models/route.rb +++ b/app/models/route.rb @@ -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 diff --git a/changelogs/unreleased/mk-delete-orphaned-routes-before-validation.yml b/changelogs/unreleased/mk-delete-orphaned-routes-before-validation.yml new file mode 100644 index 00000000000..55ab318df7d --- /dev/null +++ b/changelogs/unreleased/mk-delete-orphaned-routes-before-validation.yml @@ -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 diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index 8a3b1034f3c..88f54fd10e5 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -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