Resolve "Update `updated_at` on an issue/mr on every issue/mr changes"
This commit is contained in:
parent
d637fbe9af
commit
4c8783636c
|
@ -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
|
||||||
|
|
||||||
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
|
||||||
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
|
||||||
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))
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Updates updated_at on label changes
|
||||||
|
merge_request: 19065
|
||||||
|
author: Jacopo Beschi @jacopo-beschi
|
||||||
|
type: fixed
|
|
@ -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 @@
|
||||||
"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
|
||||||
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
|
||||||
|
|
||||||
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
|
||||||
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
|
||||||
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
|
||||||
|
|
||||||
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
|
||||||
|
|
||||||
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
|
||||||
|
|
Loading…
Reference in New Issue