Allow comparisons of branches across projects

This is already permitted in the merge request view, for creating fork
merge requests, so it should be permitted in the repository view too.

app/controllers/projects/compare_controller.rb

@@ -6,6 +6,7 @@ class Projects::CompareController < Projects::ApplicationController
   include DiffForPath
   include DiffHelper
   include RendersCommits
+  include CompareHelper
 
   # Authorize
   before_action :require_non_empty_project
@@ -37,16 +38,18 @@ class Projects::CompareController < Projects::ApplicationController
   end
 
   def create
-    if params[:from].blank? || params[:to].blank?
-      flash[:alert] = "You must select a Source and a Target revision"
     from_to_vars = {
       from: params[:from].presence,
-        to: params[:to].presence
+      to: params[:to].presence,
+      from_project_id: params[:from_project_id].presence
     }
-      redirect_to project_compare_index_path(@project, from_to_vars)
+
+    if from_to_vars[:from].blank? || from_to_vars[:to].blank?
+      flash[:alert] = "You must select a Source and a Target revision"
+
+      redirect_to project_compare_index_path(source_project, from_to_vars)
     else
-      redirect_to project_compare_path(@project,
-                                               params[:from], params[:to])
+      redirect_to project_compare_path(source_project, from_to_vars)
     end
   end
 
@@ -73,13 +76,34 @@ class Projects::CompareController < Projects::ApplicationController
     return if valid.all?
 
     flash[:alert] = "Invalid branch name"
-    redirect_to project_compare_index_path(@project)
+    redirect_to project_compare_index_path(source_project)
+  end
+
+  # target == start_ref == from
+  def target_project
+    strong_memoize(:target_project) do
+      next source_project unless params.key?(:from_project_id)
+      next source_project unless Feature.enabled?(:compare_repo_dropdown, source_project, default_enabled: :yaml)
+      next source_project if params[:from_project_id].to_i == source_project.id
+
+      target_project = target_projects(source_project).find_by_id(params[:from_project_id])
+
+      # Just ignore the field if it points at a non-existent or hidden project
+      next source_project unless target_project && can?(current_user, :download_code, target_project)
+
+      target_project
+    end
+  end
+
+  # source == head_ref == to
+  def source_project
+    project
   end
 
   def compare
     return @compare if defined?(@compare)
 
-    @compare = CompareService.new(@project, head_ref).execute(@project, start_ref)
+    @compare = CompareService.new(source_project, head_ref).execute(target_project, start_ref)
   end
 
   def start_ref
@@ -102,9 +126,9 @@ class Projects::CompareController < Projects::ApplicationController
 
   def define_environment
     if compare
-      environment_params = @repository.branch_exists?(head_ref) ? { ref: head_ref } : { commit: compare.commit }
+      environment_params = source_project.repository.branch_exists?(head_ref) ? { ref: head_ref } : { commit: compare.commit }
       environment_params[:find_latest] = true
-      @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last
+      @environment = EnvironmentsFinder.new(source_project, current_user, environment_params).execute.last
     end
   end
 
@@ -114,8 +138,8 @@ class Projects::CompareController < Projects::ApplicationController
 
   # rubocop: disable CodeReuse/ActiveRecord
   def merge_request
-    @merge_request ||= MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened
-      .find_by(source_project: @project, source_branch: head_ref, target_branch: start_ref)
+    @merge_request ||= MergeRequestsFinder.new(current_user, project_id: target_project.id).execute.opened
+      .find_by(source_project: source_project, source_branch: head_ref, target_branch: start_ref)
   end
   # rubocop: enable CodeReuse/ActiveRecord
 end

app/finders/merge_request_target_project_finder.rb

@@ -5,29 +5,30 @@ class MergeRequestTargetProjectFinder
 
   attr_reader :current_user, :source_project
 
-  def initialize(current_user: nil, source_project:)
+  def initialize(current_user: nil, source_project:, project_feature: :merge_requests)
     @current_user = current_user
     @source_project = source_project
+    @project_feature = project_feature
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def execute(include_routes: false)
     if source_project.fork_network
       include_routes ? projects.inc_routes : projects
     else
-      Project.where(id: source_project)
+      Project.id_in(source_project.id)
     end
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 
   private
 
