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

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

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)

changelogs/unreleased/rephrase-system-notes.yml

+---
+title: Rephrase some system notes to be compatible with new system note style
+merge_request: 7692
+author:

doc/api/notes.md

@@ -21,7 +21,7 @@ Parameters:
 [
   {
     "id": 302,
-    "body": "Status changed to closed",
+    "body": "closed",
     "attachment": null,
     "author": {
       "id": 1,

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

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)

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

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

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

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

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

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

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

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

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

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

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

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

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

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