Merge branch 'bring-back-advanced-global-search-default' into 'master'

Enable advanced search by default for global search scope

See merge request gitlab-org/gitlab!41041

app/controllers/search_controller.rb

@@ -15,6 +15,7 @@ class SearchController < ApplicationController
 
   around_action :allow_gitaly_ref_name_caching
 
+  before_action :block_anonymous_global_searches
   skip_before_action :authenticate_user!
   requires_cross_project_access if: -> do
     search_term_present = params[:search].present? || params[:term].present?
@@ -128,6 +129,16 @@ class SearchController < ApplicationController
     payload[:metadata]['meta.search.search'] = params[:search]
     payload[:metadata]['meta.search.scope'] = params[:scope]
   end
+
+  def block_anonymous_global_searches
+    return if params[:project_id].present? || params[:group_id].present?
+    return if current_user
+    return unless ::Feature.enabled?(:block_anonymous_global_searches)
+
+    store_location_for(:user, request.fullpath)
+
+    redirect_to new_user_session_path, alert: _('You must be logged in to search across all of GitLab')
+  end
 end
 
 SearchController.prepend_if_ee('EE::SearchController')

config/feature_flags/development/block_anonymous_global_searches.yml

+---
+name: block_anonymous_global_searches
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41041
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/244276
+group: group::global search
+type: development
+default_enabled: false
\ No newline at end of file

ee/app/models/ee/application_setting.rb

@@ -244,7 +244,7 @@ module EE
       when Project
         elasticsearch_indexes_project?(scope)
       else
-        false # Never use elasticsearch for the global scope when limiting is on
+        ::Feature.enabled?(:advanced_global_search_for_limited_indexing)
       end
     end
 

ee/config/feature_flags/development/advanced_global_search_for_limited_indexing.yml

+---
+name: advanced_global_search_for_limited_indexing
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41041
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/244276
+group: group::global search
+type: development
+default_enabled: false
\ No newline at end of file

ee/spec/features/search/elastic/global_search_spec.rb

@@ -266,7 +266,19 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
 end
 
 RSpec.describe 'Global elastic search redactions', :elastic do
+  context 'when block_anonymous_global_searches is disabled' do
+    before do
+      stub_feature_flags(block_anonymous_global_searches: false)
+    end
+
     it_behaves_like 'a redacted search results page' do
       let(:search_path) { explore_root_path }
     end
+  end
+
+  context 'when block_anonymous_global_searches is enabled' do
+    it_behaves_like 'a redacted search results page', include_anonymous: false do
+      let(:search_path) { explore_root_path }
+    end
+  end
 end

ee/spec/features/search/elastic/snippet_search_spec.rb

@@ -37,6 +37,17 @@ RSpec.describe 'Snippet elastic search', :js, :elastic, :aggregate_failures, :si
     context 'as anonymous user' do
       let(:current_user) { nil }
 
+      context 'when block_anonymous_global_searches is enabled' do
+        it 'redirects to login page' do
+          expect(page).to have_content('You must be logged in to search across all of GitLab')
+        end
+      end
+
+      context 'when block_anonymous_global_searches is disabled' do
+        before(:context) do
+          stub_feature_flags(block_anonymous_global_searches: false)
+        end
+
         it 'finds only public snippets' do
           within('.results') do
             expect(page).to have_content('public personal snippet')
@@ -53,6 +64,7 @@ RSpec.describe 'Snippet elastic search', :js, :elastic, :aggregate_failures, :si
           end
         end
       end
+    end
 
     context 'as logged in user' do
       let(:current_user) { create(:user) }

ee/spec/models/application_setting_spec.rb

@@ -407,8 +407,8 @@ RSpec.describe ApplicationSetting do
   end
 
   describe '#search_using_elasticsearch?' do
-    # Constructs a truth table with 16 entries to run the specs against
-    where(indexing: [true, false], searching: [true, false], limiting: [true, false])
+    # Constructs a truth table to run the specs against
+    where(indexing: [true, false], searching: [true, false], limiting: [true, false], advanced_global_search_for_limited_indexing: [true, false])
 
     with_them do
       let_it_be(:included_project_container) { create(:elasticsearch_indexed_project) }
@@ -430,12 +430,14 @@ RSpec.describe ApplicationSetting do
           elasticsearch_search: searching,
           elasticsearch_limit_indexing: limiting
         )
+
+        stub_feature_flags(advanced_global_search_for_limited_indexing: advanced_global_search_for_limited_indexing)
       end
 
       context 'global scope' do
         let(:scope) { nil }
 
-        it { is_expected.to eq(only_when_enabled_globally) }
+        it { is_expected.to eq(indexing && searching && (!limiting || advanced_global_search_for_limited_indexing)) }
       end
 
       context 'namespace (in scope)' do

ee/spec/requests/api/search_spec.rb

@@ -61,10 +61,7 @@ RSpec.describe API::Search do
   shared_examples 'elasticsearch enabled' do |level:|
     context 'for merge_requests scope', :sidekiq_inline do
       before do
