Skip to content
Snippets Groups Projects
Commit ddbe676d authored by Thomas Balthazar's avatar Thomas Balthazar
Browse files

Add a /wip slash command

It toggles the 'WIP' prefix in the MR title.
parent bcb2699f
No related branches found
No related tags found
No related merge requests found
Showing with 196 additions and 16 deletions
Loading
@@ -6,6 +6,7 @@ v 8.13.0 (unreleased)
Loading
@@ -6,6 +6,7 @@ v 8.13.0 (unreleased)
- Fix centering of custom header logos (Ashley Dumaine) - Fix centering of custom header logos (Ashley Dumaine)
- AbstractReferenceFilter caches project_refs on RequestStore when active - AbstractReferenceFilter caches project_refs on RequestStore when active
- Replaced the check sign to arrow in the show build view. !6501 - 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 - Speed-up group milestones show page
- Log LDAP lookup errors and don't swallow unrelated exceptions. !6103 (Markus Koller) - Log LDAP lookup errors and don't swallow unrelated exceptions. !6103 (Markus Koller)
- Add more tests for calendar contribution (ClemMakesApps) - Add more tests for calendar contribution (ClemMakesApps)
Loading
Loading
Loading
@@ -276,7 +276,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
Loading
@@ -276,7 +276,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end end
   
def remove_wip 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), redirect_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request),
notice: "The merge request can now be merged." notice: "The merge request can now be merged."
Loading
Loading
Loading
@@ -155,6 +155,20 @@ class MergeRequest < ActiveRecord::Base
Loading
@@ -155,6 +155,20 @@ class MergeRequest < ActiveRecord::Base
where("merge_requests.id IN (#{union.to_sql})") where("merge_requests.id IN (#{union.to_sql})")
end 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) def to_reference(from_project = nil)
reference = "#{self.class.reference_prefix}#{iid}" reference = "#{self.class.reference_prefix}#{iid}"
   
Loading
@@ -389,14 +403,16 @@ class MergeRequest < ActiveRecord::Base
Loading
@@ -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 @closed_event ||= 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 =~ WIP_REGEX) self.class.work_in_progress?(title)
end end
   
def wipless_title 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 end
   
def mergeable?(skip_ci_check: false) def mergeable?(skip_ci_check: false)
Loading
Loading
Loading
@@ -5,16 +5,17 @@ module MergeRequests
Loading
@@ -5,16 +5,17 @@ module MergeRequests
end end
   
def create_title_change_note(issuable, old_title) def create_title_change_note(issuable, old_title)
removed_wip = old_title =~ MergeRequest::WIP_REGEX && !issuable.work_in_progress? removed_wip = MergeRequest.work_in_progress?(old_title) && !issuable.work_in_progress?
added_wip = old_title !~ MergeRequest::WIP_REGEX && 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 if removed_wip
SystemNoteService.remove_merge_request_wip(issuable, issuable.project, current_user) SystemNoteService.remove_merge_request_wip(issuable, issuable.project, current_user)
elsif added_wip elsif added_wip
SystemNoteService.add_merge_request_wip(issuable, issuable.project, current_user) SystemNoteService.add_merge_request_wip(issuable, issuable.project, current_user)
else
super
end end
super if changed_title
end end
   
def hook_data(merge_request, action, oldrev = nil) def hook_data(merge_request, action, oldrev = nil)
Loading
Loading
Loading
@@ -16,7 +16,7 @@ module MergeRequests
Loading
@@ -16,7 +16,7 @@ module MergeRequests
end end
   
merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch)
handle_wip_event(merge_request)
update(merge_request) update(merge_request)
end end
   
Loading
@@ -81,5 +81,18 @@ module MergeRequests
Loading
@@ -81,5 +81,18 @@ module MergeRequests
def after_update(issuable) def after_update(issuable)
issuable.cache_merge_request_closes_issues!(current_user) issuable.cache_merge_request_closes_issues!(current_user)
end 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
end end
Loading
@@ -214,6 +214,18 @@ module SlashCommands
Loading
@@ -214,6 +214,18 @@ module SlashCommands
@updates[:due_date] = nil @updates[:due_date] = nil
end 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 # This is a dummy command, so that it appears in the autocomplete commands
desc 'CC' desc 'CC'
params '@user' params '@user'
Loading
Loading
Loading
@@ -27,4 +27,5 @@ do.
Loading
@@ -27,4 +27,5 @@ do.
| `/subscribe` | Subscribe | | `/subscribe` | Subscribe |
| `/unsubscribe` | Unsubscribe | | `/unsubscribe` | Unsubscribe |
| <code>/due &lt;in 2 days &#124; this Friday &#124; December 31st&gt;</code> | Set due date | | <code>/due &lt;in 2 days &#124; this Friday &#124; December 31st&gt;</code> | Set due date |
| `/remove_due_date` | Remove due date | | `/remove_due_date` | Remove due date |
| `/wip` | Toggle the Work In Progress status |
Loading
@@ -644,6 +644,20 @@ describe Projects::MergeRequestsController do
Loading
@@ -644,6 +644,20 @@ describe Projects::MergeRequestsController do
end end
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 context 'POST resolve_conflicts' do
let(:json_response) { JSON.parse(response.body) } let(:json_response) { JSON.parse(response.body) }
let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha } let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha }
Loading
Loading
Loading
@@ -99,5 +99,15 @@ feature 'Issues > User uses slash commands', feature: true, js: true do
Loading
@@ -99,5 +99,15 @@ feature 'Issues > User uses slash commands', feature: true, js: true do
end end
end 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
end end
Loading
@@ -14,21 +14,66 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do
Loading
@@ -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' } } } let(:new_url_opts) { { merge_request: { source_branch: 'feature' } } }
end end
   
describe 'adding a due date from note' do describe 'merge-request-only commands' do
before do before do
project.team << [user, :master] project.team << [user, :master]
login_with(user) login_with(user)
visit namespace_project_merge_request_path(project.namespace, project, merge_request) visit namespace_project_merge_request_path(project.namespace, project, merge_request)
end end
after do after do
wait_for_ajax wait_for_ajax
end end
   
it 'does not recognize the command nor create a note' do describe 'toggling the WIP prefix in the title from note' do
write_note("/due 2016-08-28") 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 end
end end
Loading
@@ -287,6 +287,46 @@ describe MergeRequest, models: true do
Loading
@@ -287,6 +287,46 @@ describe MergeRequest, models: true do
end end
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 describe '#can_remove_source_branch?' do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:user2) { create(:user) } let(:user2) { create(:user) }
Loading
Loading
Loading
@@ -165,6 +165,23 @@ describe SlashCommands::InterpretService, services: true do
Loading
@@ -165,6 +165,23 @@ describe SlashCommands::InterpretService, services: true do
end end
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 shared_examples 'empty command' do
it 'populates {} if content contains an unsupported command' do it 'populates {} if content contains an unsupported command' do
_, updates = service.execute(content, issuable) _, updates = service.execute(content, issuable)
Loading
@@ -376,6 +393,16 @@ describe SlashCommands::InterpretService, services: true do
Loading
@@ -376,6 +393,16 @@ describe SlashCommands::InterpretService, services: true do
let(:issuable) { issue } let(:issuable) { issue }
end 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 it_behaves_like 'empty command' do
let(:content) { '/remove_due_date' } let(:content) { '/remove_due_date' }
let(:issuable) { merge_request } let(:issuable) { merge_request }
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment