Commit c5e183d2 authored by Igor Drozdov's avatar Igor Drozdov

Allow to cherry pick into different project

Accept target_project_id param that defines the
project to commit into
parent e7b0bd33
...@@ -5,19 +5,23 @@ module CreatesCommit ...@@ -5,19 +5,23 @@ module CreatesCommit
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
# rubocop:disable Gitlab/ModuleWithInstanceVariables # rubocop:disable Gitlab/ModuleWithInstanceVariables
def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil, target_project: nil)
if user_access(@project).can_push_to_branch?(branch_name_or_ref) target_project ||= @project
@project_to_commit_into = @project
if user_access(target_project).can_push_to_branch?(branch_name_or_ref)
@project_to_commit_into = target_project
@branch_name ||= @ref @branch_name ||= @ref
else else
@project_to_commit_into = current_user.fork_of(@project) @project_to_commit_into = current_user.fork_of(target_project)
@branch_name ||= @project_to_commit_into.repository.next_branch('patch') @branch_name ||= @project_to_commit_into.repository.next_branch('patch')
end end
@start_branch ||= @ref || @branch_name @start_branch ||= @ref || @branch_name
start_project = Feature.enabled?(:pick_into_project, @project, default_enabled: :yaml) ? @project_to_commit_into : @project
commit_params = @commit_params.merge( commit_params = @commit_params.merge(
start_project: @project, start_project: start_project,
start_branch: @start_branch, start_branch: @start_branch,
branch_name: @branch_name branch_name: @branch_name
) )
...@@ -27,7 +31,7 @@ module CreatesCommit ...@@ -27,7 +31,7 @@ module CreatesCommit
if result[:status] == :success if result[:status] == :success
update_flash_notice(success_notice) update_flash_notice(success_notice)
success_path = final_success_path(success_path) success_path = final_success_path(success_path, target_project)
respond_to do |format| respond_to do |format|
format.html { redirect_to success_path } format.html { redirect_to success_path }
...@@ -79,9 +83,9 @@ module CreatesCommit ...@@ -79,9 +83,9 @@ module CreatesCommit
end end
end end
def final_success_path(success_path) def final_success_path(success_path, target_project)
if create_merge_request? if create_merge_request?
merge_request_exists? ? existing_merge_request_path : new_merge_request_path merge_request_exists? ? existing_merge_request_path : new_merge_request_path(target_project)
else else
success_path = success_path.call if success_path.respond_to?(:call) success_path = success_path.call if success_path.respond_to?(:call)
...@@ -90,12 +94,12 @@ module CreatesCommit ...@@ -90,12 +94,12 @@ module CreatesCommit
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables # rubocop:disable Gitlab/ModuleWithInstanceVariables
def new_merge_request_path def new_merge_request_path(target_project)
project_new_merge_request_path( project_new_merge_request_path(
@project_to_commit_into, @project_to_commit_into,
merge_request: { merge_request: {
source_project_id: @project_to_commit_into.id, source_project_id: @project_to_commit_into.id,
target_project_id: @project.id, target_project_id: target_project.id,
source_branch: @branch_name, source_branch: @branch_name,
target_branch: @start_branch target_branch: @start_branch
} }
......
...@@ -114,7 +114,7 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -114,7 +114,7 @@ class Projects::CommitController < Projects::ApplicationController
@branch_name = create_new_branch? ? @commit.revert_branch_name : @start_branch @branch_name = create_new_branch? ? @commit.revert_branch_name : @start_branch
create_commit(Commits::RevertService, success_notice: "The #{@commit.change_type_title(current_user)} has been successfully reverted.", create_commit(Commits::RevertService, success_notice: "The #{@commit.change_type_title(current_user)} has been successfully reverted.",
success_path: -> { successful_change_path }, failure_path: failed_change_path) success_path: -> { successful_change_path(@project) }, failure_path: failed_change_path)
end end
def cherry_pick def cherry_pick
...@@ -122,10 +122,15 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -122,10 +122,15 @@ class Projects::CommitController < Projects::ApplicationController
return render_404 if @start_branch.blank? return render_404 if @start_branch.blank?
target_project = find_cherry_pick_target_project
return render_404 unless target_project
@branch_name = create_new_branch? ? @commit.cherry_pick_branch_name : @start_branch @branch_name = create_new_branch? ? @commit.cherry_pick_branch_name : @start_branch
create_commit(Commits::CherryPickService, success_notice: "The #{@commit.change_type_title(current_user)} has been successfully cherry-picked into #{@branch_name}.", create_commit(Commits::CherryPickService, success_notice: "The #{@commit.change_type_title(current_user)} has been successfully cherry-picked into #{@branch_name}.",
success_path: -> { successful_change_path }, failure_path: failed_change_path) success_path: -> { successful_change_path(target_project) },
failure_path: failed_change_path,
target_project: target_project)
end end
private private
...@@ -138,8 +143,8 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -138,8 +143,8 @@ class Projects::CommitController < Projects::ApplicationController
params[:create_merge_request].present? || !can?(current_user, :push_code, @project) params[:create_merge_request].present? || !can?(current_user, :push_code, @project)
end end
def successful_change_path def successful_change_path(target_project)
referenced_merge_request_url || project_commits_url(@project, @branch_name) referenced_merge_request_url || project_commits_url(target_project, @branch_name)
end end
def failed_change_path def failed_change_path
...@@ -218,4 +223,14 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -218,4 +223,14 @@ class Projects::CommitController < Projects::ApplicationController
@start_branch = params[:start_branch] @start_branch = params[:start_branch]
@commit_params = { commit: @commit } @commit_params = { commit: @commit }
end end
def find_cherry_pick_target_project
return @project if params[:target_project_id].blank?
return @project unless Feature.enabled?(:pick_into_project, @project, default_enabled: :yaml)
MergeRequestTargetProjectFinder
.new(current_user: current_user, source_project: @project, project_feature: :repository)
.execute
.find_by_id(params[:target_project_id])
end
end end
...@@ -139,7 +139,7 @@ module CommitsHelper ...@@ -139,7 +139,7 @@ module CommitsHelper
def cherry_pick_projects_data(project) def cherry_pick_projects_data(project)
return [] unless Feature.enabled?(:pick_into_project, project, default_enabled: :yaml) return [] unless Feature.enabled?(:pick_into_project, project, default_enabled: :yaml)
target_projects(project).map do |project| [project, project.forked_from_project].compact.map do |project|
{ {
id: project.id.to_s, id: project.id.to_s,
name: project.full_path, name: project.full_path,
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Projects::CommitController do RSpec.describe Projects::CommitController do
include ProjectForksHelper
let_it_be(:project) { create(:project, :repository) } let_it_be(:project) { create(:project, :repository) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
...@@ -295,6 +297,102 @@ RSpec.describe Projects::CommitController do ...@@ -295,6 +297,102 @@ RSpec.describe Projects::CommitController do
expect(flash[:alert]).to match('Sorry, we cannot cherry-pick this commit automatically.') expect(flash[:alert]).to match('Sorry, we cannot cherry-pick this commit automatically.')
end end
end end
context 'when a project has a fork' do
let(:project) { create(:project, :repository) }
let(:forked_project) { fork_project(project, user, namespace: user.namespace, repository: true) }
let(:target_project) { project }
let(:create_merge_request) { nil }
def send_request
post(:cherry_pick,
params: {
namespace_id: forked_project.namespace,
project_id: forked_project,
target_project_id: target_project.id,
start_branch: 'feature',
id: forked_project.commit.id,
create_merge_request: create_merge_request
})
end
def merge_request_url(source_project, branch)
project_new_merge_request_path(
source_project,
merge_request: {
source_project_id: source_project.id,
target_project_id: project.id,
source_branch: branch,
target_branch: 'feature'
}
)
end
before do
forked_project.add_maintainer(user)
end
it 'successfully cherry picks a commit from fork to upstream project' do
send_request
expect(response).to redirect_to project_commits_path(project, 'feature')
expect(flash[:notice]).to eq('The commit has been successfully cherry-picked into feature.')
expect(project.commit('feature').message).to include(forked_project.commit.id)
end
context 'when the cherry pick is performed via merge request' do
let(:create_merge_request) { true }
it 'successfully cherry picks a commit from fork to a cherry pick branch' do
branch = forked_project.commit.cherry_pick_branch_name
send_request
expect(response).to redirect_to merge_request_url(project, branch)
expect(flash[:notice]).to start_with("The commit has been successfully cherry-picked into #{branch}")
expect(project.commit(branch).message).to include(forked_project.commit.id)
end
end
context 'when a user cannot push to upstream project' do
let(:create_merge_request) { true }
before do
project.add_reporter(user)
end
it 'cherry picks a commit to the fork' do
branch = forked_project.commit.cherry_pick_branch_name
send_request
expect(response).to redirect_to merge_request_url(forked_project, branch)
expect(flash[:notice]).to start_with("The commit has been successfully cherry-picked into #{branch}")
expect(project.commit('feature').message).not_to include(forked_project.commit.id)
expect(forked_project.commit(branch).message).to include(forked_project.commit.id)
end
end
context 'when a user do not have access to the target project' do
let(:target_project) { create(:project, :private) }
it 'cherry picks a commit to the fork' do
send_request
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'disable pick_into_project feature flag' do
before do
stub_feature_flags(pick_into_project: false)
end
it 'does not cherry pick a commit from fork to upstream' do
send_request
expect(project.commit('feature').message).not_to include(forked_project.commit.id)
end
end
end
end end
describe 'GET diff_for_path' do describe 'GET diff_for_path' do
......
...@@ -200,7 +200,7 @@ RSpec.describe CommitsHelper do ...@@ -200,7 +200,7 @@ RSpec.describe CommitsHelper do
end end
it 'returns data for cherry picking into a project' do it 'returns data for cherry picking into a project' do
expect(helper.cherry_pick_projects_data(project)).to match_array([ expect(helper.cherry_pick_projects_data(forked_project)).to match_array([
{ id: project.id.to_s, name: project.full_path, refsUrl: refs_project_path(project) }, { id: project.id.to_s, name: project.full_path, refsUrl: refs_project_path(project) },
{ id: forked_project.id.to_s, name: forked_project.full_path, refsUrl: refs_project_path(forked_project) } { id: forked_project.id.to_s, name: forked_project.full_path, refsUrl: refs_project_path(forked_project) }
]) ])
......
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