diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2016-04-12 09:13:15 +0300 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2016-04-12 09:13:15 +0300 |
commit | 5d88de092f37497d9c08878954b099c425376bda (patch) | |
tree | 6733e2a9142a73eec0c8567ae3b9f613039042dd | |
parent | 377b59da3029786f2265058972ad0bc6aefd8b53 (diff) |
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.
-rw-r--r-- | app/controllers/projects/issues_controller.rb | 2 | ||||
-rw-r--r-- | app/models/issue.rb | 13 | ||||
-rw-r--r-- | 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 |