Commit 5d88de09 authored by Timothy Andrew's avatar Timothy Andrew

Refactor `Issue#related_branches`

- Previously, the controller held the logic to calculate
  related branches, which was:

  `<branches ending with `issue.iid`> - <branches with a merge request referenced in the current issue>`

- This logic belongs in the `related_branches` method, not in the
  controller. This commit makes this change.

- This means that `Issue#related_branches` now needs to take a `User`.
  When we find the branches that have a merge request referenced in the
  current issue, this is limited to merge requests that the current user
  has access to.

- This is not directly related to #14566, but is a related refactoring.
parent 377b59da
...@@ -66,7 +66,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -66,7 +66,7 @@ class Projects::IssuesController < Projects::ApplicationController
@notes = @issue.notes.nonawards.with_associations.fresh @notes = @issue.notes.nonawards.with_associations.fresh
@noteable = @issue @noteable = @issue
@merge_requests = @issue.referenced_merge_requests(current_user) @merge_requests = @issue.referenced_merge_requests(current_user)
@related_branches = @issue.related_branches - @merge_requests.map(&:source_branch) @related_branches = @issue.related_branches(current_user)
respond_to do |format| respond_to do |format|
format.html format.html
......
...@@ -104,10 +104,17 @@ class Issue < ActiveRecord::Base ...@@ -104,10 +104,17 @@ class Issue < ActiveRecord::Base
end end
end end
def related_branches # All branches containing the current issue's ID, except for
project.repository.branch_names.select do |branch| # those with a merge request open (that the current user can see)
# referencing the current issue.
def related_branches(current_user)
branches_with_iid = project.repository.branch_names.select do |branch|
branch.end_with?("-#{iid}") branch.end_with?("-#{iid}")
end end
branches_with_merge_request = self.referenced_merge_requests(current_user).map(&:source_branch)
branches_with_iid - branches_with_merge_request
end end
# Reset issue events cache # Reset issue events cache
...@@ -161,7 +168,7 @@ class Issue < ActiveRecord::Base ...@@ -161,7 +168,7 @@ class Issue < ActiveRecord::Base
def can_be_worked_on?(current_user) def can_be_worked_on?(current_user)
!self.closed? && !self.closed? &&
!self.project.forked? && !self.project.forked? &&
self.related_branches.empty? && self.related_branches(current_user).empty? &&
self.closed_by_merge_requests(current_user).empty? self.closed_by_merge_requests(current_user).empty?
end end
end end
...@@ -192,10 +192,11 @@ describe Issue, models: true do ...@@ -192,10 +192,11 @@ describe Issue, models: true do
describe '#related_branches' do describe '#related_branches' do
it "selects the right branches" do it "selects the right branches" do
user = build(:user)
allow(subject.project.repository).to receive(:branch_names). allow(subject.project.repository).to receive(:branch_names).
and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name]) and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name])
expect(subject.related_branches).to eq([subject.to_branch_name]) expect(subject.related_branches(user)).to eq([subject.to_branch_name])
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment