From 9aa3edc61586fd79ce0760b7af0946ddfadaa65a Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 17 Aug 2016 18:58:52 -0500 Subject: Remove unneeded aliases --- app/services/slash_commands/interpret_service.rb | 35 ++++--- doc/workflow/slash_commands.md | 40 ++++---- .../issues/user_uses_slash_commands_spec.rb | 8 +- .../user_uses_slash_commands_spec.rb | 4 +- spec/lib/gitlab/slash_commands/extractor_spec.rb | 16 +-- spec/services/notes/slash_commands_service_spec.rb | 8 +- .../slash_commands/interpret_service_spec.rb | 111 +++++++++------------ ...reate_service_slash_commands_shared_examples.rb | 4 +- 8 files changed, 109 insertions(+), 117 deletions(-) diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb index dc4a892b8b1..0d7838055fe 100644 --- a/app/services/slash_commands/interpret_service.rb +++ b/app/services/slash_commands/interpret_service.rb @@ -54,7 +54,7 @@ module SlashCommands issuable.closed? && current_user.can?(:"update_#{issuable.to_ability_name}", issuable) end - command :reopen, :open do + command :reopen do @updates[:state_event] = 'reopen' end @@ -86,7 +86,7 @@ module SlashCommands issuable.assignee_id? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) end - command :unassign, :remove_assignee do + command :unassign do @updates[:assignee_id] = nil end @@ -109,7 +109,7 @@ module SlashCommands issuable.milestone_id? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) end - command :clear_milestone, :remove_milestone do + command :remove_milestone do @updates[:milestone_id] = nil end @@ -119,33 +119,40 @@ module SlashCommands current_user.can?(:"admin_#{issuable.to_ability_name}", project) && project.labels.any? end - command :label, :labels do |labels_param| + command :label do |labels_param| label_ids = find_label_ids(labels_param) @updates[:add_label_ids] = label_ids unless label_ids.empty? end - desc 'Remove label(s)' + desc 'Remove all or specific label(s)' params '~label1 ~"label 2"' condition do issuable.persisted? && issuable.labels.any? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) end - command :unlabel, :remove_label, :remove_labels do |labels_param| - label_ids = find_label_ids(labels_param) + command :unlabel do |labels_param = nil| + if labels_param.present? + label_ids = find_label_ids(labels_param) - @updates[:remove_label_ids] = label_ids unless label_ids.empty? + @updates[:remove_label_ids] = label_ids unless label_ids.empty? + else + @updates[:label_ids] = [] + end end - desc 'Remove all labels' + desc 'Replace all label(s)' + params '~label1 ~"label 2"' condition do issuable.persisted? && issuable.labels.any? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) end - command :clear_labels, :clear_label do - @updates[:label_ids] = [] + command :relabel do |labels_param| + label_ids = find_label_ids(labels_param) + + @updates[:label_ids] = label_ids unless label_ids.empty? end desc 'Add a todo' @@ -185,12 +192,12 @@ module SlashCommands end desc 'Set due date' - params '' + params '' condition do issuable.respond_to?(:due_date) && current_user.can?(:"update_#{issuable.to_ability_name}", issuable) end - command :due, :due_date do |due_date_param| + command :due do |due_date_param| due_date = Chronic.parse(due_date_param).try(:to_date) @updates[:due_date] = due_date if due_date @@ -203,7 +210,7 @@ module SlashCommands issuable.due_date? && current_user.can?(:"update_#{issuable.to_ability_name}", issuable) end - command :clear_due_date do + command :remove_due_date do @updates[:due_date] = nil end diff --git a/doc/workflow/slash_commands.md b/doc/workflow/slash_commands.md index 75fec7bcdb9..91d69d4e77e 100644 --- a/doc/workflow/slash_commands.md +++ b/doc/workflow/slash_commands.md @@ -5,26 +5,26 @@ requests that are usually done by clicking buttons or dropdowns in GitLab's UI. You can enter these commands while creating a new issue or merge request, and in comments. Each command should be on a separate line in order to be properly detected and executed. The commands are removed from the issue, merge request or -comment body before it is saved and will not be visible as such to anyone else. +comment body before it is saved and will not be visible to anyone else. -Here is a list of all of the available commands and descriptions about what they +Below is a list of all of the available commands and descriptions about what they do. -| Command | Aliases | Action | -|:---------------------------|:--------------------|:-------------| -| `/close` | None | Close the issue or merge request | -| `/reopen` | `/open` | Reopen the issue or merge request | -| `/title ` | None | Change title | -| `/assign @username` | None | Assign | -| `/unassign` | `/remove_assignee` | Remove assignee | -| `/milestone %milestone` | None | Set milestone | -| `/clear_milestone` | `/remove_milestone` | Remove milestone | -| `/label ~foo ~"bar baz"` | `/labels` | Add label(s) | -| `/unlabel ~foo ~"bar baz"` | `/remove_label`, `remove_labels` | Remove label(s) | -| `/clear_labels` | `/clear_label` | Clear all labels | -| `/todo` | None | Add a todo | -| `/done` | None | Mark todo as done | -| `/subscribe` | None | Subscribe | -| `/unsubscribe` | None | Unsubscribe | -| `/due ` | `/due_date` | Set due date | -| `/clear_due_date` | None | Remove due date | +| Command | Action | +|:---------------------------|:-------------| +| `/close` | Close the issue or merge request | +| `/reopen` | Reopen the issue or merge request | +| `/title ` | Change title | +| `/assign @username` | Assign | +| `/unassign` | Remove assignee | +| `/milestone %milestone` | Set milestone | +| `/remove_milestone` | Remove milestone | +| `/label ~foo ~"bar baz"` | Add label(s) | +| `/unlabel ~foo ~"bar baz"` | Remove all or specific label(s) | +| `/relabel ~foo ~"bar baz"` | Replace all label(s) | +| `/todo` | Add a todo | +| `/done` | Mark todo as done | +| `/subscribe` | Subscribe | +| `/unsubscribe` | Unsubscribe | +| `/due ` | Set due date | +| `/remove_due_date` | Remove due date | diff --git a/spec/features/issues/user_uses_slash_commands_spec.rb b/spec/features/issues/user_uses_slash_commands_spec.rb index 510f4254b54..2883e392694 100644 --- a/spec/features/issues/user_uses_slash_commands_spec.rb +++ b/spec/features/issues/user_uses_slash_commands_spec.rb @@ -22,11 +22,11 @@ feature 'Issues > User uses slash commands', feature: true, js: true do it 'does not create a note, and sets the due date accordingly' do page.within('.js-main-target-form') do - fill_in 'note[note]', with: "/due_date 2016-08-28" + fill_in 'note[note]', with: "/due 2016-08-28" click_button 'Comment' end - expect(page).not_to have_content '/due_date 2016-08-28' + expect(page).not_to have_content '/due 2016-08-28' expect(page).to have_content 'Your commands have been executed!' issue.reload @@ -42,11 +42,11 @@ feature 'Issues > User uses slash commands', feature: true, js: true do expect(issue.due_date).to eq Date.new(2016, 8, 28) page.within('.js-main-target-form') do - fill_in 'note[note]', with: "/clear_due_date" + fill_in 'note[note]', with: "/remove_due_date" click_button 'Comment' end - expect(page).not_to have_content '/clear_due_date' + expect(page).not_to have_content '/remove_due_date' expect(page).to have_content 'Your commands have been executed!' issue.reload 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 08c452c6e59..d9ef0d18074 100644 --- a/spec/features/merge_requests/user_uses_slash_commands_spec.rb +++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb @@ -22,11 +22,11 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do it 'does not recognize the command nor create a note' do page.within('.js-main-target-form') do - fill_in 'note[note]', with: "/due_date 2016-08-28" + fill_in 'note[note]', with: "/due 2016-08-28" click_button 'Comment' end - expect(page).not_to have_content '/due_date 2016-08-28' + expect(page).not_to have_content '/due 2016-08-28' end end end diff --git a/spec/lib/gitlab/slash_commands/extractor_spec.rb b/spec/lib/gitlab/slash_commands/extractor_spec.rb index 09f909dcdd2..1e4954c4af8 100644 --- a/spec/lib/gitlab/slash_commands/extractor_spec.rb +++ b/spec/lib/gitlab/slash_commands/extractor_spec.rb @@ -18,7 +18,7 @@ describe Gitlab::SlashCommands::Extractor do it 'extracts command' do msg, commands = extractor.extract_commands(original_msg) - expect(commands).to eq [['open']] + expect(commands).to eq [['reopen']] expect(msg).to eq final_msg end end @@ -45,31 +45,31 @@ describe Gitlab::SlashCommands::Extractor do describe 'command with no argument' do context 'at the start of content' do it_behaves_like 'command with no argument' do - let(:original_msg) { "/open\nworld" } + let(:original_msg) { "/reopen\nworld" } let(:final_msg) { "world" } end end context 'in the middle of content' do it_behaves_like 'command with no argument' do - let(:original_msg) { "hello\n/open\nworld" } + let(:original_msg) { "hello\n/reopen\nworld" } let(:final_msg) { "hello\nworld" } end end context 'in the middle of a line' do it 'does not extract command' do - msg = "hello\nworld /open" + msg = "hello\nworld /reopen" msg, commands = extractor.extract_commands(msg) expect(commands).to be_empty - expect(msg).to eq "hello\nworld /open" + expect(msg).to eq "hello\nworld /reopen" end end context 'at the end of content' do it_behaves_like 'command with no argument' do - let(:original_msg) { "hello\n/open" } + let(:original_msg) { "hello\n/reopen" } let(:final_msg) { "hello" } end end @@ -170,10 +170,10 @@ describe Gitlab::SlashCommands::Extractor do end it 'extracts multiple commands' do - msg = %(hello\n/power @user.name %9.10 ~"bar baz.2" label\nworld\n/open) + msg = %(hello\n/power @user.name %9.10 ~"bar baz.2" label\nworld\n/reopen) msg, commands = extractor.extract_commands(msg) - expect(commands).to eq [['power', '@user.name %9.10 ~"bar baz.2" label'], ['open']] + expect(commands).to eq [['power', '@user.name %9.10 ~"bar baz.2" label'], ['reopen']] expect(msg).to eq "hello\nworld" end diff --git a/spec/services/notes/slash_commands_service_spec.rb b/spec/services/notes/slash_commands_service_spec.rb index 9a262fcf32f..4f231aab161 100644 --- a/spec/services/notes/slash_commands_service_spec.rb +++ b/spec/services/notes/slash_commands_service_spec.rb @@ -69,12 +69,12 @@ describe Notes::SlashCommandsService, services: true do end end - describe '/open' do + describe '/reopen' do before do note.noteable.close! expect(note.noteable).to be_closed end - let(:note_text) { '/open' } + let(:note_text) { '/reopen' } it 'opens the noteable, and leave no note' do content, command_params = service.extract_commands(note) @@ -104,12 +104,12 @@ describe Notes::SlashCommandsService, services: true do end end - describe '/open' do + describe '/reopen' do before do note.noteable.close expect(note.noteable).to be_closed end - let(:note_text) { "HELLO\n/open\nWORLD" } + let(:note_text) { "HELLO\n/reopen\nWORLD" } it 'opens the noteable' do content, command_params = service.extract_commands(note) diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb index c20aa90ddde..a616275e883 100644 --- a/spec/services/slash_commands/interpret_service_spec.rb +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -16,8 +16,8 @@ describe SlashCommands::InterpretService, services: true do let(:service) { described_class.new(project, user) } let(:merge_request) { create(:merge_request, source_project: project) } - shared_examples 'open command' do - it 'returns state_event: "open" if content contains /open' do + shared_examples 'reopen command' do + it 'returns state_event: "reopen" if content contains /reopen' do issuable.close! _, updates = service.execute(content, issuable) @@ -26,7 +26,7 @@ describe SlashCommands::InterpretService, services: true do end shared_examples 'close command' do - it 'returns state_event: "close" if content contains /open' do + it 'returns state_event: "close" if content contains /close' do _, updates = service.execute(content, issuable) expect(updates).to eq(state_event: 'close') @@ -67,8 +67,8 @@ describe SlashCommands::InterpretService, services: true do end end - shared_examples 'clear_milestone command' do - it 'populates milestone_id: nil if content contains /clear_milestone' do + shared_examples 'remove_milestone command' do + it 'populates milestone_id: nil if content contains /remove_milestone' do issuable.update(milestone_id: milestone.id) _, updates = service.execute(content, issuable) @@ -95,8 +95,8 @@ describe SlashCommands::InterpretService, services: true do end end - shared_examples 'clear_labels command' do - it 'populates label_ids: [] if content contains /clear_labels' do + shared_examples 'unlabel command with no argument' do + it 'populates label_ids: [] if content contains /unlabel with no arguments' do issuable.update(label_ids: [inprogress.id]) # populate the label _, updates = service.execute(content, issuable) @@ -104,6 +104,16 @@ describe SlashCommands::InterpretService, services: true do end end + shared_examples 'relabel command' do + it 'populates label_ids: [] if content contains /relabel' do + issuable.update(label_ids: [bug.id]) # populate the label + inprogress # populate the label + _, updates = service.execute(content, issuable) + + expect(updates).to eq(label_ids: [inprogress.id]) + end + end + shared_examples 'todo command' do it 'populates todo_event: "add" if content contains /todo' do _, updates = service.execute(content, issuable) @@ -138,16 +148,16 @@ describe SlashCommands::InterpretService, services: true do end end - shared_examples 'due_date command' do - it 'populates due_date: Date.new(2016, 8, 28) if content contains /due_date 2016-08-28' do + shared_examples 'due command' do + it 'populates due_date: Date.new(2016, 8, 28) if content contains /due 2016-08-28' do _, updates = service.execute(content, issuable) expect(updates).to eq(due_date: defined?(expected_date) ? expected_date : Date.new(2016, 8, 28)) end end - shared_examples 'clear_due_date command' do - it 'populates due_date: nil if content contains /clear_due_date' do + shared_examples 'remove_due_date command' do + it 'populates due_date: nil if content contains /remove_due_date' do issuable.update(due_date: Date.today) _, updates = service.execute(content, issuable) @@ -163,19 +173,14 @@ describe SlashCommands::InterpretService, services: true do end end - it_behaves_like 'open command' do - let(:content) { '/open' } + it_behaves_like 'reopen command' do + let(:content) { '/reopen' } let(:issuable) { issue } end - it_behaves_like 'open command' do - let(:content) { '/open' } - let(:issuable) { merge_request } - end - - it_behaves_like 'open command' do + it_behaves_like 'reopen command' do let(:content) { '/reopen' } - let(:issuable) { issue } + let(:issuable) { merge_request } end it_behaves_like 'close command' do @@ -233,11 +238,6 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { merge_request } end - it_behaves_like 'unassign command' do - let(:content) { '/remove_assignee' } - let(:issuable) { issue } - end - it_behaves_like 'milestone command' do let(:content) { "/milestone %#{milestone.title}" } let(:issuable) { issue } @@ -248,19 +248,14 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { merge_request } end - it_behaves_like 'clear_milestone command' do - let(:content) { '/clear_milestone' } + it_behaves_like 'remove_milestone command' do + let(:content) { '/remove_milestone' } let(:issuable) { issue } end - it_behaves_like 'clear_milestone command' do - let(:content) { '/clear_milestone' } - let(:issuable) { merge_request } - end - - it_behaves_like 'clear_milestone command' do + it_behaves_like 'remove_milestone command' do let(:content) { '/remove_milestone' } - let(:issuable) { issue } + let(:issuable) { merge_request } end it_behaves_like 'label command' do @@ -273,11 +268,6 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { merge_request } end - it_behaves_like 'label command' do - let(:content) { %(/labels ~"#{inprogress.title}" ~#{bug.title} ~unknown) } - let(:issuable) { issue } - end - it_behaves_like 'unlabel command' do let(:content) { %(/unlabel ~"#{inprogress.title}") } let(:issuable) { issue } @@ -288,31 +278,26 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { merge_request } end - it_behaves_like 'unlabel command' do - let(:content) { %(/remove_labels ~"#{inprogress.title}") } + it_behaves_like 'unlabel command with no argument' do + let(:content) { %(/unlabel) } let(:issuable) { issue } end - it_behaves_like 'unlabel command' do - let(:content) { %(/remove_label ~"#{inprogress.title}") } - let(:issuable) { issue } + it_behaves_like 'unlabel command with no argument' do + let(:content) { %(/unlabel) } + let(:issuable) { merge_request } end - it_behaves_like 'clear_labels command' do - let(:content) { '/clear_labels' } + it_behaves_like 'relabel command' do + let(:content) { %(/relabel ~"#{inprogress.title}") } let(:issuable) { issue } end - it_behaves_like 'clear_labels command' do - let(:content) { '/clear_labels' } + it_behaves_like 'relabel command' do + let(:content) { %(/relabel ~"#{inprogress.title}") } let(:issuable) { merge_request } end - it_behaves_like 'clear_labels command' do - let(:content) { '/clear_label' } - let(:issuable) { issue } - end - it_behaves_like 'todo command' do let(:content) { '/todo' } let(:issuable) { issue } @@ -353,46 +338,46 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { merge_request } end - it_behaves_like 'due_date command' do - let(:content) { '/due_date 2016-08-28' } + it_behaves_like 'due command' do + let(:content) { '/due 2016-08-28' } let(:issuable) { issue } end - it_behaves_like 'due_date command' do + it_behaves_like 'due command' do let(:content) { '/due tomorrow' } let(:issuable) { issue } let(:expected_date) { Date.tomorrow } end - it_behaves_like 'due_date command' do + it_behaves_like 'due command' do let(:content) { '/due 5 days from now' } let(:issuable) { issue } let(:expected_date) { 5.days.from_now.to_date } end - it_behaves_like 'due_date command' do + it_behaves_like 'due command' do let(:content) { '/due in 2 days' } let(:issuable) { issue } let(:expected_date) { 2.days.from_now.to_date } end it_behaves_like 'empty command' do - let(:content) { '/due_date foo bar' } + let(:content) { '/due foo bar' } let(:issuable) { issue } end it_behaves_like 'empty command' do - let(:content) { '/due_date 2016-08-28' } + let(:content) { '/due 2016-08-28' } let(:issuable) { merge_request } end - it_behaves_like 'clear_due_date command' do - let(:content) { '/clear_due_date' } + it_behaves_like 'remove_due_date command' do + let(:content) { '/remove_due_date' } let(:issuable) { issue } end it_behaves_like 'empty command' do - let(:content) { '/clear_due_date' } + let(:content) { '/remove_due_date' } let(:issuable) { merge_request } end end diff --git a/spec/support/issuable_create_service_slash_commands_shared_examples.rb b/spec/support/issuable_create_service_slash_commands_shared_examples.rb index bd0201c866f..5f9645ed44f 100644 --- a/spec/support/issuable_create_service_slash_commands_shared_examples.rb +++ b/spec/support/issuable_create_service_slash_commands_shared_examples.rb @@ -14,7 +14,7 @@ shared_examples 'new issuable record that supports slash commands' do context 'with labels in command only' do let(:example_params) do { - description: "/label ~#{labels.first.name} ~#{labels.second.name}\n/remove_label ~#{labels.third.name}" + description: "/label ~#{labels.first.name} ~#{labels.second.name}\n/unlabel ~#{labels.third.name}" } end @@ -28,7 +28,7 @@ shared_examples 'new issuable record that supports slash commands' do let(:example_params) do { label_ids: [labels.second.id], - description: "/label ~#{labels.first.name}\n/remove_label ~#{labels.third.name}" + description: "/label ~#{labels.first.name}\n/unlabel ~#{labels.third.name}" } end -- cgit v1.2.3