Extract Merge Train System Note Service

Also refactoring specs

Apply review suggestions.

ee/app/services/ee/system_note_service.rb

@@ -195,56 +195,32 @@ module EE
 
     # Called when 'merge train' is executed
     def merge_train(noteable, project, author, merge_train)
-      index = merge_train.index
-
-      body = if index == 0
-               'started a merge train'
-             else
-               "added this merge request to the merge train at position #{index + 1}"
-             end
-
-      create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      EE::SystemNotes::MergeTrainService.new(noteable: noteable, project: project, author: author).enqueue(merge_train)
     end
 
     # Called when 'merge train' is canceled
     def cancel_merge_train(noteable, project, author)
-      body = 'removed this merge request from the merge train'
-
-      create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      EE::SystemNotes::MergeTrainService.new(noteable: noteable, project: project, author: author).cancel
     end
 
     # Called when 'merge train' is aborted
     def abort_merge_train(noteable, project, author, reason)
-      body = "removed this merge request from the merge train because #{reason}"
-
-      ##
-      # TODO: Abort message should be sent by the system, not a particular user.
-      # See https://gitlab.com/gitlab-org/gitlab-foss/issues/63187.
-      create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      EE::SystemNotes::MergeTrainService.new(noteable: noteable, project: project, author: author).abort(reason)
     end
 
     # Called when 'add to merge train when pipeline succeeds' is executed
     def add_to_merge_train_when_pipeline_succeeds(noteable, project, author, sha)
-      body = "enabled automatic add to merge train when the pipeline for #{sha} succeeds"
-
-      create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      EE::SystemNotes::MergeTrainService.new(noteable: noteable, project: project, author: author).add_when_pipeline_succeeds(sha)
     end
 
     # Called when 'add to merge train when pipeline succeeds' is canceled
     def cancel_add_to_merge_train_when_pipeline_succeeds(noteable, project, author)
-      body = 'cancelled automatic add to merge train'
-
-      create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      EE::SystemNotes::MergeTrainService.new(noteable: noteable, project: project, author: author).cancel_add_when_pipeline_succeeds
     end
 
     # Called when 'add to merge train when pipeline succeeds' is aborted
     def abort_add_to_merge_train_when_pipeline_succeeds(noteable, project, author, reason)
-      body = "aborted automatic add to merge train because #{reason}"
-
-      ##
-      # TODO: Abort message should be sent by the system, not a particular user.
-      # See https://gitlab.com/gitlab-org/gitlab-foss/issues/63187.
-      create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      EE::SystemNotes::MergeTrainService.new(noteable: noteable, project: project, author: author).abort_add_when_pipeline_succeeds(reason)
     end
 
     def auto_resolve_prometheus_alert(noteable, project, author)

ee/app/services/ee/system_notes/merge_train_service.rb

+# frozen_string_literal: true
+
+module EE
+  module SystemNotes
+    class MergeTrainService < ::SystemNotes::BaseService
+      # Called when 'merge train' is executed
+      def enqueue(merge_train)
+        index = merge_train.index
+
+        body = if index == 0
+                 'started a merge train'
+               else
+                 "added this merge request to the merge train at position #{index + 1}"
+               end
+
+        create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      end
+
+      # Called when 'merge train' is canceled
+      def cancel
+        body = 'removed this merge request from the merge train'
+
+        create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      end
+
+      # Called when 'merge train' is aborted
+      def abort(reason)
+        body = "removed this merge request from the merge train because #{reason}"
+
+        ##
+        # TODO: Abort message should be sent by the system, not a particular user.
+        # See https://gitlab.com/gitlab-org/gitlab/issues/29467.
+        create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      end
+
+      # Called when 'add to merge train when pipeline succeeds' is executed
+      def add_when_pipeline_succeeds(sha)
+        body = "enabled automatic add to merge train when the pipeline for #{sha} succeeds"
+
+        create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      end
+
+      # Called when 'add to merge train when pipeline succeeds' is canceled
+      def cancel_add_when_pipeline_succeeds
+        body = 'cancelled automatic add to merge train'
+
+        create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      end
+
+      # Called when 'add to merge train when pipeline succeeds' is aborted
+      def abort_add_when_pipeline_succeeds(reason)
+        body = "aborted automatic add to merge train because #{reason}"
+
+        ##
+        # TODO: Abort message should be sent by the system, not a particular user.
+        # See https://gitlab.com/gitlab-org/gitlab/issues/29467.
+        create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
+      end
+    end
+  end
+end

