From 3f7555bec895f46e787c4942ad4f6feb2aa40cd7 Mon Sep 17 00:00:00 2001
From: Alexey Dudarev <skel@nebius.com>
Date: Wed, 11 Sep 2024 17:07:19 +0200
Subject: [PATCH] Fix Zoekt global code search

There is no Code tab in search navigation now.
There are no results with manual scope=blobs now.

This fix:
 - uses :zoekt_cross_namespace_search for navigation,
 - correctly overrides allowed scopes,
 - adds and updates tests.

Changelog: fixed
EE: true
---
 .../concerns/search/zoekt_searchable.rb       |  2 +-
 ee/app/services/ee/search/global_service.rb   | 11 +++--
 ee/app/services/ee/search/group_service.rb    |  3 +-
 ee/lib/ee/search/navigation.rb                | 14 ++++--
 ee/spec/lib/search/navigation_spec.rb         | 48 ++++++++++++++-----
 .../services/search/global_service_spec.rb    | 35 ++++++++++++++
 ee/spec/services/search/group_service_spec.rb | 14 +++++-
 .../services/search/project_service_spec.rb   |  2 +-
 8 files changed, 105 insertions(+), 24 deletions(-)

diff --git a/ee/app/services/concerns/search/zoekt_searchable.rb b/ee/app/services/concerns/search/zoekt_searchable.rb
index 3acb1becc86a48d2..15fb1b5ba2fb3ba8 100644
--- a/ee/app/services/concerns/search/zoekt_searchable.rb
+++ b/ee/app/services/concerns/search/zoekt_searchable.rb
@@ -17,7 +17,7 @@ def zoekt_searchable_scope
     end
 
     def zoekt_searchable_scope?
-      scope == 'blobs' && zoekt_searchable_scope.try(:search_code_with_zoekt?)
+      zoekt_searchable_scope.try(:search_code_with_zoekt?)
     end
 
     def root_ancestor
diff --git a/ee/app/services/ee/search/global_service.rb b/ee/app/services/ee/search/global_service.rb
index acf3df4734d8ec90..213973acdefac0b3 100644
--- a/ee/app/services/ee/search/global_service.rb
+++ b/ee/app/services/ee/search/global_service.rb
@@ -21,7 +21,7 @@ def elasticsearch_results
 
       override :zoekt_searchable_scope?
       def zoekt_searchable_scope?
-        scope == 'blobs' && ::Feature.enabled?(:zoekt_cross_namespace_search, current_user)
+        ::Feature.enabled?(:zoekt_cross_namespace_search, current_user)
       end
 
       override :root_ancestor
@@ -70,10 +70,13 @@ def elastic_projects
 
       override :allowed_scopes
       def allowed_scopes
-        return super unless use_elasticsearch?
-
         strong_memoize(:ee_allowed_scopes) do
-          super + %w[blobs commits epics notes wiki_blobs]
+          scopes = super
+
+          scopes += %w[blobs commits epics notes wiki_blobs] if use_elasticsearch?
+          scopes += %w[blobs] if use_zoekt?
+
+          scopes.uniq
         end
       end
     end
diff --git a/ee/app/services/ee/search/group_service.rb b/ee/app/services/ee/search/group_service.rb
index ade7615576145c4b..93f74531a837c23d 100644
--- a/ee/app/services/ee/search/group_service.rb
+++ b/ee/app/services/ee/search/group_service.rb
@@ -58,10 +58,9 @@ def allowed_scopes
             scopes -= %w[epics] unless group.licensed_feature_available?(:epics)
           else
             scopes += %w[epics] # In basic search we have epics enabled for groups
-            scopes += %w[blobs] if ::Search::Zoekt.search?(group) && ::Search::Zoekt.enabled_for_user?(current_user)
           end
 
-          scopes
+          scopes.uniq
         end
       end
     end
diff --git a/ee/lib/ee/search/navigation.rb b/ee/lib/ee/search/navigation.rb
index c1debe47348e38fe..deff54e64cc4de17 100644
--- a/ee/lib/ee/search/navigation.rb
+++ b/ee/lib/ee/search/navigation.rb
@@ -21,14 +21,22 @@ def show_code_search_tab?
         return true if super
         return false unless project.nil?
 
