Merge branch '51485-new-issue-labels-note' into 'master'

Run CommonSystemNotesService on issuable create

Closes #51485

See merge request gitlab-org/gitlab-ce!23859
This commit is contained in:
Rémy Coutable 2018-12-18 10:08:44 +00:00
commit f3d80159fc
5 changed files with 69 additions and 14 deletions

View file

@ -4,20 +4,23 @@ module Issuable
class CommonSystemNotesService < ::BaseService class CommonSystemNotesService < ::BaseService
attr_reader :issuable attr_reader :issuable
def execute(issuable, old_labels) def execute(issuable, old_labels: [], is_update: true)
@issuable = issuable @issuable = issuable
if issuable.previous_changes.include?('title') if is_update
create_title_change_note(issuable.previous_changes['title'].first) if issuable.previous_changes.include?('title')
create_title_change_note(issuable.previous_changes['title'].first)
end
handle_description_change_note
handle_time_tracking_note if issuable.is_a?(TimeTrackable)
create_discussion_lock_note if issuable.previous_changes.include?('discussion_locked')
end end
handle_description_change_note
handle_time_tracking_note if issuable.is_a?(TimeTrackable)
create_labels_note(old_labels) if issuable.labels != old_labels
create_discussion_lock_note if issuable.previous_changes.include?('discussion_locked')
create_milestone_note if issuable.previous_changes.include?('milestone_id')
create_due_date_note if issuable.previous_changes.include?('due_date') create_due_date_note if issuable.previous_changes.include?('due_date')
create_milestone_note if issuable.previous_changes.include?('milestone_id')
create_labels_note(old_labels) if issuable.labels != old_labels
end end
private private

View file

@ -152,6 +152,10 @@ class IssuableBaseService < BaseService
before_create(issuable) before_create(issuable)
if issuable.save if issuable.save
ActiveRecord::Base.no_touching do
Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, is_update: false)
end
after_create(issuable) after_create(issuable)
execute_hooks(issuable) execute_hooks(issuable)
invalidate_cache_counts(issuable, users: issuable.assignees) invalidate_cache_counts(issuable, users: issuable.assignees)
@ -207,7 +211,7 @@ class IssuableBaseService < BaseService
if issuable.with_transaction_returning_status { issuable.save } if issuable.with_transaction_returning_status { issuable.save }
# We do not touch as it will affect a update on updated_at field # We do not touch as it will affect a update on updated_at field
ActiveRecord::Base.no_touching do ActiveRecord::Base.no_touching do
Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_associations[:labels]) Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels: old_associations[:labels])
end end
handle_changes(issuable, old_associations: old_associations) handle_changes(issuable, old_associations: old_associations)

View file

@ -0,0 +1,5 @@
---
title: Create system notes on issue / MR creation when labels, milestone, or due date is set
merge_request: 23859
author:
type: added

View file

@ -5,7 +5,7 @@ describe Issuable::CommonSystemNotesService do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:issuable) { create(:issue) } let(:issuable) { create(:issue) }
describe '#execute' do context 'on issuable update' do
it_behaves_like 'system note creation', { title: 'New title' }, 'changed title' it_behaves_like 'system note creation', { title: 'New title' }, 'changed title'
it_behaves_like 'system note creation', { description: 'New description' }, 'changed the description' it_behaves_like 'system note creation', { description: 'New description' }, 'changed the description'
it_behaves_like 'system note creation', { discussion_locked: true }, 'locked this issue' it_behaves_like 'system note creation', { discussion_locked: true }, 'locked this issue'
@ -20,7 +20,7 @@ describe Issuable::CommonSystemNotesService do
end end
it 'creates a resource label event' do it 'creates a resource label event' do
described_class.new(project, user).execute(issuable, []) described_class.new(project, user).execute(issuable, old_labels: [])
event = issuable.reload.resource_label_events.last event = issuable.reload.resource_label_events.last
expect(event).not_to be_nil expect(event).not_to be_nil
@ -68,4 +68,47 @@ describe Issuable::CommonSystemNotesService do
end end
end end
end end
context 'on issuable create' do
let(:issuable) { build(:issue) }
subject { described_class.new(project, user).execute(issuable, old_labels: [], is_update: false) }
it 'does not create system note for title and description' do
issuable.save
expect { subject }.not_to change { issuable.notes.count }
end
it 'creates a resource label event for labels added' do
label = create(:label, project: project)
issuable.labels << label
issuable.save
expect { subject }.to change { issuable.resource_label_events.count }.from(0).to(1)
event = issuable.reload.resource_label_events.last
expect(event).not_to be_nil
expect(event.label_id).to eq label.id
expect(event.user_id).to eq user.id
end
it 'creates a system note for milestone set' do
issuable.milestone = create(:milestone, project: project)
issuable.save
expect { subject }.to change { issuable.notes.count }.from(0).to(1)
expect(issuable.notes.last.note).to match('changed milestone')
end
it 'creates a system note for due_date set' do
issuable.due_date = Date.today
issuable.save
expect { subject }.to change { issuable.notes.count }.from(0).to(1)
expect(issuable.notes.last.note).to match('changed due date')
end
end
end end

View file

@ -1,5 +1,5 @@
shared_examples 'system note creation' do |update_params, note_text| shared_examples 'system note creation' do |update_params, note_text|
subject { described_class.new(project, user).execute(issuable, [])} subject { described_class.new(project, user).execute(issuable, old_labels: []) }
before do before do
issuable.assign_attributes(update_params) issuable.assign_attributes(update_params)
@ -16,7 +16,7 @@ shared_examples 'system note creation' do |update_params, note_text|
end end
shared_examples 'WIP notes creation' do |wip_action| shared_examples 'WIP notes creation' do |wip_action|
subject { described_class.new(project, user).execute(issuable, []) } subject { described_class.new(project, user).execute(issuable, old_labels: []) }
it 'creates WIP toggle and title change notes' do it 'creates WIP toggle and title change notes' do
expect { subject }.to change { Note.count }.from(0).to(2) expect { subject }.to change { Note.count }.from(0).to(2)