Merge branch '231494-draft-system-notes' into 'master'

Update system note when marking merge request as draft or ready

See merge request gitlab-org/gitlab!45644

app/services/issuable/common_system_notes_service.rb

@@ -51,11 +51,11 @@ module Issuable
       end
     end
 
-    def create_wip_note(old_title)
+    def create_draft_note(old_title)
       return unless issuable.is_a?(MergeRequest)
 
       if MergeRequest.work_in_progress?(old_title) != issuable.work_in_progress?
-        SystemNoteService.handle_merge_request_wip(issuable, issuable.project, current_user)
+        SystemNoteService.handle_merge_request_draft(issuable, issuable.project, current_user)
       end
     end
 
@@ -69,7 +69,7 @@ module Issuable
     end
 
     def create_title_change_note(old_title)
-      create_wip_note(old_title)
+      create_draft_note(old_title)
 
       if issuable.wipless_title_changed(old_title)
         SystemNoteService.change_title(issuable, issuable.project, current_user, old_title)

app/services/merge_requests/refresh_service.rb

@@ -42,7 +42,7 @@ module MergeRequests
         end
 
         notify_about_push(mr)
-        mark_mr_as_wip_from_commits(mr)
+        mark_mr_as_draft_from_commits(mr)
         execute_mr_web_hooks(mr)
       end
 
@@ -246,7 +246,7 @@ module MergeRequests
       notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits)
     end
 
-    def mark_mr_as_wip_from_commits(merge_request)
+    def mark_mr_as_draft_from_commits(merge_request)
       return unless @commits.present?
 
       commit_shas = merge_request.commit_shas
@@ -257,7 +257,7 @@ module MergeRequests
 
       if wip_commit && !merge_request.work_in_progress?
         merge_request.update(title: merge_request.wip_title)
-        SystemNoteService.add_merge_request_wip_from_commit(
+        SystemNoteService.add_merge_request_draft_from_commit(
           merge_request,
           merge_request.project,
           @current_user,

app/services/system_note_service.rb

@@ -130,12 +130,12 @@ module SystemNoteService
     ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).abort_merge_when_pipeline_succeeds(reason)
   end
 
-  def handle_merge_request_wip(noteable, project, author)
-    ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).handle_merge_request_wip
+  def handle_merge_request_draft(noteable, project, author)
+    ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).handle_merge_request_draft
   end
 
-  def add_merge_request_wip_from_commit(noteable, project, author, commit)
-    ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).add_merge_request_wip_from_commit(commit)
+  def add_merge_request_draft_from_commit(noteable, project, author, commit)
+    ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).add_merge_request_draft_from_commit(commit)
   end
 
   def resolve_all_discussions(merge_request, project, author)

app/services/system_notes/merge_requests_service.rb

@@ -26,16 +26,16 @@ module SystemNotes
       create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
     end
 
-    def handle_merge_request_wip
-      prefix = noteable.work_in_progress? ? "marked" : "unmarked"
+    def handle_merge_request_draft
+      action = noteable.work_in_progress? ? "draft" : "ready"
 
-      body = "#{prefix} as a **Work In Progress**"
+      body = "marked this merge request as **#{action}**"
 
       create_note(NoteSummary.new(noteable, project, author, body, action: 'title'))
     end
 
-    def add_merge_request_wip_from_commit(commit)
-      body = "marked as a **Work In Progress** from #{commit.to_reference(project)}"
+    def add_merge_request_draft_from_commit(commit)
+      body = "marked this merge request as **draft** from #{commit.to_reference(project)}"
 
       create_note(NoteSummary.new(noteable, project, author, body, action: 'title'))
     end

changelogs/unreleased/231494-draft-system-notes.yml

+---
+title: Update system note when marking merge request as draft or ready
+merge_request: 45644
+author:
+type: changed

spec/services/issuable/common_system_notes_service_spec.rb

@@ -36,28 +36,28 @@ RSpec.describe Issuable::CommonSystemNotesService do
       context 'adding Draft note' do
         let(:issuable) { create(:merge_request, title: "merge request") }
 
-        it_behaves_like 'system note creation', { title: "Draft: merge request" }, 'marked as a **Work In Progress**'
+        it_behaves_like 'system note creation', { title: "Draft: merge request" }, 'marked this merge request as **draft**'
 
         context 'and changing title' do
           before do
             issuable.update_attribute(:title, "Draft: changed title")
           end
 
-          it_behaves_like 'draft notes creation', 'marked'
+          it_behaves_like 'draft notes creation', 'draft'
         end
       end
 
       context 'removing Draft note' do
         let(:issuable) { create(:merge_request, title: "Draft: merge request") }
 
-        it_behaves_like 'system note creation', { title: "merge request" }, 'unmarked as a **Work In Progress**'
+        it_behaves_like 'system note creation', { title: "merge request" }, 'marked this merge request as **ready**'
 
         context 'and changing title' do
           before do
             issuable.update_attribute(:title, "changed title")
           end
 
-          it_behaves_like 'draft notes creation', 'unmarked'
+          it_behaves_like 'draft notes creation', 'ready'
         end
       end
     end

spec/services/merge_requests/refresh_service_spec.rb

@@ -683,14 +683,14 @@ RSpec.describe MergeRequests::RefreshService do
       end
     end
 
