Merge branch 'id-cherry-pick-in-source-project' into 'master'

Allow to cherry pick into different project [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!56121

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 @@ module CreatesCommit
     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 @@ module CreatesCommit
     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 @@ module CreatesCommit
   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
       }

app/controllers/projects/commit_controller.rb

@@ -114,7 +114,7 @@ class Projects::CommitController < Projects::ApplicationController
     @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 @@ class Projects::CommitController < Projects::ApplicationController
 
     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 @@ class Projects::CommitController < Projects::ApplicationController
     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 @@ class Projects::CommitController < Projects::ApplicationController
     @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

app/helpers/commits_helper.rb

@@ -139,7 +139,7 @@ module CommitsHelper
   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,

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 @@ RSpec.describe Projects::CommitController do
         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

spec/helpers/commits_helper_spec.rb

@@ -200,7 +200,7 @@ RSpec.describe CommitsHelper do
     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) }
       ])