Commit 6c06768d authored by Douwe Maan's avatar Douwe Maan

Merge branch '46478-update-updated-at-on-mr' into 'master'

Resolve "Update `updated_at` on an issue/mr on every issue/mr changes"

Closes #46478

See merge request gitlab-org/gitlab-ce!19065
parents d637fbe9 4c878363
...@@ -30,8 +30,6 @@ module TimeTrackable ...@@ -30,8 +30,6 @@ module TimeTrackable
return if @time_spent == 0 return if @time_spent == 0
touch if touchable?
if @time_spent == :reset if @time_spent == :reset
reset_spent_time reset_spent_time
else else
...@@ -59,10 +57,6 @@ module TimeTrackable ...@@ -59,10 +57,6 @@ module TimeTrackable
private private
def touchable?
valid? && persisted?
end
def reset_spent_time def reset_spent_time
timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
......
...@@ -2,8 +2,8 @@ class Timelog < ActiveRecord::Base ...@@ -2,8 +2,8 @@ class Timelog < ActiveRecord::Base
validates :time_spent, :user, presence: true validates :time_spent, :user, presence: true
validate :issuable_id_is_present validate :issuable_id_is_present
belongs_to :issue belongs_to :issue, touch: true
belongs_to :merge_request belongs_to :merge_request, touch: true
belongs_to :user belongs_to :user
def issuable def issuable
......
...@@ -183,7 +183,10 @@ class IssuableBaseService < BaseService ...@@ -183,7 +183,10 @@ class IssuableBaseService < BaseService
old_associations = associations_before_update(issuable) old_associations = associations_before_update(issuable)
label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) 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? if issuable.changed? || params.present?
issuable.assign_attributes(params.merge(updated_by: current_user)) issuable.assign_attributes(params.merge(updated_by: current_user))
......
---
title: Updates updated_at on label changes
merge_request: 19065
author: Jacopo Beschi @jacopo-beschi
type: fixed
...@@ -3,6 +3,7 @@ FactoryBot.define do ...@@ -3,6 +3,7 @@ FactoryBot.define do
title { generate(:title) } title { generate(:title) }
project project
author { project.creator } author { project.creator }
updated_by { author }
trait :confidential do trait :confidential do
confidential true confidential true
......
...@@ -27,7 +27,7 @@ ...@@ -27,7 +27,7 @@
"due_date": { "type": "date" }, "due_date": { "type": "date" },
"confidential": { "type": "boolean" }, "confidential": { "type": "boolean" },
"discussion_locked": { "type": ["boolean", "null"] }, "discussion_locked": { "type": ["boolean", "null"] },
"updated_by_id": { "type": ["string", "null"] }, "updated_by_id": { "type": ["integer", "null"] },
"time_estimate": { "type": "integer" }, "time_estimate": { "type": "integer" },
"total_time_spent": { "type": "integer" }, "total_time_spent": { "type": "integer" },
"human_time_estimate": { "type": ["integer", "null"] }, "human_time_estimate": { "type": ["integer", "null"] },
......
...@@ -12,6 +12,7 @@ describe Issuable do ...@@ -12,6 +12,7 @@ describe Issuable do
it { is_expected.to belong_to(:author) } it { is_expected.to belong_to(:author) }
it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:notes).dependent(:destroy) }
it { is_expected.to have_many(:todos).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) }
it { is_expected.to have_many(:labels) }
context 'Notes' do context 'Notes' do
let!(:note) { create(:note, noteable: issue, project: issue.project) } let!(:note) { create(:note, noteable: issue, project: issue.project) }
...@@ -274,8 +275,8 @@ describe Issuable do ...@@ -274,8 +275,8 @@ describe Issuable do
it 'skips coercion for not Integer values' do it 'skips coercion for not Integer values' do
expect { issue.time_estimate = nil }.to change { issue.time_estimate }.to(nil) 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 = 'invalid time' }.not_to raise_error
expect { issue.time_estimate = 22.33 }.not_to raise_error(StandardError) expect { issue.time_estimate = 22.33 }.not_to raise_error
end end
end end
......
...@@ -5,6 +5,9 @@ RSpec.describe Timelog do ...@@ -5,6 +5,9 @@ RSpec.describe Timelog do
let(:issue) { create(:issue) } let(:issue) { create(:issue) }
let(:merge_request) { create(:merge_request) } 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 be_valid }
it { is_expected.to validate_presence_of(:time_spent) } it { is_expected.to validate_presence_of(:time_spent) }
......
...@@ -1351,19 +1351,25 @@ describe API::Issues do ...@@ -1351,19 +1351,25 @@ describe API::Issues do
expect(json_response['labels']).to eq([label.title]) expect(json_response['labels']).to eq([label.title])
end end
it 'removes all labels' do it 'removes all labels and touches the record' do
put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: '' 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(response).to have_gitlab_http_status(200)
expect(json_response['labels']).to eq([]) expect(json_response['labels']).to eq([])
expect(json_response['updated_at']).to be > Time.now
end end
it 'updates labels' do it 'updates labels and touches the record' do
put api("/projects/#{project.id}/issues/#{issue.iid}", user), Timecop.travel(1.minute.from_now) do
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
labels: 'foo,bar' labels: 'foo,bar'
end
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(json_response['labels']).to include 'foo' expect(json_response['labels']).to include 'foo'
expect(json_response['labels']).to include 'bar' expect(json_response['labels']).to include 'bar'
expect(json_response['updated_at']).to be > Time.now
end end
it 'allows special label names' do it 'allows special label names' do
......
...@@ -337,12 +337,18 @@ describe Issues::UpdateService, :mailer do ...@@ -337,12 +337,18 @@ describe Issues::UpdateService, :mailer do
context 'when the labels change' do context 'when the labels change' do
before do before do
update_issue(label_ids: [label.id]) Timecop.freeze(1.minute.from_now) do
update_issue(label_ids: [label.id])
end
end end
it 'marks todos as done' do it 'marks todos as done' do
expect(todo.reload.done?).to eq true expect(todo.reload.done?).to eq true
end end
it 'updates updated_at' do
expect(issue.reload.updated_at).to be > Time.now
end
end end
end end
......
...@@ -326,12 +326,18 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -326,12 +326,18 @@ describe MergeRequests::UpdateService, :mailer do
context 'when the labels change' do context 'when the labels change' do
before 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 end
it 'marks pending todos as done' do it 'marks pending todos as done' do
expect(pending_todo.reload).to be_done expect(pending_todo.reload).to be_done
end end
it 'updates updated_at' do
expect(merge_request.reload.updated_at).to be > Time.now
end
end end
context 'when the assignee changes' do context 'when the assignee changes' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment