Move logic for time tracking to new TimeTrackable module.

I've also applied other minor refactorings and fixed a broken spec.

app/models/concerns/issuable.rb

@@ -13,6 +13,7 @@ module Issuable
   include StripAttribute
   include Awardable
   include Taskable
+  include TimeTrackable
 
   included do
     cache_markdown_field :title, pipeline: :single_line
@@ -314,12 +315,4 @@ module Issuable
     metrics = self.metrics || create_metrics
     metrics.record!
   end
-
-  def time_spent
-    timelogs.sum(:time_spent)
-  end
-
-  def spend_time(seconds)
-    timelogs.new(time_spent: seconds)
-  end
 end

app/models/concerns/time_trackable.rb

+# == TimeTrackable concern
+#
+# Contains functionality related to objects that support time tracking.
+#
+# Used by Issue and MergeRequest.
+#
+
+module TimeTrackable
+  extend ActiveSupport::Concern
+
+  included do
+    attr_reader :time_spent
+
+    alias_method :time_spent?, :time_spent
+  end
+
+  def spend_time=(seconds)
+    return unless seconds
+
+    timelogs.new(time_spent: seconds)
+    @time_spent = seconds
+  end
+
+  def total_time_spent
+    timelogs.sum(:time_spent)
+  end
+end

app/services/issuable_base_service.rb

@@ -40,9 +40,8 @@ class IssuableBaseService < BaseService
     SystemNoteService.change_time_estimate(issuable, issuable.project, current_user)
   end
 
-  def create_time_spent_note(issuable, time_spent)
-    SystemNoteService.change_time_spent(
-      issuable, issuable.project, current_user, time_spent)
+  def create_time_spent_note(issuable)
+    SystemNoteService.change_time_spent(issuable, issuable.project, current_user)
   end
 
   def filter_params(issuable_ability_name = :issue)
@@ -146,7 +145,6 @@ class IssuableBaseService < BaseService
   def create(issuable)
     merge_slash_commands_into_params!(issuable)
     filter_params
-    change_spent_time(issuable)
 
     params.delete(:state_event)
     params[:author] ||= current_user
@@ -187,7 +185,6 @@ class IssuableBaseService < BaseService
     change_state(issuable)
     change_subscription(issuable)
     change_todo(issuable)
-    change_spent_time(issuable)
     filter_params
     old_labels = issuable.labels.to_a
     old_mentioned_users = issuable.mentioned_users.to_a
@@ -239,16 +236,6 @@ class IssuableBaseService < BaseService
     end
   end
 
-  def change_spent_time(issuable)
-    if time_spent
-      issuable.spend_time(time_spent)
-    end
-  end
-
-  def time_spent
-    @time_spent ||= params.delete(:time_spent)
-  end
-
   def has_changes?(issuable, old_labels: [])
     valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch]
 
@@ -274,8 +261,8 @@ class IssuableBaseService < BaseService
       create_time_estimate_note(issuable)
     end
 
-    if time_spent
-      create_time_spent_note(issuable, time_spent)
+    if issuable.time_spent?
+      create_time_spent_note(issuable)
     end
 
     create_labels_note(issuable, old_labels) if issuable.labels != old_labels

app/services/slash_commands/interpret_service.rb

@@ -254,7 +254,11 @@ module SlashCommands
       current_user.can?(:"admin_#{issuable.to_ability_name}", project)
     end
     command :estimate do |raw_duration|
-      @updates[:time_estimate] = ChronicDuration.parse(raw_duration, default_unit: 'hours')
+      time_spent = ChronicDuration.parse(raw_duration, default_unit: 'hours')
+
+      if time_spent
+        @updates[:time_estimate] = time_spent
+      end
     end
 
     desc 'Add or substract spent time'
@@ -264,10 +268,11 @@ module SlashCommands
     end
     command :spend do |raw_duration|
       reduce_time = raw_duration.sub!(/\A-/, '')
-      time_spent  = ChronicDuration.parse(raw_duration, default_unit: 'hours')
-      time_spent  *= -1 if reduce_time
+      time_spent = ChronicDuration.parse(raw_duration, default_unit: 'hours')
 
-      @updates[:time_spent] = time_spent
+      if time_spent
+        @updates[:spend_time] = reduce_time ? (time_spent * -1) : time_spent
+      end
     end
 
     def find_label_ids(labels_param)

app/services/system_note_service.rb

@@ -144,10 +144,11 @@ module SystemNoteService
   #
   # Returns the created Note object
 
-  def change_time_spent(noteable, project, author, time_spent)
+  def change_time_spent(noteable, project, author)
+    time_spent  = noteable.time_spent
     parsed_time = ChronicDuration.output(time_spent.abs, format: :short)
-    action = time_spent > 0 ? 'Added' : 'Substracted'
-    body = "#{action} #{parsed_time} of time spent on this #{noteable.human_class_name}"
+    action      = time_spent > 0 ? 'Added' : 'Substracted'
+    body        = "#{action} #{parsed_time} of time spent on this #{noteable.human_class_name}"
 
     create_note(noteable: noteable, project: project, author: author, note: body)
   end

spec/models/timelog_spec.rb

@@ -6,5 +6,4 @@ RSpec.describe Timelog, type: :model do
   it { is_expected.to be_valid }
 
   it { is_expected.to validate_presence_of(:time_spent) }
-  it { is_expected.to validate_presence_of(:trackable) }
 end

spec/services/slash_commands/interpret_service_spec.rb

@@ -219,10 +219,10 @@ describe SlashCommands::InterpretService, services: true do
     end
 
     shared_examples 'spend command' do
-      it 'populates time_spent: "3600" if content contains /spend 1h' do
+      it 'populates spend_time: "3600" if content contains /spend 1h' do
         _, updates = service.execute(content, issuable)
 
-        expect(updates).to eq(time_spent: 3600)
+        expect(updates).to eq(spend_time: 3600)
       end
     end