Automatic merge of gitlab-org/gitlab-ce master

app/controllers/projects/branches_controller.rb

@@ -68,8 +68,9 @@ class Projects::BranchesController < Projects::ApplicationController
     success = (result[:status] == :success)
 
     if params[:issue_iid] && success
-      issue = IssuesFinder.new(current_user, project_id: @project.id).find_by(iid: params[:issue_iid])
-      SystemNoteService.new_issue_branch(issue, @project, current_user, branch_name) if issue
+      target_project = confidential_issue_project || @project
+      issue = IssuesFinder.new(current_user, project_id: target_project.id).find_by(iid: params[:issue_iid])
+      SystemNoteService.new_issue_branch(issue, target_project, current_user, branch_name, branch_project: @project) if issue
     end
 
     respond_to do |format|
@@ -166,4 +167,15 @@ class Projects::BranchesController < Projects::ApplicationController
       @branches = Kaminari.paginate_array(@branches).page(params[:page])
     end
   end
+
+  def confidential_issue_project
+    return unless Feature.enabled?(:create_confidential_merge_request, @project)
+    return if params[:confidential_issue_project_id].blank?
+
+    confidential_issue_project = Project.find(params[:confidential_issue_project_id])
+
+    return unless can?(current_user, :update_issue, confidential_issue_project)
+
+    confidential_issue_project
+  end
 end

app/controllers/projects/issues_controller.rb

@@ -172,6 +172,7 @@ class Projects::IssuesController < Projects::ApplicationController
 
   def create_merge_request
     create_params = params.slice(:branch_name, :ref).merge(issue_iid: issue.iid)
+    create_params[:target_project_id] = params[:target_project_id] if Feature.enabled?(:create_confidential_merge_request, @project)
     result = ::MergeRequests::CreateFromIssueService.new(project, current_user, create_params).execute
 
     if result[:status] == :success

app/mailers/emails/notes.rb

@@ -51,7 +51,7 @@ module Emails
     def note_thread_options(recipient_id)
       {
         from: sender(@note.author_id),
-        to: recipient(recipient_id, @group),
+        to: recipient(recipient_id, @project&.group || @group),
         subject: subject("#{@note.noteable.title} (#{@note.noteable.reference_link_text})")
       }
     end
@@ -60,7 +60,7 @@ module Emails
       # `note_id` is a `Note` when originating in `NotifyPreview`
       @note = note_id.is_a?(Note) ? note_id : Note.find(note_id)
       @project = @note.project
-      @group = @project.try(:group) || @note.noteable.try(:group)
+      @group = @note.noteable.try(:group)
 
       if (@project || @group) && @note.persisted?
         @sent_notification = SentNotification.record_note(@note, recipient_id, reply_key)

app/services/merge_requests/create_from_issue_service.rb

@@ -6,17 +6,20 @@ module MergeRequests
       # branch - the name of new branch
       # ref    - the source of new branch.
 
-      @branch_name = params[:branch_name]
-      @issue_iid   = params[:issue_iid]
-      @ref         = params[:ref]
+      @branch_name       = params[:branch_name]
+      @issue_iid         = params[:issue_iid]
+      @ref               = params[:ref]
+      @target_project_id = params[:target_project_id]
 
       super(project, user)
     end
 
     def execute
+      return error('Project not found') if target_project.blank?
+      return error('Not allowed to create merge request') unless can_create_merge_request?
       return error('Invalid issue iid') unless @issue_iid.present? && issue.present?
 
-      result = CreateBranchService.new(project, current_user).execute(branch_name, ref)
+      result = CreateBranchService.new(target_project, current_user).execute(branch_name, ref)
       return result if result[:status] == :error
 
       new_merge_request = create(merge_request)
@@ -26,7 +29,7 @@ module MergeRequests
 
         success(new_merge_request)
       else
