Merge branch 'time_change' into 'master'

Add `time change` in payload for Issue, MR and Note webhook

See merge request gitlab-org/gitlab!55087

app/models/concerns/issuable.rb

@@ -468,9 +468,11 @@ module Issuable
 
       if self.respond_to?(:total_time_spent)
         old_total_time_spent = old_associations.fetch(:total_time_spent, total_time_spent)
+        old_time_change = old_associations.fetch(:time_change, time_change)
 
         if old_total_time_spent != total_time_spent
           changes[:total_time_spent] = [old_total_time_spent, total_time_spent]
+          changes[:time_change] = [old_time_change, time_change]
         end
       end
     end

app/models/concerns/time_trackable.rb

@@ -33,11 +33,11 @@ module TimeTrackable
 
     return if @time_spent == 0
 
-    if @time_spent == :reset
-      reset_spent_time
-    else
-      add_or_subtract_spent_time
-    end
+    @timelog = if @time_spent == :reset
+                 reset_spent_time
+               else
+                 add_or_subtract_spent_time
+               end
   end
   alias_method :spend_time=, :spend_time
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
@@ -50,6 +50,14 @@ module TimeTrackable
     Gitlab::TimeTrackingFormatter.output(total_time_spent)
   end
 
+  def time_change
+    @timelog&.time_spent.to_i # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  end
+
+  def human_time_change
+    Gitlab::TimeTrackingFormatter.output(time_change)
+  end
+
   def human_time_estimate
     Gitlab::TimeTrackingFormatter.output(time_estimate)
   end

app/services/issuable_base_service.rb

@@ -433,6 +433,7 @@ class IssuableBaseService < ::BaseProjectService
         milestone: issuable.try(:milestone)
       }
     associations[:total_time_spent] = issuable.total_time_spent if issuable.respond_to?(:total_time_spent)
+    associations[:time_change] = issuable.time_change if issuable.respond_to?(:time_change)
     associations[:description] = issuable.description
     associations[:reviewers] = issuable.reviewers.to_a if issuable.allows_reviewers?
 

doc/user/project/integrations/webhooks.md

@@ -308,8 +308,10 @@ X-Gitlab-Event: Issue Hook
     "duplicated_to_id": null,
     "time_estimate": 0,
     "total_time_spent": 0,
+    "time_change": 0,
     "human_total_time_spent": null,
     "human_time_estimate": null,
+    "human_time_change": null,
     "weight": null,
     "iid": 23,
     "url": "http://example.com/diaspora/issues/23",

lib/gitlab/hook_data/issue_builder.rb

@@ -7,6 +7,7 @@ module Gitlab
         assignees
         labels
         total_time_spent
+        time_change
       ].freeze
 
       def self.safe_hook_attributes
@@ -43,7 +44,9 @@ module Gitlab
             description: absolute_image_urls(issue.description),
             url: Gitlab::UrlBuilder.build(issue),
             total_time_spent: issue.total_time_spent,
+            time_change: issue.time_change,
             human_total_time_spent: issue.human_total_time_spent,
+            human_time_change: issue.human_time_change,
             human_time_estimate: issue.human_time_estimate,
             assignee_ids: issue.assignee_ids,
             assignee_id: issue.assignee_ids.first, # This key is deprecated

lib/gitlab/hook_data/merge_request_builder.rb

@@ -37,6 +37,7 @@ module Gitlab
         assignees
         labels
         total_time_spent
+        time_change
       ].freeze
 
       alias_method :merge_request, :object
@@ -50,7 +51,9 @@ module Gitlab
           last_commit: merge_request.diff_head_commit&.hook_attrs,
           work_in_progress: merge_request.work_in_progress?,
           total_time_spent: merge_request.total_time_spent,
+          time_change: merge_request.time_change,
           human_total_time_spent: merge_request.human_total_time_spent,
+          human_time_change: merge_request.human_time_change,
           human_time_estimate: merge_request.human_time_estimate,
           assignee_ids: merge_request.assignee_ids,
           assignee_id: merge_request.assignee_ids.first, # This key is deprecated

lib/gitlab/time_tracking_formatter.rb

@@ -24,6 +24,12 @@ module Gitlab
     end
 
     def output(seconds)
+      seconds.to_i < 0 ? negative_output(seconds) : positive_output(seconds)
+    end
+
+    private
+
+    def positive_output(seconds)
       ChronicDuration.output(
         seconds,
         CUSTOM_DAY_AND_MONTH_LENGTH.merge(
@@ -34,7 +40,9 @@ module Gitlab
       nil
     end
 
-    private
+    def negative_output(seconds)
+      "-" + positive_output(seconds.abs)
+    end
 
     def limit_to_hours_setting
       Gitlab::CurrentSettings.time_tracking_limit_to_hours

spec/lib/gitlab/hook_data/issue_builder_spec.rb

@@ -42,8 +42,10 @@ RSpec.describe Gitlab::HookData::IssueBuilder do
 
     it 'includes additional attrs' do
       expect(data).to include(:total_time_spent)
+      expect(data).to include(:time_change)
       expect(data).to include(:human_time_estimate)
       expect(data).to include(:human_total_time_spent)
+      expect(data).to include(:human_time_change)
       expect(data).to include(:assignee_ids)
       expect(data).to include(:state)
       expect(data).to include('labels' => [label.hook_attrs])

spec/lib/gitlab/hook_data/merge_request_builder_spec.rb

@@ -57,8 +57,10 @@ RSpec.describe Gitlab::HookData::MergeRequestBuilder do
       expect(data).to include(:last_commit)
       expect(data).to include(:work_in_progress)
       expect(data).to include(:total_time_spent)
+      expect(data).to include(:time_change)
       expect(data).to include(:human_time_estimate)
       expect(data).to include(:human_total_time_spent)
+      expect(data).to include(:human_time_change)
     end
 
     context 'when the MR has an image in the description' do

spec/lib/gitlab/time_tracking_formatter_spec.rb

@@ -47,5 +47,11 @@ RSpec.describe Gitlab::TimeTrackingFormatter do
 
       it { expect(subject).to eq('1w 1d 1h 40m') }
     end
+
+    context 'handles negative time input' do
+      let(:num_seconds) { -178_800 }
+
+      it { expect(subject).to eq('-1w 1d 1h 40m') }
+    end
   end
 end

spec/models/concerns/issuable_spec.rb

@@ -715,6 +715,12 @@ RSpec.describe Issuable do
         expect(issue.total_time_spent).to eq(1800)
       end
 
+      it 'stores the time change' do
+        spend_time(1800)
+
+        expect(issue.time_change).to eq(1800)
+      end
+
       it 'updates issues updated_at' do
         issue
 
@@ -735,6 +741,12 @@ RSpec.describe Issuable do
         expect(issue.total_time_spent).to eq(900)
       end
 
+      it 'stores negative time change' do
+        spend_time(-900)
+
+        expect(issue.time_change).to eq(-900)
+      end
+
       context 'when time to subtract exceeds the total time spent' do
         it 'raise a validation error' do
           Timecop.travel(1.minute.from_now) do

spec/services/merge_requests/update_service_spec.rb

@@ -297,6 +297,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
               reviewers: [],
               milestone: nil,
               total_time_spent: 0,
+              time_change: 0,
               description: "FYI #{user2.to_reference}"
             }
           )