Create metadata when creating system notes

This commit is contained in:
Jarka Kadlecova 2017-03-15 14:19:45 +01:00
parent 1c3c7fb25d
commit c729d9dae7
13 changed files with 269 additions and 75 deletions

View file

@ -37,7 +37,7 @@ class Note < ActiveRecord::Base
has_many :todos, dependent: :destroy has_many :todos, dependent: :destroy
has_many :events, as: :target, dependent: :destroy has_many :events, as: :target, dependent: :destroy
has_one :system_note_metadata, dependent: :destroy has_one :system_note_metadata
delegate :gfm_reference, :local_reference, to: :noteable delegate :gfm_reference, :local_reference, to: :noteable
delegate :name, to: :project, prefix: true delegate :name, to: :project, prefix: true

View file

@ -5,7 +5,7 @@ class SystemNoteMetadata < ActiveRecord::Base
].freeze ].freeze
validates :note, presence: true validates :note, presence: true
validates :icon, inclusion: ICON_TYPES, allow_nil: true validates :action, inclusion: ICON_TYPES, allow_nil: true
belongs_to :note belongs_to :note
end end

View file

@ -0,0 +1,20 @@
class NoteSummary
attr_reader :note
attr_reader :metadata
def initialize(noteable, project, author, body, action: nil, commit_count: nil)
@note = { noteable: noteable, project: project, author: author, note: body }
@metadata = { action: action, commit_count: commit_count }.compact
set_commit_params if note[:noteable].is_a?(Commit)
end
def metadata?
metadata.present?
end
def set_commit_params
note.merge!(noteable_type: 'Commit', commit_id: note[:noteable].id)
note[:noteable] = nil
end
end

View file

