Commit d8ebcdb0 authored by Robert Speicher's avatar Robert Speicher

Merge branch...

Merge branch '35947-issues-no-results-in-issue-search-on-boards-view-when-search-criteria-is-1-or-2-letters' into 'master'

Include CTE search optimization in Boards search

Closes #35947

See merge request gitlab-org/gitlab!23366
parents 30901b84 dd6798a3
...@@ -20,6 +20,9 @@ module Boards ...@@ -20,6 +20,9 @@ module Boards
skip_before_action :authenticate_user!, only: [:index] skip_before_action :authenticate_user!, only: [:index]
before_action :validate_id_list, only: [:bulk_move] before_action :validate_id_list, only: [:bulk_move]
before_action :can_move_issues?, only: [:bulk_move] before_action :can_move_issues?, only: [:bulk_move]
before_action do
push_frontend_feature_flag(:board_search_optimization, board.group)
end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def index def index
......
...@@ -249,7 +249,7 @@ module Issuable ...@@ -249,7 +249,7 @@ module Issuable
Gitlab::Database.nulls_last_order('highest_priority', direction)) Gitlab::Database.nulls_last_order('highest_priority', direction))
end end
def order_labels_priority(direction = 'ASC', excluded_labels: [], extra_select_columns: []) def order_labels_priority(direction = 'ASC', excluded_labels: [], extra_select_columns: [], with_cte: false)
params = { params = {
target_type: name, target_type: name,
target_column: "#{table_name}.id", target_column: "#{table_name}.id",
...@@ -265,7 +265,7 @@ module Issuable ...@@ -265,7 +265,7 @@ module Issuable
] + extra_select_columns ] + extra_select_columns
select(select_columns.join(', ')) select(select_columns.join(', '))
.group(arel_table[:id]) .group(issue_grouping_columns(use_cte: with_cte))
.reorder(Gitlab::Database.nulls_last_order('highest_priority', direction)) .reorder(Gitlab::Database.nulls_last_order('highest_priority', direction))
end end
...@@ -294,6 +294,18 @@ module Issuable ...@@ -294,6 +294,18 @@ module Issuable
grouping_columns grouping_columns
end end
# Includes all table keys in group by clause when sorting
# preventing errors in postgres when using CTE search optimisation
#
# Returns an array of arel columns
def issue_grouping_columns(use_cte: false)
if use_cte
[arel_table[:state]] + attribute_names.map { |attr| arel_table[attr.to_sym] }
else
arel_table[:id]
end
end
def to_ability_name def to_ability_name
model_name.singular model_name.singular
end end
......
...@@ -172,8 +172,10 @@ class Issue < ApplicationRecord ...@@ -172,8 +172,10 @@ class Issue < ApplicationRecord
end end
end end
def self.order_by_position_and_priority # `with_cte` argument allows sorting when using CTE queries and prevents
order_labels_priority # errors in postgres when using CTE search optimisation
def self.order_by_position_and_priority(with_cte: false)
order_labels_priority(with_cte: with_cte)
.reorder(Gitlab::Database.nulls_last_order('relative_position', 'ASC'), .reorder(Gitlab::Database.nulls_last_order('relative_position', 'ASC'),
Gitlab::Database.nulls_last_order('highest_priority', 'ASC'), Gitlab::Database.nulls_last_order('highest_priority', 'ASC'),
"id DESC") "id DESC")
......
...@@ -10,7 +10,7 @@ module Boards ...@@ -10,7 +10,7 @@ module Boards
end end
def execute def execute
fetch_issues.order_by_position_and_priority fetch_issues.order_by_position_and_priority(with_cte: can_attempt_search_optimization?)
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
...@@ -63,6 +63,7 @@ module Boards ...@@ -63,6 +63,7 @@ module Boards
set_state set_state
set_scope set_scope
set_non_archived set_non_archived
set_attempt_search_optimizations
params params
end end
...@@ -87,6 +88,16 @@ module Boards ...@@ -87,6 +88,16 @@ module Boards
params[:non_archived] = parent.is_a?(Group) params[:non_archived] = parent.is_a?(Group)
end end
def set_attempt_search_optimizations
return unless can_attempt_search_optimization?
if board.group_board?
params[:attempt_group_search_optimizations] = true
else
params[:attempt_project_search_optimizations] = true
end
end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def board_label_ids def board_label_ids
@board_label_ids ||= board.lists.movable.pluck(:label_id) @board_label_ids ||= board.lists.movable.pluck(:label_id)
...@@ -113,6 +124,15 @@ module Boards ...@@ -113,6 +124,15 @@ module Boards
.where("label_links.label_id = ?", list.label_id).limit(1)) .where("label_links.label_id = ?", list.label_id).limit(1))
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def board_group
board.group_board? ? parent : parent.group
end
def can_attempt_search_optimization?
params[:search].present? &&
Feature.enabled?(:board_search_optimization, board_group, default_enabled: false)
end
end end
end end
end end
......
...@@ -49,6 +49,40 @@ describe Boards::IssuesController do ...@@ -49,6 +49,40 @@ describe Boards::IssuesController do
expect(json_response['issues'].length).to eq 2 expect(json_response['issues'].length).to eq 2
expect(development.issues.map(&:relative_position)).not_to include(nil) expect(development.issues.map(&:relative_position)).not_to include(nil)
end end
context 'with search param' do
context 'when board_search_optimization is enabled' do
before do
stub_feature_flags(board_search_optimization: true)
end
it 'returns matching issues using optimized search' do
create(:labeled_issue, project: project_1, labels: [planning], title: 'Test Issue')
create(:labeled_issue, project: project_1, labels: [planning], title: 'Sample Issue')
list_issues user: user, board: board, list: list1, search: 'Te'
expect(response).to match_response_schema('issues')
expect(json_response['issues'].length).to eq 1
end
end
context 'when board_search_optimization is disabled' do
before do
stub_feature_flags(board_search_optimization: false)
end
it 'returns empty result' do
create(:labeled_issue, project: project_1, labels: [planning], title: 'Test Issue')
create(:labeled_issue, project: project_1, labels: [planning], title: 'Sample Issue')
list_issues user: user, board: board, list: list1, search: 'Te'
expect(response).to match_response_schema('issues')
expect(json_response['issues'].length).to eq 0
end
end
end
end end
context 'with invalid list id' do context 'with invalid list id' do
...@@ -73,6 +107,40 @@ describe Boards::IssuesController do ...@@ -73,6 +107,40 @@ describe Boards::IssuesController do
expect(response).to match_response_schema('issues') expect(response).to match_response_schema('issues')
expect(json_response['issues'].length).to eq 2 expect(json_response['issues'].length).to eq 2
end end
context 'with search param' do
context 'when board_search_optimization is enabled' do
before do
stub_feature_flags(board_search_optimization: true)
end
it 'returns matching issues using optimized search' do
create(:issue, project: project_1, title: 'Issue XI')
create(:issue, project: project_1, title: 'Issue XX')
list_issues user: user, board: board, search: 'XI'
expect(response).to match_response_schema('issues')
expect(json_response['issues'].length).to eq 1
end
end
context 'when board_search_optimization is disabled' do
before do
stub_feature_flags(board_search_optimization: false)
end
it 'returns empty result' do
create(:issue, project: project_1, title: 'Issue XI')
create(:issue, project: project_1, title: 'Issue XX')
list_issues user: user, board: board, search: 'XI'
expect(response).to match_response_schema('issues')
expect(json_response['issues'].length).to eq 0
end
end
end
end end
context 'with unauthorized user' do context 'with unauthorized user' do
...@@ -87,12 +155,13 @@ describe Boards::IssuesController do ...@@ -87,12 +155,13 @@ describe Boards::IssuesController do
end end
end end
def list_issues(user:, board:, list: nil) def list_issues(user:, board:, list: nil, search: nil)
sign_in(user) sign_in(user)
params = { params = {
board_id: board.to_param, board_id: board.to_param,
list_id: list.try(:to_param) list_id: list.try(:to_param),
search: search.try(:to_param)
} }
get :index, params: params.compact get :index, params: params.compact
......
...@@ -151,5 +151,46 @@ describe Boards::Issues::ListService, services: true do ...@@ -151,5 +151,46 @@ describe Boards::Issues::ListService, services: true do
end end
end end
end end
context 'when search param is present' do
shared_examples 'returns correct result using 3 characters' do
it 'returns correct issues' do
params = { board_id: board.id, search: 'Iss' }
issues = described_class.new(parent, user, params).execute
expect(issues).to contain_exactly(opened_issue1, opened_issue2, reopened_issue1)
end
end
context 'when board_search_optimization feature is enabled' do
before do
stub_feature_flags(board_search_optimization: true)
end
it_behaves_like 'returns correct result using 3 characters'
it 'returns correct issues using 2 characters' do
params = { board_id: board.id, search: 'Is' }
issues = described_class.new(parent, user, params).execute
expect(issues).to contain_exactly(opened_issue1, opened_issue2, reopened_issue1)
end
end
context 'when board_search_optimization feature is disabled' do
before do
stub_feature_flags(board_search_optimization: false)
end
it_behaves_like 'returns correct result using 3 characters'
it 'returns empty result using 2 characters' do
params = { board_id: board.id, search: 'Is' }
issues = described_class.new(parent, user, params).execute
expect(issues).to be_empty
end
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment