Simplify ancestor queries

app/models/namespaces/traversal/linear.rb

@@ -67,7 +67,9 @@ module Namespaces
       def ancestors(hierarchy_order: nil)
         return super() unless use_traversal_ids?
 
-        lineage(bottom: latest_parent_id, hierarchy_order: hierarchy_order)
+        return self.class.none if parent_id.blank?
+
+        lineage(bottom: parent, hierarchy_order: hierarchy_order)
       end
 
       private
@@ -99,14 +101,14 @@ module Namespaces
         end
 
         if bottom
-          skope = skope.traversal_ids_contained_by(latest_traversal_ids(bottom))
+          skope = skope.traversal_ids_contained_by(sql_array(bottom.traversal_ids))
         end
 
         # The original `with_depth` attribute in ObjectHierarchy increments as you
         # walk away from the "base" namespace. This direction changes depending on
         # if you are walking up the ancestors or down the descendants.
         if hierarchy_order
-          depth_sql = "ABS(array_length((#{latest_traversal_ids.to_sql}), 1) - array_length(traversal_ids, 1))"
+          depth_sql = "ABS(#{traversal_ids.count} - array_length(traversal_ids, 1))"
           skope = skope.select(skope.arel_table[Arel.star], "#{depth_sql} as depth")
                        .order(depth: hierarchy_order)
         end
@@ -114,26 +116,8 @@ module Namespaces
         skope
       end
 
-      # traversal_ids are a cached value.
-      #
-      # The traversal_ids value in a loaded object can become stale when compared
-      # to the database value. For example, if you load a hierarchy and then move
-      # a group, any previously loaded descendant objects will have out of date
-      # traversal_ids.
-      #
-      # To solve this problem, we never depend on the object's traversal_ids
-      # value. We always query the database first with a sub-select for the
-      # latest traversal_ids.
-      #
-      # Note that ActiveRecord will cache query results. You can avoid this by
-      # using `Model.uncached { ... }`
-      def latest_traversal_ids(namespace = self)
-        without_sti_condition.where('id = (?)', namespace)
-                .select('traversal_ids as latest_traversal_ids')
-      end
-
-      def latest_parent_id(namespace = self)
-        without_sti_condition.where(id: namespace).select(:parent_id)
+      def sql_array(ids)
+        "{#{ids.join(',')}}"
       end
     end
   end

spec/models/group_spec.rb

@@ -427,6 +427,42 @@ RSpec.describe Group do
     end
   end
 
+  context 'traversal queries' do
+    let_it_be(:group, reload: true) { create(:group, :nested) }
+
+    context 'recursive' do
+      before do
+        stub_feature_flags(use_traversal_ids: false)
+      end
+
+      it_behaves_like 'namespace traversal'
+
+      describe '#self_and_descendants' do
+        it { expect(group.self_and_descendants.to_sql).not_to include 'traversal_ids @>' }
+      end
+
+      describe '#ancestors' do
+        it { expect(group.ancestors.to_sql).not_to include 'traversal_ids <@' }
+      end
+    end
+
+    context 'linear' do
+      it_behaves_like 'namespace traversal'
+
+      describe '#self_and_descendants' do
+        it { expect(group.self_and_descendants.to_sql).to include 'traversal_ids @>' }
+      end
+
+      describe '#ancestors' do
+        it { expect(group.ancestors.to_sql).to include 'traversal_ids <@' }
+      end
+
+      it 'hierarchy order' do
+        expect(group.ancestors(hierarchy_order: :asc).to_sql).to include 'ORDER BY "depth" ASC'
+      end
+    end
+  end
+
   describe '.without_integration' do
     let(:another_group) { create(:group) }
     let(:instance_integration) { build(:jira_service, :instance) }

spec/models/namespace_spec.rb

@@ -936,42 +936,6 @@ RSpec.describe Namespace do
     end
   end
 
-  context 'traversal queries' do
-    let_it_be(:namespace, reload: true) { create(:namespace) }
-
-    context 'recursive' do
-      before do
-        stub_feature_flags(use_traversal_ids: false)
-      end
-
-      it_behaves_like 'namespace traversal'
-
-      describe '#self_and_descendants' do
-        it { expect(namespace.self_and_descendants.to_sql).not_to include 'traversal_ids @>' }
-      end
-
-      describe '#ancestors' do
-        it { expect(namespace.ancestors.to_sql).not_to include 'traversal_ids <@' }
-      end
-    end
-
-    context 'linear' do
-      it_behaves_like 'namespace traversal'
-
-      describe '#self_and_descendants' do
-        it { expect(namespace.self_and_descendants.to_sql).to include 'traversal_ids @>' }
-      end
-
-      describe '#ancestors' do
-        it { expect(namespace.ancestors.to_sql).to include 'traversal_ids <@' }
-
-        it 'hierarchy order' do
-          expect(namespace.ancestors(hierarchy_order: :asc).to_sql).to include 'ORDER BY "depth" ASC'
-        end
-      end
-    end
-  end
-
   describe '#users_with_descendants' do
     let(:user_a) { create(:user) }
     let(:user_b) { create(:user) }

spec/services/groups/transfer_service_spec.rb

@@ -240,6 +240,7 @@ RSpec.describe Groups::TransferService do
       end
 
       context 'when the group is allowed to be transferred' do
+        let_it_be(:new_parent_group, reload: true) { create(:group, :public) }
         let_it_be(:new_parent_group_integration) { create(:slack_service, group: new_parent_group, project: nil, webhook: 'http://new-group.slack.com') }
 
         before do
@@ -273,11 +274,10 @@ RSpec.describe Groups::TransferService do
         end
 
         context 'with a group integration' do
-          let_it_be(:instance_integration) { create(:slack_service, :instance, webhook: 'http://project.slack.com') }
-
           let(:new_created_integration) { Service.find_by(group: group) }
 
           context 'with an inherited integration' do
+            let_it_be(:instance_integration) { create(:slack_service, :instance, webhook: 'http://project.slack.com') }
             let_it_be(:group_integration) { create(:slack_service, group: group, project: nil, webhook: 'http://group.slack.com', inherit_from_id: instance_integration.id) }
 
             it 'replaces inherited integrations', :aggregate_failures do
@@ -603,6 +603,7 @@ RSpec.describe Groups::TransferService do
           create(:group_member, :owner, group: new_parent_group, user: user)
           create(:group, :private, parent: group, require_two_factor_authentication: true)
           group.update!(require_two_factor_authentication: true)
+          new_parent_group.reload # make sure traversal_ids are reloaded
         end
 
         it 'does not update group two factor authentication setting' do