From a0368e91310a3b2c0e9e0b717f931a482eb0b90a Mon Sep 17 00:00:00 2001
From: Michael Kozono <mkozono@gmail.com>
Date: Mon, 1 May 2017 16:48:05 -0700
Subject: [PATCH] Create redirect routes on path change
---
app/models/route.rb | 24 +++++++++++++++++-------
spec/models/route_spec.rb | 34 +++++++++++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 8 deletions(-)
diff --git a/app/models/route.rb b/app/models/route.rb
index 4b3efab5c3c..df801714b5f 100644
--- a/app/models/route.rb
+++ b/app/models/route.rb
@@ -8,15 +8,17 @@ class Route < ActiveRecord::Base
presence: true,
uniqueness: { case_sensitive: false }
- after_update :rename_descendants
+ after_update :create_redirect_if_path_changed
+ after_update :rename_direct_descendant_routes
scope :inside_path, -> (path) { where('routes.path LIKE ?', "#{sanitize_sql_like(path)}/%") }
+ scope :direct_descendant_routes, -> (path) { where('routes.path LIKE ? AND routes.path NOT LIKE ?', "#{sanitize_sql_like(path)}/%", "#{sanitize_sql_like(path)}/%/%") }
- def rename_descendants
+ def rename_direct_descendant_routes
if path_changed? || name_changed?
- descendants = self.class.inside_path(path_was)
+ direct_descendant_routes = self.class.direct_descendant_routes(path_was)
- descendants.each do |route|
+ direct_descendant_routes.each do |route|
attributes = {}
if path_changed? && route.path.present?
@@ -27,10 +29,18 @@ class Route < ActiveRecord::Base
attributes[:name] = route.name.sub(name_was, name)
end
- # Note that update_columns skips validation and callbacks.
- # We need this to avoid recursive call of rename_descendants method
- route.update_columns(attributes) unless attributes.empty?
+ route.update(attributes) unless attributes.empty?
end
end
end
+
+ def create_redirect_if_path_changed
+ if path_changed?
+ create_redirect(path_was)
+ end
+ end
+
+ def create_redirect(old_path)
+ source.redirect_routes.create(path: old_path)
+ end
end
diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb
index 171a51fcc5b..c95043469a8 100644
--- a/spec/models/route_spec.rb
+++ b/spec/models/route_spec.rb
@@ -26,7 +26,20 @@ describe Route, models: true do
end
end
- describe '#rename_descendants' do
+ describe '.direct_descendant_routes' do
+ let!(:nested_group) { create(:group, path: 'test', name: 'test', parent: group) }
+ let!(:deep_nested_group) { create(:group, path: 'foo', name: 'foo', parent: nested_group) }
+ let!(:another_group) { create(:group, path: 'other') }
+ let!(:similar_group) { create(:group, path: 'gitllab') }
+ let!(:another_group_nested) { create(:group, path: 'another', name: 'another', parent: similar_group) }
+
+ it 'returns correct routes' do
+ expect(Route.direct_descendant_routes('git_lab')).to match_array([nested_group.route])
+ expect(Route.direct_descendant_routes('git_lab/test')).to match_array([deep_nested_group.route])
+ end
+ end
+
+ describe '#rename_direct_descendant_routes' do
let!(:nested_group) { create(:group, path: 'test', name: 'test', parent: group) }
let!(:deep_nested_group) { create(:group, path: 'foo', name: 'foo', parent: nested_group) }
let!(:similar_group) { create(:group, path: 'gitlab-org', name: 'gitlab-org') }
@@ -77,4 +90,23 @@ describe Route, models: true do
end
end
end
+
+ describe '#create_redirect_if_path_changed' do
+ context 'if the path changed' do
+ it 'creates a RedirectRoute for the old path' do
+ route.path = 'new-path'
+ route.save!
+ redirect = route.source.redirect_routes.first
+ expect(redirect.path).to eq('git_lab')
+ end
+ end
+
+ context 'if the path did not change' do
+ it 'does not create a RedirectRoute' do
+ route.updated_at = Time.zone.now.utc
+ route.save!
+ expect(route.source.redirect_routes).to be_empty
+ end
+ end
+ end
end
--
GitLab