diff options
author | Andrew Newdigate <andrew@gitlab.com> | 2017-10-10 14:56:04 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2017-10-10 14:56:04 +0000 |
commit | 47d19c2de1df14bb3ab00d051be8e4f19f4ed7c4 (patch) | |
tree | a937896d47c846185e397528bdd6fa3e64ae78b9 | |
parent | 900dd7fa93ee9de3de767aa4bf5baca7e9f83348 (diff) | |
download | gitlab-ce-47d19c2de1df14bb3ab00d051be8e4f19f4ed7c4.tar.gz |
Stop using `branch_names.include?` and use `branch_exists?`
9 files changed, 15 insertions, 10 deletions
diff --git a/app/helpers/compare_helper.rb b/app/helpers/compare_helper.rb index 2c28dd81c87..8bf96c0905f 100644 --- a/app/helpers/compare_helper.rb +++ b/app/helpers/compare_helper.rb @@ -4,8 +4,8 @@ module CompareHelper to.present? && from != to && can?(current_user, :create_merge_request, project) && - project.repository.branch_names.include?(from) && - project.repository.branch_names.include?(to) + project.repository.branch_exists?(from) && + project.repository.branch_exists?(to) end def create_mr_path(from = params[:from], to = params[:to], project = @project) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 75e9bdaaa45..972a35dde4d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -675,13 +675,13 @@ class MergeRequest < ActiveRecord::Base def source_branch_exists? return false unless self.source_project - self.source_project.repository.branch_names.include?(self.source_branch) + self.source_project.repository.branch_exists?(self.source_branch) end def target_branch_exists? return false unless self.target_project - self.target_project.repository.branch_names.include?(self.target_branch) + self.target_project.repository.branch_exists?(self.target_branch) end def merge_commit_message(include_description: false) diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index a077b3584b0..955d934838b 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -458,7 +458,7 @@ module QuickActions target_branch_param.strip end command :target_branch do |branch_name| - @updates[:target_branch] = branch_name if project.repository.branch_names.include?(branch_name) + @updates[:target_branch] = branch_name if project.repository.branch_exists?(branch_name) end desc 'Move issue from one column of the board to another' diff --git a/changelogs/unreleased/an-use-branch-exists-over-branch-names-include.yml b/changelogs/unreleased/an-use-branch-exists-over-branch-names-include.yml new file mode 100644 index 00000000000..19d950b48d6 --- /dev/null +++ b/changelogs/unreleased/an-use-branch-exists-over-branch-names-include.yml @@ -0,0 +1,5 @@ +--- +title: Avoid fetching all branches for branch existence checks +merge_request: 14778 +author: +type: changed diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index 5e0b57e9b2e..3b3b63444c7 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -62,7 +62,7 @@ describe Projects::BranchesController do let(:branch) { "feature%2Ftest" } let(:ref) { "<script>alert('ref');</script>" } it { is_expected.to render_template('new') } - it { project.repository.branch_names.include?('feature/test') } + it { project.repository.branch_exists?('feature/test') } end end diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb index 62b23121c5a..9f67216705d 100644 --- a/spec/features/projects/issuable_templates_spec.rb +++ b/spec/features/projects/issuable_templates_spec.rb @@ -118,7 +118,7 @@ feature 'issuable templates', :js do context 'user creates a merge request from a forked project using templates' do let(:template_content) { 'this is a test "feature-proposal" template' } let(:fork_user) { create(:user) } - let(:forked_project) { fork_project(project, fork_user) } + let(:forked_project) { fork_project(project, fork_user, repository: true) } let(:merge_request) { create(:merge_request, :with_diffs, source_project: forked_project, target_project: project) } background do diff --git a/spec/features/projects/merge_requests/user_views_open_merge_request_spec.rb b/spec/features/projects/merge_requests/user_views_open_merge_request_spec.rb index 8970cf54457..3aac93eaf7c 100644 --- a/spec/features/projects/merge_requests/user_views_open_merge_request_spec.rb +++ b/spec/features/projects/merge_requests/user_views_open_merge_request_spec.rb @@ -6,7 +6,7 @@ describe 'User views an open merge request' do end context 'when a merge request does not have repository' do - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, :repository) } before do visit(merge_request_path(merge_request)) diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb index 485b0b287ad..2dc3c5e3927 100644 --- a/spec/features/task_lists_spec.rb +++ b/spec/features/task_lists_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' feature 'Task Lists' do include Warden::Test::Helpers - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:user2) { create(:user) } diff --git a/spec/serializers/merge_request_entity_spec.rb b/spec/serializers/merge_request_entity_spec.rb index 4aeb593da44..87832b3dca1 100644 --- a/spec/serializers/merge_request_entity_spec.rb +++ b/spec/serializers/merge_request_entity_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe MergeRequestEntity do - let(:project) { create :project } + let(:project) { create :project, :repository } let(:resource) { create(:merge_request, source_project: project, target_project: project) } let(:user) { create(:user) } |