Merge branch 'remove-wip' into 'master'

CHANGELOG

@@ -8,6 +8,8 @@ v 8.6.0 (unreleased)
   - New branch button appears on issues where applicable
   - Contributions to forked projects are included in calendar
   - 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
     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.

app/assets/javascripts/issuable_form.js.coffee

 class @IssuableForm
+  wipRegex: /^\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i
   constructor: (@form) ->
     GitLab.GfmAutoComplete.setup()
     new UsersSelect()
@@ -14,6 +15,8 @@ class @IssuableForm
     @form.on "submit", @resetAutosave
     @form.on "click", ".btn-cancel", @resetAutosave
 
+    @initWip()
+
   initAutosave: ->
     new Autosave @titleField, [
       document.location.pathname,
@@ -30,3 +33,41 @@ class @IssuableForm
   resetAutosave: =>
     @titleField.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()}"

app/controllers/projects/merge_requests_controller.rb

@@ -5,7 +5,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   before_action :module_enabled
   before_action :merge_request, only: [
     :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 :validates_merge_request, only: [:show, :diffs, :commits, :builds]
@@ -20,7 +20,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   before_action :authorize_create_merge_request!, only: [:new, :create]
 
   # 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
     terms = params['issue_search']
@@ -164,6 +164,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     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
     @merge_request.check_if_can_be_merged
 

app/models/merge_request.rb

@@ -277,8 +277,14 @@ class MergeRequest < ActiveRecord::Base
     self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
   end
 
+  WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze
+
   def work_in_progress?
-    !!(title =~ /\A\[?WIP(\]|:| )/i)
+    title =~ WIP_REGEX
+  end
+
+  def wipless_title
+    self.title.sub(WIP_REGEX, "")
   end
 
   def mergeable?

app/services/merge_requests/base_service.rb

@@ -5,6 +5,19 @@ module MergeRequests
       SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
     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)
       hook_data = merge_request.to_hook_data(current_user)
       merge_request_url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id)

app/services/system_note_service.rb

@@ -144,6 +144,18 @@ class SystemNoteService
     create_note(noteable: noteable, project: project, author: author, note: body)
   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
   #
   # noteable  - Noteable object that responds to `title`

app/views/projects/merge_requests/widget/open/_wip.html.haml

 %h4
   This merge request is currently a Work In Progress
 
-%p
-  When this merge request is ready, remove the "WIP" prefix from the title to allow it to be merged.
+- if can?(current_user, :update_merge_request, @merge_request)
+  %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.

app/views/shared/issuable/_form.html.haml

@@ -13,12 +13,21 @@
 
     - if issuable.is_a?(MergeRequest)
       %p.help-block
-        - if issuable.work_in_progress?
-          Remove the <code>WIP</code> prefix from the title to allow this
-          <strong>Work In Progress</strong> merge request to be merged when it's ready.
-        - else
-          Start the title with <code>[WIP]</code> or <code>WIP:</code> to prevent a
-          <strong>Work In Progress</strong> merge request from being merged before it's ready.
+        .js-wip-explanation
+          %a.js-toggle-wip{href: ""}
+            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
+          %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
   = f.label :description, 'Description', class: 'control-label'
   .col-sm-10

config/routes.rb

@@ -623,6 +623,7 @@ Rails.application.routes.draw do
             post :cancel_merge_when_build_succeeds
             get :ci_status
             post :toggle_subscription
+            post :remove_wip
           end
 
           collection do

spec/models/merge_request_spec.rb

@@ -216,24 +216,11 @@ describe MergeRequest, models: true do
   end
 
   describe "#work_in_progress?" do
-    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
-
-    it "detects the '[WIP]' prefix" do
-      subject.title = "[WIP]#{subject.title}"
-      expect(subject).to be_work_in_progress
+    ['WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP [WIP] WIP: WIP '].each do |wip_prefix|
+      it "detects the '#{wip_prefix}' prefix" do
+        subject.title = "#{wip_prefix}#{subject.title}"
+        expect(subject).to be_work_in_progress
+      end
     end
 
     it "doesn't detect WIP for words starting with WIP" do
@@ -241,6 +228,11 @@ describe MergeRequest, models: true do
       expect(subject).not_to be_work_in_progress
     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
       expect(subject).not_to be_work_in_progress
     end