diff options
author | Jacob Schatz <jschatz1@gmail.com> | 2016-03-19 20:06:59 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-03-21 12:31:20 +0100 |
commit | 42c4304133299888b74800d8bb810a79a6ddb0eb (patch) | |
tree | 055c0707fbb80d79c083e5f363fa0ee29df3b4cb | |
parent | e6961c4681cf561942cb459f28c1fad8854f61c9 (diff) | |
download | gitlab-ce-42c4304133299888b74800d8bb810a79a6ddb0eb.tar.gz |
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
-rw-r--r-- | CHANGELOG | 2 | ||||
-rw-r--r-- | app/assets/javascripts/issuable_form.js.coffee | 41 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 11 | ||||
-rw-r--r-- | app/models/merge_request.rb | 8 | ||||
-rw-r--r-- | app/services/merge_requests/base_service.rb | 13 | ||||
-rw-r--r-- | app/services/system_note_service.rb | 12 | ||||
-rw-r--r-- | app/views/projects/merge_requests/widget/open/_wip.html.haml | 10 | ||||
-rw-r--r-- | app/views/shared/issuable/_form.html.haml | 21 | ||||
-rw-r--r-- | config/routes.rb | 1 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 28 |
10 files changed, 118 insertions, 29 deletions
diff --git a/CHANGELOG b/CHANGELOG index e6dec3b73b9..6df0ed400be 100644 --- a/CHANGELOG +++ b/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. diff --git a/app/assets/javascripts/issuable_form.js.coffee b/app/assets/javascripts/issuable_form.js.coffee index 48c249943f2..6c1699c178c 100644 --- a/app/assets/javascripts/issuable_form.js.coffee +++ b/app/assets/javascripts/issuable_form.js.coffee @@ -1,4 +1,5 @@ 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()}" diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 61b82c9db46..7248ede1699 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/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 diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a6140b5b04c..a015a9ef394 100644 --- a/app/models/merge_request.rb +++ b/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? diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 7b306a8a531..ac5b58db862 100644 --- a/app/services/merge_requests/base_service.rb +++ b/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) diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 2afcaad4646..c644cd0b951 100644 --- a/app/services/system_note_service.rb +++ b/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` diff --git a/app/views/projects/merge_requests/widget/open/_wip.html.haml b/app/views/projects/merge_requests/widget/open/_wip.html.haml index 0cf16542cc1..c296422a9cf 100644 --- a/app/views/projects/merge_requests/widget/open/_wip.html.haml +++ b/app/views/projects/merge_requests/widget/open/_wip.html.haml @@ -1,5 +1,11 @@ %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. diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 9ef729e960c..80418e69d9c 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/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 diff --git a/config/routes.rb b/config/routes.rb index ec79522002e..561987322b2 100644 --- a/config/routes.rb +++ b/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 diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 2165cfb7a32..f2f07e4ee17 100644 --- a/spec/models/merge_request_spec.rb +++ b/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 |