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

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 dc1539cf318..0a76570f65e 100644
--- a/spec/validators/sha_validator_spec.rb
+++ b/spec/validators/sha_validator_spec.rb
@@ -4,7 +4,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) }
 
@@ -12,6 +12,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
@@ -21,7 +23,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
@@ -32,6 +36,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