diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 04dcf9ffae0..6327df7df0b 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -17,7 +17,7 @@ class Groups::GroupMembersController < Groups::ApplicationController before_action :authorize_read_group_member!, only: :index before_action only: [:index] do - push_frontend_feature_flag(:bulk_import_user_mapping, current_user) + push_frontend_feature_flag(:importer_user_mapping, current_user) push_frontend_feature_flag(:service_accounts_crud, @group) push_frontend_feature_flag(:webui_members_inherited_users, current_user) end diff --git a/app/controllers/import/source_users_controller.rb b/app/controllers/import/source_users_controller.rb index 560dd0c88df..b4f62373516 100644 --- a/app/controllers/import/source_users_controller.rb +++ b/app/controllers/import/source_users_controller.rb @@ -56,7 +56,7 @@ module Import end def check_feature_flag! - not_found unless Feature.enabled?(:bulk_import_user_mapping, current_user) + not_found unless Feature.enabled?(:importer_user_mapping, current_user) end # TODO: This is a placeholder for the proper UI to be provided diff --git a/app/finders/import/source_users_finder.rb b/app/finders/import/source_users_finder.rb index d4e573bfabc..f3bc3d5f0ba 100644 --- a/app/finders/import/source_users_finder.rb +++ b/app/finders/import/source_users_finder.rb @@ -11,7 +11,10 @@ module Import 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 @@ module Import 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/mutations/import/source_users/cancel_reassignment.rb b/app/graphql/mutations/import/source_users/cancel_reassignment.rb index 8b8bd34cfe2..3a83fe23230 100644 --- a/app/graphql/mutations/import/source_users/cancel_reassignment.rb +++ b/app/graphql/mutations/import/source_users/cancel_reassignment.rb @@ -18,8 +18,8 @@ module Mutations authorize :admin_import_source_user def resolve(args) - if Feature.disabled?(:bulk_import_user_mapping, current_user) - raise_resource_not_available_error! '`bulk_import_user_mapping` feature flag is disabled.' + if Feature.disabled?(:importer_user_mapping, current_user) + raise_resource_not_available_error! '`importer_user_mapping` feature flag is disabled.' end import_source_user = authorized_find!(id: args[:id]) diff --git a/app/graphql/mutations/import/source_users/keep_as_placeholder.rb b/app/graphql/mutations/import/source_users/keep_as_placeholder.rb index 7b83fa0ce82..85346d97d40 100644 --- a/app/graphql/mutations/import/source_users/keep_as_placeholder.rb +++ b/app/graphql/mutations/import/source_users/keep_as_placeholder.rb @@ -18,8 +18,8 @@ module Mutations authorize :admin_import_source_user def resolve(args) - if Feature.disabled?(:bulk_import_user_mapping, current_user) - raise_resource_not_available_error! '`bulk_import_user_mapping` feature flag is disabled.' + if Feature.disabled?(:importer_user_mapping, current_user) + raise_resource_not_available_error! '`importer_user_mapping` feature flag is disabled.' end import_source_user = authorized_find!(id: args[:id]) diff --git a/app/graphql/mutations/import/source_users/reassign.rb b/app/graphql/mutations/import/source_users/reassign.rb index 909dc919986..1d5e6f5b4d3 100644 --- a/app/graphql/mutations/import/source_users/reassign.rb +++ b/app/graphql/mutations/import/source_users/reassign.rb @@ -23,8 +23,8 @@ module Mutations authorize :admin_import_source_user def resolve(args) - if Feature.disabled?(:bulk_import_user_mapping, current_user) - raise_resource_not_available_error! '`bulk_import_user_mapping` feature flag is disabled.' + if Feature.disabled?(:importer_user_mapping, current_user) + raise_resource_not_available_error! '`importer_user_mapping` feature flag is disabled.' end import_source_user = authorized_find!(id: args[:id]) diff --git a/app/graphql/mutations/import/source_users/resend_notification.rb b/app/graphql/mutations/import/source_users/resend_notification.rb index 5ab281d278e..97e9552132f 100644 --- a/app/graphql/mutations/import/source_users/resend_notification.rb +++ b/app/graphql/mutations/import/source_users/resend_notification.rb @@ -18,8 +18,8 @@ module Mutations authorize :admin_import_source_user def resolve(args) - if Feature.disabled?(:bulk_import_user_mapping, current_user) - raise_resource_not_available_error! '`bulk_import_user_mapping` feature flag is disabled.' + if Feature.disabled?(:importer_user_mapping, current_user) + raise_resource_not_available_error! '`importer_user_mapping` feature flag is disabled.' end import_source_user = authorized_find!(id: args[:id]) diff --git a/app/graphql/resolvers/import/source_users_resolver.rb b/app/graphql/resolvers/import/source_users_resolver.rb index 57210e3b6b1..edc68d93510 100644 --- a/app/graphql/resolvers/import/source_users_resolver.rb +++ b/app/graphql/resolvers/import/source_users_resolver.rb @@ -11,10 +11,23 @@ module Resolvers 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) - return [] if Feature.disabled?(:bulk_import_user_mapping, current_user) + return [] if Feature.disabled?(:importer_user_mapping, current_user) apply_lookahead(::Import::SourceUsersFinder.new(namespace, context[:current_user], args).execute) end 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 00000000000..b7a8d4a4601 --- /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 967d5e0b41b..9b7b24b5305 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 @@ module Import 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 @@ module Import 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 - ) + 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) + + 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 fa6e63e35fb..44a4185aef9 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/app/services/import/source_users/base_service.rb b/app/services/import/source_users/base_service.rb index 2a9146dc012..94b784b904d 100644 --- a/app/services/import/source_users/base_service.rb +++ b/app/services/import/source_users/base_service.rb @@ -5,6 +5,8 @@ module Import class BaseService private + attr_reader :import_source_user, :current_user + def error_invalid_permissions ServiceResponse.error( message: s_('Import|You have insufficient permissions to update the import source user'), @@ -19,6 +21,10 @@ module Import payload: import_source_user ) end + + def send_user_reassign_email + Notify.import_source_user_reassign(import_source_user.id).deliver_now + end end end end diff --git a/app/services/import/source_users/cancel_reassignment_service.rb b/app/services/import/source_users/cancel_reassignment_service.rb index e4a3a57a7c7..65069d29742 100644 --- a/app/services/import/source_users/cancel_reassignment_service.rb +++ b/app/services/import/source_users/cancel_reassignment_service.rb @@ -21,8 +21,6 @@ module Import private - attr_reader :import_source_user, :current_user, :params - def cancel_reassignment import_source_user.reassign_to_user = nil import_source_user.reassigned_by_user = nil diff --git a/app/services/import/source_users/keep_as_placeholder_service.rb b/app/services/import/source_users/keep_as_placeholder_service.rb index 08c104d78ee..cad6908ad71 100644 --- a/app/services/import/source_users/keep_as_placeholder_service.rb +++ b/app/services/import/source_users/keep_as_placeholder_service.rb @@ -21,8 +21,6 @@ module Import private - attr_reader :import_source_user, :current_user, :params - def keep_as_placeholder import_source_user.reassigned_by_user = current_user import_source_user.keep_as_placeholder diff --git a/app/services/import/source_users/reassign_service.rb b/app/services/import/source_users/reassign_service.rb index e880e70468c..e872bd124ec 100644 --- a/app/services/import/source_users/reassign_service.rb +++ b/app/services/import/source_users/reassign_service.rb @@ -15,6 +15,8 @@ module Import return error_invalid_assignee unless valid_assignee?(assignee_user) if reassign_user + send_user_reassign_email + ServiceResponse.success(payload: import_source_user) else ServiceResponse.error(payload: import_source_user, message: import_source_user.errors.full_messages) @@ -23,7 +25,7 @@ module Import private - attr_reader :import_source_user, :current_user, :assignee_user + attr_reader :assignee_user def reassign_user import_source_user.reassign_to_user = assignee_user diff --git a/app/services/import/source_users/resend_notification_service.rb b/app/services/import/source_users/resend_notification_service.rb index e81e332e335..af714948c1d 100644 --- a/app/services/import/source_users/resend_notification_service.rb +++ b/app/services/import/source_users/resend_notification_service.rb @@ -12,14 +12,10 @@ module Import return error_invalid_permissions unless current_user.can?(:admin_import_source_user, import_source_user) return error_invalid_status unless import_source_user.awaiting_approval? - # Notifier will be added in https://gitlab.com/gitlab-org/gitlab/-/issues/455912 + send_user_reassign_email ServiceResponse.success(payload: import_source_user) end - - private - - attr_reader :import_source_user, :current_user end end end diff --git a/app/workers/bulk_imports/relation_export_worker.rb b/app/workers/bulk_imports/relation_export_worker.rb index c0934b786e5..4b6b284e864 100644 --- a/app/workers/bulk_imports/relation_export_worker.rb +++ b/app/workers/bulk_imports/relation_export_worker.rb @@ -38,7 +38,7 @@ module BulkImports log_extra_metadata_on_done(:batched, true) BatchedRelationExportService.new(user, portable, relation, jid).execute elsif config.user_contributions_relation?(relation) - return if Feature.disabled?(:bulk_import_user_mapping, user) + return if Feature.disabled?(:importer_user_mapping, user) log_extra_metadata_on_done(:batched, false) UserContributionsExportWorker.perform_async(portable_id, portable_class, user_id) diff --git a/app/workers/import/load_placeholder_references_worker.rb b/app/workers/import/load_placeholder_references_worker.rb index fa6089181b0..45f4dfe185b 100644 --- a/app/workers/import/load_placeholder_references_worker.rb +++ b/app/workers/import/load_placeholder_references_worker.rb @@ -11,7 +11,7 @@ module Import loggable_arguments 0, 1 def perform(import_source, import_uid, params = {}) - return unless Feature.enabled?(:bulk_import_user_mapping, User.actor_from_id(params['current_user_id'])) + return unless Feature.enabled?(:importer_user_mapping, User.actor_from_id(params['current_user_id'])) ::Import::PlaceholderReferences::LoadService.new( import_source: import_source, diff --git a/config/feature_flags/wip/bulk_import_user_mapping.yml b/config/feature_flags/wip/importer_user_mapping.yml similarity index 70% rename from config/feature_flags/wip/bulk_import_user_mapping.yml rename to config/feature_flags/wip/importer_user_mapping.yml index a882ddfc8e9..31a1d175491 100644 --- a/config/feature_flags/wip/bulk_import_user_mapping.yml +++ b/config/feature_flags/wip/importer_user_mapping.yml @@ -1,8 +1,8 @@ --- -name: bulk_import_user_mapping +name: importer_user_mapping feature_issue_url: https://gitlab.com/groups/gitlab-org/-/epics/12378 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/149735 -rollout_issue_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/472735 milestone: '17.0' group: group::import and integrate type: wip 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 00000000000..60a2eba763e --- /dev/null +++ b/db/migrate/20240627142648_add_indexes_to_import_source_users.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class AddIndexesToImportSourceUsers < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + + milestone '17.3' + + STATUS_INDEX = 'index_import_source_users_on_namespace_id_and_status' + NAMESPACE_INDEX = 'index_import_source_users_on_namespace_id' + + def up + add_concurrent_index :import_source_users, [:namespace_id, :status], name: STATUS_INDEX + + remove_concurrent_index_by_name :import_source_users, name: NAMESPACE_INDEX + 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 + end +end diff --git a/db/schema_migrations/20240627142648 b/db/schema_migrations/20240627142648 new file mode 100644 index 00000000000..3870b50f668 --- /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 5a69cf64236..b0f04457db2 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -27536,7 +27536,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 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); CREATE INDEX index_import_source_users_on_placeholder_user_id ON import_source_users USING btree (placeholder_user_id); diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 779d4a0aa3b..288a4fc9a39 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -22044,7 +22044,6 @@ GPG signature for a signed commit. | `googleCloudLoggingConfigurations` | [`GoogleCloudLoggingConfigurationTypeConnection`](#googlecloudloggingconfigurationtypeconnection) | Google Cloud logging configurations that receive audit events belonging to the group. (see [Connections](#connections)) | | `groupMembersCount` | [`Int!`](#int) | Count of direct members of this group. | | `id` | [`ID!`](#id) | ID of the namespace. | -| `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. | | `isAdjournedDeletionEnabled` **{warning-solid}** | [`Boolean!`](#boolean) | **Introduced** in GitLab 16.11. **Status**: Experiment. Indicates if delayed group deletion is enabled. | | `lfsEnabled` | [`Boolean`](#boolean) | Indicates if Large File Storage (LFS) is enabled for namespace. | | `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. | @@ -22607,6 +22606,28 @@ four standard [pagination arguments](#pagination-arguments): | `search` | [`String`](#string) | Search query. | | `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 | +| ---- | ---- | ----------- | +| `search` | [`String`](#string) | Query to search mappings by name or username of users on source instance. | +| `sort` | [`SourceUserSort`](#sourceusersort) | Sort mapping of users on source instance to users on destination instance by the criteria. | +| `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. @@ -26500,7 +26521,6 @@ Product analytics events for a specific month and year. | `fullName` | [`String!`](#string) | Full name of the namespace. | | `fullPath` | [`ID!`](#id) | Full path of the namespace. | | `id` | [`ID!`](#id) | ID of the namespace. | -| `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. | | `lfsEnabled` | [`Boolean`](#boolean) | Indicates if Large File Storage (LFS) is enabled for namespace. | | `name` | [`String!`](#string) | Name of the namespace. | | `packageSettings` | [`PackageSettings`](#packagesettings) | Package settings for the namespace. | @@ -26606,6 +26626,28 @@ four standard [pagination arguments](#pagination-arguments): | `ids` | [`[ComplianceManagementFrameworkID!]`](#compliancemanagementframeworkid) | List of Global IDs of compliance frameworks to return. | | `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 | +| ---- | ---- | ----------- | +| `search` | [`String`](#string) | Query to search mappings by name or username of users on source instance. | +| `sort` | [`SourceUserSort`](#sourceusersort) | Sort mapping of users on source instance to users on destination instance by the criteria. | +| `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. @@ -36256,6 +36298,17 @@ Values for sort direction. | `ASC` | Ascending order. | | `DESC` | Descending order. | +### `SourceUserSort` + +Values for sorting the mapping of users on source instance to users on destination instance. + +| Value | Description | +| ----- | ----------- | +| `SOURCE_NAME_ASC` | Instance source name by ascending order. | +| `SOURCE_NAME_DESC` | Instance source name by descending order. | +| `STATUS_ASC` | Status of the mapping by ascending order. | +| `STATUS_DESC` | Status of the mapping by descending order. | + ### `TestCaseStatus` | Value | Description | diff --git a/doc/user/analytics/value_streams_dashboard.md b/doc/user/analytics/value_streams_dashboard.md index 2f6be185783..61114510b42 100644 --- a/doc/user/analytics/value_streams_dashboard.md +++ b/doc/user/analytics/value_streams_dashboard.md @@ -118,13 +118,13 @@ panels: > - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/439737) in GitLab 16.9. > - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/440694) in GitLab 16.11. Feature flag `dora_performers_score_panel` removed. -The [DORA metrics](dora_metrics.md) Performers score panel is a group-level bar chart that visualizes the status of the organization's DevOps performance levels across different projects. +The [DORA](dora_metrics.md) Performers score panel is a group-level bar chart that visualizes the status of the organization's DevOps performance levels across different projects for the last full calendar month. The chart is a breakdown of your project's DORA scores, [categorized](https://cloud.google.com/blog/products/devops-sre/dora-2022-accelerate-state-of-devops-report-now-out) as high, medium, or low. The chart aggregates all the child projects in the group. -Each bar on the chart displays the sum of total projects per score category, calculated monthly. -To exclude data from the chart (for example, **Not Included**), in the legend select the series you want to exclude. +The chart bars display the total number of projects per score category, calculated monthly. +To exclude data from the chart (for example, **Not included**), in the legend select the series you want to exclude. Hovering over each bar reveals a dialog that explains the score's definition. For example, if a project has a high score for deployment frequency (velocity), it means that the project has one or more deploys to production per day. diff --git a/lib/gitlab/import_export/json/streaming_serializer.rb b/lib/gitlab/import_export/json/streaming_serializer.rb index f7b8c4954cd..9c684d20e09 100644 --- a/lib/gitlab/import_export/json/streaming_serializer.rb +++ b/lib/gitlab/import_export/json/streaming_serializer.rb @@ -257,7 +257,7 @@ module Gitlab end def after_read_callback(record) - if Feature.enabled?(:bulk_import_user_mapping, current_user) + if Feature.enabled?(:importer_user_mapping, current_user) user_contributions_export_mapper.cache_user_contributions_on_record(record) end diff --git a/package.json b/package.json index 41aed976acb..da01821f724 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,7 @@ "@gitlab/favicon-overlay": "2.0.0", "@gitlab/fonts": "^1.3.0", "@gitlab/svgs": "3.105.0", - "@gitlab/ui": "^86.11.0", + "@gitlab/ui": "86.11.1", "@gitlab/web-ide": "^0.0.1-dev-20240613133550", "@mattiasbuelens/web-streams-adapter": "^0.1.0", "@rails/actioncable": "7.0.8-4", diff --git a/spec/features/broadcast_messages_spec.rb b/spec/features/broadcast_messages_spec.rb index 2ca84fd0aef..c1a8e4d4b15 100644 --- a/spec/features/broadcast_messages_spec.rb +++ b/spec/features/broadcast_messages_spec.rb @@ -49,6 +49,26 @@ RSpec.describe 'Broadcast Messages', feature_category: :notifications do expect_message_dismissed end + + it 'broadcast message is still hidden after logout and log back in', :js do + gitlab_sign_in(user) + + visit path + + expect_to_be_on_explore_projects_page + + find('body.page-initialised .js-dismiss-current-broadcast-notification').click + + expect_message_dismissed + + gitlab_sign_out + + gitlab_sign_in(user) + + visit path + + expect_message_dismissed + end end describe 'banner type' do diff --git a/spec/finders/import/source_users_finder_spec.rb b/spec/finders/import/source_users_finder_spec.rb index 27a117a4b78..ba08c54440d 100644 --- a/spec/finders/import/source_users_finder_spec.rb +++ b/spec/finders/import/source_users_finder_spec.rb @@ -5,10 +5,23 @@ require 'spec_helper' 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 @@ RSpec.describe Import::SourceUsersFinder, feature_category: :importers do 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/frontend/diffs/components/shared/__snapshots__/findings_drawer_spec.js.snap b/spec/frontend/diffs/components/shared/__snapshots__/findings_drawer_spec.js.snap index fbc258c6d0f..05b67c22291 100644 --- a/spec/frontend/diffs/components/shared/__snapshots__/findings_drawer_spec.js.snap +++ b/spec/frontend/diffs/components/shared/__snapshots__/findings_drawer_spec.js.snap @@ -94,7 +94,11 @@ exports[`FindingsDrawer General Rendering matches the snapshot with detected bad - detected + + detected +

@@ -314,7 +318,11 @@ exports[`FindingsDrawer General Rendering matches the snapshot with dismissed ba - detected + + detected +

diff --git a/spec/frontend/releases/components/__snapshots__/issuable_stats_spec.js.snap b/spec/frontend/releases/components/__snapshots__/issuable_stats_spec.js.snap index 1858cfa05ba..d10ece3cba6 100644 --- a/spec/frontend/releases/components/__snapshots__/issuable_stats_spec.js.snap +++ b/spec/frontend/releases/components/__snapshots__/issuable_stats_spec.js.snap @@ -11,7 +11,11 @@ exports[`~/releases/components/issuable_stats.vue matches snapshot 1`] = ` - 10 + + 10 +
- Beta + + Beta +
- Experiment + + Experiment +
'`bulk_import_user_mapping` feature flag is disabled.' + 'message' => '`importer_user_mapping` feature flag is disabled.' ) ) end diff --git a/spec/requests/api/graphql/mutations/import/source_users/keep_as_placeholder_spec.rb b/spec/requests/api/graphql/mutations/import/source_users/keep_as_placeholder_spec.rb index df4b1d28b80..041a2105bad 100644 --- a/spec/requests/api/graphql/mutations/import/source_users/keep_as_placeholder_spec.rb +++ b/spec/requests/api/graphql/mutations/import/source_users/keep_as_placeholder_spec.rb @@ -67,9 +67,9 @@ RSpec.describe 'Keep as placeholder an import source user', feature_category: :i it_behaves_like 'a mutation that returns a top-level access error' end - context 'when feature flag `bulk_import_user_mapping`` disabled' do + context 'when feature flag `importer_user_mapping`` disabled' do before do - stub_feature_flags(bulk_import_user_mapping: false) + stub_feature_flags(importer_user_mapping: false) end it 'returns a resource not available error' do @@ -77,7 +77,7 @@ RSpec.describe 'Keep as placeholder an import source user', feature_category: :i expect(graphql_errors).to contain_exactly( hash_including( - 'message' => '`bulk_import_user_mapping` feature flag is disabled.' + 'message' => '`importer_user_mapping` feature flag is disabled.' ) ) end diff --git a/spec/requests/api/graphql/mutations/import/source_users/reassign_spec.rb b/spec/requests/api/graphql/mutations/import/source_users/reassign_spec.rb index 31d46c204e7..66647ace2eb 100644 --- a/spec/requests/api/graphql/mutations/import/source_users/reassign_spec.rb +++ b/spec/requests/api/graphql/mutations/import/source_users/reassign_spec.rb @@ -44,6 +44,8 @@ RSpec.describe 'Reassign an import source user', feature_category: :importers do end it 'reassign import source user', :aggregate_failures do + expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_now) + post_graphql_mutation(mutation, current_user: current_user) import_source_user = mutation_response['importSourceUser'] @@ -73,9 +75,9 @@ RSpec.describe 'Reassign an import source user', feature_category: :importers do it_behaves_like 'a mutation that returns a top-level access error' end - context 'when feature flag `bulk_import_user_mapping`` disabled' do + context 'when feature flag `importer_user_mapping`` disabled' do before do - stub_feature_flags(bulk_import_user_mapping: false) + stub_feature_flags(importer_user_mapping: false) end it 'returns a resource not available error' do @@ -83,7 +85,7 @@ RSpec.describe 'Reassign an import source user', feature_category: :importers do expect(graphql_errors).to contain_exactly( hash_including( - 'message' => '`bulk_import_user_mapping` feature flag is disabled.' + 'message' => '`importer_user_mapping` feature flag is disabled.' ) ) end diff --git a/spec/requests/api/graphql/mutations/import/source_users/resend_notification_spec.rb b/spec/requests/api/graphql/mutations/import/source_users/resend_notification_spec.rb index db2f4da2d78..d292e7dd209 100644 --- a/spec/requests/api/graphql/mutations/import/source_users/resend_notification_spec.rb +++ b/spec/requests/api/graphql/mutations/import/source_users/resend_notification_spec.rb @@ -41,6 +41,8 @@ RSpec.describe 'Resend notification to the reassigned user of an import source u context 'when user is authorized' do it 'resends notification and does not change status', :aggregate_failures do + expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_now) + post_graphql_mutation(mutation, current_user: current_user) import_source_user = mutation_response['importSourceUser'] @@ -69,9 +71,9 @@ RSpec.describe 'Resend notification to the reassigned user of an import source u end end - context 'when feature flag `bulk_import_user_mapping`` disabled' do + context 'when feature flag `importer_user_mapping`` disabled' do before do - stub_feature_flags(bulk_import_user_mapping: false) + stub_feature_flags(importer_user_mapping: false) end it 'returns a resource not available error' do @@ -79,7 +81,7 @@ RSpec.describe 'Resend notification to the reassigned user of an import source u expect(graphql_errors).to contain_exactly( hash_including( - 'message' => '`bulk_import_user_mapping` feature flag is disabled.' + 'message' => '`importer_user_mapping` feature flag is disabled.' ) ) end diff --git a/spec/requests/groups/group_members_controller_spec.rb b/spec/requests/groups/group_members_controller_spec.rb index eae2cdcab00..1c00a144af6 100644 --- a/spec/requests/groups/group_members_controller_spec.rb +++ b/spec/requests/groups/group_members_controller_spec.rb @@ -18,7 +18,7 @@ RSpec.describe Groups::GroupMembersController, feature_category: :groups_and_pro it 'pushes feature flag to frontend' do request - expect(response.body).to have_pushed_frontend_feature_flags(bulkImportUserMapping: true) + expect(response.body).to have_pushed_frontend_feature_flags(importerUserMapping: true) expect(response.body).to have_pushed_frontend_feature_flags(serviceAccountsCrud: true) expect(response.body).to have_pushed_frontend_feature_flags(webuiMembersInheritedUsers: true) end diff --git a/spec/requests/import/source_users_controller_spec.rb b/spec/requests/import/source_users_controller_spec.rb index e55b0cfac3e..7b0e9d943b2 100644 --- a/spec/requests/import/source_users_controller_spec.rb +++ b/spec/requests/import/source_users_controller_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Import::SourceUsersController, feature_category: :importers do shared_examples 'it requires feature flag' do context 'when :improved_user_mapping is disabled' do it 'returns 404' do - stub_feature_flags(bulk_import_user_mapping: false) + stub_feature_flags(importer_user_mapping: false) subject diff --git a/spec/services/import/source_users/reassign_service_spec.rb b/spec/services/import/source_users/reassign_service_spec.rb index d166af82cca..cf00ed011ff 100644 --- a/spec/services/import/source_users/reassign_service_spec.rb +++ b/spec/services/import/source_users/reassign_service_spec.rb @@ -12,6 +12,8 @@ RSpec.describe Import::SourceUsers::ReassignService, feature_category: :importer describe '#execute' do context 'when reassignment is successful' do it 'returns success' do + expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_now) + result = service.execute expect(result).to be_success @@ -22,58 +24,53 @@ RSpec.describe Import::SourceUsers::ReassignService, feature_category: :importer end end - context 'when current user does not have permission' do - let(:current_user) { create(:user) } + shared_examples 'an error response' do |desc, error:| + it "returns #{desc} error" do + expect(Notify).not_to receive(:import_source_user_reassign) - it 'returns error no permissions' do result = service.execute expect(result).to be_error - expect(result.message).to eq('You have insufficient permissions to update the import source user') + expect(result.message).to eq(error) end end + context 'when current user does not have permission' do + let(:current_user) { create(:user) } + + it_behaves_like 'an error response', 'no permissions', + error: 'You have insufficient permissions to update the import source user' + end + context 'when import source user does not have an reassignable status' do before do allow(current_user).to receive(:can?).with(:admin_import_source_user, import_source_user).and_return(true) allow(import_source_user).to receive(:reassignable_status?).and_return(false) end - it 'returns error invalid status' do - result = service.execute - expect(result).to be_error - expect(result.message).to eq('Import source user has an invalid status for this operation') - end + it_behaves_like 'an error response', 'invalid status', + error: 'Import source user has an invalid status for this operation' end context 'when assignee user does not exist' do let(:assignee_user) { nil } - it 'returns invalid assignee error' do - result = service.execute - expect(result).to be_error - expect(result.message).to eq('Only active regular, auditor, or administrator users can be assigned') - end + it_behaves_like 'an error response', 'invalid assignee', + error: 'Only active regular, auditor, or administrator users can be assigned' end context 'when assignee user is not a human' do let(:assignee_user) { create(:user, :bot) } - it 'returns invalid assignee error' do - result = service.execute - expect(result).to be_error - expect(result.message).to eq('Only active regular, auditor, or administrator users can be assigned') - end + it_behaves_like 'an error response', 'invalid assignee', + error: 'Only active regular, auditor, or administrator users can be assigned' end context 'when assignee user is not active' do let(:assignee_user) { create(:user, :deactivated) } - it 'returns invalid assignee error' do - result = service.execute - expect(result).to be_error - expect(result.message).to eq('Only active regular, auditor, or administrator users can be assigned') - end + it_behaves_like 'an error response', 'invalid assignee', + error: 'Only active regular, auditor, or administrator users can be assigned' end context 'when an error occurs' do @@ -83,12 +80,7 @@ RSpec.describe Import::SourceUsers::ReassignService, feature_category: :importer full_messages: ['Error'])) end - it 'returns an error' do - result = service.execute - - expect(result).to be_error - expect(result.message).to eq(['Error']) - end + it_behaves_like 'an error response', 'active record', error: ['Error'] end end end diff --git a/spec/services/import/source_users/resend_notification_service_spec.rb b/spec/services/import/source_users/resend_notification_service_spec.rb index 0dc49d00824..0144a1100e0 100644 --- a/spec/services/import/source_users/resend_notification_service_spec.rb +++ b/spec/services/import/source_users/resend_notification_service_spec.rb @@ -11,6 +11,8 @@ RSpec.describe Import::SourceUsers::ResendNotificationService, feature_category: describe '#execute' do context 'when notification is successfully sent' do it 'returns success' do + expect(Notify).to receive_message_chain(:import_source_user_reassign, :deliver_now) + result = service.execute expect(result).to be_success @@ -22,6 +24,8 @@ RSpec.describe Import::SourceUsers::ResendNotificationService, feature_category: let(:current_user) { create(:user) } it 'returns error no permissions' do + expect(Notify).not_to receive(:import_source_user_reassign) + result = service.execute expect(result).to be_error @@ -35,6 +39,8 @@ RSpec.describe Import::SourceUsers::ResendNotificationService, feature_category: end it 'returns error invalid status' do + expect(Notify).not_to receive(:import_source_user_reassign) + result = service.execute expect(result).to be_error expect(result.message).to eq('Import source user has an invalid status for this operation') diff --git a/spec/workers/bulk_imports/relation_export_worker_spec.rb b/spec/workers/bulk_imports/relation_export_worker_spec.rb index 12f949ce042..6c0ce0dbb38 100644 --- a/spec/workers/bulk_imports/relation_export_worker_spec.rb +++ b/spec/workers/bulk_imports/relation_export_worker_spec.rb @@ -65,7 +65,7 @@ RSpec.describe BulkImports::RelationExportWorker, feature_category: :importers d context 'when export is user_contributions' do let(:relation) { 'user_contributions' } - context 'and :bulk_import_user_mapping feature flag is enabled' do + context 'and :importer_user_mapping feature flag is enabled' do it 'enqueues the UserContributionsExportWorker' do expect(BulkImports::UserContributionsExportWorker).to receive(:perform_async).with( group.id, group.class.name, user.id @@ -75,9 +75,9 @@ RSpec.describe BulkImports::RelationExportWorker, feature_category: :importers d end end - context 'and :bulk_import_user_mapping feature flag is disabled' do + context 'and :importer_user_mapping feature flag is disabled' do before do - stub_feature_flags(bulk_import_user_mapping: false) + stub_feature_flags(importer_user_mapping: false) end it 'does not enqueue the UserContributionsExportWorker' do diff --git a/spec/workers/import/load_placeholder_references_worker_spec.rb b/spec/workers/import/load_placeholder_references_worker_spec.rb index 92b53cd9777..e134b9603ea 100644 --- a/spec/workers/import/load_placeholder_references_worker_spec.rb +++ b/spec/workers/import/load_placeholder_references_worker_spec.rb @@ -23,9 +23,9 @@ RSpec.describe Import::LoadPlaceholderReferencesWorker, feature_category: :impor let(:job_args) { [import_source, uid, params] } end - context 'when bulk_import_user_mapping feature is disabled' do + context 'when importer_user_mapping feature is disabled' do before do - stub_feature_flags(bulk_import_user_mapping: false) + stub_feature_flags(importer_user_mapping: false) end it 'does not execute LoadService' do @@ -34,9 +34,9 @@ RSpec.describe Import::LoadPlaceholderReferencesWorker, feature_category: :impor perform end - context 'when bulk_import_user_mapping feature is enabled for the user' do + context 'when importer_user_mapping feature is enabled for the user' do before do - stub_feature_flags(bulk_import_user_mapping: user) + stub_feature_flags(importer_user_mapping: user) end it 'executes LoadService' do diff --git a/yarn.lock b/yarn.lock index 86509f924fe..e45f44902d6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1359,10 +1359,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-3.105.0.tgz#0d15978755093b79e5c9b883a27d8074bf316e9a" integrity sha512-JrXE7T3j+9FyQakG4XVg/uhXL3TZvmFgTuggaiSXdyelVOqAzYdPPm1oergdp8C+Io0zBKlLkJv/4nWG3AgkfQ== -"@gitlab/ui@^86.11.0": - version "86.11.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-86.11.0.tgz#622fa5659f5ff241679efbc5cdad396807dff3b4" - integrity sha512-9+zZaKrpYySD3kkZFZ9ib/n8fAcavLZ2QC+9o5dvaKfQ54P4hRFQqUJpO6IXQufDAAkdBriPSw1RETa9lBkbpg== +"@gitlab/ui@86.11.1": + version "86.11.1" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-86.11.1.tgz#5de5de9416946375555ecb54f1510dd4323f2abf" + integrity sha512-/KvXyMGMJ3+Qq67EkqC9Rgju9rKmOKqE1Sq8yam3fZ2ZgcQQ8RYbHmJuKsGufcdCUBAfIp6qiI++QzZMJRU6mw== dependencies: "@floating-ui/dom" "1.4.3" echarts "^5.3.2"