From 2fb59e63d8fcb1f82f378b97bb94becc2399e01b Mon Sep 17 00:00:00 2001
From: Rodrigo Tomonari <rtomonari@gitlab.com>
Date: Thu, 27 Jun 2024 23:15:37 -0300
Subject: [PATCH 1/6] Add filters to import source users Graphql resolver
Update the resolver to filter and sort results
---
app/finders/import/source_users_finder.rb | 21 +++++-
.../resolvers/import/source_users_resolver.rb | 13 ++++
.../types/import/source_user_sort_enum.rb | 15 ++++
app/models/import/source_user.rb | 40 ++++++++---
app/policies/import/source_user_policy.rb | 2 +-
...2648_add_indexes_to_import_source_users.rb | 30 ++++++++
db/schema_migrations/20240627142648 | 1 +
db/structure.sql | 6 ++
doc/api/graphql/reference/index.md | 57 ++++++++++++++-
.../import/source_users_finder_spec.rb | 71 ++++++++++++++++---
.../import/source_users_resolver_spec.rb | 14 ++++
spec/models/import/source_user_spec.rb | 68 ++++++++++++++++++
.../helpers/database/duplicate_indexes.yml | 3 +
13 files changed, 321 insertions(+), 20 deletions(-)
create mode 100644 app/graphql/types/import/source_user_sort_enum.rb
create mode 100644 db/migrate/20240627142648_add_indexes_to_import_source_users.rb
create mode 100644 db/schema_migrations/20240627142648
diff --git a/app/finders/import/source_users_finder.rb b/app/finders/import/source_users_finder.rb
index d4e573bfabc079..f3bc3d5f0ba0b8 100644
--- a/app/finders/import/source_users_finder.rb
+++ b/app/finders/import/source_users_finder.rb
@@ -11,7 +11,10 @@ def initialize(namespace, current_user, params = {})
def execute
return Import::SourceUser.none unless authorized?
- namespace.import_source_users
+ collection = namespace.import_source_users
+ collection = by_statuses(collection)
+ collection = by_search(collection)
+ sort(collection)
end
private
@@ -21,5 +24,21 @@ def execute
def authorized?
Ability.allowed?(current_user, :admin_namespace, namespace)
end
+
+ def by_statuses(collection)
+ return collection unless params[:statuses].present?
+
+ collection.by_statuses(params[:statuses])
+ end
+
+ def by_search(collection)
+ return collection unless params[:search].present?
+
+ collection.search(params[:search])
+ end
+
+ def sort(collection)
+ collection.sort_by_attribute(params[:sort] || :source_name_asc)
+ end
end
end
diff --git a/app/graphql/resolvers/import/source_users_resolver.rb b/app/graphql/resolvers/import/source_users_resolver.rb
index 57210e3b6b1faa..5a480736be0147 100644
--- a/app/graphql/resolvers/import/source_users_resolver.rb
+++ b/app/graphql/resolvers/import/source_users_resolver.rb
@@ -11,6 +11,19 @@ class SourceUsersResolver < BaseResolver
type Types::Import::SourceUserType.connection_type, null: true
+ argument :statuses, [::Types::Import::SourceUserStatusEnum],
+ required: false,
+ description: 'Filter mapping of users on source instance to users on destination instance by status.'
+
+ argument :search, GraphQL::Types::String,
+ required: false,
+ description: 'Query to search mappings by name or username of users on source instance.'
+
+ argument :sort, Types::Import::SourceUserSortEnum,
+ description: 'Sort mapping of users on source instance to users on destination instance by the criteria.',
+ required: false,
+ default_value: :source_name_asc
+
alias_method :namespace, :object
def resolve_with_lookahead(**args)
diff --git a/app/graphql/types/import/source_user_sort_enum.rb b/app/graphql/types/import/source_user_sort_enum.rb
new file mode 100644
index 00000000000000..b7a8d4a4601d54
--- /dev/null
+++ b/app/graphql/types/import/source_user_sort_enum.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module Types
+ module Import
+ class SourceUserSortEnum < BaseEnum
+ graphql_name 'SourceUserSort'
+ description 'Values for sorting the mapping of users on source instance to users on destination instance.'
+
+ value 'STATUS_ASC', 'Status of the mapping by ascending order.', value: :status_asc
+ value 'STATUS_DESC', 'Status of the mapping by descending order.', value: :status_desc
+ value 'SOURCE_NAME_ASC', 'Instance source name by ascending order.', value: :source_name_asc
+ value 'SOURCE_NAME_DESC', 'Instance source name by descending order.', value: :source_name_desc
+ end
+ end
+end
diff --git a/app/models/import/source_user.rb b/app/models/import/source_user.rb
index 967d5e0b41b75f..9b7b24b53057da 100644
--- a/app/models/import/source_user.rb
+++ b/app/models/import/source_user.rb
@@ -2,8 +2,17 @@
module Import
class SourceUser < ApplicationRecord
+ include Gitlab::SQL::Pattern
+
self.table_name = 'import_source_users'
+ SORT_ORDERS = {
+ source_name_asc: { order_by: 'source_name', sort: 'asc' },
+ source_name_desc: { order_by: 'source_name', sort: 'desc' },
+ status_asc: { order_by: 'status', sort: 'asc' },
+ status_desc: { order_by: 'status', sort: 'desc' }
+ }.freeze
+
belongs_to :placeholder_user, class_name: 'User', optional: true
belongs_to :reassign_to_user, class_name: 'User', optional: true
belongs_to :reassigned_by_user, class_name: 'User', optional: true
@@ -12,6 +21,7 @@ class SourceUser < ApplicationRecord
validates :namespace_id, :import_type, :source_hostname, :source_user_identifier, :status, presence: true
scope :for_namespace, ->(namespace_id) { where(namespace_id: namespace_id) }
+ scope :by_statuses, ->(statuses) { where(status: statuses) }
state_machine :status, initial: :pending_reassignment do
state :pending_reassignment, value: 0
@@ -55,15 +65,29 @@ class SourceUser < ApplicationRecord
end
end
- def self.find_source_user(source_user_identifier:, namespace:, source_hostname:, import_type:)
- return unless namespace
+ class << self
+ def find_source_user(source_user_identifier:, namespace:, source_hostname:, import_type:)
+ return unless namespace
+
+ find_by(
+ source_user_identifier: source_user_identifier,
+ namespace_id: namespace.id,
+ source_hostname: source_hostname,
+ import_type: import_type
+ )
+ end
+
+ def search(query)
+ return none unless query.is_a?(String)
- find_by(
- source_user_identifier: source_user_identifier,
- namespace_id: namespace.id,
- source_hostname: source_hostname,
- import_type: import_type
- )
+ fuzzy_search(query, [:source_name, :source_username])
+ end
+
+ def sort_by_attribute(method)
+ sort_order = SORT_ORDERS[method&.to_sym] || SORT_ORDERS[:source_name_asc]
+
+ reorder(sort_order[:order_by] => sort_order[:sort])
+ end
end
def reassignable_status?
diff --git a/app/policies/import/source_user_policy.rb b/app/policies/import/source_user_policy.rb
index fa6e63e35fb171..44a4185aef90a7 100644
--- a/app/policies/import/source_user_policy.rb
+++ b/app/policies/import/source_user_policy.rb
@@ -2,8 +2,8 @@
module Import
class SourceUserPolicy < ::BasePolicy
- condition(:admin_source_user_namespace) { can?(:admin_namespace, @subject.namespace) }
desc "User can administrate namespace"
+ condition(:admin_source_user_namespace) { can?(:admin_namespace, @subject.namespace) }
rule { admin_source_user_namespace }.policy do
enable :admin_import_source_user
diff --git a/db/migrate/20240627142648_add_indexes_to_import_source_users.rb b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
new file mode 100644
index 00000000000000..3742aad52066c0
--- /dev/null
+++ b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+# See https://docs.gitlab.com/ee/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddIndexesToImportSourceUsers < Gitlab::Database::Migration[2.2]
+ disable_ddl_transaction!
+
+ milestone '17.2'
+
+ STATUS_INDEX = 'index_import_source_users_on_namespace_id_and_status'
+ # SOURCE_NAME_INDEX = 'index_import_source_users_on_namespace_id_status_source_name'
+
+ SOURCE_NAME_INDEX = 'index_import_source_users_source_name_trgm'
+ SOURCE_USERNAME_INDEX = 'index_import_source_users_source_username_trgm'
+
+ def up
+ add_concurrent_index :import_source_users, [:namespace_id, :status, :id], name: STATUS_INDEX
+ add_concurrent_index :import_source_users, :source_name, name: SOURCE_NAME_INDEX, using: :gin,
+ opclass: { path: :gin_trgm_ops }
+ add_concurrent_index :import_source_users, :source_username, name: SOURCE_USERNAME_INDEX, using: :gin,
+ opclass: { path: :gin_trgm_ops }
+ end
+
+ def down
+ remove_concurrent_index_by_name :import_source_users, name: STATUS_INDEX
+ remove_concurrent_index_by_name :import_source_users, name: SOURCE_NAME_INDEX
+ remove_concurrent_index_by_name :import_source_users, name: SOURCE_USERNAME_INDEX
+ end
+end
diff --git a/db/schema_migrations/20240627142648 b/db/schema_migrations/20240627142648
new file mode 100644
index 00000000000000..3870b50f668a3f
--- /dev/null
+++ b/db/schema_migrations/20240627142648
@@ -0,0 +1 @@
+434b51c7e1faea1ce5406ed78f97235247580053c4132dd5d0e325a3a2e7034e
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index e127e7f413a468..aa1afc781e1756 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -27567,12 +27567,18 @@ CREATE INDEX index_import_source_user_placeholder_references_on_source_user_ ON
CREATE INDEX index_import_source_users_on_namespace_id ON import_source_users USING btree (namespace_id);
+CREATE INDEX index_import_source_users_on_namespace_id_and_status ON import_source_users USING btree (namespace_id, status, id);
+
CREATE INDEX index_import_source_users_on_placeholder_user_id ON import_source_users USING btree (placeholder_user_id);
CREATE INDEX index_import_source_users_on_reassign_to_user_id ON import_source_users USING btree (reassign_to_user_id);
CREATE INDEX index_import_source_users_on_reassigned_by_user_id ON import_source_users USING btree (reassigned_by_user_id);
+CREATE INDEX index_import_source_users_source_name_trgm ON import_source_users USING gin (source_name gin_trgm_ops);
+
+CREATE INDEX index_import_source_users_source_username_trgm ON import_source_users USING gin (source_username gin_trgm_ops);
+
CREATE INDEX index_imported_projects_on_import_type_creator_id_created_at ON projects USING btree (import_type, creator_id, created_at) WHERE (import_type IS NOT NULL);
CREATE INDEX index_imported_projects_on_import_type_id ON projects USING btree (import_type, id) WHERE (import_type IS NOT NULL);
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 244cf7966c5c17..7f67915ff62f91 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -22045,7 +22045,6 @@ GPG signature for a signed commit.
| <a id="groupgooglecloudloggingconfigurations"></a>`googleCloudLoggingConfigurations` | [`GoogleCloudLoggingConfigurationTypeConnection`](#googlecloudloggingconfigurationtypeconnection) | Google Cloud logging configurations that receive audit events belonging to the group. (see [Connections](#connections)) |
| <a id="groupgroupmemberscount"></a>`groupMembersCount` | [`Int!`](#int) | Count of direct members of this group. |
| <a id="groupid"></a>`id` | [`ID!`](#id) | ID of the namespace. |
-| <a id="groupimportsourceusers"></a>`importSourceUsers` **{warning-solid}** | [`ImportSourceUserConnection`](#importsourceuserconnection) | **Introduced** in GitLab 17.2. **Status**: Experiment. Import source users of the namespace. This field can only be resolved for one namespace in any single request. |
| <a id="groupisadjourneddeletionenabled"></a>`isAdjournedDeletionEnabled` **{warning-solid}** | [`Boolean!`](#boolean) | **Introduced** in GitLab 16.11. **Status**: Experiment. Indicates if delayed group deletion is enabled. |
| <a id="grouplfsenabled"></a>`lfsEnabled` | [`Boolean`](#boolean) | Indicates if Large File Storage (LFS) is enabled for namespace. |
| <a id="grouplockduofeaturesenabled"></a>`lockDuoFeaturesEnabled` **{warning-solid}** | [`Boolean`](#boolean) | **Introduced** in GitLab 16.10. **Status**: Experiment. Indicates if the GitLab Duo features enabled setting is enforced for all subgroups. |
@@ -22608,6 +22607,28 @@ four standard [pagination arguments](#pagination-arguments):
| <a id="groupgroupmemberssearch"></a>`search` | [`String`](#string) | Search query. |
| <a id="groupgroupmemberssort"></a>`sort` | [`MemberSort`](#membersort) | sort query. |
+##### `Group.importSourceUsers`
+
+Import source users of the namespace. This field can only be resolved for one namespace in any single request.
+
+DETAILS:
+**Introduced** in GitLab 17.2.
+**Status**: Experiment.
+
+Returns [`ImportSourceUserConnection`](#importsourceuserconnection).
+
+This field returns a [connection](#connections). It accepts the
+four standard [pagination arguments](#pagination-arguments):
+`before: String`, `after: String`, `first: Int`, and `last: Int`.
+
+###### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="groupimportsourceuserssearch"></a>`search` | [`String`](#string) | Query to search mappings by name or username of users on source instance. |
+| <a id="groupimportsourceuserssort"></a>`sort` | [`SourceUserSort`](#sourceusersort) | Sort mapping of users on source instance to users on destination instance by the criteria. |
+| <a id="groupimportsourceusersstatuses"></a>`statuses` | [`[ImportSourceUserStatus!]`](#importsourceuserstatus) | Filter mapping of users on source instance to users on destination instance by status. |
+
##### `Group.issues`
Issues for projects in this group.
@@ -26501,7 +26522,6 @@ Product analytics events for a specific month and year.
| <a id="namespacefullname"></a>`fullName` | [`String!`](#string) | Full name of the namespace. |
| <a id="namespacefullpath"></a>`fullPath` | [`ID!`](#id) | Full path of the namespace. |
| <a id="namespaceid"></a>`id` | [`ID!`](#id) | ID of the namespace. |
-| <a id="namespaceimportsourceusers"></a>`importSourceUsers` **{warning-solid}** | [`ImportSourceUserConnection`](#importsourceuserconnection) | **Introduced** in GitLab 17.2. **Status**: Experiment. Import source users of the namespace. This field can only be resolved for one namespace in any single request. |
| <a id="namespacelfsenabled"></a>`lfsEnabled` | [`Boolean`](#boolean) | Indicates if Large File Storage (LFS) is enabled for namespace. |
| <a id="namespacename"></a>`name` | [`String!`](#string) | Name of the namespace. |
| <a id="namespacepackagesettings"></a>`packageSettings` | [`PackageSettings`](#packagesettings) | Package settings for the namespace. |
@@ -26607,6 +26627,28 @@ four standard [pagination arguments](#pagination-arguments):
| <a id="namespacecomplianceframeworksids"></a>`ids` | [`[ComplianceManagementFrameworkID!]`](#compliancemanagementframeworkid) | List of Global IDs of compliance frameworks to return. |
| <a id="namespacecomplianceframeworkssearch"></a>`search` | [`String`](#string) | Search framework with most similar names. |
+##### `Namespace.importSourceUsers`
+
+Import source users of the namespace. This field can only be resolved for one namespace in any single request.
+
+DETAILS:
+**Introduced** in GitLab 17.2.
+**Status**: Experiment.
+
+Returns [`ImportSourceUserConnection`](#importsourceuserconnection).
+
+This field returns a [connection](#connections). It accepts the
+four standard [pagination arguments](#pagination-arguments):
+`before: String`, `after: String`, `first: Int`, and `last: Int`.
+
+###### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="namespaceimportsourceuserssearch"></a>`search` | [`String`](#string) | Query to search mappings by name or username of users on source instance. |
+| <a id="namespaceimportsourceuserssort"></a>`sort` | [`SourceUserSort`](#sourceusersort) | Sort mapping of users on source instance to users on destination instance by the criteria. |
+| <a id="namespaceimportsourceusersstatuses"></a>`statuses` | [`[ImportSourceUserStatus!]`](#importsourceuserstatus) | Filter mapping of users on source instance to users on destination instance by status. |
+
##### `Namespace.pagesDeployments`
List of the namespaces's Pages Deployments.
@@ -36242,6 +36284,17 @@ Values for sort direction.
| <a id="sortdirectionenumasc"></a>`ASC` | Ascending order. |
| <a id="sortdirectionenumdesc"></a>`DESC` | Descending order. |
+### `SourceUserSort`
+
+Values for sorting the mapping of users on source instance to users on destination instance.
+
+| Value | Description |
+| ----- | ----------- |
+| <a id="sourceusersortsource_name_asc"></a>`SOURCE_NAME_ASC` | Instance source name by ascending order. |
+| <a id="sourceusersortsource_name_desc"></a>`SOURCE_NAME_DESC` | Instance source name by descending order. |
+| <a id="sourceusersortstatus_asc"></a>`STATUS_ASC` | Status of the mapping by ascending order. |
+| <a id="sourceusersortstatus_desc"></a>`STATUS_DESC` | Status of the mapping by descending order. |
+
### `TestCaseStatus`
| Value | Description |
diff --git a/spec/finders/import/source_users_finder_spec.rb b/spec/finders/import/source_users_finder_spec.rb
index 27a117a4b78ac7..4d0d7fbf6b0c53 100644
--- a/spec/finders/import/source_users_finder_spec.rb
+++ b/spec/finders/import/source_users_finder_spec.rb
@@ -5,10 +5,23 @@
RSpec.describe Import::SourceUsersFinder, feature_category: :importers do
let_it_be(:user) { build_stubbed(:user) }
let_it_be(:group) { create(:group) }
- let_it_be(:import_source_users) { create_list(:import_source_user, 3, namespace: group) }
+ let_it_be(:source_user_1) { create(:import_source_user, namespace: group, status: 0, source_name: 'b') }
+ let_it_be(:source_user_2) { create(:import_source_user, namespace: group, status: 1, source_name: 'c') }
+ let_it_be(:source_user_3) { create(:import_source_user, namespace: group, status: 2, source_name: 'a') }
+ let_it_be(:import_source_users) { [source_user_1, source_user_2, source_user_3] }
+
+ let(:params) { {} }
describe '#execute' do
- subject(:source_user_result) { described_class.new(group, user).execute }
+ subject(:source_user_result) { described_class.new(group, user, params).execute }
+
+ context 'when user is not authorized to read the import source users' do
+ before do
+ stub_member_access_level(group, maintainer: user)
+ end
+
+ it { expect(source_user_result).to be_empty }
+ end
context 'when user is authorized to read the import source users' do
before do
@@ -18,15 +31,57 @@
it 'returns all import source users' do
expect(source_user_result).to match_array(import_source_users)
end
- end
- context 'when user is not authorized to read the import source users' do
- before do
- stub_member_access_level(group, maintainer: user)
+ describe 'filtering by statuses' do
+ context 'when statuses are not provided' do
+ let(:params) { {} }
+
+ it 'returns all import source users' do
+ expect(source_user_result).to match_array(import_source_users)
+ end
+ end
+
+ context 'when statuses are is provided' do
+ let(:params) { { statuses: [0, 1] } }
+
+ it 'returns import source users with the corresponding status' do
+ expect(source_user_result.pluck(:status)).to match_array([0, 1])
+ end
+ end
end
- it 'is empty' do
- expect(source_user_result).to be_empty
+ describe 'filtering by search' do
+ context 'when search are not provided' do
+ let(:params) { {} }
+
+ it 'returns all import source users' do
+ expect(source_user_result).to match_array(import_source_users)
+ end
+ end
+
+ context 'when search is is provided' do
+ let(:params) { { search: 'b' } }
+
+ it 'returns import source users with matches the search query' do
+ expect(source_user_result).to match_array(source_user_1)
+ end
+ end
+ end
+
+ describe 'sorting' do
+ let(:params) { { sort: 'source_name_desc' } }
+
+ it 'returns import source users sorted by the provided method' do
+ expect(source_user_result.pluck(:source_name)).to eq(%w[c b a])
+ end
+
+ context 'when sort is not provided' do
+ let(:params) { {} }
+
+ it 'returns import source users sorted by source_name_asc' do
+ expect(source_user_result.pluck(:source_name)).to eq(%w[a b c])
+ end
+ end
end
end
end
diff --git a/spec/graphql/resolvers/import/source_users_resolver_spec.rb b/spec/graphql/resolvers/import/source_users_resolver_spec.rb
index c908791dc12d45..31dfc221f7e423 100644
--- a/spec/graphql/resolvers/import/source_users_resolver_spec.rb
+++ b/spec/graphql/resolvers/import/source_users_resolver_spec.rb
@@ -37,6 +37,20 @@
it { expect(resolve_import_source_users).to be_empty }
end
+
+ describe 'arguments' do
+ let(:args) { { search: 'search', statuses: ['AWAITING_APPROVAL'], sort: 'STATUS_ASC' } }
+
+ it 'calls Import::SourceUsersFinder with the expected arguments' do
+ expected_args = { search: 'search', statuses: [1], sort: :status_asc }
+
+ expect_next_instance_of(::Import::SourceUsersFinder, group, current_user, expected_args) do |finder|
+ expect(finder).to receive(:execute)
+ end
+
+ resolve_import_source_users
+ end
+ end
end
def resolve_import_source_users
diff --git a/spec/models/import/source_user_spec.rb b/spec/models/import/source_user_spec.rb
index c22e02f56571a0..931818105069a7 100644
--- a/spec/models/import/source_user_spec.rb
+++ b/spec/models/import/source_user_spec.rb
@@ -118,6 +118,74 @@
end
end
+ describe '.search' do
+ let!(:source_user) do
+ create(:import_source_user, source_name: 'Source Name', source_username: 'Username')
+ end
+
+ it 'searches by source_name or source_username' do
+ expect(described_class.search('name')).to eq([source_user])
+ expect(described_class.search('username')).to eq([source_user])
+ expect(described_class.search('source')).to eq([source_user])
+ expect(described_class.search('inexistent')).to eq([])
+ end
+ end
+
+ describe '.sort_by_attribute' do
+ let_it_be(:namespace) { create(:namespace) }
+ let_it_be(:source_user_1) { create(:import_source_user, namespace: namespace, status: 4, source_name: 'd') }
+ let_it_be(:source_user_2) { create(:import_source_user, namespace: namespace, status: 2, source_name: 'b') }
+ let_it_be(:source_user_3) { create(:import_source_user, namespace: namespace, status: 1, source_name: 'a') }
+ let_it_be(:source_user_4) { create(:import_source_user, namespace: namespace, status: 3, source_name: 'c') }
+
+ let(:sort_by_attribute) { described_class.sort_by_attribute(method).pluck(attribute) }
+
+ context 'with method status_asc' do
+ let(:method) { 'status_asc' }
+ let(:attribute) { :status }
+
+ it 'order by status_desc ascending' do
+ expect(sort_by_attribute).to eq([1, 2, 3, 4])
+ end
+ end
+
+ context 'with method status_desc' do
+ let(:method) { 'status_desc' }
+ let(:attribute) { :status }
+
+ it 'order by status_desc descending' do
+ expect(sort_by_attribute).to eq([4, 3, 2, 1])
+ end
+ end
+
+ context 'with method source_name_asc' do
+ let(:method) { 'source_name_asc' }
+ let(:attribute) { :source_name }
+
+ it 'order by source_name_asc ascending' do
+ expect(sort_by_attribute).to eq(%w[a b c d])
+ end
+ end
+
+ context 'with method source_name_desc' do
+ let(:method) { 'source_name_desc' }
+ let(:attribute) { :source_name }
+
+ it 'order by source_name_desc descending' do
+ expect(sort_by_attribute).to eq(%w[d c b a])
+ end
+ end
+
+ context 'with an unexpected method' do
+ let(:method) { 'id_asc' }
+ let(:attribute) { :source_name }
+
+ it 'order by source_name_asc ascending' do
+ expect(sort_by_attribute).to eq(%w[a b c d])
+ end
+ end
+ end
+
describe '#reassignable_status?' do
reassignable_statuses = [:pending_reassignment, :rejected]
all_states = described_class.state_machines[:status].states
diff --git a/spec/support/helpers/database/duplicate_indexes.yml b/spec/support/helpers/database/duplicate_indexes.yml
index 784be0bf14085a..c9f763d09c43e4 100644
--- a/spec/support/helpers/database/duplicate_indexes.yml
+++ b/spec/support/helpers/database/duplicate_indexes.yml
@@ -20,6 +20,9 @@ error_tracking_errors:
geo_node_namespace_links:
index_geo_node_namespace_links_on_geo_node_id_and_namespace_id:
- index_geo_node_namespace_links_on_geo_node_id
+import_source_users:
+ index_import_source_users_on_namespace_id_and_status:
+ - index_import_source_users_on_namespace_id
incident_management_oncall_participants:
index_inc_mgmnt_oncall_participants_on_user_id_and_rotation_id:
- index_inc_mgmnt_oncall_participants_on_oncall_user_id
--
GitLab
From 081d1a98e516c28f7918b14d8f16d6a20c57179d Mon Sep 17 00:00:00 2001
From: Rodrigo Tomonari <rtomonari@gitlab.com>
Date: Tue, 2 Jul 2024 15:05:05 -0300
Subject: [PATCH 2/6] Use array in rspec
---
spec/finders/import/source_users_finder_spec.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/spec/finders/import/source_users_finder_spec.rb b/spec/finders/import/source_users_finder_spec.rb
index 4d0d7fbf6b0c53..ba08c54440d1f7 100644
--- a/spec/finders/import/source_users_finder_spec.rb
+++ b/spec/finders/import/source_users_finder_spec.rb
@@ -63,7 +63,7 @@
let(:params) { { search: 'b' } }
it 'returns import source users with matches the search query' do
- expect(source_user_result).to match_array(source_user_1)
+ expect(source_user_result).to match_array([source_user_1])
end
end
end
--
GitLab
From ca2116884f9e8dccc4bee2f38b6591ff49e74a8b Mon Sep 17 00:00:00 2001
From: Rodrigo Tomonari <rtomonari@gitlab.com>
Date: Tue, 2 Jul 2024 15:49:29 -0300
Subject: [PATCH 3/6] Remove comments from migration
---
.../20240627142648_add_indexes_to_import_source_users.rb | 5 -----
1 file changed, 5 deletions(-)
diff --git a/db/migrate/20240627142648_add_indexes_to_import_source_users.rb b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
index 3742aad52066c0..e1caf3ced40827 100644
--- a/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
+++ b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
@@ -1,16 +1,11 @@
# frozen_string_literal: true
-# See https://docs.gitlab.com/ee/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
class AddIndexesToImportSourceUsers < Gitlab::Database::Migration[2.2]
disable_ddl_transaction!
milestone '17.2'
STATUS_INDEX = 'index_import_source_users_on_namespace_id_and_status'
- # SOURCE_NAME_INDEX = 'index_import_source_users_on_namespace_id_status_source_name'
-
SOURCE_NAME_INDEX = 'index_import_source_users_source_name_trgm'
SOURCE_USERNAME_INDEX = 'index_import_source_users_source_username_trgm'
--
GitLab
From 0a1646ee6ea88568dce1015fe301a11b98e1f4ab Mon Sep 17 00:00:00 2001
From: Rodrigo Tomonari <rtomonari@gitlab.com>
Date: Fri, 12 Jul 2024 10:55:51 -0300
Subject: [PATCH 4/6] Remove unnecessary indexes
---
...0627142648_add_indexes_to_import_source_users.rb | 13 +++++--------
db/structure.sql | 6 ------
spec/support/helpers/database/duplicate_indexes.yml | 3 ---
3 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/db/migrate/20240627142648_add_indexes_to_import_source_users.rb b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
index e1caf3ced40827..b7fddcacbe6ad6 100644
--- a/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
+++ b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
@@ -6,20 +6,17 @@ class AddIndexesToImportSourceUsers < Gitlab::Database::Migration[2.2]
milestone '17.2'
STATUS_INDEX = 'index_import_source_users_on_namespace_id_and_status'
- SOURCE_NAME_INDEX = 'index_import_source_users_source_name_trgm'
- SOURCE_USERNAME_INDEX = 'index_import_source_users_source_username_trgm'
+ NAMESPACE_INDEX = 'index_import_source_users_on_namespace_id'
def up
+ remove_concurrent_index_by_name :import_source_users, name: NAMESPACE_INDEX
+
add_concurrent_index :import_source_users, [:namespace_id, :status, :id], name: STATUS_INDEX
- add_concurrent_index :import_source_users, :source_name, name: SOURCE_NAME_INDEX, using: :gin,
- opclass: { path: :gin_trgm_ops }
- add_concurrent_index :import_source_users, :source_username, name: SOURCE_USERNAME_INDEX, using: :gin,
- opclass: { path: :gin_trgm_ops }
end
def down
+ add_concurrent_index :import_source_users, [:namespace_id], name: NAMESPACE_INDEX
+
remove_concurrent_index_by_name :import_source_users, name: STATUS_INDEX
- remove_concurrent_index_by_name :import_source_users, name: SOURCE_NAME_INDEX
- remove_concurrent_index_by_name :import_source_users, name: SOURCE_USERNAME_INDEX
end
end
diff --git a/db/structure.sql b/db/structure.sql
index aa1afc781e1756..36fdeb3622810d 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -27565,8 +27565,6 @@ CREATE INDEX index_import_source_user_placeholder_references_on_namespace_id ON
CREATE INDEX index_import_source_user_placeholder_references_on_source_user_ ON import_source_user_placeholder_references USING btree (source_user_id);
-CREATE INDEX index_import_source_users_on_namespace_id ON import_source_users USING btree (namespace_id);
-
CREATE INDEX index_import_source_users_on_namespace_id_and_status ON import_source_users USING btree (namespace_id, status, id);
CREATE INDEX index_import_source_users_on_placeholder_user_id ON import_source_users USING btree (placeholder_user_id);
@@ -27575,10 +27573,6 @@ CREATE INDEX index_import_source_users_on_reassign_to_user_id ON import_source_u
CREATE INDEX index_import_source_users_on_reassigned_by_user_id ON import_source_users USING btree (reassigned_by_user_id);
-CREATE INDEX index_import_source_users_source_name_trgm ON import_source_users USING gin (source_name gin_trgm_ops);
-
-CREATE INDEX index_import_source_users_source_username_trgm ON import_source_users USING gin (source_username gin_trgm_ops);
-
CREATE INDEX index_imported_projects_on_import_type_creator_id_created_at ON projects USING btree (import_type, creator_id, created_at) WHERE (import_type IS NOT NULL);
CREATE INDEX index_imported_projects_on_import_type_id ON projects USING btree (import_type, id) WHERE (import_type IS NOT NULL);
diff --git a/spec/support/helpers/database/duplicate_indexes.yml b/spec/support/helpers/database/duplicate_indexes.yml
index c9f763d09c43e4..784be0bf14085a 100644
--- a/spec/support/helpers/database/duplicate_indexes.yml
+++ b/spec/support/helpers/database/duplicate_indexes.yml
@@ -20,9 +20,6 @@ error_tracking_errors:
geo_node_namespace_links:
index_geo_node_namespace_links_on_geo_node_id_and_namespace_id:
- index_geo_node_namespace_links_on_geo_node_id
-import_source_users:
- index_import_source_users_on_namespace_id_and_status:
- - index_import_source_users_on_namespace_id
incident_management_oncall_participants:
index_inc_mgmnt_oncall_participants_on_user_id_and_rotation_id:
- index_inc_mgmnt_oncall_participants_on_oncall_user_id
--
GitLab
From c36fd652475e4055afe5e2949f58940dbde715b9 Mon Sep 17 00:00:00 2001
From: Rodrigo Tomonari <rtomonari@gitlab.com>
Date: Fri, 12 Jul 2024 11:58:15 -0300
Subject: [PATCH 5/6] Update migration milestone
---
db/migrate/20240627142648_add_indexes_to_import_source_users.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/db/migrate/20240627142648_add_indexes_to_import_source_users.rb b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
index b7fddcacbe6ad6..6519149a331fda 100644
--- a/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
+++ b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
@@ -3,7 +3,7 @@
class AddIndexesToImportSourceUsers < Gitlab::Database::Migration[2.2]
disable_ddl_transaction!
- milestone '17.2'
+ milestone '17.3'
STATUS_INDEX = 'index_import_source_users_on_namespace_id_and_status'
NAMESPACE_INDEX = 'index_import_source_users_on_namespace_id'
--
GitLab
From 432923d2877c16512459929b1fa1012b0f5ec857 Mon Sep 17 00:00:00 2001
From: Rodrigo Tomonari <rtomonari@gitlab.com>
Date: Mon, 15 Jul 2024 10:39:30 -0300
Subject: [PATCH 6/6] Change index order and remove :id from index
---
.../20240627142648_add_indexes_to_import_source_users.rb | 4 ++--
db/structure.sql | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/db/migrate/20240627142648_add_indexes_to_import_source_users.rb b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
index 6519149a331fda..60a2eba763e011 100644
--- a/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
+++ b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb
@@ -9,9 +9,9 @@ class AddIndexesToImportSourceUsers < Gitlab::Database::Migration[2.2]
NAMESPACE_INDEX = 'index_import_source_users_on_namespace_id'
def up
- remove_concurrent_index_by_name :import_source_users, name: NAMESPACE_INDEX
+ add_concurrent_index :import_source_users, [:namespace_id, :status], name: STATUS_INDEX
- add_concurrent_index :import_source_users, [:namespace_id, :status, :id], name: STATUS_INDEX
+ remove_concurrent_index_by_name :import_source_users, name: NAMESPACE_INDEX
end
def down
diff --git a/db/structure.sql b/db/structure.sql
index 36fdeb3622810d..0403afd7b7754a 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -27565,7 +27565,7 @@ CREATE INDEX index_import_source_user_placeholder_references_on_namespace_id ON
CREATE INDEX index_import_source_user_placeholder_references_on_source_user_ ON import_source_user_placeholder_references USING btree (source_user_id);
-CREATE INDEX index_import_source_users_on_namespace_id_and_status ON import_source_users USING btree (namespace_id, status, id);
+CREATE INDEX index_import_source_users_on_namespace_id_and_status ON import_source_users USING btree (namespace_id, status);
CREATE INDEX index_import_source_users_on_placeholder_user_id ON import_source_users USING btree (placeholder_user_id);
--
GitLab