Commit fe4ae12e authored by Robert Speicher's avatar Robert Speicher

Merge branch 'rephrase-system-notes' into 'master'

Rephrase some system notes to be compatible with new system note style

See merge request !7692
parents 40fa5604 6df22f72
......@@ -171,7 +171,6 @@ class NotificationService
return true unless note.noteable_type.present?
# ignore gitlab service messages
return true if note.note.start_with?('Status changed to closed')
return true if note.cross_reference? && note.system?
target = note.noteable
......
This diff is collapsed.
---
title: Rephrase some system notes to be compatible with new system note style
merge_request: 7692
author:
......@@ -21,7 +21,7 @@ Parameters:
[
{
"id": 302,
"body": "Status changed to closed",
"body": "closed",
"attachment": null,
"author": {
"id": 1,
......
......@@ -515,7 +515,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
step 'I should see new target branch changes' do
expect(page).to have_content 'Request to merge fix into feature'
expect(page).to have_content 'Target branch changed from merge-test to feature'
expect(page).to have_content 'changed target branch from merge-test to feature'
wait_for_ajax
end
......
......@@ -179,7 +179,7 @@ module SharedIssuable
project = Project.find_by(name: from_project_name)
expect(page).to have_content(user_name)
expect(page).to have_content("Mentioned in #{issuable.class.to_s.titleize.downcase} #{issuable.to_reference(project)}")
expect(page).to have_content("mentioned in #{issuable.class.to_s.titleize.downcase} #{issuable.to_reference(project)}")
end
def expect_sidebar_content(content)
......
......@@ -31,7 +31,7 @@ describe Projects::MilestonesController do
# Check system note left for milestone removal
last_note = project.issues.find(issue.id).notes[-1].note
expect(last_note).to eq('Milestone removed')
expect(last_note).to eq('removed milestone')
end
end
end
......@@ -27,7 +27,7 @@ feature 'issue move to another project' do
let!(:mr) { create(:merge_request, source_project: old_project) }
let(:new_project) { create(:project) }
let(:new_project_search) { create(:project) }
let(:text) { 'Text with !1' }
let(:text) { "Text with #{mr.to_reference}" }
let(:cross_reference) { old_project.to_reference }
background do
......@@ -43,8 +43,8 @@ feature 'issue move to another project' do
expect(current_url).to include project_path(new_project)
expect(page).to have_content("Text with #{cross_reference}!1")
expect(page).to have_content("Moved from #{cross_reference}#1")
expect(page).to have_content("Text with #{cross_reference}#{mr.to_reference}")
expect(page).to have_content("moved from #{cross_reference}#{issue.to_reference}")
expect(page).to have_content(issue.title)
end
......
......@@ -20,12 +20,12 @@ feature 'Start new branch from an issue', feature: true do
context "when there is a referenced merge request" do
let!(:note) do
create(:note, :on_issue, :system, project: project, noteable: issue,
note: "Mentioned in !#{referenced_mr.iid}")
note: "mentioned in #{referenced_mr.to_reference}")
end
let(:referenced_mr) do
create(:merge_request, :simple, source_project: project, target_project: project,
description: "Fixes ##{issue.iid}", author: user)
description: "Fixes #{issue.to_reference}", author: user)
end
before do
......
......@@ -44,7 +44,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do
expect(page).to have_content "The source branch will not be removed."
visit_merge_request(merge_request) # Needed to refresh the page
expect(page).to have_content /Enabled an automatic merge when the build for [0-9a-f]{8} succeeds/i
expect(page).to have_content /enabled an automatic merge when the build for \h{8} succeeds/i
end
end
end
......
......@@ -141,7 +141,7 @@ describe 'Comments', feature: true do
let(:project2) { create(:project, :private) }
let(:issue) { create(:issue, project: project2) }
let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'markdown') }
let!(:note) { create(:note_on_merge_request, :system, noteable: merge_request, project: project, note: "Mentioned in #{issue.to_reference(project)}") }
let!(:note) { create(:note_on_merge_request, :system, noteable: merge_request, project: project, note: "mentioned in #{issue.to_reference(project)}") }
it 'shows the system note' do
login_as :admin
......
......@@ -223,7 +223,7 @@ describe Note, models: true do
let(:note) do
create :note,
noteable: ext_issue, project: ext_proj,
note: "Mentioned in issue #{private_issue.to_reference(ext_proj)}",
note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
system: true
end
......
......@@ -25,7 +25,7 @@ describe API::API, api: true do
let!(:cross_reference_note) do
create :note,
noteable: ext_issue, project: ext_proj,
note: "Mentioned in issue #{private_issue.to_reference(ext_proj)}",
note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
system: true
end
......
......@@ -62,7 +62,7 @@ describe Issues::CloseService, services: true do
it 'creates system note about issue reassign' do
note = issue.notes.last
expect(note.note).to include "Status changed to closed"
expect(note.note).to include "closed"
end
it 'marks todos as done' do
......
......@@ -81,11 +81,11 @@ describe Issues::MoveService, services: true do
end
it 'adds system note to old issue at the end' do
expect(old_issue.notes.last.note).to match /^Moved to/
expect(old_issue.notes.last.note).to start_with 'moved to'
end
it 'adds system note to new issue at the end' do
expect(new_issue.notes.last.note).to match /^Moved from/
expect(new_issue.notes.last.note).to start_with 'moved from'
end
it 'closes old issue' do
......@@ -151,7 +151,7 @@ describe Issues::MoveService, services: true do
end
it 'adds a system note about move after rewritten notes' do
expect(system_notes.last.note).to match /^Moved from/
expect(system_notes.last.note).to match /^moved from/
end
it 'preserves orignal author of comment' do
......
......@@ -91,24 +91,24 @@ describe Issues::UpdateService, services: true do
end
it 'creates system note about issue reassign' do
note = find_note('Reassigned to')
note = find_note('assigned to')
expect(note).not_to be_nil
expect(note.note).to include "Reassigned to \@#{user2.username}"
expect(note.note).to include "assigned to #{user2.to_reference}"
end
it 'creates system note about issue label edit' do
note = find_note('Added ~')
note = find_note('added ~')
expect(note).not_to be_nil
expect(note.note).to include "Added ~#{label.id} label"
expect(note.note).to include "added #{label.to_reference} label"
end
it 'creates system note about title change' do
note = find_note('Changed title:')
note = find_note('changed title')
expect(note).not_to be_nil
expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**'
expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**'
end
end
end
......@@ -128,10 +128,10 @@ describe Issues::UpdateService, services: true do
it 'creates system note about confidentiality change' do
update_issue(confidential: true)
note = find_note('Made the issue confidential')
note = find_note('made the issue confidential')
expect(note).not_to be_nil
expect(note.note).to eq 'Made the issue confidential'
expect(note.note).to eq 'made the issue confidential'
end
it 'executes confidential issue hooks' do
......@@ -269,8 +269,8 @@ describe Issues::UpdateService, services: true do
before { update_issue(description: "- [x] Task 1\n- [X] Task 2") }
it 'creates system note about task status change' do
note1 = find_note('Marked the task **Task 1** as completed')
note2 = find_note('Marked the task **Task 2** as completed')
note1 = find_note('marked the task **Task 1** as completed')
note2 = find_note('marked the task **Task 2** as completed')
expect(note1).not_to be_nil
expect(note2).not_to be_nil
......@@ -284,8 +284,8 @@ describe Issues::UpdateService, services: true do
end
it 'creates system note about task status change' do
note1 = find_note('Marked the task **Task 1** as incomplete')
note2 = find_note('Marked the task **Task 2** as incomplete')
note1 = find_note('marked the task **Task 1** as incomplete')
note2 = find_note('marked the task **Task 2** as incomplete')
expect(note1).not_to be_nil
expect(note2).not_to be_nil
......@@ -299,7 +299,7 @@ describe Issues::UpdateService, services: true do
end
it 'does not create a system note' do
note = find_note('Marked the task **Task 2** as incomplete')
note = find_note('marked the task **Task 2** as incomplete')
expect(note).to be_nil
end
......@@ -312,7 +312,7 @@ describe Issues::UpdateService, services: true do
end
it 'does not create a system note referencing the position the old item' do
note = find_note('Marked the task **Two** as incomplete')
note = find_note('marked the task **Two** as incomplete')
expect(note).to be_nil
end
......
......@@ -42,7 +42,7 @@ describe MergeRequests::CloseService, services: true do
it 'creates system note about merge_request reassign' do
note = @merge_request.notes.last
expect(note.note).to include 'Status changed to closed'
expect(note.note).to include 'closed'
end
it 'marks todos as done' do
......
......@@ -34,7 +34,7 @@ describe MergeRequests::MergeService, services: true do
it 'creates system note about merge_request merge' do
note = merge_request.notes.last
expect(note.note).to include 'Status changed to merged'
expect(note.note).to include 'merged'
end
end
......
......@@ -34,7 +34,7 @@ describe MergeRequests::MergeWhenBuildSucceedsService do
it 'creates a system note' do
note = merge_request.notes.last
expect(note.note).to match /Enabled an automatic merge when the build for (\w+\/\w+@)?[0-9a-z]{8}/
expect(note.note).to match /enabled an automatic merge when the build for (\w+\/\w+@)?\h{8}/
end
end
......@@ -113,7 +113,7 @@ describe MergeRequests::MergeWhenBuildSucceedsService do
it 'Posts a system note' do
note = mr_merge_if_green_enabled.notes.last
expect(note.note).to include 'Canceled the automatic merge'
expect(note.note).to include 'canceled the automatic merge'
end
end
......
......@@ -76,10 +76,10 @@ describe MergeRequests::RefreshService, services: true do
reload_mrs
end
it { expect(@merge_request.notes.last.note).to include('changed to merged') }
it { expect(@merge_request.notes.last.note).to include('merged') }
it { expect(@merge_request).to be_merged }
it { expect(@fork_merge_request).to be_merged }
it { expect(@fork_merge_request.notes.last.note).to include('changed to merged') }
it { expect(@fork_merge_request.notes.last.note).to include('merged') }
it { expect(@build_failed_todo).to be_done }
it { expect(@fork_build_failed_todo).to be_done }
end
......@@ -95,11 +95,11 @@ describe MergeRequests::RefreshService, services: true do
reload_mrs
end
it { expect(@merge_request.notes.last.note).to include('changed to merged') }
it { expect(@merge_request.notes.last.note).to include('merged') }
it { expect(@merge_request).to be_merged }
it { expect(@merge_request.diffs.size).to be > 0 }
it { expect(@fork_merge_request).to be_merged }
it { expect(@fork_merge_request.notes.last.note).to include('changed to merged') }
it { expect(@fork_merge_request.notes.last.note).to include('merged') }
it { expect(@build_failed_todo).to be_done }
it { expect(@fork_build_failed_todo).to be_done }
end
......@@ -119,7 +119,7 @@ describe MergeRequests::RefreshService, services: true do
it { expect(@merge_request.notes).to be_empty }
it { expect(@merge_request).to be_open }
it { expect(@fork_merge_request.notes.last.note).to include('Added 28 commits') }
it { expect(@fork_merge_request.notes.last.note).to include('added 28 commits') }
it { expect(@fork_merge_request).to be_open }
it { expect(@build_failed_todo).to be_pending }
it { expect(@fork_build_failed_todo).to be_pending }
......@@ -146,7 +146,7 @@ describe MergeRequests::RefreshService, services: true do
reload_mrs
end
it { expect(@merge_request.notes.last.note).to include('changed to merged') }
it { expect(@merge_request.notes.last.note).to include('merged') }
it { expect(@merge_request).to be_merged }
it { expect(@fork_merge_request).to be_open }
it { expect(@fork_merge_request.notes).to be_empty }
......@@ -169,8 +169,8 @@ describe MergeRequests::RefreshService, services: true do
expect(@merge_request).to be_open
notes = @fork_merge_request.notes.reorder(:created_at).map(&:note)
expect(notes[0]).to include('Restored source branch `master`')
expect(notes[1]).to include('Added 28 commits')
expect(notes[0]).to include('restored source branch `master`')
expect(notes[1]).to include('added 28 commits')
expect(@fork_merge_request).to be_open
end
end
......
......@@ -41,7 +41,7 @@ describe MergeRequests::ReopenService, services: true do
it 'creates system note about merge_request reopen' do
note = merge_request.notes.last
expect(note.note).to include 'Status changed to reopened'
expect(note.note).to include 'reopened'
end
end
......
......@@ -79,31 +79,31 @@ describe MergeRequests::UpdateService, services: true do
end
it 'creates system note about merge_request reassign' do
note = find_note('Reassigned to')
note = find_note('assigned to')
expect(note).not_to be_nil
expect(note.note).to include "Reassigned to \@#{user2.username}"
expect(note.note).to include "assigned to #{user2.to_reference}"
end
it 'creates system note about merge_request label edit' do
note = find_note('Added ~')
note = find_note('added ~')
expect(note).not_to be_nil
expect(note.note).to include "Added ~#{label.id} label"
expect(note.note).to include "added #{label.to_reference} label"
end
it 'creates system note about title change' do
note = find_note('Changed title:')
note = find_note('changed title')
expect(note).not_to be_nil
expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**'
expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**'
end
it 'creates system note about branch change' do
note = find_note('Target')
note = find_note('changed target')
expect(note).not_to be_nil
expect(note.note).to eq 'Target branch changed from `master` to `target`'
expect(note.note).to eq 'changed target branch from `master` to `target`'
end
context 'when not including source branch removal options' do
......@@ -258,8 +258,8 @@ describe MergeRequests::UpdateService, services: true do
before { update_merge_request({ description: "- [x] Task 1\n- [X] Task 2" }) }
it 'creates system note about task status change' do
note1 = find_note('Marked the task **Task 1** as completed')
note2 = find_note('Marked the task **Task 2** as completed')
note1 = find_note('marked the task **Task 1** as completed')
note2 = find_note('marked the task **Task 2** as completed')
expect(note1).not_to be_nil
expect(note2).not_to be_nil
......@@ -273,8 +273,8 @@ describe MergeRequests::UpdateService, services: true do
end
it 'creates system note about task status change' do
note1 = find_note('Marked the task **Task 1** as incomplete')
note2 = find_note('Marked the task **Task 2** as incomplete')
note1 = find_note('marked the task **Task 1** as incomplete')
note2 = find_note('marked the task **Task 2** as incomplete')
expect(note1).not_to be_nil
expect(note2).not_to be_nil
......
......@@ -50,7 +50,7 @@ describe SystemNoteService, services: true do
context 'without existing commits' do
it 'adds a message header' do
expect(note_lines[0]).to eq "Added #{new_commits.size} commits:"
expect(note_lines[0]).to eq "added #{new_commits.size} commits"
end
it 'adds a message line for each commit' do
......@@ -120,7 +120,7 @@ describe SystemNoteService, services: true do
context 'when assignee added' do
it 'sets the note text' do
expect(subject.note).to eq "Reassigned to @#{assignee.username}"
expect(subject.note).to eq "assigned to @#{assignee.username}"
end
end
......@@ -128,7 +128,7 @@ describe SystemNoteService, services: true do
let(:assignee) { nil }
it 'sets the note text' do
expect(subject.note).to eq 'Assignee removed'
expect(subject.note).to eq 'removed assignee'
end
end
end
......@@ -147,7 +147,7 @@ describe SystemNoteService, services: true do
let(:removed) { [] }
it 'sets the note text' do
expect(subject.note).to eq "Added ~#{labels[0].id} ~#{labels[1].id} labels"
expect(subject.note).to eq "added ~#{labels[0].id} ~#{labels[1].id} labels"
end
end
......@@ -156,7 +156,7 @@ describe SystemNoteService, services: true do
let(:removed) { labels }
it 'sets the note text' do
expect(subject.note).to eq "Removed ~#{labels[0].id} ~#{labels[1].id} labels"
expect(subject.note).to eq "removed ~#{labels[0].id} ~#{labels[1].id} labels"
end
end
......@@ -165,7 +165,7 @@ describe SystemNoteService, services: true do
let(:removed) { [labels[1]] }
it 'sets the note text' do
expect(subject.note).to eq "Added ~#{labels[0].id} and removed ~#{labels[1].id} labels"
expect(subject.note).to eq "added ~#{labels[0].id} and removed ~#{labels[1].id} labels"
end
end
end
......@@ -179,7 +179,7 @@ describe SystemNoteService, services: true do
context 'when milestone added' do
it 'sets the note text' do
expect(subject.note).to eq "Milestone changed to #{milestone.to_reference}"
expect(subject.note).to eq "changed milestone to #{milestone.to_reference}"
end
end
......@@ -187,7 +187,7 @@ describe SystemNoteService, services: true do
let(:milestone) { nil }
it 'sets the note text' do
expect(subject.note).to eq 'Milestone removed'
expect(subject.note).to eq 'removed milestone'
end
end
end
......@@ -204,13 +204,13 @@ describe SystemNoteService, services: true do
let(:source) { double('commit', gfm_reference: 'commit 123456') }
it 'sets the note text' do
expect(subject.note).to eq "Status changed to #{status} by commit 123456"
expect(subject.note).to eq "#{status} via commit 123456"
end
end
context 'without a source' do
it 'sets the note text' do
expect(subject.note).to eq "Status changed to #{status}"
expect(subject.note).to eq status
end
end
end
......@@ -226,7 +226,7 @@ describe SystemNoteService, services: true do
it_behaves_like 'a system note'
it "posts the Merge When Build Succeeds system note" do
expect(subject.note).to match /Enabled an automatic merge when the build for (\w+\/\w+@)?[0-9a-f]{40} succeeds/
expect(subject.note).to match /enabled an automatic merge when the build for (\w+\/\w+@)?\h{40} succeeds/
end
end
......@@ -240,7 +240,7 @@ describe SystemNoteService, services: true do
it_behaves_like 'a system note'
it "posts the Merge When Build Succeeds system note" do
expect(subject.note).to eq "Canceled the automatic merge"
expect(subject.note).to eq "canceled the automatic merge"
end
end
......@@ -252,7 +252,7 @@ describe SystemNoteService, services: true do
it 'sets the note text' do
expect(subject.note).
to eq "Changed title: **{-Old title-}** → **{+#{noteable.title}+}**"
to eq "changed title from **{-Old title-}** to **{+#{noteable.title}+}**"
end
end
end
......@@ -264,7 +264,7 @@ describe SystemNoteService, services: true do
it_behaves_like 'a system note'
it 'sets the note text' do
expect(subject.note).to eq 'Made the issue visible'
expect(subject.note).to eq 'made the issue visible to everyone'
end
end
end
......@@ -278,7 +278,7 @@ describe SystemNoteService, services: true do
context 'when target branch name changed' do
it 'sets the note text' do
expect(subject.note).to eq "Target branch changed from `#{old_branch}` to `#{new_branch}`"
expect(subject.note).to eq "changed target branch from `#{old_branch}` to `#{new_branch}`"
end
end
end
......@@ -290,7 +290,7 @@ describe SystemNoteService, services: true do
context 'when source branch deleted' do
it 'sets the note text' do
expect(subject.note).to eq "Deleted source branch `feature`"
expect(subject.note).to eq "deleted source branch `feature`"
end
end
end
......@@ -302,7 +302,7 @@ describe SystemNoteService, services: true do
context 'when a branch is created from the new branch button' do
it 'sets the note text' do
expect(subject.note).to match /\AStarted branch [`1-mepmep`]/
expect(subject.note).to match /\Acreated branch [`1-mepmep`]/
end
end
end
......@@ -338,13 +338,13 @@ describe SystemNoteService, services: true do
let(:mentioner) { project2.repository.commit }
it 'references the mentioning commit' do
expect(subject.note).to eq "Mentioned in commit #{mentioner.to_reference(project)}"
expect(subject.note).to eq "mentioned in commit #{mentioner.to_reference(project)}"
end
end
context 'from non-Commit' do
it 'references the mentioning object' do
expect(subject.note).to eq "Mentioned in issue #{mentioner.to_reference(project)}"
expect(subject.note).to eq "mentioned in issue #{mentioner.to_reference(project)}"
end
end
end
......@@ -354,13 +354,13 @@ describe SystemNoteService, services: true do
let(:mentioner) { project.repository.commit }
it 'references the mentioning commit' do
expect(subject.note).to eq "Mentioned in commit #{mentioner.to_reference}"
expect(subject.note).to eq "mentioned in commit #{mentioner.to_reference}"
end
end
context 'from non-Commit' do
it 'references the mentioning object' do
expect(subject.note).to eq "Mentioned in issue #{mentioner.to_reference}"
expect(subject.note).to eq "mentioned in issue #{mentioner.to_reference}"
end
end
end
......@@ -370,7 +370,11 @@ describe SystemNoteService, services: true do
describe '.cross_reference?' do
it 'is truthy when text begins with expected text' do
expect(described_class.cross_reference?('Mentioned in something')).to be_truthy
expect(described_class.cross_reference?('mentioned in something')).to be_truthy
end
it 'is truthy when text begins with legacy capitalized expected text' do
expect(described_class.cross_reference?('mentioned in something')).to be_truthy
end
it 'is falsey when text does not begin with expected text' do
......@@ -433,6 +437,19 @@ describe SystemNoteService, services: true do
expect(described_class.cross_reference_exists?(noteable, commit1)).
to be_falsey
end
context 'legacy capitalized cross reference' do
before do
# Mention issue (noteable) from commit0
system_note = described_class.cross_reference(noteable, commit0, author)
system_note.update(note: system_note.note.capitalize)
end
it 'is truthy when already mentioned' do
expect(described_class.cross_reference_exists?(noteable, commit0)).
to be_truthy
end
end
end
context 'commit from commit' do
......@@ -450,6 +467,19 @@ describe SystemNoteService, services: true do
expect(described_class.cross_reference_exists?(commit1, commit0)).
to be_falsey
end
context 'legacy capitalized cross reference' do
before do
# Mention commit1 from commit0
system_note = described_class.cross_reference(commit0, commit1, author)
system_note.update(note: system_note.note.capitalize)
end
it 'is truthy when already mentioned' do
expect(described_class.cross_reference_exists?(commit0, commit1)).
to be_truthy
end
end
end
context 'commit with cross-reference from fork' do
......@@ -465,6 +495,18 @@ describe SystemNoteService, services: true do
expect(described_class.cross_reference_exists?(noteable, commit2)).
to be true
end
context 'legacy capitalized cross reference' do
before do
system_note = described_class.cross_reference(noteable, commit0, author2)
system_note.update(note: system_note.note.capitalize)
end
it 'is true when a fork mentions an external issue' do
expect(described_class.cross_reference_exists?(noteable, commit2)).
to be true
end
end
end
end
......@@ -498,7 +540,7 @@ describe SystemNoteService, services: true do
it_behaves_like 'cross project mentionable'
it 'notifies about noteable being moved to' do
expect(subject.note).to match /Moved to/
expect(subject.note).to match /moved to/
end
end
......@@ -508,7 +550,7 @@ describe SystemNoteService, services: true do
it_behaves_like 'cross project mentionable'
it 'notifies about noteable being moved from' do
expect(subject.note).to match /Moved from/
expect(subject.note).to match /moved from/
end
end
......
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