From 56388dc9c245b320499b43722e225a3130b08144 Mon Sep 17 00:00:00 2001
From: Terri Chu <tchu@gitlab.com>
Date: Thu, 13 Jul 2023 23:34:48 +0000
Subject: [PATCH] Fix search projects api requests missing scope permission
Return an empty array if the user does not have
permission to view the scope requested. Also a refactor
to move scope permission check and search navigation
into a library so it can be shared across the code
Changelog: fixed
---
app/helpers/projects_helper.rb | 47 ---
app/helpers/search_helper.rb | 70 +---
app/services/search/project_service.rb | 23 +-
app/services/search_service.rb | 10 -
ee/app/helpers/ee/search_helper.rb | 12 +-
ee/app/services/ee/search_service.rb | 1 -
ee/lib/ee/search/navigation.rb | 20 +
.../search/elastic/project_search_spec.rb | 11 +-
ee/spec/helpers/search_helper_spec.rb | 71 +---
ee/spec/lib/search/navigation_spec.rb | 41 ++
lib/api/search.rb | 10 +-
lib/search/navigation.rb | 158 ++++++++
.../features/snippets/search_snippets_spec.rb | 2 +-
spec/helpers/search_helper_spec.rb | 363 ++----------------
spec/lib/search/navigation_spec.rb | 280 ++++++++++++++
spec/requests/api/search_spec.rb | 10 +
16 files changed, 586 insertions(+), 543 deletions(-)
create mode 100644 ee/lib/ee/search/navigation.rb
create mode 100644 ee/spec/lib/search/navigation_spec.rb
create mode 100644 lib/search/navigation.rb
create mode 100644 spec/lib/search/navigation_spec.rb
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index b7f15118b8d70..a7ea26b264b9a 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -172,18 +172,6 @@ def visible_fork_source(project)
project.fork_source if project.fork_source && can?(current_user, :read_project, project.fork_source)
end
- def project_search_tabs?(tab)
- return false unless @project.present?
-
- abilities = Array(search_tab_ability_map[tab])
-
- if @project.respond_to?(:each) # support multi-project select
- @project.any? { |project| abilities.any? { |ability| can?(current_user, ability, project) } }
- else
- abilities.any? { |ability| can?(current_user, ability, @project) }
- end
- end
-
def can_change_visibility_level?(project, current_user)
can?(current_user, :change_visibility_level, project)
end
@@ -614,41 +602,6 @@ def configure_oauth_import_message(provider, help_url)
s_(str).html_safe % { provider: provider, link_start: link_start, link_end: '</a>'.html_safe }
end
- def tab_ability_map
- {
- cycle_analytics: :read_cycle_analytics,
- environments: :read_environment,
- metrics_dashboards: :metrics_dashboard,
- milestones: :read_milestone,
- snippets: :read_snippet,
- settings: :admin_project,
- builds: :read_build,
- clusters: :read_cluster,
- serverless: :read_cluster,
- terraform: :read_terraform_state,
- error_tracking: :read_sentry_issue,
- alert_management: :read_alert_management_alert,
- incidents: :read_issue,
- labels: :read_label,
- issues: :read_issue,
- project_members: :read_project_member,
- wiki: :read_wiki,
- feature_flags: :read_feature_flag,
- analytics: :read_analytics
- }
- end
-
- def search_tab_ability_map
- @search_tab_ability_map ||= tab_ability_map.merge(
- blobs: :read_code,
- commits: :read_code,
- merge_requests: :read_merge_request,
- notes: [:read_merge_request, :read_code, :read_issue, :read_snippet],
- users: :read_project_member,
- wiki_blobs: :read_wiki
- )
- end
-
def project_lfs_status(project)
if project.lfs_enabled?
content_tag(:span, class: 'lfs-enabled') do
diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb
index 1dd949630c277..cd32023adb630 100644
--- a/app/helpers/search_helper.rb
+++ b/app/helpers/search_helper.rb
@@ -464,65 +464,15 @@ def search_filter_link_json(scope, label, data, search)
result
end
- def show_code_search_tab?
- return true if project_search_tabs?(:blobs)
-
- @project.nil? && search_service.show_elasticsearch_tabs? && feature_flag_tab_enabled?(:global_search_code_tab)
- end
-
- def show_wiki_search_tab?
- return true if project_search_tabs?(:wiki_blobs)
-
- @project.nil? && search_service.show_elasticsearch_tabs? && feature_flag_tab_enabled?(:global_search_wiki_tab)
- end
-
- def show_commits_search_tab?
- return true if project_search_tabs?(:commits)
-
- @project.nil? && search_service.show_elasticsearch_tabs? && feature_flag_tab_enabled?(:global_search_commits_tab)
- end
-
- def show_issues_search_tab?
- return true if project_search_tabs?(:issues)
-
- @project.nil? && feature_flag_tab_enabled?(:global_search_issues_tab)
- end
-
- def show_merge_requests_search_tab?
- return true if project_search_tabs?(:merge_requests)
-
- @project.nil? && feature_flag_tab_enabled?(:global_search_merge_requests_tab)
- end
-
- def show_comments_search_tab?
- return true if project_search_tabs?(:notes)
-
- @project.nil? && search_service.show_elasticsearch_tabs?
- end
-
- def show_snippets_search_tab?
- search_service.show_snippets? && @project.nil? && feature_flag_tab_enabled?(:global_search_snippet_titles_tab)
- end
-
- # search page scope navigation
- def search_navigation
+ def nav_options
{
- projects: { sort: 1, label: _("Projects"), data: { qa_selector: 'projects_tab' }, condition: @project.nil? },
- blobs: { sort: 2, label: _("Code"), data: { qa_selector: 'code_tab' }, condition: show_code_search_tab? },
- # sort: 3 is reserved for EE items
- issues: { sort: 4, label: _("Issues"), condition: show_issues_search_tab? },
- merge_requests: { sort: 5, label: _("Merge requests"), condition: show_merge_requests_search_tab? },
- wiki_blobs: { sort: 6, label: _("Wiki"), condition: show_wiki_search_tab? },
- commits: { sort: 7, label: _("Commits"), condition: show_commits_search_tab? },
- notes: { sort: 8, label: _("Comments"), condition: show_comments_search_tab? },
- milestones: { sort: 9, label: _("Milestones"), condition: project_search_tabs?(:milestones) || @project.nil? },
- users: { sort: 10, label: _("Users"), condition: show_user_search_tab? },
- snippet_titles: { sort: 11, label: _("Snippets"), search: { snippets: true, group_id: nil, project_id: nil }, condition: show_snippets_search_tab? }
+ show_snippets: search_service.show_snippets?
}
end
def search_navigation_json
- sorted_navigation = search_navigation.sort_by { |_, h| h[:sort] }
+ search_navigation = Search::Navigation.new(user: current_user, project: @project, group: @group, options: nav_options)
+ sorted_navigation = search_navigation.tabs.sort_by { |_, h| h[:sort] }
sorted_navigation.each_with_object({}) do |(key, value), hash|
hash[key] = search_filter_link_json(key, value[:label], value[:data], value[:search]) if value[:condition]
@@ -604,14 +554,6 @@ def highlight_and_truncate_issuable(issuable, search_term, _search_highlight)
simple_search_highlight_and_truncate(issuable.description, search_term, highlighter: '<span class="gl-text-gray-900 gl-font-weight-bold">\1</span>')
end
- def show_user_search_tab?
- return project_search_tabs?(:users) if @project
- return false unless can?(current_user, :read_users_list)
- return true if @group
-
- Feature.enabled?(:global_search_users_tab, current_user, type: :ops)
- end
-
def issuable_state_to_badge_class(issuable)
# Closed is considered "danger" for MR so we need to handle separately
if issuable.is_a?(::MergeRequest)
@@ -640,10 +582,6 @@ def issuable_state_text(issuable)
end
end
- def feature_flag_tab_enabled?(flag)
- @group.present? || Feature.enabled?(flag, current_user, type: :ops)
- end
-
def sanitized_search_params
sanitized_params = params.dup
diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb
index 71314f85984cb..73d46a9ba70c2 100644
--- a/app/services/search/project_service.rb
+++ b/app/services/search/project_service.rb
@@ -4,7 +4,6 @@ module Search
class ProjectService
include Search::Filter
include Gitlab::Utils::StrongMemoize
- include ProjectsHelper
ALLOWED_SCOPES = %w(blobs issues merge_requests wiki_blobs commits notes milestones users).freeze
@@ -18,13 +17,13 @@ def initialize(user, project_or_projects, params)
def execute
Gitlab::ProjectSearchResults.new(current_user,
- params[:search],
- project: project,
- repository_ref: params[:repository_ref],
- order_by: params[:order_by],
- sort: params[:sort],
- filters: filters
- )
+ params[:search],
+ project: project,
+ repository_ref: params[:repository_ref],
+ order_by: params[:order_by],
+ sort: params[:sort],
+ filters: filters
+ )
end
def allowed_scopes
@@ -33,10 +32,12 @@ def allowed_scopes
def scope
strong_memoize(:scope) do
- next params[:scope] if allowed_scopes.include?(params[:scope]) && project_search_tabs?(params[:scope].to_sym)
+ search_navigation = Search::Navigation.new(user: current_user, project: project)
+ scope = params[:scope]
+ next scope if allowed_scopes.include?(scope) && search_navigation.tab_enabled_for_project?(scope.to_sym)
- allowed_scopes.find do |scope|
- project_search_tabs?(scope.to_sym)
+ allowed_scopes.find do |s|
+ search_navigation.tab_enabled_for_project?(s.to_sym)
end
end
end
diff --git a/app/services/search_service.rb b/app/services/search_service.rb
index 5705e4c7cef0e..433e9b0da6d35 100644
--- a/app/services/search_service.rb
+++ b/app/services/search_service.rb
@@ -102,16 +102,6 @@ def level
end
end
- def show_elasticsearch_tabs?
- # overridden in EE
- false
- end
-
- def show_epics?
- # overridden in EE
- false
- end
-
def global_search_enabled_for_scope?
return false if show_snippets? && Feature.disabled?(:global_search_snippet_titles_tab, current_user, type: :ops)
diff --git a/ee/app/helpers/ee/search_helper.rb b/ee/app/helpers/ee/search_helper.rb
index 13721f6c706ca..c3fd6d226329e 100644
--- a/ee/app/helpers/ee/search_helper.rb
+++ b/ee/app/helpers/ee/search_helper.rb
@@ -144,11 +144,6 @@ def search_sort_options
options + original_options
end
- override :search_navigation
- def search_navigation
- super.merge(epics: { sort: 3, label: _("Epics"), condition: @project.nil? && search_service.show_epics? })
- end
-
override :search_scope
def search_scope
if current_controller?(:epics)
@@ -206,5 +201,12 @@ def gitlab_com_snippet_db_search?
::Gitlab.com? &&
::Gitlab::CurrentSettings.search_using_elasticsearch?(scope: nil)
end
+
+ override :nav_options
+ def nav_options
+ super.merge(show_epics: search_service.show_epics?,
+ show_elasticsearch_tabs: search_service.show_elasticsearch_tabs?
+ )
+ end
end
end
diff --git a/ee/app/services/ee/search_service.rb b/ee/app/services/ee/search_service.rb
index 2e2c53267ed68..40a4fe532376f 100644
--- a/ee/app/services/ee/search_service.rb
+++ b/ee/app/services/ee/search_service.rb
@@ -17,7 +17,6 @@ def show_epics?
search_service.allowed_scopes.include?('epics')
end
- override :show_elasticsearch_tabs?
def show_elasticsearch_tabs?
::Gitlab::CurrentSettings.search_using_elasticsearch?(scope: search_service.elasticsearchable_scope)
end
diff --git a/ee/lib/ee/search/navigation.rb b/ee/lib/ee/search/navigation.rb
new file mode 100644
index 0000000000000..8971191502e9d
--- /dev/null
+++ b/ee/lib/ee/search/navigation.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module EE
+ module Search
+ module Navigation
+ extend ::Gitlab::Utils::Override
+
+ override :tabs
+ def tabs
+ super.merge(epics: { sort: 3, label: _("Epics"), condition: show_epics_search_tab? })
+ end
+
+ private
+
+ def show_epics_search_tab?
+ project.nil? && !!options[:show_epics]
+ end
+ end
+ end
+end
diff --git a/ee/spec/features/search/elastic/project_search_spec.rb b/ee/spec/features/search/elastic/project_search_spec.rb
index 3604a3d33992d..d3685fc1f99d7 100644
--- a/ee/spec/features/search/elastic/project_search_spec.rb
+++ b/ee/spec/features/search/elastic/project_search_spec.rb
@@ -127,7 +127,15 @@
describe 'renders error when zoekt search fails' do
let(:query) { 'test' }
- let(:search_service) { instance_double(SearchService, scope: 'blobs', use_elasticsearch?: true, use_zoekt?: true) }
+ let(:search_service) do
+ instance_double(Search::ProjectService,
+ scope: 'blobs',
+ use_elasticsearch?: true,
+ use_zoekt?: true,
+ elasticsearchable_scope: project
+ )
+ end
+
let(:results) { Gitlab::Zoekt::SearchResults.new(user, query) }
before do
@@ -135,6 +143,7 @@
allow_next_instance_of(SearchService) do |service|
allow(service).to receive(:search_service).and_return(search_service)
+ allow(service).to receive(:show_epics?).and_return(false)
allow(service).to receive(:search_results).and_return(results)
allow(results).to receive(:zoekt_search).and_return({ Error: 'failed to parse query' })
end
diff --git a/ee/spec/helpers/search_helper_spec.rb b/ee/spec/helpers/search_helper_spec.rb
index aec7c4bf66c0f..de302fdc9fe1b 100644
--- a/ee/spec/helpers/search_helper_spec.rb
+++ b/ee/spec/helpers/search_helper_spec.rb
@@ -327,53 +327,25 @@
end
end
- describe '.search_navigation' do
- using RSpec::Parameterized::TableSyntax
- let(:user) { build(:user) }
- let(:project) { build(:project) }
-
- before do
- allow(self).to receive(:current_user).and_return(user)
- allow(self).to receive(:can?).and_return(true)
- end
-
- context 'for epics' do
- where(:global_project, :show_epics, :condition) do
- nil | false | false
- ref(:project) | true | false
- ref(:project) | false | false
- nil | true | true
+ describe '.search_navigation_json' do
+ context 'when all options enabled' do
+ before do
+ allow(self).to receive(:current_user).and_return(build(:user))
+ allow(self).to receive(:can?).and_return(true)
+ allow(self).to receive(:project_search_tabs?).and_return(true)
+ allow(self).to receive(:feature_flag_tab_enabled?).and_return(true)
+ allow(search_service).to receive(:show_elasticsearch_tabs?).and_return(true)
+ allow(search_service).to receive(:show_epics?).and_return(true)
+ allow(search_service).to receive(:show_snippets?).and_return(true)
+ @project = nil
end
- with_them do
- it 'data item condition is set correctly' do
- @project = global_project
- allow(search_service).to receive(:show_epics?).and_return(show_epics)
-
- expect(search_navigation[:epics][:condition]).to eq(condition)
- end
+ it 'returns items in order' do
+ expect(Gitlab::Json.parse(search_navigation_json).keys).to eq(%w[projects blobs epics issues merge_requests wiki_blobs commits notes milestones users snippet_titles])
end
end
end
- describe '.search_navigation_json with .search_navigation' do
- before do
- allow(self).to receive(:current_user).and_return(build(:user))
- allow(self).to receive(:can?).and_return(true)
- allow(self).to receive(:project_search_tabs?).and_return(true)
- allow(self).to receive(:feature_flag_tab_enabled?).and_return(true)
- allow(self).to receive(:feature_flag_tab_enabled?).and_return(true)
- allow(search_service).to receive(:show_elasticsearch_tabs?).and_return(true)
- allow(search_service).to receive(:show_epics?).and_return(true)
- allow(search_service).to receive(:show_snippets?).and_return(true)
- @project = nil
- end
-
- it 'test search navigation item order for CE all options enabled' do
- expect(Gitlab::Json.parse(search_navigation_json).keys).to eq(%w[projects blobs epics issues merge_requests wiki_blobs commits notes milestones users snippet_titles])
- end
- end
-
describe '.search_filter_link_json' do
using RSpec::Parameterized::TableSyntax
@@ -415,23 +387,6 @@
end
end
- describe 'show_elasticsearch_tabs' do
- let(:user) { build(:user) }
- let_it_be(:project) { build(:project) }
-
- before do
- stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
- allow(self).to receive(:current_user).and_return(user)
- allow(self).to receive(:can?).and_return(false)
- allow(self).to receive(:project_search_tabs?).and_return(false)
- allow(self).to receive(:feature_flag_tab_enabled?).and_return(false)
- end
-
- it 'show_elasticsearch_tabs? returns true' do
- expect(search_service.show_elasticsearch_tabs?).to eq(true)
- end
- end
-
describe '#wiki_blob_link' do
let_it_be(:group) { create(:group, :wiki_repo) }
let(:wiki_blob) do
diff --git a/ee/spec/lib/search/navigation_spec.rb b/ee/spec/lib/search/navigation_spec.rb
new file mode 100644
index 0000000000000..3b7b816607748
--- /dev/null
+++ b/ee/spec/lib/search/navigation_spec.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Search::Navigation, feature_category: :global_search do
+ describe '#tabs' do
+ using RSpec::Parameterized::TableSyntax
+
+ let(:user) { instance_double(User) }
+ let(:project_double) { instance_double(Project) }
+ let(:options) { {} }
+ let(:search_navigation) { described_class.new(user: user, project: project, options: options) }
+
+ before do
+ allow(search_navigation).to receive(:can?).and_return(true)
+ allow(search_navigation).to receive(:tab_enabled_for_project?).and_return(false)
+ allow(search_navigation).to receive(:feature_flag_tab_enabled?).and_return(false)
+ end
+
+ subject(:tabs) { search_navigation.tabs }
+
+ context 'for epics tab' do
+ where(:project, :show_epics, :condition) do
+ nil | false | false
+ nil | nil | false
+ ref(:project_double) | true | false
+ ref(:project_double) | false | false
+ ref(:project_double) | nil | false
+ nil | true | true
+ end
+
+ with_them do
+ let(:options) { { show_epics: show_epics } }
+
+ it 'data item condition is set correctly' do
+ expect(tabs[:epics][:condition]).to eq(condition)
+ end
+ end
+ end
+ end
+end
diff --git a/lib/api/search.rb b/lib/api/search.rb
index 954c3cd9f9ef0..b14fce13f5e2d 100644
--- a/lib/api/search.rb
+++ b/lib/api/search.rb
@@ -59,6 +59,8 @@ def search_service(additional_params = {})
end
def search(additional_params = {})
+ return Kaminari.paginate_array([]) if @project.present? && !project_scope_allowed?
+
search_service = search_service(additional_params)
if search_service.global_search? && !search_service.global_search_enabled_for_scope?
forbidden!('Global Search is disabled for this scope')
@@ -95,6 +97,10 @@ def search(additional_params = {})
)
end
+ def project_scope_allowed?
+ ::Search::Navigation.new(user: current_user, project: @project).tab_enabled_for_project?(params[:scope].to_sym)
+ end
+
def snippets?
%w(snippet_titles).include?(params[:scope]).to_s
end
@@ -108,9 +114,7 @@ def preload_method
end
def verify_search_scope!(resource:)
- # In EE we have additional validation requirements for searches.
- # Defining this method here as a noop allows us to easily extend it in
- # EE, without having to modify this file directly.
+ # no-op
end
def search_type(additional_params = {})
diff --git a/lib/search/navigation.rb b/lib/search/navigation.rb
new file mode 100644
index 0000000000000..3594ac0dc306c
--- /dev/null
+++ b/lib/search/navigation.rb
@@ -0,0 +1,158 @@
+# frozen_string_literal: true
+
+module Search
+ class Navigation
+ include Gitlab::Allowable
+
+ def initialize(user:, project: nil, group: nil, options: {})
+ @user = user
+ @project = project
+ @group = group
+ @options = options
+ end
+
+ def tab_enabled_for_project?(tab)
+ return false unless project.present?
+
+ abilities = Array(search_tab_ability_map[tab])
+ Array.wrap(project).any? { |p| abilities.any? { |ability| can?(user, ability, p) } }
+ end
+
+ def tabs
+ {
+ projects: {
+ sort: 1,
+ label: _("Projects"),
+ data: { qa_selector: 'projects_tab' },
+ condition: project.nil?
+ },
+ blobs: {
+ sort: 2,
+ label: _("Code"),
+ data: { qa_selector: 'code_tab' },
+ condition: show_code_search_tab?
+ },
+ # sort: 3 is reserved for EE items
+ issues: {
+ sort: 4,
+ label: _("Issues"),
+ condition: show_issues_search_tab?
+ },
+ merge_requests: {
+ sort: 5,
+ label: _("Merge requests"),
+ condition: show_merge_requests_search_tab?
+ },
+ wiki_blobs: {
+ sort: 6,
+ label: _("Wiki"),
+ condition: show_wiki_search_tab?
+ },
+ commits: {
+ sort: 7,
+ label: _("Commits"),
+ condition: show_commits_search_tab?
+ },
+ notes: {
+ sort: 8,
+ label: _("Comments"),
+ condition: show_comments_search_tab?
+ },
+ milestones: {
+ sort: 9, label: _("Milestones"),
+ condition: show_milestones_search_tab?
+ },
+ users: {
+ sort: 10,
+ label: _("Users"),
+ condition: show_user_search_tab?
+ },
+ snippet_titles: {
+ sort: 11,
+ label: _("Snippets"),
+ search: { snippets: true, group_id: nil, project_id: nil },
+ condition: show_snippets_search_tab?
+ }
+ }
+ end
+
+ private
+
+ attr_reader :user, :project, :group, :options
+
+ def show_elasticsearch_tabs?
+ !!options[:show_elasticsearch_tabs]
+ end
+
+ def search_tab_ability_map
+ {
+ milestones: :read_milestone,
+ snippets: :read_snippet,
+ issues: :read_issue,
+ blobs: :read_code,
+ commits: :read_code,
+ merge_requests: :read_merge_request,
+ notes: [:read_merge_request, :read_code, :read_issue, :read_snippet],
+ users: :read_project_member,
+ wiki_blobs: :read_wiki
+ }
+ end
+
+ def show_user_search_tab?
+ return true if tab_enabled_for_project?(:users)
+ return false unless can?(user, :read_users_list)
+
+ project.nil? && feature_flag_tab_enabled?(:global_search_users_tab)
+ end
+
+ def show_code_search_tab?
+ return true if tab_enabled_for_project?(:blobs)
+
+ project.nil? && show_elasticsearch_tabs? && feature_flag_tab_enabled?(:global_search_code_tab)
+ end
+
+ def show_wiki_search_tab?
+ return true if tab_enabled_for_project?(:wiki_blobs)
+
+ project.nil? && show_elasticsearch_tabs? && feature_flag_tab_enabled?(:global_search_wiki_tab)
+ end
+
+ def show_commits_search_tab?
+ return true if tab_enabled_for_project?(:commits)
+
+ project.nil? && show_elasticsearch_tabs? && feature_flag_tab_enabled?(:global_search_commits_tab)
+ end
+
+ def show_issues_search_tab?
+ return true if tab_enabled_for_project?(:issues)
+
+ project.nil? && feature_flag_tab_enabled?(:global_search_issues_tab)
+ end
+
+ def show_merge_requests_search_tab?
+ return true if tab_enabled_for_project?(:merge_requests)
+
+ project.nil? && feature_flag_tab_enabled?(:global_search_merge_requests_tab)
+ end
+
+ def show_comments_search_tab?
+ return true if tab_enabled_for_project?(:notes)
+
+ project.nil? && show_elasticsearch_tabs?
+ end
+
+ def show_snippets_search_tab?
+ !!options[:show_snippets] && project.nil? && feature_flag_tab_enabled?(:global_search_snippet_titles_tab)
+ end
+
+ def show_milestones_search_tab?
+ project.nil? || tab_enabled_for_project?(:milestones)
+ end
+
+ def feature_flag_tab_enabled?(flag)
+ group.present? || Feature.enabled?(flag, user, type: :ops)
+ end
+ end
+end
+
+Search::Navigation.prepend_mod
diff --git a/spec/features/snippets/search_snippets_spec.rb b/spec/features/snippets/search_snippets_spec.rb
index 66c4041241ce7..afb53c563de96 100644
--- a/spec/features/snippets/search_snippets_spec.rb
+++ b/spec/features/snippets/search_snippets_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe 'Search Snippets', :js, feature_category: :source_code_management do
+RSpec.describe 'Search Snippets', :js, feature_category: :global_search do
it 'user searches for snippets by title' do
public_snippet = create(:personal_snippet, :public, title: 'Beginning and Middle')
private_snippet = create(:personal_snippet, :private, title: 'Middle and End')
diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb
index a71a8392e1c1f..1ff7e48abfc25 100644
--- a/spec/helpers/search_helper_spec.rb
+++ b/spec/helpers/search_helper_spec.rb
@@ -722,78 +722,6 @@ def simple_sanitize(str)
end
end
- describe '#show_user_search_tab?' do
- subject { show_user_search_tab? }
-
- let(:current_user) { build(:user) }
-
- before do
- allow(self).to receive(:current_user).and_return(current_user)
- end
-
- context 'when project search' do
- before do
- @project = :some_project
-
- expect(self).to receive(:project_search_tabs?)
- .with(:users)
- .and_return(:value)
- end
-
- it 'delegates to project_search_tabs?' do
- expect(subject).to eq(:value)
- end
- end
-
- context 'when group search' do
- before do
- @group = :some_group
- end
-
- context 'when current_user can read_users_list' do
- before do
- allow(self).to receive(:can?).with(current_user, :read_users_list).and_return(true)
- end
-
- it { is_expected.to eq(true) }
- end
-
- context 'when current_user cannot read_users_list' do
- before do
- allow(self).to receive(:can?).with(current_user, :read_users_list).and_return(false)
- end
-
- it { is_expected.to eq(false) }
- end
- end
-
- context 'when global search' do
- context 'when current_user can read_users_list' do
- before do
- allow(self).to receive(:can?).with(current_user, :read_users_list).and_return(true)
- end
-
- it { is_expected.to eq(true) }
-
- context 'when global_search_user_tab feature flag is disabled' do
- before do
- stub_feature_flags(global_search_users_tab: false)
- end
-
- it { is_expected.to eq(false) }
- end
- end
-
- context 'when current_user cannot read_users_list' do
- before do
- allow(self).to receive(:can?).with(current_user, :read_users_list).and_return(false)
- end
-
- it { is_expected.to eq(false) }
- end
- end
- end
-
describe '#repository_ref' do
using RSpec::Parameterized::TableSyntax
@@ -1113,248 +1041,24 @@ def simple_sanitize(str)
end
end
- describe '.search_navigation' do
- using RSpec::Parameterized::TableSyntax
- let(:user) { build(:user) }
- let_it_be(:project) { build(:project) }
-
- before do
- allow(self).to receive(:current_user).and_return(user)
- allow(self).to receive(:can?).and_return(true)
- allow(self).to receive(:project_search_tabs?).and_return(false)
- allow(self).to receive(:feature_flag_tab_enabled?).and_return(false)
- end
-
- context 'projects' do
- where(:global_project, :condition) do
- nil | true
- ref(:project) | false
- end
-
- with_them do
- it 'data item condition is set correctly' do
- @project = global_project
-
- expect(search_navigation[:projects][:condition]).to eq(condition)
- end
- end
- end
-
- context 'code' do
- where(:feature_flag_tab_enabled, :show_elasticsearch_tabs, :global_project, :project_search_tabs, :condition) do
- false | false | nil | false | false
- true | true | nil | true | true
- true | false | nil | false | false
- false | true | nil | false | false
- false | false | ref(:project) | true | true
- true | false | ref(:project) | false | false
- end
-
- with_them do
- it 'data item condition is set correctly' do
- @project = global_project
- allow(search_service).to receive(:show_elasticsearch_tabs?).and_return(show_elasticsearch_tabs)
- allow(self).to receive(:feature_flag_tab_enabled?).with(:global_search_code_tab).and_return(feature_flag_tab_enabled)
- allow(self).to receive(:project_search_tabs?).with(:blobs).and_return(project_search_tabs)
-
- expect(search_navigation[:blobs][:condition]).to eq(condition)
- end
- end
- end
-
- context 'issues' do
- where(:project_search_tabs, :global_search_issues_tab, :global_project, :condition) do
- false | false | nil | false
- false | true | nil | true
- false | true | ref(:project) | false
- false | false | ref(:project) | false
- true | false | nil | true
- true | true | nil | true
- true | false | ref(:project) | true
- true | true | ref(:project) | true
- end
-
- with_them do
- it 'data item condition is set correctly' do
- @project = global_project
- allow(self).to receive(:feature_flag_tab_enabled?).with(:global_search_issues_tab).and_return(global_search_issues_tab)
- allow(self).to receive(:project_search_tabs?).with(:issues).and_return(project_search_tabs)
-
- expect(search_navigation[:issues][:condition]).to eq(condition)
- end
- end
- end
-
- context 'merge requests' do
- where(:project_search_tabs, :feature_flag_tab_enabled, :global_project, :condition) do
- false | false | nil | false
- true | false | nil | true
- false | false | ref(:project) | false
- true | false | ref(:project) | true
- false | true | nil | true
- true | true | nil | true
- false | true | ref(:project) | false
- true | true | ref(:project) | true
- end
-
- with_them do
- it 'data item condition is set correctly' do
- @project = global_project
- allow(self).to receive(:feature_flag_tab_enabled?).with(:global_search_merge_requests_tab).and_return(feature_flag_tab_enabled)
- allow(self).to receive(:project_search_tabs?).with(:merge_requests).and_return(project_search_tabs)
-
- expect(search_navigation[:merge_requests][:condition]).to eq(condition)
- end
- end
- end
-
- context 'wiki' do
- where(:global_search_wiki_tab, :show_elasticsearch_tabs, :global_project, :project_search_tabs, :condition) do
- false | false | nil | true | true
- false | false | nil | false | false
- false | false | ref(:project) | false | false
- false | true | nil | false | false
- false | true | ref(:project) | false | false
- true | false | nil | false | false
- true | true | ref(:project) | false | false
- end
-
- with_them do
- it 'data item condition is set correctly' do
- @project = global_project
- allow(search_service).to receive(:show_elasticsearch_tabs?).and_return(show_elasticsearch_tabs)
- allow(self).to receive(:feature_flag_tab_enabled?).with(:global_search_wiki_tab).and_return(global_search_wiki_tab)
- allow(self).to receive(:project_search_tabs?).with(:wiki_blobs).and_return(project_search_tabs)
-
- expect(search_navigation[:wiki_blobs][:condition]).to eq(condition)
- end
- end
- end
-
- context 'commits' do
- where(:global_search_commits_tab, :show_elasticsearch_tabs, :global_project, :project_search_tabs, :condition) do
- false | false | nil | true | true
- false | false | ref(:project) | true | true
- false | false | nil | false | false
- false | true | ref(:project) | false | false
- false | true | nil | false | false
- true | false | nil | false | false
- true | false | ref(:project) | false | false
- true | true | ref(:project) | false | false
- true | true | nil | false | true
- end
-
- with_them do
- it 'data item condition is set correctly' do
- @project = global_project
- allow(search_service).to receive(:show_elasticsearch_tabs?).and_return(show_elasticsearch_tabs)
- allow(self).to receive(:feature_flag_tab_enabled?).with(:global_search_commits_tab).and_return(global_search_commits_tab)
- allow(self).to receive(:project_search_tabs?).with(:commits).and_return(project_search_tabs)
-
- expect(search_navigation[:commits][:condition]).to eq(condition)
- end
- end
- end
-
- context 'comments' do
- where(:project_search_tabs, :show_elasticsearch_tabs, :global_project, :condition) do
- true | true | nil | true
- true | true | ref(:project) | true
- false | false | nil | false
- false | false | ref(:project) | false
- false | true | nil | true
- false | true | ref(:project) | false
- true | false | nil | true
- true | false | ref(:project) | true
- end
-
- with_them do
- it 'data item condition is set correctly' do
- @project = global_project
- allow(search_service).to receive(:show_elasticsearch_tabs?).and_return(show_elasticsearch_tabs)
- allow(self).to receive(:project_search_tabs?).with(:notes).and_return(project_search_tabs)
-
- expect(search_navigation[:notes][:condition]).to eq(condition)
- end
- end
- end
-
- context 'milestones' do
- where(:global_project, :project_search_tabs, :condition) do
- ref(:project) | true | true
- nil | false | true
- ref(:project) | false | false
- nil | true | true
- end
-
- with_them do
- it 'data item condition is set correctly' do
- @project = global_project
- allow(self).to receive(:project_search_tabs?).with(:milestones).and_return(project_search_tabs)
-
- expect(search_navigation[:milestones][:condition]).to eq(condition)
- end
- end
- end
-
- context 'users' do
- where(:show_user_search_tab, :condition) do
- true | true
- false | false
- end
-
- with_them do
- it 'data item condition is set correctly' do
- allow(self).to receive(:show_user_search_tab?).and_return(show_user_search_tab)
-
- expect(search_navigation[:users][:condition]).to eq(condition)
- end
- end
- end
-
- context 'snippet_titles' do
- where(:global_project, :global_show_snippets, :global_feature_flag_enabled, :condition) do
- ref(:project) | true | false | false
- nil | false | false | false
- ref(:project) | false | false | false
- nil | true | false | false
- ref(:project) | true | true | false
- nil | false | true | false
- ref(:project) | false | true | false
- nil | true | true | true
- end
-
- with_them do
- it 'data item condition is set correctly' do
- allow(search_service).to receive(:show_snippets?).and_return(global_show_snippets)
- allow(self).to receive(:feature_flag_tab_enabled?).with(:global_search_snippet_titles_tab)
- .and_return(global_feature_flag_enabled)
- @project = global_project
-
- expect(search_navigation[:snippet_titles][:condition]).to eq(condition)
- end
- end
- end
- end
-
describe '.search_navigation_json' do
using RSpec::Parameterized::TableSyntax
- context 'with data' do
+ context 'with some tab conditions set to false' do
example_data_1 = {
projects: { label: _("Projects"), condition: true },
- blobs: { label: _("Code"), condition: false }
+ blobs: { label: _("Code"), condition: false }
}
example_data_2 = {
projects: { label: _("Projects"), condition: false },
- blobs: { label: _("Code"), condition: false }
+ blobs: { label: _("Code"), condition: false }
}
example_data_3 = {
projects: { label: _("Projects"), condition: true },
- blobs: { label: _("Code"), condition: true },
- epics: { label: _("Epics"), condition: true }
+ blobs: { label: _("Code"), condition: true },
+ epics: { label: _("Epics"), condition: true }
}
where(:data, :matcher) do
@@ -1365,28 +1069,31 @@ def simple_sanitize(str)
with_them do
it 'renders data correctly' do
- allow(self).to receive(:search_navigation).with(no_args).and_return(data)
+ allow(self).to receive(:current_user).and_return(build(:user))
+ allow_next_instance_of(Search::Navigation) do |search_nav|
+ allow(search_nav).to receive(:tabs).and_return(data)
+ end
expect(search_navigation_json).to instance_exec(&matcher)
end
end
end
- end
- describe '.search_navigation_json with .search_navigation' do
- before do
- allow(self).to receive(:current_user).and_return(build(:user))
- allow(self).to receive(:can?).and_return(true)
- allow(self).to receive(:project_search_tabs?).and_return(true)
- allow(self).to receive(:feature_flag_tab_enabled?).and_return(true)
- allow(self).to receive(:feature_flag_tab_enabled?).and_return(true)
- allow(search_service).to receive(:show_elasticsearch_tabs?).and_return(true)
- allow(search_service).to receive(:show_snippets?).and_return(true)
- @project = nil
- end
+ context 'when all options enabled' do
+ before do
+ allow(self).to receive(:current_user).and_return(build(:user))
+ allow(search_service).to receive(:show_snippets?).and_return(true)
+ allow_next_instance_of(Search::Navigation) do |search_nav|
+ allow(search_nav).to receive(:tab_enabled_for_project?).and_return(true)
+ allow(search_nav).to receive(:feature_flag_tab_enabled?).and_return(true)
+ end
- it 'test search navigation item order for CE all options enabled' do
- expect(Gitlab::Json.parse(search_navigation_json).keys).to eq(%w[projects blobs issues merge_requests wiki_blobs commits notes milestones users snippet_titles])
+ @project = nil
+ end
+
+ it 'returns items in order' do
+ expect(Gitlab::Json.parse(search_navigation_json).keys).to eq(%w[projects blobs issues merge_requests wiki_blobs commits notes milestones users snippet_titles])
+ end
end
end
@@ -1431,30 +1138,6 @@ def simple_sanitize(str)
end
end
- describe 'show_elasticsearch_tabs' do
- subject { search_service.show_elasticsearch_tabs? }
-
- let(:user) { build(:user) }
-
- before do
- allow(self).to receive(:current_user).and_return(user)
- end
-
- it { is_expected.to eq(false) }
- end
-
- describe 'show_epics' do
- subject { search_service.show_epics? }
-
- let(:user) { build(:user) }
-
- before do
- allow(self).to receive(:current_user).and_return(user)
- end
-
- it { is_expected.to eq(false) }
- end
-
describe '#wiki_blob_link' do
let_it_be(:project) { create :project, :wiki_repo }
let(:wiki_blob) do
diff --git a/spec/lib/search/navigation_spec.rb b/spec/lib/search/navigation_spec.rb
new file mode 100644
index 0000000000000..da8c27b499023
--- /dev/null
+++ b/spec/lib/search/navigation_spec.rb
@@ -0,0 +1,280 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Search::Navigation, feature_category: :global_search do
+ let(:user) { instance_double(User) }
+ let(:project_double) { instance_double(Project) }
+ let(:options) { {} }
+ let(:search_navigation) { described_class.new(user: user, project: project, options: options) }
+
+ describe '#tab_enabled_for_project?' do
+ let(:project) { project_double }
+ let(:tab) { :blobs }
+
+ subject(:tab_enabled_for_project) { search_navigation.tab_enabled_for_project?(tab) }
+
+ context 'when user has ability for tab' do
+ before do
+ allow(search_navigation).to receive(:can?).with(user, :read_code, project_double).and_return(true)
+ end
+
+ it { is_expected.to eq(true) }
+ end
+
+ context 'when user does not have ability for tab' do
+ before do
+ allow(search_navigation).to receive(:can?).with(user, :read_code, project_double).and_return(false)
+ end
+
+ it { is_expected.to eq(false) }
+ end
+
+ context 'when an array of projects is provided' do
+ let(:project) { Array.wrap(project_double) }
+
+ before do
+ allow(search_navigation).to receive(:can?).with(user, :read_code, project_double).and_return(true)
+ end
+
+ it { is_expected.to eq(true) }
+ end
+
+ context 'when project is not present' do
+ let_it_be(:project) { nil }
+
+ it { is_expected.to eq(false) }
+ end
+ end
+
+ describe '#tabs' do
+ using RSpec::Parameterized::TableSyntax
+
+ before do
+ allow(search_navigation).to receive(:can?).and_return(true)
+ allow(search_navigation).to receive(:tab_enabled_for_project?).and_return(false)
+ allow(search_navigation).to receive(:feature_flag_tab_enabled?).and_return(false)
+ end
+
+ subject(:tabs) { search_navigation.tabs }
+
+ context 'for projects tab' do
+ where(:project, :condition) do
+ nil | true
+ ref(:project_double) | false
+ end
+
+ with_them do
+ it 'data item condition is set correctly' do
+ expect(tabs[:projects][:condition]).to eq(condition)
+ end
+ end
+ end
+
+ context 'for code tab' do
+ where(:feature_flag_enabled, :show_elasticsearch_tabs, :project, :tab_enabled, :condition) do
+ false | false | nil | false | false
+ true | true | nil | true | true
+ true | false | nil | false | false
+ false | true | nil | false | false
+ false | false | ref(:project_double) | true | true
+ true | false | ref(:project_double) | false | false
+ end
+
+ with_them do
+ let(:options) { { show_elasticsearch_tabs: show_elasticsearch_tabs } }
+
+ it 'data item condition is set correctly' do
+ allow(search_navigation).to receive(:feature_flag_tab_enabled?)
+ .with(:global_search_code_tab).and_return(feature_flag_enabled)
+ allow(search_navigation).to receive(:tab_enabled_for_project?).with(:blobs).and_return(tab_enabled)
+
+ expect(tabs[:blobs][:condition]).to eq(condition)
+ end
+ end
+ end
+
+ context 'for issues tab' do
+ where(:tab_enabled, :feature_flag_enabled, :project, :condition) do
+ false | false | nil | false
+ false | true | nil | true
+ false | true | ref(:project_double) | false
+ false | false | ref(:project_double) | false
+ true | false | nil | true
+ true | true | nil | true
+ true | false | ref(:project_double) | true
+ true | true | ref(:project_double) | true
+ end
+
+ with_them do
+ it 'data item condition is set correctly' do
+ allow(search_navigation).to receive(:feature_flag_tab_enabled?)
+ .with(:global_search_issues_tab).and_return(feature_flag_enabled)
+ allow(search_navigation).to receive(:tab_enabled_for_project?).with(:issues).and_return(tab_enabled)
+
+ expect(tabs[:issues][:condition]).to eq(condition)
+ end
+ end
+ end
+
+ context 'for merge requests tab' do
+ where(:tab_enabled, :feature_flag_enabled, :project, :condition) do
+ false | false | nil | false
+ true | false | nil | true
+ false | false | ref(:project_double) | false
+ true | false | ref(:project_double) | true
+ false | true | nil | true
+ true | true | nil | true
+ false | true | ref(:project_double) | false
+ true | true | ref(:project_double) | true
+ end
+
+ with_them do
+ it 'data item condition is set correctly' do
+ allow(search_navigation).to receive(:feature_flag_tab_enabled?)
+ .with(:global_search_merge_requests_tab).and_return(feature_flag_enabled)
+ allow(search_navigation).to receive(:tab_enabled_for_project?).with(:merge_requests).and_return(tab_enabled)
+
+ expect(tabs[:merge_requests][:condition]).to eq(condition)
+ end
+ end
+ end
+
+ context 'for wiki tab' do
+ where(:feature_flag_enabled, :show_elasticsearch_tabs, :project, :tab_enabled, :condition) do
+ false | false | nil | true | true
+ false | false | nil | false | false
+ false | false | ref(:project_double) | false | false
+ false | true | nil | false | false
+ false | true | ref(:project_double) | false | false
+ true | false | nil | false | false
+ true | true | ref(:project_double) | false | false
+ end
+
+ with_them do
+ let(:options) { { show_elasticsearch_tabs: show_elasticsearch_tabs } }
+
+ it 'data item condition is set correctly' do
+ allow(search_navigation).to receive(:feature_flag_tab_enabled?)
+ .with(:global_search_wiki_tab).and_return(feature_flag_enabled)
+ allow(search_navigation).to receive(:tab_enabled_for_project?).with(:wiki_blobs).and_return(tab_enabled)
+
+ expect(tabs[:wiki_blobs][:condition]).to eq(condition)
+ end
+ end
+ end
+
+ context 'for commits tab' do
+ where(:feature_flag_enabled, :show_elasticsearch_tabs, :project, :tab_enabled, :condition) do
+ false | false | nil | true | true
+ false | false | ref(:project_double) | true | true
+ false | false | nil | false | false
+ false | true | ref(:project_double) | false | false
+ false | true | nil | false | false
+ true | false | nil | false | false
+ true | false | ref(:project_double) | false | false
+ true | true | ref(:project_double) | false | false
+ true | true | nil | false | true
+ end
+
+ with_them do
+ let(:options) { { show_elasticsearch_tabs: show_elasticsearch_tabs } }
+
+ it 'data item condition is set correctly' do
+ allow(search_navigation).to receive(:feature_flag_tab_enabled?)
+ .with(:global_search_commits_tab).and_return(feature_flag_enabled)
+ allow(search_navigation).to receive(:tab_enabled_for_project?).with(:commits).and_return(tab_enabled)
+
+ expect(tabs[:commits][:condition]).to eq(condition)
+ end
+ end
+ end
+
+ context 'for comments tab' do
+ where(:tab_enabled, :show_elasticsearch_tabs, :project, :condition) do
+ true | true | nil | true
+ true | true | ref(:project_double) | true
+ false | false | nil | false
+ false | false | ref(:project_double) | false
+ false | true | nil | true
+ false | true | ref(:project_double) | false
+ true | false | nil | true
+ true | false | ref(:project_double) | true
+ end
+
+ with_them do
+ let(:options) { { show_elasticsearch_tabs: show_elasticsearch_tabs } }
+
+ it 'data item condition is set correctly' do
+ allow(search_navigation).to receive(:tab_enabled_for_project?).with(:notes).and_return(tab_enabled)
+
+ expect(tabs[:notes][:condition]).to eq(condition)
+ end
+ end
+ end
+
+ context 'for milestones tab' do
+ where(:project, :tab_enabled, :condition) do
+ ref(:project_double) | true | true
+ nil | false | true
+ ref(:project_double) | false | false
+ nil | true | true
+ end
+
+ with_them do
+ it 'data item condition is set correctly' do
+ allow(search_navigation).to receive(:tab_enabled_for_project?).with(:milestones).and_return(tab_enabled)
+
+ expect(tabs[:milestones][:condition]).to eq(condition)
+ end
+ end
+ end
+
+ context 'for users tab' do
+ where(:feature_flag_enabled, :can_read_users_list, :project, :tab_enabled, :condition) do
+ false | false | ref(:project_double) | true | true
+ false | false | nil | false | false
+ false | true | nil | false | false
+ false | true | ref(:project_double) | false | false
+ true | true | nil | false | true
+ true | true | ref(:project_double) | false | false
+ end
+
+ with_them do
+ it 'data item condition is set correctly' do
+ allow(search_navigation).to receive(:tab_enabled_for_project?).with(:users).and_return(tab_enabled)
+ allow(search_navigation).to receive(:can?)
+ .with(user, :read_users_list, project_double).and_return(can_read_users_list)
+ allow(search_navigation).to receive(:feature_flag_tab_enabled?)
+ .with(:global_search_users_tab).and_return(feature_flag_enabled)
+
+ expect(tabs[:users][:condition]).to eq(condition)
+ end
+ end
+ end
+
+ context 'for snippet_titles tab' do
+ where(:project, :show_snippets, :feature_flag_enabled, :condition) do
+ ref(:project_double) | true | false | false
+ nil | false | false | false
+ ref(:project_double) | false | false | false
+ nil | true | false | false
+ ref(:project_double) | true | true | false
+ nil | false | true | false
+ ref(:project_double) | false | true | false
+ nil | true | true | true
+ end
+
+ with_them do
+ let(:options) { { show_snippets: show_snippets } }
+
+ it 'data item condition is set correctly' do
+ allow(search_navigation).to receive(:feature_flag_tab_enabled?)
+ .with(:global_search_snippet_titles_tab).and_return(feature_flag_enabled)
+
+ expect(tabs[:snippet_titles][:condition]).to eq(condition)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb
index 1b331e9c0992d..0f0ce6ae61e5a 100644
--- a/spec/requests/api/search_spec.rb
+++ b/spec/requests/api/search_spec.rb
@@ -688,6 +688,16 @@ def request
end
end
+ context 'when user does not have permissions for scope' do
+ it 'returns an empty array' do
+ project.project_feature.update!(issues_access_level: Gitlab::VisibilityLevel::PRIVATE)
+
+ get api(endpoint, user), params: { scope: 'issues', search: 'awesome' }
+
+ expect(json_response).to be_empty
+ end
+ end
+
context 'when project does not exist' do
it 'returns 404 error' do
get api('/projects/0/search', user), params: { scope: 'issues', search: 'awesome' }
--
GitLab