-        create(:labeled_merge_request, target_branch: 'feature_1', source_project: project, labels: [create(:label), create(:label)])
-        create(:merge_request, target_branch: 'feature_2', source_project: project, author: create(:user))
-        create(:merge_request, target_branch: 'feature_3', source_project: project, milestone: create(:milestone, project: project))
-        create(:merge_request, target_branch: 'feature_4', source_project: project)
+        create_list(:merge_request, 3, :unique_branches, source_project: project, author: create(:user), milestone: create(:milestone, project: project), labels: [create(:label)])
         ensure_elasticsearch_index!
       end
 
@@ -72,19 +69,14 @@ RSpec.describe API::Search do
 
       it 'avoids N+1 queries' do
         control = ActiveRecord::QueryRecorder.new { get api(endpoint, user), params: { scope: 'merge_requests', search: '*' } }
-
-        create(:labeled_merge_request, target_branch: 'feature_5', source_project: project, labels: [create(:label), create(:label)])
-        create(:merge_request, target_branch: 'feature_6', source_project: project, author: create(:user))
-        create(:merge_request, target_branch: 'feature_7', source_project: project, milestone: create(:milestone, project: project))
-        create(:merge_request, target_branch: 'feature_8', source_project: project)
-
+        create_list(:merge_request, 3, :unique_branches, source_project: project, author: create(:user), milestone: create(:milestone, project: project), labels: [create(:label)])
         ensure_elasticsearch_index!
 
         expect { get api(endpoint, user), params: { scope: 'merge_requests', search: '*' } }.not_to exceed_query_limit(control)
       end
     end
 
-    context 'for wiki_blobs scope', :sidekiq_might_not_need_inline do
+    context 'for wiki_blobs scope', :sidekiq_inline do
       before do
         wiki = create(:project_wiki, project: project)
         create(:wiki_page, wiki: wiki, title: 'home', content: "Awesome page")
@@ -101,11 +93,14 @@ RSpec.describe API::Search do
       it_behaves_like 'pagination', scope: 'wiki_blobs'
     end
 
-    context 'for commits scope', :sidekiq_inline do
+    context 'for commits and blobs', :sidekiq_inline do
       before do
         project.repository.index_commits_and_blobs
         ensure_elasticsearch_index!
+      end
 
+      context 'for commits scope' do
+        before do
           get api(endpoint, user), params: { scope: 'commits', search: 'folder' }
         end
 
@@ -128,11 +123,8 @@ RSpec.describe API::Search do
         end
       end
 
-    context 'for blobs scope', :sidekiq_might_not_need_inline do
+      context 'for blobs scope' do
         before do
-        project.repository.index_commits_and_blobs
-        ensure_elasticsearch_index!
-
           get api(endpoint, user), params: { scope: 'blobs', search: 'monitors' }
         end
 
@@ -159,7 +151,7 @@ RSpec.describe API::Search do
             end
 
             it 'by path' do
-            get api("/projects/#{project.id}/search", user), params: { scope: 'blobs', search: 'mon* path:files/markdown' }
+              get api("/projects/#{project.id}/search", user), params: { scope: 'blobs', search: 'mon* path:markdown' }
 
               expect(response).to have_gitlab_http_status(:ok)
               expect(json_response.size).to eq(1)
@@ -171,7 +163,6 @@ RSpec.describe API::Search do
 
               expect(response).to have_gitlab_http_status(:ok)
               expect(json_response.size).to eq(3)
-
               expect(results_filenames).to all(match(%r{.*.md$}))
             end
           end
@@ -204,6 +195,7 @@ RSpec.describe API::Search do
           end
         end
       end
+    end
 
     context 'for issues scope', :sidekiq_inline do
       before do
@@ -270,7 +262,7 @@ RSpec.describe API::Search do
       end
     end
 
-    context 'for users scope', :sidekiq_inline do
+    context 'for users scope', :sidekiq_might_not_need_inline do
       before do
         create_list(:user, 2).each do |user|
           project.add_developer(user)
@@ -326,7 +318,13 @@ RSpec.describe API::Search do
             stub_ee_application_setting(elasticsearch_limit_indexing: true)
           end
 
-          it_behaves_like 'elasticsearch disabled'
+          context 'and namespace is indexed' do
+            before do
+              create :elasticsearch_indexed_namespace, namespace: group
+            end
+
+            it_behaves_like 'elasticsearch enabled', level: :global
+          end
         end
 
         context 'when elasticsearch_limit_indexing off' do

ee/spec/services/search/global_service_spec.rb

@@ -177,15 +177,37 @@ RSpec.describe Search::GlobalService do
       end
     end
 
-    context 'when ES is not used' do
+    context 'when elasticearch_search is disabled' do
+      before do
+        stub_ee_application_setting(elasticsearch_search: false)
+      end
+
+      it 'does not include ES-specific scopes' do
+        expect(described_class.new(user, {}).allowed_scopes).not_to include('commits')
+      end
+    end
+
+    context 'when elasticsearch_limit_indexing is enabled' do
       before do
         stub_ee_application_setting(elasticsearch_limit_indexing: true)
       end
 
