Merge branch '17597-extract-time-tracking-system-note-service-pd' into 'master'

Extract Time Tracking System Note Service

See merge request gitlab-org/gitlab!18159

app/services/system_note_service.rb

@@ -60,9 +60,7 @@ module SystemNoteService
   #
   # Returns the created Note object
   def change_due_date(noteable, project, author, due_date)
-    body = due_date ? "changed due date to #{due_date.to_s(:long)}" : 'removed due date'
-
-    create_note(NoteSummary.new(noteable, project, author, body, action: 'due_date'))
+    ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_due_date(due_date)
   end
 
   # Called when the estimated time of a Noteable is changed
@@ -80,14 +78,7 @@ module SystemNoteService
   #
   # Returns the created Note object
   def change_time_estimate(noteable, project, author)
-    parsed_time = Gitlab::TimeTrackingFormatter.output(noteable.time_estimate)
-    body = if noteable.time_estimate == 0
-             "removed time estimate"
-           else
-             "changed time estimate to #{parsed_time}"
-           end
-
-    create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking'))
+    ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_time_estimate
   end
 
   # Called when the spent time of a Noteable is changed
@@ -105,21 +96,7 @@ module SystemNoteService
   #
   # Returns the created Note object
   def change_time_spent(noteable, project, author)
-    time_spent = noteable.time_spent
-
-    if time_spent == :reset
-      body = "removed time spent"
-    else
-      spent_at = noteable.spent_at
-      parsed_time = Gitlab::TimeTrackingFormatter.output(time_spent.abs)
-      action = time_spent > 0 ? 'added' : 'subtracted'
-
-      text_parts = ["#{action} #{parsed_time} of time spent"]
-      text_parts << "at #{spent_at}" if spent_at
-      body = text_parts.join(' ')
-    end
-
-    create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking'))
+    ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_time_spent
   end
 
   def change_status(noteable, project, author, status, source = nil)

app/services/system_notes/time_tracking_service.rb

+# frozen_string_literal: true
+
+module SystemNotes
+  class TimeTrackingService < ::SystemNotes::BaseService
+    # Called when the due_date of a Noteable is changed
+    #
+    # due_date  - Due date being assigned, or nil
+    #
+    # Example Note text:
+    #
+    #   "removed due date"
+    #
+    #   "changed due date to September 20, 2018"
+    #
+    # Returns the created Note object
+    def change_due_date(due_date)
+      body = due_date ? "changed due date to #{due_date.to_s(:long)}" : 'removed due date'
+
+      create_note(NoteSummary.new(noteable, project, author, body, action: 'due_date'))
+    end
+
+    # Called when the estimated time of a Noteable is changed
+    #
+    # time_estimate - Estimated time
+    #
+    # Example Note text:
+    #
+    #   "removed time estimate"
+    #
+    #   "changed time estimate to 3d 5h"
+    #
+    # Returns the created Note object
+    def change_time_estimate
+      parsed_time = Gitlab::TimeTrackingFormatter.output(noteable.time_estimate)
+      body = if noteable.time_estimate == 0
+               "removed time estimate"
+             else
+               "changed time estimate to #{parsed_time}"
+             end
+
+      create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking'))
+    end
+
+    # Called when the spent time of a Noteable is changed
+    #
+    # time_spent - Spent time
+    #
+    # Example Note text:
+    #
+    #   "removed time spent"
+    #
+    #   "added 2h 30m of time spent"
+    #
+    # Returns the created Note object
+    def change_time_spent
+      time_spent = noteable.time_spent
+
+      if time_spent == :reset
+        body = "removed time spent"
+      else
+        spent_at = noteable.spent_at
+        parsed_time = Gitlab::TimeTrackingFormatter.output(time_spent.abs)
+        action = time_spent > 0 ? 'added' : 'subtracted'
+
+        text_parts = ["#{action} #{parsed_time} of time spent"]
+        text_parts << "at #{spent_at}" if spent_at
+        body = text_parts.join(' ')
+      end
+
+      create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking'))
+    end
+  end
+end

spec/services/system_note_service_spec.rb

@@ -76,28 +76,14 @@ describe SystemNoteService do
   end
 
   describe '.change_due_date' do
-    subject { described_class.change_due_date(noteable, project, author, due_date) }
+    let(:due_date) { double }
 
-    let(:due_date) { Date.today }
-
-    it_behaves_like 'a note with overridable created_at'
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'due_date' }
-    end
-
-    context 'when due date added' do
-      it 'sets the note text' do
-        expect(subject.note).to eq "changed due date to #{Date.today.to_s(:long)}"
+    it 'calls TimeTrackingService' do
+      expect_next_instance_of(::SystemNotes::TimeTrackingService) do |service|
+        expect(service).to receive(:change_due_date).with(due_date)
       end
-    end
 
-    context 'when due date removed' do
-      let(:due_date) { nil }
-
-      it 'sets the note text' do
-        expect(subject.note).to eq 'removed due date'
-      end
+      described_class.change_due_date(noteable, project, author, due_date)
     end
   end
 
@@ -488,36 +474,12 @@ describe SystemNoteService do
   end
 
   describe '.change_time_estimate' do
-    subject { described_class.change_time_estimate(noteable, project, author) }
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'time_tracking' }
-    end
-
-    context 'with a time estimate' do
-      it 'sets the note text' do
-        noteable.update_attribute(:time_estimate, 277200)
-
-        expect(subject.note).to eq "changed time estimate to 1w 4d 5h"
-      end
-
-      context 'when time_tracking_limit_to_hours setting is true' do
-        before do
-          stub_application_setting(time_tracking_limit_to_hours: true)
-        end
-
-        it 'sets the note text' do
-          noteable.update_attribute(:time_estimate, 277200)
-
-          expect(subject.note).to eq "changed time estimate to 77h"
-        end
+    it 'calls TimeTrackingService' do
+      expect_next_instance_of(::SystemNotes::TimeTrackingService) do |service|
+        expect(service).to receive(:change_time_estimate)
       end
