From 5d88de092f37497d9c08878954b099c425376bda Mon Sep 17 00:00:00 2001
From: Timothy Andrew <mail@timothyandrew.net>
Date: Tue, 12 Apr 2016 11:43:15 +0530
Subject: [PATCH] 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.
---
 app/controllers/projects/issues_controller.rb |  2 +-
 app/models/issue.rb                           | 13 ++++++++++---
 spec/models/issue_spec.rb                     |  3 ++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 6d649e72f84..a3842036982 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -66,7 +66,7 @@ class Projects::IssuesController < Projects::ApplicationController
     @notes = @issue.notes.nonawards.with_associations.fresh
     @noteable = @issue
     @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|
       format.html
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 68e0113380e..252545d1147 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -104,10 +104,17 @@ class Issue < ActiveRecord::Base
     end
   end
 
-  def related_branches
-    project.repository.branch_names.select do |branch|
+  # All branches containing the current issue's ID, except for
+  # 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}")
     end
+
+    branches_with_merge_request = self.referenced_merge_requests(current_user).map(&:source_branch)
+
+    branches_with_iid - branches_with_merge_request
   end
 
   # Reset issue events cache
@@ -161,7 +168,7 @@ class Issue < ActiveRecord::Base
   def can_be_worked_on?(current_user)
     !self.closed? &&
       !self.project.forked? &&
-      self.related_branches.empty? &&
+      self.related_branches(current_user).empty? &&
       self.closed_by_merge_requests(current_user).empty?
   end
 end
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 15052aaca28..f33b705810e 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -192,10 +192,11 @@ describe Issue, models: true do
 
   describe '#related_branches' do
     it "selects the right branches" do
+      user = build(:user)
       allow(subject.project.repository).to receive(:branch_names).
         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