diff --git a/app/models/concerns/time_trackable.rb b/app/models/concerns/time_trackable.rb
index 1caf47072bc551f2733de4ad50ec092f35dfab9c..0fc321c52bcfa5ebb31c9414d59b5222def8f8fd 100644
--- a/app/models/concerns/time_trackable.rb
+++ b/app/models/concerns/time_trackable.rb
@@ -30,8 +30,6 @@ def spend_time(options)
return if @time_spent == 0
- touch if touchable?
-
if @time_spent == :reset
reset_spent_time
else
@@ -59,10 +57,6 @@ def time_estimate=(val)
private
- def touchable?
- valid? && persisted?
- end
-
def reset_spent_time
timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
diff --git a/app/models/timelog.rb b/app/models/timelog.rb
index e166cf69703a960e711b4dd908f2d1f3e110bdef..f4c5c581a11627dbf854426832b636576301bf9f 100644
--- a/app/models/timelog.rb
+++ b/app/models/timelog.rb
@@ -2,8 +2,8 @@ class Timelog < ActiveRecord::Base
validates :time_spent, :user, presence: true
validate :issuable_id_is_present
- belongs_to :issue
- belongs_to :merge_request
+ belongs_to :issue, touch: true
+ belongs_to :merge_request, touch: true
belongs_to :user
def issuable
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 1f67e3ecf9dfda5dd013d28a635917a9d705dfb0..683f64e82ad89e082bfe9db5541667acd1f18c0b 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -183,7 +183,10 @@ def update(issuable)
old_associations = associations_before_update(issuable)
label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids)
- params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids)
+ if labels_changing?(issuable.label_ids, label_ids)
+ params[:label_ids] = label_ids
+ issuable.touch
+ end
if issuable.changed? || params.present?
issuable.assign_attributes(params.merge(updated_by: current_user))
diff --git a/changelogs/unreleased/46478-update-updated-at-on-mr.yml b/changelogs/unreleased/46478-update-updated-at-on-mr.yml
new file mode 100644
index 0000000000000000000000000000000000000000..c58b4fc8f84529df2164918a7d02dfccb43b6af6
--- /dev/null
+++ b/changelogs/unreleased/46478-update-updated-at-on-mr.yml
@@ -0,0 +1,5 @@
+---
+title: Updates updated_at on label changes
+merge_request: 19065
+author: Jacopo Beschi @jacopo-beschi
+type: fixed
diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb
index 998080a3dd5dce2c2ea6e81bbbfcf4657e92d06e..3a35bdd25deb9fbb8addfd0dce1e91b36471f39e 100644
--- a/spec/factories/issues.rb
+++ b/spec/factories/issues.rb
@@ -3,6 +3,7 @@
title { generate(:title) }
project
author { project.creator }
+ updated_by { author }
trait :confidential do
confidential true
diff --git a/spec/fixtures/api/schemas/entities/issue.json b/spec/fixtures/api/schemas/entities/issue.json
index 38467b4ca20af181f25087d8b5254842ad6bc9f4..00abe73ec8aaa730088e2f31d41f5bac90ecf2d4 100644
--- a/spec/fixtures/api/schemas/entities/issue.json
+++ b/spec/fixtures/api/schemas/entities/issue.json
@@ -27,7 +27,7 @@
"due_date": { "type": "date" },
"confidential": { "type": "boolean" },
"discussion_locked": { "type": ["boolean", "null"] },
- "updated_by_id": { "type": ["string", "null"] },
+ "updated_by_id": { "type": ["integer", "null"] },
"time_estimate": { "type": "integer" },
"total_time_spent": { "type": "integer" },
"human_time_estimate": { "type": ["integer", "null"] },
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index bd6bf5b07123168a7da959ab77531f1f4d2a7524..1cfd526834c5629c205746d95711d339941af2e8 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -12,6 +12,7 @@
it { is_expected.to belong_to(:author) }
it { is_expected.to have_many(:notes).dependent(:destroy) }
it { is_expected.to have_many(:todos).dependent(:destroy) }
+ it { is_expected.to have_many(:labels) }
context 'Notes' do
let!(:note) { create(:note, noteable: issue, project: issue.project) }
@@ -274,8 +275,8 @@
it 'skips coercion for not Integer values' do
expect { issue.time_estimate = nil }.to change { issue.time_estimate }.to(nil)
- expect { issue.time_estimate = 'invalid time' }.not_to raise_error(StandardError)
- expect { issue.time_estimate = 22.33 }.not_to raise_error(StandardError)
+ expect { issue.time_estimate = 'invalid time' }.not_to raise_error
+ expect { issue.time_estimate = 22.33 }.not_to raise_error
end
end
diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb
index 6e30798356c36c891d89b867f2b4a2f9b983b629..a0c93c531eaacbcbcea23e7b466968bcf7fc8596 100644
--- a/spec/models/timelog_spec.rb
+++ b/spec/models/timelog_spec.rb
@@ -5,6 +5,9 @@
let(:issue) { create(:issue) }
let(:merge_request) { create(:merge_request) }
+ it { is_expected.to belong_to(:issue).touch(true) }
+ it { is_expected.to belong_to(:merge_request).touch(true) }
+
it { is_expected.to be_valid }
it { is_expected.to validate_presence_of(:time_spent) }
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 3106083293fbef6dabaa5b26f4dbac57289a0a13..4181f4ebbbe894deb60a7a2411d5c8c73ee2da53 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -1351,19 +1351,25 @@
expect(json_response['labels']).to eq([label.title])
end
- it 'removes all labels' do
- put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: ''
+ it 'removes all labels and touches the record' do
+ Timecop.travel(1.minute.from_now) do
+ put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: ''
+ end
expect(response).to have_gitlab_http_status(200)
expect(json_response['labels']).to eq([])
+ expect(json_response['updated_at']).to be > Time.now
end
- it 'updates labels' do
- put api("/projects/#{project.id}/issues/#{issue.iid}", user),
+ it 'updates labels and touches the record' do
+ Timecop.travel(1.minute.from_now) do
+ put api("/projects/#{project.id}/issues/#{issue.iid}", user),
labels: 'foo,bar'
+ end
expect(response).to have_gitlab_http_status(200)
expect(json_response['labels']).to include 'foo'
expect(json_response['labels']).to include 'bar'
+ expect(json_response['updated_at']).to be > Time.now
end
it 'allows special label names' do
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 23b1134b5a31cb66a205117942bfbc73d5d8b479..158541d36e30b84e900bc6646083f503ac3c5899 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -337,12 +337,18 @@ def update_issue(opts)
context 'when the labels change' do
before do
- update_issue(label_ids: [label.id])
+ Timecop.freeze(1.minute.from_now) do
+ update_issue(label_ids: [label.id])
+ end
end
it 'marks todos as done' do
expect(todo.reload.done?).to eq true
end
+
+ it 'updates updated_at' do
+ expect(issue.reload.updated_at).to be > Time.now
+ end
end
end
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 5279ea6164e2c07959a86151cbb7c53abbf2030b..bd2e91f1f7a5d6989d73282c10723e293d3bc1b1 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -326,12 +326,18 @@ def update_merge_request(opts)
context 'when the labels change' do
before do
- update_merge_request({ label_ids: [label.id] })
+ Timecop.freeze(1.minute.from_now) do
+ update_merge_request({ label_ids: [label.id] })
+ end
end
it 'marks pending todos as done' do
expect(pending_todo.reload).to be_done
end
+
+ it 'updates updated_at' do
+ expect(merge_request.reload.updated_at).to be > Time.now
+ end
end
context 'when the assignee changes' do