ee/spec/services/ee/system_notes/merge_train_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EE::SystemNotes::MergeTrainService do
+  let_it_be(:author) { create(:user) }
+  let_it_be(:project) { create(:project) }
+
+  describe '#enqueue' do
+    subject { described_class.new(noteable: noteable, project: project, author: author).enqueue(noteable.merge_train) }
+
+    let(:noteable) { create(:merge_request, :on_train, source_project: project, target_project: project) }
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'merge' }
+    end
+
+    it "posts the 'merge train' system note" do
+      expect(subject.note).to eq('started a merge train')
+    end
+
+    context 'when index of the merge request is not zero' do
+      before do
+        allow(noteable.merge_train).to receive(:index) { 1 }
+      end
+
+      it "posts the 'merge train' system note" do
+        expect(subject.note).to eq('added this merge request to the merge train at position 2')
+      end
+    end
+  end
+
+  describe '#cancel' do
+    subject { described_class.new(noteable: noteable, project: project, author: author).cancel }
+
+    let(:noteable) { create(:merge_request, :on_train, source_project: project, target_project: project) }
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'merge' }
+    end
+
+    it "posts the 'merge train' system note" do
+      expect(subject.note).to eq('removed this merge request from the merge train')
+    end
+  end
+
+  describe '#abort' do
+    subject { described_class.new(noteable: noteable, project: project, author: author).abort('source branch was updated') }
+
+    let(:noteable) { create(:merge_request, :on_train, source_project: project, target_project: project) }
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'merge' }
+    end
+
+    it "posts the 'merge train' system note" do
+      expect(subject.note).to eq('removed this merge request from the merge train because source branch was updated')
+    end
+  end
+
+  describe '#add_when_pipeline_succeeds' do
+    subject { described_class.new(noteable: noteable, project: project, author: author).add_when_pipeline_succeeds(pipeline.sha) }
+
+    let(:pipeline) { build(:ci_pipeline) }
+
+    let(:noteable) do
+      create(:merge_request, source_project: project, target_project: project)
+    end
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'merge' }
+    end
+
+    it "posts the 'add to merge train when pipeline succeeds' system note" do
+      expect(subject.note).to match(%r{enabled automatic add to merge train when the pipeline for (\w+/\w+@)?\h{40} succeeds})
+    end
+  end
+
+  describe '#cancel_add_when_pipeline_succeeds' do
+    subject { described_class.new(noteable: noteable, project: project, author: author).cancel_add_when_pipeline_succeeds }
+
+    let(:noteable) do
+      create(:merge_request, source_project: project, target_project: project)
+    end
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'merge' }
+    end
+
+    it "posts the 'add to merge train when pipeline succeeds' system note" do
+      expect(subject.note).to eq 'cancelled automatic add to merge train'
+    end
+  end
+
+  describe '#abort_add_when_pipeline_succeeds' do
+    subject { described_class.new(noteable: noteable, project: project, author: author).abort_add_when_pipeline_succeeds('target branch was changed') }
+
+    let(:noteable) do
+      create(:merge_request, source_project: project, target_project: project)
+    end
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'merge' }
+    end
+
+    it "posts the 'add to merge train when pipeline succeeds' system note" do
+      expect(subject.note).to eq 'aborted automatic add to merge train because target branch was changed'
+    end
+  end
+end

ee/spec/services/system_note_service_spec.rb

@@ -297,106 +297,70 @@ describe SystemNoteService do
   end
 
   describe '.merge_train' do
-    subject { described_class.merge_train(noteable, project, author, noteable.merge_train) }
+    let(:merge_train) { double }
 
-    let(:noteable) { create(:merge_request, :on_train, source_project: project, target_project: project) }
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'merge' }
-    end
-
-    it "posts the 'merge train' system note" do
-      expect(subject.note).to eq('started a merge train')
-    end
-
-    context 'when index of the merge request is not zero' do
-      before do
-        allow(noteable.merge_train).to receive(:index) { 1 }
+    it 'calls MergeTrainService' do
+      expect_next_instance_of(EE::SystemNotes::MergeTrainService) do |service|
+        expect(service).to receive(:enqueue).with(merge_train)
       end
 
