diff --git a/app/assets/javascripts/import_entities/constants.js b/app/assets/javascripts/import_entities/constants.js index 6f1ca92d80d1e91ee20df93fd48bde2b3344c480..37bc0ca058df758a6b5aa38336462bffe585a46e 100644 --- a/app/assets/javascripts/import_entities/constants.js +++ b/app/assets/javascripts/import_entities/constants.js @@ -11,6 +11,7 @@ export const STATUSES = { CANCELED: 'canceled', TIMEOUT: 'timeout', PARTIAL: 'partial', // only present client-side, finished but with failures + SKIPPED: 'skipped', }; export const PROVIDERS = { @@ -66,4 +67,9 @@ export const STATUS_ICON_MAP = { text: s__('Import|Partially completed'), variant: 'warning', }, + [STATUSES.SKIPPED]: { + icon: 'status-alert', + text: s__('Import|Skipped'), + variant: 'warning', + }, }; diff --git a/app/assets/javascripts/import_entities/import_groups/components/import_table.vue b/app/assets/javascripts/import_entities/import_groups/components/import_table.vue index 27bfdef50db92bb8fb3d9a9c475816903e895093..6d29d16bf9163cce6bef5c05cff8ec220ba9454c 100644 --- a/app/assets/javascripts/import_entities/import_groups/components/import_table.vue +++ b/app/assets/javascripts/import_entities/import_groups/components/import_table.vue @@ -117,6 +117,7 @@ export default { importTargets: {}, unavailableFeaturesAlertVisible: true, shouldMigrateMemberships: true, + shouldMigrateBannedContributions: false, }; }, @@ -460,6 +461,7 @@ export default { targetNamespace: group.importTarget.targetNamespace.fullPath, newName: group.importTarget.newName, migrateMemberships: this.shouldMigrateMemberships, + migrateBannedContributions: this.shouldMigrateBannedContributions, ...extraArgs, }, ]); @@ -484,6 +486,7 @@ export default { targetNamespace: group.importTarget.targetNamespace.fullPath, newName: group.importTarget.newName, migrateMemberships: this.shouldMigrateMemberships, + migrateBannedContributions: this.shouldMigrateBannedContributions, ...extraArgs, })); @@ -657,6 +660,7 @@ export default { }), popoverOptions: { title: __('What is listed here?') }, learnMoreOptions: { title: s__('BulkImport|Import user memberships') }, + learnMoreBannedContributions: { title: s__('BulkImport|Import banned contributions') }, i18n, LOCAL_STORAGE_KEY: 'gl-bulk-imports-status-page-size-v1', }; @@ -859,6 +863,22 @@ export default { /> +
+ + {{ s__('BulkImport|Import banned contributions') }} + + + + +
:canceled end + event :skip do + transition any => :skipped + end + # rubocop:disable Style/SymbolProc after_transition any => [:finished, :failed, :timeout] do |entity| entity.update_has_failures diff --git a/app/models/note.rb b/app/models/note.rb index f1d1a18e5048f37bce0547791cc9fbde118e7916..cb7b6cae2603c2dd807b0906bcc9d4b2d6695881 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -713,6 +713,16 @@ def attribute_names_for_serialization attributes.keys end + def uploads_sharding_key + { namespace_id: namespace_id } + end + + def hidden? + return false if Feature.disabled?(:hidden_notes) + + author&.banned? + end + private def trigger_note_subscription? diff --git a/app/services/bulk_imports/create_service.rb b/app/services/bulk_imports/create_service.rb index 45703bfd538a4664bf7188161d104fbbbd11655f..9945b70e3dd1875b0ad2d5a96551be5d22aa789d 100644 --- a/app/services/bulk_imports/create_service.rb +++ b/app/services/bulk_imports/create_service.rb @@ -110,7 +110,10 @@ def create_bulk_import destination_slug: entity_params[:destination_slug] || entity_params[:destination_name], destination_namespace: entity_params[:destination_namespace], migrate_projects: Gitlab::Utils.to_boolean(entity_params[:migrate_projects], default: true), - migrate_memberships: Gitlab::Utils.to_boolean(entity_params[:migrate_memberships], default: true) + migrate_memberships: Gitlab::Utils.to_boolean(entity_params[:migrate_memberships], default: true), + migrate_banned_contributions: Gitlab::Utils.to_boolean( + entity_params[:migrate_banned_contributions], default: false + ) ) end bulk_import diff --git a/app/services/bulk_imports/process_service.rb b/app/services/bulk_imports/process_service.rb index 01ee2d6125226d3e57d9232c592edd9716f810e8..9546bd87d951542d5b24beb23881499cb0b9950f 100644 --- a/app/services/bulk_imports/process_service.rb +++ b/app/services/bulk_imports/process_service.rb @@ -47,7 +47,7 @@ def created_entities end def all_entities_processed? - entities.all? { |entity| entity.finished? || entity.failed? } + entities.all? { |entity| entity.finished? || entity.failed? || entity.skipped? } end def all_entities_failed? diff --git a/app/workers/bulk_imports/pipeline_worker.rb b/app/workers/bulk_imports/pipeline_worker.rb index d5fdaa72a986f3b3f1fff82b9285c3660621161b..9f16458fdab4fd85830877f343d696add6c1f308 100644 --- a/app/workers/bulk_imports/pipeline_worker.rb +++ b/app/workers/bulk_imports/pipeline_worker.rb @@ -187,6 +187,8 @@ def retry_tracker(exception) def invalid_entity_status? if entity.failed? handle_invalid_status('skip', 'Skipping pipeline due to failed entity') + elsif entity.skipped? + handle_invalid_status('skip', 'Skipping pipeline due to skipped entity') elsif entity.timeout? handle_invalid_status('cleanup_stale', 'Timeout pipeline due to timeout entity') elsif entity.canceled? diff --git a/db/migrate/20250320150755_add_migrate_banned_contributions_column_to_bulk_import_entities.rb b/db/migrate/20250320150755_add_migrate_banned_contributions_column_to_bulk_import_entities.rb new file mode 100644 index 0000000000000000000000000000000000000000..0172b608f8d413d6d65a7db0d9e4d4208f914679 --- /dev/null +++ b/db/migrate/20250320150755_add_migrate_banned_contributions_column_to_bulk_import_entities.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddMigrateBannedContributionsColumnToBulkImportEntities < Gitlab::Database::Migration[2.2] + milestone '17.11' + + def change + add_column :bulk_import_entities, :migrate_banned_contributions, :boolean, default: false, null: false + end +end diff --git a/db/schema_migrations/20250320150755 b/db/schema_migrations/20250320150755 new file mode 100644 index 0000000000000000000000000000000000000000..c9049abeff884a895ea5ba33aa22170dfa7ffc7b --- /dev/null +++ b/db/schema_migrations/20250320150755 @@ -0,0 +1 @@ +dd37d0919139f9dc04a53d3d801919a563d745e445400317198c2d1d2510b93b \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 7bcb164c3f8b01bbfaef4b0bdf9231b4c90fe13d..efe856d4b857b7ca8c18e25a5c1dad5a29ac16ad 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10485,6 +10485,7 @@ CREATE TABLE bulk_import_entities ( has_failures boolean DEFAULT false, migrate_memberships boolean DEFAULT true NOT NULL, organization_id bigint, + migrate_banned_contributions boolean DEFAULT false NOT NULL, CONSTRAINT check_13f279f7da CHECK ((char_length(source_full_path) <= 255)), CONSTRAINT check_469f9235c5 CHECK ((num_nonnulls(namespace_id, organization_id, project_id) = 1)), CONSTRAINT check_715d725ea2 CHECK ((char_length(destination_name) <= 255)), diff --git a/doc/api/bulk_imports.md b/doc/api/bulk_imports.md index d2754bbf94352eb77f70b28320af83e23705ca0d..89efa4e7429a72ac110bb4269ce544e9d15b6112 100644 --- a/doc/api/bulk_imports.md +++ b/doc/api/bulk_imports.md @@ -50,19 +50,20 @@ Use this endpoint to start a new group or project migration. Specify: POST /bulk_imports ``` -| Attribute | Type | Required | Description | -| --------------------------------- | ------ | -------- | ----------- | -| `configuration` | Hash | yes | The source GitLab instance configuration. | -| `configuration[url]` | String | yes | Source GitLab instance URL. | -| `configuration[access_token]` | String | yes | Access token to the source GitLab instance. | -| `entities` | Array | yes | List of entities to import. | -| `entities[source_type]` | String | yes | Source entity type. Valid values are `group_entity` and `project_entity` (GitLab 15.11 and later). | -| `entities[source_full_path]` | String | yes | Source full path of the entity to import. For example, `gitlab-org/gitlab`. | -| `entities[destination_slug]` | String | yes | Destination slug for the entity. GitLab uses the slug as the URL path to the entity. The name of the imported entity is copied from the name of the source entity and not the slug. | -| `entities[destination_name]` | String | no | Deprecated: Use `destination_slug` instead. Destination slug for the entity. | -| `entities[destination_namespace]` | String | yes | Full path of the destination group [namespace](../user/namespace/_index.md) for the entity. Must be an existing group in the destination instance. | -| `entities[migrate_projects]` | Boolean | no | Also import all nested projects of the group (if `source_type` is `group_entity`). Defaults to `true`. | -| `entities[migrate_memberships]` | Boolean | no | Import user memberships. Defaults to `true`. | +| Attribute | Type | Required | Description | +|------------------------------------------| ------ | -------- |--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `configuration` | Hash | yes | The source GitLab instance configuration. | +| `configuration[url]` | String | yes | Source GitLab instance URL. | +| `configuration[access_token]` | String | yes | Access token to the source GitLab instance. | +| `entities` | Array | yes | List of entities to import. | +| `entities[source_type]` | String | yes | Source entity type. Valid values are `group_entity` and `project_entity` (GitLab 15.11 and later). | +| `entities[source_full_path]` | String | yes | Source full path of the entity to import. For example, `gitlab-org/gitlab`. | +| `entities[destination_slug]` | String | yes | Destination slug for the entity. GitLab uses the slug as the URL path to the entity. The name of the imported entity is copied from the name of the source entity and not the slug. | +| `entities[destination_name]` | String | no | Deprecated: Use `destination_slug` instead. Destination slug for the entity. | +| `entities[destination_namespace]` | String | yes | Full path of the destination group [namespace](../user/namespace/_index.md) for the entity. Must be an existing group in the destination instance. | +| `entities[migrate_projects]` | Boolean | no | Also import all nested projects of the group (if `source_type` is `group_entity`). Defaults to `true`. | +| `entities[migrate_memberships]` | Boolean | no | Import user memberships. Defaults to `true`. | +| `entities[migrate_banned_contributions]` | Boolean | no | Import banned user contributions. Defaults to `false`. For this feature to work, the `hide_merge_requests_from_banned_users` and `hidden_notes` (GitLab version at least 17.11) flags must be enabled on the source instance. | ```shell curl --request POST \ @@ -192,6 +193,7 @@ curl --request GET \ "failures": [], "migrate_projects": true, "migrate_memberships": true, + "migrate_banned_contributions": true, "has_failures": false, "stats": { "labels": { diff --git a/doc/api/openapi/openapi_v2.yaml b/doc/api/openapi/openapi_v2.yaml index 3fdfc2e9fb7bb69ec0a790286412f20e8072fd8d..10efffb507fb5e36a910452da27cece5addb6409 100644 --- a/doc/api/openapi/openapi_v2.yaml +++ b/doc/api/openapi/openapi_v2.yaml @@ -37073,6 +37073,14 @@ paths: required: false items: type: boolean + - in: formData + name: entities[migrate_banned_contributions] + description: The option to migrate banned contributions + type: array + default: false + required: false + items: + type: boolean responses: '200': description: Start a new GitLab Migration @@ -37195,6 +37203,7 @@ paths: - timeout - failed - canceled + - skipped required: false responses: '200': @@ -37263,6 +37272,7 @@ paths: - timeout - failed - canceled + - skipped required: false - in: query name: page @@ -64727,6 +64737,9 @@ definitions: example: false stats: type: object + migrate_banned_contributions: + type: boolean + example: false description: API_Entities_BulkImports model API_Entities_BulkImports_EntityFailure: type: object diff --git a/doc/user/group/import/direct_transfer_migrations.md b/doc/user/group/import/direct_transfer_migrations.md index c5d84046ab17debff35aea392f7586f526399e28..5a4bafdcf3abac5ec12b5bb8cc5370bc6f445569 100644 --- a/doc/user/group/import/direct_transfer_migrations.md +++ b/doc/user/group/import/direct_transfer_migrations.md @@ -164,13 +164,26 @@ On the destination GitLab instance, create the group you want to import to and c - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/385689) in GitLab 15.8, option to import groups with or without projects. - **Import user memberships** checkbox [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/477734) in GitLab 17.6. +- **Import banned contributions** checkbox [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/508111) in GitLab 17.11 [with flags](../../../administration/feature_flags.md) named `hide_merge_requests_from_banned_users`, and `hidden_notes`. Disabled by default. {{< /history >}} +{{< alert type="flag" >}} + +The availability of this feature is controlled by a feature flag. +For more information, see the history. + +{{< /alert >}} + After you have authorized access to the source GitLab instance, you are redirected to the GitLab group importer page. Here you can see a list of the top-level groups on the connected source instance where you have the Owner role. If you do not want to import all user memberships from the source instance, ensure the **Import user memberships** checkbox is cleared. For example, the source instance might have 200 members, but you might want to import 50 members only. After the import completes, you can add more members to groups and projects. +To import contributions from banned users, ensure the **Import banned contributions** checkbox is selected. +These contributions include projects, merge requests, issues, and comments. +For this feature to work, the `hide_merge_requests_from_banned_users` +and `hidden_notes` (GitLab version at least 17.11) flags must be enabled on the source instance. + 1. By default, the proposed group namespaces match the names as they exist in source instance, but based on your permissions, you can choose to edit these names before you proceed to import any of them. Group and project paths must conform to [naming rules](../../reserved_names.md#rules-for-usernames-project-and-group-names-and-slugs) and are normalized if necessary to avoid import failures. 1. Next to the groups you want to import, select either: - **Import with projects**. If this is not available, see [prerequisites](#prerequisites). diff --git a/lib/api/bulk_imports.rb b/lib/api/bulk_imports.rb index a42245bf732a62c84699d0ec2a2dae6ea16ccf5b..7c367a97f4cd2fcbda2673dcb307e6d5046cad1f 100644 --- a/lib/api/bulk_imports.rb +++ b/lib/api/bulk_imports.rb @@ -90,6 +90,10 @@ def bulk_import_entity type: Boolean, default: true, desc: 'The option to migrate memberships or not' + optional :migrate_banned_contributions, + type: Boolean, + default: false, + desc: 'The option to migrate banned contributions' mutually_exclusive :destination_slug, :destination_name at_least_one_of :destination_slug, :destination_name diff --git a/lib/api/entities/bulk_imports/entity.rb b/lib/api/entities/bulk_imports/entity.rb index 18afe96abd1c3759a34087949d4e5e10bce249c3..e37aeeeacdfbb8fda019929b03d740f2f7315eb2 100644 --- a/lib/api/entities/bulk_imports/entity.rb +++ b/lib/api/entities/bulk_imports/entity.rb @@ -27,6 +27,7 @@ class Entity < Grape::Entity expose :migrate_memberships, documentation: { type: 'boolean', example: true } expose :has_failures, documentation: { type: 'boolean', example: false } expose :checksums, as: :stats, documentation: { type: 'object' } + expose :migrate_banned_contributions, documentation: { type: 'boolean', example: false } end end end diff --git a/lib/bulk_imports/common/graphql/get_members_query.rb b/lib/bulk_imports/common/graphql/get_members_query.rb index 9fcd1da7013dab58987bf0d2d0a025faba084f9e..c73341f7005a2c44848744140268507512e8bff5 100644 --- a/lib/bulk_imports/common/graphql/get_members_query.rb +++ b/lib/bulk_imports/common/graphql/get_members_query.rb @@ -31,6 +31,7 @@ def to_s public_email: publicEmail username: username name: name + state: state } } } diff --git a/lib/bulk_imports/common/rest/get_project_query.rb b/lib/bulk_imports/common/rest/get_project_query.rb new file mode 100644 index 0000000000000000000000000000000000000000..ebcc695fea00cf420d2a1e456c5c6413594a96f2 --- /dev/null +++ b/lib/bulk_imports/common/rest/get_project_query.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module BulkImports # rubocop:disable Gitlab/BoundedContexts -- legacy use + module Common + module Rest + module GetProjectQuery + extend self + + def to_h(context) + { + resource: ['projects', context.entity.encoded_source_full_path].join('/'), + query: {} + } + end + end + end + end +end diff --git a/lib/bulk_imports/common/rest/get_user_query.rb b/lib/bulk_imports/common/rest/get_user_query.rb new file mode 100644 index 0000000000000000000000000000000000000000..51d2cd3ce4eaea3f9df6034a988d5fd71ce8f88c --- /dev/null +++ b/lib/bulk_imports/common/rest/get_user_query.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module BulkImports # rubocop:disable Gitlab/BoundedContexts -- legacy use + module Common + module Rest + module GetUserQuery + extend self + + def to_h(context) + { + resource: ['users', context.extra[:user_id]].join('/'), + query: {} + } + end + end + end + end +end diff --git a/lib/bulk_imports/common/transformers/member_attributes_transformer.rb b/lib/bulk_imports/common/transformers/member_attributes_transformer.rb index 54f8be08a9a7781cfb905bb3aaf810e464a8fee5..851b135004ad5a27baa282dfc6c2ccde24c50fef 100644 --- a/lib/bulk_imports/common/transformers/member_attributes_transformer.rb +++ b/lib/bulk_imports/common/transformers/member_attributes_transformer.rb @@ -9,8 +9,10 @@ def transform(context, data) user = find_user(data&.dig('user', 'public_email')) access_level = data&.dig('access_level', 'integer_value') + user_state = data&.dig('user', 'state') return unless data + return if skip_banned_user?(user_state, context) return unless user return unless valid_access_level?(access_level) @@ -54,6 +56,13 @@ def cache_source_user_data(data, user, context) mapper.cache_source_username(source_username, user.username) end + + def skip_banned_user?(user_state, context) + return if context.entity.migrate_banned_contributions + return unless user_state == 'banned' + + true + end end end end diff --git a/lib/bulk_imports/groups/pipelines/project_entities_pipeline.rb b/lib/bulk_imports/groups/pipelines/project_entities_pipeline.rb index 1d224d55128139687c6b33761a4cc9adf12d294a..e1a3c40a74383c7947ca685ddb35fe10c2b6dc37 100644 --- a/lib/bulk_imports/groups/pipelines/project_entities_pipeline.rb +++ b/lib/bulk_imports/groups/pipelines/project_entities_pipeline.rb @@ -17,7 +17,8 @@ def transform(context, data) destination_name: data['path'], destination_namespace: context.entity.group.full_path, parent_id: context.entity.id, - source_xid: GlobalID.parse(data['id']).model_id + source_xid: GlobalID.parse(data['id']).model_id, + migrate_banned_contributions: context.entity.migrate_banned_contributions } end diff --git a/lib/bulk_imports/groups/transformers/subgroup_to_entity_transformer.rb b/lib/bulk_imports/groups/transformers/subgroup_to_entity_transformer.rb index fd44ad7f95db94dd55cc76ef7c9415acd014f17c..7ca6c2df698391e3f7246b653925e9aee4b0b108 100644 --- a/lib/bulk_imports/groups/transformers/subgroup_to_entity_transformer.rb +++ b/lib/bulk_imports/groups/transformers/subgroup_to_entity_transformer.rb @@ -13,7 +13,8 @@ def transform(context, entry) organization_id: context.entity.group.organization_id, parent_id: context.entity.id, migrate_projects: context.entity.migrate_projects, - migrate_memberships: context.entity.migrate_memberships + migrate_memberships: context.entity.migrate_memberships, + migrate_banned_contributions: context.entity.migrate_banned_contributions } end end diff --git a/lib/bulk_imports/ndjson_pipeline.rb b/lib/bulk_imports/ndjson_pipeline.rb index 122ad48db1f5554c9c2be675cbbdbe1c194ec9f0..b76aa70c3b2c881b379bd3ba4d6744795732afdd 100644 --- a/lib/bulk_imports/ndjson_pipeline.rb +++ b/lib/bulk_imports/ndjson_pipeline.rb @@ -42,11 +42,12 @@ def transform(context, data) excluded_keys: import_export_config.relation_excluded_keys(key), import_source: Import::SOURCE_DIRECT_TRANSFER, original_users_map: original_users_map, - rewrite_mentions: context.importer_user_mapping_enabled? + rewrite_mentions: context.importer_user_mapping_enabled?, + migrate_banned_contributions: context.entity.migrate_banned_contributions ) end - relation_object.assign_attributes(portable_class_sym => portable) + relation_object.assign_attributes(portable_class_sym => portable) if relation_object.presence [relation_object, original_users_map] end diff --git a/lib/bulk_imports/pipeline/runner.rb b/lib/bulk_imports/pipeline/runner.rb index 650f6399d0ac26eb84f653ca532073ed066f4144..a151ff1f3100a17f14e63d043ebe76715e484b92 100644 --- a/lib/bulk_imports/pipeline/runner.rb +++ b/lib/bulk_imports/pipeline/runner.rb @@ -6,9 +6,11 @@ module Runner extend ActiveSupport::Concern MarkedAsFailedError = Class.new(StandardError) + MarkedAsSkippedError = Class.new(StandardError) def run raise MarkedAsFailedError if context.entity.failed? + raise MarkedAsSkippedError if context.entity.skipped? info(message: 'Pipeline started') @@ -62,6 +64,8 @@ def run info(message: 'Pipeline finished') rescue MarkedAsFailedError skip!('Skipping pipeline due to failed entity') + rescue MarkedAsSkippedError + skip!('Skipping pipeline due to skipped entity') end def on_finish; end diff --git a/lib/bulk_imports/projects/pipelines/project_pipeline.rb b/lib/bulk_imports/projects/pipelines/project_pipeline.rb index 03b9a8302e345d206c488cc830dcd88924d2efb2..34ab56c45bf9c07f716c327923132573b44fa559 100644 --- a/lib/bulk_imports/projects/pipelines/project_pipeline.rb +++ b/lib/bulk_imports/projects/pipelines/project_pipeline.rb @@ -9,10 +9,24 @@ class ProjectPipeline abort_on_failure! - extractor ::BulkImports::Common::Extractors::GraphqlExtractor, query: Graphql::GetProjectQuery transformer ::BulkImports::Common::Transformers::ProhibitedAttributesTransformer transformer ::BulkImports::Projects::Transformers::ProjectAttributesTransformer + def extract(context) + project_data = project_extractor.extract(context) + + return project_data if context.entity.migrate_banned_contributions + + context.extra[:user_id] = project_data.data.first['creator_id'] + creator_data = user_extractor.extract(context) + + return project_data unless creator_data.data.first['state'] == 'banned' + + context.entity.skip! + + BulkImports::Pipeline::ExtractedData.new(data: nil) + end + def load(context, data) project = ::Projects::CreateService.new(context.current_user, data).execute @@ -24,6 +38,16 @@ def load(context, data) raise(::BulkImports::Error, "Unable to import project #{project.full_path}. #{project.errors.full_messages}.") end end + + private + + def project_extractor + @project_extractor ||= BulkImports::Common::Extractors::RestExtractor.new(query: BulkImports::Common::Rest::GetProjectQuery) + end + + def user_extractor + @user_extractor ||= BulkImports::Common::Extractors::RestExtractor.new(query: BulkImports::Common::Rest::GetUserQuery) + end end end end diff --git a/lib/gitlab/import_export/base/relation_factory.rb b/lib/gitlab/import_export/base/relation_factory.rb index 51c38cd7018b3c7e84c2d2952060f388fae51da3..85458444f0f98ad6dc65a33c5ee5da7e1f0d7482 100644 --- a/lib/gitlab/import_export/base/relation_factory.rb +++ b/lib/gitlab/import_export/base/relation_factory.rb @@ -51,7 +51,7 @@ def self.relation_class(relation_name) end # rubocop:disable Metrics/ParameterLists -- Keyword arguments are not adding complexity to initializer - def initialize(relation_sym:, relation_index:, relation_hash:, members_mapper:, object_builder:, user:, importable:, import_source:, excluded_keys: [], original_users_map: nil, rewrite_mentions: false) + def initialize(relation_sym:, relation_index:, relation_hash:, members_mapper:, object_builder:, user:, importable:, import_source:, excluded_keys: [], original_users_map: nil, rewrite_mentions: false, migrate_banned_contributions: false) @relation_sym = relation_sym @relation_name = self.class.overrides[relation_sym]&.to_sym || relation_sym @relation_index = relation_index @@ -66,6 +66,7 @@ def initialize(relation_sym:, relation_index:, relation_hash:, members_mapper:, @original_user = {} @original_users_map = original_users_map @rewrite_mentions = rewrite_mentions + @migrate_banned_contributions = migrate_banned_contributions # Remove excluded keys from relation_hash # We don't do this in the parsed_relation_hash because of the 'transformed attributes' @@ -281,6 +282,8 @@ def find_or_create_object! end def setup_note + return @relation_hash = {} if banned_contribution? + set_note_author # attachment is deprecated and note uploads are handled by Markdown uploader @relation_hash['attachment'] = nil @@ -389,6 +392,10 @@ def uses_importable_fk_as_primary_key? def importable_foreign_key relation_class.reflect_on_association(importable_class_name.to_sym)&.foreign_key end + + def banned_contribution? + @relation_hash.delete('hidden?') && !@migrate_banned_contributions + end end end end diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index 4c91c7bc3584dd840e8ba1fd355eb0288e66007f..c5c20951f0d6987499db869532489708339d7798 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -1129,6 +1129,7 @@ methods: - :squash_commit_template notes: - :type + - :hidden? commit_notes: - :type labels: @@ -1144,6 +1145,7 @@ methods: - :source_branch_sha - :target_branch_sha - :state + - :hidden? events: - :action push_event_payload: @@ -1154,6 +1156,7 @@ methods: - :list_type issues: - :state + - :hidden? note_diff_file: - :diff_export @@ -1163,6 +1166,7 @@ methods: preloads: issues: project: :route + author: builds: metadata: project: @@ -1172,6 +1176,9 @@ preloads: source_project: :route # needed by source_branch_sha and diff_head_sha target_project: :route # needed by target_branch_sha assignees: # needed by assigne_id that is implemented by DeprecatedAssignee + author: + notes: + author: # Specify a custom export reordering for a given relationship # For example for issues we use a custom export reordering by relative_position, so that on import, we can reset the diff --git a/lib/gitlab/import_export/project/relation_factory.rb b/lib/gitlab/import_export/project/relation_factory.rb index a5ed67fa5a4ddbc205bc3c5b6195d3c2b0dd17fe..334f6e7dea88d91d0f94bf4bd02423edf8502895 100644 --- a/lib/gitlab/import_export/project/relation_factory.rb +++ b/lib/gitlab/import_export/project/relation_factory.rb @@ -206,6 +206,8 @@ def setup_pipeline end def setup_work_item + return @relation_hash = {} if banned_contribution? + @relation_hash['relative_position'] = compute_relative_position issue_type = @relation_hash.delete('issue_type') @@ -227,6 +229,8 @@ def setup_pipeline_schedule end def setup_merge_request + return @relation_hash = {} if banned_contribution? + @relation_hash['merge_when_pipeline_succeeds'] = false end diff --git a/lib/import/bulk_imports/common/transformers/source_user_member_attributes_transformer.rb b/lib/import/bulk_imports/common/transformers/source_user_member_attributes_transformer.rb index 91c94cd00dec65190c0ef9ca2ee61547ae03122b..954a0db8b36e3ed16977621e917066729f90bed2 100644 --- a/lib/import/bulk_imports/common/transformers/source_user_member_attributes_transformer.rb +++ b/lib/import/bulk_imports/common/transformers/source_user_member_attributes_transformer.rb @@ -16,6 +16,9 @@ def transform(context, data) source_user = find_or_create_source_user(context, data) access_level = data.dig('access_level', 'integer_value') + user_state = data&.dig('user', 'state') + + return if skip_banned_user?(user_state, context) return unless valid_access_level?(access_level) if source_user.accepted_status? @@ -58,6 +61,13 @@ def find_or_create_source_user(context, data) cache: false ) end + + def skip_banned_user?(user_state, context) + return if context.entity.migrate_banned_contributions + return unless user_state == 'banned' + + true + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0f761461cb6a6d972fe99fdfe3bf921c7b1d9eb0..c3646cbf6d8c9043e8273b850fb911f943d97efc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11124,6 +11124,9 @@ msgstr "" msgid "BulkImport|Following data will not be migrated: %{bullets} Contact system administrator of %{host} to upgrade GitLab if you need this data in your migration" msgstr "" +msgid "BulkImport|Import banned contributions" +msgstr "" + msgid "BulkImport|Import completed" msgstr "" @@ -11244,6 +11247,9 @@ msgstr "" msgid "BulkImport|Select the groups and projects you want to import." msgstr "" +msgid "BulkImport|Select whether contributions from banned users are imported." +msgstr "" + msgid "BulkImport|Select whether user memberships in groups and projects are imported." msgstr "" @@ -31228,6 +31234,9 @@ msgstr "" msgid "Import|Show errors" msgstr "" +msgid "Import|Skipped" +msgstr "" + msgid "Import|The import %{project_creator_name} started on %{start_date} from %{strong_open}%{hostname}%{strong_close} has completed." msgstr "" diff --git a/spec/db/clickhouse_siphon_tables_spec.rb b/spec/db/clickhouse_siphon_tables_spec.rb index fd9314d038d8eaedab4292b1136a77e4e3035e5d..49c2077529cfebe1cfe376ef0de449fffc7ef523 100644 --- a/spec/db/clickhouse_siphon_tables_spec.rb +++ b/spec/db/clickhouse_siphon_tables_spec.rb @@ -4,8 +4,8 @@ RSpec.describe 'ClickHouse siphon tables', :click_house, feature_category: :database do let_it_be(:siphon_table_prefix) { 'siphon_' } - let_it_be(:skip_tables) { [] } # insert table name in the array to be skipped on specs - let_it_be(:skip_fields) { [] } # insert field name in the array to be skipped on specs + let_it_be(:skip_tables) { ['bulk_import_entities'] } # insert table name in the array to be skipped on specs + let_it_be(:skip_fields) { ['migrate_banned_contributions'] } # insert field name in the array to be skipped on specs let_it_be(:ch_database_name) { ClickHouse::Client.configuration.databases[:main].database } let_it_be(:pg_type_map) { Gitlab::ClickHouse::SiphonGenerator::PG_TYPE_MAP } diff --git a/spec/factories/bulk_import/entities.rb b/spec/factories/bulk_import/entities.rb index e88d65b8053aac60f09c839f69d428577cf3bab1..6f8cc34ad32a667125720611e5561cc12023626a 100644 --- a/spec/factories/bulk_import/entities.rb +++ b/spec/factories/bulk_import/entities.rb @@ -59,5 +59,9 @@ trait :failed do status { -1 } end + + trait :skipped do + status { -3 } + end end end diff --git a/spec/frontend/import_entities/import_groups/components/import_table_spec.js b/spec/frontend/import_entities/import_groups/components/import_table_spec.js index b499b450bff60aed11c8e892d699bbe7e0ff5beb..8c689b96f8007f6c06013851547dc7854899678c 100644 --- a/spec/frontend/import_entities/import_groups/components/import_table_spec.js +++ b/spec/frontend/import_entities/import_groups/components/import_table_spec.js @@ -54,8 +54,7 @@ describe('import table', () => { }, }; - const findImportSelectedDropdown = () => - wrapper.find('[data-testid="import-selected-groups-dropdown"]'); + const findImportSelectedDropdown = () => wrapper.findByTestId('import-selected-groups-dropdown'); const findRowImportDropdownAtIndex = (idx) => wrapper.findAll('tbody td button').wrappers.filter((w) => w.text() === 'Import with projects')[ idx @@ -359,6 +358,7 @@ describe('import table', () => { importRequests: [ { migrateMemberships: true, + migrateBannedContributions: false, migrateProjects: true, newName: FAKE_GROUP.lastImportTarget.newName, sourceGroupId: FAKE_GROUP.id, @@ -826,6 +826,7 @@ describe('import table', () => { sourceGroupId: NEW_GROUPS[0].id, migrateProjects: true, migrateMemberships: true, + migrateBannedContributions: false, }), expect.objectContaining({ targetNamespace: AVAILABLE_NAMESPACES[0].fullPath, @@ -833,6 +834,7 @@ describe('import table', () => { sourceGroupId: NEW_GROUPS[1].id, migrateProjects: true, migrateMemberships: true, + migrateBannedContributions: false, }), ], }, @@ -971,6 +973,7 @@ describe('import table', () => { sourceGroupId: NEW_GROUPS[0].id, migrateProjects: true, migrateMemberships: true, + migrateBannedContributions: false, }), expect.objectContaining({ targetNamespace: AVAILABLE_NAMESPACES[0].fullPath, @@ -978,6 +981,7 @@ describe('import table', () => { sourceGroupId: NEW_GROUPS[1].id, migrateProjects: true, migrateMemberships: true, + migrateBannedContributions: false, }), ], }, @@ -999,6 +1003,7 @@ describe('import table', () => { sourceGroupId: NEW_GROUPS[0].id, migrateProjects: false, migrateMemberships: true, + migrateBannedContributions: false, }), expect.objectContaining({ targetNamespace: AVAILABLE_NAMESPACES[0].fullPath, @@ -1006,6 +1011,7 @@ describe('import table', () => { sourceGroupId: NEW_GROUPS[1].id, migrateProjects: false, migrateMemberships: true, + migrateBannedContributions: false, }), ], }, @@ -1015,10 +1021,10 @@ describe('import table', () => { describe('migrateMemberships', () => { const findImportUserMembershipsCheckbox = () => - wrapper.find('[data-testid="toggle-import-user-memberships"]'); + wrapper.findByTestId('toggle-import-user-memberships'); it('checkbox is rendered as checked by default', async () => { - await createComponent({ + createComponent({ bulkImportSourceGroups: () => ({ nodes: FAKE_GROUPS, pageInfo: FAKE_PAGE_INFO, @@ -1031,7 +1037,7 @@ describe('import table', () => { }); it('is included as false in the importGroupsMutation when checkbox is unchecked', async () => { - await createComponent({ + createComponent({ bulkImportSourceGroups: () => ({ nodes: FAKE_GROUPS, pageInfo: FAKE_PAGE_INFO, @@ -1042,9 +1048,8 @@ describe('import table', () => { await waitForPromises(); await findImportUserMembershipsCheckbox().setChecked(false); - await nextTick(); - await findRowImportDropdownAtIndex(0).trigger('click'); + findRowImportDropdownAtIndex(0).trigger('click'); await waitForPromises(); expect(mutateSpy).toHaveBeenCalledWith({ @@ -1053,6 +1058,58 @@ describe('import table', () => { importRequests: [ { migrateMemberships: false, + migrateBannedContributions: false, + migrateProjects: true, + newName: FAKE_GROUP.lastImportTarget.newName, + sourceGroupId: FAKE_GROUP.id, + targetNamespace: AVAILABLE_NAMESPACES[0].fullPath, + }, + ], + }, + }); + }); + }); + + describe('migrateBannedContributions', () => { + const findImportBannedContributionsCheckbox = () => + wrapper.findByTestId('toggle-import-banned-contributions'); + + it('checkbox is rendered as unchecked by default', async () => { + createComponent({ + bulkImportSourceGroups: () => ({ + nodes: FAKE_GROUPS, + pageInfo: FAKE_PAGE_INFO, + versionValidation: FAKE_VERSION_VALIDATION, + }), + }); + + await waitForPromises(); + expect(findImportBannedContributionsCheckbox().element.checked).toBe(false); + }); + + it('is included as true in the importGroupsMutation when checkbox is checked', async () => { + createComponent({ + bulkImportSourceGroups: () => ({ + nodes: FAKE_GROUPS, + pageInfo: FAKE_PAGE_INFO, + versionValidation: FAKE_VERSION_VALIDATION, + }), + }); + const mutateSpy = jest.spyOn(apolloProvider.defaultClient, 'mutate'); + + await waitForPromises(); + await findImportBannedContributionsCheckbox().setChecked(true); + + findRowImportDropdownAtIndex(0).trigger('click'); + await waitForPromises(); + + expect(mutateSpy).toHaveBeenCalledWith({ + mutation: importGroupsMutation, + variables: { + importRequests: [ + { + migrateMemberships: true, + migrateBannedContributions: true, migrateProjects: true, newName: FAKE_GROUP.lastImportTarget.newName, sourceGroupId: FAKE_GROUP.id, diff --git a/spec/lib/api/entities/bulk_imports/entity_spec.rb b/spec/lib/api/entities/bulk_imports/entity_spec.rb index 759692f42bdab8cc5dd6e5e91bce4e2d458a9386..9fcad6bb4b485c6b2d36900fcca9e85fceeea140 100644 --- a/spec/lib/api/entities/bulk_imports/entity_spec.rb +++ b/spec/lib/api/entities/bulk_imports/entity_spec.rb @@ -25,7 +25,8 @@ :migrate_projects, :migrate_memberships, :has_failures, - :stats + :stats, + :migrate_banned_contributions ) end end diff --git a/spec/lib/bulk_imports/common/rest/get_project_query_spec.rb b/spec/lib/bulk_imports/common/rest/get_project_query_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..15adf9305aeac7bd5dfe10f3af224472acb80145 --- /dev/null +++ b/spec/lib/bulk_imports/common/rest/get_project_query_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::Common::Rest::GetProjectQuery, feature_category: :importers do + let(:entity) { create(:bulk_import_entity) } + let(:tracker) { create(:bulk_import_tracker, entity: entity) } + let(:context) { BulkImports::Pipeline::Context.new(tracker) } + let(:encoded_full_path) { ERB::Util.url_encode(entity.source_full_path) } + + describe '.to_h' do + it 'returns correct query' do + expected = { + resource: ['projects', encoded_full_path].join('/'), + query: {} + } + + expect(described_class.to_h(context)).to eq(expected) + end + end +end diff --git a/spec/lib/bulk_imports/common/rest/get_user_query_spec.rb b/spec/lib/bulk_imports/common/rest/get_user_query_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..7c87b90f511586a38d2f43f170adf51089d5d478 --- /dev/null +++ b/spec/lib/bulk_imports/common/rest/get_user_query_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::Common::Rest::GetUserQuery, feature_category: :importers do + let(:tracker) { create(:bulk_import_tracker) } + let(:context) { BulkImports::Pipeline::Context.new(tracker) } + + describe '.to_h' do + it 'returns correct query' do + context.extra[:user_id] = 1 + + expected = { resource: ['users', 1].join('/'), query: {} } + + expect(described_class.to_h(context)).to eq(expected) + end + end +end diff --git a/spec/lib/bulk_imports/common/transformers/member_attributes_transformer_spec.rb b/spec/lib/bulk_imports/common/transformers/member_attributes_transformer_spec.rb index 90bebe3d4f012ce63f2727cfb64a4f7a9e7eb750..c06cd4c5343a2d1c62d3674188dfd033ab8b57da 100644 --- a/spec/lib/bulk_imports/common/transformers/member_attributes_transformer_spec.rb +++ b/spec/lib/bulk_imports/common/transformers/member_attributes_transformer_spec.rb @@ -150,6 +150,16 @@ expect(subject.transform(context, { 'id' => 1 })).to eq({ 'id' => 1 }) end end + + context 'when user state is banned' do + it 'returns nil' do + gid = 'gid://gitlab/User/7' + data = member_data(email: user.email, gid: gid) + data["user"]["state"] = 'banned' + + expect(subject.transform(context, data)).to be_nil + end + end end context 'with a project' do diff --git a/spec/lib/bulk_imports/groups/pipelines/project_entities_pipeline_spec.rb b/spec/lib/bulk_imports/groups/pipelines/project_entities_pipeline_spec.rb index e9a8f47f54f78678d4cc994301cbacf8f4a4c82b..542a98449925b1019cf6889b4c96ef88bbb8de91 100644 --- a/spec/lib/bulk_imports/groups/pipelines/project_entities_pipeline_spec.rb +++ b/spec/lib/bulk_imports/groups/pipelines/project_entities_pipeline_spec.rb @@ -10,7 +10,8 @@ create( :bulk_import_entity, group: destination_group, - destination_namespace: destination_group.full_path + destination_namespace: destination_group.full_path, + migrate_banned_contributions: true ) end @@ -51,6 +52,7 @@ expect(project_entity.destination_namespace).to eq(destination_group.full_path) expect(project_entity.organization).to eq(destination_group.organization) expect(project_entity.source_xid).to eq(1234567) + expect(project_entity.migrate_banned_contributions).to be(true) end it 'does not create duplicate entities on rerun' do diff --git a/spec/lib/bulk_imports/groups/transformers/subgroup_to_entity_transformer_spec.rb b/spec/lib/bulk_imports/groups/transformers/subgroup_to_entity_transformer_spec.rb index b0df06f0ded76807de6594ec48d671441f2476e8..40d30746e7226456ddfd845b92af561b60de9d24 100644 --- a/spec/lib/bulk_imports/groups/transformers/subgroup_to_entity_transformer_spec.rb +++ b/spec/lib/bulk_imports/groups/transformers/subgroup_to_entity_transformer_spec.rb @@ -7,7 +7,7 @@ it "transforms subgroups data in entity params" do parent = create(:group) parent_entity = instance_double(BulkImports::Entity, group: parent, id: 1, - migrate_projects: false, migrate_memberships: false) + migrate_projects: false, migrate_memberships: false, migrate_banned_contributions: true) context = instance_double(BulkImports::Pipeline::Context, entity: parent_entity) subgroup_data = { "path" => "sub-group", @@ -22,7 +22,8 @@ organization_id: parent.organization_id, parent_id: 1, migrate_projects: false, - migrate_memberships: false + migrate_memberships: false, + migrate_banned_contributions: true ) end end diff --git a/spec/lib/bulk_imports/ndjson_pipeline_spec.rb b/spec/lib/bulk_imports/ndjson_pipeline_spec.rb index 721c15a4056e61427d41c2313e37c1b504b45bdf..84e5925e4dad3c6d74fe974eba6cbce6ef9ba644 100644 --- a/spec/lib/bulk_imports/ndjson_pipeline_spec.rb +++ b/spec/lib/bulk_imports/ndjson_pipeline_spec.rb @@ -266,7 +266,8 @@ def initialize(portable, user, context) excluded_keys: nil, import_source: Import::SOURCE_DIRECT_TRANSFER, original_users_map: {}, - rewrite_mentions: false + rewrite_mentions: false, + migrate_banned_contributions: false ) .and_return(relation_object) expect(relation_object).to receive(:assign_attributes).with(group: group) @@ -293,7 +294,8 @@ def initialize(portable, user, context) excluded_keys: nil, import_source: Import::SOURCE_DIRECT_TRANSFER, original_users_map: {}, - rewrite_mentions: true + rewrite_mentions: true, + migrate_banned_contributions: false ).and_return(double(assign_attributes: nil)) subject.transform(context, data) diff --git a/spec/lib/bulk_imports/pipeline/runner_spec.rb b/spec/lib/bulk_imports/pipeline/runner_spec.rb index d91aa1e2d134205d84b72fb044f51f0a520ab1f4..307abb8d6b9d2d84a34f8fc8ead69700a2ed81cd 100644 --- a/spec/lib/bulk_imports/pipeline/runner_spec.rb +++ b/spec/lib/bulk_imports/pipeline/runner_spec.rb @@ -410,6 +410,26 @@ def load(context, data); end end end + context 'when entity is marked as skipped' do + it 'logs and returns without execution' do + entity.skip! + + expect_next_instance_of(BulkImports::Logger) do |logger| + expect(logger).to receive(:with_entity).with(context.entity).and_call_original + expect(logger).to receive(:warn) + .with( + log_params( + context, + message: 'Skipping pipeline due to skipped entity', + pipeline_class: 'BulkImports::MyPipeline' + ) + ) + end + + subject.run + end + end + describe 'object counting' do it 'increments object counters' do allow_next_instance_of(BulkImports::Extractor) do |extractor| diff --git a/spec/lib/bulk_imports/projects/pipelines/project_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/project_pipeline_spec.rb index 25ea193335ecf8cca92a45d58f519a63fc60f9c1..82747f4b85e788310cd73b31d3471acff835779f 100644 --- a/spec/lib/bulk_imports/projects/pipelines/project_pipeline_spec.rb +++ b/spec/lib/bulk_imports/projects/pipelines/project_pipeline_spec.rb @@ -3,39 +3,41 @@ require 'spec_helper' RSpec.describe BulkImports::Projects::Pipelines::ProjectPipeline, feature_category: :importers do - describe '#run', :clean_gitlab_redis_shared_state do - let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group) } - let_it_be(:bulk_import) { create(:bulk_import, user: user) } - - let(:entity) do - create( - :bulk_import_entity, - source_type: :project_entity, - bulk_import: bulk_import, - source_full_path: 'source/full/path', - destination_slug: 'My-Destination-Project', - destination_namespace: group.full_path - ) - end + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:bulk_import) { create(:bulk_import, user: user) } + + let(:entity) do + create( + :bulk_import_entity, + source_type: :project_entity, + bulk_import: bulk_import, + source_full_path: 'source/full/path', + destination_slug: 'My-Destination-Project', + destination_namespace: group.full_path + ) + end - let(:tracker) { create(:bulk_import_tracker, entity: entity) } - let(:context) { BulkImports::Pipeline::Context.new(tracker) } + let(:tracker) { create(:bulk_import_tracker, entity: entity) } + let(:context) { BulkImports::Pipeline::Context.new(tracker) } - let(:project_data) do - { - 'visibility' => 'private', - 'created_at' => '2016-08-12T09:41:03' - } - end + let(:project_data) do + { + 'visibility' => 'private', + 'created_at' => '2016-08-12T09:41:03' + } + end - subject(:project_pipeline) { described_class.new(context) } + subject(:project_pipeline) { described_class.new(context) } - before do - allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor| - allow(extractor).to receive(:extract).and_return(BulkImports::Pipeline::ExtractedData.new(data: project_data)) - end + before do + allow_next_instance_of(BulkImports::Common::Extractors::RestExtractor) do |extractor| + allow(extractor).to receive(:extract).and_return(BulkImports::Pipeline::ExtractedData.new(data: project_data)) + end + end + describe '#run', :clean_gitlab_redis_shared_state do + before do allow(project_pipeline).to receive(:set_source_objects_counter) group.add_owner(user) @@ -63,18 +65,58 @@ end end + describe '#extract' do + context 'when migrate banned contributions is true' do + it 'returns project data' do + entity.update!(migrate_banned_contributions: true) + + expected_data = project_pipeline.extract(context) + + expect(expected_data.data.first).to eq(project_data) + end + end + + context 'when migrate banned contributions is false' do + let(:user_extractor) { instance_double(BulkImports::Common::Extractors::RestExtractor) } + + before do + allow(BulkImports::Common::Extractors::RestExtractor) + .to receive(:new) + .with(query: BulkImports::Common::Rest::GetUserQuery) + .and_return(user_extractor) + + allow(user_extractor) + .to receive(:extract) + .and_return(BulkImports::Pipeline::ExtractedData.new(data: user_data)) + end + + context 'when creator is not banned' do + let(:user_data) { { 'state' => 'active' } } + + it 'returns project data' do + expected_data = project_pipeline.extract(context) + + expect(expected_data.data.first).to eq(project_data) + end + end + + context 'when creator is banned' do + let(:user_data) { { 'state' => 'banned' } } + + it 'skips entity and returns nil' do + expected_data = project_pipeline.extract(context) + + expect(expected_data.data.first).to be_nil + expect(context.entity.skipped?).to be(true) + end + end + end + end + describe 'pipeline parts' do it { expect(described_class).to include_module(BulkImports::Pipeline) } it { expect(described_class).to include_module(BulkImports::Pipeline::Runner) } - it 'has extractors' do - expect(described_class.get_extractor) - .to eq( - klass: BulkImports::Common::Extractors::GraphqlExtractor, - options: { query: BulkImports::Projects::Graphql::GetProjectQuery } - ) - end - it 'has transformers' do expect(described_class.transformers) .to contain_exactly( diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb index 7001418dafcbd150d92a3ec963740d4947873522..e6ce9a55609a585afbdefe857df098702174874d 100644 --- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb @@ -42,6 +42,60 @@ def values end end + shared_examples 'banned contributions' do |expected_model| + context 'when migrate banned contributions is false' do + context 'when relation is hidden' do + it 'returns empty relation' do + relation_hash['hidden?'] = true + + expect(created_object).to be_nil + end + end + + context 'when relation is not hidden' do + it 'creates imported object' do + relation_hash['hidden?'] = false + + expect(created_object).to be_instance_of(expected_model) + end + end + end + + context 'when migrate banned contributions is true' do + let(:created_object) do + described_class.create( # rubocop:disable Rails/SaveBang -- not an active record model + relation_sym: relation_sym, + relation_hash: relation_hash.merge(additional_relation_attributes), + relation_index: 1, + object_builder: Gitlab::ImportExport::Project::ObjectBuilder, + members_mapper: members_mapper, + user: importer_user, + importable: project, + import_source: ::Import::SOURCE_PROJECT_EXPORT_IMPORT, + excluded_keys: excluded_keys, + rewrite_mentions: true, + migrate_banned_contributions: true + ) + end + + context 'when relation is hidden' do + it 'creates imported object' do + relation_hash['hidden?'] = true + + expect(created_object).to be_instance_of(expected_model) + end + end + + context 'when relation is not hidden' do + it 'creates imported object' do + relation_hash['hidden?'] = false + + expect(created_object).to be_instance_of(expected_model) + end + end + end + end + context 'hook object' do let(:relation_sym) { :hooks } let(:id) { 999 } @@ -180,6 +234,8 @@ def values it 'inserts backticks around username mentions' do expect(created_object.description).to eq("I said to `@sam` the code should follow `@bob`'s advice. `@alice`?") end + + include_examples 'banned contributions', MergeRequest end context 'issue object' do @@ -289,6 +345,8 @@ def values it 'inserts backticks around username mentions' do expect(created_object.description).to eq("I said to `@sam` the code should follow `@bob`'s advice. `@alice`?") end + + include_examples 'banned contributions', Issue end context 'label object' do @@ -673,6 +731,8 @@ def values expect(created_object.change_position.line_range).to eq(expected_line_range) end end + + include_examples 'banned contributions', DiffNote end end diff --git a/spec/lib/import/bulk_imports/common/transformers/source_user_member_attributes_transformer_spec.rb b/spec/lib/import/bulk_imports/common/transformers/source_user_member_attributes_transformer_spec.rb index 822112ea42817a3af9bc1c71743349924ca80d17..5725358ac023ef6c9ea9d2d206b3b60f8daa67ee 100644 --- a/spec/lib/import/bulk_imports/common/transformers/source_user_member_attributes_transformer_spec.rb +++ b/spec/lib/import/bulk_imports/common/transformers/source_user_member_attributes_transformer_spec.rb @@ -63,6 +63,15 @@ end end + context 'when user state is banned' do + it 'returns nil' do + data = member_data(source_user_id: 1) + data["user"]["state"] = 'banned' + + expect(subject.transform(context, data)).to be_nil + end + end + context 'when importer_user_mapping is disabled' do let(:importer_user_mapping_enabled) { false } diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index c9c9c7b1c8bcac9bfecf1d34449a41c9cb3441d9..b2198456747d3ec2f1000ef5c4e4a5e503cbdfba 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -345,7 +345,7 @@ describe '.all_human_statuses' do it 'returns all human readable entity statuses' do expect(described_class.all_human_statuses) - .to contain_exactly('created', 'started', 'finished', 'failed', 'timeout', 'canceled') + .to contain_exactly('created', 'started', 'finished', 'failed', 'timeout', 'canceled', 'skipped') end end @@ -730,4 +730,14 @@ end end end + + describe 'entity skipping' do + let(:entity) { create(:bulk_import_entity, :started) } + + it 'marks entity as skipped' do + entity.skip! + + expect(entity.skipped?).to eq(true) + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 7abb7664f4273ab60260a8d73cf1445b77d71148..a998514aad29c7d10e5a2c9da76e208699fd6310 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1904,6 +1904,35 @@ def update_note(note, **attributes) end end + describe '#hidden?' do + let(:author) { build(:user) } + let(:note) { build(:note, author: author) } + + context 'when author is banned' do + it 'returns true' do + author.ban! + + expect(note.hidden?).to be(true) + end + end + + context 'when author is not banned' do + it 'returns false' do + expect(note.hidden?).to be(false) + end + end + + context 'when the :hidden_notes feature is disabled' do + it 'returns false' do + stub_feature_flags(hidden_notes: false) + + author.ban! + + expect(note.hidden?).to be(false) + end + end + end + describe '.authored_by' do subject(:notes_by_author) { described_class.authored_by(author) } @@ -2098,4 +2127,13 @@ def update_note(note, **attributes) end end end + + describe '#uploads_sharding_key' do + it 'returns namespace_id' do + namespace = build_stubbed(:namespace) + note = build_stubbed(:note, namespace: namespace) + + expect(note.uploads_sharding_key).to eq(namespace_id: namespace.id) + end + end end diff --git a/spec/requests/api/bulk_imports_spec.rb b/spec/requests/api/bulk_imports_spec.rb index 32d1b17573bb25fcc2b10d674f54be91fad55a55..c01ad648d2f1f4deb8c96265ea273530738b5070 100644 --- a/spec/requests/api/bulk_imports_spec.rb +++ b/spec/requests/api/bulk_imports_spec.rb @@ -240,6 +240,36 @@ end end + describe 'migrate banned contributions flag' do + context 'when true' do + it 'sets true' do + params[:entities][0][:migrate_banned_contributions] = true + + request + + expect(user.bulk_imports.last.entities.pluck(:migrate_banned_contributions)).to contain_exactly(true) + end + end + + context 'when false' do + it 'sets false' do + params[:entities][0][:migrate_banned_contributions] = false + + request + + expect(user.bulk_imports.last.entities.pluck(:migrate_banned_contributions)).to contain_exactly(false) + end + end + + context 'when unspecified' do + it 'sets false' do + request + + expect(user.bulk_imports.last.entities.pluck(:migrate_banned_contributions)).to contain_exactly(false) + end + end + end + context 'when entities do not specify a namespace', :with_current_organization do let(:params) do { diff --git a/spec/services/bulk_imports/create_service_spec.rb b/spec/services/bulk_imports/create_service_spec.rb index 5ca8f0d252fa9cfacda960449108a610a0d6cf12..1f369eca22ae008152aa50aaeb8f5425d7da714a 100644 --- a/spec/services/bulk_imports/create_service_spec.rb +++ b/spec/services/bulk_imports/create_service_spec.rb @@ -11,6 +11,7 @@ let(:destination_group) { create(:group, path: 'destination1') } let(:migrate_projects) { true } let(:migrate_memberships) { true } + let(:migrate_banned_contributions) { false } let_it_be(:parent_group) { create(:group, path: 'parent-group') } # note: destination_name and destination_slug are currently interchangable so we need to test for both possibilities let(:params) do @@ -21,7 +22,8 @@ destination_slug: 'destination-group-1', destination_namespace: 'parent-group', migrate_projects: migrate_projects, - migrate_memberships: migrate_memberships + migrate_memberships: migrate_memberships, + migrate_banned_contributions: migrate_banned_contributions }, { source_type: 'group_entity', @@ -29,7 +31,8 @@ destination_name: 'destination-group-2', destination_namespace: 'parent-group', migrate_projects: migrate_projects, - migrate_memberships: migrate_memberships + migrate_memberships: migrate_memberships, + migrate_banned_contributions: migrate_banned_contributions }, { source_type: 'project_entity', @@ -37,7 +40,8 @@ destination_slug: 'destination-project-1', destination_namespace: 'parent-group', migrate_projects: migrate_projects, - migrate_memberships: migrate_memberships + migrate_memberships: migrate_memberships, + migrate_banned_contributions: migrate_banned_contributions } ] end @@ -432,6 +436,40 @@ end end end + + describe 'banned contributions flag' do + let(:import) { BulkImport.last } + + context 'when false' do + let(:migrate_banned_contributions) { false } + + it 'sets false' do + subject.execute + + expect(import.entities.pluck(:migrate_banned_contributions)).to contain_exactly(false, false, false) + end + end + + context 'when true' do + let(:migrate_banned_contributions) { true } + + it 'sets true' do + subject.execute + + expect(import.entities.pluck(:migrate_banned_contributions)).to contain_exactly(true, true, true) + end + end + + context 'when nil' do + let(:migrate_banned_contributions) { nil } + + it 'sets false' do + subject.execute + + expect(import.entities.pluck(:migrate_banned_contributions)).to contain_exactly(false, false, false) + end + end + end end end diff --git a/spec/services/bulk_imports/process_service_spec.rb b/spec/services/bulk_imports/process_service_spec.rb index 6e62874f115ae70f02f8fa3c6b2f842899cdeb27..8ffa5c9fbb183dc3de1ed24269ce48b0e393c17d 100644 --- a/spec/services/bulk_imports/process_service_spec.rb +++ b/spec/services/bulk_imports/process_service_spec.rb @@ -49,6 +49,7 @@ bulk_import.update!(status: 1) create(:bulk_import_entity, :finished, bulk_import: bulk_import) create(:bulk_import_entity, :failed, bulk_import: bulk_import) + create(:bulk_import_entity, :skipped, bulk_import: bulk_import) end it 'marks bulk import as finished' do diff --git a/spec/workers/bulk_imports/pipeline_worker_spec.rb b/spec/workers/bulk_imports/pipeline_worker_spec.rb index 6fdfd03a03eaa75427d7032f4fa3e75a1f85bab4..c58b4d083e7ae5890e54af2183700294a96228f5 100644 --- a/spec/workers/bulk_imports/pipeline_worker_spec.rb +++ b/spec/workers/bulk_imports/pipeline_worker_spec.rb @@ -324,6 +324,33 @@ def self.abort_on_failure? end end + context 'when entity is skipped' do + before do + entity.update!(status: -3) + end + + it 'marks tracker as skipped and logs the skip' do + pipeline_tracker = create( + :bulk_import_tracker, + entity: entity, + pipeline_name: 'FakePipeline', + status_event: 'enqueue' + ) + + expect_next_instance_of(BulkImports::Logger) do |logger| + allow(logger).to receive(:info) + + expect(logger) + .to receive(:info) + .with(hash_including(message: 'Skipping pipeline due to skipped entity')) + end + + worker.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + + expect(pipeline_tracker.reload.status_name).to eq(:skipped) + end + end + context 'when entity is canceled' do it 'marks tracker as canceled and logs the cancel' do entity.update!(status: -2)