+      context 'when advanced_global_search_for_limited_indexing feature flag is disabled' do
+        before do
+          stub_feature_flags(advanced_global_search_for_limited_indexing: false)
+        end
+
         it 'does not include ES-specific scopes' do
           expect(described_class.new(user, {}).allowed_scopes).not_to include('commits')
         end
       end
+
+      context 'when advanced_global_search_for_limited_indexing feature flag is enabled' do
+        it 'includes ES-specific scopes' do
+          expect(described_class.new(user, {}).allowed_scopes).to include('commits')
+        end
+      end
+    end
   end
 
   context 'confidential notes' do

ee/spec/support/features/redacted_search_results_examples.rb

 # frozen_string_literal: true
 
-RSpec.shared_examples 'a redacted search results page' do
+RSpec.shared_examples 'a redacted search results page' do |include_anonymous: true|
   let(:public_group) { create(:group, :public) }
   let(:public_restricted_project) { create(:project, :repository, :public, :wiki_repo, namespace: public_group, name: 'The Project') }
   let(:issue_access_level) { ProjectFeature::PRIVATE }
@@ -41,7 +41,7 @@ RSpec.shared_examples 'a redacted search results page' do
   end
 
   it_behaves_like 'redacted search results page assertions', true
-  it_behaves_like 'redacted search results page assertions', false
+  it_behaves_like 'redacted search results page assertions', false if include_anonymous
 end
 
 # Only intended to be used in the above shared examples to avoid duplication of

locale/gitlab.pot

@@ -28955,6 +28955,9 @@ msgstr ""
 msgid "You must accept our Terms of Service and privacy policy in order to register an account"
 msgstr ""
 
+msgid "You must be logged in to search across all of GitLab"
+msgstr ""
+
 msgid "You must disassociate %{domain} from all clusters it is attached to before deletion."
 msgstr ""
 

spec/controllers/search_controller_spec.rb

@@ -95,6 +95,11 @@ RSpec.describe SearchController do
       using RSpec::Parameterized::TableSyntax
       render_views
 
+      context 'when block_anonymous_global_searches is disabled' do
+        before do
+          stub_feature_flags(block_anonymous_global_searches: false)
+        end
+
         it 'omits pipeline status from load' do
           project = create(:project, :public)
           expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects)
@@ -140,6 +145,29 @@ RSpec.describe SearchController do
         end
       end
 
+      context 'when block_anonymous_global_searches is enabled' do
+        context 'for unauthenticated user' do
+          before do
+            sign_out(user)
+          end
+
+          it 'redirects to login page' do
+            get :show, params: { scope: 'projects', search: '*' }
+
+            expect(response).to redirect_to new_user_session_path
+          end
+        end
+
+        context 'for authenticated user' do
+          it 'succeeds' do
+            get :show, params: { scope: 'projects', search: '*' }
+
+            expect(response).to have_gitlab_http_status(:ok)
+          end
+        end
+      end
+    end
+
     it 'finds issue comments' do
       project = create(:project, :public)
       note = create(:note_on_issue, project: project)

spec/features/search/user_searches_for_issues_spec.rb

@@ -86,9 +86,11 @@ RSpec.describe 'User searches for issues', :js do
   end
 
   context 'when signed out' do
+    context 'when block_anonymous_global_searches is disabled' do
       let(:project) { create(:project, :public) }
 
       before do
+        stub_feature_flags(block_anonymous_global_searches: false)
         visit(search_path)
       end
 
@@ -103,4 +105,15 @@ RSpec.describe 'User searches for issues', :js do
         end
       end
     end
+
+    context 'when block_anonymous_global_searches is enabled' do
+      before do
+        visit(search_path)
+      end
+
+      it 'is redirected to login page' do
+        expect(page).to have_content('You must be logged in to search across all of GitLab')
+      end
+    end
+  end
 end

spec/features/search/user_searches_for_projects_spec.rb

@@ -6,6 +6,11 @@ RSpec.describe 'User searches for projects' do
   let!(:project) { create(:project, :public, name: 'Shop') }
 
   context 'when signed out' do
+    context 'when block_anonymous_global_searches is disabled' do
+      before do
+        stub_feature_flags(block_anonymous_global_searches: false)
+      end
+
       include_examples 'top right search form'
 
       it 'finds a project' do
@@ -33,4 +38,12 @@ RSpec.describe 'User searches for projects' do
         expect(find('#project_id', visible: false).value).to eq(project.id.to_s)
       end
     end
+
+    context 'when block_anonymous_global_searches is enabled' do
+      it 'is redirected to login page' do
+        visit(search_path)
+        expect(page).to have_content('You must be logged in to search across all of GitLab')
+      end
+    end
+  end
 end

spec/frontend/fixtures/search.rb

@@ -7,10 +7,16 @@ RSpec.describe SearchController, '(JavaScript fixtures)', type: :controller do
 
   render_views
 
+  let_it_be(:user) { create(:admin) }
+
   before(:all) do
     clean_frontend_fixtures('search/')
   end
 
+  before do
+    sign_in(user)
+  end
+
   it 'search/show.html' do
     get :show