Commit 3f552e94 authored by Vasilii Iakliushin's avatar Vasilii Iakliushin

Persist squash_commit_sha when squashing

Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/21686
Feature flag - https://gitlab.com/gitlab-org/gitlab/-/issues/294243

**Problem**

We already populate `squash_commit_sha` field for fast forward merges,
however for the regular merge with squashing we keep this field
empty.

**Solution**

Persist squash commit sha in database after merging with squash
option enabled.
parent 4ec31104
......@@ -88,7 +88,13 @@ module MergeRequests
end
def try_merge
repository.merge(current_user, source, merge_request, commit_message)
merge = repository.merge(current_user, source, merge_request, commit_message)
if merge_request.squash_on_merge? && Feature.enabled?(:persist_squash_commit_sha_for_squashes, project)
merge_request.update_column(:squash_commit_sha, source)
end
merge
rescue Gitlab::Git::PreReceiveError => e
raise MergeError,
"Something went wrong during merge pre-receive hook. #{e.message}".strip
......
---
name: persist_squash_commit_sha_for_squashes
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50178
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/294243
milestone: '13.8'
type: development
group: group::source code
default_enabled: false
......@@ -19,8 +19,12 @@ RSpec.describe MergeRequests::MergeService do
{ commit_message: 'Awesome message', sha: merge_request.diff_head_sha }
end
let(:feature_flag_persist_squash) { true }
context 'valid params' do
before do
stub_feature_flags(persist_squash_commit_sha_for_squashes: feature_flag_persist_squash)
allow(service).to receive(:execute_hooks)
expect(merge_request).to receive(:update_and_mark_in_progress_merge_commit_sha).twice.and_call_original
......@@ -37,6 +41,10 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request.in_progress_merge_commit_sha).to be_nil
end
it 'does not update squash_commit_sha if it is not a squash' do
expect(merge_request.squash_commit_sha).to be_nil
end
it 'sends email to user2 about merge of new merge_request' do
email = ActionMailer::Base.deliveries.last
expect(email.to.first).to eq(user2.email)
......@@ -76,6 +84,20 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_commit.message).to eq('Merge commit message')
expect(squash_commit.message).to eq("Squash commit message\n")
end
it 'persists squash_commit_sha' do
squash_commit = merge_request.merge_commit.parents.last
expect(merge_request.squash_commit_sha).to eq(squash_commit.id)
end
context 'when feature flag is disabled' do
let(:feature_flag_persist_squash) { false }
it 'does not populate squash_commit_sha' do
expect(merge_request.squash_commit_sha).to be_nil
end
end
end
end
......@@ -361,6 +383,7 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil
expect(merge_request.squash_commit_sha).to be_nil
expect(merge_request.merge_error).to include(error_message)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end
......@@ -381,6 +404,7 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil
expect(merge_request.squash_commit_sha).to be_nil
expect(merge_request.merge_error).to include(error_message)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end
......@@ -395,10 +419,27 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil
expect(merge_request.squash_commit_sha).to be_nil
expect(merge_request.merge_error).to include(error_message)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end
it 'logs and saves error if there is an PreReceiveError exception' do
error_message = 'error message'
allow(service).to receive(:repository).and_raise(Gitlab::Git::PreReceiveError, "GitLab: #{error_message}")
allow(service).to receive(:execute_hooks)
merge_request.update!(squash: true)
service.execute(merge_request)
expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil
expect(merge_request.squash_commit_sha).to be_nil
expect(merge_request.merge_error).to include('Something went wrong during merge pre-receive hook')
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end
context 'when fast-forward merge is not allowed' do
before do
allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil)
......@@ -415,6 +456,7 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil
expect(merge_request.squash_commit_sha).to be_nil
expect(merge_request.merge_error).to include(error_message)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
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