Commit 4f9d82a9 authored by Douwe Maan's avatar Douwe Maan Committed by Yorick Peterse

Merge branch 'gh-review-comments' into 'master'

Fix the line code when importing PR review comments from GitHub

Pull Request Review Comments are comments on a portion of the unified diff.

Closes #17205

See merge request !4010
parent f3e41f00
...@@ -26,6 +26,7 @@ v 8.8.0 (unreleased) ...@@ -26,6 +26,7 @@ v 8.8.0 (unreleased)
v 8.7.3 v 8.7.3
- Emails, Gitlab::Email::Message, Gitlab::Diff, and Premailer::Adapter::Nokogiri are now instrumented - Emails, Gitlab::Email::Message, Gitlab::Diff, and Premailer::Adapter::Nokogiri are now instrumented
- Merge request widget displays TeamCity build state and code coverage correctly again. - Merge request widget displays TeamCity build state and code coverage correctly again.
- Fix the line code when importing PR review comments from GitHub. !4010
v 8.7.2 v 8.7.2
- The "New Branch" button is now loaded asynchronously - The "New Branch" button is now loaded asynchronously
......
...@@ -28,13 +28,26 @@ module Gitlab ...@@ -28,13 +28,26 @@ module Gitlab
end end
def line_code def line_code
if on_diff? return unless on_diff?
Gitlab::Diff::LineCode.generate(raw_data.path, raw_data.position, 0)
end parsed_lines = Gitlab::Diff::Parser.new.parse(diff_hunk.lines)
generate_line_code(parsed_lines.to_a.last)
end
def generate_line_code(line)
Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
end end
def on_diff? def on_diff?
raw_data.path && raw_data.position diff_hunk.present?
end
def diff_hunk
raw_data.diff_hunk
end
def file_path
raw_data.path
end end
def note def note
......
...@@ -2,23 +2,25 @@ require 'spec_helper' ...@@ -2,23 +2,25 @@ require 'spec_helper'
describe Gitlab::GithubImport::CommentFormatter, lib: true do describe Gitlab::GithubImport::CommentFormatter, lib: true do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:octocat) { OpenStruct.new(id: 123456, login: 'octocat') } let(:octocat) { double(id: 123456, login: 'octocat') }
let(:created_at) { DateTime.strptime('2013-04-10T20:09:31Z') } let(:created_at) { DateTime.strptime('2013-04-10T20:09:31Z') }
let(:updated_at) { DateTime.strptime('2014-03-03T18:58:10Z') } let(:updated_at) { DateTime.strptime('2014-03-03T18:58:10Z') }
let(:base_data) do let(:base) do
{ {
body: "I'm having a problem with this.", body: "I'm having a problem with this.",
user: octocat, user: octocat,
commit_id: nil,
diff_hunk: nil,
created_at: created_at, created_at: created_at,
updated_at: updated_at updated_at: updated_at
} }
end end
subject(:comment) { described_class.new(project, raw_data)} subject(:comment) { described_class.new(project, raw)}
describe '#attributes' do describe '#attributes' do
context 'when do not reference a portion of the diff' do context 'when do not reference a portion of the diff' do
let(:raw_data) { OpenStruct.new(base_data) } let(:raw) { double(base) }
it 'returns formatted attributes' do it 'returns formatted attributes' do
expected = { expected = {
...@@ -36,24 +38,23 @@ describe Gitlab::GithubImport::CommentFormatter, lib: true do ...@@ -36,24 +38,23 @@ describe Gitlab::GithubImport::CommentFormatter, lib: true do
end end
context 'when on a portion of the diff' do context 'when on a portion of the diff' do
let(:diff_data) do let(:diff) do
{ {
body: 'Great stuff', body: 'Great stuff',
commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e', commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
diff_hunk: '@@ -16,33 +16,40 @@ public class Connection : IConnection...', diff_hunk: "@@ -1,5 +1,9 @@\n class User\n def name\n- 'John Doe'\n+ 'Jane Doe'",
path: 'file1.txt', path: 'file1.txt'
position: 1
} }
end end
let(:raw_data) { OpenStruct.new(base_data.merge(diff_data)) } let(:raw) { double(base.merge(diff)) }
it 'returns formatted attributes' do it 'returns formatted attributes' do
expected = { expected = {
project: project, project: project,
note: "*Created by: octocat*\n\nGreat stuff", note: "*Created by: octocat*\n\nGreat stuff",
commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e', commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
line_code: 'ce1be0ff4065a6e9415095c95f25f47a633cef2b_0_1', line_code: 'ce1be0ff4065a6e9415095c95f25f47a633cef2b_4_3',
author_id: project.creator_id, author_id: project.creator_id,
created_at: created_at, created_at: created_at,
updated_at: updated_at updated_at: updated_at
...@@ -64,15 +65,10 @@ describe Gitlab::GithubImport::CommentFormatter, lib: true do ...@@ -64,15 +65,10 @@ describe Gitlab::GithubImport::CommentFormatter, lib: true do
end end
context 'when author is a GitLab user' do context 'when author is a GitLab user' do
let(:raw_data) { OpenStruct.new(base_data.merge(user: octocat)) } let(:raw) { double(base.merge(user: octocat)) }
it 'returns project#creator_id as author_id when is not a GitLab user' do it 'returns GitLab user id as author_id' do
expect(comment.attributes.fetch(:author_id)).to eq project.creator_id
end
it 'returns GitLab user id as author_id when is a GitLab user' do
gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github')
expect(comment.attributes.fetch(:author_id)).to eq gl_user.id expect(comment.attributes.fetch(:author_id)).to eq gl_user.id
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