@ -26,7 +26,7 @@ module SystemNoteService
body << new_commit_summary(new_commits).join("\n") body << new_commit_summary(new_commits).join("\n")
body << "\n\n[Compare with previous version](#{diff_comparison_url(noteable, project, oldrev)})" body << "\n\n[Compare with previous version](#{diff_comparison_url(noteable, project, oldrev)})"
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'commit', commit_count: total_count))
end end
# Called when the assignee of a Noteable is changed or removed # Called when the assignee of a Noteable is changed or removed
@ -46,7 +46,7 @@ module SystemNoteService
def change_assignee(noteable, project, author, assignee) def change_assignee(noteable, project, author, assignee)
body = assignee.nil? ? 'removed assignee' : "assigned to #{assignee.to_reference}" body = assignee.nil? ? 'removed assignee' : "assigned to #{assignee.to_reference}"
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'assignee'))
end end
# Called when one or more labels on a Noteable are added and/or removed # Called when one or more labels on a Noteable are added and/or removed
@ -86,7 +86,7 @@ module SystemNoteService
body << ' ' << 'label'.pluralize(labels_count) body << ' ' << 'label'.pluralize(labels_count)
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'label'))
end end
# Called when the milestone of a Noteable is changed # Called when the milestone of a Noteable is changed
@ -106,7 +106,7 @@ module SystemNoteService
def change_milestone(noteable, project, author, milestone) def change_milestone(noteable, project, author, milestone)
body = milestone.nil? ? 'removed milestone' : "changed milestone to #{milestone.to_reference(project)}" body = milestone.nil? ? 'removed milestone' : "changed milestone to #{milestone.to_reference(project)}"
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'milestone'))
end end
# Called when the estimated time of a Noteable is changed # Called when the estimated time of a Noteable is changed
@ -132,7 +132,7 @@ module SystemNoteService
"changed time estimate to #{parsed_time}" "changed time estimate to #{parsed_time}"
end end
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking'))
end end
# Called when the spent time of a Noteable is changed # Called when the spent time of a Noteable is changed
@ -161,7 +161,7 @@ module SystemNoteService
body = "#{action} #{parsed_time} of time spent" body = "#{action} #{parsed_time} of time spent"
end end
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking'))
end end
# Called when the status of a Noteable is changed # Called when the status of a Noteable is changed
@ -183,53 +183,57 @@ module SystemNoteService
body = status.dup body = status.dup
body << " via #{source.gfm_reference(project)}" if source body << " via #{source.gfm_reference(project)}" if source
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'status'))
end end
# Called when 'merge when pipeline succeeds' is executed # Called when 'merge when pipeline succeeds' is executed
def merge_when_pipeline_succeeds(noteable, project, author, last_commit) def merge_when_pipeline_succeeds(noteable, project, author, last_commit)
body = "enabled an automatic merge when the pipeline for #{last_commit.to_reference(project)} succeeds" body = "enabled an automatic merge when the pipeline for #{last_commit.to_reference(project)} succeeds"
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
end end
# Called when 'merge when pipeline succeeds' is canceled # Called when 'merge when pipeline succeeds' is canceled
def cancel_merge_when_pipeline_succeeds(noteable, project, author) def cancel_merge_when_pipeline_succeeds(noteable, project, author)
body = 'canceled the automatic merge' body = 'canceled the automatic merge'
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
end end
def remove_merge_request_wip(noteable, project, author) def remove_merge_request_wip(noteable, project, author)
body = 'unmarked as a **Work In Progress**' body = 'unmarked as a **Work In Progress**'
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'title'))
end end
def add_merge_request_wip(noteable, project, author) def add_merge_request_wip(noteable, project, author)
body = 'marked as a **Work In Progress**' body = 'marked as a **Work In Progress**'
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'title'))
end end
def add_merge_request_wip_from_commit(noteable, project, author, commit) def add_merge_request_wip_from_commit(noteable, project, author, commit)
body = "marked as a **Work In Progress** from #{commit.to_reference(project)}" body = "marked as a **Work In Progress** from #{commit.to_reference(project)}"
create_note(noteable: noteable, project: project, author: author, note: body) create_note(NoteSummary.new(noteable, project, author, body, action: 'title'))
end end
def self.resolve_all_discussions(merge_request, project, author) def self.resolve_all_discussions(merge_request, project, author)
body = "resolved all discussions" body = "resolved all discussions"
create_note(noteable: merge_request, project: project, author: author, note: body) create_note(NoteSummary.new(merge_request, project, author, body, action: 'discussion'))
end end
def discussion_continued_in_issue(discussion, project, author, issue) def discussion_continued_in_issue(discussion, project, author, issue)
body = "created #{issue.to_reference} to continue this discussion" body = "created #{issue.to_reference} to continue this discussion"
note_attributes = discussion.reply_attributes.merge(project: project, author: author, note: body)
note_attributes[:type] = note_attributes.delete(:note_type)
create_note(note_attributes) note_params = discussion.reply_attributes.merge(project: project, author: author, note: body)
note_params[:type] = note_params.delete(:note_type)
note = Note.create(note_params.merge(system: true))
note.system_note_metadata = SystemNoteMetadata.new({ action: 'discussion' })
note
end end
# Called when the title of a Noteable is changed # Called when the title of a Noteable is changed
@ -253,7 +257,8 @@ module SystemNoteService
marked_new_title = Gitlab::Diff::InlineDiffMarker.new(new_title).mark(new_diffs, mode: :addition, markdown: true) marked_new_title = Gitlab::Diff::InlineDiffMarker.new(new_title).mark(new_diffs, mode: :addition, markdown: true)
body = "changed title from **#{marked_old_title}** to **#{marked_new_title}**" body = "changed title from **#{marked_old_title}** to **#{marked_new_title}**"
create_note(noteable: noteable, project: project, author: author, note: body)
create_note(NoteSummary.new(noteable, project, author, body, action: 'title'))
end end
# Called when the confidentiality changes # Called when the confidentiality changes
@ -269,7 +274,8 @@ module SystemNoteService
# Returns the created Note object # Returns the created Note object
def change_issue_confidentiality(issue, project, author) def change_issue_confidentiality(issue, project, author)
body = issue.confidential ? 'made the issue confidential' : 'made the issue visible to everyone' body = issue.confidential ? 'made the issue confidential' : 'made the issue visible to everyone'
create_note(noteable: issue, project: project, author: author, note: body)
create_note(NoteSummary.new(issue, project, author, body, action: 'confidentiality'))
end end
# Called when a branch in Noteable is changed # Called when a branch in Noteable is changed
@ -288,7 +294,8 @@ module SystemNoteService
# Returns the created Note object # Returns the created Note object
def change_branch(noteable, project, author, branch_type, old_branch, new_branch) def change_branch(noteable, project, author, branch_type, old_branch, new_branch)
body = "changed #{branch_type} branch from `#{old_branch}` to `#{new_branch}`" body = "changed #{branch_type} branch from `#{old_branch}` to `#{new_branch}`"
create_note(noteable: noteable, project: project, author: author, note: body)
create_note(NoteSummary.new(noteable, project, author, body, action: 'branch'))
end end
# Called when a branch in Noteable is added or deleted # Called when a branch in Noteable is added or deleted
@ -314,7 +321,8 @@ module SystemNoteService
end end
body = "#{verb} #{branch_type} branch `#{branch}`" body = "#{verb} #{branch_type} branch `#{branch}`"
create_note(noteable: noteable, project: project, author: author, note: body)
create_note(NoteSummary.new(noteable, project, author, body, action: 'branch'))
end end
# Called when a branch is created from the 'new branch' button on a issue # Called when a branch is created from the 'new branch' button on a issue
@ -325,7 +333,8 @@ module SystemNoteService
link = url_helpers.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch) link = url_helpers.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch)
body = "created branch [`#{branch}`](#{link})" body = "created branch [`#{branch}`](#{link})"
create_note(noteable: issue, project: project, author: author, note: body)
create_note(NoteSummary.new(issue, project, author, body, action: 'branch'))
end end
# Called when a Mentionable references a Noteable # Called when a Mentionable references a Noteable
@ -349,23 +358,12 @@ module SystemNoteService
return if cross_reference_disallowed?(noteable, mentioner) return if cross_reference_disallowed?(noteable, mentioner)
gfm_reference = mentioner.gfm_reference(noteable.project) gfm_reference = mentioner.gfm_reference(noteable.project)
body = cross_reference_note_content(gfm_reference)
note_options = {
project: noteable.project,
author: author,
note: cross_reference_note_content(gfm_reference)
}
if noteable.is_a?(Commit)
note_options.merge!(noteable_type: 'Commit', commit_id: noteable.id)
else
note_options[:noteable] = noteable
end
if noteable.is_a?(ExternalIssue) if noteable.is_a?(ExternalIssue)
noteable.project.issues_tracker.create_cross_reference_note(noteable, mentioner, author) noteable.project.issues_tracker.create_cross_reference_note(noteable, mentioner, author)
else else
create_note(note_options) create_note(NoteSummary.new(noteable, noteable.project, author, body, action: 'cross_reference'))
end end
end end
@ -444,7 +442,8 @@ module SystemNoteService
def change_task_status(noteable, project, author, new_task) def change_task_status(noteable, project, author, new_task)
status_label = new_task.complete? ? Taskable::COMPLETED : Taskable::INCOMPLETE status_label = new_task.complete? ? Taskable::COMPLETED : Taskable::INCOMPLETE
body = "marked the task **#{new_task.source}** as #{status_label}" body = "marked the task **#{new_task.source}** as #{status_label}"
create_note(noteable: noteable, project: project, author: author, note: body)
create_note(NoteSummary.new(noteable, project, author, body, action: 'task'))
end end
# Called when noteable has been moved to another project # Called when noteable has been moved to another project
@ -466,7 +465,8 @@ module SystemNoteService
cross_reference = noteable_ref.to_reference(project) cross_reference = noteable_ref.to_reference(project)
body = "moved #{direction} #{cross_reference}" body = "moved #{direction} #{cross_reference}"
create_note(noteable: noteable, project: project, author: author, note: body)
create_note(NoteSummary.new(noteable, project, author, body, action: 'moved'))
end end
private private
@ -482,8 +482,11 @@ module SystemNoteService
end end
end end
def create_note(args = {}) def create_note(note_summary)
Note.create(args.merge(system: true)) note = Note.create(note_summary.note.merge(system: true))
note.system_note_metadata = SystemNoteMetadata.new(note_summary.metadata) if note_summary.metadata?
note
end end
def cross_reference_note_prefix def cross_reference_note_prefix

