Fix `updated_at` doesn't apply to `state_event` updates of issues via API

9 files changed, 67 insertions, 10 deletions

diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb
index 3c74034b527..423ce7e7db1 100644
--- a/app/models/concerns/noteable.rb
+++ b/app/models/concerns/noteable.rb
@@ -13,6 +13,14 @@ module Noteable
     end
   end
 
+  # The timestamp of the note (e.g. the :updated_at attribute if provided via
+  # API call)
+  def system_note_timestamp
+    @system_note_timestamp || Time.now # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  end
+
+  attr_writer :system_note_timestamp
+
   def base_class_name
     self.class.base_class.name
   end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 97c6dcc4745..261935fd054 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -90,7 +90,7 @@ class Issue < ApplicationRecord
     state :closed
 
     before_transition any => :closed do |issue|
-      issue.closed_at = Time.zone.now
+      issue.closed_at = issue.system_note_timestamp
     end
 
     before_transition closed: :opened do |issue|
diff --git a/app/services/note_summary.rb b/app/services/note_summary.rb
index 81f6f92f75c..60a68568833 100644
--- a/app/services/note_summary.rb
+++ b/app/services/note_summary.rb
@@ -5,7 +5,9 @@ class NoteSummary
   attr_reader :metadata
 
   def initialize(noteable, project, author, body, action: nil, commit_count: nil)
-    @note = { noteable: noteable, project: project, author: author, note: body }
+    @note = { noteable: noteable,
+              created_at: noteable.system_note_timestamp,
+              project: project, author: author, note: body }
     @metadata = { action: action, commit_count: commit_count }.compact
 
     set_commit_params if note[:noteable].is_a?(Commit)
diff --git a/app/services/resource_events/change_labels_service.rb b/app/services/resource_events/change_labels_service.rb
index 039d6e2ebad..b45e567079b 100644
--- a/app/services/resource_events/change_labels_service.rb
+++ b/app/services/resource_events/change_labels_service.rb
@@ -12,7 +12,7 @@ module ResourceEvents
       label_hash = {
         resource_column(resource) => resource.id,
         user_id: user.id,
-        created_at: Time.now
+        created_at: resource.system_note_timestamp
       }
 
       labels = added_labels.map do |label|
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index ea8ac7e4656..acbbb0da929 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -258,7 +258,7 @@ module SystemNoteService
     body = "created #{issue.to_reference} to continue this discussion"
     note_attributes = discussion.reply_attributes.merge(project: project, author: author, note: body)
 
-    note = Note.create(note_attributes.merge(system: true))
+    note = Note.create(note_attributes.merge(system: true, created_at: issue.system_note_timestamp))
     note.system_note_metadata = SystemNoteMetadata.new(action: 'discussion')
 
     note
diff --git a/changelogs/unreleased/53279-fix-updated_at-api.yml b/changelogs/unreleased/53279-fix-updated_at-api.yml
new file mode 100644
index 00000000000..c64dada7eaa
--- /dev/null
+++ b/changelogs/unreleased/53279-fix-updated_at-api.yml
@@ -0,0 +1,5 @@
+---
+title: "Respect updated_at attribute in notes produced by API calls"
+merge_request: 27124
+author: Ben Gamari
+type: fixed
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 999a9cb5a82..000c00ea9f9 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -230,9 +230,13 @@ module API
         issue = user_project.issues.find_by!(iid: params.delete(:issue_iid))
         authorize! :update_issue, issue
 
-        # Setting created_at time only allowed for admins and project/group owners
-        unless current_user.admin? || user_project.owner == current_user || current_user.owned_groups.include?(user_project.owner)
-          params.delete(:updated_at)
+        # Setting updated_at only allowed for admins and owners as well
+        if params[:updated_at].present?
+          if current_user.admin? || user_project.owner == current_user || current_user.owned_groups.include?(user_project.owner)
+            issue.system_note_timestamp = params[:updated_at]
+          else
+            params.delete(:updated_at)
+          end
         end
 
         update_params = declared_params(include_missing: false).merge(request: request, api: true)
diff --git a/spec/services/note_summary_spec.rb b/spec/services/note_summary_spec.rb
index a6cc2251e48..f6ee15f750c 100644
--- a/spec/services/note_summary_spec.rb
+++ b/spec/services/note_summary_spec.rb
@@ -21,16 +21,20 @@ describe NoteSummary do
 
   describe '#note' do
     it 'returns note hash' do
-      expect(create_note_summary.note).to eq(noteable: noteable, project: project, author: user, note: 'note')
+      Timecop.freeze do
+        expect(create_note_summary.note).to eq(noteable: noteable, project: project, author: user, note: 'note',
+                                               created_at: Time.now)
+      end
     end
 
     context 'when noteable is a commit' do
-      let(:noteable) { build(:commit) }
+      let(:noteable) { build(:commit, system_note_timestamp: Time.at(43)) }
 
       it 'returns note hash specific to commit' do
         expect(create_note_summary.note).to eq(
           noteable: nil, project: project, author: user, note: 'note',
-          noteable_type: 'Commit', commit_id: noteable.id
+          noteable_type: 'Commit', commit_id: noteable.id,
+          created_at: Time.at(43)
         )
       end
     end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index b917de14b2e..8d446d1c9d5 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -11,6 +11,14 @@ describe SystemNoteService do
   let(:noteable) { create(:issue, project: project) }
   let(:issue)    { noteable }
 
+  shared_examples_for 'a note with overridable created_at' do
+    let(:noteable) { create(:issue, project: project, system_note_timestamp: Time.at(42)) }
+
+    it 'the note has the correct time' do
+      expect(subject.created_at).to eq Time.at(42)
+    end
+  end
+
   shared_examples_for 'a system note' do
     let(:expected_noteable) { noteable }
     let(:commit_count)      { nil }
@@ -137,6 +145,8 @@ describe SystemNoteService do
     end
 
     context 'when assignee added' do
+      it_behaves_like 'a note with overridable created_at'
+
       it 'sets the note text' do
         expect(subject.note).to eq "assigned to @#{assignee.username}"
       end
@@ -145,6 +155,8 @@ describe SystemNoteService do
     context 'when assignee removed' do
       let(:assignee) { nil }
 
+      it_behaves_like 'a note with overridable created_at'
+
       it 'sets the note text' do
         expect(subject.note).to eq 'removed assignee'
       end
@@ -168,6 +180,8 @@ describe SystemNoteService do
       described_class.change_issue_assignees(issue, project, author, old_assignees).note
     end
 
+    it_behaves_like 'a note with overridable created_at'
+
     it 'builds a correct phrase when an assignee is added to a non-assigned issue' do
       expect(build_note([], [assignee1])).to eq "assigned to @#{assignee1.username}"
     end
@@ -213,6 +227,8 @@ describe SystemNoteService do
 
           expect(subject.note).to eq "changed milestone to #{reference}"
         end
+
+        it_behaves_like 'a note with overridable created_at'
       end
 
       context 'when milestone removed' do
@@ -221,6 +237,8 @@ describe SystemNoteService do
         it 'sets the note text' do
           expect(subject.note).to eq 'removed milestone'
         end
+
+        it_behaves_like 'a note with overridable created_at'
       end
     end
 
@@ -237,6 +255,8 @@ describe SystemNoteService do
         it 'sets the note text to use the milestone name' do
           expect(subject.note).to eq "changed milestone to #{milestone.to_reference(format: :name)}"
         end
+
+        it_behaves_like 'a note with overridable created_at'
       end
 
       context 'when milestone removed' do
@@ -245,6 +265,8 @@ describe SystemNoteService do
         it 'sets the note text' do
           expect(subject.note).to eq 'removed milestone'
         end
+
+        it_behaves_like 'a note with overridable created_at'
       end
     end
   end
@@ -254,6 +276,8 @@ describe SystemNoteService do
 
     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
@@ -280,6 +304,8 @@ describe SystemNoteService do
       let(:status) { 'reopened' }
       let(:source) { nil }
 
+      it_behaves_like 'a note with overridable created_at'
+
       it_behaves_like 'a system note' do
         let(:action) { 'opened' }
       end
@@ -289,6 +315,8 @@ describe SystemNoteService do
       let(:status) { 'opened' }
       let(:source) { double('commit', gfm_reference: 'commit 123456') }
 
+      it_behaves_like 'a note with overridable created_at'
+
       it 'sets the note text' do
         expect(subject.note).to eq "#{status} via commit 123456"
       end
@@ -338,6 +366,8 @@ describe SystemNoteService do
         let(:action) { 'title' }
       end
 
+      it_behaves_like 'a note with overridable created_at'
+
       it 'sets the note text' do
         expect(subject.note)
           .to eq "changed title from **{-Old title-}** to **{+Lorem ipsum+}**"
@@ -353,6 +383,8 @@ describe SystemNoteService do
         let(:action) { 'description' }
       end
 
+      it_behaves_like 'a note with overridable created_at'
+
       it 'sets the note text' do
         expect(subject.note).to eq('changed the description')
       end
@@ -478,6 +510,8 @@ describe SystemNoteService do
         let(:action) { 'cross_reference' }
       end
 
+      it_behaves_like 'a note with overridable created_at'
+
       describe 'note_body' do
         context 'cross-project' do
           let(:project2) { create(:project, :repository) }