-    end
 
-    context 'without a time estimate' do
-      it 'sets the note text' do
-        expect(subject.note).to eq "removed time estimate"
-      end
+      described_class.change_time_estimate(noteable, project, author)
     end
   end
 
@@ -548,61 +510,12 @@ describe SystemNoteService do
   end
 
   describe '.change_time_spent' do
-    # We need a custom noteable in order to the shared examples to be green.
-    let(:noteable) do
-      mr = create(:merge_request, source_project: project)
-      mr.spend_time(duration: 360000, user_id: author.id)
-      mr.save!
-      mr
-    end
-
-    subject do
-      described_class.change_time_spent(noteable, project, author)
-    end
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'time_tracking' }
-    end
-
-    context 'when time was added' do
-      it 'sets the note text' do
-        spend_time!(277200)
-
-        expect(subject.note).to eq "added 1w 4d 5h of time spent"
+    it 'calls TimeTrackingService' do
+      expect_next_instance_of(::SystemNotes::TimeTrackingService) do |service|
+        expect(service).to receive(:change_time_spent)
       end
-    end
-
-    context 'when time was subtracted' do
-      it 'sets the note text' do
-        spend_time!(-277200)
-
-        expect(subject.note).to eq "subtracted 1w 4d 5h of time spent"
-      end
-    end
-
-    context 'when time was removed' do
-      it 'sets the note text' do
-        spend_time!(:reset)
 
-        expect(subject.note).to eq "removed time spent"
-      end
-    end
-
-    context 'when time_tracking_limit_to_hours setting is true' do
-      before do
-        stub_application_setting(time_tracking_limit_to_hours: true)
-      end
-
-      it 'sets the note text' do
-        spend_time!(277200)
-
-        expect(subject.note).to eq "added 77h of time spent"
-      end
-    end
-
-    def spend_time!(seconds)
-      noteable.spend_time(duration: seconds, user_id: author.id)
-      noteable.save!
+      described_class.change_time_spent(noteable, project, author)
     end
   end
 

spec/services/system_notes/time_tracking_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::SystemNotes::TimeTrackingService do
+  let_it_be(:author)   { create(:user) }
+  let_it_be(:project)  { create(:project, :repository) }
+
+  let(:noteable) { create(:issue, project: project) }
+
+  describe '#change_due_date' do
+    subject { described_class.new(noteable: noteable, project: project, author: author).change_due_date(due_date) }
+
+    let(:due_date) { Date.today }
+
+    it_behaves_like 'a note with overridable created_at'
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'due_date' }
+    end
+
+    context 'when due date added' do
+      it 'sets the note text' do
+        expect(subject.note).to eq "changed due date to #{due_date.to_s(:long)}"
+      end
+    end
+
+    context 'when due date removed' do
+      let(:due_date) { nil }
+
+      it 'sets the note text' do
+        expect(subject.note).to eq 'removed due date'
+      end
+    end
+  end
+
+  describe '.change_time_estimate' do
+    subject { described_class.new(noteable: noteable, project: project, author: author).change_time_estimate }
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'time_tracking' }
+    end
+
+    context 'with a time estimate' do
+      it 'sets the note text' do
+        noteable.update_attribute(:time_estimate, 277200)
+
+        expect(subject.note).to eq "changed time estimate to 1w 4d 5h"
+      end
+
+      context 'when time_tracking_limit_to_hours setting is true' do
+        before do
+          stub_application_setting(time_tracking_limit_to_hours: true)
+        end
+
+        it 'sets the note text' do
+          noteable.update_attribute(:time_estimate, 277200)
+
+          expect(subject.note).to eq "changed time estimate to 77h"
+        end
+      end
+    end
+
+    context 'without a time estimate' do
+      it 'sets the note text' do
+        expect(subject.note).to eq "removed time estimate"
+      end
+    end
+  end
+
+  describe '.change_time_spent' do
+    # We need a custom noteable in order to the shared examples to be green.
+    let(:noteable) do
+      mr = create(:merge_request, source_project: project)
+      mr.spend_time(duration: 360000, user_id: author.id)
+      mr.save!
+      mr
+    end
+
+    subject do
+      described_class.new(noteable: noteable, project: project, author: author).change_time_spent
+    end
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'time_tracking' }
+    end
+
+    context 'when time was added' do
+      it 'sets the note text' do
+        spend_time!(277200)
+
+        expect(subject.note).to eq "added 1w 4d 5h of time spent"
+      end
+    end
+
+    context 'when time was subtracted' do
+      it 'sets the note text' do
+        spend_time!(-277200)
+
+        expect(subject.note).to eq "subtracted 1w 4d 5h of time spent"
+      end
+    end
+
+    context 'when time was removed' do
+      it 'sets the note text' do
+        spend_time!(:reset)
+
+        expect(subject.note).to eq "removed time spent"
+      end
+    end
+
+    context 'when time_tracking_limit_to_hours setting is true' do
+      before do
+        stub_application_setting(time_tracking_limit_to_hours: true)
+      end
+
+      it 'sets the note text' do
+        spend_time!(277200)
+
+        expect(subject.note).to eq "added 77h of time spent"
+      end
+    end
+
+    def spend_time!(seconds)
+      noteable.spend_time(duration: seconds, user_id: author.id)
+      noteable.save!
+    end
+  end
+end