View file

@ -0,0 +1,4 @@
---
title: Add metadata to system notes
merge_request: 9964
author:

View file

@ -5,15 +5,19 @@ class CreateSystemNoteMetadata < ActiveRecord::Migration
disable_ddl_transaction! disable_ddl_transaction!
def change def up
create_table :system_note_metadata do |t| create_table :system_note_metadata do |t|
t.references :note, null: false t.references :note, null: false
t.integer :commit_count t.integer :commit_count
t.string :icon t.string :action
t.timestamps null: false t.timestamps null: false
end end
add_concurrent_foreign_key :system_note_metadata, :notes, column: :note_id, on_delete: :cascade add_concurrent_foreign_key :system_note_metadata, :notes, column: :note_id
end
def down
drop_table :system_note_metadata
end end
end end

View file

@ -1078,7 +1078,7 @@ ActiveRecord::Schema.define(version: 20170317203554) do
create_table "system_note_metadata", force: :cascade do |t| create_table "system_note_metadata", force: :cascade do |t|
t.integer "note_id", null: false t.integer "note_id", null: false
t.integer "commit_count" t.integer "commit_count"
t.string "icon" t.string "action"
t.datetime "created_at", null: false t.datetime "created_at", null: false
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
end end

View file

@ -1,6 +1,6 @@
FactoryGirl.define do FactoryGirl.define do
factory :system_note_metadata do factory :system_note_metadata do
note note
icon 'merge' action 'merge'
end end
end end

