diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index ecdcbf08ee1..9a7af5730d2 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -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 diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 1ce66d50368..a33845848b4 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -21,7 +21,7 @@ module SystemNoteService total_count = new_commits.length + existing_commits.length commits_text = "#{total_count} commit".pluralize(total_count) - body = "Added #{commits_text}:\n\n" + body = "added #{commits_text}\n\n" body << existing_commit_summary(noteable, existing_commits, oldrev) body << new_commit_summary(new_commits).join("\n") body << "\n\n[Compare with previous version](#{diff_comparison_url(noteable, project, oldrev)})" @@ -38,13 +38,13 @@ module SystemNoteService # # Example Note text: # - # "Assignee removed" + # "removed assignee" # - # "Reassigned to @rspeicher" + # "assigned to @rspeicher" # # Returns the created Note object def change_assignee(noteable, project, author, assignee) - body = assignee.nil? ? 'Assignee removed' : "Reassigned to #{assignee.to_reference}" + body = assignee.nil? ? 'removed assignee' : "assigned to #{assignee.to_reference}" create_note(noteable: noteable, project: project, author: author, note: body) end @@ -59,11 +59,11 @@ module SystemNoteService # # Example Note text: # - # "Added ~1 and removed ~2 ~3 labels" + # "added ~1 and removed ~2 ~3 labels" # - # "Added ~4 label" + # "added ~4 label" # - # "Removed ~5 label" + # "removed ~5 label" # # Returns the created Note object def change_label(noteable, project, author, added_labels, removed_labels) @@ -85,7 +85,6 @@ module SystemNoteService end body << ' ' << 'label'.pluralize(labels_count) - body = body.capitalize create_note(noteable: noteable, project: project, author: author, note: body) end @@ -99,14 +98,13 @@ module SystemNoteService # # Example Note text: # - # "Milestone removed" + # "removed milestone" # - # "Miletone changed to 7.11" + # "changed milestone to 7.11" # # Returns the created Note object def change_milestone(noteable, project, author, milestone) - body = 'Milestone ' - body += milestone.nil? ? 'removed' : "changed 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) end @@ -121,46 +119,46 @@ module SystemNoteService # # Example Note text: # - # "Status changed to merged" + # "merged" # - # "Status changed to closed by bc17db76" + # "closed via bc17db76" # # Returns the created Note object def change_status(noteable, project, author, status, source) - body = "Status changed to #{status}" - body << " by #{source.gfm_reference(project)}" if source + body = status.dup + body << " via #{source.gfm_reference(project)}" if source create_note(noteable: noteable, project: project, author: author, note: body) end # Called when 'merge when build succeeds' is executed def merge_when_build_succeeds(noteable, project, author, last_commit) - body = "Enabled an automatic merge when the build for #{last_commit.to_reference(project)} succeeds" + body = "enabled an automatic merge when the build for #{last_commit.to_reference(project)} succeeds" create_note(noteable: noteable, project: project, author: author, note: body) end # Called when 'merge when build succeeds' is canceled def cancel_merge_when_build_succeeds(noteable, project, author) - body = 'Canceled the automatic merge' + body = 'canceled the automatic merge' create_note(noteable: noteable, project: project, author: author, note: body) end def remove_merge_request_wip(noteable, project, author) - body = 'Unmarked this merge request as a Work In Progress' + body = 'unmarked as a Work In Progress' create_note(noteable: noteable, project: project, author: author, note: body) end def add_merge_request_wip(noteable, project, author) - body = 'Marked this merge request as a **Work In Progress**' + body = 'marked as a **Work In Progress**' create_note(noteable: noteable, project: project, author: author, note: body) end 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) end @@ -174,7 +172,7 @@ module SystemNoteService # # Example Note text: # - # "Title changed from **Old** to **New**" + # "changed title from **Old** to **New**" # # Returns the created Note object def change_title(noteable, project, author, old_title) @@ -185,7 +183,7 @@ module SystemNoteService marked_old_title = Gitlab::Diff::InlineDiffMarker.new(old_title).mark(old_diffs, mode: :deletion, markdown: true) marked_new_title = Gitlab::Diff::InlineDiffMarker.new(new_title).mark(new_diffs, mode: :addition, markdown: true) - body = "Changed title: **#{marked_old_title}** → **#{marked_new_title}**" + body = "changed title from **#{marked_old_title}** to **#{marked_new_title}**" create_note(noteable: noteable, project: project, author: author, note: body) end @@ -197,11 +195,11 @@ module SystemNoteService # # Example Note text: # - # "Made the issue confidential" + # "made the issue confidential" # # Returns the created Note object def change_issue_confidentiality(issue, project, author) - body = issue.confidential ? 'Made the issue confidential' : 'Made the issue visible' + body = issue.confidential ? 'made the issue confidential' : 'made the issue visible to everyone' create_note(noteable: issue, project: project, author: author, note: body) end @@ -216,11 +214,11 @@ module SystemNoteService # # Example Note text: # - # "Target branch changed from `Old` to `New`" + # "changed target branch from `Old` to `New`" # # Returns the created Note object def change_branch(noteable, project, author, branch_type, old_branch, new_branch) - body = "#{branch_type} branch changed from `#{old_branch}` to `#{new_branch}`".capitalize + body = "changed #{branch_type} branch from `#{old_branch}` to `#{new_branch}`" create_note(noteable: noteable, project: project, author: author, note: body) end @@ -235,7 +233,7 @@ module SystemNoteService # # Example Note text: # - # "Restored target branch `feature`" + # "restored target branch `feature`" # # Returns the created Note object def change_branch_presence(noteable, project, author, branch_type, branch, presence) @@ -246,18 +244,18 @@ module SystemNoteService 'deleted' end - body = "#{verb} #{branch_type} branch `#{branch}`".capitalize + body = "#{verb} #{branch_type} branch `#{branch}`" create_note(noteable: noteable, project: project, author: author, note: body) end # Called when a branch is created from the 'new branch' button on a issue # Example note text: # - # "Started branch `201-issue-branch-button`" + # "created branch `201-issue-branch-button`" def new_issue_branch(issue, project, author, branch) link = url_helpers.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch) - body = "Started branch [`#{branch}`](#{link})" + body = "created branch [`#{branch}`](#{link})" create_note(noteable: issue, project: project, author: author, note: body) end @@ -269,11 +267,11 @@ module SystemNoteService # # Example Note text: # - # "Mentioned in #1" + # "mentioned in #1" # - # "Mentioned in !2" + # "mentioned in !2" # - # "Mentioned in 54f7727c" + # "mentioned in 54f7727c" # # See cross_reference_note_content. # @@ -303,12 +301,12 @@ module SystemNoteService end def cross_reference?(note_text) - note_text.start_with?(cross_reference_note_prefix) + note_text =~ /\A#{cross_reference_note_prefix}/i end # Check if a cross-reference is disallowed # - # This method prevents adding a "Mentioned in !1" note on every single commit + # This method prevents adding a "mentioned in !1" note on every single commit # in a merge request. Additionally, it prevents the creation of references to # external issues (which would fail). # @@ -370,12 +368,12 @@ module SystemNoteService # # Example Note text: # - # "Soandso marked the task Whatever as completed." + # "marked the task Whatever as completed." # # Returns the created Note object def change_task_status(noteable, project, author, new_task) 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) end @@ -388,7 +386,7 @@ module SystemNoteService # # Example Note text: # - # "Moved to some_namespace/project_new#11" + # "moved to some_namespace/project_new#11" # # Returns the created Note object def noteable_moved(noteable, project, noteable_ref, author, direction:) @@ -397,7 +395,7 @@ module SystemNoteService end 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) end @@ -405,10 +403,12 @@ module SystemNoteService def notes_for_mentioner(mentioner, noteable, notes) if mentioner.is_a?(Commit) - notes.where('note LIKE ?', "#{cross_reference_note_prefix}%#{mentioner.to_reference(nil)}") + text = "#{cross_reference_note_prefix}%#{mentioner.to_reference(nil)}" + notes.where('(note LIKE ? OR note LIKE ?)', text, text.capitalize) else gfm_reference = mentioner.gfm_reference(noteable.project) - notes.where(note: cross_reference_note_content(gfm_reference)) + text = cross_reference_note_content(gfm_reference) + notes.where(note: [text, text.capitalize]) end end @@ -417,7 +417,7 @@ module SystemNoteService end def cross_reference_note_prefix - 'Mentioned in ' + 'mentioned in ' end def cross_reference_note_content(gfm_reference) diff --git a/changelogs/unreleased/rephrase-system-notes.yml b/changelogs/unreleased/rephrase-system-notes.yml new file mode 100644 index 00000000000..e77c3a31cb4 --- /dev/null +++ b/changelogs/unreleased/rephrase-system-notes.yml @@ -0,0 +1,4 @@ +--- +title: Rephrase some system notes to be compatible with new system note style +merge_request: 7692 +author: diff --git a/doc/api/notes.md b/doc/api/notes.md index 58d40eecf3e..9971806be56 100644 --- a/doc/api/notes.md +++ b/doc/api/notes.md @@ -21,7 +21,7 @@ Parameters: [ { "id": 302, - "body": "Status changed to closed", + "body": "closed", "attachment": null, "author": { "id": 1, diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index f728d243cdc..d2fa8cd39af 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -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 diff --git a/features/steps/shared/issuable.rb b/features/steps/shared/issuable.rb index df9845ba569..aa666a954bc 100644 --- a/features/steps/shared/issuable.rb +++ b/features/steps/shared/issuable.rb @@ -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) diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 7c5f33c63b8..6d30d085056 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -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 diff --git a/spec/features/issues/move_spec.rb b/spec/features/issues/move_spec.rb index 055210399a7..c9bec05a9da 100644 --- a/spec/features/issues/move_spec.rb +++ b/spec/features/issues/move_spec.rb @@ -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 diff --git a/spec/features/issues/new_branch_button_spec.rb b/spec/features/issues/new_branch_button_spec.rb index ab901e74617..a4d3053d10c 100644 --- a/spec/features/issues/new_branch_button_spec.rb +++ b/spec/features/issues/new_branch_button_spec.rb @@ -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 diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb index 8eceaad2457..9ad225e3a1b 100644 --- a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb +++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb @@ -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 diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 5d7247e2a62..9fffbb43e87 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -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 diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index e6b6e7c0634..17a15b12dcb 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -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 diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 0124b7271b3..b71a4c5a56e 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -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 diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 4465f22a001..7a54373963e 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -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 diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index f0ded06b785..c7de0d0c534 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -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 diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 4777a90639e..4c878d748c0 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -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 diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 24c25e4350f..5f6a7716beb 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -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 diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 7db32a33c93..dff1781d2aa 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -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 diff --git a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb index 1f90efdbd6a..c0164138713 100644 --- a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb +++ b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb @@ -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 diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index e515bc9f89c..bc340ff9d3c 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -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 diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index af7424a76a9..a99d4eac9bd 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -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 diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index cb5d7cdb467..0bd6db1810a 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -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 diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 2a5709c6322..4a8f6c321aa 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -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