From 71b0f8ea0b7d4460fdbb70ca9b61789d37ed4885 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 31 Mar 2013 17:08:10 +0300 Subject: [PATCH] Use existing methods for branch names: Ex use @repository.branch_names instead of @repository.heads.map(&:name) --- app/controllers/merge_requests_controller.rb | 4 ++-- app/models/project.rb | 21 +++++++++++++------- app/models/repository.rb | 3 ++- app/views/merge_requests/_form.html.haml | 4 ++-- spec/models/project_spec.rb | 13 +++++++++++- 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 88e0df16..e2185361 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -129,11 +129,11 @@ class MergeRequestsController < ProjectResourceController def validates_merge_request # Show git not found page if target branch doesn't exist - return invalid_mr unless @project.repo.heads.map(&:name).include?(@merge_request.target_branch) + return invalid_mr unless @project.repository.branch_names.include?(@merge_request.target_branch) # Show git not found page if source branch doesn't exist # and there is no saved commits between source & target branch - return invalid_mr if !@project.repo.heads.map(&:name).include?(@merge_request.source_branch) && @merge_request.commits.blank? + return invalid_mr if !@project.repository.branch_names.include?(@merge_request.source_branch) && @merge_request.commits.blank? end def define_show_vars diff --git a/app/models/project.rb b/app/models/project.rb index 0da35b5d..6871afca 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -369,12 +369,19 @@ class Project < ActiveRecord::Base end def open_branches - if protected_branches.empty? - self.repo.heads - else - pnames = protected_branches.map(&:name) - self.repo.heads.reject { |h| pnames.include?(h.name) } - end.sort_by(&:name) + all_branches = repository.branches + + if protected_branches.present? + all_branches.reject! do |branch| + protected_branches_names.include?(branch.name) + end + end + + all_branches + end + + def protected_branches_names + @protected_branches_names ||= protected_branches.map(&:name) end def root_ref?(branch) @@ -396,6 +403,6 @@ class Project < ActiveRecord::Base # Check if current branch name is marked as protected in the system def protected_branch? branch_name - protected_branches.map(&:name).include?(branch_name) + protected_branches_names.include?(branch_name) end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 934c1a6e..7f56047b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -63,8 +63,9 @@ class Repository end # Returns an Array of branch names + # sorted by name ASC def branch_names - repo.branches.collect(&:name).sort + branches.map(&:name) end # Returns an Array of Branches diff --git a/app/views/merge_requests/_form.html.haml b/app/views/merge_requests/_form.html.haml index 816c852d..6d64988c 100644 --- a/app/views/merge_requests/_form.html.haml +++ b/app/views/merge_requests/_form.html.haml @@ -13,7 +13,7 @@ .mr_branch_box %h5.cgray From (Head Branch) .body - .padded= f.select(:source_branch, @repository.heads.map(&:name), { include_blank: "Select branch" }, {class: 'chosen span4'}) + .padded= f.select(:source_branch, @repository.branch_names, { include_blank: "Select branch" }, {class: 'chosen span4'}) .mr_source_commit .span2 @@ -22,7 +22,7 @@ .mr_branch_box %h5.cgray To (Base Branch) .body - .padded= f.select(:target_branch, @repository.heads.map(&:name), { include_blank: "Select branch" }, {class: 'chosen span4'}) + .padded= f.select(:target_branch, @repository.branch_names, { include_blank: "Select branch" }, {class: 'chosen span4'}) .mr_target_commit %fieldset diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e6585f78..53388b93 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -233,7 +233,7 @@ describe Project do it "should be true for projects with external issues tracker if issues enabled" do ext_project.can_have_issues_tracker_id?.should be_true - end + end it "should be false for projects with internal issue tracker if issues enabled" do project.can_have_issues_tracker_id?.should be_false @@ -247,4 +247,15 @@ describe Project do ext_project.can_have_issues_tracker_id?.should be_false end end + + describe :open_branches do + let(:project) { create(:project) } + + before do + project.protected_branches.create(name: 'master') + end + + it { project.open_branches.map(&:name).should include('bootstrap') } + it { project.open_branches.map(&:name).should_not include('master') } + end end