Merge branch 'dz-fix-route-rename-descendants' into 'master'

Fix route rename descendants if route.name is blank

See merge request !9074

2 files changed, 28 insertions, 11 deletions

diff --git a/app/models/route.rb b/app/models/route.rb
index 4eec3e73d5b..73574a6206b 100644
--- a/app/models/route.rb
+++ b/app/models/route.rb
@@ -15,14 +15,19 @@ class Route < ActiveRecord::Base
       descendants = Route.where('path LIKE ?', "#{path_was}/%")
 
       descendants.each do |route|
-        attributes = {
-          path: route.path.sub(path_was, path),
-          name: route.name.sub(name_was, name)
-        }
+        attributes = {}
+
+        if path_changed? && route.path.present?
+          attributes[:path] = route.path.sub(path_was, path)
+        end
+
+        if name_changed? && route.name.present?
+          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)
+        route.update_columns(attributes) unless attributes.empty?
       end
     end
   end
diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb
index a276aa4b13a..0b222022e62 100644
--- a/spec/models/route_spec.rb
+++ b/spec/models/route_spec.rb
@@ -20,13 +20,25 @@ describe Route, models: true do
     let!(:similar_group) { create(:group, path: 'gitlab-org', name: 'gitlab-org') }
 
     context 'path update' do
-      before { route.update_attributes(path: 'bar') }
+      context 'when route name is set' do
+        before { route.update_attributes(path: 'bar') }
+
+        it "updates children routes with new path" do
+          expect(described_class.exists?(path: 'bar')).to be_truthy
+          expect(described_class.exists?(path: 'bar/test')).to be_truthy
+          expect(described_class.exists?(path: 'bar/test/foo')).to be_truthy
+          expect(described_class.exists?(path: 'gitlab-org')).to be_truthy
+        end
+      end
 
-      it "updates children routes with new path" do
-        expect(described_class.exists?(path: 'bar')).to be_truthy
-        expect(described_class.exists?(path: 'bar/test')).to be_truthy
-        expect(described_class.exists?(path: 'bar/test/foo')).to be_truthy
-        expect(described_class.exists?(path: 'gitlab-org')).to be_truthy
+      context 'when route name is nil' do
+        before do
+          route.update_column(:name, nil)
+        end
+
+        it "does not fail" do
+          expect(route.update_attributes(path: 'bar')).to be_truthy
+        end
       end
     end