-        SystemNoteService.new_issue_branch(issue, project, current_user, branch_name)
+        SystemNoteService.new_issue_branch(issue, project, current_user, branch_name, branch_project: target_project)
 
         error(new_merge_request.errors)
       end
@@ -34,6 +37,10 @@ module MergeRequests
 
     private
 
+    def can_create_merge_request?
+      can?(current_user, :create_merge_request_from, target_project)
+    end
+
     # rubocop: disable CodeReuse/ActiveRecord
     def issue
       @issue ||= IssuesFinder.new(current_user, project_id: project.id).find_by(iid: @issue_iid)
@@ -45,21 +52,21 @@ module MergeRequests
     end
 
     def ref
-      return @ref if project.repository.branch_exists?(@ref)
+      return @ref if target_project.repository.branch_exists?(@ref)
 
-      project.default_branch || 'master'
+      target_project.default_branch || 'master'
     end
 
     def merge_request
-      MergeRequests::BuildService.new(project, current_user, merge_request_params).execute
+      MergeRequests::BuildService.new(target_project, current_user, merge_request_params).execute
     end
 
     def merge_request_params
       {
         issue_iid: @issue_iid,
-        source_project_id: project.id,
+        source_project_id: target_project.id,
         source_branch: branch_name,
-        target_project_id: project.id,
+        target_project_id: target_project.id,
         target_branch: ref
       }
     end
@@ -67,5 +74,14 @@ module MergeRequests
     def success(merge_request)
       super().merge(merge_request: merge_request)
     end
+
+    def target_project
+      @target_project ||=
+        if @target_project_id.present?
+          project.forks.find_by_id(@target_project_id)
+        else
+          project
+        end
+    end
   end
 end

app/services/system_note_service.rb

@@ -404,8 +404,9 @@ module SystemNoteService
   # Example note text:
   #
   #   "created branch `201-issue-branch-button`"
-  def new_issue_branch(issue, project, author, branch)
-    link = url_helpers.project_compare_path(project, from: project.default_branch, to: branch)
+  def new_issue_branch(issue, project, author, branch, branch_project: nil)
+    branch_project ||= project
+    link = url_helpers.project_compare_path(branch_project, from: branch_project.default_branch, to: branch)
 
     body = "created branch [`#{branch}`](#{link}) to address this issue"
 
@@ -413,7 +414,7 @@ module SystemNoteService
   end
 
   def new_merge_request(issue, project, author, merge_request)
-    body = "created merge request #{merge_request.to_reference} to address this issue"
+    body = "created merge request #{merge_request.to_reference(project)} to address this issue"
 
     create_note(NoteSummary.new(issue, project, author, body, action: 'merge'))
   end

spec/controllers/projects/branches_controller_spec.rb

@@ -92,7 +92,7 @@ describe Projects::BranchesController do
       end
 
       it 'posts a system note' do
-        expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, "1-feature-branch")
+        expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, "1-feature-branch", branch_project: project)
 
         post :create,
              params: {
@@ -103,6 +103,75 @@ describe Projects::BranchesController do
              }
       end
 
