Commit 71b0f8ea0b7d4460fdbb70ca9b61789d37ed4885
1 parent
025e4157
Exists in
master
and in
4 other branches
Use existing methods for branch names: Ex use @repository.branch_names instead o…
…f @repository.heads.map(&:name)
Showing
5 changed files
with
32 additions
and
13 deletions
Show diff stats
app/controllers/merge_requests_controller.rb
@@ -129,11 +129,11 @@ class MergeRequestsController < ProjectResourceController | @@ -129,11 +129,11 @@ class MergeRequestsController < ProjectResourceController | ||
129 | 129 | ||
130 | def validates_merge_request | 130 | def validates_merge_request |
131 | # Show git not found page if target branch doesn't exist | 131 | # Show git not found page if target branch doesn't exist |
132 | - return invalid_mr unless @project.repo.heads.map(&:name).include?(@merge_request.target_branch) | 132 | + return invalid_mr unless @project.repository.branch_names.include?(@merge_request.target_branch) |
133 | 133 | ||
134 | # Show git not found page if source branch doesn't exist | 134 | # Show git not found page if source branch doesn't exist |
135 | # and there is no saved commits between source & target branch | 135 | # and there is no saved commits between source & target branch |
136 | - return invalid_mr if !@project.repo.heads.map(&:name).include?(@merge_request.source_branch) && @merge_request.commits.blank? | 136 | + return invalid_mr if !@project.repository.branch_names.include?(@merge_request.source_branch) && @merge_request.commits.blank? |
137 | end | 137 | end |
138 | 138 | ||
139 | def define_show_vars | 139 | def define_show_vars |
app/models/project.rb
@@ -369,12 +369,19 @@ class Project < ActiveRecord::Base | @@ -369,12 +369,19 @@ class Project < ActiveRecord::Base | ||
369 | end | 369 | end |
370 | 370 | ||
371 | def open_branches | 371 | def open_branches |
372 | - if protected_branches.empty? | ||
373 | - self.repo.heads | ||
374 | - else | ||
375 | - pnames = protected_branches.map(&:name) | ||
376 | - self.repo.heads.reject { |h| pnames.include?(h.name) } | ||
377 | - end.sort_by(&:name) | 372 | + all_branches = repository.branches |
373 | + | ||
374 | + if protected_branches.present? | ||
375 | + all_branches.reject! do |branch| | ||
376 | + protected_branches_names.include?(branch.name) | ||
377 | + end | ||
378 | + end | ||
379 | + | ||
380 | + all_branches | ||
381 | + end | ||
382 | + | ||
383 | + def protected_branches_names | ||
384 | + @protected_branches_names ||= protected_branches.map(&:name) | ||
378 | end | 385 | end |
379 | 386 | ||
380 | def root_ref?(branch) | 387 | def root_ref?(branch) |
@@ -396,6 +403,6 @@ class Project < ActiveRecord::Base | @@ -396,6 +403,6 @@ class Project < ActiveRecord::Base | ||
396 | 403 | ||
397 | # Check if current branch name is marked as protected in the system | 404 | # Check if current branch name is marked as protected in the system |
398 | def protected_branch? branch_name | 405 | def protected_branch? branch_name |
399 | - protected_branches.map(&:name).include?(branch_name) | 406 | + protected_branches_names.include?(branch_name) |
400 | end | 407 | end |
401 | end | 408 | end |
app/models/repository.rb
@@ -63,8 +63,9 @@ class Repository | @@ -63,8 +63,9 @@ class Repository | ||
63 | end | 63 | end |
64 | 64 | ||
65 | # Returns an Array of branch names | 65 | # Returns an Array of branch names |
66 | + # sorted by name ASC | ||
66 | def branch_names | 67 | def branch_names |
67 | - repo.branches.collect(&:name).sort | 68 | + branches.map(&:name) |
68 | end | 69 | end |
69 | 70 | ||
70 | # Returns an Array of Branches | 71 | # Returns an Array of Branches |
app/views/merge_requests/_form.html.haml
@@ -13,7 +13,7 @@ | @@ -13,7 +13,7 @@ | ||
13 | .mr_branch_box | 13 | .mr_branch_box |
14 | %h5.cgray From (Head Branch) | 14 | %h5.cgray From (Head Branch) |
15 | .body | 15 | .body |
16 | - .padded= f.select(:source_branch, @repository.heads.map(&:name), { include_blank: "Select branch" }, {class: 'chosen span4'}) | 16 | + .padded= f.select(:source_branch, @repository.branch_names, { include_blank: "Select branch" }, {class: 'chosen span4'}) |
17 | .mr_source_commit | 17 | .mr_source_commit |
18 | 18 | ||
19 | .span2 | 19 | .span2 |
@@ -22,7 +22,7 @@ | @@ -22,7 +22,7 @@ | ||
22 | .mr_branch_box | 22 | .mr_branch_box |
23 | %h5.cgray To (Base Branch) | 23 | %h5.cgray To (Base Branch) |
24 | .body | 24 | .body |
25 | - .padded= f.select(:target_branch, @repository.heads.map(&:name), { include_blank: "Select branch" }, {class: 'chosen span4'}) | 25 | + .padded= f.select(:target_branch, @repository.branch_names, { include_blank: "Select branch" }, {class: 'chosen span4'}) |
26 | .mr_target_commit | 26 | .mr_target_commit |
27 | 27 | ||
28 | %fieldset | 28 | %fieldset |
spec/models/project_spec.rb
@@ -233,7 +233,7 @@ describe Project do | @@ -233,7 +233,7 @@ describe Project do | ||
233 | 233 | ||
234 | it "should be true for projects with external issues tracker if issues enabled" do | 234 | it "should be true for projects with external issues tracker if issues enabled" do |
235 | ext_project.can_have_issues_tracker_id?.should be_true | 235 | ext_project.can_have_issues_tracker_id?.should be_true |
236 | - end | 236 | + end |
237 | 237 | ||
238 | it "should be false for projects with internal issue tracker if issues enabled" do | 238 | it "should be false for projects with internal issue tracker if issues enabled" do |
239 | project.can_have_issues_tracker_id?.should be_false | 239 | project.can_have_issues_tracker_id?.should be_false |
@@ -247,4 +247,15 @@ describe Project do | @@ -247,4 +247,15 @@ describe Project do | ||
247 | ext_project.can_have_issues_tracker_id?.should be_false | 247 | ext_project.can_have_issues_tracker_id?.should be_false |
248 | end | 248 | end |
249 | end | 249 | end |
250 | + | ||
251 | + describe :open_branches do | ||
252 | + let(:project) { create(:project) } | ||
253 | + | ||
254 | + before do | ||
255 | + project.protected_branches.create(name: 'master') | ||
256 | + end | ||
257 | + | ||
258 | + it { project.open_branches.map(&:name).should include('bootstrap') } | ||
259 | + it { project.open_branches.map(&:name).should_not include('master') } | ||
260 | + end | ||
250 | end | 261 | end |