Merge branch 'kassio/githubimporter-import-merged-at' into 'master'

Github Importer: Import Pull request "merged_at" attribute

See merge request gitlab-org/gitlab!54862

changelogs/unreleased/kassio-githubimporter-import-merged-at.yml

+---
+title: 'Github Importer: Import Pull request "merged_at" attribute'
+merge_request: 54862
+author:
+type: fixed

lib/gitlab/github_import/importer/pull_request_merged_by_importer.rb

@@ -12,25 +12,27 @@ module Gitlab
 
         def execute
           merge_request = project.merge_requests.find_by_iid(pull_request.iid)
+          timestamp = Time.new.utc
+          merged_at = pull_request.merged_at
           user_finder = GithubImport::UserFinder.new(project, client)
           gitlab_user_id = user_finder.user_id_for(pull_request.merged_by)
 
-          if gitlab_user_id
-            timestamp = Time.new.utc
-            MergeRequest::Metrics.upsert({
-              target_project_id: project.id,
-              merge_request_id: merge_request.id,
-              merged_by_id: gitlab_user_id,
-              created_at: timestamp,
-              updated_at: timestamp
-            }, unique_by: :merge_request_id)
-          else
+          MergeRequest::Metrics.upsert({
+            target_project_id: project.id,
+            merge_request_id: merge_request.id,
+            merged_by_id: gitlab_user_id,
+            merged_at: merged_at,
+            created_at: timestamp,
+            updated_at: timestamp
+          }, unique_by: :merge_request_id)
+
+          unless gitlab_user_id
             merge_request.notes.create!(
               importing: true,
-              note: "*Merged by: #{pull_request.merged_by.login}*",
+              note: missing_author_note,
               author_id: project.creator_id,
               project: project,
-              created_at: pull_request.created_at
+              created_at: merged_at
             )
           end
         end
@@ -38,6 +40,13 @@ module Gitlab
         private
 
         attr_reader :project, :pull_request, :client
+
+        def missing_author_note
+          s_("GitHubImporter|*Merged by: %{author} at %{timestamp}*") % {
+            author: pull_request.merged_by.login,
+            timestamp: pull_request.merged_at
+          }
+        end
       end
     end
   end

locale/gitlab.pot

@@ -13746,6 +13746,9 @@ msgstr ""
 msgid "GitHub import"
 msgstr ""
 
+msgid "GitHubImporter|*Merged by: %{author} at %{timestamp}*"
+msgstr ""
+
 msgid "GitLab"
 msgstr ""
 

spec/lib/gitlab/github_import/importer/pull_request_merged_by_importer_spec.rb

@@ -5,37 +5,46 @@ require 'spec_helper'
 RSpec.describe Gitlab::GithubImport::Importer::PullRequestMergedByImporter, :clean_gitlab_redis_cache do
   let_it_be(:merge_request) { create(:merged_merge_request) }
   let(:project) { merge_request.project }
-  let(:created_at) { Time.new(2017, 1, 1, 12, 00).utc }
+  let(:merged_at) { Time.new(2017, 1, 1, 12, 00).utc }
   let(:client_double) { double(user: double(id: 999, login: 'merger', email: 'merger@email.com')) }
 
   let(:pull_request) do
     instance_double(
       Gitlab::GithubImport::Representation::PullRequest,
       iid: merge_request.iid,
-      created_at: created_at,
+      merged_at: merged_at,
       merged_by: double(id: 999, login: 'merger')
     )
   end
 
   subject { described_class.new(pull_request, project, client_double) }
 
-  it 'assigns the merged by user when mapped' do
-    merge_user = create(:user, email: 'merger@email.com')
+  context 'when the merger user can be mapped' do
+    it 'assigns the merged by user when mapped' do
+      merge_user = create(:user, email: 'merger@email.com')
 
-    subject.execute
+      subject.execute
 
-    expect(merge_request.metrics.reload.merged_by).to eq(merge_user)
+      metrics = merge_request.metrics.reload
+      expect(metrics.merged_by).to eq(merge_user)
+      expect(metrics.merged_at).to eq(merged_at)
+    end
   end
 
-  it 'adds a note referencing the merger user when the user cannot be mapped' do
-    expect { subject.execute }
-      .to change(Note, :count).by(1)
-      .and not_change(merge_request, :updated_at)
-
-    last_note = merge_request.notes.last
-
-    expect(last_note.note).to eq("*Merged by: merger*")
-    expect(last_note.created_at).to eq(created_at)
-    expect(last_note.author).to eq(project.creator)
+  context 'when the merger user cannot be mapped to a gitlab user' do
+    it 'adds a note referencing the merger user' do
+      expect { subject.execute }
+        .to change(Note, :count).by(1)
+        .and not_change(merge_request, :updated_at)
+
+      metrics = merge_request.metrics.reload
+      expect(metrics.merged_by).to be_nil
+      expect(metrics.merged_at).to eq(merged_at)
+
+      last_note = merge_request.notes.last
+      expect(last_note.note).to eq("*Merged by: merger at 2017-01-01 12:00:00 UTC*")
+      expect(last_note.created_at).to eq(merged_at)
+      expect(last_note.author).to eq(project.creator)
+    end
   end
 end