diff options
| -rw-r--r-- | app/assets/javascripts/gfm_auto_complete.js.es6 | 4 | ||||
| -rw-r--r-- | app/controllers/projects_controller.rb | 8 | ||||
| -rw-r--r-- | app/services/issuable_base_service.rb | 4 | ||||
| -rw-r--r-- | app/services/notes/create_service.rb | 5 | ||||
| -rw-r--r-- | app/services/notes/slash_commands_service.rb | 15 | ||||
| -rw-r--r-- | app/services/projects/autocomplete_service.rb | 34 | ||||
| -rw-r--r-- | app/services/projects/participants_service.rb | 25 | ||||
| -rw-r--r-- | app/services/slash_commands/interpret_service.rb | 33 | ||||
| -rw-r--r-- | app/views/layouts/_init_auto_complete.html.haml | 2 | ||||
| -rw-r--r-- | doc/workflow/slash_commands.md | 2 | ||||
| -rw-r--r-- | lib/gitlab/slash_commands/dsl.rb | 88 | ||||
| -rw-r--r-- | spec/features/issues/user_uses_slash_commands_spec.rb | 1 | ||||
| -rw-r--r-- | spec/fixtures/emails/commands_in_reply.eml | 43 | ||||
| -rw-r--r-- | spec/fixtures/emails/commands_only_reply.eml | 1 | ||||
| -rw-r--r-- | spec/lib/gitlab/email/handler/create_note_handler_spec.rb | 45 | ||||
| -rw-r--r-- | spec/lib/gitlab/slash_commands/dsl_spec.rb | 145 |
16 files changed, 296 insertions, 159 deletions
diff --git a/app/assets/javascripts/gfm_auto_complete.js.es6 b/app/assets/javascripts/gfm_auto_complete.js.es6 index 21639c7c084..9be32ed5937 100644 --- a/app/assets/javascripts/gfm_auto_complete.js.es6 +++ b/app/assets/javascripts/gfm_auto_complete.js.es6 @@ -249,7 +249,8 @@ } } }); - return this.input.atwho({ + // We don't instantiate the slash commands autocomplete for note edit forms + $("form:not(.edit-note) .js-gfm-input").atwho({ at: '/', alias: 'commands', displayTpl: function(value) { @@ -284,6 +285,7 @@ beforeInsert: this.DefaultOptions.beforeInsert } }); + return; }, destroyAtWho: function() { return this.input.atwho('destroy'); diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index af20984cbe7..9c387fd3daa 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -134,10 +134,8 @@ class ProjectsController < Projects::ApplicationController end def autocomplete_sources - note_type = params['type'] - note_id = params['type_id'] - autocomplete = ::Projects::AutocompleteService.new(@project, current_user) - participants = ::Projects::ParticipantsService.new(@project, current_user).execute(note_type, note_id) + autocomplete = ::Projects::AutocompleteService.new(@project, current_user, params) + participants = ::Projects::ParticipantsService.new(@project, current_user, params).execute @suggestions = { emojis: Gitlab::AwardEmoji.urls, @@ -146,7 +144,7 @@ class ProjectsController < Projects::ApplicationController mergerequests: autocomplete.merge_requests, labels: autocomplete.labels, members: participants, - commands: autocomplete.commands(note_type, note_id) + commands: autocomplete.commands } respond_to do |format| diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index c14bda811c2..1a01b333366 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -94,10 +94,10 @@ class IssuableBaseService < BaseService end def merge_slash_commands_into_params!(issuable) - command_params = SlashCommands::InterpretService.new(project, current_user). + commands = SlashCommands::InterpretService.new(project, current_user). execute(params[:description], issuable) - params.merge!(command_params) + params.merge!(commands) end def create_issuable(issuable, attributes) diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 0c2513409a1..1b2d63034b8 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -14,7 +14,8 @@ module Notes # We execute commands (extracted from `params[:note]`) on the noteable # **before** we save the note because if the note consists of commands # only, there is no need be create a note! - commands_executed = SlashCommandsService.new(project, current_user).execute(note) + slash_commands_service = SlashCommandsService.new(project, current_user) + commands = slash_commands_service.extract_commands(note) if note.save # Finish the harder work in the background @@ -24,7 +25,7 @@ module Notes # We must add the error after we call #save because errors are reset # when #save is called - if commands_executed && note.note.blank? + if slash_commands_service.execute(commands, note) && note.note.blank? note.errors.add(:commands_only, 'Your commands are being executed.') end diff --git a/app/services/notes/slash_commands_service.rb b/app/services/notes/slash_commands_service.rb index 54d43d06466..ebced9577d8 100644 --- a/app/services/notes/slash_commands_service.rb +++ b/app/services/notes/slash_commands_service.rb @@ -6,16 +6,19 @@ module Notes 'MergeRequest' => MergeRequests::UpdateService } - def execute(note) - noteable_update_service = UPDATE_SERVICES[note.noteable_type] - return false unless noteable_update_service - return false unless can?(current_user, :"update_#{note.noteable_type.underscore}", note.noteable) + def extract_commands(note) + @noteable_update_service = UPDATE_SERVICES[note.noteable_type] + return [] unless @noteable_update_service + return [] unless can?(current_user, :"update_#{note.noteable_type.underscore}", note.noteable) - commands = SlashCommands::InterpretService.new(project, current_user). + SlashCommands::InterpretService.new(project, current_user). execute(note.note, note.noteable) + end + def execute(commands, note) if commands.any? - noteable_update_service.new(project, current_user, commands).execute(note.noteable) + @noteable_update_service.new(project, current_user, commands). + execute(note.noteable) end end end diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index 779f64f584e..477c999eff4 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -16,26 +16,34 @@ module Projects @project.labels.select([:title, :color]) end - def commands(noteable_type, noteable_id) + def commands + # We don't return commands when editing an issue or merge request + # This should be improved by not enabling autocomplete at the JS-level + # following this suggestion: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5021#note_13837384 + return [] if !target || %w[edit update].include?(params[:action_name]) + SlashCommands::InterpretService.command_definitions( - project: @project, - noteable: command_target(noteable_type, noteable_id), + project: project, + noteable: target, current_user: current_user ) end private - def command_target(noteable_type, noteable_id) - case noteable_type - when 'Issue' - IssuesFinder.new(current_user, project_id: @project.id, state: 'all'). - execute.find_or_initialize_by(iid: noteable_id) - when 'MergeRequest' - MergeRequestsFinder.new(current_user, project_id: @project.id, state: 'all'). - execute.find_or_initialize_by(iid: noteable_id) - else - nil + def target + @target ||= begin + noteable_id = params[:type_id] + case params[:type] + when 'Issue' + IssuesFinder.new(current_user, project_id: project.id, state: 'all'). + execute.find_or_initialize_by(iid: noteable_id) + when 'MergeRequest' + MergeRequestsFinder.new(current_user, project_id: project.id, state: 'all'). + execute.find_or_initialize_by(iid: noteable_id) + else + nil + end end end end diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index 02c4eee3d02..1c8f2913e8b 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -1,8 +1,11 @@ module Projects class ParticipantsService < BaseService - def execute(noteable_type, noteable_id) - @noteable_type = noteable_type - @noteable_id = noteable_id + attr_reader :noteable_type, :noteable_id + + def execute + @noteable_type = params[:type] + @noteable_id = params[:type_id] + project_members = sorted(project.team.members) participants = target_owner + participants_in_target + all_members + groups + project_members participants.uniq @@ -10,13 +13,15 @@ module Projects def target @target ||= - case @noteable_type - when "Issue" - project.issues.find_by_iid(@noteable_id) - when "MergeRequest" - project.merge_requests.find_by_iid(@noteable_id) - when "Commit" - project.commit(@noteable_id) + case noteable_type + when 'Issue' + IssuesFinder.new(current_user, project_id: project.id, state: 'all'). + execute.find_by(iid: noteable_id) + when 'MergeRequest' + MergeRequestsFinder.new(current_user, project_id: project.id, state: 'all'). + execute.find_by(iid: noteable_id) + when 'Commit' + project.commit(noteable_id) else nil end diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb index f8aeefbfbce..112bebe423a 100644 --- a/app/services/slash_commands/interpret_service.rb +++ b/app/services/slash_commands/interpret_service.rb @@ -11,8 +11,8 @@ module SlashCommands @updates = {} commands = extractor(noteable: noteable).extract_commands!(content) - commands.each do |command| - __send__(*command) + commands.each do |command, *args| + execute_command(command, *args) end @updates @@ -30,8 +30,9 @@ module SlashCommands "Close this #{noteable.to_ability_name.humanize(capitalize: false)}" end condition do + noteable.persisted? && noteable.open? && - current_user.can?(:"update_#{noteable.to_ability_name}", project) + current_user.can?(:"update_#{noteable.to_ability_name}", noteable) end command :close do @updates[:state_event] = 'close' @@ -42,7 +43,7 @@ module SlashCommands end condition do noteable.closed? && - current_user.can?(:"update_#{noteable.to_ability_name}", project) + current_user.can?(:"update_#{noteable.to_ability_name}", noteable) end command :open, :reopen do @updates[:state_event] = 'reopen' @@ -52,7 +53,7 @@ module SlashCommands params '<New title>' condition do noteable.persisted? && - current_user.can?(:"update_#{noteable.to_ability_name}", project) + current_user.can?(:"update_#{noteable.to_ability_name}", noteable) end command :title do |title_param| @updates[:title] = title_param @@ -65,9 +66,8 @@ module SlashCommands end command :assign, :reassign do |assignee_param| user = extract_references(assignee_param, :user).first - return unless user - @updates[:assignee_id] = user.id + @updates[:assignee_id] = user.id if user end desc 'Remove assignee' @@ -87,9 +87,8 @@ module SlashCommands end command :milestone do |milestone_param| milestone = extract_references(milestone_param, :milestone).first - return unless milestone - @updates[:milestone_id] = milestone.id + @updates[:milestone_id] = milestone.id if milestone end desc 'Remove milestone' @@ -109,9 +108,8 @@ module SlashCommands end command :label, :labels do |labels_param| label_ids = find_label_ids(labels_param) - return if label_ids.empty? - @updates[:add_label_ids] = label_ids + @updates[:add_label_ids] = label_ids unless label_ids.empty? end desc 'Remove label(s)' @@ -122,9 +120,8 @@ module SlashCommands end command :unlabel, :remove_label, :remove_labels do |labels_param| label_ids = find_label_ids(labels_param) - return if label_ids.empty? - @updates[:remove_label_ids] = label_ids + @updates[:remove_label_ids] = label_ids unless label_ids.empty? end desc 'Remove all labels' @@ -139,7 +136,6 @@ module SlashCommands desc 'Add a todo' condition do noteable.persisted? && - current_user && !TodoService.new.todo_exist?(noteable, current_user) end command :todo do @@ -148,7 +144,6 @@ module SlashCommands desc 'Mark todo as done' condition do - current_user && TodoService.new.todo_exist?(noteable, current_user) end command :done do @@ -174,12 +169,12 @@ module SlashCommands end desc 'Set due date' - params 'a date in natural language' + params '<in 2 days | this Friday | December 31st>' condition do noteable.respond_to?(:due_date) && - current_user.can?(:"update_#{noteable.to_ability_name}", project) + current_user.can?(:"update_#{noteable.to_ability_name}", noteable) end - command :due_date, :due do |due_date_param| + command :due, :due_date do |due_date_param| due_date = Chronic.parse(due_date_param).try(:to_date) @updates[:due_date] = due_date if due_date @@ -189,7 +184,7 @@ module SlashCommands condition do noteable.respond_to?(:due_date) && noteable.due_date? && - current_user.can?(:"update_#{noteable.to_ability_name}", project) + current_user.can?(:"update_#{noteable.to_ability_name}", noteable) end command :clear_due_date do @updates[:due_date] = nil diff --git a/app/views/layouts/_init_auto_complete.html.haml b/app/views/layouts/_init_auto_complete.html.haml index 351100f3523..a51347fde83 100644 --- a/app/views/layouts/_init_auto_complete.html.haml +++ b/app/views/layouts/_init_auto_complete.html.haml @@ -2,6 +2,6 @@ - noteable_class = @noteable.class if @noteable.present? :javascript - GitLab.GfmAutoComplete.dataSource = "#{autocomplete_sources_namespace_project_path(project.namespace, project, type: noteable_class, type_id: params[:id])}" + GitLab.GfmAutoComplete.dataSource = "#{autocomplete_sources_namespace_project_path(project.namespace, project, type: noteable_class, type_id: params[:id], action_name: action_name)}" GitLab.GfmAutoComplete.cachedData = undefined; GitLab.GfmAutoComplete.setup(); diff --git a/doc/workflow/slash_commands.md b/doc/workflow/slash_commands.md index c4edbeddd40..2bdc18ad248 100644 --- a/doc/workflow/slash_commands.md +++ b/doc/workflow/slash_commands.md @@ -25,5 +25,5 @@ do. | `/done` | None | Mark todo as done | | `/subscribe` | None | Subscribe | | `/unsubscribe` | None | Unsubscribe | -| `/due_date a date in natural language` | `/due` | Set due date | +| `/due <in 2 days | this Friday | December 31st>` | `/due_date` | Set due date | | `/clear_due_date` | None | Remove due date | diff --git a/lib/gitlab/slash_commands/dsl.rb b/lib/gitlab/slash_commands/dsl.rb index 3affd6253e9..ce659aff1da 100644 --- a/lib/gitlab/slash_commands/dsl.rb +++ b/lib/gitlab/slash_commands/dsl.rb @@ -4,20 +4,34 @@ module Gitlab extend ActiveSupport::Concern included do - @command_definitions = [] + cattr_accessor :definitions end - module ClassMethods - # This method is used to generate the autocompletion menu - # It returns no-op slash commands (such as `/cc`) + def execute_command(name, *args) + name = name.to_sym + cmd_def = self.class.definitions.find do |cmd_def| + self.class.command_name_and_aliases(cmd_def).include?(name) + end + return unless cmd_def && cmd_def[:action_block] + return if self.class.command_unavailable?(cmd_def, self) + + block_arity = cmd_def[:action_block].arity + if block_arity == -1 || block_arity == args.size + instance_exec(*args, &cmd_def[:action_block]) + end + end + + class_methods do + # This method is used to generate the autocompletion menu. + # It returns no-op slash commands (such as `/cc`). def command_definitions(opts = {}) - @command_definitions.map do |cmd_def| + self.definitions.map do |cmd_def| context = OpenStruct.new(opts) - next if cmd_def[:cond_block] && !context.instance_exec(&cmd_def[:cond_block]) + next if command_unavailable?(cmd_def, context) cmd_def = cmd_def.dup - if cmd_def[:description].present? && cmd_def[:description].respond_to?(:call) + if cmd_def[:description].respond_to?(:call) cmd_def[:description] = context.instance_exec(&cmd_def[:description]) rescue '' end @@ -30,13 +44,24 @@ module Gitlab # It excludes no-op slash commands (such as `/cc`). # This list can then be given to `Gitlab::SlashCommands::Extractor`. def command_names(opts = {}) - command_definitions(opts).flat_map do |command_definition| - next if command_definition[:noop] + self.definitions.flat_map do |cmd_def| + next if cmd_def[:opts].fetch(:noop, false) - [command_definition[:name], *command_definition[:aliases]] + context = OpenStruct.new(opts) + next if command_unavailable?(cmd_def, context) + + command_name_and_aliases(cmd_def) end.compact end + def command_unavailable?(cmd_def, context) + cmd_def[:condition_block] && !context.instance_exec(&cmd_def[:condition_block]) + end + + def command_name_and_aliases(cmd_def) + [cmd_def[:name], *cmd_def[:aliases]] + end + # Allows to give a description to the next slash command. # This description is shown in the autocomplete menu. # It accepts a block that will be evaluated with the context given to @@ -81,7 +106,7 @@ module Gitlab # # Awesome code block # end def condition(&block) - @cond_block = block + @condition_block = block end # Registers a new command which is recognizeable from body of email or @@ -95,45 +120,22 @@ module Gitlab # end def command(*command_names, &block) opts = command_names.extract_options! - command_name, *aliases = command_names - proxy_method_name = "__#{command_name}__" - - if block_given? - # This proxy method is needed because calling `return` from inside a - # block/proc, causes a `return` from the enclosing method or lambda, - # otherwise a LocalJumpError error is raised. - define_method(proxy_method_name, &block) - - define_method(command_name) do |*args| - return if @cond_block && !instance_exec(&@cond_block) - - proxy_method = method(proxy_method_name) - - if proxy_method.arity == -1 || proxy_method.arity == args.size - instance_exec(*args, &proxy_method) - end - end - - private command_name - aliases.each do |alias_command| - alias_method alias_command, command_name - private alias_command - end - end + name, *aliases = command_names - command_definition = { - name: command_name, + self.definitions ||= [] + self.definitions << { + name: name, aliases: aliases, description: @description || '', - params: @params || [] + params: @params || [], + condition_block: @condition_block, + action_block: block, + opts: opts } - command_definition[:noop] = opts[:noop] || false - command_definition[:cond_block] = @cond_block - @command_definitions << command_definition @description = nil @params = nil - @cond_block = nil + @condition_block = nil end end end diff --git a/spec/features/issues/user_uses_slash_commands_spec.rb b/spec/features/issues/user_uses_slash_commands_spec.rb index 47c4ce306e9..fe320070704 100644 --- a/spec/features/issues/user_uses_slash_commands_spec.rb +++ b/spec/features/issues/user_uses_slash_commands_spec.rb @@ -55,5 +55,4 @@ feature 'Issues > User uses slash commands', feature: true, js: true do end end end - end diff --git a/spec/fixtures/emails/commands_in_reply.eml b/spec/fixtures/emails/commands_in_reply.eml new file mode 100644 index 00000000000..06bf60ab734 --- /dev/null +++ b/spec/fixtures/emails/commands_in_reply.eml @@ -0,0 +1,43 @@ +Return-Path: <jake@adventuretime.ooo> +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog <jake@adventuretime.ooo> +To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +In-Reply-To: <issue_1@localhost> +References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> +Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 + +Cool! + +/close +/todo +/due tomorrow + + +On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta +<reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo> wrote: +> +> +> +> eviltrout posted in 'Adventure Time Sux' on Discourse Meta: +> +> --- +> hey guys everyone knows adventure time sucks! +> +> --- +> Please visit this link to respond: http://localhost:3000/t/adventure-time-sux/1234/3 +> +> To unsubscribe from these emails, visit your [user preferences](http://localhost:3000/user_preferences). +> diff --git a/spec/fixtures/emails/commands_only_reply.eml b/spec/fixtures/emails/commands_only_reply.eml index ccd92e406c4..aed64224b06 100644 --- a/spec/fixtures/emails/commands_only_reply.eml +++ b/spec/fixtures/emails/commands_only_reply.eml @@ -21,6 +21,7 @@ X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 /close /todo +/due tomorrow On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb index afb072105cf..4909fed6b77 100644 --- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -75,13 +75,54 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do project.team << [user, :developer] end - it 'raises a CommandsOnlyNoteError' do - expect { receiver.execute }.not_to raise_error + it 'does not raise an error' do + expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy + + # One system note is created for the 'close' event + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + + expect(noteable.reload).to be_closed + expect(noteable.due_date).to eq(Date.tomorrow) + expect(TodoService.new.todo_exist?(noteable, user)).to be_truthy end end end end + context 'when the note contains slash commands' do + let!(:email_raw) { fixture_file("emails/commands_in_reply.eml") } + + context 'and current user cannot update noteable' do + it 'post a note and does not update the noteable' do + expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy + + # One system note is created for the new note + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + + expect(noteable.reload).to be_open + expect(noteable.due_date).to be_nil + expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy + end + end + + context 'and current user can update noteable' do + before do + project.team << [user, :developer] + end + + it 'post a note and updates the noteable' do + expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy + + # One system note is created for the new note, one for the 'close' event + expect { receiver.execute }.to change { noteable.notes.count }.by(2) + + expect(noteable.reload).to be_closed + expect(noteable.due_date).to eq(Date.tomorrow) + expect(TodoService.new.todo_exist?(noteable, user)).to be_truthy + end + end + end + context "when the reply is blank" do let!(:email_raw) { fixture_file("emails/no_content_reply.eml") } diff --git a/spec/lib/gitlab/slash_commands/dsl_spec.rb b/spec/lib/gitlab/slash_commands/dsl_spec.rb index 385f534ad6f..500ff3ca1fe 100644 --- a/spec/lib/gitlab/slash_commands/dsl_spec.rb +++ b/spec/lib/gitlab/slash_commands/dsl_spec.rb @@ -46,12 +46,42 @@ describe Gitlab::SlashCommands::Dsl do describe '.command_definitions' do let(:base_expected) do [ - { name: :no_args, aliases: [:none], description: 'A command with no args', params: [], noop: false, cond_block: nil }, - { name: :returning, aliases: [], description: 'A command returning a value', params: [], noop: false, cond_block: nil }, - { name: :one_arg, aliases: [:once, :first], description: '', params: ['The first argument'], noop: false, cond_block: nil }, - { name: :two_args, aliases: [], description: '', params: ['The first argument', 'The second argument'], noop: false, cond_block: nil }, - { name: :cc, aliases: [], description: '', params: [], noop: true, cond_block: nil }, - { name: :wildcard, aliases: [], description: '', params: [], noop: false, cond_block: nil } + { + name: :no_args, aliases: [:none], + description: 'A command with no args', params: [], + condition_block: nil, action_block: a_kind_of(Proc), + opts: {} + }, + { + name: :returning, aliases: [], + description: 'A command returning a value', params: [], + condition_block: nil, action_block: a_kind_of(Proc), + opts: {} + }, + { + name: :one_arg, aliases: [:once, :first], + description: '', params: ['The first argument'], + condition_block: nil, action_block: a_kind_of(Proc), + opts: {} + }, + { + name: :two_args, aliases: [], + description: '', params: ['The first argument', 'The second argument'], + condition_block: nil, action_block: a_kind_of(Proc), + opts: {} + }, + { + name: :cc, aliases: [], + description: '', params: [], + condition_block: nil, action_block: nil, + opts: { noop: true } + }, + { + name: :wildcard, aliases: [], + description: '', params: [], + condition_block: nil, action_block: a_kind_of(Proc), + opts: {} + } ] end @@ -61,7 +91,14 @@ describe Gitlab::SlashCommands::Dsl do context 'with options passed' do context 'when condition is met' do - let(:expected) { base_expected << { name: :cond_action, aliases: [], description: '', params: [], noop: false, cond_block: a_kind_of(Proc) } } + let(:expected) do + base_expected << { + name: :cond_action, aliases: [], + description: '', params: [], + condition_block: a_kind_of(Proc), action_block: a_kind_of(Proc), + opts: {} + } + end it 'returns an array with commands definitions' do expect(DummyClass.command_definitions(project: 'foo')).to match_array expected @@ -115,76 +152,78 @@ describe Gitlab::SlashCommands::Dsl do let(:dummy) { DummyClass.new(nil) } - describe 'command with no args' do - context 'called with no args' do - it 'succeeds' do - expect(dummy.__send__(:no_args)).to eq 'Hello World!' + describe '#execute_command' do + describe 'command with no args' do + context 'called with no args' do + it 'succeeds' do + expect(dummy.execute_command(:no_args)).to eq 'Hello World!' + end end end - end - describe 'command with an explicit return' do - context 'called with no args' do - it 'succeeds' do - expect(dummy.__send__(:returning)).to eq 42 + describe 'command with an explicit return' do + context 'called with no args' do + it 'succeeds' do + expect { dummy.execute_command(:returning) }.to raise_error(LocalJumpError) + end end end - end - describe 'command with one arg' do - context 'called with one arg' do - it 'succeeds' do - expect(dummy.__send__(:one_arg, 42)).to eq 42 + describe 'command with one arg' do + context 'called with one arg' do + it 'succeeds' do + expect(dummy.execute_command(:one_arg, 42)).to eq 42 + end end end - end - describe 'command with two args' do - context 'called with two args' do - it 'succeeds' do - expect(dummy.__send__(:two_args, 42, 'foo')).to eq [42, 'foo'] + describe 'command with two args' do + context 'called with two args' do + it 'succeeds' do + expect(dummy.execute_command(:two_args, 42, 'foo')).to eq [42, 'foo'] + end end end - end - - describe 'noop command' do - it 'is not meant to be called directly' do - expect { dummy.__send__(:cc) }.to raise_error(NoMethodError) - end - end - describe 'command with condition' do - context 'when condition is not met' do + describe 'noop command' do it 'returns nil' do - expect(dummy.__send__(:cond_action)).to be_nil + expect(dummy.execute_command(:cc)).to be_nil end end - context 'when condition is met' do - let(:dummy) { DummyClass.new('foo') } + describe 'command with condition' do + context 'when condition is not met' do + it 'returns nil' do + expect(dummy.execute_command(:cond_action)).to be_nil + end + end - it 'succeeds' do - expect(dummy.__send__(:cond_action, 42)).to eq 42 + context 'when condition is met' do + let(:dummy) { DummyClass.new('foo') } + + it 'succeeds' do + expect(dummy.execute_command(:cond_action, 42)).to eq 42 + end end end - end - describe 'command with wildcard' do - context 'called with no args' do - it 'succeeds' do - expect(dummy.__send__(:wildcard)).to eq [] + describe 'command with wildcard' do + context 'called with no args' do + it 'succeeds' do + expect(dummy.execute_command(:wildcard)).to eq [] + end end - end - context 'called with one arg' do - it 'succeeds' do - expect(dummy.__send__(:wildcard, 42)).to eq [42] + context 'called with one arg' do + it 'succeeds' do + expect(dummy.execute_command(:wildcard, 42)).to eq [42] + end end - end - context 'called with two args' do - it 'succeeds' do - expect(dummy.__send__(:wildcard, 42, 'foo')).to eq [42, 'foo'] + context 'called with two args' do + it 'succeeds' do + expect(dummy.execute_command(:wildcard, 42, 'foo')).to eq [42, 'foo'] + end end end end |