Use existing methods for branch names: Ex use @repository.branch_names instead of @repository.heads.map(&:name)

5 files changed, 32 insertions, 13 deletions

diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb
index 88e0df16409..e2185361f4f 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 0da35b5d742..6871afca50a 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 934c1a6e086..7f56047b3de 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 816c852d24b..6d64988c15a 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 e6585f78aa8..53388b933ee 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