diff --git a/CHANGELOG b/CHANGELOG
index a52ac53bae772d6c642c4ff71e04f2953cd14da3..6ce15ef28bfe965c9ca2b794657ea734577ff4e3 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -6,6 +6,7 @@ v 8.13.0 (unreleased)
- Fix centering of custom header logos (Ashley Dumaine)
- AbstractReferenceFilter caches project_refs on RequestStore when active
- Replaced the check sign to arrow in the show build view. !6501
+ - Add a /wip slash command to toggle the Work In Progress status of a merge request. !6259 (tbalthazar)
- Speed-up group milestones show page
- Log LDAP lookup errors and don't swallow unrelated exceptions. !6103 (Markus Koller)
- Add more tests for calendar contribution (ClemMakesApps)
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 68bb4232f5b7f022bafdeda48de975f423ff8e7b..8c8c56228adf1daa5bea2549289ef363565ff685 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -276,7 +276,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end
def remove_wip
- MergeRequests::UpdateService.new(project, current_user, title: @merge_request.wipless_title).execute(@merge_request)
+ MergeRequests::UpdateService.new(project, current_user, wip_event: 'unwip').execute(@merge_request)
redirect_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request),
notice: "The merge request can now be merged."
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index aec555dcec0626530cd3aea4efce71f3e35f9b7a..a431d46cc9efd8f2ea4739bd631bdddd9441f447 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -155,6 +155,20 @@ class MergeRequest < ActiveRecord::Base
where("merge_requests.id IN (#{union.to_sql})")
end
+ WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze
+
+ def self.work_in_progress?(title)
+ !!(title =~ WIP_REGEX)
+ end
+
+ def self.wipless_title(title)
+ title.sub(WIP_REGEX, "")
+ end
+
+ def self.wip_title(title)
+ work_in_progress?(title) ? title : "WIP: #{title}"
+ end
+
def to_reference(from_project = nil)
reference = "#{self.class.reference_prefix}#{iid}"
@@ -389,14 +403,16 @@ class MergeRequest < ActiveRecord::Base
@closed_event ||= 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 =~ WIP_REGEX)
+ self.class.work_in_progress?(title)
end
def wipless_title
- self.title.sub(WIP_REGEX, "")
+ self.class.wipless_title(self.title)
+ end
+
+ def wip_title
+ self.class.wip_title(self.title)
end
def mergeable?(skip_ci_check: false)
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index ba424b09463a83f7116ce8a6c80d4db44e9a8c34..d0d155b7ee1e8dd4b0752c8c5a4fe8099f9d3fcd 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -5,16 +5,17 @@ module MergeRequests
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?
+ removed_wip = MergeRequest.work_in_progress?(old_title) && !issuable.work_in_progress?
+ added_wip = !MergeRequest.work_in_progress?(old_title) && issuable.work_in_progress?
+ changed_title = MergeRequest.wipless_title(old_title) != issuable.wipless_title
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
+
+ super if changed_title
end
def hook_data(merge_request, action, oldrev = nil)
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index f14f9e4b32796081b915eedd10b0349182d7988a..9dbec49d163a508ab08932577e821e8f2841365e 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -16,7 +16,7 @@ module MergeRequests
end
merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
-
+ handle_wip_event(merge_request)
update(merge_request)
end
@@ -81,5 +81,18 @@ module MergeRequests
def after_update(issuable)
issuable.cache_merge_request_closes_issues!(current_user)
end
+
+ private
+
+ def handle_wip_event(merge_request)
+ if wip_event = params.delete(:wip_event)
+ # We update the title that is provided in the params or we use the mr title
+ title = params[:title] || merge_request.title
+ params[:title] = case wip_event
+ when 'wip' then MergeRequest.wip_title(title)
+ when 'unwip' then MergeRequest.wipless_title(title)
+ end
+ end
+ end
end
end
diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb
index ffcad5b3a8782c1df1da70819c2e4261544c84aa..1725a30fae57382600df0e72a0ca0edc53e6521a 100644
--- a/app/services/slash_commands/interpret_service.rb
+++ b/app/services/slash_commands/interpret_service.rb
@@ -214,6 +214,18 @@ module SlashCommands
@updates[:due_date] = nil
end
+ desc do
+ "Toggle the Work In Progress status"
+ end
+ condition do
+ issuable.persisted? &&
+ issuable.respond_to?(:work_in_progress?) &&
+ current_user.can?(:"update_#{issuable.to_ability_name}", issuable)
+ end
+ command :wip do
+ @updates[:wip_event] = issuable.work_in_progress? ? 'unwip' : 'wip'
+ end
+
# This is a dummy command, so that it appears in the autocomplete commands
desc 'CC'
params '@user'
diff --git a/doc/user/project/slash_commands.md b/doc/user/project/slash_commands.md
index 1792a0c501d23ee4cce4423f0a88fdfe6270e04c..5f6a6c6503e4e72adab1bcf163f987892476db32 100644
--- a/doc/user/project/slash_commands.md
+++ b/doc/user/project/slash_commands.md
@@ -27,4 +27,5 @@ do.
| `/subscribe` | Subscribe |
| `/unsubscribe` | Unsubscribe |
| <code>/due <in 2 days | this Friday | December 31st></code> | Set due date |
-| `/remove_due_date` | Remove due date |
+| `/remove_due_date` | Remove due date |
+| `/wip` | Toggle the Work In Progress status |
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 94c9edc91fe2b455229c132ae9d1322ea8b493fd..742edd8ba3d47a8c1b113f50d03ea13ab83728ee 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -644,6 +644,20 @@ describe Projects::MergeRequestsController do
end
end
+ context 'POST remove_wip' do
+ it 'removes the wip status' do
+ merge_request.title = merge_request.wip_title
+ merge_request.save
+
+ post :remove_wip,
+ namespace_id: merge_request.project.namespace.to_param,
+ project_id: merge_request.project.to_param,
+ id: merge_request.iid
+
+ expect(merge_request.reload.title).to eq(merge_request.wipless_title)
+ end
+ end
+
context 'POST resolve_conflicts' do
let(:json_response) { JSON.parse(response.body) }
let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha }
diff --git a/spec/features/issues/user_uses_slash_commands_spec.rb b/spec/features/issues/user_uses_slash_commands_spec.rb
index bf2b93c92fb68578f1af1f5d90b8449846c2c1cb..3f2da1c380c0e0c13d11f03a21c54effe3df58cf 100644
--- a/spec/features/issues/user_uses_slash_commands_spec.rb
+++ b/spec/features/issues/user_uses_slash_commands_spec.rb
@@ -99,5 +99,15 @@ feature 'Issues > User uses slash commands', feature: true, js: true do
end
end
end
+
+ describe 'toggling the WIP prefix from the title from note' do
+ let(:issue) { create(:issue, project: project) }
+
+ it 'does not recognize the command nor create a note' do
+ write_note("/wip")
+
+ expect(page).not_to have_content '/wip'
+ end
+ end
end
end
diff --git a/spec/features/merge_requests/user_uses_slash_commands_spec.rb b/spec/features/merge_requests/user_uses_slash_commands_spec.rb
index 22d9d1b9fd518b8d14d9a520a7fac86c99f42609..cb3cea3fd51f0612456fe5a72770556b7d7daafb 100644
--- a/spec/features/merge_requests/user_uses_slash_commands_spec.rb
+++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb
@@ -14,21 +14,66 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do
let(:new_url_opts) { { merge_request: { source_branch: 'feature' } } }
end
- describe 'adding a due date from note' do
+ describe 'merge-request-only commands' do
before do
project.team << [user, :master]
login_with(user)
visit namespace_project_merge_request_path(project.namespace, project, merge_request)
end
-
+
after do
wait_for_ajax
end
- it 'does not recognize the command nor create a note' do
- write_note("/due 2016-08-28")
+ describe 'toggling the WIP prefix in the title from note' do
+ context 'when the current user can toggle the WIP prefix' do
+ it 'adds the WIP: prefix to the title' do
+ write_note("/wip")
+
+ expect(page).not_to have_content '/wip'
+ expect(page).to have_content 'Your commands have been executed!'
+
+ expect(merge_request.reload.work_in_progress?).to eq true
+ end
+
+ it 'removes the WIP: prefix from the title' do
+ merge_request.title = merge_request.wip_title
+ merge_request.save
+ write_note("/wip")
+
+ expect(page).not_to have_content '/wip'
+ expect(page).to have_content 'Your commands have been executed!'
+
+ expect(merge_request.reload.work_in_progress?).to eq false
+ end
+ end
+
+ context 'when the current user cannot toggle the WIP prefix' do
+ let(:guest) { create(:user) }
+ before do
+ project.team << [guest, :guest]
+ logout
+ login_with(guest)
+ visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+ end
+
+ it 'does not change the WIP prefix' do
+ write_note("/wip")
+
+ expect(page).not_to have_content '/wip'
+ expect(page).not_to have_content 'Your commands have been executed!'
+
+ expect(merge_request.reload.work_in_progress?).to eq false
+ end
+ end
+ end
+
+ describe 'adding a due date from note' do
+ it 'does not recognize the command nor create a note' do
+ write_note('/due 2016-08-28')
- expect(page).not_to have_content '/due 2016-08-28'
+ expect(page).not_to have_content '/due 2016-08-28'
+ end
end
end
end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 580a3235127b842f03592f172c30be61b2f92d4f..9d7be2429edcdd8c064b5b9a3d255caf99da0484 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -287,6 +287,46 @@ describe MergeRequest, models: true do
end
end
+ describe "#wipless_title" do
+ ['WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP [WIP] WIP: WIP '].each do |wip_prefix|
+ it "removes the '#{wip_prefix}' prefix" do
+ wipless_title = subject.title
+ subject.title = "#{wip_prefix}#{subject.title}"
+
+ expect(subject.wipless_title).to eq wipless_title
+ end
+
+ it "is satisfies the #work_in_progress? method" do
+ subject.title = "#{wip_prefix}#{subject.title}"
+ subject.title = subject.wipless_title
+
+ expect(subject.work_in_progress?).to eq false
+ end
+ end
+ end
+
+ describe "#wip_title" do
+ it "adds the WIP: prefix to the title" do
+ wip_title = "WIP: #{subject.title}"
+
+ expect(subject.wip_title).to eq wip_title
+ end
+
+ it "does not add the WIP: prefix multiple times" do
+ wip_title = "WIP: #{subject.title}"
+ subject.title = subject.wip_title
+ subject.title = subject.wip_title
+
+ expect(subject.wip_title).to eq wip_title
+ end
+
+ it "is satisfies the #work_in_progress? method" do
+ subject.title = subject.wip_title
+
+ expect(subject.work_in_progress?).to eq true
+ end
+ end
+
describe '#can_remove_source_branch?' do
let(:user) { create(:user) }
let(:user2) { create(:user) }
diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb
index 5b1edba87a1d3166bb3b24daae02159105b29258..ae4d286d250bebcc3a4b2ef34a85a177edbf952c 100644
--- a/spec/services/slash_commands/interpret_service_spec.rb
+++ b/spec/services/slash_commands/interpret_service_spec.rb
@@ -165,6 +165,23 @@ describe SlashCommands::InterpretService, services: true do
end
end
+ shared_examples 'wip command' do
+ it 'returns wip_event: "wip" if content contains /wip' do
+ _, updates = service.execute(content, issuable)
+
+ expect(updates).to eq(wip_event: 'wip')
+ end
+ end
+
+ shared_examples 'unwip command' do
+ it 'returns wip_event: "unwip" if content contains /wip' do
+ issuable.update(title: issuable.wip_title)
+ _, updates = service.execute(content, issuable)
+
+ expect(updates).to eq(wip_event: 'unwip')
+ end
+ end
+
shared_examples 'empty command' do
it 'populates {} if content contains an unsupported command' do
_, updates = service.execute(content, issuable)
@@ -376,6 +393,16 @@ describe SlashCommands::InterpretService, services: true do
let(:issuable) { issue }
end
+ it_behaves_like 'wip command' do
+ let(:content) { '/wip' }
+ let(:issuable) { merge_request }
+ end
+
+ it_behaves_like 'unwip command' do
+ let(:content) { '/wip' }
+ let(:issuable) { merge_request }
+ end
+
it_behaves_like 'empty command' do
let(:content) { '/remove_due_date' }
let(:issuable) { merge_request }