diff options
author | Douwe Maan <douwe@gitlab.com> | 2019-03-14 10:05:18 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-04-23 10:10:38 +0000 |
commit | be180a44801d8e6755fa07d259b8900228f5c0c1 (patch) | |
tree | 00bbcec7d3cdfd6b414385f23bb8677f465bc016 | |
parent | c929dd570c2b32104d5f85686106fe199bbd0b07 (diff) | |
download | gitlab-ce-be180a44801d8e6755fa07d259b8900228f5c0c1.tar.gz |
Merge branch 'fj-58804-fix-bitbucket-import' into 'master'
Fix Bitbucket import
Closes #58804
See merge request gitlab-org/gitlab-ce!26050
(cherry picked from commit 516987e4ba40bffa68a1060efd901af2f1e6a3c3)
e4663942 Fix Bitbucket import
a89851e7 Added changelog to MR
b5b9925e Removing SHA length validation
4268fc87 Added SHA length validation
2e2e3a8c Small replacement in spec to use a constant
-rw-r--r-- | app/validators/sha_validator.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/fj-58804-fix-bitbucket-import.yml | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/bitbucket_import/importer_spec.rb | 20 | ||||
-rw-r--r-- | spec/validators/sha_validator_spec.rb | 9 |
4 files changed, 31 insertions, 5 deletions
diff --git a/app/validators/sha_validator.rb b/app/validators/sha_validator.rb index 085fca4d65d..77e7cfa4f6b 100644 --- a/app/validators/sha_validator.rb +++ b/app/validators/sha_validator.rb @@ -2,7 +2,7 @@ class ShaValidator < ActiveModel::EachValidator 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') end diff --git a/changelogs/unreleased/fj-58804-fix-bitbucket-import.yml b/changelogs/unreleased/fj-58804-fix-bitbucket-import.yml new file mode 100644 index 00000000000..dc44c64a055 --- /dev/null +++ b/changelogs/unreleased/fj-58804-fix-bitbucket-import.yml @@ -0,0 +1,5 @@ +--- +title: Fix bug in BitBucket imports with SHA shorter than 40 chars +merge_request: 26050 +author: +type: fixed diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index c432cc223b9..e1a2bae5fe8 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -95,6 +95,9 @@ describe Gitlab::BitbucketImport::Importer do subject { described_class.new(project) } describe '#import_pull_requests' do + let(:source_branch_sha) { sample.commits.last } + let(:target_branch_sha) { sample.commits.first } + before do allow(subject).to receive(:import_wiki) allow(subject).to receive(:import_issues) @@ -102,9 +105,9 @@ describe Gitlab::BitbucketImport::Importer do pull_request = instance_double( Bitbucket::Representation::PullRequest, 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, - target_branch_sha: sample.commits.first, + target_branch_sha: target_branch_sha, target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, title: 'This is a title', description: 'This is a test pull request', @@ -162,6 +165,19 @@ describe Gitlab::BitbucketImport::Importer do expect(reply_note).to be_a(DiffNote) expect(reply_note.note).to eq(@reply.note) 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 context 'issues statuses' do diff --git a/spec/validators/sha_validator_spec.rb b/spec/validators/sha_validator_spec.rb index b9242ef931e..fa3dd68df2d 100644 --- a/spec/validators/sha_validator_spec.rb +++ b/spec/validators/sha_validator_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe ShaValidator do 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) } @@ -10,6 +10,8 @@ describe ShaValidator do let(:value) { nil } it 'does not add any error if value is empty' do + expect(Commit).not_to receive(:valid_hash?) + subject expect(merge_diff.errors).to be_empty @@ -19,7 +21,9 @@ describe ShaValidator do context 'with valid sha' do 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 expect(merge_diff.errors).to be_empty @@ -30,6 +34,7 @@ describe ShaValidator do let(:value) { 'foo' } it 'adds error to the record' do + expect(Commit).to receive(:valid_hash?).and_call_original expect(merge_diff.errors).to be_empty subject |