Merge branch 'issue_43097' into 'master'

Move related branches to service

Closes #43097

See merge request gitlab-org/gitlab-ce!22094

app/controllers/projects/issues_controller.rb

@@ -127,7 +127,7 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def related_branches
-    @related_branches = @issue.related_branches(current_user)
+    @related_branches = Issues::RelatedBranchesService.new(project, current_user).execute(issue)
 
     respond_to do |format|
       format.json do

app/models/issue.rb

@@ -170,24 +170,6 @@ class Issue < ActiveRecord::Base
     "#{project.to_reference(from, full: full)}#{reference}"
   end
 
-  # All branches containing the current issue's ID, except for
-  # those with a merge request open referencing the current issue.
-  # rubocop: disable CodeReuse/ServiceClass
-  def related_branches(current_user)
-    branches_with_iid = project.repository.branch_names.select do |branch|
-      branch =~ /\A#{iid}-(?!\d+-stable)/i
-    end
-
-    branches_with_merge_request =
-      Issues::ReferencedMergeRequestsService
-        .new(project, current_user)
-        .referenced_merge_requests(self)
-        .map(&:source_branch)
-
-    branches_with_iid - branches_with_merge_request
-  end
-  # rubocop: enable CodeReuse/ServiceClass
-
   def suggested_branch_name
     return to_branch_name unless project.repository.branch_exists?(to_branch_name)
 

app/services/issues/related_branches_service.rb

+# frozen_string_literal: true
+
+# This service fetches all branches containing the current issue's ID, except for
+# those with a merge request open referencing the current issue.
+module Issues
+  class RelatedBranchesService < Issues::BaseService
+    def execute(issue)
+      branches_with_iid_of(issue) - branches_with_merge_request_for(issue)
+    end
+
+    private
+
+    def branches_with_merge_request_for(issue)
+      Issues::ReferencedMergeRequestsService
+        .new(project, current_user)
+        .referenced_merge_requests(issue)
+        .map(&:source_branch)
+    end
+
+    def branches_with_iid_of(issue)
+      project.repository.branch_names.select do |branch|
+        branch =~ /\A#{issue.iid}-(?!\d+-stable)/i
+      end
+    end
+  end
+end

spec/models/issue_spec.rb

@@ -268,45 +268,6 @@ describe Issue do
     end
   end
 
-  describe '#related_branches' do
-    let(:user) { create(:admin) }
-
-    before do
-      allow(subject.project.repository).to receive(:branch_names)
-                                            .and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name, "#{subject.iid}-branch"])
-
-      # Without this stub, the `create(:merge_request)` above fails because it can't find
-      # the source branch. This seems like a reasonable compromise, in comparison with
-      # setting up a full repo here.
-      allow_any_instance_of(MergeRequest).to receive(:create_merge_request_diff)
-    end
-
-    it "selects the right branches when there are no referenced merge requests" do
-      expect(subject.related_branches(user)).to eq([subject.to_branch_name, "#{subject.iid}-branch"])
-    end
-
-    it "selects the right branches when there is a referenced merge request" do
-      merge_request = create(:merge_request, { description: "Closes ##{subject.iid}",
-                                               source_project: subject.project,
-                                               source_branch: "#{subject.iid}-branch" })
-      merge_request.create_cross_references!(user)
-
-      referenced_merge_requests = Issues::ReferencedMergeRequestsService
-                                    .new(subject.project, user)
-                                    .referenced_merge_requests(subject)
-
-      expect(referenced_merge_requests).not_to be_empty
-      expect(subject.related_branches(user)).to eq([subject.to_branch_name])
-    end
-
-    it 'excludes stable branches from the related branches' do
-      allow(subject.project.repository).to receive(:branch_names)
-        .and_return(["#{subject.iid}-0-stable"])
-
-      expect(subject.related_branches(user)).to eq []
-    end
-  end
-
   describe '#suggested_branch_name' do
     let(:repository) { double }
 

spec/services/issues/related_branches_service_spec.rb

+require 'spec_helper'
+
+describe Issues::RelatedBranchesService do
+  let(:user) { create(:admin) }
+  let(:issue) { create(:issue) }
+
+  subject { described_class.new(issue.project, user) }
+
+  describe '#execute' do
+    before do
+      allow(issue.project.repository).to receive(:branch_names).and_return(["mpempe", "#{issue.iid}mepmep", issue.to_branch_name, "#{issue.iid}-branch"])
+    end
+
+    it "selects the right branches when there are no referenced merge requests" do
+      expect(subject.execute(issue)).to eq([issue.to_branch_name, "#{issue.iid}-branch"])
+    end
+
+    it "selects the right branches when there is a referenced merge request" do
+      merge_request = create(:merge_request, { description: "Closes ##{issue.iid}",
+                                               source_project: issue.project,
+                                               source_branch: "#{issue.iid}-branch" })
+      merge_request.create_cross_references!(user)
+
+      referenced_merge_requests = Issues::ReferencedMergeRequestsService
+                                    .new(issue.project, user)
+                                    .referenced_merge_requests(issue)
+
+      expect(referenced_merge_requests).not_to be_empty
+      expect(subject.execute(issue)).to eq([issue.to_branch_name])
+    end
+
+    it 'excludes stable branches from the related branches' do
+      allow(issue.project.repository).to receive(:branch_names)
+        .and_return(["#{issue.iid}-0-stable"])
+
+      expect(subject.execute(issue)).to eq []
+    end
+  end
+end