diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index e8e681ce649c44518d60c63aed2db4d5d475599b..7bfcda67aa2703178fd2e994d34cf8c0006c5a51 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -5,19 +5,23 @@ module CreatesCommit include Gitlab::Utils::StrongMemoize # rubocop:disable Gitlab/ModuleWithInstanceVariables - def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) - if user_access(@project).can_push_to_branch?(branch_name_or_ref) - @project_to_commit_into = @project + def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil, target_project: nil) + target_project ||= @project + + if user_access(target_project).can_push_to_branch?(branch_name_or_ref) + @project_to_commit_into = target_project @branch_name ||= @ref 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') end @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( - start_project: @project, + start_project: start_project, start_branch: @start_branch, branch_name: @branch_name ) @@ -27,7 +31,7 @@ def create_commit(service, success_path:, failure_path:, failure_view: nil, succ if result[:status] == :success 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| format.html { redirect_to success_path } @@ -79,9 +83,9 @@ def update_flash_notice(success_notice) end end - def final_success_path(success_path) + def final_success_path(success_path, target_project) 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 success_path = success_path.call if success_path.respond_to?(:call) @@ -90,12 +94,12 @@ def final_success_path(success_path) end # rubocop:disable Gitlab/ModuleWithInstanceVariables - def new_merge_request_path + def new_merge_request_path(target_project) project_new_merge_request_path( @project_to_commit_into, merge_request: { source_project_id: @project_to_commit_into.id, - target_project_id: @project.id, + target_project_id: target_project.id, source_branch: @branch_name, target_branch: @start_branch } diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index fdf66340cbbc888a6b850f8609b59887618b9aba..1e65974a3cd2c34e25931381a5503f13fc9749b0 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -114,7 +114,7 @@ def revert @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.", - success_path: -> { successful_change_path }, failure_path: failed_change_path) + success_path: -> { successful_change_path(@project) }, failure_path: failed_change_path) end def cherry_pick @@ -122,10 +122,15 @@ def cherry_pick 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 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 private @@ -138,8 +143,8 @@ def create_new_branch? params[:create_merge_request].present? || !can?(current_user, :push_code, @project) end - def successful_change_path - referenced_merge_request_url || project_commits_url(@project, @branch_name) + def successful_change_path(target_project) + referenced_merge_request_url || project_commits_url(target_project, @branch_name) end def failed_change_path @@ -218,4 +223,14 @@ def assign_change_commit_vars @start_branch = params[:start_branch] @commit_params = { commit: @commit } 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 diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 652ba9950bc831f76a505776ec43ba46c193fffd..e7a81eb56292f1bfee722c42310bd7b049413898 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -139,7 +139,7 @@ def conditionally_paginate_diff_files(diffs, paginate:, per: Projects::CommitCon def cherry_pick_projects_data(project) 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, name: project.full_path, diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 2d7f036be217fe02240c21bc0c8a20ffe457d5f3..a231b54419e7bc8adcaa318138ea73393c71822f 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Projects::CommitController do + include ProjectForksHelper + let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } @@ -295,6 +297,102 @@ def go(extra_params = {}) expect(flash[:alert]).to match('Sorry, we cannot cherry-pick this commit automatically.') 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 describe 'GET diff_for_path' do diff --git a/spec/helpers/commits_helper_spec.rb b/spec/helpers/commits_helper_spec.rb index 9e66fee10c5a86b28ec3ff5df96774ab45768a8e..86ed133e599307c6308720e141507908da19bc31 100644 --- a/spec/helpers/commits_helper_spec.rb +++ b/spec/helpers/commits_helper_spec.rb @@ -200,7 +200,7 @@ end 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: forked_project.id.to_s, name: forked_project.full_path, refsUrl: refs_project_path(forked_project) } ])