Commit 0b398bcf authored by Thomas Randolph's avatar Thomas Randolph Committed by Enrique Alcántara

Add Draft to WIP MRs

Also, default to Draft when the dynamic link is clicked
parent 12a419dc
...@@ -48,7 +48,19 @@ export default class IssuableForm { ...@@ -48,7 +48,19 @@ export default class IssuableForm {
this.renderWipExplanation = this.renderWipExplanation.bind(this); this.renderWipExplanation = this.renderWipExplanation.bind(this);
this.resetAutosave = this.resetAutosave.bind(this); this.resetAutosave = this.resetAutosave.bind(this);
this.handleSubmit = this.handleSubmit.bind(this); this.handleSubmit = this.handleSubmit.bind(this);
this.wipRegex = /^\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i; /* eslint-disable @gitlab/require-i18n-strings */
this.wipRegex = new RegExp(
'^\\s*(' + // Line start, then any amount of leading whitespace
'draft\\s-\\s' + // Draft_-_ where "_" are *exactly* one whitespace
'|\\[(draft|wip)\\]\\s*' + // [Draft] or [WIP] and any following whitespace
'|(draft|wip):\\s*' + // Draft: or WIP: and any following whitespace
'|(draft|wip)\\s+' + // Draft_ or WIP_ where "_" is at least one whitespace
'|\\(draft\\)\\s*' + // (Draft) and any following whitespace
')+' + // At least one repeated match of the preceding parenthetical
'\\s*', // Any amount of trailing whitespace
'i', // Match any case(s)
);
/* eslint-enable @gitlab/require-i18n-strings */
this.gfmAutoComplete = new GfmAutoComplete( this.gfmAutoComplete = new GfmAutoComplete(
gl.GfmAutoComplete && gl.GfmAutoComplete.dataSources, gl.GfmAutoComplete && gl.GfmAutoComplete.dataSources,
...@@ -131,9 +143,18 @@ export default class IssuableForm { ...@@ -131,9 +143,18 @@ export default class IssuableForm {
workInProgress() { workInProgress() {
return this.wipRegex.test(this.titleField.val()); return this.wipRegex.test(this.titleField.val());
} }
titlePrefixContainsDraft() {
const prefix = this.titleField.val().match(this.wipRegex);
return prefix && prefix[0].match(/draft/i);
}
renderWipExplanation() { renderWipExplanation() {
if (this.workInProgress()) { if (this.workInProgress()) {
// These strings are not "translatable" (the code is hard-coded to look for them)
this.$wipExplanation.find('code')[0].textContent = this.titlePrefixContainsDraft()
? 'Draft' /* eslint-disable-line @gitlab/require-i18n-strings */
: 'WIP';
this.$wipExplanation.show(); this.$wipExplanation.show();
return this.$noWipExplanation.hide(); return this.$noWipExplanation.hide();
} }
...@@ -156,7 +177,7 @@ export default class IssuableForm { ...@@ -156,7 +177,7 @@ export default class IssuableForm {
} }
addWip() { addWip() {
this.titleField.val(`WIP: ${this.titleField.val()}`); this.titleField.val(`Draft: ${this.titleField.val()}`);
} }
initTargetBranchDropdown() { initTargetBranchDropdown() {
......
<script> <script>
import $ from 'jquery'; import $ from 'jquery';
import { GlDeprecatedButton } from '@gitlab/ui'; import { GlButton } from '@gitlab/ui';
import { __, s__ } from '~/locale'; import { __ } from '~/locale';
import createFlash from '~/flash'; import createFlash from '~/flash';
import StatusIcon from '../mr_widget_status_icon.vue'; import StatusIcon from '../mr_widget_status_icon.vue';
import tooltip from '../../../vue_shared/directives/tooltip'; import tooltip from '../../../vue_shared/directives/tooltip';
...@@ -11,7 +11,7 @@ export default { ...@@ -11,7 +11,7 @@ export default {
name: 'WorkInProgress', name: 'WorkInProgress',
components: { components: {
StatusIcon, StatusIcon,
GlDeprecatedButton, GlButton,
}, },
directives: { directives: {
tooltip, tooltip,
...@@ -25,13 +25,6 @@ export default { ...@@ -25,13 +25,6 @@ export default {
isMakingRequest: false, isMakingRequest: false,
}; };
}, },
computed: {
wipInfoTooltip() {
return s__(
'mrWidget|When this merge request is ready, remove the WIP: prefix from the title to allow it to be merged',
);
},
},
methods: { methods: {
handleRemoveWIP() { handleRemoveWIP() {
this.isMakingRequest = true; this.isMakingRequest = true;
...@@ -55,28 +48,25 @@ export default { ...@@ -55,28 +48,25 @@ export default {
<template> <template>
<div class="mr-widget-body media"> <div class="mr-widget-body media">
<status-icon :show-disabled-button="Boolean(mr.removeWIPPath)" status="warning" /> <status-icon :show-disabled-button="Boolean(mr.removeWIPPath)" status="warning" />
<div class="media-body space-children"> <div class="media-body">
<span class="bold"> <div class="gl-ml-3 float-left">
{{ __('This is a Work in Progress') }} <span class="gl-font-weight-bold">
<i {{ __('This merge request is still a work in progress.') }}
v-tooltip </span>
class="fa fa-question-circle" <span class="gl-display-block text-muted">{{
:title="wipInfoTooltip" __("Draft merge requests can't be merged.")
:aria-label="wipInfoTooltip" }}</span>
> </div>
</i> <gl-button
</span>
<gl-deprecated-button
v-if="mr.removeWIPPath" v-if="mr.removeWIPPath"
size="sm" size="small"
variant="default"
:disabled="isMakingRequest" :disabled="isMakingRequest"
:loading="isMakingRequest" :loading="isMakingRequest"
class="js-remove-wip" class="js-remove-wip gl-ml-3"
@click="handleRemoveWIP" @click="handleRemoveWIP"
> >
{{ s__('mrWidget|Resolve WIP status') }} {{ s__('mrWidget|Mark as ready') }}
</gl-deprecated-button> </gl-button>
</div> </div>
</div> </div>
</template> </template>
...@@ -3,6 +3,13 @@ ...@@ -3,6 +3,13 @@
- form = local_assigns.fetch(:form) - form = local_assigns.fetch(:form)
- no_issuable_templates = issuable_templates(issuable).empty? - no_issuable_templates = issuable_templates(issuable).empty?
- div_class = no_issuable_templates ? 'col-sm-10' : 'col-sm-7 col-lg-8' - div_class = no_issuable_templates ? 'col-sm-10' : 'col-sm-7 col-lg-8'
- toggle_wip_link_start = '<a href="" class="js-toggle-wip">'
- toggle_wip_link_end = '</a>'
- draft_snippet = '<code>Draft:</code>'.html_safe
- wip_snippet = '<code>WIP:</code>'.html_safe
- draft_or_wip_snippet = '<code>Draft/WIP</code>'.html_safe
- add_wip_text = (_('%{link_start}Start the title with %{draft_snippet} or %{wip_snippet}%{link_end} to prevent a merge request that is a work in progress from being merged before it\'s ready.') % { link_start: toggle_wip_link_start, link_end: toggle_wip_link_end, draft_snippet: draft_snippet, wip_snippet: wip_snippet } ).html_safe
- remove_wip_text = (_('%{link_start}Remove the %{draft_or_wip_snippet} prefix%{link_end} from the title to allow this merge request to be merged when it\'s ready.' ) % { link_start: toggle_wip_link_start, link_end: toggle_wip_link_end, draft_or_wip_snippet: draft_or_wip_snippet } ).html_safe
%div{ class: div_class } %div{ class: div_class }
= form.text_field :title, required: true, maxlength: 255, autofocus: true, = form.text_field :title, required: true, maxlength: 255, autofocus: true,
...@@ -11,23 +18,12 @@ ...@@ -11,23 +18,12 @@
- if issuable.respond_to?(:work_in_progress?) - if issuable.respond_to?(:work_in_progress?)
.form-text.text-muted .form-text.text-muted
.js-wip-explanation .js-wip-explanation
%a.js-toggle-wip{ href: '' } = remove_wip_text
Remove the
%code WIP:
prefix from the title
to allow this
%strong Work In Progress
merge request to be merged when it's ready.
.js-no-wip-explanation .js-no-wip-explanation
- if has_wip_commits - if has_wip_commits
It looks like you have some WIP commits in this branch. = _('It looks like you have some draft commits in this branch.')
%br %br
%a.js-toggle-wip{ href: '' } = add_wip_text
Start the title with
%code WIP:
to prevent a
%strong Work In Progress
merge request from being merged before it's ready.
- if no_issuable_templates && can?(current_user, :push_code, issuable.project) - if no_issuable_templates && can?(current_user, :push_code, issuable.project)
= render 'shared/issuable/form/default_templates' = render 'shared/issuable/form/default_templates'
---
title: Add Draft to WIP for work in progress merge requests
merge_request: 36666
author:
type: added
...@@ -474,6 +474,12 @@ msgstr "" ...@@ -474,6 +474,12 @@ msgstr ""
msgid "%{link_start}Read more%{link_end} about role permissions" msgid "%{link_start}Read more%{link_end} about role permissions"
msgstr "" msgstr ""
msgid "%{link_start}Remove the %{draft_or_wip_snippet} prefix%{link_end} from the title to allow this merge request to be merged when it's ready."
msgstr ""
msgid "%{link_start}Start the title with %{draft_snippet} or %{wip_snippet}%{link_end} to prevent a merge request that is a work in progress from being merged before it's ready."
msgstr ""
msgid "%{listToShow}, and %{awardsListLength} more." msgid "%{listToShow}, and %{awardsListLength} more."
msgstr "" msgstr ""
...@@ -8500,6 +8506,9 @@ msgstr "" ...@@ -8500,6 +8506,9 @@ msgstr ""
msgid "Downvotes" msgid "Downvotes"
msgstr "" msgstr ""
msgid "Draft merge requests can't be merged."
msgstr ""
msgid "Drop or %{linkStart}upload%{linkEnd} Designs to attach" msgid "Drop or %{linkStart}upload%{linkEnd} Designs to attach"
msgstr "" msgstr ""
...@@ -13141,6 +13150,9 @@ msgstr "" ...@@ -13141,6 +13150,9 @@ msgstr ""
msgid "Issue|Title" msgid "Issue|Title"
msgstr "" msgstr ""
msgid "It looks like you have some draft commits in this branch."
msgstr ""
msgid "It must have a header row and at least two columns: the first column is the issue title and the second column is the issue description. The separator is automatically detected." msgid "It must have a header row and at least two columns: the first column is the issue title and the second column is the issue description. The separator is automatically detected."
msgstr "" msgstr ""
...@@ -24155,9 +24167,6 @@ msgstr "" ...@@ -24155,9 +24167,6 @@ msgstr ""
msgid "This is a Premium feature" msgid "This is a Premium feature"
msgstr "" msgstr ""
msgid "This is a Work in Progress"
msgstr ""
msgid "This is a confidential %{noteableTypeText}." msgid "This is a confidential %{noteableTypeText}."
msgstr "" msgstr ""
...@@ -24311,6 +24320,9 @@ msgstr "" ...@@ -24311,6 +24320,9 @@ msgstr ""
msgid "This merge request is locked." msgid "This merge request is locked."
msgstr "" msgstr ""
msgid "This merge request is still a work in progress."
msgstr ""
msgid "This merge request was merged. To apply this suggestion, edit this file directly." msgid "This merge request was merged. To apply this suggestion, edit this file directly."
msgstr "" msgstr ""
...@@ -28412,6 +28424,9 @@ msgstr "" ...@@ -28412,6 +28424,9 @@ msgstr ""
msgid "mrWidget|Loading deployment statistics" msgid "mrWidget|Loading deployment statistics"
msgstr "" msgstr ""
msgid "mrWidget|Mark as ready"
msgstr ""
msgid "mrWidget|Mentions" msgid "mrWidget|Mentions"
msgstr "" msgstr ""
...@@ -28469,9 +28484,6 @@ msgstr "" ...@@ -28469,9 +28484,6 @@ msgstr ""
msgid "mrWidget|Request to merge" msgid "mrWidget|Request to merge"
msgstr "" msgstr ""
msgid "mrWidget|Resolve WIP status"
msgstr ""
msgid "mrWidget|Resolve conflicts" msgid "mrWidget|Resolve conflicts"
msgstr "" msgstr ""
...@@ -28550,9 +28562,6 @@ msgstr "" ...@@ -28550,9 +28562,6 @@ msgstr ""
msgid "mrWidget|Use %{linkStart}CI pipelines to test your code%{linkEnd} by simply adding a GitLab CI configuration file to your project. It only takes a minute to make your code more secure and robust." msgid "mrWidget|Use %{linkStart}CI pipelines to test your code%{linkEnd} by simply adding a GitLab CI configuration file to your project. It only takes a minute to make your code more secure and robust."
msgstr "" msgstr ""
msgid "mrWidget|When this merge request is ready, remove the WIP: prefix from the title to allow it to be merged"
msgstr ""
msgid "mrWidget|You are not allowed to edit this project directly. Please fork to make changes." msgid "mrWidget|You are not allowed to edit this project directly. Please fork to make changes."
msgstr "" msgstr ""
......
...@@ -32,9 +32,9 @@ RSpec.describe 'Merge request > User resolves Work in Progress', :js do ...@@ -32,9 +32,9 @@ RSpec.describe 'Merge request > User resolves Work in Progress', :js do
it 'retains merge request data after clicking Resolve WIP status' do it 'retains merge request data after clicking Resolve WIP status' do
expect(page.find('.ci-widget-content')).to have_content("Pipeline ##{pipeline.id}") expect(page.find('.ci-widget-content')).to have_content("Pipeline ##{pipeline.id}")
expect(page).to have_content "This is a Work in Progress" expect(page).to have_content "This merge request is still a work in progress."
click_button('Resolve WIP status') click_button('Mark as ready')
wait_for_requests wait_for_requests
...@@ -42,7 +42,7 @@ RSpec.describe 'Merge request > User resolves Work in Progress', :js do ...@@ -42,7 +42,7 @@ RSpec.describe 'Merge request > User resolves Work in Progress', :js do
# merge request widget refreshes, which masks missing elements # merge request widget refreshes, which masks missing elements
# that should already be present. # that should already be present.
expect(page.find('.ci-widget-content', wait: 0)).to have_content("Pipeline ##{pipeline.id}") expect(page.find('.ci-widget-content', wait: 0)).to have_content("Pipeline ##{pipeline.id}")
expect(page).not_to have_content('This is a Work in Progress') expect(page).not_to have_content('This merge request is still a work in progress.')
end end
end end
end end
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Merge request > User sees WIP help message' do RSpec.describe 'Merge request > User sees draft help message' do
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
let(:user) { project.creator } let(:user) { project.creator }
...@@ -11,8 +11,8 @@ RSpec.describe 'Merge request > User sees WIP help message' do ...@@ -11,8 +11,8 @@ RSpec.describe 'Merge request > User sees WIP help message' do
sign_in(user) sign_in(user)
end end
context 'with WIP commits' do context 'with draft commits' do
it 'shows a specific WIP hint' do it 'shows a specific draft hint' do
visit project_new_merge_request_path( visit project_new_merge_request_path(
project, project,
merge_request: { merge_request: {
...@@ -24,14 +24,14 @@ RSpec.describe 'Merge request > User sees WIP help message' do ...@@ -24,14 +24,14 @@ RSpec.describe 'Merge request > User sees WIP help message' do
within_wip_explanation do within_wip_explanation do
expect(page).to have_text( expect(page).to have_text(
'It looks like you have some WIP commits in this branch' 'It looks like you have some draft commits in this branch'
) )
end end
end end
end end
context 'without WIP commits' do context 'without draft commits' do
it 'shows the regular WIP message' do it 'shows the regular draft message' do
visit project_new_merge_request_path( visit project_new_merge_request_path(
project, project,
merge_request: { merge_request: {
...@@ -43,11 +43,11 @@ RSpec.describe 'Merge request > User sees WIP help message' do ...@@ -43,11 +43,11 @@ RSpec.describe 'Merge request > User sees WIP help message' do
within_wip_explanation do within_wip_explanation do
expect(page).not_to have_text( expect(page).not_to have_text(
'It looks like you have some WIP commits in this branch' 'It looks like you have some draft commits in this branch'
) )
expect(page).to have_text( expect(page).to have_text(
"Start the title with WIP: to prevent a Work In Progress merge \ "Start the title with Draft: or WIP: to prevent a merge request that is a \
request from being merged before it's ready" work in progress from being merged before it's ready."
) )
end end
end end
......
import $ from 'jquery';
import IssuableForm from '~/issuable_form';
function createIssuable() {
const instance = new IssuableForm($(document.createElement('form')));
instance.titleField = $(document.createElement('input'));
return instance;
}
describe('IssuableForm', () => {
let instance;
beforeEach(() => {
instance = createIssuable();
});
describe('removeWip', () => {
it.each`
prefix
${'wip '}
${' wIP: '}
${'[WIp] '}
${'wIP:'}
${' [WIp]'}
${'drAft '}
${'draFT: '}
${' [DRaft] '}
${'drAft:'}
${'[draFT]'}
${' dRaFt - '}
${'dRaFt - '}
${'(draft) '}
${' (DrafT)'}
${'wip wip: [wip] draft draft - draft: [draft] (draft)'}
`('removes "$prefix" from the beginning of the title', ({ prefix }) => {
instance.titleField.val(`${prefix}The Issuable's Title Value`);
instance.removeWip();
expect(instance.titleField.val()).toBe("The Issuable's Title Value");
});
});
describe('addWip', () => {
it("properly adds the work in progress prefix to the Issuable's title", () => {
instance.titleField.val("The Issuable's Title Value");
instance.addWip();
expect(instance.titleField.val()).toBe("Draft: The Issuable's Title Value");
});
});
});
...@@ -84,11 +84,11 @@ describe('Wip', () => { ...@@ -84,11 +84,11 @@ describe('Wip', () => {
it('should have correct elements', () => { it('should have correct elements', () => {
expect(el.classList.contains('mr-widget-body')).toBeTruthy(); expect(el.classList.contains('mr-widget-body')).toBeTruthy();
expect(el.innerText).toContain('This is a Work in Progress'); expect(el.innerText).toContain('This merge request is still a work in progress.');
expect(el.querySelector('button').getAttribute('disabled')).toBeTruthy(); expect(el.querySelector('button').getAttribute('disabled')).toBeTruthy();
expect(el.querySelector('button').innerText).toContain('Merge'); expect(el.querySelector('button').innerText).toContain('Merge');
expect(el.querySelector('.js-remove-wip').innerText.replace(/\s\s+/g, ' ')).toContain( expect(el.querySelector('.js-remove-wip').innerText.replace(/\s\s+/g, ' ')).toContain(
'Resolve WIP status', 'Mark as ready',
); );
}); });
......
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