+        global_search_code_tab_enabled = ::Feature.enabled?(:global_search_code_tab, user, type: :ops)
+        global_search_with_zoekt_enabled = ::Feature.enabled?(:zoekt_cross_namespace_search, user, type: :ops)
+
+        zoekt_enabled_for_user = zoekt_enabled? && ::Search::Zoekt.enabled_for_user?(user)
+
         if show_elasticsearch_tabs?
           return true if group.present?
 
-          return ::Feature.enabled?(:global_search_code_tab, user, type: :ops)
+          return global_search_code_tab_enabled
+        elsif zoekt_enabled_for_user
+          return ::Search::Zoekt.search?(group) if group.present?
+
+          return global_search_code_tab_enabled && global_search_with_zoekt_enabled
         end
 
-        group.present? && zoekt_enabled? &&
-          ::Search::Zoekt.search?(group) && ::Search::Zoekt.enabled_for_user?(user)
+        false
       end
 
       override :show_wiki_search_tab?
diff --git a/ee/spec/lib/search/navigation_spec.rb b/ee/spec/lib/search/navigation_spec.rb
index 8fec789f88a6e159..b2ac27ed9b5bbcb8 100644
--- a/ee/spec/lib/search/navigation_spec.rb
+++ b/ee/spec/lib/search/navigation_spec.rb
@@ -219,23 +219,49 @@
         let(:project) { nil }
         let(:group) { nil }
 
-        where(:feature_flag, :show_elasticsearch_tabs, :zoekt_enabled, :condition) do
-          false | false | false | false
-          false | false | true | false
-          true | true | false | true
-          true | true | true | true
-          true | false | false | false
-          true | false | true | false
-          true | false | true | false
-          false | true | false | false
-          false | true | true | false
+        where(:global_search_code_tab_enabled, :global_search_with_zoekt_enabled, :show_elasticsearch_tabs,
+          :zoekt_enabled, :zoekt_enabled_for_user, :condition) do
+          false | false | false | false | false | false
+          false | false | false | false | true  | false
+          false | false | false | true  | false | false
+          false | false | false | true  | true  | false
+          false | false | true  | false | false | false
+          false | false | true  | false | true  | false
+          false | false | true  | true  | false | false
+          false | false | true  | true  | true  | false
+          false | true  | false | false | false | false
+          false | true  | false | false | true  | false
+          false | true  | false | true  | false | false
+          false | true  | false | true  | true  | false
+          false | true  | true  | false | false | false
+          false | true  | true  | false | true  | false
+          false | true  | true  | true  | false | false
+          false | true  | true  | true  | true  | false
+          true  | false | false | false | false | false
+          true  | false | false | false | true  | false
+          true  | false | false | true  | false | false
+          true  | false | false | true  | true  | false
+          true  | false | true  | false | false | true
+          true  | false | true  | false | true  | true
+          true  | false | true  | true  | false | true
+          true  | false | true  | true  | true  | true
+          true  | true  | false | false | false | false
+          true  | true  | false | false | true  | false
+          true  | true  | false | true  | false | false
+          true  | true  | false | true  | true  | true
+          true  | true  | true  | false | false | true
+          true  | true  | true  | false | true  | true
+          true  | true  | true  | true  | false | true
+          true  | true  | true  | true  | true  | true
         end
 
         with_them do
           let(:options) { { show_elasticsearch_tabs: show_elasticsearch_tabs, zoekt_enabled: zoekt_enabled } }
 
           before do
-            stub_feature_flags(global_search_code_tab: feature_flag)
+            stub_feature_flags(global_search_code_tab: global_search_code_tab_enabled)
+            stub_feature_flags(zoekt_cross_namespace_search: global_search_with_zoekt_enabled)
+            allow(::Search::Zoekt).to receive(:enabled_for_user?).and_return(zoekt_enabled_for_user)
           end
 
           it 'data item condition is set correctly' do
