diff --git a/app/models/route.rb b/app/models/route.rb index b34cce9077ac1e1b3911406d82116bb39f009b78..12a7fa3d01bfb859c144879eff2cc7d1585b81e3 100644 --- a/app/models/route.rb +++ b/app/models/route.rb @@ -8,19 +8,19 @@ class Route < ActiveRecord::Base presence: true, uniqueness: { case_sensitive: false } - after_save :delete_conflicting_redirects + after_create :delete_conflicting_redirects + after_update :delete_conflicting_redirects, if: :path_changed? after_update :create_redirect_for_old_path - after_update :rename_direct_descendant_routes + after_update :rename_descendants 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_direct_descendant_routes + def rename_descendants return unless path_changed? || name_changed? - direct_descendant_routes = self.class.direct_descendant_routes(path_was) + descendant_routes = self.class.inside_path(path_was) - direct_descendant_routes.each do |route| + descendant_routes.each do |route| attributes = {} if path_changed? && route.path.present? @@ -31,7 +31,17 @@ class Route < ActiveRecord::Base attributes[:name] = route.name.sub(name_was, name) end - route.update(attributes) unless attributes.empty? + if attributes.present? + old_path = route.path + + # Callbacks must be run manually + route.update_columns(attributes) + + # We are not calling route.delete_conflicting_redirects here, in hopes + # of avoiding deadlocks. The parent (self, in this method) already + # called it, which deletes conflicts for all descendants. + route.create_redirect(old_path) if attributes[:path] + end end end diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index 1aeddcef982b3129cfbf5d5e8a666ca2b390fdfb..c1fe1b06c52b8b7847a77cf941c5e5be00a53ee7 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -50,20 +50,7 @@ describe Route, models: true do end end - 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 + describe '#rename_descendants' 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') }