Don't allow merges with new commits
Set a `sha` parameter on the MR form. If this doesn't match the HEAD of the source branch when the form is submitted, show a warning (like with a merge conflict) and don't merge the branch.
This commit is contained in:
parent
d863d86aeb
commit
f680eca912
|
@ -14,6 +14,7 @@ v 8.9.0 (unreleased)
|
||||||
- Redesign account and email confirmation emails
|
- Redesign account and email confirmation emails
|
||||||
- Use gitlab-shell v3.0.0
|
- Use gitlab-shell v3.0.0
|
||||||
- Add `sha` parameter to MR merge API, to ensure only reviewed changes are merged
|
- Add `sha` parameter to MR merge API, to ensure only reviewed changes are merged
|
||||||
|
- Don't allow MRs to be merged when commits were added since the last review / page load
|
||||||
- Add DB index on users.state
|
- Add DB index on users.state
|
||||||
- Add rake task 'gitlab:db:configure' for conditionally seeding or migrating the database
|
- Add rake task 'gitlab:db:configure' for conditionally seeding or migrating the database
|
||||||
- Changed the Slack build message to use the singular duration if necessary (Aran Koning)
|
- Changed the Slack build message to use the singular duration if necessary (Aran Koning)
|
||||||
|
|
|
@ -190,6 +190,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController
|
||||||
return
|
return
|
||||||
end
|
end
|
||||||
|
|
||||||
|
if params[:sha] != @merge_request.source_sha
|
||||||
|
@status = :sha_mismatch
|
||||||
|
return
|
||||||
|
end
|
||||||
|
|
||||||
TodoService.new.merge_merge_request(merge_request, current_user)
|
TodoService.new.merge_merge_request(merge_request, current_user)
|
||||||
|
|
||||||
@merge_request.update(merge_error: nil)
|
@merge_request.update(merge_error: nil)
|
||||||
|
|
|
@ -5,6 +5,9 @@
|
||||||
- when :merge_when_build_succeeds
|
- when :merge_when_build_succeeds
|
||||||
:plain
|
:plain
|
||||||
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/merge_when_build_succeeds'))}");
|
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/merge_when_build_succeeds'))}");
|
||||||
|
- when :sha_mismatch
|
||||||
|
:plain
|
||||||
|
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/sha_mismatch'))}");
|
||||||
- else
|
- else
|
||||||
:plain
|
:plain
|
||||||
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/reload'))}");
|
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/reload'))}");
|
||||||
|
|
|
@ -2,6 +2,7 @@
|
||||||
|
|
||||||
= form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-quick-submit js-requires-input' } do |f|
|
= form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-quick-submit js-requires-input' } do |f|
|
||||||
= hidden_field_tag :authenticity_token, form_authenticity_token
|
= hidden_field_tag :authenticity_token, form_authenticity_token
|
||||||
|
= hidden_field_tag :sha, @merge_request.source_sha
|
||||||
.accept-merge-holder.clearfix.js-toggle-container
|
.accept-merge-holder.clearfix.js-toggle-container
|
||||||
.clearfix
|
.clearfix
|
||||||
.accept-action
|
.accept-action
|
||||||
|
|
|
@ -16,7 +16,7 @@
|
||||||
- if remove_source_branch_button || user_can_cancel_automatic_merge
|
- if remove_source_branch_button || user_can_cancel_automatic_merge
|
||||||
.clearfix.prepend-top-10
|
.clearfix.prepend-top-10
|
||||||
- if remove_source_branch_button
|
- if remove_source_branch_button
|
||||||
= link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
|
= link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true, sha: @merge_request.source_sha), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
|
||||||
= icon('times')
|
= icon('times')
|
||||||
Remove Source Branch When Merged
|
Remove Source Branch When Merged
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
%h4
|
||||||
|
= icon("exclamation-triangle")
|
||||||
|
This merge request has new commits since the page was loaded.
|
||||||
|
|
||||||
|
%p
|
||||||
|
Please review the new commits before merging.
|
|
@ -185,6 +185,92 @@ describe Projects::MergeRequestsController do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'POST #merge' do
|
||||||
|
let(:base_params) do
|
||||||
|
{
|
||||||
|
namespace_id: project.namespace.path,
|
||||||
|
project_id: project.path,
|
||||||
|
id: merge_request.iid,
|
||||||
|
format: 'raw'
|
||||||
|
}
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when the user does not have access' do
|
||||||
|
before do
|
||||||
|
project.team.truncate
|
||||||
|
project.team << [user, :reporter]
|
||||||
|
post :merge, base_params
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns not found' do
|
||||||
|
expect(response).to be_not_found
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when the merge request is not mergeable' do
|
||||||
|
before do
|
||||||
|
merge_request.update_attributes(title: "WIP: #{merge_request.title}")
|
||||||
|
|
||||||
|
post :merge, base_params
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns :failed' do
|
||||||
|
expect(assigns(:status)).to eq(:failed)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when the sha parameter does not match the source SHA' do
|
||||||
|
before { post :merge, base_params.merge(sha: 'foo') }
|
||||||
|
|
||||||
|
it 'returns :sha_mismatch' do
|
||||||
|
expect(assigns(:status)).to eq(:sha_mismatch)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when the sha parameter matches the source SHA' do
|
||||||
|
def merge_with_sha
|
||||||
|
post :merge, base_params.merge(sha: merge_request.source_sha)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns :success' do
|
||||||
|
merge_with_sha
|
||||||
|
|
||||||
|
expect(assigns(:status)).to eq(:success)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'starts the merge immediately' do
|
||||||
|
expect(MergeWorker).to receive(:perform_async).with(merge_request.id, anything, anything)
|
||||||
|
|
||||||
|
merge_with_sha
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when merge_when_build_succeeds is passed' do
|
||||||
|
def merge_when_build_succeeds
|
||||||
|
post :merge, base_params.merge(sha: merge_request.source_sha, merge_when_build_succeeds: '1')
|
||||||
|
end
|
||||||
|
|
||||||
|
before do
|
||||||
|
create(:ci_empty_commit, project: project, sha: merge_request.source_sha, ref: merge_request.source_branch)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns :merge_when_build_succeeds' do
|
||||||
|
merge_when_build_succeeds
|
||||||
|
|
||||||
|
expect(assigns(:status)).to eq(:merge_when_build_succeeds)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'sets the MR to merge when the build succeeds' do
|
||||||
|
service = double(:merge_when_build_succeeds_service)
|
||||||
|
|
||||||
|
expect(MergeRequests::MergeWhenBuildSucceedsService).to receive(:new).with(project, anything, anything).and_return(service)
|
||||||
|
expect(service).to receive(:execute).with(merge_request)
|
||||||
|
|
||||||
|
merge_when_build_succeeds
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe "DELETE #destroy" do
|
describe "DELETE #destroy" do
|
||||||
it "denies access to users unless they're admin or project owner" do
|
it "denies access to users unless they're admin or project owner" do
|
||||||
delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
|
delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
|
||||||
|
|
Loading…
Reference in New Issue