Commit 56a48bce authored by Marc Shaw's avatar Marc Shaw

Check (without locking) if the request merge SHA matches the MR SHA

This check is done in the merge service with locking - but we now
preemptively return a more approriate error.

Issue: gitlab.com/gitlab-org/gitlab/-/issues/355567
MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/82775
parent d14b5c3f
...@@ -25,6 +25,8 @@ module Gitlab ...@@ -25,6 +25,8 @@ module Gitlab
execution_message do execution_message do
if params[:merge_request_diff_head_sha].blank? if params[:merge_request_diff_head_sha].blank?
_("Merge request diff sha parameter is required for the merge quick action.") _("Merge request diff sha parameter is required for the merge quick action.")
elsif params[:merge_request_diff_head_sha] != quick_action_target.diff_head_sha
_("Branch has been updated since the merge was requested.")
elsif preferred_strategy = preferred_auto_merge_strategy(quick_action_target) elsif preferred_strategy = preferred_auto_merge_strategy(quick_action_target)
_("Scheduled to merge this merge request (%{strategy}).") % { strategy: preferred_strategy.humanize } _("Scheduled to merge this merge request (%{strategy}).") % { strategy: preferred_strategy.humanize }
else else
...@@ -39,6 +41,8 @@ module Gitlab ...@@ -39,6 +41,8 @@ module Gitlab
command :merge do command :merge do
next unless params[:merge_request_diff_head_sha].present? next unless params[:merge_request_diff_head_sha].present?
next unless params[:merge_request_diff_head_sha] == quick_action_target.diff_head_sha
@updates[:merge] = params[:merge_request_diff_head_sha] @updates[:merge] = params[:merge_request_diff_head_sha]
end end
......
...@@ -5996,6 +5996,9 @@ msgstr "" ...@@ -5996,6 +5996,9 @@ msgstr ""
msgid "Branch changed" msgid "Branch changed"
msgstr "" msgstr ""
msgid "Branch has been updated since the merge was requested."
msgstr ""
msgid "Branch is already taken" msgid "Branch is already taken"
msgstr "" msgstr ""
......
...@@ -759,6 +759,7 @@ RSpec.describe QuickActions::InterpretService do ...@@ -759,6 +759,7 @@ RSpec.describe QuickActions::InterpretService do
context 'merge command' do context 'merge command' do
let(:service) { described_class.new(project, developer, { merge_request_diff_head_sha: merge_request.diff_head_sha }) } let(:service) { described_class.new(project, developer, { merge_request_diff_head_sha: merge_request.diff_head_sha }) }
let(:merge_request) { create(:merge_request, source_project: repository_project) }
it_behaves_like 'merge immediately command' do it_behaves_like 'merge immediately command' do
let(:content) { '/merge' } let(:content) { '/merge' }
...@@ -789,7 +790,7 @@ RSpec.describe QuickActions::InterpretService do ...@@ -789,7 +790,7 @@ RSpec.describe QuickActions::InterpretService do
context 'can not be merged when sha does not match' do context 'can not be merged when sha does not match' do
let(:service) { described_class.new(project, developer, { merge_request_diff_head_sha: 'othersha' }) } let(:service) { described_class.new(project, developer, { merge_request_diff_head_sha: 'othersha' }) }
it_behaves_like 'failed command', 'Could not apply merge command.' do it_behaves_like 'failed command', 'Branch has been updated since the merge was requested.' do
let(:content) { "/merge" } let(:content) { "/merge" }
let(:issuable) { merge_request } let(:issuable) { merge_request }
end 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