Merge branch '31830-limit-mr-target-branches' into 'master'

Limit the merge request target branch autocompletion action to group or project scope

See merge request gitlab-org/gitlab!20933

app/controllers/autocomplete_controller.rb

@@ -40,10 +40,20 @@ class AutocompleteController < ApplicationController
   end
 
   def merge_request_target_branches
-    merge_requests = MergeRequestsFinder.new(current_user, params).execute
-    target_branches = merge_requests.recent_target_branches
+    if target_branch_params.present?
+      merge_requests = MergeRequestsFinder.new(current_user, target_branch_params).execute
+      target_branches = merge_requests.recent_target_branches
+
+      render json: target_branches.map { |target_branch| { title: target_branch } }
+    else
+      render json: { error: _('At least one of group_id or project_id must be specified') }, status: :bad_request
+    end
+  end
+
+  private
 
-    render json: target_branches.map { |target_branch| { title: target_branch } }
+  def target_branch_params
+    params.permit(:group_id, :project_id)
   end
 end
 

app/models/merge_request.rb

@@ -277,7 +277,7 @@ class MergeRequest < ApplicationRecord
   def self.recent_target_branches(limit: 100)
     group(:target_branch)
       .select(:target_branch)
-      .reorder('MAX(merge_requests.updated_at) DESC')
+      .reorder(arel_table[:updated_at].maximum.desc)
       .limit(limit)
       .pluck(:target_branch)
   end

changelogs/unreleased/31830-limit-mr-target-branches.yml

+---
+title: Require group_id or project_id for MR target branch autocomplete action
+merge_request: 20933
+author:
+type: performance

locale/gitlab.pot

@@ -2171,6 +2171,9 @@ msgstr ""
 msgid "At least one approval from a code owner is required to change files matching the respective CODEOWNER rules."
 msgstr ""
 
+msgid "At least one of group_id or project_id must be specified"
+msgstr ""
+
 msgid "Attach a file"
 msgstr ""
 

spec/controllers/autocomplete_controller_spec.rb

@@ -365,35 +365,67 @@ describe AutocompleteController do
         expect(json_response[3]).to match('name' => 'thumbsdown')
       end
     end
+  end
 
-    context 'Get merge_request_target_branches' do
-      let(:user2) { create(:user) }
-      let!(:merge_request1) { create(:merge_request, source_project: project, target_branch: 'feature') }
+  context 'Get merge_request_target_branches' do
+    let!(:merge_request) { create(:merge_request, source_project: project, target_branch: 'feature') }
 
-      context 'unauthorized user' do
-        it 'returns empty json' do
-          get :merge_request_target_branches
+    context 'anonymous user' do
+      it 'returns empty json' do
+        get :merge_request_target_branches, params: { project_id: project.id }
 
-          expect(json_response).to be_empty
-        end
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_empty
       end
+    end
 
-      context 'sign in as user without any accesible merge requests' do
-        it 'returns empty json' do
-          sign_in(user2)
-          get :merge_request_target_branches
+    context 'user without any accessible merge requests' do
+      it 'returns empty json' do
+        sign_in(create(:user))
 
-          expect(json_response).to be_empty
-        end
+        get :merge_request_target_branches, params: { project_id: project.id }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_empty
       end
+    end
 
-      context 'sign in as user with a accesible merge request' do
-        it 'returns json' do
-          sign_in(user)
-          get :merge_request_target_branches
+    context 'user with an accessible merge request but no scope' do
+      it 'returns an error' do
+        sign_in(user)
 
-          expect(json_response).to contain_exactly({ 'title' => 'feature' })
-        end
+        get :merge_request_target_branches
+
+        expect(response).to have_gitlab_http_status(400)
+        expect(json_response).to eq({ 'error' => 'At least one of group_id or project_id must be specified' })
+      end
+    end
+
+    context 'user with an accessible merge request by project' do
+      it 'returns json' do
+        sign_in(user)
+
+        get :merge_request_target_branches, params: { project_id: project.id }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to contain_exactly({ 'title' => 'feature' })
+      end
+    end
+
+    context 'user with an accessible merge request by group' do
+      let(:group) { create(:group) }
+      let(:project) { create(:project, namespace: group) }
+      let(:user) { create(:user) }
+
+      it 'returns json' do
+        group.add_owner(user)
+
+        sign_in(user)
+
+        get :merge_request_target_branches, params: { group_id: group.id }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to contain_exactly({ 'title' => 'feature' })
       end
     end
   end