Commit 42c43041 authored by Jacob Schatz's avatar Jacob Schatz Committed by Rémy Coutable

Merge branch 'remove-wip' into 'master'

Easily (un)mark merge request as WIP using link

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/3768 and https://gitlab.com/gitlab-org/gitlab-ce/issues/3516

## Link to add `WIP` prefix (underline is visible because of hover)
![wipless_title](/uploads/72a6f7119ba9d8043ca8329641e97c3b/wipless_title.png)

## Link to remove `WIP` prefix
![wip_title](/uploads/8620ad65da9ef620b180603520fead55/wip_title.png)

## System note after WIP is added
![wip_sysnote](/uploads/2de073b75e854d2c9e243eb8b5d5c259/wip_sysnote.png)

## Widget with link to remove WIP
![wip_widget](/uploads/cf83ea93743c4c26d9df759c17cb9d7b/wip_widget.png)

## Flash after WIP is removed
![wip_flash](/uploads/27b7240cd5d7ceeb8b7b477abd94d7ff/wip_flash.png)

## System note after WIP is removed
![wipless_sysnote](/uploads/c0d3368abdf21a2f253532a9a9594d90/wipless_sysnote.png)

## Widget when current user cannot remove the WIP prefix
![wip_widget_unauthorized](/uploads/174ccf1674be86dc81c3078fe297acb7/wip_widget_unauthorized.png)

cc @creamzy 

