Commit 8acfe8ea authored by Kassio Borges's avatar Kassio Borges

Github Importer: Import Pull request "merged_at" attribute

Ensure to import "merged_at" attribute for every merged pull request.
If the merger user cannot be mapped to a GitLab user, besides creating a
`MergeRequest::Metrics` with the `merged_at` attribute, also add the
`merged_at` information to the "merged by" note.
parent bc7dd70e
---
title: 'Github Importer: Import Pull request "merged_at" attribute'
merge_request: 54862
author:
type: fixed
...@@ -12,25 +12,27 @@ module Gitlab ...@@ -12,25 +12,27 @@ module Gitlab
def execute def execute
merge_request = project.merge_requests.find_by_iid(pull_request.iid) 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) user_finder = GithubImport::UserFinder.new(project, client)
gitlab_user_id = user_finder.user_id_for(pull_request.merged_by) gitlab_user_id = user_finder.user_id_for(pull_request.merged_by)
if gitlab_user_id MergeRequest::Metrics.upsert({
timestamp = Time.new.utc target_project_id: project.id,
MergeRequest::Metrics.upsert({ merge_request_id: merge_request.id,
target_project_id: project.id, merged_by_id: gitlab_user_id,
merge_request_id: merge_request.id, merged_at: merged_at,
merged_by_id: gitlab_user_id, created_at: timestamp,
created_at: timestamp, updated_at: timestamp
updated_at: timestamp }, unique_by: :merge_request_id)
}, unique_by: :merge_request_id)
else unless gitlab_user_id
merge_request.notes.create!( merge_request.notes.create!(
importing: true, importing: true,
note: "*Merged by: #{pull_request.merged_by.login}*", note: missing_author_note,
author_id: project.creator_id, author_id: project.creator_id,
project: project, project: project,
created_at: pull_request.created_at created_at: merged_at
) )
end end
end end
...@@ -38,6 +40,13 @@ module Gitlab ...@@ -38,6 +40,13 @@ module Gitlab
private private
attr_reader :project, :pull_request, :client 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 end
end end
......
...@@ -13752,6 +13752,9 @@ msgstr "" ...@@ -13752,6 +13752,9 @@ msgstr ""
msgid "GitHub import" msgid "GitHub import"
msgstr "" msgstr ""
msgid "GitHubImporter|*Merged by: %{author} at %{timestamp}*"
msgstr ""
msgid "GitLab" msgid "GitLab"
msgstr "" msgstr ""
......
...@@ -5,37 +5,46 @@ require 'spec_helper' ...@@ -5,37 +5,46 @@ require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Importer::PullRequestMergedByImporter, :clean_gitlab_redis_cache do RSpec.describe Gitlab::GithubImport::Importer::PullRequestMergedByImporter, :clean_gitlab_redis_cache do
let_it_be(:merge_request) { create(:merged_merge_request) } let_it_be(:merge_request) { create(:merged_merge_request) }
let(:project) { merge_request.project } 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(:client_double) { double(user: double(id: 999, login: 'merger', email: 'merger@email.com')) }
let(:pull_request) do let(:pull_request) do
instance_double( instance_double(
Gitlab::GithubImport::Representation::PullRequest, Gitlab::GithubImport::Representation::PullRequest,
iid: merge_request.iid, iid: merge_request.iid,
created_at: created_at, merged_at: merged_at,
merged_by: double(id: 999, login: 'merger') merged_by: double(id: 999, login: 'merger')
) )
end end
subject { described_class.new(pull_request, project, client_double) } subject { described_class.new(pull_request, project, client_double) }
it 'assigns the merged by user when mapped' do context 'when the merger user can be mapped' do
merge_user = create(:user, email: 'merger@email.com') 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 end
it 'adds a note referencing the merger user when the user cannot be mapped' do context 'when the merger user cannot be mapped to a gitlab user' do
expect { subject.execute } it 'adds a note referencing the merger user' do
.to change(Note, :count).by(1) expect { subject.execute }
.and not_change(merge_request, :updated_at) .to change(Note, :count).by(1)
.and not_change(merge_request, :updated_at)
last_note = merge_request.notes.last
metrics = merge_request.metrics.reload
expect(last_note.note).to eq("*Merged by: merger*") expect(metrics.merged_by).to be_nil
expect(last_note.created_at).to eq(created_at) expect(metrics.merged_at).to eq(merged_at)
expect(last_note.author).to eq(project.creator)
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
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