Add latest changes from gitlab-org/gitlab@master

This commit is contained in:
GitLab Bot 2020-09-05 03:08:31 +00:00
parent 63a7b9df22
commit 58b9df24e6
17 changed files with 345 additions and 29 deletions

View file

@ -7,7 +7,9 @@ class UserGroupNotificationSettingsFinder
end
def execute
groups_with_ancestors = Gitlab::ObjectHierarchy.new(groups).base_and_ancestors
# rubocop: disable CodeReuse/ActiveRecord
groups_with_ancestors = Gitlab::ObjectHierarchy.new(Group.where(id: groups.select(:id))).base_and_ancestors
# rubocop: enable CodeReuse/ActiveRecord
@loaded_groups_with_ancestors = groups_with_ancestors.index_by(&:id)
@loaded_notification_settings = user.notification_settings_for_groups(groups_with_ancestors).preload_source_route.index_by(&:source_id)
@ -27,6 +29,8 @@ class UserGroupNotificationSettingsFinder
parent_setting = loaded_notification_settings[group.parent_id]
return user.notification_settings.build(source: group) unless parent_setting
if should_copy?(parent_setting)
user.notification_settings.build(source: group) do |ns|
ns.assign_attributes(parent_setting.slice(*NotificationSetting.allowed_fields))

View file

@ -7,19 +7,33 @@ module Resolvers
default_value: false,
description: 'Include also subgroup projects'
argument :search, GraphQL::STRING_TYPE,
required: false,
default_value: nil,
description: 'Search project with most similar names or paths'
argument :sort, Types::Projects::NamespaceProjectSortEnum,
required: false,
default_value: nil,
description: 'Sort projects by this criteria'
type Types::ProjectType, null: true
def resolve(include_subgroups:)
def resolve(include_subgroups:, sort:, search:)
# The namespace could have been loaded in batch by `BatchLoader`.
# At this point we need the `id` or the `full_path` of the namespace
# to query for projects, so make sure it's loaded and not `nil` before continuing.
namespace = object.respond_to?(:sync) ? object.sync : object
return Project.none if namespace.nil?
if include_subgroups
namespace.all_projects.with_route
query = include_subgroups ? namespace.all_projects.with_route : namespace.projects.with_route
return query unless search.present?
if sort == :similarity
query.sorted_by_similarity_desc(search, include_in_select: true).merge(Project.search(search))
else
namespace.projects.with_route
query.merge(Project.search(search))
end
end

View file

@ -0,0 +1,12 @@
# frozen_string_literal: true
module Types
module Projects
class NamespaceProjectSortEnum < BaseEnum
graphql_name 'NamespaceProjectSort'
description 'Values for sorting projects'
value 'SIMILARITY', 'Most similar to the search query', value: :similarity
end
end
end

View file

@ -461,14 +461,17 @@ class Project < ApplicationRecord
# Sometimes queries (e.g. using CTEs) require explicit disambiguation with table name
scope :projects_order_id_desc, -> { reorder(self.arel_table['id'].desc) }
scope :sorted_by_similarity_desc, -> (search) do
scope :sorted_by_similarity_desc, -> (search, include_in_select: false) do
order_expression = Gitlab::Database::SimilarityScore.build_expression(search: search, rules: [
{ column: arel_table["path"], multiplier: 1 },
{ column: arel_table["name"], multiplier: 0.7 },
{ column: arel_table["description"], multiplier: 0.2 }
])
reorder(order_expression.desc, arel_table['id'].desc)
query = reorder(order_expression.desc, arel_table['id'].desc)
query = query.select(*query.arel.projections, order_expression.as('similarity')) if include_in_select
query
end
scope :with_packages, -> { joins(:packages) }

View file

@ -0,0 +1,5 @@
---
title: NotificationsController - Handle mising parent notificationsetting
merge_request: 41612
author:
type: fixed

View file

@ -0,0 +1,5 @@
---
title: Add similarity sorting for projects for GraphQL API
merge_request: 38916
author:
type: added

View file

