diff --git a/app/workers/gitlab/github_import/advance_stage_worker.rb b/app/workers/gitlab/github_import/advance_stage_worker.rb
index ca47de2bebb5f03ab42a0c330699ac5c79015721..1e1614be46adee1287436a976b7371bd4160c766 100644
--- a/app/workers/gitlab/github_import/advance_stage_worker.rb
+++ b/app/workers/gitlab/github_import/advance_stage_worker.rb
@@ -43,9 +43,25 @@ def find_import_state(id)
private
+ def proceed_to_next_stage(import_state_jid, next_stage, project_id)
+ project = Project.find_by_id(project_id)
+ import_settings = Gitlab::GithubImport::Settings.new(project)
+
+ load_references(project) if import_settings.user_mapping_enabled?
+
+ super
+ end
+
def next_stage_worker(next_stage)
STAGES.fetch(next_stage.to_sym)
end
+
+ def load_references(project)
+ ::Import::LoadPlaceholderReferencesWorker.perform_async(
+ ::Import::SOURCE_GITHUB,
+ project.import_state.id,
+ { 'current_user_id' => project.creator_id })
+ end
end
end
end
diff --git a/app/workers/gitlab/github_import/stage/finish_import_worker.rb b/app/workers/gitlab/github_import/stage/finish_import_worker.rb
index 8d5a98136afe1980939a0b67bbbd77d1059d7b4c..3f59d83a6f8ca0158fff8d877d1a5f4f50fe3753 100644
--- a/app/workers/gitlab/github_import/stage/finish_import_worker.rb
+++ b/app/workers/gitlab/github_import/stage/finish_import_worker.rb
@@ -13,7 +13,11 @@ class FinishImportWorker # rubocop:disable Scalability/IdempotentWorker
# project - An instance of Project.
def import(_, project)
@project = project
+
+ return self.class.perform_in(30.seconds, project.id) if reference_store_pending?
+
project.after_import
+
report_import_time
end
@@ -21,6 +25,26 @@ def import(_, project)
attr_reader :project
+ def reference_store_pending?
+ return false unless import_settings(project).user_mapping_enabled?
+
+ return false unless placeholder_reference_store.any?
+
+ ::Import::LoadPlaceholderReferencesWorker.perform_async(
+ ::Import::SOURCE_GITHUB,
+ project.import_state.id,
+ { 'current_user_id' => project.creator.id }
+ )
+
+ info(
+ project.id,
+ message: 'Delaying finalization as placeholder references are pending',
+ placeholder_store_count: placeholder_reference_store.count
+ )
+
+ true
+ end
+
def report_import_time
metrics.track_finished_import
@@ -35,6 +59,13 @@ def report_import_time
def metrics
@metrics ||= Gitlab::Import::Metrics.new(:github_importer, project)
end
+
+ def placeholder_reference_store
+ @placeholder_reference_store ||= ::Import::PlaceholderReferences::Store.new(
+ import_source: ::Import::SOURCE_GITHUB,
+ import_uid: project.import_state.id
+ )
+ end
end
end
end
diff --git a/lib/gitlab/github_import/contributions_mapper.rb b/lib/gitlab/github_import/contributions_mapper.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0f391666edd09063b28d3e5341aba81f32b4086a
--- /dev/null
+++ b/lib/gitlab/github_import/contributions_mapper.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module GithubImport
+ class ContributionsMapper
+ def initialize(project)
+ @project = project
+ end
+
+ def user_mapper
+ ::Gitlab::Import::SourceUserMapper.new(
+ namespace: project.root_ancestor,
+ source_hostname: project.import_url,
+ import_type: ::Import::SOURCE_GITHUB
+ )
+ end
+
+ def user_mapping_enabled?
+ Gitlab::GithubImport::Settings.new(project).user_mapping_enabled?
+ end
+
+ private
+
+ attr_reader :project
+ end
+ end
+end
diff --git a/lib/gitlab/github_import/importer/issue_importer.rb b/lib/gitlab/github_import/importer/issue_importer.rb
index 97a76987e70ea78233a17e72d6a427a5abde6445..0b0a1b74128577aa4b97b2ed15536ec339454048 100644
--- a/lib/gitlab/github_import/importer/issue_importer.rb
+++ b/lib/gitlab/github_import/importer/issue_importer.rb
@@ -5,9 +5,10 @@ module GithubImport
module Importer
class IssueImporter
include Gitlab::Import::UsernameMentionRewriter
+ include Gitlab::GithubImport::PushPlaceholderReferences
attr_reader :project, :issue, :client, :user_finder, :milestone_finder,
- :issuable_finder
+ :issuable_finder, :mapper
# Imports an issue if it's a regular issue and not a pull request.
def self.import_if_issue(issue, project, client)
@@ -24,16 +25,28 @@ def initialize(issue, project, client)
@user_finder = GithubImport::UserFinder.new(project, client)
@milestone_finder = MilestoneFinder.new(project)
@issuable_finder = GithubImport::IssuableFinder.new(project, issue)
+ @mapper = Gitlab::GithubImport::ContributionsMapper.new(project)
end
def execute
new_issue = create_issue
issuable_finder.cache_database_id(new_issue.id)
+
+ push_issue_placeholder_references(new_issue)
+
new_issue
end
private
+ # Returns a Hash of { GitLabUserId => GitHubUserId }
+ # that can be used for both importing and pushing user references.
+ def issue_assignee_map
+ @map ||= issue.assignees.each_with_object({}) do |assignee, map|
+ map[user_finder.user_id_for(assignee)] = assignee[:id]
+ end
+ end
+
# Creates a new GitLab issue for the current GitHub issue.
def create_issue
author_id, author_found = user_finder.author_id_for(issue)
@@ -41,9 +54,7 @@ def create_issue
description = wrap_mentions_in_backticks(issue.description)
description = MarkdownText.format(description, issue.author, author_found)
- assignee_ids = issue.assignees.filter_map do |assignee|
- user_finder.user_id_for(assignee)
- end
+ assignee_ids = issue_assignee_map.keys.compact
attributes = {
iid: issue.iid,
@@ -63,6 +74,25 @@ def create_issue
project.issues.create!(attributes.merge(importing: true))
end
+
+ def push_issue_placeholder_references(new_issue)
+ return unless mapper.user_mapping_enabled?
+
+ user_mapper = mapper.user_mapper
+
+ push_with_record(new_issue, :author_id, issue.author.id, user_mapper)
+
+ new_issue.issue_assignees.each do |issue_assignee|
+ github_user_id = issue_assignee_map[issue_assignee.user_id]
+ push_with_composite_key(
+ issue_assignee,
+ :user_id,
+ { 'user_id' => issue_assignee.user_id, 'issue_id' => issue_assignee.issue_id },
+ github_user_id,
+ user_mapper
+ )
+ end
+ end
end
end
end
diff --git a/lib/gitlab/github_import/importer/note_importer.rb b/lib/gitlab/github_import/importer/note_importer.rb
index ce8aad344349176fc6b693013c25e08afd082275..69f1e6daccfa9444ea9ada844b77931ed08efc89 100644
--- a/lib/gitlab/github_import/importer/note_importer.rb
+++ b/lib/gitlab/github_import/importer/note_importer.rb
@@ -5,8 +5,9 @@ module GithubImport
module Importer
class NoteImporter
include Gitlab::Import::UsernameMentionRewriter
+ include Gitlab::GithubImport::PushPlaceholderReferences
- attr_reader :note, :project, :client, :user_finder
+ attr_reader :note, :project, :client, :user_finder, :mapper
# note - An instance of `Gitlab::GithubImport::Representation::Note`.
# project - An instance of `Project`.
@@ -16,6 +17,7 @@ def initialize(note, project, client)
@project = project
@client = client
@user_finder = GithubImport::UserFinder.new(project, client)
+ @mapper = Gitlab::GithubImport::ContributionsMapper.new(project)
end
def execute
@@ -47,7 +49,9 @@ def execute
# Note: if you're going to replace `legacy_bulk_insert` with something that trigger callback
# to generate HTML version - you also need to regenerate it in
# Gitlab::GithubImport::Importer::NoteAttachmentsImporter.
- ApplicationRecord.legacy_bulk_insert(Note.table_name, [attributes]) # rubocop:disable Gitlab/BulkInsert
+ ids = ApplicationRecord.legacy_bulk_insert(Note.table_name, [attributes], return_ids: true) # rubocop:disable Gitlab/BulkInsert
+
+ push_note_refs_with_ids(ids, mapper.user_mapper) if mapper.user_mapping_enabled?
end
# Returns the ID of the issue or merge request to create the note for.
diff --git a/lib/gitlab/github_import/push_placeholder_references.rb b/lib/gitlab/github_import/push_placeholder_references.rb
new file mode 100644
index 0000000000000000000000000000000000000000..5a7f695ef4cc55593c0104e5db0a541fd7eab476
--- /dev/null
+++ b/lib/gitlab/github_import/push_placeholder_references.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module GithubImport
+ # Contains methods to push placeholder references for user contribution mapping
+ module PushPlaceholderReferences
+ # Pushes a placeholder reference using .from_record
+ # Used when the record is available and the reference only requires a numeric key
+ def push_with_record(record, attribute, source_user_identifier, user_mapper)
+ source_user = user_mapper.find_source_user(source_user_identifier)
+
+ return if source_user.accepted_status?
+
+ ::Import::PlaceholderReferences::PushService.from_record(
+ import_source: ::Import::SOURCE_GITHUB,
+ import_uid: project.import_state.id,
+ record: record,
+ source_user: source_user,
+ user_reference_column: attribute
+ ).execute
+ end
+
+ # Pushes placeholder references for each Note record found via an id look-up using .new
+ # This is used as Note records are created using legacy_bulk_insert which
+ # can return the ids of records created, but not the records themselves
+ def push_note_refs_with_ids(ids, user_mapper)
+ ids.each do |id|
+ source_user = user_mapper.find_source_user(note[:author].id)
+
+ next if source_user.accepted_status?
+
+ ::Import::PlaceholderReferences::PushService.new(
+ import_source: ::Import::SOURCE_GITHUB,
+ import_uid: project.import_state.id,
+ source_user_id: source_user.id,
+ source_user_namespace_id: source_user.namespace_id,
+ model: Note,
+ user_reference_column: :author_id,
+ numeric_key: id).execute
+ end
+ end
+
+ # Pushes a placeholder reference using a composite key.
+ # This is used when the record requires a composite key for the reference.
+ def push_with_composite_key(record, attribute, composite_key, source_user_identifier, user_mapper)
+ source_user = user_mapper.find_source_user(source_user_identifier)
+ return if source_user.accepted_status?
+
+ ::Import::PlaceholderReferences::PushService.new(
+ import_source: ::Import::SOURCE_GITHUB,
+ import_uid: project.import_state.id,
+ source_user_id: source_user.id,
+ source_user_namespace_id: source_user.namespace_id,
+ model: record.class,
+ user_reference_column: attribute,
+ composite_key: composite_key
+ ).execute
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/github_import/user_finder.rb b/lib/gitlab/github_import/user_finder.rb
index 603851b1de392e91cc61fc8f4288340d1f9f0def..6e9749ec82ae2cfe14f0c138675c74cc434bc3c5 100644
--- a/lib/gitlab/github_import/user_finder.rb
+++ b/lib/gitlab/github_import/user_finder.rb
@@ -14,7 +14,7 @@ module GithubImport
class UserFinder
include Gitlab::ExclusiveLeaseHelpers
- attr_reader :project, :client
+ attr_reader :project, :client, :mapper
# The base cache key to use for caching user IDs for a given GitHub user ID.
ID_CACHE_KEY = 'github-import/user-finder/user-id/%s'
@@ -41,6 +41,7 @@ class UserFinder
def initialize(project, client)
@project = project
@client = client
+ @mapper = Gitlab::GithubImport::ContributionsMapper.new(project)
end
# Returns the GitLab user ID of an object's author.
@@ -62,6 +63,8 @@ def author_id_for(object, author_key: :author)
object ? object[:author] : nil
end
+ # TODO when improved user mapping is released we can refactor everything below to just
+ # user_id_for(user_info)
id = user_info ? user_id_for(user_info) : GithubImport.ghost_user_id
if id
@@ -80,7 +83,22 @@ def assignee_id_for(issuable)
#
# user - An instance of `Gitlab::GithubImport::Representation::User` or `Hash`.
def user_id_for(user)
- find(user[:id], user[:login]) if user.present?
+ return unless user.present?
+
+ if mapper.user_mapping_enabled?
+ source_user(user).mapped_user_id
+ else
+ find(user[:id], user[:login])
+ end
+ end
+
+ # Returns the GitLab user ID from placeholder or reassigned_to user.
+ def source_user(user)
+ mapper.user_mapper.find_or_create_source_user(
+ source_name: user[:login],
+ source_username: user[:login],
+ source_user_identifier: user[:id]
+ )
end
# Returns the GitLab ID for the given GitHub ID or username.
@@ -250,7 +268,7 @@ def email_fetched_for_project?(username)
def fetch_email_from_github(username, etag: nil)
log(EMAIL_API_CALL_LOGGING_MESSAGE[etag.present?], username: username)
- # Only make a rate-limited API call if the ETAG is not available })
+ # Only make a rate-limited API call if the ETAG is not available )
user = client.user(username, { headers: { 'If-None-Match' => etag }.compact })
user[:email] || '' if user
end
diff --git a/spec/lib/gitlab/github_import/contributions_mapper_spec.rb b/spec/lib/gitlab/github_import/contributions_mapper_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..e01ab0437db3fa2bd798d7170d9e8521376be8ea
--- /dev/null
+++ b/spec/lib/gitlab/github_import/contributions_mapper_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::GithubImport::ContributionsMapper, :clean_gitlab_redis_shared_state, feature_category: :importers do
+ let_it_be(:project) { create(:project, :with_import_url) }
+
+ let(:mapper) { described_class.new(project) }
+
+ let(:user_mapping_enabled) { true }
+
+ before do
+ project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: user_mapping_enabled })
+ end
+
+ describe '#user_mapper' do
+ it 'creates an instance of a source user mapper' do
+ expect(mapper.user_mapper).to be_an_instance_of(::Gitlab::Import::SourceUserMapper)
+ end
+ end
+
+ describe '#user_mapping_enabled?' do
+ context 'when user mapping is enabled' do
+ it 'returns true' do
+ expect(mapper.user_mapping_enabled?).to be true
+ end
+ end
+
+ context 'when user mapping is disbled' do
+ let(:user_mapping_enabled) { false }
+
+ it 'returns false' do
+ expect(mapper.user_mapping_enabled?).to be false
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb
index eb68059f35d917c3098e109b7b05565c529948b6..7fa35bf0287b7ad86f633d1158141ab2d3a02171 100644
--- a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb
@@ -3,16 +3,18 @@
require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redis_shared_state, feature_category: :importers do
+ include Import::UserMappingHelper
+
let_it_be(:work_item_type_id) { ::WorkItems::Type.default_issue_type.id }
- let_it_be(:project) { create(:project) }
- let_it_be(:user) { create(:user) }
- let_it_be(:user_2) { create(:user) }
+ let_it_be(:group) { create(:group) }
+ let_it_be(:project) { create(:project, :with_import_url, group: group) }
let_it_be(:milestone) { create(:milestone, project: project) }
let(:client) { instance_double(Gitlab::GithubImport::Client) }
let(:created_at) { Time.new(2017, 1, 1, 12, 00) }
let(:updated_at) { Time.new(2017, 1, 1, 12, 15) }
let(:description) { 'This is my issue' }
+ let(:author) { Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice') }
let(:issue) do
Gitlab::GithubImport::Representation::Issue.new(
@@ -22,11 +24,11 @@
milestone_number: 1,
state: :opened,
assignees: [
- Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice'),
+ author,
Gitlab::GithubImport::Representation::User.new(id: 5, login: 'bob')
],
label_names: %w[bug],
- author: Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice'),
+ author: author,
created_at: created_at,
updated_at: updated_at,
pull_request: false,
@@ -34,6 +36,31 @@
)
end
+ let(:user_mapping_enabled) { true }
+ let_it_be(:source_user_alice) do
+ create(
+ :import_source_user,
+ source_user_identifier: '4',
+ source_hostname: project.import_url,
+ namespace_id: group.id
+ )
+ end
+
+ let_it_be(:source_user_bob) do
+ create(
+ :import_source_user,
+ source_user_identifier: '5',
+ source_hostname: project.import_url,
+ namespace_id: group.id
+ )
+ end
+
+ let(:importer) { described_class.new(issue, project, client) }
+
+ before do
+ project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: user_mapping_enabled })
+ end
+
describe '.import_if_issue' do
it 'imports an issuable if it is a regular issue' do
expect_next_instance_of(Gitlab::GithubImport::Importer::IssueImporter, issue, project, client) do |importer|
@@ -53,29 +80,17 @@
end
describe '#execute' do
- let(:importer) { described_class.new(issue, project, client) }
-
- before do
- allow_next_instance_of(Gitlab::GithubImport::MilestoneFinder) do |finder|
- allow(finder).to receive(:id_for).with(issue).and_return(milestone.id)
- end
-
- allow_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder|
- allow(finder).to receive(:author_id_for).with(issue).and_return([project.creator_id, false])
- allow(finder).to receive(:user_id_for).and_return(nil)
- end
- end
-
it 'creates the issue' do
expect { importer.execute }.to change { Issue.count }.by(1)
+
expect(Issue.last).to have_attributes(
iid: 42,
title: 'My Issue',
- author_id: project.creator_id,
- assignee_ids: [],
+ author_id: source_user_alice.mapped_user_id,
+ assignee_ids: contain_exactly(source_user_bob.mapped_user_id, source_user_alice.mapped_user_id),
project_id: project.id,
namespace_id: project.project_namespace_id,
- description: "*Created by: alice*\n\nThis is my issue",
+ description: "This is my issue",
milestone_id: milestone.id,
state_id: 1,
created_at: created_at,
@@ -93,43 +108,139 @@
expect(database_id).to eq(Issue.last.id)
end
- context 'when author is mapped to a user' do
- it 'sets the author ID to the mapped user and preserves the original issue description' do
- expect_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder|
- expect(finder).to receive(:author_id_for).and_return([user.id, true])
- expect(finder).to receive(:user_id_for).and_return(nil).twice
- end
+ it 'pushes the author and assignee references' do
+ importer.execute
+
+ created_issue = Issue.last
+ user_references = placeholder_user_references(::Import::SOURCE_GITHUB, project.import_state.id)
+
+ expect(user_references).to match_array([
+ ['Issue', created_issue.id, 'author_id', source_user_alice.id],
+ [
+ 'IssueAssignee', { 'user_id' => source_user_alice.mapped_user_id, 'issue_id' => created_issue.id },
+ 'user_id', source_user_alice.id
+ ],
+ [
+ 'IssueAssignee', { 'user_id' => source_user_bob.mapped_user_id, 'issue_id' => created_issue.id },
+ 'user_id', source_user_bob.id
+ ]
+ ])
+ end
+
+ context 'when the description has user mentions' do
+ let(:description) { 'You can ask @knejad by emailing xyz@gitlab.com' }
+ it 'adds backticks to the username' do
importer.execute
- expect(Issue.last).to have_attributes(
- description: "This is my issue",
- author_id: user.id
- )
+ expect(Issue.last.description).to eq("You can ask `@knejad` by emailing xyz@gitlab.com")
end
end
+ end
- context 'when assigness are mapped to users' do
- it 'sets the assignee IDs to the mapped users' do
- expect_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder|
- expect(finder).to receive(:author_id_for).and_return([user.id, true])
- expect(finder).to receive(:user_id_for).and_return(user.id)
- expect(finder).to receive(:user_id_for).and_return(user_2.id)
+ context 'when user_mapping is not enabled' do
+ let(:user_mapping_enabled) { false }
+
+ describe '.import_if_issue' do
+ it 'imports an issuable if it is a regular issue' do
+ expect_next_instance_of(Gitlab::GithubImport::Importer::IssueImporter, issue, project, client) do |importer|
+ expect(importer).to receive(:execute)
end
- importer.execute
+ described_class.import_if_issue(issue, project, client)
+ end
- expect(Issue.last.assignee_ids).to match_array([user.id, user_2.id])
+ it 'does not import the issuable if it is a pull request' do
+ expect(issue).to receive(:pull_request?).and_return(true)
+
+ expect(described_class).not_to receive(:new)
+
+ described_class.import_if_issue(issue, project, client)
end
end
- context 'when the description has user mentions' do
- let(:description) { 'You can ask @knejad by emailing xyz@gitlab.com' }
+ describe '#execute' do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:user_2) { create(:user) }
+ let(:importer) { described_class.new(issue, project, client) }
- it 'adds backticks to the username' do
+ before do
+ allow_next_instance_of(Gitlab::GithubImport::MilestoneFinder) do |finder|
+ allow(finder).to receive(:id_for).with(issue).and_return(milestone.id)
+ end
+
+ allow_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder|
+ allow(finder).to receive(:author_id_for).with(issue).and_return([project.creator_id, false])
+ allow(finder).to receive(:user_id_for).and_return(nil)
+ end
+ end
+
+ it 'creates the issue' do
+ expect { importer.execute }.to change { Issue.count }.by(1)
+ expect(Issue.last).to have_attributes(
+ iid: 42,
+ title: 'My Issue',
+ author_id: project.creator_id,
+ assignee_ids: [],
+ project_id: project.id,
+ namespace_id: project.project_namespace_id,
+ description: "*Created by: alice*\n\nThis is my issue",
+ milestone_id: milestone.id,
+ state_id: 1,
+ created_at: created_at,
+ updated_at: updated_at,
+ work_item_type_id: work_item_type_id,
+ imported_from: 'github'
+ )
+ end
+
+ it 'caches the created issue ID' do
importer.execute
- expect(Issue.last.description).to eq("*Created by: alice*\n\nYou can ask `@knejad` by emailing xyz@gitlab.com")
+ database_id = Gitlab::GithubImport::IssuableFinder.new(project, issue).database_id
+
+ expect(database_id).to eq(Issue.last.id)
+ end
+
+ context 'when author is mapped to a user' do
+ it 'sets the author ID to the mapped user and preserves the original issue description' do
+ expect_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder|
+ expect(finder).to receive(:author_id_for).and_return([user.id, true])
+ expect(finder).to receive(:user_id_for).and_return(nil).twice
+ end
+
+ importer.execute
+
+ expect(Issue.last).to have_attributes(
+ description: "This is my issue",
+ author_id: user.id
+ )
+ end
+ end
+
+ context 'when assigness are mapped to users' do
+ it 'sets the assignee IDs to the mapped users' do
+ expect_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder|
+ expect(finder).to receive(:author_id_for).and_return([user.id, true])
+ expect(finder).to receive(:user_id_for).and_return(user.id)
+ expect(finder).to receive(:user_id_for).and_return(user_2.id)
+ end
+
+ importer.execute
+
+ expect(Issue.last.assignee_ids).to match_array([user.id, user_2.id])
+ end
+ end
+
+ context 'when the description has user mentions' do
+ let(:description) { 'You can ask @knejad by emailing xyz@gitlab.com' }
+
+ it 'adds backticks to the username' do
+ importer.execute
+
+ expect(Issue.last.description)
+ .to eq("*Created by: alice*\n\nYou can ask `@knejad` by emailing xyz@gitlab.com")
+ end
end
end
end
diff --git a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
index 893ed47584327ac12fccc9180bcf0d820bef3a5e..1a3d222a168220d21f237e509c1c666238cca4e6 100644
--- a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
@@ -3,20 +3,31 @@
require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Importer::NoteImporter, feature_category: :importers do
+ let_it_be(:imported_from) { ::Import::HasImportSource::IMPORT_SOURCES[:github] }
+ let_it_be(:project) { create(:project, :with_import_url) }
+ let_it_be(:user) { create(:user) }
+
+ let_it_be(:source_user) do
+ create(:import_source_user,
+ placeholder_user_id: user.id,
+ namespace_id: project.root_ancestor.id,
+ source_user_identifier: '4',
+ source_hostname: project.import_url
+ )
+ end
+
let(:client) { double(:client) }
- let(:project) { create(:project) }
- let(:user) { create(:user) }
let(:created_at) { Time.new(2017, 1, 1, 12, 00) }
let(:updated_at) { Time.new(2017, 1, 1, 12, 15) }
let(:note_body) { 'This is my note' }
- let_it_be(:imported_from) { ::Import::HasImportSource::IMPORT_SOURCES[:github] }
+ let(:import_state) { create(:import_state, :started, project: project) }
let(:github_note) do
Gitlab::GithubImport::Representation::Note.new(
note_id: 100,
noteable_id: 1,
noteable_type: 'Issue',
- author: Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice'),
+ author: Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice', email: 'alice@alice.com'),
note: note_body,
created_at: created_at,
updated_at: updated_at
@@ -24,6 +35,11 @@
end
let(:importer) { described_class.new(github_note, project, client) }
+ let(:user_mapping_enabled) { true }
+
+ before do
+ project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: user_mapping_enabled })
+ end
describe '#execute' do
context 'when the noteable exists' do
@@ -35,12 +51,9 @@
.and_return(issue_row.id)
end
- context 'when the author could be found' do
- it 'imports the note with the found author as the note author' do
- expect(importer.user_finder)
- .to receive(:author_id_for)
- .with(github_note)
- .and_return([user.id, true])
+ context 'when user_mapping_enabled is true' do
+ it 'maps the correct user and pushes a reference' do
+ expect(importer.user_finder).to receive(:author_id_for).with(github_note).and_call_original
expect(ApplicationRecord)
.to receive(:legacy_bulk_insert)
@@ -52,7 +65,7 @@
noteable_id: issue_row.id,
project_id: project.id,
namespace_id: project.project_namespace_id,
- author_id: user.id,
+ author_id: source_user.mapped_user_id,
note: 'This is my note',
discussion_id: match(/\A[0-9a-f]{40}\z/),
system: false,
@@ -60,44 +73,106 @@
updated_at: updated_at,
imported_from: imported_from
}
- ]
+ ],
+ { return_ids: true }
)
.and_call_original
+ expect_next_instance_of(::Import::PlaceholderReferences::PushService,
+ import_source: ::Import::SOURCE_GITHUB,
+ import_uid: project.import_state.id,
+ source_user_id: source_user.id,
+ source_user_namespace_id: project.root_ancestor.id,
+ model: Note,
+ user_reference_column: :author_id,
+ numeric_key: an_instance_of(Integer)) do |push_service|
+ expect(push_service).to receive(:execute).and_call_original
+ end
+
importer.execute
end
end
- context 'when the note author could not be found' do
- it 'imports the note with the project creator as the note author' do
- expect(importer.user_finder)
- .to receive(:author_id_for)
- .with(github_note)
- .and_return([project.creator_id, false])
+ context 'when user_mapping_enabled is false' do
+ let(:user_mapping_enabled) { false }
- expect(ApplicationRecord)
- .to receive(:legacy_bulk_insert)
- .with(
- Note.table_name,
- [
+ before do
+ allow(importer.user_finder)
+ .to receive(:email_for_github_username)
+ .and_return('alice@alice.com')
+ end
+
+ context 'when the author could be found' do
+ it 'imports the note with the found author as the note author and does not push a placeholder reference' do
+ expect(importer.user_finder)
+ .to receive(:author_id_for)
+ .with(github_note)
+ .and_return([user.id, true])
+
+ expect(ApplicationRecord)
+ .to receive(:legacy_bulk_insert)
+ .with(
+ Note.table_name,
+ [
+ {
+ noteable_type: 'Issue',
+ noteable_id: issue_row.id,
+ project_id: project.id,
+ namespace_id: project.project_namespace_id,
+ author_id: user.id,
+ note: 'This is my note',
+ discussion_id: match(/\A[0-9a-f]{40}\z/),
+ system: false,
+ created_at: created_at,
+ updated_at: updated_at,
+ imported_from: imported_from
+ }
+ ],
+ { return_ids: true }
+ )
+ .and_call_original
+
+ expect(::Import::PlaceholderReferences::PushService)
+ .not_to receive(:new)
+
+ importer.execute
+ end
+ end
+
+ context 'when the note author could not be found' do
+ it 'imports the note with the project creator as the note author' do
+ expect(importer.user_finder)
+ .to receive(:author_id_for)
+ .with(github_note)
+ .and_return([project.creator_id, false])
+
+ expect(ApplicationRecord)
+ .to receive(:legacy_bulk_insert)
+ .with(
+ Note.table_name,
+ [
+ {
+ noteable_type: 'Issue',
+ noteable_id: issue_row.id,
+ project_id: project.id,
+ namespace_id: project.project_namespace_id,
+ author_id: project.creator_id,
+ note: "*Created by: alice*\n\nThis is my note",
+ discussion_id: match(/\A[0-9a-f]{40}\z/),
+ system: false,
+ created_at: created_at,
+ updated_at: updated_at,
+ imported_from: imported_from
+ }
+ ],
{
- noteable_type: 'Issue',
- noteable_id: issue_row.id,
- project_id: project.id,
- namespace_id: project.project_namespace_id,
- author_id: project.creator_id,
- note: "*Created by: alice*\n\nThis is my note",
- discussion_id: match(/\A[0-9a-f]{40}\z/),
- system: false,
- created_at: created_at,
- updated_at: updated_at,
- imported_from: imported_from
+ return_ids: true
}
- ]
- )
- .and_call_original
+ )
+ .and_call_original
- importer.execute
+ importer.execute
+ end
end
end
diff --git a/spec/lib/gitlab/github_import/importer/pull_requests/review_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests/review_request_importer_spec.rb
index b1f84213ad994c4a6e433da239ffc36ddaffb92d..b231d8eb6b5f9113ee730ce4043c661a40a8211f 100644
--- a/spec/lib/gitlab/github_import/importer/pull_requests/review_request_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/pull_requests/review_request_importer_spec.rb
@@ -2,10 +2,10 @@
require 'spec_helper'
-RSpec.describe Gitlab::GithubImport::Importer::PullRequests::ReviewRequestImporter, :clean_gitlab_redis_shared_state do
+RSpec.describe Gitlab::GithubImport::Importer::PullRequests::ReviewRequestImporter, :clean_gitlab_redis_shared_state, feature_category: :importers do
subject(:importer) { described_class.new(review_request, project, client) }
- let(:project) { instance_double('Project') }
+ let_it_be(:project) { create(:project) }
let(:client) { instance_double('Gitlab::GithubImport::Client') }
let(:merge_request) { create(:merge_request) }
let(:reviewer) { create(:user, username: 'alice') }
diff --git a/spec/lib/gitlab/github_import/push_placeholder_references_spec.rb b/spec/lib/gitlab/github_import/push_placeholder_references_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..43e64ee52c35ca51bded24fb0ccf727a45f913ef
--- /dev/null
+++ b/spec/lib/gitlab/github_import/push_placeholder_references_spec.rb
@@ -0,0 +1,108 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::GithubImport::PushPlaceholderReferences, feature_category: :importers do
+ let(:client) { instance_double(Gitlab::GithubImport::Client) }
+ let_it_be(:source_id) { 'source_identifier' }
+ let_it_be(:project) { create(:project, :with_import_url) }
+ let_it_be(:author) { create(:user) }
+ let_it_be(:import_source_user) do
+ create(
+ :import_source_user,
+ source_user_identifier: source_id,
+ source_hostname: project.import_url,
+ namespace_id: project.root_ancestor.id,
+ placeholder_user_id: author.id
+ )
+ end
+
+ let(:github_note) do
+ Gitlab::GithubImport::Representation::Note.new(
+ note_id: 100,
+ noteable_id: 1,
+ noteable_type: 'Issue',
+ author: Gitlab::GithubImport::Representation::User.new(id: source_id, login: 'alice'),
+ note: 'This is my note',
+ created_at: Time.current,
+ updated_at: Time.current
+ )
+ end
+
+ let(:gitlab_note) { create(:note) }
+ let(:user_mapper) { Gitlab::GithubImport::ContributionsMapper.new(project).user_mapper }
+
+ let(:importer) { Gitlab::GithubImport::Importer::NoteImporter.new(github_note, project, client) }
+
+ before do
+ project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: true })
+ end
+
+ describe '#push_with_record' do
+ before do
+ allow(::Import::PlaceholderReferences::PushService)
+ .to receive_message_chain(:from_record, :execute)
+ end
+
+ it 'pushes the reference using .from_record' do
+ importer.push_with_record(gitlab_note, :author_id, source_id, user_mapper)
+
+ expect(::Import::PlaceholderReferences::PushService)
+ .to have_received(:from_record)
+ .with(
+ import_source: ::Import::SOURCE_GITHUB,
+ import_uid: project.import_state.id,
+ record: gitlab_note,
+ source_user: import_source_user,
+ user_reference_column: :author_id)
+ end
+ end
+
+ describe '#push_note_refs_with_ids' do
+ before do
+ allow(::Import::PlaceholderReferences::PushService)
+ .to receive_message_chain(:new, :execute)
+ end
+
+ it 'pushes the reference using .new' do
+ importer.push_note_refs_with_ids([gitlab_note.id], user_mapper)
+
+ expect(::Import::PlaceholderReferences::PushService)
+ .to have_received(:new)
+ .with(
+ import_source: ::Import::SOURCE_GITHUB,
+ import_uid: project.import_state.id,
+ source_user_id: import_source_user.id,
+ source_user_namespace_id: import_source_user.namespace_id,
+ model: gitlab_note.class,
+ user_reference_column: :author_id,
+ numeric_key: gitlab_note.id
+ )
+ end
+ end
+
+ describe '#push_with_composite_key' do
+ let(:composite_key) { { "user_id" => "user_id", "merge_request_id" => "merge_request_id" } }
+
+ before do
+ allow(::Import::PlaceholderReferences::PushService)
+ .to receive_message_chain(:new, :execute)
+ end
+
+ it 'pushes the reference with composite key' do
+ importer.push_with_composite_key(gitlab_note, :user_id, composite_key, source_id, user_mapper)
+
+ expect(::Import::PlaceholderReferences::PushService)
+ .to have_received(:new)
+ .with(
+ import_source: ::Import::SOURCE_GITHUB,
+ import_uid: project.import_state.id,
+ source_user_id: import_source_user.id,
+ source_user_namespace_id: import_source_user.namespace_id,
+ model: gitlab_note.class,
+ user_reference_column: :user_id,
+ composite_key: composite_key
+ )
+ end
+ end
+end
diff --git a/spec/lib/gitlab/github_import/user_finder_spec.rb b/spec/lib/gitlab/github_import/user_finder_spec.rb
index 1cd1e76c5c2743c96f9fa47a99453d541805db36..63d1ac771b907be93d2160659e266af3ee480192 100644
--- a/spec/lib/gitlab/github_import/user_finder_spec.rb
+++ b/spec/lib/gitlab/github_import/user_finder_spec.rb
@@ -3,16 +3,22 @@
require 'spec_helper'
RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_shared_state, feature_category: :importers do
- let(:project) do
+ let_it_be(:project) do
create(
:project,
import_type: 'github',
- import_url: 'https://api.github.com/user/repo'
+ import_url: 'https://github.com/user/repo.git'
)
end
- let(:client) { double(:client) }
+ let(:client) { instance_double(Gitlab::GithubImport::Client) }
let(:finder) { described_class.new(project, client) }
+ let(:settings) { Gitlab::GithubImport::Settings.new }
+ let(:user_mapping_enabled) { true }
+
+ before do
+ project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: user_mapping_enabled })
+ end
describe '#author_id_for' do
context 'with default author_key' do
@@ -100,15 +106,42 @@
end
describe '#user_id_for' do
- it 'returns the user ID for the given user' do
- user = { id: 4, login: 'kittens' }
+ context 'when user mapping is disabled' do
+ let(:user_mapping_enabled) { false }
- expect(finder).to receive(:find).with(user[:id], user[:login]).and_return(42)
- expect(finder.user_id_for(user)).to eq(42)
+ it 'returns the user ID for the given user' do
+ user = { id: 4, login: 'kittens' }
+
+ expect(finder).to receive(:find).with(user[:id], user[:login]).and_return(42)
+ expect(finder.user_id_for(user)).to eq(42)
+ end
end
- it 'does not fail with empty input' do
- expect(finder.user_id_for(nil)).to eq(nil)
+ context 'when user mapping is enabled' do
+ let!(:source_user) do
+ create(:import_source_user,
+ namespace_id: project.root_ancestor.id,
+ source_user_identifier: '7',
+ source_hostname: 'https://github.com'
+ )
+ end
+
+ it 'returns the mapped_user_id of source user with matching user identifier' do
+ user = { id: 7, login: 'anything' }
+
+ expect(finder.user_id_for(user)).to eq(source_user.mapped_user_id)
+ end
+
+ it 'creates a new source user when user identifier does not match' do
+ user = { id: 6, login: 'anything' }
+
+ expect { finder.user_id_for(user) }.to change { Import::SourceUser.count }.by(1)
+ expect(finder.user_id_for(user)).not_to eq(source_user.mapped_user_id)
+ end
+
+ it 'does not fail with empty input' do
+ expect(finder.user_id_for(nil)).to eq(nil)
+ end
end
end
@@ -535,6 +568,10 @@
describe '#id_for_github_id' do
let(:id) { 4 }
+ before do
+ allow(project).to receive(:github_enterprise_import?).and_return(false)
+ end
+
it 'queries and caches the user ID for a given GitHub ID' do
expect(finder).to receive(:query_id_for_github_id)
.with(id)
@@ -560,12 +597,8 @@
end
context 'when importing from github enterprise' do
- let(:project) do
- create(
- :project,
- import_type: 'github',
- import_url: 'https://othergithub.net/user/repo'
- )
+ before do
+ allow(project).to receive(:github_enterprise_import?).and_return(true)
end
it 'does not look up the user by external id' do
@@ -583,6 +616,10 @@
describe '#id_for_github_email' do
let(:email) { 'kittens@example.com' }
+ before do
+ allow(project).to receive(:github_enterprise_import?).and_return(true)
+ end
+
it 'queries and caches the user ID for a given Email address' do
expect(finder).to receive(:query_id_for_github_email)
.with(email)
diff --git a/spec/support/helpers/import/user_mapping_helper.rb b/spec/support/helpers/import/user_mapping_helper.rb
index e9b3ec1ea4cceaef35fa7384013e5696ed6ccc33..49a80d39580c7d3d0fc80b3ddff2eb1591a7672e 100644
--- a/spec/support/helpers/import/user_mapping_helper.rb
+++ b/spec/support/helpers/import/user_mapping_helper.rb
@@ -23,8 +23,9 @@ def placeholder_user_references(import_type, import_uid, limit = 100)
user_references.map do |item|
item = Import::SourceUserPlaceholderReference.from_serialized(item)
+ key = item.numeric_key || item.composite_key
- [item.model, item.numeric_key, item.user_reference_column, item.source_user_id]
+ [item.model, key, item.user_reference_column, item.source_user_id]
end
end
end
diff --git a/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb b/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb
index dcf016c550b68b75e68c7cf751a9126debf54276..9e4b55bca7afe03b18156a44d550e85e59503a17 100644
--- a/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb
+++ b/spec/workers/gitlab/github_import/advance_stage_worker_spec.rb
@@ -8,4 +8,39 @@
it 'has a Sidekiq retry of 6' do
expect(described_class.sidekiq_options['retry']).to eq(6)
end
+
+ context 'when there are no remaining jobs' do
+ let_it_be(:project) { create(:project, :import_user_mapping_enabled, import_status: :started, import_jid: '123') }
+
+ subject(:worker) { described_class.new }
+
+ before do
+ allow(worker)
+ .to receive(:wait_for_jobs)
+ .with({ '123' => 2 })
+ .and_return({})
+ end
+
+ it 'enqueues LoadPlaceholderReferencesWorker to save placeholder references' do
+ expect(::Import::LoadPlaceholderReferencesWorker).to receive(:perform_async).with(
+ ::Import::SOURCE_GITHUB,
+ project.import_state.id,
+ { 'current_user_id' => project.creator_id }
+ )
+
+ worker.perform(project.id, { '123' => 2 }, 'finish')
+ end
+
+ context 'when user contribution mapping is disabled' do
+ before do
+ allow(Gitlab::GithubImport::Settings).to receive_message_chain(:new, :user_mapping_enabled?).and_return(false)
+ end
+
+ it 'does not enqueue LoadPlaceholderReferencesWorker' do
+ expect(::Import::LoadPlaceholderReferencesWorker).not_to receive(:perform_async)
+
+ worker.perform(project.id, { '123' => 2 }, 'finish')
+ end
+ end
+ end
end
diff --git a/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb b/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb
index ba8fcdb240602ff277511e06a0ab6718910c50ca..dea2d61e481bb64f4bf22fffdbf79c5b278dc023 100644
--- a/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb
+++ b/spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb
@@ -3,13 +3,18 @@
require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Stage::FinishImportWorker, feature_category: :importers do
- let_it_be(:project) { create(:project) }
+ let_it_be_with_reload(:project) { create(:project) }
+ let_it_be(:import_state) { create(:import_state, :started, project: project) }
subject(:worker) { described_class.new }
it_behaves_like Gitlab::GithubImport::StageMethods
describe '#perform' do
+ before do
+ project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: true })
+ end
+
it 'marks the import as finished and reports import statistics' do
expect(project).to receive(:after_import)
expect_next_instance_of(Gitlab::Import::Metrics) do |instance|
@@ -34,5 +39,43 @@
worker.import(double(:client), project)
end
+
+ context 'when the reference store is empty' do
+ it 'checks the reference store and does not push placeholder references' do
+ allow(described_class).to receive(:perform_in)
+ allow(worker).to receive_message_chain(:placeholder_reference_store, :any?).and_return(false)
+
+ expect(Import::LoadPlaceholderReferencesWorker).not_to receive(:perform_async)
+
+ worker.import(double(:client), project)
+
+ expect(described_class).not_to have_received(:perform_in)
+ end
+ end
+
+ context 'when the reference store is not empty' do
+ it 'checks the reference store, queues LoadPlaceholderReferencesWorker, and requeues itself' do
+ allow(described_class).to receive(:perform_in)
+ allow(worker).to receive_message_chain(:placeholder_reference_store, :any?).and_return(true)
+ allow(worker).to receive_message_chain(:placeholder_reference_store, :count).and_return(1)
+
+ expect(Import::LoadPlaceholderReferencesWorker).to receive(:perform_async)
+
+ expect(Gitlab::GithubImport::Logger)
+ .to receive(:info)
+ .with(
+ {
+ message: 'Delaying finalization as placeholder references are pending',
+ import_stage: 'Gitlab::GithubImport::Stage::FinishImportWorker',
+ placeholder_store_count: 1,
+ project_id: project.id
+ }
+ )
+
+ worker.import(double(:client), project)
+
+ expect(described_class).to have_received(:perform_in).with(30.seconds, project.id)
+ end
+ end
end
end