Merge branch 'georgekoltsov/log-added-team-members-on-import' into 'master'

Log member additions when importing Project/Group

See merge request gitlab-org/gitlab!27930

changelogs/unreleased/georgekoltsov-log-added-team-members-on-import.yml

+---
+title: Log member additions when importing Project/Group
+merge_request: 27930
+author:
+type: other

lib/gitlab/import_export/members_mapper.rb

@@ -64,8 +64,19 @@ module Gitlab
 
       def add_team_member(member, existing_user = nil)
         member['user'] = existing_user
+        member_hash = member_hash(member)
 
-        relation_class.create(member_hash(member)).persisted?
+        member = relation_class.create(member_hash)
+
+        if member.persisted?
+          log_member_addition(member_hash)
+
+          true
+        else
+          log_member_addition_failure(member_hash, member.errors.full_messages)
+
+          false
+        end
       end
 
       def member_hash(member)
@@ -103,6 +114,34 @@ module Gitlab
 
         relation_class::MAINTAINER
       end
+
+      def log_member_addition(member_hash)
+        log_params = base_log_params(member_hash)
+        log_params[:message] = '[Project/Group Import] Added new member'
+
+        logger.info(log_params)
+      end
+
+      def log_member_addition_failure(member_hash, errors)
+        log_params = base_log_params(member_hash)
+        log_params[:message] = "[Project/Group Import] Member addition failed: #{errors&.join(', ')}"
+
+        logger.info(log_params)
+      end
+
+      def base_log_params(member_hash)
+        {
+          user_id: member_hash['user']&.id,
+          access_level: member_hash['access_level'],
+          importable_type: @importable.class.to_s,
+          importable_id: @importable.id,
+          root_namespace_id: @importable.try(:root_ancestor)&.id
+        }
+      end
+
+      def logger
+        @logger ||= Gitlab::Import::Logger.build
+      end
     end
   end
 end

spec/lib/gitlab/import_export/members_mapper_spec.rb

@@ -78,6 +78,66 @@ describe Gitlab::ImportExport::MembersMapper do
         members_mapper.map
       end
 
+      context 'logging' do
+        let(:logger) { Gitlab::Import::Logger.build }
+
+        before do
+          allow(logger).to receive(:info)
+          allow(members_mapper).to receive(:logger).and_return(logger)
+        end
+
+        it 'logs member addition' do
+          expected_log_params = ->(user_id) do
+            {
+              user_id: user_id,
+              root_namespace_id: importable.root_ancestor.id,
+              importable_type: importable.class.to_s,
+              importable_id: importable.id,
+              access_level: exported_members.first['access_level'],
+              message: '[Project/Group Import] Added new member'
+            }
+          end
+
+          expect(logger).to receive(:info).with(hash_including(expected_log_params.call(user2.id))).once
+          expect(logger).to receive(:info).with(hash_including(expected_log_params.call(nil))).once
+
+          members_mapper.map
+        end
+
+        context 'when exporter member is invalid' do
+          let(:exported_members) do
+            [
+              {
+                "id" => 2,
+                "access_level" => -5, # invalid access level
+                "source_id" => 14,
+                "source_type" => source_type,
+                "notification_level" => 3,
+                "created_at" => "2016-03-11T10:21:44.822Z",
+                "updated_at" => "2016-03-11T10:21:44.822Z",
+                "created_by_id" => nil,
+                "invite_email" => nil,
+                "invite_token" => nil,
+                "invite_accepted_at" => nil,
+                "user" =>
+                  {
+                    "id" => exported_user_id,
+                    "email" => user2.email,
+                    "username" => 'test'
+                  },
+                "user_id" => 19
+              }
+            ]
+          end
+
+          it 'logs member addition failure' do
+            expect(logger).to receive(:info).with(hash_including(message: a_string_including('Access level is not included in the list'))).once
+
+            members_mapper.map
+          end
+        end
+      end
+
       context 'user is not an admin' do
         let(:user) { create(:user) }