Commit 7d128591 authored by mbergeron's avatar mbergeron

Improve the ElasticSearch settings autocomplete

The new approach uses `Route.path` filtering instead of trying to
search through all of the available fields of the entity we looking
for.

It makes the results more predictable and ensure a perfect match is
always on top, which makes sense in this situation.
parent 2220ceea
# frozen_string_literal: true
module Autocomplete
# Finder that returns a list of routes that match on the `path` attribute.
class RoutesFinder
attr_reader :current_user, :search
LIMIT = 20
def initialize(current_user, params = {})
@current_user = current_user
@search = params[:search]
end
def execute
return [] if @search.blank?
Route
.for_routable(routables)
.sort_by_path_length
.fuzzy_search(@search, [:path])
.limit(LIMIT) # rubocop: disable CodeReuse/ActiveRecord
end
private
def routables
raise NotImplementedError
end
class NamespacesOnly < self
def routables
return Namespace.all if current_user.admin?
current_user.namespaces
end
end
class ProjectsOnly < self
def routables
return Project.all if current_user.admin?
current_user.projects
end
end
end
end
......@@ -2,9 +2,9 @@
class Route < ApplicationRecord
include CaseSensitivity
include Gitlab::SQL::Pattern
belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
validates :source, presence: true
validates :path,
......@@ -19,6 +19,8 @@ class Route < ApplicationRecord
after_update :rename_descendants
scope :inside_path, -> (path) { where('routes.path LIKE ?', "#{sanitize_sql_like(path)}/%") }
scope :for_routable, -> (routable) { where(source: routable) }
scope :sort_by_path_length, -> { order('LENGTH(routes.path)', :path) }
def rename_descendants
return unless saved_change_to_path? || saved_change_to_name?
......
# frozen_string_literal: true
class RouteEntity < Grape::Entity
expose :id
expose :source_id
expose :source_type
expose :path
end
# frozen_string_literal: true
class RouteSerializer < BaseSerializer
entity RouteEntity
end
......@@ -53,6 +53,8 @@ Rails.application.routes.draw do
Gitlab.ee do
get '/autocomplete/project_groups' => 'autocomplete#project_groups'
get '/autocomplete/project_routes' => 'autocomplete#project_routes'
get '/autocomplete/namespace_routes' => 'autocomplete#namespace_routes'
end
# Sign up
......
......@@ -8573,6 +8573,8 @@ CREATE UNIQUE INDEX design_management_designs_versions_uniqueness ON public.desi
CREATE INDEX design_user_mentions_on_design_id_and_note_id_index ON public.design_user_mentions USING btree (design_id, note_id);
CREATE INDEX dev_index_route_on_path_trigram ON public.routes USING gin (path public.gin_trgm_ops);
CREATE UNIQUE INDEX epic_user_mentions_on_epic_id_and_note_id_index ON public.epic_user_mentions USING btree (epic_id, note_id);
CREATE UNIQUE INDEX epic_user_mentions_on_epic_id_index ON public.epic_user_mentions USING btree (epic_id) WHERE (note_id IS NULL);
......
import 'select2/select2';
import $ from 'jquery';
import { s__ } from '~/locale';
import Api from '~/api';
import PersistentUserCallout from '~/persistent_user_callout';
const onLimitCheckboxChange = (checked, $limitByNamespaces, $limitByProjects) => {
......@@ -11,14 +10,14 @@ const onLimitCheckboxChange = (checked, $limitByNamespaces, $limitByProjects) =>
$limitByProjects.toggleClass('hidden', !checked);
};
const getDropdownConfig = (placeholder, apiPath, textProp) => ({
const getDropdownConfig = (placeholder, url) => ({
placeholder,
multiple: true,
initSelection($el, callback) {
callback($el.data('selected'));
},
ajax: {
url: Api.buildUrl(apiPath),
url,
dataType: 'JSON',
quietMillis: 250,
data(search) {
......@@ -29,8 +28,8 @@ const getDropdownConfig = (placeholder, apiPath, textProp) => ({
results(data) {
return {
results: data.map(entity => ({
id: entity.id,
text: entity[textProp],
id: entity.source_id,
text: entity.path,
})),
};
},
......@@ -59,8 +58,7 @@ document.addEventListener('DOMContentLoaded', () => {
.select2(
getDropdownConfig(
s__('Elastic|None. Select namespaces to index.'),
Api.namespacesPath,
'full_path',
'/autocomplete/namespace_routes.json',
),
);
......@@ -69,8 +67,7 @@ document.addEventListener('DOMContentLoaded', () => {
.select2(
getDropdownConfig(
s__('Elastic|None. Select projects to index.'),
Api.projectsPath,
'name_with_namespace',
'/autocomplete/project_routes.json',
),
);
});
......@@ -9,5 +9,21 @@ module EE
render json: InvitedGroupSerializer.new.represent(groups)
end
def project_routes
routes = ::Autocomplete::RoutesFinder::ProjectsOnly
.new(current_user, params)
.execute
render json: RouteSerializer.new.represent(routes)
end
def namespace_routes
routes = ::Autocomplete::RoutesFinder::NamespacesOnly
.new(current_user, params)
.execute
render json: RouteSerializer.new.represent(routes)
end
end
end
......@@ -56,7 +56,7 @@ module EE
end
def elasticsearch_objects_options(objects)
objects.map { |g| { id: g.id, text: g.full_name } }
objects.map { |g| { id: g.id, text: g.full_path } }
end
# The admin UI cannot handle so many namespaces so we just hide it. We
......
......@@ -19,7 +19,7 @@ class ElasticsearchIndexedNamespace < ApplicationRecord
end
def self.limited(ignore_descendants: false)
namespaces = Namespace.where(id: target_ids)
namespaces = Namespace.with_route.where(id: target_ids)
return namespaces if ignore_descendants
......
......@@ -15,9 +15,9 @@ class ElasticsearchIndexedProject < ApplicationRecord
end
def self.limited(ignore_namespaces: false)
return Project.where(id: target_ids) if ignore_namespaces
return Project.inc_routes.where(id: target_ids) if ignore_namespaces
Project.from_union(
Project.inc_routes.from_union(
[
Project.where(namespace_id: ElasticsearchIndexedNamespace.limited.select(:id)),
Project.where(id: target_ids)
......
---
title: Fix missing autocomplete results in the ElasticSearch admin UI.
merge_request: 29043
author: mbergeron
type: fixed
......@@ -57,7 +57,7 @@ describe AutocompleteController do
end
end
context "groups" do
context 'groups' do
let(:matching_group) { create(:group) }
let(:non_matching_group) { create(:group) }
let(:user2) { create(:user) }
......@@ -97,4 +97,133 @@ describe AutocompleteController do
it { expect(response).to be_not_found }
end
end
shared_examples 'has expected results' do
it 'returns the matching routes', :aggregate_failures do
expect(json_response).to be_kind_of(Array)
expect(json_response.size).to eq(expected_results.length)
json_response.each do |result|
expect(expected_results).to include(result.values_at('source_id', 'source_type'))
end
end
end
context 'GET project_routes' do
let_it_be(:group) { create(:group) }
let_it_be(:projects) { create_list(:project, 3, group: group) }
before do
sign_in(user)
get(:project_routes, params: { search: search })
end
context 'as admin' do
let(:user) { create(:admin) }
describe "while searching for a project by namespace" do
let(:search) { group.path }
let!(:expected_results) { group.projects.map { |p| [p.id, 'Project'] }}
include_examples 'has expected results'
end
describe "while searching for a project by path" do
let(:search) { projects.first.path }
let!(:expected_results) { [[projects.first.id, 'Project']] }
include_examples 'has expected results'
end
end
context 'as project owner' do
let(:user) { project.owner }
let!(:expected_results) { [[project.id, 'Project']] }
context "while searching for a project by namespace" do
let(:search) { user.namespace.path }
include_examples 'has expected results'
end
context "while searching for a project by path" do
let(:search) { project.path }
include_examples 'has expected results'
end
end
context 'while searching for nothing' do
let(:search) { nil }
let(:expected_results) { [] }
include_examples 'has expected results'
end
end
context 'GET namespace_routes' do
let_it_be(:groups) { create_list(:group, 3, :private) }
let_it_be(:users) { create_list(:user, 3) }
before do
sign_in(user)
get(:namespace_routes, params: { search: search })
end
context 'as admin' do
let(:user) { create(:admin) }
describe "while searching for a namespace by group path" do
let(:search) { 'group' }
let!(:expected_results) do
Group.all.map { |g| [g.id, 'Namespace'] }
end
include_examples 'has expected results'
end
describe "while searching for a namespace by user path" do
let(:search) { 'user' }
let!(:expected_results) do
User.all.map { |u| [u.namespace.id, 'Namespace'] }
end
include_examples 'has expected results'
end
end
context 'as a user' do
let(:search) { user.namespace.path }
context "while searching for a namespace by path" do
let!(:expected_results) { [[user.namespace.id, 'Namespace']] }
include_examples 'has expected results'
end
end
context 'as group member' do
let_it_be(:group_developer) do
groups.first.add_developer(users.first)
users.first
end
let(:search) { groups.first.path }
let(:user) { group_developer }
context "while searching for a namespace by path" do
let!(:expected_results) { [[groups.first.id, 'Namespace']] }
include_examples 'has expected results'
end
end
context 'while searching for nothing' do
let(:search) { nil }
let(:expected_results) { [] }
include_examples 'has expected results'
end
end
end
......@@ -102,7 +102,7 @@ describe 'Admin updates EE-only settings' do
expect(page).to have_content('Namespaces to index')
expect(page).to have_content('Projects to index')
fill_in 'Namespaces to index', with: namespace.name
fill_in 'Namespaces to index', with: namespace.path
wait_for_requests
end
......@@ -113,12 +113,12 @@ describe 'Admin updates EE-only settings' do
page.within('.as-elasticsearch') do
find('.js-limit-namespaces .select2-choices input[type=text]').native.send_keys(:enter)
fill_in 'Projects to index', with: project.name
fill_in 'Projects to index', with: project.path
wait_for_requests
end
page.within('#select2-drop') do
expect(page).to have_content(project.full_name)
expect(page).to have_content(project.full_path)
end
page.within('.as-elasticsearch') do
......@@ -148,14 +148,14 @@ describe 'Admin updates EE-only settings' do
expect(page).to have_content('Namespaces to index')
expect(page).to have_content('Projects to index')
expect(page).to have_content(namespace.full_name)
expect(page).to have_content(project.full_name)
expect(page).to have_content(namespace.full_path)
expect(page).to have_content(project.full_path)
find('.js-limit-namespaces .select2-search-choice-close').click
find('.js-limit-projects .select2-search-choice-close').click
expect(page).not_to have_content(namespace.full_name)
expect(page).not_to have_content(project.full_name)
expect(page).not_to have_content(namespace.full_path)
expect(page).not_to have_content(project.full_path)
click_button 'Save changes'
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