Commit 55d18a5b authored by Patrick Bajao's avatar Patrick Bajao

Update system note when marking merge request as draft or ready

Since we deprecated the use of WIP, we are also updating the
system notes we generate when a merge request is marked/unmarked
as draft.

This changes some method calls and test descriptions to mention
draft instead of wip as well.
parent 8492a3dd
...@@ -51,11 +51,11 @@ module Issuable ...@@ -51,11 +51,11 @@ module Issuable
end end
end end
def create_wip_note(old_title) def create_draft_note(old_title)
return unless issuable.is_a?(MergeRequest) return unless issuable.is_a?(MergeRequest)
if MergeRequest.work_in_progress?(old_title) != issuable.work_in_progress? 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
end end
...@@ -69,7 +69,7 @@ module Issuable ...@@ -69,7 +69,7 @@ module Issuable
end end
def create_title_change_note(old_title) def create_title_change_note(old_title)
create_wip_note(old_title) create_draft_note(old_title)
if issuable.wipless_title_changed(old_title) if issuable.wipless_title_changed(old_title)
SystemNoteService.change_title(issuable, issuable.project, current_user, old_title) SystemNoteService.change_title(issuable, issuable.project, current_user, old_title)
......
...@@ -42,7 +42,7 @@ module MergeRequests ...@@ -42,7 +42,7 @@ module MergeRequests
end end
notify_about_push(mr) notify_about_push(mr)
mark_mr_as_wip_from_commits(mr) mark_mr_as_draft_from_commits(mr)
execute_mr_web_hooks(mr) execute_mr_web_hooks(mr)
end end
...@@ -246,7 +246,7 @@ module MergeRequests ...@@ -246,7 +246,7 @@ module MergeRequests
notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits) notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits)
end end
def mark_mr_as_wip_from_commits(merge_request) def mark_mr_as_draft_from_commits(merge_request)
return unless @commits.present? return unless @commits.present?
commit_shas = merge_request.commit_shas commit_shas = merge_request.commit_shas
...@@ -257,7 +257,7 @@ module MergeRequests ...@@ -257,7 +257,7 @@ module MergeRequests
if wip_commit && !merge_request.work_in_progress? if wip_commit && !merge_request.work_in_progress?
merge_request.update(title: merge_request.wip_title) 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,
merge_request.project, merge_request.project,
@current_user, @current_user,
......
...@@ -130,12 +130,12 @@ module SystemNoteService ...@@ -130,12 +130,12 @@ module SystemNoteService
::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).abort_merge_when_pipeline_succeeds(reason) ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).abort_merge_when_pipeline_succeeds(reason)
end end
def handle_merge_request_wip(noteable, project, author) def handle_merge_request_draft(noteable, project, author)
::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).handle_merge_request_wip ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).handle_merge_request_draft
end end
def add_merge_request_wip_from_commit(noteable, project, author, commit) def add_merge_request_draft_from_commit(noteable, project, author, commit)
::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).add_merge_request_wip_from_commit(commit) ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).add_merge_request_draft_from_commit(commit)
end end
def resolve_all_discussions(merge_request, project, author) def resolve_all_discussions(merge_request, project, author)
......
...@@ -26,16 +26,16 @@ module SystemNotes ...@@ -26,16 +26,16 @@ module SystemNotes
create_note(NoteSummary.new(noteable, project, author, body, action: 'merge')) create_note(NoteSummary.new(noteable, project, author, body, action: 'merge'))
end end
def handle_merge_request_wip def handle_merge_request_draft
prefix = noteable.work_in_progress? ? "marked" : "unmarked" 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')) create_note(NoteSummary.new(noteable, project, author, body, action: 'title'))
end end
def add_merge_request_wip_from_commit(commit) def add_merge_request_draft_from_commit(commit)
body = "marked as a **Work In Progress** from #{commit.to_reference(project)}" body = "marked this merge request as **draft** from #{commit.to_reference(project)}"
create_note(NoteSummary.new(noteable, project, author, body, action: 'title')) create_note(NoteSummary.new(noteable, project, author, body, action: 'title'))
end end
......
---
title: Update system note when marking merge request as draft or ready
merge_request: 45644
author:
type: changed
...@@ -36,28 +36,28 @@ RSpec.describe Issuable::CommonSystemNotesService do ...@@ -36,28 +36,28 @@ RSpec.describe Issuable::CommonSystemNotesService do
context 'adding Draft note' do context 'adding Draft note' do
let(:issuable) { create(:merge_request, title: "merge request") } 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 context 'and changing title' do
before do before do
issuable.update_attribute(:title, "Draft: changed title") issuable.update_attribute(:title, "Draft: changed title")
end end
it_behaves_like 'draft notes creation', 'marked' it_behaves_like 'draft notes creation', 'draft'
end end
end end
context 'removing Draft note' do context 'removing Draft note' do
let(:issuable) { create(:merge_request, title: "Draft: merge request") } 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 context 'and changing title' do
before do before do
issuable.update_attribute(:title, "changed title") issuable.update_attribute(:title, "changed title")
end end
it_behaves_like 'draft notes creation', 'unmarked' it_behaves_like 'draft notes creation', 'ready'
end end
end end
end end
......
...@@ -683,14 +683,14 @@ RSpec.describe MergeRequests::RefreshService do ...@@ -683,14 +683,14 @@ RSpec.describe MergeRequests::RefreshService do
end end
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) } let(:refresh_service) { service.new(@project, @user) }
before do before do
allow(refresh_service).to receive(:execute_hooks) allow(refresh_service).to receive(:execute_hooks)
end 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, fixup_merge_request = create(:merge_request,
source_project: @project, source_project: @project,
source_branch: 'wip', source_branch: 'wip',
...@@ -705,11 +705,11 @@ RSpec.describe MergeRequests::RefreshService do ...@@ -705,11 +705,11 @@ RSpec.describe MergeRequests::RefreshService do
expect(fixup_merge_request.work_in_progress?).to eq(true) expect(fixup_merge_request.work_in_progress?).to eq(true)
expect(fixup_merge_request.notes.last.note).to match( 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 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, wip_merge_request = create(:merge_request,
source_project: @project, source_project: @project,
source_branch: 'wip', source_branch: 'wip',
...@@ -724,11 +724,11 @@ RSpec.describe MergeRequests::RefreshService do ...@@ -724,11 +724,11 @@ RSpec.describe MergeRequests::RefreshService do
refresh_service.execute(oldrev, newrev, 'refs/heads/wip') refresh_service.execute(oldrev, newrev, 'refs/heads/wip')
expect(wip_merge_request.reload.notes.last.note).to eq( 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 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) allow(refresh_service).to receive(:find_new_commits)
refresh_service.instance_variable_set("@commits", [ refresh_service.instance_variable_set("@commits", [
double( double(
......
...@@ -566,25 +566,25 @@ RSpec.describe SystemNoteService do ...@@ -566,25 +566,25 @@ RSpec.describe SystemNoteService do
end end
end end
describe '.handle_merge_request_wip' do describe '.handle_merge_request_draft' do
it 'calls MergeRequestsService' do it 'calls MergeRequestsService' do
expect_next_instance_of(::SystemNotes::MergeRequestsService) do |service| 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 end
described_class.handle_merge_request_wip(noteable, project, author) described_class.handle_merge_request_draft(noteable, project, author)
end end
end end
describe '.add_merge_request_wip_from_commit' do describe '.add_merge_request_draft_from_commit' do
it 'calls MergeRequestsService' do it 'calls MergeRequestsService' do
commit = double commit = double
expect_next_instance_of(::SystemNotes::MergeRequestsService) do |service| 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 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
end end
......
...@@ -51,44 +51,44 @@ RSpec.describe ::SystemNotes::MergeRequestsService do ...@@ -51,44 +51,44 @@ RSpec.describe ::SystemNotes::MergeRequestsService do
end end
end end
describe '.handle_merge_request_wip' do describe '.handle_merge_request_draft' do
context 'adding draft note' do context 'adding draft note' do
let(:noteable) { create(:merge_request, source_project: project, title: 'Draft: Lorem ipsum') } 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 it_behaves_like 'a system note' do
let(:action) { 'title' } let(:action) { 'title' }
end end
it 'sets the note text' do 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
end end
context 'removing wip note' do context 'removing draft note' do
subject { service.handle_merge_request_wip } subject { service.handle_merge_request_draft }
it_behaves_like 'a system note' do it_behaves_like 'a system note' do
let(:action) { 'title' } let(:action) { 'title' }
end end
it 'sets the note text' do 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 end
end end
describe '.add_merge_request_wip_from_commit' do describe '.add_merge_request_draft_from_commit' do
subject { service.add_merge_request_wip_from_commit(noteable.diff_head_commit) } subject { service.add_merge_request_draft_from_commit(noteable.diff_head_commit) }
it_behaves_like 'a system note' do it_behaves_like 'a system note' do
let(:action) { 'title' } let(:action) { 'title' }
end 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( 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
end end
......
...@@ -17,13 +17,13 @@ RSpec.shared_examples 'system note creation' do |update_params, note_text| ...@@ -17,13 +17,13 @@ RSpec.shared_examples 'system note creation' do |update_params, note_text|
end end
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: []) } subject { described_class.new(project, user).execute(issuable, old_labels: []) }
it 'creates Draft toggle and title change notes' do it 'creates Draft toggle and title change notes' do
expect { subject }.to change { Note.count }.from(0).to(2) 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') expect(Note.second.note).to match('changed title')
end end
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