@ -6776,6 +6776,16 @@ type Group {
Returns the last _n_ elements from the list.
"""
last: Int
"""
Search project with most similar names or paths
"""
search: String = null
"""
Sort projects by this criteria
"""
sort: NamespaceProjectSort = null
): ProjectConnection!
"""
@ -10366,6 +10376,16 @@ type Namespace {
Returns the last _n_ elements from the list.
"""
last: Int
"""
Search project with most similar names or paths
"""
search: String = null
"""
Sort projects by this criteria
"""
sort: NamespaceProjectSort = null
): ProjectConnection!
"""
@ -10464,6 +10484,16 @@ type NamespaceIncreaseStorageTemporarilyPayload {
namespace: Namespace
}
"""
Values for sorting projects
"""
enum NamespaceProjectSort {
"""
Most similar to the search query
"""
SIMILARITY
}
input NegatedBoardIssueInput {
"""
Filter by assignee username

View file

@ -18629,6 +18629,26 @@
},
"defaultValue": "false"
},
{
"name": "search",
"description": "Search project with most similar names or paths",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": "null"
},
{
"name": "sort",
"description": "Sort projects by this criteria",
"type": {
"kind": "ENUM",
"name": "NamespaceProjectSort",
"ofType": null
},
"defaultValue": "null"
},
{
"name": "hasVulnerabilities",
"description": "Returns only the projects which have vulnerabilities",
@ -30963,6 +30983,26 @@
},
"defaultValue": "false"
},
{
"name": "search",
"description": "Search project with most similar names or paths",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": "null"
},
{
"name": "sort",
"description": "Sort projects by this criteria",
"type": {
"kind": "ENUM",
"name": "NamespaceProjectSort",
"ofType": null
},
"defaultValue": "null"
},
{
"name": "hasVulnerabilities",
"description": "Returns only the projects which have vulnerabilities",
@ -31318,6 +31358,23 @@
"enumValues": null,
"possibleTypes": null
},
{
"kind": "ENUM",
"name": "NamespaceProjectSort",
"description": "Values for sorting projects",
"fields": null,
"inputFields": null,
"interfaces": null,
"enumValues": [
{
"name": "SIMILARITY",
"description": "Most similar to the search query",
"isDeprecated": false,
"deprecationReason": null
}
],
"possibleTypes": null
},
{
"kind": "INPUT_OBJECT",
"name": "NegatedBoardIssueInput",

View file

@ -29,7 +29,10 @@ module Gitlab
def table_condition(order_info, value, operator)
if order_info.named_function
target = order_info.named_function
value = value&.downcase if target.respond_to?(:name) && target&.name&.downcase == 'lower'
if target.try(:name)&.casecmp('lower') == 0
value = value&.downcase
end
else
target = arel_table[order_info.attribute_name]
end

View file

@ -90,21 +90,24 @@ module Gitlab
end
def extract_attribute_values(order_value)
named = nil
name = if ordering_by_lower?(order_value)
named = order_value.expr
named.expressions[0].name.to_s
else
order_value.expr.name
end
[name, order_value.direction, named]
if ordering_by_lower?(order_value)
[order_value.expr.expressions[0].name.to_s, order_value.direction, order_value.expr]
elsif ordering_by_similarity?(order_value)
['similarity', order_value.direction, order_value.expr]
else
[order_value.expr.name, order_value.direction, nil]
end
end
# determine if ordering using LOWER, eg. "ORDER BY LOWER(boards.name)"
def ordering_by_lower?(order_value)
order_value.expr.is_a?(Arel::Nodes::NamedFunction) && order_value.expr&.name&.downcase == 'lower'
end
# determine if ordering using SIMILARITY scoring based on Gitlab::Database::SimilarityScore
def ordering_by_similarity?(order_value)
order_value.to_sql.match?(/SIMILARITY\(.+\*/)
end
end
end
end

View file

@ -71,6 +71,43 @@ RSpec.describe UserGroupNotificationSettingsFinder do
end
end
context 'when the group has parent_id set but that does not belong to any group' do
let_it_be(:group) { create(:group) }
let_it_be(:groups) { [group] }
before do
# Let's set a parent_id for a group that definitely doesn't exist
group.update_columns(parent_id: 19283746)
end
it 'returns a default Global notification setting' do
expect(subject.count).to eq(1)
expect(attributes(&:level)).to eq(['global'])
expect(attributes(&:notification_email)).to eq([nil])
end
end
context 'when the group has a private parent' do
let_it_be(:ancestor) { create(:group, :private) }
let_it_be(:group) { create(:group, :private, parent: ancestor) }
let_it_be(:ancestor_email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) }
let_it_be(:groups) { [group] }
before do
group.add_reporter(user)
# Adding the user creates a NotificationSetting, so we remove it here
user.notification_settings.where(source: group).delete_all
create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: ancestor_email.email)
end
it 'still inherits the notification settings' do
expect(subject.count).to eq(1)
expect(attributes(&:level)).to eq(['participating'])
expect(attributes(&:notification_email)).to eq([ancestor_email.email])
end
end
it 'does not cause an N+1', :aggregate_failures do
parent = create(:group)
child = create(:group, parent: parent)

View file

@ -27,7 +27,7 @@ RSpec.describe Resolvers::NamespaceProjectsResolver do
end
it 'finds all projects including the subgroups' do
expect(resolve_projects(include_subgroups: true)).to contain_exactly(project1, project2, nested_project)
expect(resolve_projects(include_subgroups: true, sort: nil, search: nil)).to contain_exactly(project1, project2, nested_project)
end
context 'with an user namespace' do
@ -38,7 +38,52 @@ RSpec.describe Resolvers::NamespaceProjectsResolver do
end
it 'finds all projects including the subgroups' do
expect(resolve_projects(include_subgroups: true)).to contain_exactly(project1, project2)
expect(resolve_projects(include_subgroups: true, sort: nil, search: nil)).to contain_exactly(project1, project2)
end
end
end
context 'search and similarity sorting' do
let(:project_1) { create(:project, name: 'Project', path: 'project', namespace: namespace) }
let(:project_2) { create(:project, name: 'Test Project', path: 'test-project', namespace: namespace) }
let(:project_3) { create(:project, name: 'Test', path: 'test', namespace: namespace) }
before do
project_1.add_developer(current_user)
project_2.add_developer(current_user)
project_3.add_developer(current_user)
end
it 'returns projects ordered by similarity to the search input' do
projects = resolve_projects(include_subgroups: true, sort: :similarity, search: 'test')
project_names = projects.map { |proj| proj['name'] }
expect(project_names.first).to eq('Test')
expect(project_names.second).to eq('Test Project')
end
it 'filters out result that do not match the search input' do
projects = resolve_projects(include_subgroups: true, sort: :similarity, search: 'test')
project_names = projects.map { |proj| proj['name'] }
expect(project_names).not_to include('Project')
end
context 'when `search` parameter is not given' do
it 'returns projects not ordered by similarity' do
projects = resolve_projects(include_subgroups: true, sort: :similarity, search: nil)
project_names = projects.map { |proj| proj['name'] }
expect(project_names.first).not_to eq('Test')
end
end
context 'when only search term is given' do
it 'filters out result that do not match the search input, but does not sort them' do
projects = resolve_projects(include_subgroups: true, sort: :nil, search: 'test')
project_names = projects.map { |proj| proj['name'] }
expect(project_names).to contain_exactly('Test', 'Test Project')
end
end
end
@ -63,7 +108,7 @@ RSpec.describe Resolvers::NamespaceProjectsResolver do
expect(field.to_graphql.complexity.call({}, { include_subgroups: true }, 1)).to eq 24
end
def resolve_projects(args = { include_subgroups: false }, context = { current_user: current_user })
def resolve_projects(args = { include_subgroups: false, sort: nil, search: nil }, context = { current_user: current_user })
resolve(described_class, obj: namespace, args: args, ctx: context)
end
end

View file

@ -262,6 +262,22 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do
end
end
context 'when ordering by similarity' do
let!(:project1) { create(:project, name: 'test') }
let!(:project2) { create(:project, name: 'testing') }
let!(:project3) { create(:project, name: 'tests') }
let!(:project4) { create(:project, name: 'testing stuff') }
let!(:project5) { create(:project, name: 'test') }
let(:nodes) do
Project.sorted_by_similarity_desc('test', include_in_select: true)
end
let(:descending_nodes) { nodes.to_a }
it_behaves_like 'nodes are in descending order'
end
context 'when an invalid cursor is provided' do
let(:arguments) { { before: Base64Bp.urlsafe_encode64('invalidcursor', padding: false) } }
@ -358,15 +374,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do
end
end
context 'when before and last does not request all remaining nodes' do
let(:arguments) { { before: encoded_cursor(project_list.last), last: 2 } }
it 'has a previous and a next' do
expect(subject.has_previous_page).to be_truthy
expect(subject.has_next_page).to be_truthy
end
end
context 'when before and last does request all remaining nodes' do
let(:arguments) { { before: encoded_cursor(project_list[1]), last: 3 } }

View file

@ -51,6 +51,18 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::OrderInfo do
expect(order_list.last.operator_for(:after)).to eq '>'
end
end
context 'when ordering by SIMILARITY' do
let(:relation) { Project.sorted_by_similarity_desc('test', include_in_select: true) }
it 'assigns the right attribute name, named function, and direction' do
expect(order_list.count).to eq 2
expect(order_list.first.attribute_name).to eq 'similarity'
expect(order_list.first.named_function).to be_kind_of(Arel::Nodes::Addition)
expect(order_list.first.named_function.to_sql).to include 'SIMILARITY('
expect(order_list.first.sort_direction).to eq :desc
end
end
end
describe '#validate_ordering' do

View file

@ -131,5 +131,42 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::QueryBuilder do
end
end
end
context 'when sorting using SIMILARITY' do
let(:relation) { Project.sorted_by_similarity_desc('test', include_in_select: true) }
let(:arel_table) { Project.arel_table }
let(:decoded_cursor) { { 'similarity' => 0.5, 'id' => 100 } }
let(:similarity_sql) do
[
'(SIMILARITY(COALESCE("projects"."path", \'\'), \'test\') * CAST(\'1\' AS numeric))',
'(SIMILARITY(COALESCE("projects"."name", \'\'), \'test\') * CAST(\'0.7\' AS numeric))',
'(SIMILARITY(COALESCE("projects"."description", \'\'), \'test\') * CAST(\'0.2\' AS numeric))'
].join(' + ')
end
context 'when no values are nil' do
context 'when :after' do
it 'generates the correct condition' do
conditions = builder.conditions.gsub(/\s+/, ' ')
expect(conditions).to include "(#{similarity_sql} < 0.5)"
expect(conditions).to include '"projects"."id" < 100'
expect(conditions).to include "OR (#{similarity_sql} IS NULL)"
end
end
context 'when :before' do
let(:before_or_after) { :before }
it 'generates the correct condition' do
conditions = builder.conditions.gsub(/\s+/, ' ')
expect(conditions).to include "(#{similarity_sql} > 0.5)"
expect(conditions).to include '"projects"."id" > 100'
expect(conditions).to include "OR ( #{similarity_sql} = 0.5"
end
end
end
end
end
end

View file

@ -78,4 +78,43 @@ RSpec.describe 'getting projects' do
it_behaves_like 'a graphql namespace'
end
describe 'sorting and pagination' do
let(:data_path) { [:namespace, :projects] }
def pagination_query(params, page_info)
graphql_query_for(
'namespace',
{ 'fullPath' => subject.full_path },
<<~QUERY
projects(includeSubgroups: #{include_subgroups}, search: "#{search}", #{params}) {
#{page_info} edges {
node {
#{all_graphql_fields_for('Project')}
}
}
}
QUERY
)
end
def pagination_results_data(data)
data.map { |project| project.dig('node', 'name') }
end
context 'when sorting by similarity' do
let!(:project_1) { create(:project, name: 'Project', path: 'project', namespace: subject) }
let!(:project_2) { create(:project, name: 'Test Project', path: 'test-project', namespace: subject) }
let!(:project_3) { create(:project, name: 'Test', path: 'test', namespace: subject) }
let!(:project_4) { create(:project, name: 'Test Project Other', path: 'other-test-project', namespace: subject) }
let(:search) { 'test' }
let(:current_user) { user }
it_behaves_like 'sorted paginated query' do
let(:sort_param) { 'SIMILARITY' }
let(:first_param) { 2 }
let(:expected_results) { [project_3.name, project_2.name, project_4.name] }
end
end
end
end

View file

@ -84,6 +84,9 @@ RSpec.shared_examples 'sorted paginated query' do
cursored_query = pagination_query([sort_argument, "after: \"#{end_cursor}\""].compact.join(','), page_info)
post_graphql(cursored_query, current_user: current_user)
expect(response).to have_gitlab_http_status(:ok)
response_data = graphql_dig_at(Gitlab::Json.parse(response.body), :data, *data_path, :edges)
expect(pagination_results_data(response_data)).to eq expected_results.drop(first_param)