diff --git a/app/services/merge_requests/ff_merge_service.rb b/app/services/merge_requests/ff_merge_service.rb index c56400478997c1529dcab5280af94d0a4130b8b5..6e1d1b6ad232623550006221949cd1d57067feea 100644 --- a/app/services/merge_requests/ff_merge_service.rb +++ b/app/services/merge_requests/ff_merge_service.rb @@ -8,26 +8,22 @@ module MergeRequests # Executed when you do fast-forward merge via GitLab UI # class FfMergeService < MergeRequests::MergeService - private + extend ::Gitlab::Utils::Override - def commit - ff_merge = repository.ff_merge(current_user, - source, - merge_request.target_branch, - merge_request: merge_request) + private - if merge_request.squash_on_merge? - merge_request.update_column(:squash_commit_sha, merge_request.in_progress_merge_commit_sha) - end + override :execute_git_merge + def execute_git_merge + repository.ff_merge(current_user, + source, + merge_request.target_branch, + merge_request: merge_request) + end - ff_merge - rescue Gitlab::Git::PreReceiveError => e - Gitlab::ErrorTracking.track_exception(e, pre_receive_message: e.raw_message, merge_request_id: merge_request&.id) - raise MergeError, e.message - rescue StandardError => e - raise MergeError, "Something went wrong during merge: #{e.message}" - ensure - merge_request.update_and_mark_in_progress_merge_commit_sha(nil) + override :merge_success_data + def merge_success_data(commit_id) + # There is no merge commit to update, so this is just blank. + {} end end end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 5244f2acd66eae5fcb241c99a309dc5ec9d9efd0..6d31a29f5a78067ec5a09fc5c2bf418a8bb03907 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -92,16 +92,26 @@ def commit raise_error(GENERIC_ERROR_MESSAGE) end - data_to_update = { merge_commit_sha: commit_id } - data_to_update[:squash_commit_sha] = source if merge_request.squash_on_merge? + update_merge_sha_metadata(commit_id) - merge_request.update!(**data_to_update) + commit_id ensure merge_request.update_and_mark_in_progress_merge_commit_sha(nil) end + def update_merge_sha_metadata(commit_id) + data_to_update = merge_success_data(commit_id) + data_to_update[:squash_commit_sha] = source if merge_request.squash_on_merge? + + merge_request.update!(**data_to_update) if data_to_update.present? + end + + def merge_success_data(commit_id) + { merge_commit_sha: commit_id } + end + def try_merge - repository.merge(current_user, source, merge_request, commit_message) + execute_git_merge rescue Gitlab::Git::PreReceiveError => e raise MergeError, "Something went wrong during merge pre-receive hook. #{e.message}".strip @@ -110,6 +120,10 @@ def try_merge raise_error(GENERIC_ERROR_MESSAGE) end + def execute_git_merge + repository.merge(current_user, source, merge_request, commit_message) + end + def after_merge log_info("Post merge started on JID #{merge_jid} with state #{state}") MergeRequests::PostMergeService.new(project: project, current_user: current_user).execute(merge_request) diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb index 24a1a8b31139a9cdadb63c455dc4291e99db53c7..aa5d6dcd1fb41d4a385aa5d1221e02ceb82a759d 100644 --- a/spec/services/merge_requests/ff_merge_service_spec.rb +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -75,6 +75,7 @@ def execute_ff_merge expect(merge_request).to receive(:update_and_mark_in_progress_merge_commit_sha).twice.and_call_original expect { execute_ff_merge }.not_to change { merge_request.squash_commit_sha } + expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.in_progress_merge_commit_sha).to be_nil end @@ -87,6 +88,7 @@ def execute_ff_merge .to change { merge_request.squash_commit_sha } .from(nil) + expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.in_progress_merge_commit_sha).to be_nil end end @@ -106,7 +108,6 @@ def execute_ff_merge service.execute(merge_request) - expect(merge_request.merge_error).to include(error_message) expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end @@ -117,11 +118,6 @@ def execute_ff_merge pre_receive_error = Gitlab::Git::PreReceiveError.new(raw_message, fallback_message: error_message) allow(service).to receive(:repository).and_raise(pre_receive_error) allow(service).to receive(:execute_hooks) - expect(Gitlab::ErrorTracking).to receive(:track_exception).with( - pre_receive_error, - pre_receive_message: raw_message, - merge_request_id: merge_request.id - ) service.execute(merge_request)