+  attr_reader :project_feature
+
   def projects
     source_project
       .fork_network
       .projects
       .public_or_visible_to_user(current_user)
       .non_archived
-      .with_feature_available_for_user(:merge_requests, current_user)
+      .with_feature_available_for_user(project_feature, current_user)
   end
 end

app/helpers/compare_helper.rb

 # frozen_string_literal: true
 
 module CompareHelper
-  def create_mr_button?(from = params[:from], to = params[:to], project = @project)
+  def create_mr_button?(from: params[:from], to: params[:to], source_project: @project, target_project: @target_project)
     from.present? &&
       to.present? &&
       from != to &&
-      can?(current_user, :create_merge_request_from, project) &&
-      project.repository.branch_exists?(from) &&
-      project.repository.branch_exists?(to)
+      can?(current_user, :create_merge_request_from, source_project) &&
+      can?(current_user, :create_merge_request_in, target_project) &&
+      target_project.repository.branch_exists?(from) &&
+      source_project.repository.branch_exists?(to)
   end
 
-  def create_mr_path(from = params[:from], to = params[:to], project = @project)
+  def create_mr_path(from: params[:from], to: params[:to], source_project: @project, target_project: @target_project)
     project_new_merge_request_path(
-      project,
+      target_project,
       merge_request: {
+        source_project_id: source_project.id,
         source_branch: to,
+        target_project_id: target_project.id,
         target_branch: from
       }
     )
   end
+
+  def target_projects(source_project)
+    MergeRequestTargetProjectFinder
+      .new(current_user: current_user, source_project: source_project, project_feature: :repository)
+      .execute(include_routes: true)
+  end
 end

app/views/events/event/_push.html.haml

@@ -21,7 +21,8 @@
     %ul.content-list.event_commits
       = render "events/commit", project: project, event: event
 
-      - create_mr = event.new_ref? && create_mr_button?(project.default_branch, event.ref_name, project) && event.authored_by?(current_user)
+      - create_mr = event.new_ref? && create_mr_button?(from: project.default_branch, to: event.ref_name, source_project: project, target_project: project) && event.authored_by?(current_user)
+      - create_mr_path = create_mr_path(from: project.default_branch, to: event.ref_name, source_project: project, target_project: project) if create_mr
       - if event.commits_count > 1
         %li.commits-stat
           %span ... and #{pluralize(event.commits_count - 1, 'more commit')}.
@@ -40,9 +41,9 @@
           - if create_mr
             %span
               or
-              = link_to create_mr_path(project.default_branch, event.ref_name, project) do
+              = link_to create_mr_path do
                 create a merge request
       - elsif create_mr
         %li.commits-stat
-          = link_to create_mr_path(project.default_branch, event.ref_name, project) do
+          = link_to create_mr_path do
             Create Merge Request

app/views/projects/branches/_branch.html.haml

@@ -35,8 +35,8 @@
       .gl-display-inline-flex.gl-vertical-align-middle.gl-mr-5
         %svg.s24
 
-    - if merge_project && create_mr_button?(@repository.root_ref, branch.name)
-      = link_to create_mr_path(@repository.root_ref, branch.name), class: 'gl-button btn btn-default' do
+    - if merge_project && create_mr_button?(from: @repository.root_ref, to: branch.name, source_project: @project, target_project: @project)
+      = link_to create_mr_path(from: @repository.root_ref, to: branch.name, source_project: @project, target_project: @project), class: 'gl-button btn btn-default' do
         = _('Merge request')
 
     - if branch.name != @repository.root_ref

app/views/projects/commits/show.html.haml

@@ -18,9 +18,9 @@
         - if @merge_request.present?
           .control.d-none.d-md-block
             = link_to _("View open merge request"), project_merge_request_path(@project, @merge_request), class: 'btn gl-button'
-        - elsif create_mr_button?(@repository.root_ref, @ref)
+        - elsif create_mr_button?(from: @repository.root_ref, to: @ref, source_project: @project, target_project: @project)
           .control.d-none.d-md-block
-            = link_to _("Create merge request"), create_mr_path(@repository.root_ref, @ref), class: 'btn gl-button btn-success'
+            = link_to _("Create merge request"), create_mr_path(from: @repository.root_ref, to: @ref, source_project: @project, target_project: @project), class: 'btn gl-button btn-success'
 
         .control
           = form_tag(project_commits_path(@project, @id), method: :get, class: 'commits-search-form js-signature-container', data: { 'signatures-path' => namespace_project_signatures_path }) do

