Commit 516987e4 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'fj-58804-fix-bitbucket-import' into 'master'

Fix Bitbucket import

Closes #58804

See merge request gitlab-org/gitlab-ce!26050
parents d0053d51 150f7c1e
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class ShaValidator < ActiveModel::EachValidator class ShaValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value) def validate_each(record, attribute, value)
return if value.blank? || value.match(/\A\h{40}\z/) return if value.blank? || Commit.valid_hash?(value)
record.errors.add(attribute, 'is not a valid SHA') record.errors.add(attribute, 'is not a valid SHA')
end end
......
---
title: Fix bug in BitBucket imports with SHA shorter than 40 chars
merge_request: 26050
author:
type: fixed
...@@ -95,6 +95,9 @@ describe Gitlab::BitbucketImport::Importer do ...@@ -95,6 +95,9 @@ describe Gitlab::BitbucketImport::Importer do
subject { described_class.new(project) } subject { described_class.new(project) }
describe '#import_pull_requests' do describe '#import_pull_requests' do
let(:source_branch_sha) { sample.commits.last }
let(:target_branch_sha) { sample.commits.first }
before do before do
allow(subject).to receive(:import_wiki) allow(subject).to receive(:import_wiki)
allow(subject).to receive(:import_issues) allow(subject).to receive(:import_issues)
...@@ -102,9 +105,9 @@ describe Gitlab::BitbucketImport::Importer do ...@@ -102,9 +105,9 @@ describe Gitlab::BitbucketImport::Importer do
pull_request = instance_double( pull_request = instance_double(
Bitbucket::Representation::PullRequest, Bitbucket::Representation::PullRequest,
iid: 10, iid: 10,
source_branch_sha: sample.commits.last, source_branch_sha: source_branch_sha,
source_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.source_branch, source_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.source_branch,
target_branch_sha: sample.commits.first, target_branch_sha: target_branch_sha,
target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch,
title: 'This is a title', title: 'This is a title',
description: 'This is a test pull request', description: 'This is a test pull request',
...@@ -162,6 +165,19 @@ describe Gitlab::BitbucketImport::Importer do ...@@ -162,6 +165,19 @@ describe Gitlab::BitbucketImport::Importer do
expect(reply_note).to be_a(DiffNote) expect(reply_note).to be_a(DiffNote)
expect(reply_note.note).to eq(@reply.note) expect(reply_note.note).to eq(@reply.note)
end end
context "when branches' sha is not found in the repository" do
let(:source_branch_sha) { 'a' * Commit::MIN_SHA_LENGTH }
let(:target_branch_sha) { 'b' * Commit::MIN_SHA_LENGTH }
it 'uses the pull request sha references' do
expect { subject.execute }.to change { MergeRequest.count }.by(1)
merge_request_diff = MergeRequest.first.merge_request_diff
expect(merge_request_diff.head_commit_sha).to eq source_branch_sha
expect(merge_request_diff.start_commit_sha).to eq target_branch_sha
end
end
end end
context 'issues statuses' do context 'issues statuses' do
......
...@@ -4,7 +4,7 @@ require 'spec_helper' ...@@ -4,7 +4,7 @@ require 'spec_helper'
describe ShaValidator do describe ShaValidator do
let(:validator) { described_class.new(attributes: [:base_commit_sha]) } let(:validator) { described_class.new(attributes: [:base_commit_sha]) }
let(:merge_diff) { build(:merge_request_diff) } let!(:merge_diff) { build(:merge_request_diff) }
subject { validator.validate_each(merge_diff, :base_commit_sha, value) } subject { validator.validate_each(merge_diff, :base_commit_sha, value) }
...@@ -12,6 +12,8 @@ describe ShaValidator do ...@@ -12,6 +12,8 @@ describe ShaValidator do
let(:value) { nil } let(:value) { nil }
it 'does not add any error if value is empty' do it 'does not add any error if value is empty' do
expect(Commit).not_to receive(:valid_hash?)
subject subject
expect(merge_diff.errors).to be_empty expect(merge_diff.errors).to be_empty
...@@ -21,7 +23,9 @@ describe ShaValidator do ...@@ -21,7 +23,9 @@ describe ShaValidator do
context 'with valid sha' do context 'with valid sha' do
let(:value) { Digest::SHA1.hexdigest(SecureRandom.hex) } let(:value) { Digest::SHA1.hexdigest(SecureRandom.hex) }
it 'does not add any error if value is empty' do it 'does not add any error' do
expect(Commit).to receive(:valid_hash?).and_call_original
subject subject
expect(merge_diff.errors).to be_empty expect(merge_diff.errors).to be_empty
...@@ -32,6 +36,7 @@ describe ShaValidator do ...@@ -32,6 +36,7 @@ describe ShaValidator do
let(:value) { 'foo' } let(:value) { 'foo' }
it 'adds error to the record' do it 'adds error to the record' do
expect(Commit).to receive(:valid_hash?).and_call_original
expect(merge_diff.errors).to be_empty expect(merge_diff.errors).to be_empty
subject subject
......
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