See merge request !3006
parent e6961c46
...@@ -8,6 +8,8 @@ v 8.6.0 (unreleased) ...@@ -8,6 +8,8 @@ v 8.6.0 (unreleased)
- New branch button appears on issues where applicable - New branch button appears on issues where applicable
- Contributions to forked projects are included in calendar - Contributions to forked projects are included in calendar
- Improve the formatting for the user page bio (Connor Shea) - Improve the formatting for the user page bio (Connor Shea)
- Easily (un)mark merge request as WIP using link
- Use specialized system notes when MR is (un)marked as WIP
- Removed the default password from the initial admin account created during - Removed the default password from the initial admin account created during
setup. A password can be provided during setup (see installation docs), or setup. A password can be provided during setup (see installation docs), or
GitLab will ask the user to create a new one upon first visit. GitLab will ask the user to create a new one upon first visit.
......
class @IssuableForm class @IssuableForm
wipRegex: /^\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i
constructor: (@form) -> constructor: (@form) ->
GitLab.GfmAutoComplete.setup() GitLab.GfmAutoComplete.setup()
new UsersSelect() new UsersSelect()
...@@ -14,6 +15,8 @@ class @IssuableForm ...@@ -14,6 +15,8 @@ class @IssuableForm
@form.on "submit", @resetAutosave @form.on "submit", @resetAutosave
@form.on "click", ".btn-cancel", @resetAutosave @form.on "click", ".btn-cancel", @resetAutosave
@initWip()
initAutosave: -> initAutosave: ->
new Autosave @titleField, [ new Autosave @titleField, [
document.location.pathname, document.location.pathname,
...@@ -30,3 +33,41 @@ class @IssuableForm ...@@ -30,3 +33,41 @@ class @IssuableForm
resetAutosave: => resetAutosave: =>
@titleField.data("autosave").reset() @titleField.data("autosave").reset()
@descriptionField.data("autosave").reset() @descriptionField.data("autosave").reset()
initWip: ->
@$wipExplanation = @form.find(".js-wip-explanation")
@$noWipExplanation = @form.find(".js-no-wip-explanation")
return unless @$wipExplanation.length and @$noWipExplanation.length
@form.on "click", ".js-toggle-wip", @toggleWip
@titleField.on "keyup blur", @renderWipExplanation
@renderWipExplanation()
workInProgress: ->
@wipRegex.test @titleField.val()
renderWipExplanation: =>
if @workInProgress()
@$wipExplanation.show()
@$noWipExplanation.hide()
else
@$wipExplanation.hide()
@$noWipExplanation.show()
toggleWip: (event) =>
event.preventDefault()
if @workInProgress()
@removeWip()
else
@addWip()
@renderWipExplanation()
removeWip: ->
@titleField.val @titleField.val().replace(@wipRegex, "")
addWip: ->
@titleField.val "WIP: #{@titleField.val()}"
...@@ -5,7 +5,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -5,7 +5,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
before_action :module_enabled before_action :module_enabled
before_action :merge_request, only: [ before_action :merge_request, only: [
:edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check, :edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check,
:ci_status, :cancel_merge_when_build_succeeds :ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip
] ]
before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits, :builds] before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits, :builds]
before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds] before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds]
...@@ -20,7 +20,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -20,7 +20,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
before_action :authorize_create_merge_request!, only: [:new, :create] before_action :authorize_create_merge_request!, only: [:new, :create]
# Allow modify merge_request # Allow modify merge_request
before_action :authorize_update_merge_request!, only: [:close, :edit, :update, :sort] before_action :authorize_update_merge_request!, only: [:close, :edit, :update, :remove_wip, :sort]
def index def index
terms = params['issue_search'] terms = params['issue_search']
...@@ -164,6 +164,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -164,6 +164,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end end
end end
def remove_wip
MergeRequests::UpdateService.new(project, current_user, title: @merge_request.wipless_title).execute(@merge_request)
redirect_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request),
notice: "The merge request can now be merged."
end
def merge_check def merge_check
@merge_request.check_if_can_be_merged @merge_request.check_if_can_be_merged
......
...@@ -277,8 +277,14 @@ class MergeRequest < ActiveRecord::Base ...@@ -277,8 +277,14 @@ class MergeRequest < ActiveRecord::Base
self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
end end
WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze
def work_in_progress? def work_in_progress?
!!(title =~ /\A\[?WIP(\]|:| )/i) title =~ WIP_REGEX
end
def wipless_title
self.title.sub(WIP_REGEX, "")
end end
def mergeable? def mergeable?
......
...@@ -5,6 +5,19 @@ module MergeRequests ...@@ -5,6 +5,19 @@ module MergeRequests
SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, merge_request.state, nil) SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
end end
def create_title_change_note(issuable, old_title)
removed_wip = old_title =~ MergeRequest::WIP_REGEX && !issuable.work_in_progress?
added_wip = old_title !~ MergeRequest::WIP_REGEX && issuable.work_in_progress?
if removed_wip
SystemNoteService.remove_merge_request_wip(issuable, issuable.project, current_user)
elsif added_wip
SystemNoteService.add_merge_request_wip(issuable, issuable.project, current_user)
else
super
end
end
def hook_data(merge_request, action) def hook_data(merge_request, action)
hook_data = merge_request.to_hook_data(current_user) hook_data = merge_request.to_hook_data(current_user)
merge_request_url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id) merge_request_url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id)
......
...@@ -144,6 +144,18 @@ class SystemNoteService ...@@ -144,6 +144,18 @@ class SystemNoteService
create_note(noteable: noteable, project: project, author: author, note: body) create_note(noteable: noteable, project: project, author: author, note: body)
end end
def self.remove_merge_request_wip(noteable, project, author)
body = 'Unmarked this merge request as a Work In Progress'
create_note(noteable: noteable, project: project, author: author, note: body)
end
def self.add_merge_request_wip(noteable, project, author)
body = 'Marked this merge request as a **Work In Progress**'
create_note(noteable: noteable, project: project, author: author, note: body)
end
# Called when the title of a Noteable is changed # Called when the title of a Noteable is changed
# #
# noteable - Noteable object that responds to `title` # noteable - Noteable object that responds to `title`
......
%h4 %h4
This merge request is currently a Work In Progress This merge request is currently a Work In Progress
%p - if can?(current_user, :update_merge_request, @merge_request)
When this merge request is ready, remove the "WIP" prefix from the title to allow it to be merged. %p
When this merge request is ready,
= link_to remove_wip_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), method: :post do
remove the
%code WIP:
prefix from the title
to allow it to be merged.
...@@ -13,12 +13,21 @@ ...@@ -13,12 +13,21 @@
- if issuable.is_a?(MergeRequest) - if issuable.is_a?(MergeRequest)
%p.help-block %p.help-block
- if issuable.work_in_progress? .js-wip-explanation
Remove the <code>WIP</code> prefix from the title to allow this %a.js-toggle-wip{href: ""}
<strong>Work In Progress</strong> merge request to be merged when it's ready. Remove the
- else %code WIP:
Start the title with <code>[WIP]</code> or <code>WIP:</code> to prevent a prefix from the title
<strong>Work In Progress</strong> merge request from being merged before it's ready. to allow this
%strong Work In Progress
merge request to be merged when it's ready.
.js-no-wip-explanation
%a.js-toggle-wip{href: ""}
Start the title with
%code WIP:
to prevent a
%strong Work In Progress
merge request from being merged before it's ready.
.form-group.detail-page-description .form-group.detail-page-description
= f.label :description, 'Description', class: 'control-label' = f.label :description, 'Description', class: 'control-label'
.col-sm-10 .col-sm-10
......
...@@ -623,6 +623,7 @@ Rails.application.routes.draw do ...@@ -623,6 +623,7 @@ Rails.application.routes.draw do
post :cancel_merge_when_build_succeeds post :cancel_merge_when_build_succeeds
get :ci_status get :ci_status
post :toggle_subscription post :toggle_subscription
post :remove_wip
end end
collection do collection do
......
...@@ -216,24 +216,11 @@ describe MergeRequest, models: true do ...@@ -216,24 +216,11 @@ describe MergeRequest, models: true do
end end
describe "#work_in_progress?" do describe "#work_in_progress?" do
it "detects the 'WIP ' prefix" do ['WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP [WIP] WIP: WIP '].each do |wip_prefix|
subject.title = "WIP #{subject.title}" it "detects the '#{wip_prefix}' prefix" do
subject.title = "#{wip_prefix}#{subject.title}"
expect(subject).to be_work_in_progress expect(subject).to be_work_in_progress
end end
it "detects the 'WIP: ' prefix" do
subject.title = "WIP: #{subject.title}"
expect(subject).to be_work_in_progress
end
it "detects the '[WIP] ' prefix" do
subject.title = "[WIP] #{subject.title}"
expect(subject).to be_work_in_progress
end
it "detects the '[WIP]' prefix" do
subject.title = "[WIP]#{subject.title}"
expect(subject).to be_work_in_progress
end end
it "doesn't detect WIP for words starting with WIP" do it "doesn't detect WIP for words starting with WIP" do
...@@ -241,6 +228,11 @@ describe MergeRequest, models: true do ...@@ -241,6 +228,11 @@ describe MergeRequest, models: true do
expect(subject).not_to be_work_in_progress expect(subject).not_to be_work_in_progress
end end
it "doesn't detect WIP for words containing with WIP" do
subject.title = "WupWipwap #{subject.title}"
expect(subject).not_to be_work_in_progress
end
it "doesn't detect WIP by default" do it "doesn't detect WIP by default" do
expect(subject).not_to be_work_in_progress expect(subject).not_to be_work_in_progress
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