config/feature_flags/development/compare_repo_dropdown.yml

+---
+name: compare_repo_dropdown
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/14615
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/322141
+milestone: '13.9'
+type: development
+group: group::source code
+default_enabled: false

spec/features/projects/merge_request_button_spec.rb

@@ -3,11 +3,13 @@
 require 'spec_helper'
 
 RSpec.describe 'Merge Request button' do
-  shared_examples 'Merge request button only shown when allowed' do
-    let(:user) { create(:user) }
-    let(:project) { create(:project, :public, :repository) }
-    let(:forked_project) { create(:project, :public, :repository, forked_from_project: project) }
+  include ProjectForksHelper
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project, :public, :repository) }
+  let(:forked_project) { fork_project(project, user, repository: true) }
 
+  shared_examples 'Merge request button only shown when allowed' do
     context 'not logged in' do
       it 'does not show Create merge request button' do
         visit url
@@ -23,9 +25,15 @@ RSpec.describe 'Merge Request button' do
       end
 
       it 'shows Create merge request button' do
-        href = project_new_merge_request_path(project,
-                                              merge_request: { source_branch: 'feature',
-                                                               target_branch: 'master' })
+        href = project_new_merge_request_path(
+          project,
+          merge_request: {
+            source_project_id: project.id,
+            source_branch: 'feature',
+            target_project_id: project.id,
+            target_branch: 'master'
+          }
+        )
 
         visit url
 
@@ -75,12 +83,16 @@ RSpec.describe 'Merge Request button' do
       end
 
       context 'on own fork of project' do
-        let(:user) { forked_project.owner }
-
         it 'shows Create merge request button' do
-          href = project_new_merge_request_path(forked_project,
-                                                merge_request: { source_branch: 'feature',
-                                                                 target_branch: 'master' })
+          href = project_new_merge_request_path(
+            forked_project,
+            merge_request: {
+              source_project_id: forked_project.id,
+              source_branch: 'feature',
+              target_project_id: forked_project.id,
+              target_branch: 'master'
+            }
+          )
 
           visit fork_url
 
@@ -101,11 +113,33 @@ RSpec.describe 'Merge Request button' do
   end
 
   context 'on compare page' do
-    it_behaves_like 'Merge request button only shown when allowed' do
     let(:label) { 'Create merge request' }
+
+    it_behaves_like 'Merge request button only shown when allowed' do
       let(:url) { project_compare_path(project, from: 'master', to: 'feature') }
       let(:fork_url) { project_compare_path(forked_project, from: 'master', to: 'feature') }
     end
+
+    it 'shows the correct merge request button when viewing across forks' do
+      sign_in(user)
+      project.add_developer(user)
+
+      href = project_new_merge_request_path(
+        project,
+        merge_request: {
+          source_project_id: forked_project.id,
+          source_branch: 'feature',
+          target_project_id: project.id,
+          target_branch: 'master'
+        }
+      )
+
+      visit project_compare_path(forked_project, from: 'master', to: 'feature', from_project_id: project.id)
+
+      within("#content-body") do
+        expect(page).to have_link(label, href: href)
+      end
+    end
   end
 
   context 'on commits page' do

spec/finders/merge_request_target_project_finder_spec.rb

@@ -16,13 +16,22 @@ RSpec.describe MergeRequestTargetProjectFinder do
       expect(finder.execute).to contain_exactly(base_project, other_fork, forked_project)
     end
 
-    it 'does not include projects that have merge requests turned off' do
+    it 'does not include projects that have merge requests turned off by default' do
       other_fork.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
       base_project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
 
       expect(finder.execute).to contain_exactly(forked_project)
     end
 
+    it 'includes projects that have merge requests turned off by default with a more-permissive project feature' do
+      finder = described_class.new(current_user: user, source_project: forked_project, project_feature: :repository)
+
+      other_fork.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
+      base_project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
+
+      expect(finder.execute).to contain_exactly(base_project, other_fork, forked_project)
+    end
+
     it 'does not contain archived projects' do
       base_project.update!(archived: true)