Commit 153bca12 authored by Kassio Borges's avatar Kassio Borges

Fix members mapping when importing from GitLab

Use the original author id to find the members when importing from
GitLab.
parent 67149ad1
---
title: Fixed a bug causing 'Missing author note' to be added to notes for mapped users when importing project using GitLab Import.
merge_request: 42648
author:
type: fixed
...@@ -5,16 +5,19 @@ require 'spec_helper' ...@@ -5,16 +5,19 @@ require 'spec_helper'
RSpec.describe Gitlab::ImportExport::Group::RelationFactory do RSpec.describe Gitlab::ImportExport::Group::RelationFactory do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:members_mapper) { double('members_mapper').as_null_object } let(:members_mapper) { double('members_mapper').as_null_object }
let(:user) { create(:admin) } let(:admin) { create(:admin) }
let(:importer_user) { admin }
let(:excluded_keys) { [] } let(:excluded_keys) { [] }
let(:created_object) do let(:created_object) do
described_class.create(relation_sym: relation_sym, described_class.create(
relation_sym: relation_sym,
relation_hash: relation_hash, relation_hash: relation_hash,
members_mapper: members_mapper, members_mapper: members_mapper,
object_builder: Gitlab::ImportExport::Group::ObjectBuilder, object_builder: Gitlab::ImportExport::Group::ObjectBuilder,
user: user, user: importer_user,
importable: group, importable: group,
excluded_keys: excluded_keys) excluded_keys: excluded_keys
)
end end
context 'epic object' do context 'epic object' do
...@@ -65,27 +68,8 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do ...@@ -65,27 +68,8 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do
end end
end end
context 'Notes user references' do it_behaves_like 'Notes user references' do
let(:relation_sym) { :notes } let(:importable) { group }
let(:new_user) { create(:user) }
let(:exported_member) do
{
'id' => 111,
'access_level' => 30,
'source_id' => 1,
'source_type' => 'Namespace',
'user_id' => 3,
'notification_level' => 3,
'created_at' => '2016-11-18T09:29:42.634Z',
'updated_at' => '2016-11-18T09:29:42.634Z',
'user' => {
'id' => 999,
'email' => new_user.email,
'username' => new_user.username
}
}
end
let(:relation_hash) do let(:relation_hash) do
{ {
'id' => 4947, 'id' => 4947,
...@@ -106,17 +90,6 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do ...@@ -106,17 +90,6 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do
'events' => [] 'events' => []
} }
end end
let(:members_mapper) do
Gitlab::ImportExport::MembersMapper.new(
exported_members: [exported_member],
user: user,
importable: group)
end
it 'maps the right author to the imported note' do
expect(created_object.author).to eq(new_user)
end
end end
def random_id def random_id
......
...@@ -53,6 +53,7 @@ module Gitlab ...@@ -53,6 +53,7 @@ module Gitlab
@importable = importable @importable = importable
@imported_object_retries = 0 @imported_object_retries = 0
@relation_hash[importable_column_name] = @importable.id @relation_hash[importable_column_name] = @importable.id
@original_user = {}
# Remove excluded keys from relation_hash # Remove excluded keys from relation_hash
# We don't do this in the parsed_relation_hash because of the 'transformed attributes' # We don't do this in the parsed_relation_hash because of the 'transformed attributes'
...@@ -112,6 +113,7 @@ module Gitlab ...@@ -112,6 +113,7 @@ module Gitlab
def update_user_references def update_user_references
self.class::USER_REFERENCES.each do |reference| self.class::USER_REFERENCES.each do |reference|
if @relation_hash[reference] if @relation_hash[reference]
@original_user[reference] = @relation_hash[reference]
@relation_hash[reference] = @members_mapper.map[@relation_hash[reference]] @relation_hash[reference] = @members_mapper.map[@relation_hash[reference]]
end end
end end
...@@ -243,7 +245,7 @@ module Gitlab ...@@ -243,7 +245,7 @@ module Gitlab
# will be used. Otherwise, a note stating the original author name # will be used. Otherwise, a note stating the original author name
# is left. # is left.
def set_note_author def set_note_author
old_author_id = @relation_hash['author_id'] old_author_id = @original_user['author_id']
author = @relation_hash.delete('author') author = @relation_hash.delete('author')
update_note_for_missing_author(author['name']) unless has_author?(old_author_id) update_note_for_missing_author(author['name']) unless has_author?(old_author_id)
......
...@@ -34,8 +34,8 @@ module Gitlab ...@@ -34,8 +34,8 @@ module Gitlab
@user.id @user.id
end end
def include?(old_author_id) def include?(old_user_id)
map.has_key?(old_author_id) && map[old_author_id] != default_user_id map.has_key?(old_user_id) && map[old_user_id] != default_user_id
end end
private private
......
...@@ -5,16 +5,19 @@ require 'spec_helper' ...@@ -5,16 +5,19 @@ require 'spec_helper'
RSpec.describe Gitlab::ImportExport::Group::RelationFactory do RSpec.describe Gitlab::ImportExport::Group::RelationFactory do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:members_mapper) { double('members_mapper').as_null_object } let(:members_mapper) { double('members_mapper').as_null_object }
let(:user) { create(:admin) } let(:admin) { create(:admin) }
let(:importer_user) { admin }
let(:excluded_keys) { [] } let(:excluded_keys) { [] }
let(:created_object) do let(:created_object) do
described_class.create(relation_sym: relation_sym, described_class.create(
relation_sym: relation_sym,
relation_hash: relation_hash, relation_hash: relation_hash,
members_mapper: members_mapper, members_mapper: members_mapper,
object_builder: Gitlab::ImportExport::Group::ObjectBuilder, object_builder: Gitlab::ImportExport::Group::ObjectBuilder,
user: user, user: importer_user,
importable: group, importable: group,
excluded_keys: excluded_keys) excluded_keys: excluded_keys
)
end end
context 'label object' do context 'label object' do
...@@ -60,27 +63,8 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do ...@@ -60,27 +63,8 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do
end end
end end
context 'Notes user references' do it_behaves_like 'Notes user references' do
let(:relation_sym) { :notes } let(:importable) { group }
let(:new_user) { create(:user) }
let(:exported_member) do
{
'id' => 111,
'access_level' => 30,
'source_id' => 1,
'source_type' => 'Namespace',
'user_id' => 3,
'notification_level' => 3,
'created_at' => '2016-11-18T09:29:42.634Z',
'updated_at' => '2016-11-18T09:29:42.634Z',
'user' => {
'id' => 999,
'email' => new_user.email,
'username' => new_user.username
}
}
end
let(:relation_hash) do let(:relation_hash) do
{ {
'id' => 4947, 'id' => 4947,
...@@ -101,17 +85,6 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do ...@@ -101,17 +85,6 @@ RSpec.describe Gitlab::ImportExport::Group::RelationFactory do
'events' => [] 'events' => []
} }
end end
let(:members_mapper) do
Gitlab::ImportExport::MembersMapper.new(
exported_members: [exported_member],
user: user,
importable: group)
end
it 'maps the right author to the imported note' do
expect(created_object.author).to eq(new_user)
end
end end
def random_id def random_id
......
...@@ -6,16 +6,19 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do ...@@ -6,16 +6,19 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:project) { create(:project, :repository, group: group) } let(:project) { create(:project, :repository, group: group) }
let(:members_mapper) { double('members_mapper').as_null_object } let(:members_mapper) { double('members_mapper').as_null_object }
let(:user) { create(:admin) } let(:admin) { create(:admin) }
let(:importer_user) { admin }
let(:excluded_keys) { [] } let(:excluded_keys) { [] }
let(:created_object) do let(:created_object) do
described_class.create(relation_sym: relation_sym, described_class.create(
relation_sym: relation_sym,
relation_hash: relation_hash, relation_hash: relation_hash,
object_builder: Gitlab::ImportExport::Project::ObjectBuilder, object_builder: Gitlab::ImportExport::Project::ObjectBuilder,
members_mapper: members_mapper, members_mapper: members_mapper,
user: user, user: importer_user,
importable: project, importable: project,
excluded_keys: excluded_keys) excluded_keys: excluded_keys
)
end end
before do before do
...@@ -113,9 +116,9 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do ...@@ -113,9 +116,9 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do
"created_at" => "2016-11-18T09:29:42.634Z", "created_at" => "2016-11-18T09:29:42.634Z",
"updated_at" => "2016-11-18T09:29:42.634Z", "updated_at" => "2016-11-18T09:29:42.634Z",
"user" => { "user" => {
"id" => user.id, "id" => admin.id,
"email" => user.email, "email" => admin.email,
"username" => user.username "username" => admin.username
} }
} }
end end
...@@ -123,7 +126,7 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do ...@@ -123,7 +126,7 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do
let(:members_mapper) do let(:members_mapper) do
Gitlab::ImportExport::MembersMapper.new( Gitlab::ImportExport::MembersMapper.new(
exported_members: [exported_member], exported_members: [exported_member],
user: user, user: importer_user,
importable: project) importable: project)
end end
...@@ -134,9 +137,9 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do ...@@ -134,9 +137,9 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do
'source_branch' => "feature_conflict", 'source_branch' => "feature_conflict",
'source_project_id' => project.id, 'source_project_id' => project.id,
'target_project_id' => project.id, 'target_project_id' => project.id,
'author_id' => user.id, 'author_id' => admin.id,
'assignee_id' => user.id, 'assignee_id' => admin.id,
'updated_by_id' => user.id, 'updated_by_id' => admin.id,
'title' => "MR1", 'title' => "MR1",
'created_at' => "2016-06-14T15:02:36.568Z", 'created_at' => "2016-06-14T15:02:36.568Z",
'updated_at' => "2016-06-14T15:02:56.815Z", 'updated_at' => "2016-06-14T15:02:56.815Z",
...@@ -151,11 +154,11 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do ...@@ -151,11 +154,11 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do
end end
it 'has preloaded author' do it 'has preloaded author' do
expect(created_object.author).to equal(user) expect(created_object.author).to equal(admin)
end end
it 'has preloaded updated_by' do it 'has preloaded updated_by' do
expect(created_object.updated_by).to equal(user) expect(created_object.updated_by).to equal(admin)
end end
it 'has preloaded source project' do it 'has preloaded source project' do
...@@ -264,27 +267,8 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do ...@@ -264,27 +267,8 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do
end end
end end
context 'Notes user references' do it_behaves_like 'Notes user references' do
let(:relation_sym) { :notes } let(:importable) { project }
let(:new_user) { create(:user) }
let(:exported_member) do
{
"id" => 111,
"access_level" => 30,
"source_id" => 1,
"source_type" => "Project",
"user_id" => 3,
"notification_level" => 3,
"created_at" => "2016-11-18T09:29:42.634Z",
"updated_at" => "2016-11-18T09:29:42.634Z",
"user" => {
"id" => 999,
"email" => new_user.email,
"username" => new_user.username
}
}
end
let(:relation_hash) do let(:relation_hash) do
{ {
"id" => 4947, "id" => 4947,
...@@ -305,17 +289,6 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do ...@@ -305,17 +289,6 @@ RSpec.describe Gitlab::ImportExport::Project::RelationFactory do
"events" => [] "events" => []
} }
end end
let(:members_mapper) do
Gitlab::ImportExport::MembersMapper.new(
exported_members: [exported_member],
user: user,
importable: project)
end
it 'maps the right author to the imported note' do
expect(created_object.author).to eq(new_user)
end
end end
context 'encrypted attributes' do context 'encrypted attributes' do
......
# frozen_string_literal: true
# required context:
# - importable: group or project
# - relation_hash: a note relation that's being imported
# - created_object: the object created with the relation factory
RSpec.shared_examples 'Notes user references' do
let(:relation_sym) { :notes }
let(:mapped_user) { create(:user) }
let(:exported_member) do
{
'id' => 111,
'access_level' => 30,
'source_id' => 1,
'source_type' => importable.class.name == 'Project' ? 'Project' : 'Namespace',
'user_id' => 3,
'notification_level' => 3,
'created_at' => '2016-11-18T09:29:42.634Z',
'updated_at' => '2016-11-18T09:29:42.634Z',
'user' => {
'id' => 999,
'email' => mapped_user.email,
'username' => mapped_user.username
}
}
end
let(:members_mapper) do
Gitlab::ImportExport::MembersMapper.new(
exported_members: [exported_member].compact,
user: importer_user,
importable: importable
)
end
shared_examples 'sets the note author to the importer user' do
it { expect(created_object.author).to eq(importer_user) }
end
shared_examples 'sets the note author to the mapped user' do
it { expect(created_object.author).to eq(mapped_user) }
end
shared_examples 'does not add original autor note' do
it { expect(created_object.note).not_to include('*By Administrator') }
end
shared_examples 'adds original autor note' do
it { expect(created_object.note).to include('*By Administrator') }
end
context 'when the importer is admin' do
let(:importer_user) { create(:admin) }
context 'and the note author is not mapped' do
let(:exported_member) { nil }
include_examples 'sets the note author to the importer user'
include_examples 'adds original autor note'
end
context 'and the note author is the importer user' do
let(:mapped_user) { importer_user }
include_examples 'sets the note author to the mapped user'
include_examples 'adds original autor note'
end
context 'and the note author exists in the target instance' do
let(:mapped_user) { create(:user) }
include_examples 'sets the note author to the mapped user'
include_examples 'does not add original autor note'
end
end
context 'when the importer is not admin' do
let(:importer_user) { create(:user) }
context 'and the note author is not mapped' do
let(:exported_member) { nil }
include_examples 'sets the note author to the importer user'
include_examples 'adds original autor note'
end
context 'and the note author is the importer user' do
let(:mapped_user) { importer_user }
include_examples 'sets the note author to the importer user'
include_examples 'adds original autor note'
end
context 'and the note author exists in the target instance' do
let(:mapped_user) { create(:user) }
include_examples 'sets the note author to the importer user'
include_examples 'adds original autor note'
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment