Commit 7a26a7c1 authored by Jan Provaznik's avatar Jan Provaznik

Merge branch '332889-group-search-sort-based-on-algorithmic-score' into 'master'

Add similarity sort to Groups API search

See merge request gitlab-org/gitlab!63674
parents bb993483 87abfed2
...@@ -116,6 +116,14 @@ class Namespace < ApplicationRecord ...@@ -116,6 +116,14 @@ class Namespace < ApplicationRecord
) )
end end
scope :sorted_by_similarity_and_parent_id_desc, -> (search) do
order_expression = Gitlab::Database::SimilarityScore.build_expression(search: search, rules: [
{ column: arel_table["path"], multiplier: 1 },
{ column: arel_table["name"], multiplier: 0.7 }
])
reorder(order_expression.desc, Namespace.arel_table['parent_id'].desc.nulls_last, Namespace.arel_table['id'].desc)
end
# Make sure that the name is same as strong_memoize name in root_ancestor # Make sure that the name is same as strong_memoize name in root_ancestor
# method # method
attr_writer :root_ancestor, :emails_disabled_memoized attr_writer :root_ancestor, :emails_disabled_memoized
......
...@@ -20,7 +20,7 @@ Parameters: ...@@ -20,7 +20,7 @@ Parameters:
| `skip_groups` | array of integers | no | Skip the group IDs passed | | `skip_groups` | array of integers | no | Skip the group IDs passed |
| `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users, `true` for administrators); Attributes `owned` and `min_access_level` have precedence | | `all_available` | boolean | no | Show all the groups you have access to (defaults to `false` for authenticated users, `true` for administrators); Attributes `owned` and `min_access_level` have precedence |
| `search` | string | no | Return the list of authorized groups matching the search criteria | | `search` | string | no | Return the list of authorized groups matching the search criteria |
| `order_by` | string | no | Order groups by `name`, `path` or `id`. Default is `name` | | `order_by` | string | no | Order groups by `name`, `path`, `id`, or `similarity` (if searching, [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/332889) in GitLab 14.1). Default is `name` |
| `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` | | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` |
| `statistics` | boolean | no | Include group statistics (administrators only) | | `statistics` | boolean | no | Include group statistics (administrators only) |
| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (administrators only) | | `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (administrators only) |
......
...@@ -22,7 +22,7 @@ module API ...@@ -22,7 +22,7 @@ module API
optional :all_available, type: Boolean, desc: 'Show all group that you have access to' optional :all_available, type: Boolean, desc: 'Show all group that you have access to'
optional :search, type: String, desc: 'Search for a specific group' optional :search, type: String, desc: 'Search for a specific group'
optional :owned, type: Boolean, default: false, desc: 'Limit by owned by authenticated user' optional :owned, type: Boolean, default: false, desc: 'Limit by owned by authenticated user'
optional :order_by, type: String, values: %w[name path id], default: 'name', desc: 'Order by name, path or id' optional :order_by, type: String, values: %w[name path id similarity], default: 'name', desc: 'Order by name, path, id or similarity if searching'
optional :sort, type: String, values: %w[asc desc], default: 'asc', desc: 'Sort by asc (ascending) or desc (descending)' optional :sort, type: String, values: %w[asc desc], default: 'asc', desc: 'Sort by asc (ascending) or desc (descending)'
optional :min_access_level, type: Integer, values: Gitlab::Access.all_values, desc: 'Minimum access level of authenticated user' optional :min_access_level, type: Integer, values: Gitlab::Access.all_values, desc: 'Minimum access level of authenticated user'
optional :top_level_only, type: Boolean, desc: 'Only include top level groups' optional :top_level_only, type: Boolean, desc: 'Only include top level groups'
...@@ -50,9 +50,8 @@ module API ...@@ -50,9 +50,8 @@ module API
groups = GroupsFinder.new(current_user, find_params).execute groups = GroupsFinder.new(current_user, find_params).execute
groups = groups.search(params[:search], include_parents: true) if params[:search].present? groups = groups.search(params[:search], include_parents: true) if params[:search].present?
groups = groups.where.not(id: params[:skip_groups]) if params[:skip_groups].present? groups = groups.where.not(id: params[:skip_groups]) if params[:skip_groups].present?
order_options = { params[:order_by] => params[:sort] }
order_options["id"] ||= "asc" order_groups(groups)
groups.reorder(order_options)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
...@@ -123,6 +122,23 @@ module API ...@@ -123,6 +122,23 @@ module API
reorder_projects(projects) reorder_projects(projects)
end end
def order_groups(groups)
return groups.sorted_by_similarity_and_parent_id_desc(params[:search]) if order_by_similarity?
groups.reorder(group_without_similarity_options) # rubocop: disable CodeReuse/ActiveRecord
end
def order_by_similarity?
params[:order_by] == 'similarity' && params[:search].present?
end
def group_without_similarity_options
order_options = { params[:order_by] => params[:sort] }
order_options['name'] = order_options.delete('similarity') if order_options.has_key?('similarity')
order_options["id"] ||= "asc"
order_options
end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def handle_similarity_order(group, projects) def handle_similarity_order(group, projects)
if params[:search].present? && Feature.enabled?(:similarity_search, group, default_enabled: true) if params[:search].present? && Feature.enabled?(:similarity_search, group, default_enabled: true)
......
...@@ -156,7 +156,7 @@ RSpec.describe Namespace do ...@@ -156,7 +156,7 @@ RSpec.describe Namespace do
end end
end end
describe 'scopes' do describe 'scopes', :aggregate_failures do
let_it_be(:namespace1) { create(:group, name: 'Namespace 1', path: 'namespace-1') } let_it_be(:namespace1) { create(:group, name: 'Namespace 1', path: 'namespace-1') }
let_it_be(:namespace2) { create(:group, name: 'Namespace 2', path: 'namespace-2') } let_it_be(:namespace2) { create(:group, name: 'Namespace 2', path: 'namespace-2') }
let_it_be(:namespace1sub) { create(:group, name: 'Sub Namespace', path: 'sub-namespace', parent: namespace1) } let_it_be(:namespace1sub) { create(:group, name: 'Sub Namespace', path: 'sub-namespace', parent: namespace1) }
...@@ -181,6 +181,15 @@ RSpec.describe Namespace do ...@@ -181,6 +181,15 @@ RSpec.describe Namespace do
expect(described_class.filter_by_path(namespace1.path.upcase)).to eq([namespace1]) expect(described_class.filter_by_path(namespace1.path.upcase)).to eq([namespace1])
end end
end end
describe '.sorted_by_similarity_and_parent_id_desc' do
it 'returns exact matches and top level groups first' do
expect(described_class.sorted_by_similarity_and_parent_id_desc(namespace1.path)).to eq([namespace1, namespace2, namespace2sub, namespace1sub, namespace])
expect(described_class.sorted_by_similarity_and_parent_id_desc(namespace2.path)).to eq([namespace2, namespace1, namespace2sub, namespace1sub, namespace])
expect(described_class.sorted_by_similarity_and_parent_id_desc(namespace2sub.name)).to eq([namespace2sub, namespace1sub, namespace2, namespace1, namespace])
expect(described_class.sorted_by_similarity_and_parent_id_desc('Namespace')).to eq([namespace2, namespace1, namespace2sub, namespace1sub, namespace])
end
end
end end
describe 'delegate' do describe 'delegate' do
......
...@@ -17,7 +17,7 @@ RSpec.describe API::Groups do ...@@ -17,7 +17,7 @@ RSpec.describe API::Groups do
let_it_be(:project3) { create(:project, namespace: group1, path: 'test', visibility_level: Gitlab::VisibilityLevel::PRIVATE) } let_it_be(:project3) { create(:project, namespace: group1, path: 'test', visibility_level: Gitlab::VisibilityLevel::PRIVATE) }
let_it_be(:archived_project) { create(:project, namespace: group1, archived: true) } let_it_be(:archived_project) { create(:project, namespace: group1, archived: true) }
before do before_all do
group1.add_owner(user1) group1.add_owner(user1)
group2.add_owner(user2) group2.add_owner(user2)
end end
...@@ -255,13 +255,14 @@ RSpec.describe API::Groups do ...@@ -255,13 +255,14 @@ RSpec.describe API::Groups do
end end
context "when using sorting" do context "when using sorting" do
let(:group3) { create(:group, name: "a#{group1.name}", path: "z#{group1.path}") } let_it_be(:group3) { create(:group, name: "a#{group1.name}", path: "z#{group1.path}") }
let(:group4) { create(:group, name: "same-name", path: "y#{group1.path}") } let_it_be(:group4) { create(:group, name: "same-name", path: "y#{group1.path}") }
let(:group5) { create(:group, name: "same-name") } let_it_be(:group5) { create(:group, name: "same-name") }
let(:response_groups) { json_response.map { |group| group['name'] } } let(:response_groups) { json_response.map { |group| group['name'] } }
let(:response_groups_ids) { json_response.map { |group| group['id'] } } let(:response_groups_ids) { json_response.map { |group| group['id'] } }
before do before_all do
group3.add_owner(user1) group3.add_owner(user1)
group4.add_owner(user1) group4.add_owner(user1)
group5.add_owner(user1) group5.add_owner(user1)
...@@ -330,6 +331,44 @@ RSpec.describe API::Groups do ...@@ -330,6 +331,44 @@ RSpec.describe API::Groups do
expect(response_groups_ids).to eq(Group.select { |group| group['name'] == 'same-name' }.map { |group| group['id'] }.sort) expect(response_groups_ids).to eq(Group.select { |group| group['name'] == 'same-name' }.map { |group| group['id'] }.sort)
end end
context 'when searching with similarity ordering', :aggregate_failures do
let_it_be(:group6) { create(:group, name: 'same-name subgroup', parent: group4) }
let_it_be(:group7) { create(:group, name: 'same-name parent') }
let(:params) { { order_by: 'similarity', search: 'same-name' } }
before_all do
group6.add_owner(user1)
group7.add_owner(user1)
end
subject { get api('/groups', user1), params: params }
it 'sorts top level groups before subgroups with exact matches first' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response).to include_pagination_headers
expect(json_response.length).to eq(4)
expect(response_groups).to eq(['same-name', 'same-name parent', 'same-name subgroup', 'same-name'])
end
context 'when `search` parameter is not given' do
let(:params) { { order_by: 'similarity' } }
it 'sorts items ordered by name' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response).to include_pagination_headers
expect(json_response.length).to eq(6)
expect(response_groups).to eq(groups_visible_to_user(user1).order(:name).pluck(:name))
end
end
end
def groups_visible_to_user(user) def groups_visible_to_user(user)
Group.where(id: user.authorized_groups.select(:id).reorder(nil)) Group.where(id: user.authorized_groups.select(:id).reorder(nil))
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