View file

@ -29,6 +29,7 @@ notes:
- resolved_by - resolved_by
- todos - todos
- events - events
- system_note_metadata
label_links: label_links:
- target - target
- label - label

View file

@ -9,7 +9,6 @@ describe Note, models: true do
it { is_expected.to belong_to(:author).class_name('User') } it { is_expected.to belong_to(:author).class_name('User') }
it { is_expected.to have_many(:todos).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) }
it { is_expected.to have_one(:system_note_metadata).dependent(:destroy) }
end end
describe 'modules' do describe 'modules' do

View file

@ -8,17 +8,17 @@ describe SystemNoteMetadata, models: true do
describe 'validation' do describe 'validation' do
it { is_expected.to validate_presence_of(:note) } it { is_expected.to validate_presence_of(:note) }
context 'when icon type is invalid' do context 'when action type is invalid' do
subject do subject do
build(:system_note_metadata, note: build(:note), icon: 'invalid_type' ) build(:system_note_metadata, note: build(:note), action: 'invalid_type' )
end end
it { is_expected.to be_invalid } it { is_expected.to be_invalid }
end end
context 'when icon type is valid' do context 'when action type is valid' do
subject do subject do
build(:system_note_metadata, note: build(:note), icon: 'merge' ) build(:system_note_metadata, note: build(:note), action: 'merge' )
end end
it { is_expected.to be_valid } it { is_expected.to be_valid }

View file

@ -0,0 +1,44 @@
require 'spec_helper'
describe NoteSummary, services: true do
let(:project) { build(:empty_project) }
let(:noteable) { build(:issue) }
let(:user) { build(:user) }
def create_note_summary
described_class.new(noteable, project, user, 'note', action: 'icon', commit_count: 5)
end
describe '#metadata?' do
it 'returns true when metadata present' do
expect(create_note_summary.metadata?).to be_truthy
end
it 'returns false when metadata not present' do
expect(described_class.new(noteable, project, user, 'note').metadata?).to be_falsey
end
end
describe '#note' do
it 'returns note hash' do
expect(create_note_summary.note).to eq(noteable: noteable, project: project, author: user, note: 'note')
end
context 'when noteable is a commit' do
let(:noteable) { build(:commit) }
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
)
end
end
end
describe '#metadata' do
it 'returns metadata hash' do
expect(create_note_summary.metadata).to eq(action: 'icon', commit_count: 5)
end
end
end

