Commit 2396e4cc authored by Sean McGivern's avatar Sean McGivern

Fix MR creation on fork of deleted project

The `selected_target_project` helper method is used for actions where
`target_project_id` is a top-level parameter, not nested under the
`merge_request` hash.

Even though that logic was wrong, if the target project wasn't a fork,
or was a fork of an active project, this would work through
coincidence (although it might check the wrong value for
`approvals_before_merge`).

If a project was a fork of a deleted project, however, then
`selected_target_project` would be `nil`, causing a 500 error. Ensure
that there is always a target project to compare to.
parent b6ddf37d
......@@ -4,6 +4,7 @@ v 8.10.0 (unreleased)
v 8.9.3 (unreleased)
- Fix encrypted data backwards compatibility after upgrading attr_encrypted gem
- Fix creating MRs on forks of deleted projects
v 8.9.1
- Improve Geo documentation. !431
......
......@@ -410,7 +410,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end
def clamp_approvals_before_merge(mr_params)
if mr_params[:approvals_before_merge].to_i <= selected_target_project.approvals_before_merge
target_project = @project.forked_from_project if @project.id.to_s != mr_params[:target_project_id]
target_project ||= @project
if mr_params[:approvals_before_merge].to_i <= target_project.approvals_before_merge
mr_params.delete(:approvals_before_merge)
end
......
......@@ -92,6 +92,25 @@ describe Projects::MergeRequestsController do
expect(response).to redirect_to(namespace_project_merge_request_path(id: created_merge_request.iid, project_id: project.to_param))
end
end
context 'when the target project is a fork of a deleted project' do
before do
original_project = create(:empty_project)
project.update_attributes(forked_from_project: original_project, approvals_before_merge: 4)
original_project.update_attributes(pending_delete: true)
create_merge_request(approvals_before_merge: 3)
end
it 'uses the default from the target project' do
expect(created_merge_request.approvals_before_merge).to eq(nil)
end
it 'creates the merge request' do
expect(created_merge_request).to be_valid
expect(response).to redirect_to(namespace_project_merge_request_path(id: created_merge_request.iid, project_id: project.to_param))
end
end
end
context 'when the merge request is invalid' do
......
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