-    context 'marking the merge request as work in progress' do
+    context 'marking the merge request as draft' do
       let(:refresh_service) { service.new(@project, @user) }
 
       before do
         allow(refresh_service).to receive(:execute_hooks)
       end
 
-      it 'marks the merge request as work in progress from fixup commits' do
+      it 'marks the merge request as draft from fixup commits' do
         fixup_merge_request = create(:merge_request,
                                      source_project: @project,
                                      source_branch: 'wip',
@@ -705,11 +705,11 @@ RSpec.describe MergeRequests::RefreshService do
 
         expect(fixup_merge_request.work_in_progress?).to eq(true)
         expect(fixup_merge_request.notes.last.note).to match(
-          /marked as a \*\*Work In Progress\*\* from #{Commit.reference_pattern}/
+          /marked this merge request as \*\*draft\*\* from #{Commit.reference_pattern}/
         )
       end
 
-      it 'references the commit that caused the Work in Progress status' do
+      it 'references the commit that caused the draft status' do
         wip_merge_request = create(:merge_request,
                                    source_project: @project,
                                    source_branch: 'wip',
@@ -724,11 +724,11 @@ RSpec.describe MergeRequests::RefreshService do
         refresh_service.execute(oldrev, newrev, 'refs/heads/wip')
 
         expect(wip_merge_request.reload.notes.last.note).to eq(
-          "marked as a **Work In Progress** from #{wip_commit.id}"
+          "marked this merge request as **draft** from #{wip_commit.id}"
         )
       end
 
-      it 'does not mark as WIP based on commits that do not belong to an MR' do
+      it 'does not mark as draft based on commits that do not belong to an MR' do
         allow(refresh_service).to receive(:find_new_commits)
         refresh_service.instance_variable_set("@commits", [
           double(

spec/services/system_note_service_spec.rb

@@ -566,25 +566,25 @@ RSpec.describe SystemNoteService do
     end
   end
 
-  describe '.handle_merge_request_wip' do
+  describe '.handle_merge_request_draft' do
     it 'calls MergeRequestsService' do
       expect_next_instance_of(::SystemNotes::MergeRequestsService) do |service|
-        expect(service).to receive(:handle_merge_request_wip)
+        expect(service).to receive(:handle_merge_request_draft)
       end
 
-      described_class.handle_merge_request_wip(noteable, project, author)
+      described_class.handle_merge_request_draft(noteable, project, author)
     end
   end
 
-  describe '.add_merge_request_wip_from_commit' do
+  describe '.add_merge_request_draft_from_commit' do
     it 'calls MergeRequestsService' do
       commit = double
 
       expect_next_instance_of(::SystemNotes::MergeRequestsService) do |service|
-        expect(service).to receive(:add_merge_request_wip_from_commit).with(commit)
+        expect(service).to receive(:add_merge_request_draft_from_commit).with(commit)
       end
 
-      described_class.add_merge_request_wip_from_commit(noteable, project, author, commit)
+      described_class.add_merge_request_draft_from_commit(noteable, project, author, commit)
     end
   end
 

spec/services/system_notes/merge_requests_service_spec.rb

@@ -51,44 +51,44 @@ RSpec.describe ::SystemNotes::MergeRequestsService do
     end
   end
 
-  describe '.handle_merge_request_wip' do
+  describe '.handle_merge_request_draft' do
     context 'adding draft note' do
       let(:noteable) { create(:merge_request, source_project: project, title: 'Draft: Lorem ipsum') }
 
-      subject { service.handle_merge_request_wip }
+      subject { service.handle_merge_request_draft }
 
       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**'
+        expect(subject.note).to eq 'marked this merge request as **draft**'
       end
     end
 
-    context 'removing wip note' do
-      subject { service.handle_merge_request_wip }
+    context 'removing draft note' do
+      subject { service.handle_merge_request_draft }
 
       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**'
+        expect(subject.note).to eq 'marked this merge request as **ready**'
       end
     end
   end
 
-  describe '.add_merge_request_wip_from_commit' do
-    subject { service.add_merge_request_wip_from_commit(noteable.diff_head_commit) }
+  describe '.add_merge_request_draft_from_commit' do
+    subject { service.add_merge_request_draft_from_commit(noteable.diff_head_commit) }
 
     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 this merge request as draft from commit' system note" do
       expect(subject.note).to match(
-        /marked as a \*\*Work In Progress\*\* from #{Commit.reference_pattern}/
+        /marked this merge request as \*\*draft\*\* from #{Commit.reference_pattern}/
       )
     end
   end

spec/support/shared_examples/services/common_system_notes_shared_examples.rb

@@ -17,13 +17,13 @@ RSpec.shared_examples 'system note creation' do |update_params, note_text|
   end
 end
 
-RSpec.shared_examples 'draft notes creation' do |wip_action|
+RSpec.shared_examples 'draft notes creation' do |action|
   subject { described_class.new(project, user).execute(issuable, old_labels: []) }
 
   it 'creates Draft toggle and title change notes' do
     expect { subject }.to change { Note.count }.from(0).to(2)
 
-    expect(Note.first.note).to match("#{wip_action} as a **Work In Progress**")
+    expect(Note.first.note).to match("marked this merge request as **#{action}**")
     expect(Note.second.note).to match('changed title')
   end
 end