Commit c8cd76e4 authored by Stan Hu's avatar Stan Hu

Merge branch 'id-draft-wip' into 'master'

Allow prefixing with Draft to mark MR as WIP

See merge request gitlab-org/gitlab!35940
parents 97dda8cd 180d10fc
...@@ -469,10 +469,12 @@ class Commit ...@@ -469,10 +469,12 @@ class Commit
# We don't want to do anything for `Commit` model, so this is empty. # We don't want to do anything for `Commit` model, so this is empty.
end end
WIP_REGEX = /\A\s*(((?i)(\[WIP\]|WIP:|WIP)\s|WIP$))|(fixup!|squash!)\s/.freeze # WIP is deprecated in favor of Draft. Currently both options are supported
# https://gitlab.com/gitlab-org/gitlab/-/issues/227426
DRAFT_REGEX = /\A\s*#{Regexp.union(Gitlab::Regex.merge_request_wip, Gitlab::Regex.merge_request_draft)}|(fixup!|squash!)\s/.freeze
def work_in_progress? def work_in_progress?
!!(title =~ WIP_REGEX) !!(title =~ DRAFT_REGEX)
end end
def merged_merge_request?(user) def merged_merge_request?(user)
......
...@@ -391,25 +391,27 @@ class MergeRequest < ApplicationRecord ...@@ -391,25 +391,27 @@ class MergeRequest < ApplicationRecord
end end
end end
WIP_REGEX = /\A*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze # WIP is deprecated in favor of Draft. Currently both options are supported
# https://gitlab.com/gitlab-org/gitlab/-/issues/227426
DRAFT_REGEX = /\A*#{Regexp.union(Gitlab::Regex.merge_request_wip, Gitlab::Regex.merge_request_draft)}+\s*/i.freeze
def self.work_in_progress?(title) def self.work_in_progress?(title)
!!(title =~ WIP_REGEX) !!(title =~ DRAFT_REGEX)
end end
def self.wipless_title(title) def self.wipless_title(title)
title.sub(WIP_REGEX, "") title.sub(DRAFT_REGEX, "")
end end
def self.wip_title(title) def self.wip_title(title)
work_in_progress?(title) ? title : "WIP: #{title}" work_in_progress?(title) ? title : "Draft: #{title}"
end end
def committers def committers
@committers ||= commits.committers @committers ||= commits.committers
end end
# Verifies if title has changed not taking into account WIP prefix # Verifies if title has changed not taking into account Draft prefix
# for merge requests. # for merge requests.
def wipless_title_changed(old_title) def wipless_title_changed(old_title)
self.class.wipless_title(old_title) != self.wipless_title self.class.wipless_title(old_title) != self.wipless_title
......
---
title: Allow prefixing with Draft to mark MR as WIP
merge_request: 35940
author:
type: added
...@@ -250,6 +250,14 @@ module Gitlab ...@@ -250,6 +250,14 @@ module Gitlab
@utc_date_regex ||= /\A[0-9]{4}-[0-9]{2}-[0-9]{2}\z/.freeze @utc_date_regex ||= /\A[0-9]{4}-[0-9]{2}-[0-9]{2}\z/.freeze
end end
def merge_request_wip
/(?i)(\[WIP\]\s*|WIP:\s*|WIP\s+|WIP$)/
end
def merge_request_draft
/(?i)(\[draft\]|\(draft\)|draft:|draft\s\-\s|draft\s|draft$)/
end
def issue def issue
@issue ||= /(?<issue>\d+\b)/ @issue ||= /(?<issue>\d+\b)/
end end
......
...@@ -71,7 +71,7 @@ RSpec.describe 'User creates branch and merge request on issue page', :js do ...@@ -71,7 +71,7 @@ RSpec.describe 'User creates branch and merge request on issue page', :js do
perform_enqueued_jobs do perform_enqueued_jobs do
select_dropdown_option('create-mr') select_dropdown_option('create-mr')
expect(page).to have_content('WIP: Resolve "Cherry-Coloured Funk"') expect(page).to have_content('Draft: Resolve "Cherry-Coloured Funk"')
expect(current_path).to eq(project_merge_request_path(project, MergeRequest.first)) expect(current_path).to eq(project_merge_request_path(project, MergeRequest.first))
wait_for_requests wait_for_requests
...@@ -100,7 +100,7 @@ RSpec.describe 'User creates branch and merge request on issue page', :js do ...@@ -100,7 +100,7 @@ RSpec.describe 'User creates branch and merge request on issue page', :js do
perform_enqueued_jobs do perform_enqueued_jobs do
select_dropdown_option('create-mr', branch_name) select_dropdown_option('create-mr', branch_name)
expect(page).to have_content('WIP: Resolve "Cherry-Coloured Funk"') expect(page).to have_content('Draft: Resolve "Cherry-Coloured Funk"')
expect(page).to have_content('Request to merge custom-branch-name into') expect(page).to have_content('Request to merge custom-branch-name into')
expect(current_path).to eq(project_merge_request_path(project, MergeRequest.first)) expect(current_path).to eq(project_merge_request_path(project, MergeRequest.first))
......
...@@ -675,7 +675,10 @@ eos ...@@ -675,7 +675,10 @@ eos
end end
describe '#work_in_progress?' do describe '#work_in_progress?' do
['squash! ', 'fixup! ', 'wip: ', 'WIP: ', '[WIP] '].each do |wip_prefix| [
'squash! ', 'fixup! ', 'wip: ', 'WIP: ', '[WIP] ',
'draft: ', 'Draft - ', '[Draft] ', '(draft) ', 'Draft: '
].each do |wip_prefix|
it "detects the '#{wip_prefix}' prefix" do it "detects the '#{wip_prefix}' prefix" do
commit.message = "#{wip_prefix}#{commit.message}" commit.message = "#{wip_prefix}#{commit.message}"
...@@ -689,6 +692,12 @@ eos ...@@ -689,6 +692,12 @@ eos
expect(commit).to be_work_in_progress expect(commit).to be_work_in_progress
end end
it "detects WIP for a commit just saying 'draft'" do
commit.message = "draft"
expect(commit).to be_work_in_progress
end
it "doesn't detect WIP for a commit that begins with 'FIXUP! '" do it "doesn't detect WIP for a commit that begins with 'FIXUP! '" do
commit.message = "FIXUP! #{commit.message}" commit.message = "FIXUP! #{commit.message}"
......
...@@ -1109,13 +1109,31 @@ RSpec.describe MergeRequest do ...@@ -1109,13 +1109,31 @@ RSpec.describe MergeRequest do
end end
describe "#work_in_progress?" do describe "#work_in_progress?" do
['WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP [WIP] WIP: WIP '].each do |wip_prefix| subject { build_stubbed(:merge_request) }
[
'WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP [WIP] WIP: WIP ',
'Draft ', 'draft:', 'Draft: ', '[Draft]', '[DRAFT] ', 'Draft - '
].each do |wip_prefix|
it "detects the '#{wip_prefix}' prefix" do it "detects the '#{wip_prefix}' prefix" do
subject.title = "#{wip_prefix}#{subject.title}" subject.title = "#{wip_prefix}#{subject.title}"
expect(subject.work_in_progress?).to eq true expect(subject.work_in_progress?).to eq true
end end
end end
it "detects merge request title just saying 'wip'" do
subject.title = "wip"
expect(subject.work_in_progress?).to eq true
end
it "detects merge request title just saying 'draft'" do
subject.title = "draft"
expect(subject.work_in_progress?).to eq true
end
it "doesn't detect WIP for words starting with WIP" do it "doesn't detect WIP for words starting with WIP" do
subject.title = "Wipwap #{subject.title}" subject.title = "Wipwap #{subject.title}"
expect(subject.work_in_progress?).to eq false expect(subject.work_in_progress?).to eq false
...@@ -1132,7 +1150,12 @@ RSpec.describe MergeRequest do ...@@ -1132,7 +1150,12 @@ RSpec.describe MergeRequest do
end end
describe "#wipless_title" do describe "#wipless_title" do
['WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', '[WIP] WIP [WIP] WIP: WIP '].each do |wip_prefix| subject { build_stubbed(:merge_request) }
[
'WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', '[WIP] WIP [WIP] WIP: WIP ',
'Draft ', 'draft:', 'Draft: ', '[Draft]', '[DRAFT] ', 'Draft - '
].each do |wip_prefix|
it "removes the '#{wip_prefix}' prefix" do it "removes the '#{wip_prefix}' prefix" do
wipless_title = subject.title wipless_title = subject.title
subject.title = "#{wip_prefix}#{subject.title}" subject.title = "#{wip_prefix}#{subject.title}"
...@@ -1150,14 +1173,14 @@ RSpec.describe MergeRequest do ...@@ -1150,14 +1173,14 @@ RSpec.describe MergeRequest do
end end
describe "#wip_title" do describe "#wip_title" do
it "adds the WIP: prefix to the title" do it "adds the Draft: prefix to the title" do
wip_title = "WIP: #{subject.title}" wip_title = "Draft: #{subject.title}"
expect(subject.wip_title).to eq wip_title expect(subject.wip_title).to eq wip_title
end end
it "does not add the WIP: prefix multiple times" do it "does not add the Draft: prefix multiple times" do
wip_title = "WIP: #{subject.title}" wip_title = "Draft: #{subject.title}"
subject.title = subject.wip_title subject.title = subject.wip_title
subject.title = subject.wip_title subject.title = subject.wip_title
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Setting WIP status of a merge request' do RSpec.describe 'Setting Draft status of a merge request' do
include GraphqlHelpers include GraphqlHelpers
let(:current_user) { create(:user) } let(:current_user) { create(:user) }
...@@ -41,39 +41,39 @@ RSpec.describe 'Setting WIP status of a merge request' do ...@@ -41,39 +41,39 @@ RSpec.describe 'Setting WIP status of a merge request' do
expect(graphql_errors).not_to be_empty expect(graphql_errors).not_to be_empty
end end
it 'marks the merge request as WIP' do it 'marks the merge request as Draft' do
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
expect(response).to have_gitlab_http_status(:success) expect(response).to have_gitlab_http_status(:success)
expect(mutation_response['mergeRequest']['title']).to start_with('WIP:') expect(mutation_response['mergeRequest']['title']).to start_with('Draft:')
end end
it 'does not do anything if the merge request was already marked `WIP`' do it 'does not do anything if the merge request was already marked `Draft`' do
merge_request.update!(title: 'wip: hello world') merge_request.update!(title: 'draft: hello world')
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
expect(response).to have_gitlab_http_status(:success) expect(response).to have_gitlab_http_status(:success)
expect(mutation_response['mergeRequest']['title']).to start_with('wip:') expect(mutation_response['mergeRequest']['title']).to start_with('draft:')
end end
context 'when passing WIP false as input' do context 'when passing Draft false as input' do
let(:input) { { wip: false } } let(:input) { { wip: false } }
it 'does not do anything if the merge reqeust was not marked wip' do it 'does not do anything if the merge reqeust was not marked draft' do
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
expect(response).to have_gitlab_http_status(:success) expect(response).to have_gitlab_http_status(:success)
expect(mutation_response['mergeRequest']['title']).not_to start_with(/wip\:/) expect(mutation_response['mergeRequest']['title']).not_to start_with(/draft\:/)
end end
it 'unmarks the merge request as `WIP`' do it 'unmarks the merge request as `Draft`' do
merge_request.update!(title: 'wip: hello world') merge_request.update!(title: 'draft: hello world')
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
expect(response).to have_gitlab_http_status(:success) expect(response).to have_gitlab_http_status(:success)
expect(mutation_response['mergeRequest']['title']).not_to start_with('/wip\:/') expect(mutation_response['mergeRequest']['title']).not_to start_with('/draft\:/')
end end
end end
end end
...@@ -189,8 +189,8 @@ RSpec.describe MergeRequests::BuildService do ...@@ -189,8 +189,8 @@ RSpec.describe MergeRequests::BuildService do
it_behaves_like 'allows the merge request to be created' it_behaves_like 'allows the merge request to be created'
it 'adds a WIP prefix to the merge request title' do it 'adds a Draft prefix to the merge request title' do
expect(merge_request.title).to eq('WIP: Feature branch') expect(merge_request.title).to eq('Draft: Feature branch')
end end
end end
......
...@@ -163,10 +163,10 @@ RSpec.describe MergeRequests::CreateFromIssueService do ...@@ -163,10 +163,10 @@ RSpec.describe MergeRequests::CreateFromIssueService do
expect(result[:merge_request].milestone_id).to eq(milestone_id) expect(result[:merge_request].milestone_id).to eq(milestone_id)
end end
it 'sets the merge request title to: "WIP: Resolves "$issue-title"' do it 'sets the merge request title to: "Draft: Resolves "$issue-title"' do
result = service.execute result = service.execute
expect(result[:merge_request].title).to eq("WIP: Resolve \"#{issue.title}\"") expect(result[:merge_request].title).to eq("Draft: Resolve \"#{issue.title}\"")
end end
end end
...@@ -193,10 +193,10 @@ RSpec.describe MergeRequests::CreateFromIssueService do ...@@ -193,10 +193,10 @@ RSpec.describe MergeRequests::CreateFromIssueService do
it_behaves_like 'a service that creates a merge request from an issue' it_behaves_like 'a service that creates a merge request from an issue'
it 'sets the merge request title to: "WIP: $issue-branch-name', :sidekiq_might_not_need_inline do it 'sets the merge request title to: "Draft: $issue-branch-name', :sidekiq_might_not_need_inline do
result = service.execute result = service.execute
expect(result[:merge_request].title).to eq("WIP: #{issue.to_branch_name.titleize.humanize}") expect(result[:merge_request].title).to eq("Draft: #{issue.to_branch_name.titleize.humanize}")
end end
end end
end end
......
...@@ -47,7 +47,7 @@ RSpec.shared_examples 'create_merge_request quick action' do ...@@ -47,7 +47,7 @@ RSpec.shared_examples 'create_merge_request quick action' do
expect(created_mr.source_branch).to eq(issue.to_branch_name) expect(created_mr.source_branch).to eq(issue.to_branch_name)
visit project_merge_request_path(project, created_mr) visit project_merge_request_path(project, created_mr)
expect(page).to have_content %{WIP: Resolve "#{issue.title}"} expect(page).to have_content %{Draft: Resolve "#{issue.title}"}
end end
it 'creates a merge request using the given branch name' do it 'creates a merge request using the given branch name' do
...@@ -60,7 +60,7 @@ RSpec.shared_examples 'create_merge_request quick action' do ...@@ -60,7 +60,7 @@ RSpec.shared_examples 'create_merge_request quick action' do
expect(created_mr.source_branch).to eq(branch_name) expect(created_mr.source_branch).to eq(branch_name)
visit project_merge_request_path(project, created_mr) visit project_merge_request_path(project, created_mr)
expect(page).to have_content %{WIP: Resolve "#{issue.title}"} expect(page).to have_content %{Draft: Resolve "#{issue.title}"}
end end
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