View file

@ -8,12 +8,15 @@ describe SystemNoteService, services: true do
let(:noteable) { create(:issue, project: project) } let(:noteable) { create(:issue, project: project) }
shared_examples_for 'a system note' do shared_examples_for 'a system note' do
let(:expected_noteable) { noteable }
let(:commit_count) { nil }
it 'is valid' do it 'is valid' do
expect(subject).to be_valid expect(subject).to be_valid
end end
it 'sets the noteable model' do it 'sets the noteable model' do
expect(subject.noteable).to eq noteable expect(subject.noteable).to eq expected_noteable
end end
it 'sets the project' do it 'sets the project' do
@ -27,6 +30,19 @@ describe SystemNoteService, services: true do
it 'is a system note' do it 'is a system note' do
expect(subject).to be_system expect(subject).to be_system
end end
context 'metadata' do
it 'creates a new system note metadata record' do
expect { subject }.to change{ SystemNoteMetadata.count }.from(0).to(1)
end
it 'creates a record correctly' do
metadata = subject.system_note_metadata
expect(metadata.commit_count).to eq(commit_count)
expect(metadata.action).to eq(action)
end
end
end end
describe '.add_commits' do describe '.add_commits' do
@ -38,7 +54,10 @@ describe SystemNoteService, services: true do
let(:old_commits) { [] } let(:old_commits) { [] }
let(:oldrev) { nil } let(:oldrev) { nil }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:commit_count) { new_commits.size }
let(:action) { 'commit' }
end
describe 'note body' do describe 'note body' do
let(:note_lines) { subject.note.split("\n").reject(&:blank?) } let(:note_lines) { subject.note.split("\n").reject(&:blank?) }
@ -117,7 +136,9 @@ describe SystemNoteService, services: true do
let(:assignee) { create(:user) } let(:assignee) { create(:user) }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'assignee' }
end
context 'when assignee added' do context 'when assignee added' do
it 'sets the note text' do it 'sets the note text' do
@ -141,7 +162,9 @@ describe SystemNoteService, services: true do
let(:added) { [] } let(:added) { [] }
let(:removed) { [] } let(:removed) { [] }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'label' }
end
context 'with added labels' do context 'with added labels' do
let(:added) { labels } let(:added) { labels }
@ -176,7 +199,9 @@ describe SystemNoteService, services: true do
let(:milestone) { create(:milestone, project: project) } let(:milestone) { create(:milestone, project: project) }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'milestone' }
end
context 'when milestone added' do context 'when milestone added' do
it 'sets the note text' do it 'sets the note text' do
@ -199,7 +224,9 @@ describe SystemNoteService, services: true do
let(:status) { 'new_status' } let(:status) { 'new_status' }
let(:source) { nil } let(:source) { nil }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'status' }
end
context 'with a source' do context 'with a source' do
let(:source) { double('commit', gfm_reference: 'commit 123456') } let(:source) { double('commit', gfm_reference: 'commit 123456') }
@ -225,7 +252,9 @@ describe SystemNoteService, services: true do
subject { described_class.merge_when_pipeline_succeeds(noteable, project, author, noteable.diff_head_commit) } subject { described_class.merge_when_pipeline_succeeds(noteable, project, author, noteable.diff_head_commit) }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'merge' }
end
it "posts the 'merge when pipeline succeeds' system note" do it "posts the 'merge when pipeline succeeds' system note" do
expect(subject.note).to match(/enabled an automatic merge when the pipeline for (\w+\/\w+@)?\h{40} succeeds/) expect(subject.note).to match(/enabled an automatic merge when the pipeline for (\w+\/\w+@)?\h{40} succeeds/)
@ -240,7 +269,9 @@ describe SystemNoteService, services: true do
subject { described_class.cancel_merge_when_pipeline_succeeds(noteable, project, author) } subject { described_class.cancel_merge_when_pipeline_succeeds(noteable, project, author) }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'merge' }
end
it "posts the 'merge when pipeline succeeds' system note" do it "posts the 'merge when pipeline succeeds' system note" do
expect(subject.note).to eq "canceled the automatic merge" expect(subject.note).to eq "canceled the automatic merge"
@ -253,7 +284,9 @@ describe SystemNoteService, services: true do
subject { described_class.change_title(noteable, project, author, 'Old title') } subject { described_class.change_title(noteable, project, author, 'Old title') }
context 'when noteable responds to `title`' do context 'when noteable responds to `title`' do
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'title' }
end
it 'sets the note text' do it 'sets the note text' do
expect(subject.note). expect(subject.note).
@ -266,7 +299,9 @@ describe SystemNoteService, services: true do
subject { described_class.change_issue_confidentiality(noteable, project, author) } subject { described_class.change_issue_confidentiality(noteable, project, author) }
context 'when noteable responds to `confidential`' do context 'when noteable responds to `confidential`' do
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'confidentiality' }
end
it 'sets the note text' do it 'sets the note text' do
expect(subject.note).to eq 'made the issue visible to everyone' expect(subject.note).to eq 'made the issue visible to everyone'
@ -281,7 +316,9 @@ describe SystemNoteService, services: true do
let(:old_branch) { 'old_branch'} let(:old_branch) { 'old_branch'}
let(:new_branch) { 'new_branch'} let(:new_branch) { 'new_branch'}
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'branch' }
end
context 'when target branch name changed' do context 'when target branch name changed' do
it 'sets the note text' do it 'sets the note text' do
@ -295,7 +332,9 @@ describe SystemNoteService, services: true do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'branch' }
end
context 'when source branch deleted' do context 'when source branch deleted' do
it 'sets the note text' do it 'sets the note text' do
@ -309,7 +348,9 @@ describe SystemNoteService, services: true do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'branch' }
end
context 'when a branch is created from the new branch button' do context 'when a branch is created from the new branch button' do
it 'sets the note text' do it 'sets the note text' do
@ -323,7 +364,9 @@ describe SystemNoteService, services: true do
let(:mentioner) { create(:issue, project: project) } let(:mentioner) { create(:issue, project: project) }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'cross_reference' }
end
context 'when cross-reference disallowed' do context 'when cross-reference disallowed' do
before do before do
@ -333,6 +376,10 @@ describe SystemNoteService, services: true do
it 'returns nil' do it 'returns nil' do
expect(subject).to be_nil expect(subject).to be_nil
end end
it 'does not create a system note metadata record' do
expect { subject }.not_to change{ SystemNoteMetadata.count }
end
end end
context 'when cross-reference allowed' do context 'when cross-reference allowed' do
@ -340,6 +387,10 @@ describe SystemNoteService, services: true do
expect(described_class).to receive(:cross_reference_disallowed?).and_return(false) expect(described_class).to receive(:cross_reference_disallowed?).and_return(false)
end end
it_behaves_like 'a system note' do
let(:action) { 'cross_reference' }
end
describe 'note_body' do describe 'note_body' do
context 'cross-project' do context 'cross-project' do
let(:project2) { create(:project, :repository) } let(:project2) { create(:project, :repository) }
@ -552,6 +603,9 @@ describe SystemNoteService, services: true do
let(:direction) { :to } let(:direction) { :to }
it_behaves_like 'cross project mentionable' it_behaves_like 'cross project mentionable'
it_behaves_like 'a system note' do
let(:action) { 'moved' }
end
it 'notifies about noteable being moved to' do it 'notifies about noteable being moved to' do
expect(subject.note).to match('moved to') expect(subject.note).to match('moved to')
@ -562,6 +616,9 @@ describe SystemNoteService, services: true do
let(:direction) { :from } let(:direction) { :from }
it_behaves_like 'cross project mentionable' it_behaves_like 'cross project mentionable'
it_behaves_like 'a system note' do
let(:action) { 'moved' }
end
it 'notifies about noteable being moved from' do it 'notifies about noteable being moved from' do
expect(subject.note).to match('moved from') expect(subject.note).to match('moved from')
@ -732,33 +789,34 @@ describe SystemNoteService, services: true do
let(:merge_request) { discussion.noteable } let(:merge_request) { discussion.noteable }
let(:project) { merge_request.source_project } let(:project) { merge_request.source_project }
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:user) { create(:user) }
def reloaded_merge_request def reloaded_merge_request
MergeRequest.find(merge_request.id) MergeRequest.find(merge_request.id)
end end
before do subject { described_class.discussion_continued_in_issue(discussion, project, author, issue) }
project.team << [user, :developer]
it_behaves_like 'a system note' do
let(:expected_noteable) { discussion.first_note.noteable }
let(:action) { 'discussion' }
end end
it 'creates a new note in the discussion' do it 'creates a new note in the discussion' do
# we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded. # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded.
expect { SystemNoteService.discussion_continued_in_issue(discussion, project, user, issue) }. expect { subject }.to change { reloaded_merge_request.discussions.first.notes.size }.by(1)
to change { reloaded_merge_request.discussions.first.notes.size }.by(1)
end end
it 'mentions the created issue in the system note' do it 'mentions the created issue in the system note' do
note = SystemNoteService.discussion_continued_in_issue(discussion, project, user, issue) expect(subject.note).to include(issue.to_reference)
expect(note.note).to include(issue.to_reference)
end end
end end
describe '.change_time_estimate' do describe '.change_time_estimate' do
subject { described_class.change_time_estimate(noteable, project, author) } subject { described_class.change_time_estimate(noteable, project, author) }
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'time_tracking' }
end
context 'with a time estimate' do context 'with a time estimate' do
it 'sets the note text' do it 'sets the note text' do
@ -788,7 +846,9 @@ describe SystemNoteService, services: true do
described_class.change_time_spent(noteable, project, author) described_class.change_time_spent(noteable, project, author)
end end
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'time_tracking' }
end
context 'when time was added' do context 'when time was added' do
it 'sets the note text' do it 'sets the note text' do
@ -820,6 +880,34 @@ describe SystemNoteService, services: true do
end end
end end
describe '.remove_merge_request_wip' do
let(:noteable) { create(:issue, project: project, title: 'WIP: Lorem ipsum') }
subject { described_class.remove_merge_request_wip(noteable, project, author) }
it_behaves_like 'a system note' do
let(:action) { 'title' }
end
it 'sets the note text' do
expect(subject.note).to eq 'unmarked as a **Work In Progress**'
end
end
describe '.add_merge_request_wip' do
let(:noteable) { create(:issue, project: project, title: 'Lorem ipsum') }
subject { described_class.add_merge_request_wip(noteable, project, author) }
it_behaves_like 'a system note' do
let(:action) { 'title' }
end
it 'sets the note text' do
expect(subject.note).to eq 'marked as a **Work In Progress**'
end
end
describe '.add_merge_request_wip_from_commit' do describe '.add_merge_request_wip_from_commit' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:noteable) do let(:noteable) do
@ -835,7 +923,9 @@ describe SystemNoteService, services: true do
) )
end end
it_behaves_like 'a system note' it_behaves_like 'a system note' do
let(:action) { 'title' }
end
it "posts the 'marked as a Work In Progress from commit' system note" do it "posts the 'marked as a Work In Progress from commit' system note" do
expect(subject.note).to match( expect(subject.note).to match(
@ -843,4 +933,33 @@ describe SystemNoteService, services: true do
) )
end end
end end
describe '.change_task_status' do
let(:noteable) { create(:issue, project: project) }
let(:task) { double(:task, complete?: true, source: 'task') }
subject { described_class.change_task_status(noteable, project, author, task) }
it_behaves_like 'a system note' do
let(:action) { 'task' }
end
it "posts the 'marked as a Work In Progress from commit' system note" do
expect(subject.note).to eq("marked the task **task** as completed")
end
end
describe '.resolve_all_discussions' do
let(:noteable) { create(:merge_request, source_project: project, target_project: project) }
subject { described_class.resolve_all_discussions(noteable, project, author) }
it_behaves_like 'a system note' do
let(:action) { 'discussion' }
end
it 'sets the note text' do
expect(subject.note).to eq 'resolved all discussions'
end
end
end end