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 516987e4)

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

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

changelogs/unreleased/fj-58804-fix-bitbucket-import.yml

+---
+title: Fix bug in BitBucket imports with SHA shorter than 40 chars
+merge_request: 26050
+author:
+type: fixed

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

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