summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Kozono <mkozono@gmail.com>2018-01-04 14:16:13 -0800
committerMichael Kozono <mkozono@gmail.com>2018-01-04 22:29:23 -0800
commitdd180232a8e56d59d81228203057a87a9c0dbb11 (patch)
tree9310ca97839f0bddc90b801454ceed4b687c379d
parentf7cbfe1d9133f1641b831695d646bc9b85de65bb (diff)
downloadgitlab-ce-mk-delete-orphaned-routes-before-validation.tar.gz
Delete conflicting orphaned routesmk-delete-orphaned-routes-before-validation
-rw-r--r--app/models/route.rb12
-rw-r--r--changelogs/unreleased/mk-delete-orphaned-routes-before-validation.yml6
-rw-r--r--spec/models/route_spec.rb61
3 files changed, 79 insertions, 0 deletions
diff --git a/app/models/route.rb b/app/models/route.rb
index 7ba3ec06041..46346df5943 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
+ 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 ddad6862a63..da6238e3899 100644
--- a/spec/models/route_spec.rb
+++ b/spec/models/route_spec.rb
@@ -19,6 +19,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)
@@ -318,4 +325,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