From 0ae48e068794a732f728de76ffbf3f95e702ba9a Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Wed, 3 Oct 2018 17:45:39 -0300 Subject: [PATCH] Move issue related_branches to service Moves the related_branches method from Issue model to RelatedBranchesService --- app/controllers/projects/issues_controller.rb | 2 +- app/models/issue.rb | 18 --------- .../issues/related_branches_service.rb | 26 +++++++++++++ spec/models/issue_spec.rb | 39 ------------------- .../issues/related_branches_service_spec.rb | 39 +++++++++++++++++++ 5 files changed, 66 insertions(+), 58 deletions(-) create mode 100644 app/services/issues/related_branches_service.rb create mode 100644 spec/services/issues/related_branches_service_spec.rb diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 4e859de6fde..b06a6f3bb0d 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/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 diff --git a/app/models/issue.rb b/app/models/issue.rb index d13fbcf002c..4ace5d3ab97 100644 --- a/app/models/issue.rb +++ b/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) diff --git a/app/services/issues/related_branches_service.rb b/app/services/issues/related_branches_service.rb new file mode 100644 index 00000000000..76af482b7ac --- /dev/null +++ b/app/services/issues/related_branches_service.rb @@ -0,0 +1,26 @@ +# 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 diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 19bc2713ef5..6f900a60213 100644 --- a/spec/models/issue_spec.rb +++ b/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 } diff --git a/spec/services/issues/related_branches_service_spec.rb b/spec/services/issues/related_branches_service_spec.rb new file mode 100644 index 00000000000..c2e1eba6a63 --- /dev/null +++ b/spec/services/issues/related_branches_service_spec.rb @@ -0,0 +1,39 @@ +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