diff --git a/ee/spec/services/search/global_service_spec.rb b/ee/spec/services/search/global_service_spec.rb
index 9bbdb8b194675905..53d81846b82323db 100644
--- a/ee/spec/services/search/global_service_spec.rb
+++ b/ee/spec/services/search/global_service_spec.rb
@@ -422,6 +422,41 @@
         end
       end
     end
+
+    context 'for blobs scope' do
+      context 'when elasticearch_search is disabled and zoekt is disabled' do
+        before do
+          stub_ee_application_setting(elasticsearch_search: false)
+          allow(::Search::Zoekt).to receive(:enabled_for_user?).and_return(false)
+        end
+
+        it 'does not include blobs scope' do
+          expect(described_class.new(user, {}).allowed_scopes).not_to include('blobs')
+        end
+      end
+
+      context 'when elasticsearch_search is enabled and zoekt is disabled' do
+        before do
+          stub_ee_application_setting(elasticsearch_search: true)
+          allow(::Search::Zoekt).to receive(:enabled_for_user?).and_return(false)
+        end
+
+        it 'includes blobs scope' do
+          expect(described_class.new(user, {}).allowed_scopes).to include('blobs')
+        end
+      end
+
+      context 'when elasticsearch_search is disabled and zoekt is enabled' do
+        before do
+          stub_ee_application_setting(elasticsearch_search: false)
+          allow(::Search::Zoekt).to receive(:enabled_for_user?).and_return(true)
+        end
+
+        it 'includes blobs scope' do
+          expect(described_class.new(user, {}).allowed_scopes).to include('blobs')
+        end
+      end
+    end
   end
 
   describe '#elastic_projects' do
diff --git a/ee/spec/services/search/group_service_spec.rb b/ee/spec/services/search/group_service_spec.rb
index 662294c6bfe128b1..e6fb7c0f77ab3889 100644
--- a/ee/spec/services/search/group_service_spec.rb
+++ b/ee/spec/services/search/group_service_spec.rb
@@ -213,7 +213,7 @@
       let(:scope) { 'issues' }
 
       it 'does not search with Zoekt' do
-        expect(service.use_zoekt?).to eq(false)
+        expect(service.search_type).not_to eq('zoekt')
         expect(service.execute).not_to be_kind_of(::Search::Zoekt::SearchResults)
       end
     end
@@ -653,8 +653,9 @@
 
   describe '#allowed_scopes' do
     let_it_be(:group) { create(:group) }
+    let(:service) { described_class.new(user, group, {}) }
 
-    subject(:allowed_scopes) { described_class.new(user, group, {}).allowed_scopes }
+    subject(:allowed_scopes) { service.allowed_scopes }
 
     context 'for blobs scope' do
       context 'when elasticearch_search is disabled and zoekt is disabled' do
@@ -680,10 +681,19 @@
           stub_ee_application_setting(elasticsearch_search: false)
           allow(::Search::Zoekt).to receive(:enabled_for_user?).and_return(true)
           allow(::Search::Zoekt).to receive(:search?).with(group).and_return(true)
+          allow(service).to receive(:zoekt_node_available_for_search?).and_return(true)
         end
 
         it { is_expected.to include('blobs') }
 
+        context 'and zoekt node is not available' do
+          before do
+            allow(service).to receive(:zoekt_node_available_for_search?).and_return(false)
+          end
+
+          it { is_expected.not_to include('blobs') }
+        end
+
         context 'but the group does is not enabled for zoekt' do
           before do
             allow(::Search::Zoekt).to receive(:search?).with(group).and_return(false)
diff --git a/ee/spec/services/search/project_service_spec.rb b/ee/spec/services/search/project_service_spec.rb
index c572ebc69d64ffd9..383b39531448e608 100644
--- a/ee/spec/services/search/project_service_spec.rb
+++ b/ee/spec/services/search/project_service_spec.rb
@@ -167,7 +167,7 @@
       let(:scope) { 'issues' }
 
       it 'does not search with Zoekt' do
-        expect(service.use_zoekt?).to eq(false)
+        expect(service.search_type).not_to eq('zoekt')
         expect(service.execute).not_to be_kind_of(::Search::Zoekt::SearchResults)
       end
     end
-- 
GitLab