diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index ba22b4587771fb3c83a4b1f07817f48ebe2be142..407f2ad70f8668f6811dded1ba097409e5dbb591 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -88,7 +88,13 @@ def commit 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 diff --git a/config/feature_flags/development/persist_squash_commit_sha_for_squashes.yml b/config/feature_flags/development/persist_squash_commit_sha_for_squashes.yml new file mode 100644 index 0000000000000000000000000000000000000000..835437278c4987dfb01984237966795099eb368f --- /dev/null +++ b/config/feature_flags/development/persist_squash_commit_sha_for_squashes.yml @@ -0,0 +1,8 @@ +--- +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 diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index d0e3102f1573a1cb832a995648b1d984f795acb7..4e1d8b3a9a96caae95dcc900af90c025e83ef35a 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -19,8 +19,12 @@ { 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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