diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb
index 7771af18cf57411e86e7013327ba88fd8ded0844..7245b697beb9a6a8485c74f1cb277e6d693a9ed8 100644
--- a/app/services/import/github_service.rb
+++ b/app/services/import/github_service.rb
@@ -142,7 +142,8 @@ def store_import_settings(project)
.write(
timeout_strategy: params[:timeout_strategy] || ProjectImportData::PESSIMISTIC_TIMEOUT,
optional_stages: params[:optional_stages],
- extended_events: Feature.enabled?(:github_import_extended_events, current_user)
+ extended_events: Feature.enabled?(:github_import_extended_events, current_user),
+ prioritize_collaborators: Feature.enabled?(:github_import_prioritize_collaborators, current_user)
)
end
end
diff --git a/app/workers/gitlab/github_import/advance_stage_worker.rb b/app/workers/gitlab/github_import/advance_stage_worker.rb
index 316035fd683af4f1801d3ccdf0967cf46b215d61..82b3303e28cf81eee18f55868e9b8cda6b45b9d4 100644
--- a/app/workers/gitlab/github_import/advance_stage_worker.rb
+++ b/app/workers/gitlab/github_import/advance_stage_worker.rb
@@ -17,8 +17,9 @@ class AdvanceStageWorker # rubocop:disable Scalability/IdempotentWorker
# The known importer stages and their corresponding Sidekiq workers.
#
- # Note: AdvanceStageWorker is not used for the repository, base_data, and pull_requests stages.
+ # Note: AdvanceStageWorker is not used to initiate the repository, base_data, and pull_requests stages.
# They are included in the list for us to easily see all stage workers and the order in which they are executed.
+
STAGES = {
repository: Stage::ImportRepositoryWorker,
base_data: Stage::ImportBaseDataWorker,
diff --git a/app/workers/gitlab/github_import/stage/import_base_data_worker.rb b/app/workers/gitlab/github_import/stage/import_base_data_worker.rb
index d965c1ae8470647738193df75fd0b77c116c2405..82e5f396238b7ea6da1ad5ff66f1c78d0ff1eeea 100644
--- a/app/workers/gitlab/github_import/stage/import_base_data_worker.rb
+++ b/app/workers/gitlab/github_import/stage/import_base_data_worker.rb
@@ -26,7 +26,11 @@ def import(client, project)
klass.new(project, client).execute
end
- ImportPullRequestsWorker.perform_async(project.id)
+ if import_settings(project).prioritize_collaborators?
+ ImportCollaboratorsWorker.perform_async(project.id)
+ else
+ ImportPullRequestsWorker.perform_async(project.id)
+ end
end
end
end
diff --git a/app/workers/gitlab/github_import/stage/import_collaborators_worker.rb b/app/workers/gitlab/github_import/stage/import_collaborators_worker.rb
index 38e1fd52889c7b372e0f5ea0bc43550806cba5b2..fd8b494f9372ece01e0877a2dda4111ca989aa9d 100644
--- a/app/workers/gitlab/github_import/stage/import_collaborators_worker.rb
+++ b/app/workers/gitlab/github_import/stage/import_collaborators_worker.rb
@@ -47,9 +47,14 @@ def move_to_next_stage(project, waiters = {})
end
def next_stage(project)
- return 'issues_and_diff_notes' if import_settings(project).extended_events?
+ if import_settings(project).prioritize_collaborators?
+ 'pull_requests'
+ else
- 'pull_requests_merged_by'
+ return 'issues_and_diff_notes' if import_settings(project).extended_events?
+
+ 'pull_requests_merged_by'
+ end
end
end
end
diff --git a/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb b/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb
index bcc39b169afeb80d35e82879e909fc0066b50e92..0ff7af6b83ed7f46a16b3f14a7b9641d43a762b2 100644
--- a/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb
+++ b/app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb
@@ -26,11 +26,7 @@ def import(client, project)
.new(project, client)
.execute
- AdvanceStageWorker.perform_async(
- project.id,
- { waiter.key => waiter.jobs_remaining },
- 'collaborators'
- )
+ move_to_next_stage(project, { waiter.key => waiter.jobs_remaining })
end
private
@@ -45,6 +41,22 @@ def allocate_merge_requests_internal_id!(project, client)
MergeRequest.track_target_project_iid!(project, last_github_pull_request[:number])
end
+
+ def move_to_next_stage(project, waiters = {})
+ AdvanceStageWorker.perform_async(
+ project.id, waiters.deep_stringify_keys, next_stage(project)
+ )
+ end
+
+ def next_stage(project)
+ if import_settings(project).prioritize_collaborators?
+ return 'issues_and_diff_notes' if import_settings(project).extended_events?
+
+ 'pull_requests_merged_by'
+ else
+ 'collaborators'
+ end
+ end
end
end
end
diff --git a/config/feature_flags/development/github_import_prioritize_collaborators.yml b/config/feature_flags/development/github_import_prioritize_collaborators.yml
new file mode 100644
index 0000000000000000000000000000000000000000..f323a83a40779c50640cf7794b05e0bf32e7cc32
--- /dev/null
+++ b/config/feature_flags/development/github_import_prioritize_collaborators.yml
@@ -0,0 +1,8 @@
+---
+name: github_import_prioritize_collaborators
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139410
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/439742
+milestone: '16.9'
+type: development
+group: group::import and integrate
+default_enabled: false
diff --git a/lib/gitlab/github_import/settings.rb b/lib/gitlab/github_import/settings.rb
index da5833df3a1d13463a544ca08cedac22a2bc46a8..f99cd6c6931c321348da3d85250db6e273601caf 100644
--- a/lib/gitlab/github_import/settings.rb
+++ b/lib/gitlab/github_import/settings.rb
@@ -67,7 +67,8 @@ def write(user_settings)
data: {
optional_stages: optional_stages,
timeout_strategy: user_settings[:timeout_strategy],
- extended_events: user_settings[:extended_events]
+ extended_events: user_settings[:extended_events],
+ prioritize_collaborators: user_settings[:prioritize_collaborators]
},
credentials: project.import_data&.credentials
)
@@ -87,6 +88,10 @@ def extended_events?
!!project.import_data&.data&.dig('extended_events')
end
+ def prioritize_collaborators?
+ !!project.import_data&.data&.dig('prioritize_collaborators')
+ end
+
private
attr_reader :project
diff --git a/spec/lib/gitlab/github_import/settings_spec.rb b/spec/lib/gitlab/github_import/settings_spec.rb
index d268f3a865095050ead77b409cfac4903deb55b0..022fde057cebda9cc5ea7ff2b929232e9dc027cd 100644
--- a/spec/lib/gitlab/github_import/settings_spec.rb
+++ b/spec/lib/gitlab/github_import/settings_spec.rb
@@ -137,4 +137,24 @@
expect(settings.extended_events?).to eq(false)
end
end
+
+ describe '#prioritize_collaborators?' do
+ it 'when prioritize_collaborators is set to true' do
+ project.build_or_assign_import_data(data: { prioritize_collaborators: true })
+
+ expect(settings.prioritize_collaborators?).to eq(true)
+ end
+
+ it 'when prioritize_collaborators is set to false' do
+ project.build_or_assign_import_data(data: { prioritize_collaborators: false })
+
+ expect(settings.prioritize_collaborators?).to eq(false)
+ end
+
+ it 'when prioritize_collaborators is not present' do
+ project.build_or_assign_import_data(data: {})
+
+ expect(settings.prioritize_collaborators?).to eq(false)
+ end
+ end
end
diff --git a/spec/services/import/github_service_spec.rb b/spec/services/import/github_service_spec.rb
index 06ce00260edcddbb97396e1b6ee6772d259a9c96..f4045318c1630bdad32adacdcb10807206992f8e 100644
--- a/spec/services/import/github_service_spec.rb
+++ b/spec/services/import/github_service_spec.rb
@@ -33,7 +33,8 @@
.with(
extended_events: true,
optional_stages: optional_stages,
- timeout_strategy: timeout_strategy
+ timeout_strategy: timeout_strategy,
+ prioritize_collaborators: true
)
end
@@ -94,7 +95,8 @@
.to have_received(:write)
.with(optional_stages: nil,
extended_events: true,
- timeout_strategy: timeout_strategy
+ timeout_strategy: timeout_strategy,
+ prioritize_collaborators: true
)
expect_snowplow_event(
category: 'Import::GithubService',
@@ -120,7 +122,8 @@
.with(
optional_stages: nil,
extended_events: true,
- timeout_strategy: timeout_strategy
+ timeout_strategy: timeout_strategy,
+ prioritize_collaborators: true
)
expect_snowplow_event(
category: 'Import::GithubService',
@@ -153,7 +156,8 @@
.with(
optional_stages: nil,
extended_events: true,
- timeout_strategy: timeout_strategy
+ timeout_strategy: timeout_strategy,
+ prioritize_collaborators: true
)
expect_snowplow_event(
category: 'Import::GithubService',
@@ -190,7 +194,8 @@
.with(
optional_stages: optional_stages,
extended_events: true,
- timeout_strategy: timeout_strategy
+ timeout_strategy: timeout_strategy,
+ prioritize_collaborators: true
)
end
end
@@ -206,7 +211,8 @@
.with(
optional_stages: optional_stages,
extended_events: true,
- timeout_strategy: timeout_strategy
+ timeout_strategy: timeout_strategy,
+ prioritize_collaborators: true
)
end
end
@@ -220,12 +226,13 @@
.with(
optional_stages: optional_stages,
extended_events: true,
- timeout_strategy: timeout_strategy
+ timeout_strategy: timeout_strategy,
+ prioritize_collaborators: true
)
end
end
- context 'when `github_import_extended_events`` feature flag is disabled' do
+ context 'when `github_import_extended_events` feature flag is disabled' do
before do
stub_feature_flags(github_import_extended_events: false)
end
@@ -238,6 +245,20 @@
subject.execute(access_params, :github)
end
end
+
+ context 'when `prioritize_collaborators` feature flag is disabled' do
+ before do
+ stub_feature_flags(github_import_prioritize_collaborators: false)
+ end
+
+ it 'saves prioritize_collaborators to import_data' do
+ expect(settings)
+ .to receive(:write)
+ .with(a_hash_including(prioritize_collaborators: false))
+
+ subject.execute(access_params, :github)
+ end
+ end
end
context 'when import source is disabled' do
diff --git a/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb
index 49dc905f43007919a7abe14bf97907191bd14786..081aa507763a92534379da18562036296c32b88e 100644
--- a/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb
+++ b/spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb
@@ -14,6 +14,10 @@
describe '#import' do
it 'imports the base data of a project' do
+ allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting|
+ allow(setting).to receive(:prioritize_collaborators?).and_return(true)
+ end
+
described_class::IMPORTERS.each do |klass|
expect(klass)
.to receive(:new)
@@ -23,11 +27,34 @@
expect(importer).to receive(:execute)
end
- expect(Gitlab::GithubImport::Stage::ImportPullRequestsWorker)
+ expect(Gitlab::GithubImport::Stage::ImportCollaboratorsWorker)
.to receive(:perform_async)
.with(project.id)
worker.import(client, project)
end
+
+ context 'when the prioritize_collaborators feature flag is disabled' do
+ it 'imports the base data of a project' do
+ allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting|
+ allow(setting).to receive(:prioritize_collaborators?).and_return(false)
+ end
+
+ described_class::IMPORTERS.each do |klass|
+ expect(klass)
+ .to receive(:new)
+ .with(project, client)
+ .and_return(importer)
+
+ expect(importer).to receive(:execute)
+ end
+
+ expect(Gitlab::GithubImport::Stage::ImportPullRequestsWorker)
+ .to receive(:perform_async)
+ .with(project.id)
+
+ worker.import(client, project)
+ end
+ end
end
end
diff --git a/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb
index 7a085227b367d8072231ac65a38748a737f131ff..152b70a9567e44c3185d5a1ee4c7951984422f58 100644
--- a/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb
+++ b/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb
@@ -21,6 +21,9 @@
settings.write({ optional_stages: { collaborators_import: stage_enabled } })
allow(client).to receive(:repository).with(project.import_source)
.and_return({ permissions: { push: push_rights_granted } })
+ allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting|
+ allow(setting).to receive(:prioritize_collaborators?).and_return(true)
+ end
end
context 'when user has push access for this repo' do
@@ -35,7 +38,7 @@
expect(Gitlab::GithubImport::AdvanceStageWorker)
.to receive(:perform_async)
- .with(project.id, { '123' => 2 }, 'pull_requests_merged_by')
+ .with(project.id, { '123' => 2 }, 'pull_requests')
worker.import(client, project)
end
@@ -49,7 +52,7 @@
expect(Gitlab::GithubImport::AdvanceStageWorker)
.to receive(:perform_async)
- .with(project.id, {}, 'pull_requests_merged_by')
+ .with(project.id, {}, 'pull_requests')
worker.import(client, project)
end
@@ -63,7 +66,29 @@
expect(Gitlab::GithubImport::AdvanceStageWorker)
.to receive(:perform_async)
- .with(project.id, {}, 'pull_requests_merged_by')
+ .with(project.id, {}, 'pull_requests')
+
+ worker.import(client, project)
+ end
+ end
+
+ context 'when prioritize_collaborators import setting is disabled' do
+ it 'imports all collaborators and advances to the correct next stage' do
+ allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting|
+ allow(setting).to receive(:prioritize_collaborators?).and_return(false)
+ end
+
+ waiter = Gitlab::JobWaiter.new(2, '123')
+
+ expect(Gitlab::GithubImport::Importer::CollaboratorsImporter)
+ .to receive(:new)
+ .with(project, client)
+ .and_return(importer)
+ expect(importer).to receive(:execute).and_return(waiter)
+
+ expect(Gitlab::GithubImport::AdvanceStageWorker)
+ .to receive(:perform_async)
+ .with(project.id, { '123' => 2 }, 'pull_requests_merged_by')
worker.import(client, project)
end
diff --git a/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb
index 2c1beb29fa19c7d01cb245929c7baebdcb365240..d9f174fea822b40cf0a0cfd3bdba6581bab0f325 100644
--- a/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb
+++ b/spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb
@@ -14,6 +14,10 @@
it_behaves_like Gitlab::GithubImport::StageMethods
describe '#import' do
+ before do
+ project.build_or_assign_import_data(data: { prioritize_collaborators: true })
+ end
+
context 'with pull requests' do
it 'imports all the pull requests and allocates internal iids' do
waiter = Gitlab::JobWaiter.new(2, '123')
@@ -35,7 +39,7 @@
expect(Gitlab::GithubImport::AdvanceStageWorker)
.to receive(:perform_async)
- .with(project.id, { '123' => 2 }, 'collaborators')
+ .with(project.id, { '123' => 2 }, 'pull_requests_merged_by')
expect(MergeRequest).to receive(:track_target_project_iid!)
@@ -64,7 +68,7 @@
expect(Gitlab::GithubImport::AdvanceStageWorker)
.to receive(:perform_async)
- .with(project.id, { '123' => 2 }, 'collaborators')
+ .with(project.id, { '123' => 2 }, 'pull_requests_merged_by')
expect(MergeRequest).not_to receive(:track_target_project_iid!)
@@ -93,5 +97,38 @@
worker.import(client, project)
end
end
+
+ context 'when prioritize_collaborators import setting is disabled' do
+ before do
+ project.build_or_assign_import_data(data: { prioritize_collaborators: false })
+ end
+
+ it 'advances to the correct next stage' do
+ waiter = Gitlab::JobWaiter.new(2, '123')
+
+ expect(Gitlab::GithubImport::Importer::PullRequestsImporter)
+ .to receive(:new)
+ .with(project, client)
+ .and_return(importer)
+
+ expect(importer)
+ .to receive(:execute)
+ .and_return(waiter)
+
+ expect(InternalId).to receive(:exists?).and_return(false)
+
+ expect(client).to receive(:each_object).with(
+ :pulls, project.import_source, options
+ ).and_return([{ number: 4 }].each)
+
+ expect(Gitlab::GithubImport::AdvanceStageWorker)
+ .to receive(:perform_async)
+ .with(project.id, { '123' => 2 }, 'collaborators')
+
+ expect(MergeRequest).to receive(:track_target_project_iid!)
+
+ worker.import(client, project)
+ end
+ end
end
end