From 4bb16efb97091900d14eddff8566a37ebcfa5cdc Mon Sep 17 00:00:00 2001
From: Siddharth Dungarwal <sdungarwal@gitlab.com>
Date: Thu, 26 Sep 2024 16:07:02 +0000
Subject: [PATCH] Apply 1 suggestion(s) to 1 file(s)
Co-authored-by: DANGER_GITLAB_API_TOKEN <project_278964_bot_445da1b60fc7336b6b6776383134d10f@noreply.gitlab.com>
---
app/helpers/search_helper.rb | 5 +
app/views/search/_results_list.html.haml | 3 +
doc/development/advanced_search.md | 202 ++++++++++++++++++
.../views/search/results/_work_item.html.haml | 20 ++
.../search_epics_uses_work_items_index.yml | 9 +
ee/lib/gitlab/elastic/search_results.rb | 39 +++-
ee/lib/search/elastic/filters.rb | 86 ++++++++
.../elastic/work_item_group_query_builder.rb | 20 ++
.../search/elastic/work_item_query_builder.rb | 15 +-
.../search/elastic/group_search_spec.rb | 8 +-
.../search/user_searches_for_epics_spec.rb | 45 +++-
.../elastic/group_search_results_spec.rb | 56 +++++
ee/spec/lib/search/elastic/filters_spec.rb | 176 +++++++++++++++
.../services/search/global_service_spec.rb | 32 ++-
ee/spec/services/search/group_service_spec.rb | 57 ++++-
locale/gitlab.pot | 3 +
16 files changed, 751 insertions(+), 25 deletions(-)
create mode 100644 ee/app/views/search/results/_work_item.html.haml
create mode 100644 ee/config/feature_flags/development/search_epics_uses_work_items_index.yml
create mode 100644 ee/lib/search/elastic/work_item_group_query_builder.rb
diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb
index 528c1b2cc9d77..4465ee587fa2b 100644
--- a/app/helpers/search_helper.rb
+++ b/app/helpers/search_helper.rb
@@ -248,6 +248,11 @@ def search_scope
end
end
+ def should_show_work_items_as_epics_in_results?
+ ::Feature.enabled?(:search_epics_uses_work_items_index, current_user) &&
+ ::Elastic::DataMigrationService.migration_has_finished?(:backfill_work_items)
+ end
+
def should_show_zoekt_results?(_scope, _search_type)
false
end
diff --git a/app/views/search/_results_list.html.haml b/app/views/search/_results_list.html.haml
index 40e15e33e49c2..c5365bd03441b 100644
--- a/app/views/search/_results_list.html.haml
+++ b/app/views/search/_results_list.html.haml
@@ -20,6 +20,9 @@
- if @scope == 'projects'
.term
= render 'shared/projects/list', projects: @search_objects, pipeline_status: false
+ - elsif @scope === 'epics' && should_show_work_items_as_epics_in_results?
+ - @search_objects.each_with_index do |work_item, index|
+ = render partial: "search/results/work_item", locals: { work_item: work_item, index: index }
- elsif @scope === 'users'
%table.table.b-table.gl-table.b-table-stacked-md{ role: 'table' }
%thead
diff --git a/doc/development/advanced_search.md b/doc/development/advanced_search.md
index f227f4112fe1d..49311fa22daae 100644
--- a/doc/development/advanced_search.md
+++ b/doc/development/advanced_search.md
@@ -450,6 +450,43 @@ Requires `type` field. Query with `doc_type` in options.
}
```
+##### `by_group_level_confidentiality`
+
+Requires `current_user` and `group_ids` fields. Query based on the permissions to user to read confidential group entities.
+
+```json
+{
+ "bool": {
+ "must": [
+ {
+ "term": {
+ "confidential": {
+ "value": true,
+ "_name": "confidential:true"
+ }
+ }
+ },
+ {
+ "terms": {
+ "namespace_id": [
+ 1
+ ],
+ "_name": "groups:can:read_confidential_work_items"
+ }
+ }
+ ]
+ },
+ "should": {
+ "term": {
+ "confidential": {
+ "value": false,
+ "_name": "confidential:false"
+ }
+ }
+ }
+}
+```
+
##### `by_confidentiality`
Requires `confidential`, `author_id`, `assignee_id`, `project_id` fields. Query with `confidential` in options.
@@ -685,6 +722,171 @@ Requires `source_branch` field. Query with `source_branch` or `not_source_branch
}
```
+##### `by_group_level_authorization`
+
+Requires `current_user`, `group_ids`, `traversal_id`, `search_level` fields. Query with `search_level` and
+filter on `namespace_visibility_level` based on permissions user has for each group.
+
+NOTE:
+Examples are shown for a logged in user. The JSON may be different for users with authorizations, admins, external, or anonymous users
+
+###### global
+
+```json
+{
+ "bool": {
+ "should": [
+ {
+ "bool": {
+ "filter": [
+ {
+ "term": {
+ "namespace_visibility_level": {
+ "value": 20,
+ "_name": "filters:namespace_visibility_level:public"
+ }
+ }
+ }
+ ]
+ }
+ },
+ {
+ "bool": {
+ "filter": [
+ {
+ "term": {
+ "namespace_visibility_level": {
+ "value": 10,
+ "_name": "filters:namespace_visibility_level:internal"
+ }
+ }
+ }
+ ]
+ }
+ },
+ {
+ "bool": {
+ "filter": [
+ {
+ "term": {
+ "namespace_visibility_level": {
+ "value": 0,
+ "_name": "filters:namespace_visibility_level:private"
+ }
+ }
+ },
+ {
+ "terms": {
+ "namespace_id": [
+ 33,
+ 22
+ ]
+ }
+ }
+ ]
+ }
+ }
+ ],
+ "minimum_should_match": 1
+ }
+}
+```
+
+###### group
+
+```json
+[
+ {
+ "bool": {
+ "_name": "filters:level:group",
+ "minimum_should_match": 1,
+ "should": [
+ {
+ "prefix": {
+ "traversal_ids": {
+ "_name": "filters:level:group:ancestry_filter:descendants",
+ "value": "22-"
+ }
+ }
+ }
+ ]
+ }
+ },
+ {
+ "bool": {
+ "should": [
+ {
+ "bool": {
+ "filter": [
+ {
+ "term": {
+ "namespace_visibility_level": {
+ "value": 20,
+ "_name": "filters:namespace_visibility_level:public"
+ }
+ }
+ }
+ ]
+ }
+ },
+ {
+ "bool": {
+ "filter": [
+ {
+ "term": {
+ "namespace_visibility_level": {
+ "value": 10,
+ "_name": "filters:namespace_visibility_level:internal"
+ }
+ }
+ }
+ ]
+ }
+ },
+ {
+ "bool": {
+ "filter": [
+ {
+ "term": {
+ "namespace_visibility_level": {
+ "value": 0,
+ "_name": "filters:namespace_visibility_level:private"
+ }
+ }
+ },
+ {
+ "terms": {
+ "namespace_id": [
+ 22
+ ]
+ }
+ }
+ ]
+ }
+ }
+ ],
+ "minimum_should_match": 1
+ }
+ },
+ {
+ "bool": {
+ "_name": "filters:level:group",
+ "minimum_should_match": 1,
+ "should": [
+ {
+ "prefix": {
+ "traversal_ids": {
+ "_name": "filters:level:group:ancestry_filter:descendants",
+ "value": "22-"
+ }
+ }
+ }
+ ]
+ }
+ }
+]
+```
+
##### `by_search_level_and_membership`
Requires `project_id` and `traversal_id` fields. Supports feature `*_access_level` fields. Query with `search_level`
diff --git a/ee/app/views/search/results/_work_item.html.haml b/ee/app/views/search/results/_work_item.html.haml
new file mode 100644
index 0000000000000..ad10d2d632c14
--- /dev/null
+++ b/ee/app/views/search/results/_work_item.html.haml
@@ -0,0 +1,20 @@
+- index = local_assigns.fetch(:index, nil)
+- work_item = local_assigns.fetch(:work_item, nil)
+- position = index + 1
+
+.search-result-row.gl-mt-5
+ %span.gl-flex.gl-items-center
+ - if work_item.closed?
+ = gl_badge_tag _('Closed'), variant: :info
+ - else
+ = gl_badge_tag _('Open'), variant: :success
+ = sprite_icon('eye-slash', css_class: 'gl-text-secondary gl-ml-2') if work_item.confidential?
+ = link_to group_work_item_path(work_item.namespace, work_item), data: { event_tracking: 'click_search_result', event_label: @scope, event_value: position, event_property: @search_term }, class: 'gl-w-full' do
+ %span.term.str-truncated.gl-font-bold.gl-ml-2= work_item.title
+ .gl-text-secondary.gl-mb-3.gl-text-sm
+ = sprintf(s_('%{group_name}&%{work_item_iid} · created %{work_item_created} by %{author}'), { group_name: work_item.namespace.full_name, work_item_iid: work_item.iid, work_item_created: time_ago_with_tooltip(work_item.created_at, placement: 'bottom'), author: link_to_member(work_item.author, avatar: false) }).html_safe
+ ·
+ = _('updated %{time_ago}').html_safe % { time_ago: time_ago_with_tooltip(work_item.updated_at, placement: 'bottom') }
+ - if work_item.description.present?
+ .description.term.col-sm-10.gl-px-0.gl-text-sm
+ = truncate(work_item.description, length: 200)
diff --git a/ee/config/feature_flags/development/search_epics_uses_work_items_index.yml b/ee/config/feature_flags/development/search_epics_uses_work_items_index.yml
new file mode 100644
index 0000000000000..37a9dcc578a0b
--- /dev/null
+++ b/ee/config/feature_flags/development/search_epics_uses_work_items_index.yml
@@ -0,0 +1,9 @@
+---
+name: search_epics_uses_work_items_index
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/480118
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165827
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/487755
+milestone: '17.5'
+type: development
+group: group::global search
+default_enabled: false
diff --git a/ee/lib/gitlab/elastic/search_results.rb b/ee/lib/gitlab/elastic/search_results.rb
index 84cf5e0ab804a..27b94fd9f5cbc 100644
--- a/ee/lib/gitlab/elastic/search_results.rb
+++ b/ee/lib/gitlab/elastic/search_results.rb
@@ -57,7 +57,7 @@ def objects(scope, page: 1, per_page: DEFAULT_PER_PAGE, preload_method: nil)
when 'notes'
eager_load(notes, page, per_page, preload_method, project: [:invited_groups, :route, :namespace])
when 'epics'
- eager_load(epics, page, per_page, preload_method, nil)
+ epics(page: page, per_page: per_page, preload_method: preload_method)
when 'blobs'
blobs(page: page, per_page: per_page, preload_method: preload_method)
when 'wiki_blobs'
@@ -333,6 +333,16 @@ def scope_options(scope)
# Otherwise it will ignore project_ids and return milestones
# from projects with milestones disabled.
base_options.merge({ features: [:issues, :merge_requests] }, filters.slice(:include_archived))
+ when :epics
+ if work_item_index_available_for_searching_epics?
+ work_item_scope_options.merge(
+ not_work_item_type_ids: nil,
+ klass: WorkItem,
+ work_item_type_ids: [::WorkItems::Type.find_by_name(::WorkItems::Type::TYPE_NAMES[:epic]).id]
+ )
+ else
+ base_options
+ end
when :users
base_options.merge(admin: current_user&.admin?, routing_disabled: true) # rubocop:disable Cop/UserAdmin
when :blobs
@@ -373,7 +383,7 @@ def projects(count_only: false)
def issues(page: 1, per_page: DEFAULT_PER_PAGE, count_only: false, preload_method: nil)
strong_memoize(memoize_key('issues', count_only: count_only)) do
- if work_item_index_available_for_searching?
+ if work_item_index_available_for_searching_issues?
options = scope_options(:work_items)
.merge(count_only: count_only, per_page: per_page, page: page, preload_method: preload_method)
search_query = ::Search::Elastic::WorkItemQueryBuilder.build(query: query, options: options)
@@ -389,7 +399,12 @@ def issues(page: 1, per_page: DEFAULT_PER_PAGE, count_only: false, preload_metho
end
end
- def work_item_index_available_for_searching?
+ def work_item_index_available_for_searching_epics?
+ ::Feature.enabled?(:search_epics_uses_work_items_index, current_user) &&
+ ::Elastic::DataMigrationService.migration_has_finished?(:backfill_work_items)
+ end
+
+ def work_item_index_available_for_searching_issues?
::Feature.enabled?(:search_issues_uses_work_items_index, current_user) &&
::Elastic::DataMigrationService.migration_has_finished?(:backfill_work_items)
end
@@ -406,8 +421,20 @@ def notes(count_only: false)
scope_results :notes, Note, count_only: count_only
end
- def epics(count_only: false)
- scope_results :epics, Epic, count_only: count_only
+ def epics(page: 1, per_page: DEFAULT_PER_PAGE, count_only: false, preload_method: nil)
+ unless work_item_index_available_for_searching_epics?
+ epics = scope_results :epics, Epic, count_only: count_only
+ return eager_load(epics, page, per_page, preload_method, nil)
+ end
+
+ strong_memoize(memoize_key('epics', count_only: count_only)) do
+ options = scope_options(:epics)
+ .merge(count_only: count_only, per_page: per_page, page: page, preload_method: preload_method)
+ search_query = ::Search::Elastic::WorkItemGroupQueryBuilder.build(query: query, options: options)
+ ::Gitlab::Search::Client.execute_search(query: search_query, options: options) do |response|
+ ::Search::Elastic::ResponseMapper.new(response, options).paginated_array
+ end
+ end
end
def users(page: 1, per_page: DEFAULT_PER_PAGE, count_only: false, preload_method: nil)
@@ -486,7 +513,7 @@ def blob_aggregations
strong_memoize_attr :blob_aggregations
def issue_aggregations
- if work_item_index_available_for_searching?
+ if work_item_index_available_for_searching_issues?
options = scope_options(:work_items).merge(aggregation: true)
search_query = ::Search::Elastic::WorkItemQueryBuilder.build(query: query, options: options)
else
diff --git a/ee/lib/search/elastic/filters.rb b/ee/lib/search/elastic/filters.rb
index dcf35f95a1945..a4c5c82bc6497 100644
--- a/ee/lib/search/elastic/filters.rb
+++ b/ee/lib/search/elastic/filters.rb
@@ -231,6 +231,82 @@ def by_label_ids(query_hash:, options:)
end
end
+ def by_group_level_confidentiality(query_hash:, options:)
+ user = options[:current_user]
+ query_hash = search_level_filter(query_hash: query_hash, options: options)
+ return query_hash if user.nil? || user&.can_read_all_resources?
+
+ confidential_group_ids = group_ids_user_has_min_access_as(
+ access_level: ::Gitlab::Access::REPORTER, user: options[:current_user], group_ids: options[:group_ids]
+ )
+
+ return query_hash if confidential_group_ids.empty?
+
+ context.name(:filters) do
+ add_filter(query_hash, :query, :bool, :filter) do
+ { bool: { should: [
+ {
+ bool: {
+ must: [
+ { term: { confidential: { value: true, _name: context.name(:confidential, :groups) } } },
+ { terms: { namespace_id: confidential_group_ids,
+ _name: context.name(:confidential, :groups, "can_read_confidential_work_items") } }
+ ]
+ }
+ },
+ { term: { confidential: { value: false, _name: context.name(:non_confidential, :groups) } } }
+ ] } }
+ end
+ end
+ end
+
+ def by_group_level_authorization(query_hash:, options:)
+ user = options[:current_user]
+ query_hash = search_level_filter(query_hash: query_hash, options: options)
+ return query_hash if user&.can_read_all_resources?
+
+ context.name(:filters) do
+ add_filter(query_hash, :query, :bool, :filter) do
+ visibility_filters = Search::Elastic::BoolExpr.new
+ visibility_filters.minimum_should_match = 1
+ add_filter(visibility_filters, :should) do
+ { bool: { filter: [
+ { term: { namespace_visibility_level: { value: ::Gitlab::VisibilityLevel::PUBLIC,
+ _name: context.name(:namespace_visibility_level,
+ :public) } } }
+ ] } }
+ end
+
+ if user && !user.external?
+ add_filter(visibility_filters, :should) do
+ { bool: { filter: [
+ { term: { namespace_visibility_level: { value: ::Gitlab::VisibilityLevel::INTERNAL,
+ _name: context.name(:namespace_visibility_level,
+ :internal) } } }
+ ] } }
+ end
+ authorized_group_ids = group_ids_user_has_min_access_as(access_level: ::Gitlab::Access::GUEST,
+ user: user, group_ids: options[:group_ids])
+
+ unless authorized_group_ids.empty?
+ add_filter(visibility_filters, :should) do
+ { bool: { filter: [
+ { term: { namespace_visibility_level: { value: ::Gitlab::VisibilityLevel::PRIVATE,
+ _name: context.name(:namespace_visibility_level,
+ :private) } } },
+ { terms: { namespace_id: authorized_group_ids } }
+
+ ] } }
+ end
+ end
+
+ end
+
+ { bool: visibility_filters.to_h }
+ end
+ end
+ end
+
def by_confidentiality(query_hash:, options:)
confidential = options[:confidential]
user = options[:current_user]
@@ -356,6 +432,16 @@ def add_filter(query_hash, *path)
query_hash
end
+ def group_ids_user_has_min_access_as(access_level:, user:, group_ids:)
+ finder_params = { min_access_level: access_level }
+ if group_ids.present?
+ finder_params[:filter_group_ids] =
+ Group.id_in(group_ids.uniq).map(&:self_and_descendants_ids).uniq
+ end
+
+ ::GroupsFinder.new(user, finder_params).execute.pluck("#{Group.table_name}.#{Group.primary_key}") # rubocop:disable CodeReuse/ActiveRecord -- we need pluck only the ids from the finder
+ end
+
def scoped_project_ids(current_user, project_ids)
return :any if project_ids == :any
diff --git a/ee/lib/search/elastic/work_item_group_query_builder.rb b/ee/lib/search/elastic/work_item_group_query_builder.rb
new file mode 100644
index 0000000000000..da7ed7d2c957b
--- /dev/null
+++ b/ee/lib/search/elastic/work_item_group_query_builder.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module Search
+ module Elastic
+ class WorkItemGroupQueryBuilder < ::Search::Elastic::WorkItemQueryBuilder
+ extend ::Gitlab::Utils::Override
+
+ private
+
+ override :extra_options
+ def extra_options
+ super.merge({
+ features: nil,
+ group_level_authorization: true,
+ group_level_confidentiality: true
+ })
+ end
+ end
+ end
+end
diff --git a/ee/lib/search/elastic/work_item_query_builder.rb b/ee/lib/search/elastic/work_item_query_builder.rb
index 2e4fedf57f878..55a709cfeaac7 100644
--- a/ee/lib/search/elastic/work_item_query_builder.rb
+++ b/ee/lib/search/elastic/work_item_query_builder.rb
@@ -24,8 +24,19 @@ def build
end
end
- query_hash = ::Search::Elastic::Filters.by_authorization(query_hash: query_hash, options: options)
- query_hash = ::Search::Elastic::Filters.by_confidentiality(query_hash: query_hash, options: options)
+ query_hash = if options[:group_level_authorization]
+ ::Search::Elastic::Filters.by_group_level_authorization(query_hash: query_hash, options: options)
+ else
+ ::Search::Elastic::Filters.by_authorization(query_hash: query_hash, options: options)
+ end
+
+ query_hash = if options[:group_level_confidentiality]
+ ::Search::Elastic::Filters.by_group_level_confidentiality(query_hash: query_hash,
+ options: options)
+ else
+ ::Search::Elastic::Filters.by_confidentiality(query_hash: query_hash, options: options)
+ end
+
query_hash = ::Search::Elastic::Filters.by_state(query_hash: query_hash, options: options)
query_hash = ::Search::Elastic::Filters.by_not_hidden(query_hash: query_hash, options: options)
query_hash = ::Search::Elastic::Filters.by_label_ids(query_hash: query_hash, options: options)
diff --git a/ee/spec/features/search/elastic/group_search_spec.rb b/ee/spec/features/search/elastic/group_search_spec.rb
index b98b88dd1205f..63fe3f22871cb 100644
--- a/ee/spec/features/search/elastic/group_search_spec.rb
+++ b/ee/spec/features/search/elastic/group_search_spec.rb
@@ -29,7 +29,13 @@ def choose_group(group)
stub_feature_flags(search_issues_uses_work_items_index: (document_type == :work_item))
project.repository.index_commits_and_blobs
stub_licensed_features(epics: true, group_wikis: true)
- create(:epic, group: group, title: 'chosen epic title')
+ stub_feature_flags(search_epics_uses_work_items_index: (document_type == :work_item))
+ if document_type == :work_item # rubocop:disable RSpec/AvoidConditionalStatements -- We need to create objects based on document type.
+ create(:work_item, :group_level, :epic_with_legacy_epic, namespace: group, title: 'chosen epic title')
+ else
+ create(:epic, group: group, title: 'chosen epic title')
+ end
+
Sidekiq::Worker.skipping_transaction_check do
[group_wiki, wiki].each do |w|
w.create_page('test.md', '# term')
diff --git a/ee/spec/features/search/user_searches_for_epics_spec.rb b/ee/spec/features/search/user_searches_for_epics_spec.rb
index 1746c22d56249..d6dcb84a0d08e 100644
--- a/ee/spec/features/search/user_searches_for_epics_spec.rb
+++ b/ee/spec/features/search/user_searches_for_epics_spec.rb
@@ -2,11 +2,9 @@
require 'spec_helper'
-RSpec.describe 'User searches for epics', :elastic, :sidekiq_inline, :js, :disable_rate_limiter, feature_category: :global_search do
+RSpec.describe 'User searches for epics', :js, :disable_rate_limiter, feature_category: :global_search do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
- let_it_be(:epic1) { create(:epic, title: 'Foo', group: group, updated_at: 6.days.ago) }
- let_it_be(:epic2) { create(:epic, :closed, :confidential, title: 'Bar', group: group) }
before do
stub_licensed_features(epics: true)
@@ -18,9 +16,17 @@
visit(search_path(group_id: group.id))
end
- include_examples 'top right search form'
- include_examples 'search timeouts', 'epics' do
- let(:additional_params) { { group_id: group.id } }
+ [:work_item, :epic].each do |document_type|
+ context "when we have document_type as #{document_type}", :elastic do
+ before do
+ stub_feature_flags(search_epics_uses_work_items_index: (document_type == :work_item))
+ end
+
+ include_examples 'top right search form'
+ include_examples 'search timeouts', 'epics' do
+ let(:additional_params) { { group_id: group.id } }
+ end
+ end
end
shared_examples 'searches for epics' do
@@ -74,18 +80,41 @@
end
end
- context 'when advanced_search is enabled' do
+ context 'when advanced_search is enabled', :elastic do
+ let_it_be(:epic1) do
+ create(:work_item, :group_level, :epic_with_legacy_epic, title: 'Foo', namespace: group,
+ updated_at: 6.days.ago)
+ end
+
+ let_it_be(:epic2) do
+ create(:work_item, :group_level, :epic_with_legacy_epic, :closed, :confidential, title: 'Bar',
+ namespace: group)
+ end
+
before do
stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
-
+ stub_feature_flags(search_epics_uses_work_items_index: true)
Elastic::ProcessBookkeepingService.track!(*[epic1, epic2])
ensure_elasticsearch_index!
end
include_examples 'searches for epics'
+ context 'when ff is disabled' do
+ let_it_be(:epic1) { create(:epic, title: 'Foo', group: group, updated_at: 6.days.ago) }
+ let_it_be(:epic2) { create(:epic, :closed, :confidential, title: 'Bar', group: group) }
+
+ before do
+ stub_feature_flags(search_epics_uses_work_items_index: false)
+ end
+
+ include_examples 'searches for epics'
+ end
end
context 'when advanced_search is disabled' do
+ let_it_be(:epic1) { create(:epic, title: 'Foo', group: group, updated_at: 6.days.ago) }
+ let_it_be(:epic2) { create(:epic, :closed, :confidential, title: 'Bar', group: group) }
+
before do
stub_ee_application_setting(elasticsearch_search: false, elasticsearch_indexing: false)
end
diff --git a/ee/spec/lib/gitlab/elastic/group_search_results_spec.rb b/ee/spec/lib/gitlab/elastic/group_search_results_spec.rb
index 167635b47344d..1a051173e3c3d 100644
--- a/ee/spec/lib/gitlab/elastic/group_search_results_spec.rb
+++ b/ee/spec/lib/gitlab/elastic/group_search_results_spec.rb
@@ -236,6 +236,61 @@
end
end
+ context 'group level work_items search', :sidekiq_inline do
+ let(:query) { 'foo' }
+ let(:scope) { 'epics' }
+
+ let_it_be(:public_parent_group) { create(:group, :public) }
+ let_it_be(:group) { create(:group, :private, parent: public_parent_group) }
+ let_it_be(:child_group) { create(:group, :private, parent: group) }
+ let_it_be(:child_of_child_group) { create(:group, :private, parent: child_group) }
+ let_it_be(:another_group) { create(:group, :private, parent: public_parent_group) }
+ let!(:parent_group_epic) { create(:work_item, :group_level, :epic_with_legacy_epic, namespace: public_parent_group, title: query) }
+ let!(:group_epic) { create(:work_item, :group_level, :epic_with_legacy_epic, namespace: group, title: query) }
+ let!(:child_group_epic) { create(:work_item, :group_level, :epic_with_legacy_epic, namespace: child_group, title: query) }
+ let!(:confidential_child_group_epic) { create(:work_item, :group_level, :epic_with_legacy_epic, :confidential, namespace: child_group, title: query) }
+ let!(:confidential_child_of_child_epic) { create(:work_item, :group_level, :epic_with_legacy_epic, :confidential, namespace: child_of_child_group, title: query) }
+ let!(:another_group_epic) { create(:work_item, :group_level, :epic_with_legacy_epic, namespace: another_group, title: query) }
+
+ before do
+ ensure_elasticsearch_index!
+ end
+
+ it 'returns no epics' do
+ expect(results.objects('epics')).to be_empty
+ end
+
+ context 'when the user is a developer on the group' do
+ before_all do
+ group.add_developer(user)
+ end
+
+ it 'returns matching epics belonging to the group or its descendants, including confidential epics' do
+ epics = results.objects('epics')
+
+ expect(epics).to include(group_epic)
+ expect(epics).to include(child_group_epic)
+ expect(epics).to include(confidential_child_group_epic)
+
+ expect(epics).not_to include(parent_group_epic)
+ expect(epics).not_to include(another_group_epic)
+ end
+
+ context 'when searching from the child group' do
+ it 'returns matching epics belonging to the child group, including confidential epics' do
+ epics = described_class.new(user, query, [], group: child_group, filters: filters).objects('epics')
+
+ expect(epics).to include(child_group_epic)
+ expect(epics).to include(confidential_child_group_epic)
+
+ expect(epics).not_to include(group_epic)
+ expect(epics).not_to include(parent_group_epic)
+ expect(epics).not_to include(another_group_epic)
+ end
+ end
+ end
+ end
+
context 'epics search', :sidekiq_inline do
let(:query) { 'foo' }
let(:scope) { 'epics' }
@@ -253,6 +308,7 @@
let!(:another_group_epic) { create(:epic, group: another_group, title: query) }
before do
+ stub_feature_flags(search_epics_uses_work_items_index: false)
ensure_elasticsearch_index!
end
diff --git a/ee/spec/lib/search/elastic/filters_spec.rb b/ee/spec/lib/search/elastic/filters_spec.rb
index 76f44a8f5bb79..eb686deb7034b 100644
--- a/ee/spec/lib/search/elastic/filters_spec.rb
+++ b/ee/spec/lib/search/elastic/filters_spec.rb
@@ -767,6 +767,182 @@
end
end
+ describe '.by_group_level_authorization' do
+ subject(:by_group_level_authorization) do
+ described_class.by_group_level_authorization(query_hash: query_hash, options: options)
+ end
+
+ context 'when user.can_read_all_resources? is true' do
+ let(:base_options) { { current_user: user, search_level: 'global' } }
+ let(:options) { base_options }
+
+ before do
+ allow(user).to receive(:can_read_all_resources?).and_return(true)
+ end
+
+ it_behaves_like 'does not modify the query_hash'
+ end
+
+ context 'when user is having permission for the group' do
+ let_it_be(:group) { create(:group, :private) }
+ let(:base_options) { { current_user: user, search_level: 'group', group_id: group.id, group_ids: [group.id] } }
+ let(:options) { base_options }
+
+ before_all do
+ group.add_developer(user)
+ end
+
+ it 'shows private filter' do
+ expected_filter = [
+ { bool: { _name: "filters:level:group", minimum_should_match: 1,
+ should: [{ prefix: {
+ traversal_ids: {
+ _name: "filters:level:group:ancestry_filter:descendants",
+ value: group.elastic_namespace_ancestry
+ }
+ } }] } },
+ { bool: {
+ minimum_should_match: 1,
+ should: [
+ { bool: { filter: [
+ { term: { namespace_visibility_level: {
+ _name: 'filters:namespace_visibility_level:public', value: ::Gitlab::VisibilityLevel::PUBLIC
+ } } }
+ ] } },
+ { bool: { filter: [{ term: { namespace_visibility_level: {
+ _name: 'filters:namespace_visibility_level:internal', value: ::Gitlab::VisibilityLevel::INTERNAL
+ } } }] } },
+ { bool: { filter: [
+ { term: {
+ namespace_visibility_level: { _name: 'filters:namespace_visibility_level:private',
+ value: ::Gitlab::VisibilityLevel::PRIVATE }
+ } },
+ { terms: { namespace_id: [group.id] } }
+ ] } }
+ ]
+ } }
+ ]
+
+ expect(by_group_level_authorization.dig(:query, :bool, :filter)).to match(expected_filter)
+ expect(by_group_level_authorization.dig(:query, :bool, :must)).to be_empty
+ expect(by_group_level_authorization.dig(:query, :bool, :must_not)).to be_empty
+ expect(by_group_level_authorization.dig(:query, :bool, :should)).to be_empty
+ end
+ end
+
+ context 'when user is nil' do
+ let(:options) { base_options }
+ let(:base_options) { { current_user: nil, search_level: 'global' } }
+
+ it 'shows only the public filter' do
+ expected_filter = [
+ bool: {
+ minimum_should_match: 1,
+ should: [
+ { bool: { filter: [{ term: { namespace_visibility_level: {
+ _name: 'filters:namespace_visibility_level:public', value: ::Gitlab::VisibilityLevel::PUBLIC
+ } } }] } }
+ ]
+ }
+ ]
+
+ expect(by_group_level_authorization.dig(:query, :bool, :filter)).to match(expected_filter)
+ expect(by_group_level_authorization.dig(:query, :bool, :must)).to be_empty
+ expect(by_group_level_authorization.dig(:query, :bool, :must_not)).to be_empty
+ expect(by_group_level_authorization.dig(:query, :bool, :should)).to be_empty
+ end
+ end
+
+ context 'when user is not having permissions to read confidential epics' do
+ let(:options) { base_options }
+ let(:base_options) { { current_user: user, search_level: 'global' } }
+
+ it 'shows only the public filter' do
+ expected_filter = [
+ bool: {
+ minimum_should_match: 1,
+ should: [
+ { bool: { filter: [{ term: { namespace_visibility_level: {
+ _name: 'filters:namespace_visibility_level:public', value: ::Gitlab::VisibilityLevel::PUBLIC
+ } } }] } },
+ { bool: { filter: [{ term: { namespace_visibility_level: {
+ _name: 'filters:namespace_visibility_level:internal', value: ::Gitlab::VisibilityLevel::INTERNAL
+ } } }] } }
+ ]
+ }
+ ]
+
+ expect(by_group_level_authorization.dig(:query, :bool, :filter)).to match(expected_filter)
+ expect(by_group_level_authorization.dig(:query, :bool, :must)).to be_empty
+ expect(by_group_level_authorization.dig(:query, :bool, :must_not)).to be_empty
+ expect(by_group_level_authorization.dig(:query, :bool, :should)).to be_empty
+ end
+ end
+ end
+
+ describe '.by_group_level_confidentiality' do
+ subject(:by_group_level_confidentiality) do
+ described_class.by_group_level_confidentiality(query_hash: query_hash, options: options)
+ end
+
+ context 'when user.can_read_all_resources? is true' do
+ let(:base_options) { { current_user: user, search_level: 'global' } }
+ let(:options) { base_options }
+
+ before do
+ allow(user).to receive(:can_read_all_resources?).and_return(true)
+ end
+
+ it_behaves_like 'does not modify the query_hash'
+ end
+
+ context 'when user is having permission for the group' do
+ let_it_be(:group) { create(:group, :private) }
+ let(:base_options) { { current_user: user, search_level: 'global' } }
+ let(:options) { base_options }
+
+ before_all do
+ group.add_developer(user)
+ end
+
+ it 'shows the expected filter' do
+ expected_filter = [
+ bool: { should: [
+ {
+ bool: {
+ must: [
+ { term: { confidential: { value: true, _name: "filters:confidential:groups" } } },
+ { terms: { namespace_id: [group.id],
+ _name: "filters:confidential:groups:can_read_confidential_work_items" } }
+ ]
+ }
+ },
+ { term: { confidential: { value: false, _name: "filters:non_confidential:groups" } } }
+ ] }
+ ]
+
+ expect(by_group_level_confidentiality.dig(:query, :bool, :filter)).to match(expected_filter)
+ expect(by_group_level_confidentiality.dig(:query, :bool, :must)).to be_empty
+ expect(by_group_level_confidentiality.dig(:query, :bool, :must_not)).to be_empty
+ expect(by_group_level_confidentiality.dig(:query, :bool, :should)).to be_empty
+ end
+ end
+
+ context 'when user is nil' do
+ let(:options) { base_options }
+ let(:base_options) { { current_user: nil, search_level: 'global' } }
+
+ it_behaves_like 'does not modify the query_hash'
+ end
+
+ context 'when user is not having permissions to read confidential epics' do
+ let(:options) { base_options }
+ let(:base_options) { { current_user: user, search_level: 'global' } }
+
+ it_behaves_like 'does not modify the query_hash'
+ end
+ end
+
describe '.by_confidentiality' do
let_it_be(:authorized_project) { create(:project, developers: [user]) }
let_it_be(:private_project) { create(:project, :private) }
diff --git a/ee/spec/services/search/global_service_spec.rb b/ee/spec/services/search/global_service_spec.rb
index 9bbdb8b194675..b5134159e5f40 100644
--- a/ee/spec/services/search/global_service_spec.rb
+++ b/ee/spec/services/search/global_service_spec.rb
@@ -200,8 +200,12 @@
context 'epic' do
let(:scope) { 'epics' }
- let!(:epic) { create :epic, group: group }
- let(:search) { epic.title }
+ let(:search) { 'chosen epic title' }
+ let!(:epic) { create(:work_item, :group_level, :epic_with_legacy_epic, namespace: group, title: 'chosen epic title') }
+
+ before do
+ ensure_elasticsearch_index!
+ end
where(:group_level, :membership, :admin_mode, :expected_count) do
permission_table_for_epics_access
@@ -218,6 +222,30 @@
end
end
end
+ context "when we have ff disabled" do
+ let!(:epic) { create(:epic, group: group, title: 'chosen epic title') }
+
+ before do
+ stub_feature_flags(search_epics_uses_work_items_index: false)
+ ensure_elasticsearch_index!
+ end
+
+ where(:group_level, :membership, :admin_mode, :expected_count) do
+ permission_table_for_epics_access
+ end
+
+ with_them do
+ it 'respects visibility' do
+ enable_admin_mode!(user_in_group) if admin_mode
+
+ group.update!(visibility_level: Gitlab::VisibilityLevel.level_value(group_level.to_s))
+ ensure_elasticsearch_index!
+ expect_search_results(user_in_group, scope, expected_count: expected_count) do |user|
+ described_class.new(user, search: search).execute
+ end
+ end
+ end
+ end
end
context 'wiki' do
diff --git a/ee/spec/services/search/group_service_spec.rb b/ee/spec/services/search/group_service_spec.rb
index 2041ebfcf1215..64f7914de249f 100644
--- a/ee/spec/services/search/group_service_spec.rb
+++ b/ee/spec/services/search/group_service_spec.rb
@@ -605,21 +605,65 @@
end
end
+ context 'for group level work_items' do
+ let(:scope) { 'epics' }
+ let_it_be(:member) { create(:group_member, :owner, group: group, user: user) }
+
+ let_it_be(:old_result) do
+ create(:work_item, :group_level, :epic_with_legacy_epic, namespace: group, title: 'sorted old',
+ created_at: 1.month.ago)
+ end
+
+ let_it_be(:new_result) do
+ create(:work_item, :group_level, :epic_with_legacy_epic, namespace: group, title: 'sorted recent',
+ created_at: 1.day.ago)
+ end
+
+ let_it_be(:very_old_result) do
+ create(:work_item, :group_level, :epic_with_legacy_epic, namespace: group, title: 'sorted very old',
+ created_at: 1.year.ago)
+ end
+
+ let_it_be(:old_updated) do
+ create(:work_item, :group_level, :epic_with_legacy_epic, namespace: group, title: 'updated old',
+ updated_at: 1.month.ago)
+ end
+
+ let_it_be(:new_updated) do
+ create(:work_item, :group_level, :epic_with_legacy_epic, namespace: group, title: 'updated recent',
+ updated_at: 1.day.ago)
+ end
+
+ let_it_be(:very_old_updated) do
+ create(:work_item, :group_level, :epic_with_legacy_epic, namespace: group, title: 'updated very old',
+ updated_at: 1.year.ago)
+ end
+
+ let(:results_created) { described_class.new(user, group, search: 'sorted', sort: sort).execute }
+ let(:results_updated) { described_class.new(user, group, search: 'updated', sort: sort).execute }
+
+ before do
+ stub_licensed_features(epics: true)
+ Elastic::ProcessInitialBookkeepingService.track!(old_result, new_result, very_old_result,
+ old_updated, new_updated, very_old_updated
+ )
+ ensure_elasticsearch_index!
+ end
+
+ include_examples 'search results sorted'
+ end
+
context 'for epics' do
let(:scope) { 'epics' }
let_it_be(:member) { create(:group_member, :owner, group: group, user: user) }
let_it_be(:old_result) { create(:epic, group: group, title: 'sorted old', created_at: 1.month.ago) }
let_it_be(:new_result) { create(:epic, group: group, title: 'sorted recent', created_at: 1.day.ago) }
- let_it_be(:very_old_result) do
- create(:epic, group: group, title: 'sorted very old', created_at: 1.year.ago)
- end
+ let_it_be(:very_old_result) { create(:epic, group: group, title: 'sorted very old', created_at: 1.year.ago) }
let_it_be(:old_updated) { create(:epic, group: group, title: 'updated old', updated_at: 1.month.ago) }
let_it_be(:new_updated) { create(:epic, group: group, title: 'updated recent', updated_at: 1.day.ago) }
- let_it_be(:very_old_updated) do
- create(:epic, group: group, title: 'updated very old', updated_at: 1.year.ago)
- end
+ let_it_be(:very_old_updated) { create(:epic, group: group, title: 'updated very old', updated_at: 1.year.ago) }
let(:results_created) { described_class.new(user, group, search: 'sorted', sort: sort).execute }
let(:results_updated) { described_class.new(user, group, search: 'updated', sort: sort).execute }
@@ -629,6 +673,7 @@
Elastic::ProcessInitialBookkeepingService.track!(old_result, new_result, very_old_result,
old_updated, new_updated, very_old_updated
)
+ stub_feature_flags(search_epics_uses_work_items_index: false)
ensure_elasticsearch_index!
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 0f8b662aeec97..fd8e810ccbf5e 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -883,6 +883,9 @@ msgstr ""
msgid "%{group_name}&%{epic_iid} · created %{epic_created} by %{author}"
msgstr ""
+msgid "%{group_name}&%{work_item_iid} · created %{work_item_created} by %{author}"
+msgstr ""
+
msgid "%{groupsLength} group"
msgid_plural "%{groupsLength} groups"
msgstr[0] ""
--
GitLab