-      it "posts the 'merge train' system note" do
-        expect(subject.note).to eq('added this merge request to the merge train at position 2')
-      end
+      described_class.merge_train(noteable, project, author, merge_train)
     end
   end
 
   describe '.cancel_merge_train' do
-    subject { described_class.cancel_merge_train(noteable, project, author) }
-
-    let(:noteable) { create(:merge_request, :on_train, source_project: project, target_project: project) }
-    let(:reason) { }
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'merge' }
-    end
+    it 'calls MergeTrainService' do
+      expect_next_instance_of(EE::SystemNotes::MergeTrainService) do |service|
+        expect(service).to receive(:cancel)
+      end
 
-    it "posts the 'merge train' system note" do
-      expect(subject.note).to eq('removed this merge request from the merge train')
+      described_class.cancel_merge_train(noteable, project, author)
     end
   end
 
   describe '.abort_merge_train' do
-    subject { described_class.abort_merge_train(noteable, project, author, 'source branch was updated') }
+    let(:message) { double }
 
-    let(:noteable) { create(:merge_request, :on_train, source_project: project, target_project: project) }
-    let(:reason) { }
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'merge' }
-    end
+    it 'calls MergeTrainService' do
+      expect_next_instance_of(EE::SystemNotes::MergeTrainService) do |service|
+        expect(service).to receive(:abort).with(message)
+      end
 
-    it "posts the 'merge train' system note" do
-      expect(subject.note).to eq('removed this merge request from the merge train because source branch was updated')
+      described_class.abort_merge_train(noteable, project, author, message)
     end
   end
 
   describe '.add_to_merge_train_when_pipeline_succeeds' do
-    subject { described_class.add_to_merge_train_when_pipeline_succeeds(noteable, project, author, pipeline.sha) }
+    let(:sha) { double }
 
-    let(:pipeline) { build(:ci_pipeline) }
-
-    let(:noteable) do
-      create(:merge_request, source_project: project, target_project: project)
-    end
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'merge' }
-    end
+    it 'calls MergeTrainService' do
+      expect_next_instance_of(EE::SystemNotes::MergeTrainService) do |service|
+        expect(service).to receive(:add_when_pipeline_succeeds).with(sha)
+      end
 
-    it "posts the 'add to merge train when pipeline succeeds' system note" do
-      expect(subject.note).to match(%r{enabled automatic add to merge train when the pipeline for (\w+/\w+@)?\h{40} succeeds})
+      described_class.add_to_merge_train_when_pipeline_succeeds(noteable, project, author, sha)
     end
   end
 
   describe '.cancel_add_to_merge_train_when_pipeline_succeeds' do
-    subject { described_class.cancel_add_to_merge_train_when_pipeline_succeeds(noteable, project, author) }
-
-    let(:noteable) do
-      create(:merge_request, source_project: project, target_project: project)
-    end
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'merge' }
-    end
+    it 'calls MergeTrainService' do
+      expect_next_instance_of(EE::SystemNotes::MergeTrainService) do |service|
+        expect(service).to receive(:cancel_add_when_pipeline_succeeds)
+      end
 
-    it "posts the 'add to merge train when pipeline succeeds' system note" do
-      expect(subject.note).to eq 'cancelled automatic add to merge train'
+      described_class.cancel_add_to_merge_train_when_pipeline_succeeds(noteable, project, author)
     end
   end
 
   describe '.abort_add_to_merge_train_when_pipeline_succeeds' do
-    subject { described_class.abort_add_to_merge_train_when_pipeline_succeeds(noteable, project, author, 'target branch was changed') }
+    let(:message) { double }
 
-    let(:noteable) do
-      create(:merge_request, source_project: project, target_project: project)
-    end
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'merge' }
-    end
+    it 'calls MergeTrainService' do
+      expect_next_instance_of(EE::SystemNotes::MergeTrainService) do |service|
+        expect(service).to receive(:abort_add_when_pipeline_succeeds).with(message)
+      end
 
-    it "posts the 'add to merge train when pipeline succeeds' system note" do
-      expect(subject.note).to eq 'aborted automatic add to merge train because target branch was changed'
+      described_class.abort_add_to_merge_train_when_pipeline_succeeds(noteable, project, author, message)
     end
   end
 end