+      context 'confidential_issue_project_id is present' do
+        let(:confidential_issue_project) { create(:project) }
+
+        def create_branch_with_confidential_issue_project
+          post(
+            :create,
+            params: {
+              namespace_id: project.namespace,
+              project_id: project,
+              branch_name: branch,
+              confidential_issue_project_id: confidential_issue_project.id,
+              issue_iid: issue.iid
+            }
+          )
+        end
+
+        context 'create_confidential_merge_request feature is enabled' do
+          before do
+            stub_feature_flags(create_confidential_merge_request: true)
+          end
+
+          context 'user cannot update issue' do
+            let(:issue) { create(:issue, project: confidential_issue_project) }
+
+            it 'does not post a system note' do
+              expect(SystemNoteService).not_to receive(:new_issue_branch)
+
+              create_branch_with_confidential_issue_project
+            end
+          end
+
+          context 'user can update issue' do
+            before do
+              confidential_issue_project.add_reporter(user)
+            end
+
+            context 'issue is under the specified project' do
+              let(:issue) { create(:issue, project: confidential_issue_project) }
+
+              it 'posts a system note' do
+                expect(SystemNoteService).to receive(:new_issue_branch).with(issue, confidential_issue_project, user, "1-feature-branch", branch_project: project)
+
+                create_branch_with_confidential_issue_project
+              end
+            end
+
+            context 'issue is not under the specified project' do
+              it 'does not post a system note' do
+                expect(SystemNoteService).not_to receive(:new_issue_branch)
+
+                create_branch_with_confidential_issue_project
+              end
+            end
+          end
+        end
+
+        context 'create_confidential_merge_request feature is disabled' do
+          before do
+            stub_feature_flags(create_confidential_merge_request: false)
+          end
+
+          it 'posts a system note on project' do
+            expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, "1-feature-branch", branch_project: project)
+
+            create_branch_with_confidential_issue_project
+          end
+        end
+      end
+
       context 'repository-less project' do
         let(:project) { create :project }
 

spec/controllers/projects/issues_controller_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Projects::IssuesController do
+  include ProjectForksHelper
+
   let(:project) { create(:project) }
   let(:user)    { create(:user) }
   let(:issue)   { create(:issue, project: project) }
@@ -1130,6 +1132,7 @@ describe Projects::IssuesController do
   end
 
   describe 'POST create_merge_request' do
+    let(:target_project_id) { nil }
     let(:project) { create(:project, :repository, :public) }
 
     before do
@@ -1163,13 +1166,42 @@ describe Projects::IssuesController do
       expect(response).to have_gitlab_http_status(404)
     end
 
+    context 'target_project_id is set' do
+      let(:target_project) { fork_project(project, user, repository: true) }
+      let(:target_project_id) { target_project.id }
+
+      context 'create_confidential_merge_request feature is enabled' do
+        before do
+          stub_feature_flags(create_confidential_merge_request: true)
+        end
+
+        it 'creates a new merge request' do
+          expect { create_merge_request }.to change(target_project.merge_requests, :count).by(1)
+        end
+      end
+
+      context 'create_confidential_merge_request feature is disabled' do
+        before do
+          stub_feature_flags(create_confidential_merge_request: false)
+        end
+
+        it 'creates a new merge request' do
+          expect { create_merge_request }.to change(project.merge_requests, :count).by(1)
+        end
+      end
+    end
+
     def create_merge_request
-      post :create_merge_request, params: {
-                                    namespace_id: project.namespace.to_param,
-                                    project_id: project.to_param,
-                                    id: issue.to_param
-                                  },
-                                  format: :json
+      post(
+        :create_merge_request,
+        params: {
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param,
+          id: issue.to_param,
+          target_project_id: target_project_id
+        },
+        format: :json
+      )
     end
   end
 

spec/mailers/notify_spec.rb

@@ -606,7 +606,7 @@ describe Notify do
       it_behaves_like 'a user cannot unsubscribe through footer link'
 
       it 'has the correct subject and body' do
-        is_expected.to have_referable_subject(project_snippet, include_group: true, reply: true)
+        is_expected.to have_referable_subject(project_snippet, reply: true)
         is_expected.to have_body_text project_snippet_note.note
       end
     end
@@ -813,7 +813,7 @@ describe Notify do
 
         it 'has the correct subject and body' do
           aggregate_failures do
-            is_expected.to have_subject("Re: #{project.name} | #{project.group.name} | #{commit.title} (#{commit.short_id})")
+            is_expected.to have_subject("Re: #{project.name} | #{commit.title} (#{commit.short_id})")
             is_expected.to have_body_text(commit.short_id)
           end
         end
@@ -839,7 +839,7 @@ describe Notify do
 
         it 'has the correct subject and body' do
           aggregate_failures do
-            is_expected.to have_referable_subject(merge_request, include_group: true, reply: true)
+            is_expected.to have_referable_subject(merge_request, reply: true)
             is_expected.to have_body_text note_on_merge_request_path
           end
         end
@@ -865,7 +865,7 @@ describe Notify do
 
         it 'has the correct subject and body' do
           aggregate_failures do
-            is_expected.to have_referable_subject(issue, include_group: true, reply: true)
+            is_expected.to have_referable_subject(issue, reply: true)
             is_expected.to have_body_text(note_on_issue_path)
           end
         end
@@ -931,7 +931,7 @@ describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'has the correct subject' do
-          is_expected.to have_subject "Re: #{project.name} | #{project.group.name} | #{commit.title} (#{commit.short_id})"
+          is_expected.to have_subject "Re: #{project.name} | #{commit.title} (#{commit.short_id})"
         end
 
         it 'contains a link to the commit' do
@@ -959,7 +959,7 @@ describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'has the correct subject' do
-          is_expected.to have_referable_subject(merge_request, include_group: true, reply: true)
+          is_expected.to have_referable_subject(merge_request, reply: true)
         end
 
         it 'contains a link to the merge request note' do
@@ -987,7 +987,7 @@ describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
 
         it 'has the correct subject' do
-          is_expected.to have_referable_subject(issue, include_group: true, reply: true)
+          is_expected.to have_referable_subject(issue, reply: true)
         end
 
         it 'contains a link to the issue note' do

spec/services/system_note_service_spec.rb

@@ -454,16 +454,32 @@ describe SystemNoteService do
   end
 
   describe '.new_issue_branch' do
-    subject { described_class.new_issue_branch(noteable, project, author, "1-mepmep") }
+    let(:branch) { '1-mepmep' }
 
-    it_behaves_like 'a system note' do
-      let(:action) { 'branch' }
-    end
+    subject { described_class.new_issue_branch(noteable, project, author, branch, branch_project: branch_project) }
 
-    context 'when a branch is created from the new branch button' do
-      it 'sets the note text' do
-        expect(subject.note).to start_with("created branch [`1-mepmep`]")
+    shared_examples_for 'a system note for new issue branch' do
+      it_behaves_like 'a system note' do
+        let(:action) { 'branch' }
       end
+
+      context 'when a branch is created from the new branch button' do
+        it 'sets the note text' do
+          expect(subject.note).to start_with("created branch [`#{branch}`]")
+        end
+      end
+    end
+
+    context 'branch_project is set' do
+      let(:branch_project) { create(:project, :repository) }
+
+      it_behaves_like 'a system note for new issue branch'
+    end
+
+    context 'branch_project is not set' do
+      let(:branch_project) { nil }
+
+      it_behaves_like 'a system note for new issue branch'
     end
   end
 
@@ -477,7 +493,7 @@ describe SystemNoteService do
     end
 
     it 'sets the new merge request note text' do
-      expect(subject.note).to eq("created merge request #{merge_request.to_reference} to address this issue")
+      expect(subject.note).to eq("created merge request #{merge_request.to_reference(project)} to address this issue")
     end
   end
 

spec/support/helpers/email_helpers.rb

@@ -37,19 +37,8 @@ module EmailHelpers
     ActionMailer::Base.deliveries.find { |d| d.to.include?(user.notification_email) }
   end
 
-  def have_referable_subject(referable, include_project: true, include_group: false, reply: false)
-    context = []
-
-    context << referable.project.name if include_project && referable.project
-    context << referable.project.group.name if include_group && referable.project.group
-
-    prefix =
-      if context.any?
-        context.join(' | ') + ' | '
-      else
-        ''
-      end
-
+  def have_referable_subject(referable, include_project: true, reply: false)
+    prefix = (include_project && referable.project ? "#{referable.project.name} | " : '').freeze
     prefix = "Re: #{prefix}" if reply
 
     suffix = "#{referable.title} (#{referable.to_reference})"