diff options
| -rw-r--r-- | app/services/slash_commands/interpret_service.rb | 153 | ||||
| -rw-r--r-- | lib/gitlab/slash_commands/dsl.rb | 111 | ||||
| -rw-r--r-- | spec/lib/gitlab/slash_commands/dsl_spec.rb | 61 |
3 files changed, 171 insertions, 154 deletions
diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb index e94ee83df85..ae22ed6b845 100644 --- a/app/services/slash_commands/interpret_service.rb +++ b/app/services/slash_commands/interpret_service.rb @@ -26,25 +26,23 @@ module SlashCommands Gitlab::SlashCommands::Extractor.new(self.class.command_names(opts)) end - desc ->(opts) { "Close this #{opts[:noteable].to_ability_name.humanize(capitalize: false)}" } - condition ->(opts) do - opts[:noteable] && - opts[:noteable].open? && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"update_#{opts[:noteable].to_ability_name}", opts[:project]) + desc do + "Close this #{noteable.to_ability_name.humanize(capitalize: false)}" + end + condition do + noteable.open? && + current_user.can?(:"update_#{noteable.to_ability_name}", project) end command :close do @updates[:state_event] = 'close' end - desc ->(opts) { "Reopen this #{opts[:noteable].to_ability_name.humanize(capitalize: false)}" } - condition ->(opts) do - opts[:noteable] && - opts[:noteable].closed? && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"update_#{opts[:noteable].to_ability_name}", opts[:project]) + desc do + "Reopen this #{noteable.to_ability_name.humanize(capitalize: false)}" + end + condition do + noteable.closed? && + current_user.can?(:"update_#{noteable.to_ability_name}", project) end command :open, :reopen do @updates[:state_event] = 'reopen' @@ -52,12 +50,9 @@ module SlashCommands desc 'Change title' params '<New title>' - condition ->(opts) do - opts[:noteable] && - opts[:noteable].persisted? && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"update_#{opts[:noteable].to_ability_name}", opts[:project]) + condition do + noteable.persisted? && + current_user.can?(:"update_#{noteable.to_ability_name}", project) end command :title do |title_param| @updates[:title] = title_param @@ -65,11 +60,8 @@ module SlashCommands desc 'Assign' params '@user' - condition ->(opts) do - opts[:noteable] && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project]) + condition do + current_user.can?(:"admin_#{noteable.to_ability_name}", project) end command :assign, :reassign do |assignee_param| user = extract_references(assignee_param, :user).first @@ -79,12 +71,9 @@ module SlashCommands end desc 'Remove assignee' - condition ->(opts) do - opts[:noteable] && - opts[:noteable].assignee_id? && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project]) + condition do + noteable.assignee_id? && + current_user.can?(:"admin_#{noteable.to_ability_name}", project) end command :unassign, :remove_assignee do @updates[:assignee_id] = nil @@ -92,12 +81,9 @@ module SlashCommands desc 'Set milestone' params '%"milestone"' - condition ->(opts) do - opts[:noteable] && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project]) && - opts[:project].milestones.active.any? + condition do + current_user.can?(:"admin_#{noteable.to_ability_name}", project) && + project.milestones.active.any? end command :milestone do |milestone_param| milestone = extract_references(milestone_param, :milestone).first @@ -107,12 +93,9 @@ module SlashCommands end desc 'Remove milestone' - condition ->(opts) do - opts[:noteable] && - opts[:noteable].milestone_id? && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project]) + condition do + noteable.milestone_id? && + current_user.can?(:"admin_#{noteable.to_ability_name}", project) end command :clear_milestone, :remove_milestone do @updates[:milestone_id] = nil @@ -120,12 +103,9 @@ module SlashCommands desc 'Add label(s)' params '~label1 ~"label 2"' - condition ->(opts) do - opts[:noteable] && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project]) && - opts[:project].labels.any? + condition do + current_user.can?(:"admin_#{noteable.to_ability_name}", project) && + project.labels.any? end command :label, :labels do |labels_param| label_ids = find_label_ids(labels_param) @@ -136,12 +116,9 @@ module SlashCommands desc 'Remove label(s)' params '~label1 ~"label 2"' - condition ->(opts) do - opts[:noteable] && - opts[:noteable].labels.any? && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project]) + condition do + noteable.labels.any? && + current_user.can?(:"admin_#{noteable.to_ability_name}", project) end command :unlabel, :remove_label, :remove_labels do |labels_param| label_ids = find_label_ids(labels_param) @@ -151,55 +128,46 @@ module SlashCommands end desc 'Remove all labels' - condition ->(opts) do - opts[:noteable] && - opts[:noteable].labels.any? && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"admin_#{opts[:noteable].to_ability_name}", opts[:project]) + condition do + noteable.labels.any? && + current_user.can?(:"admin_#{noteable.to_ability_name}", project) end command :clear_labels, :clear_label do @updates[:label_ids] = [] end desc 'Add a todo' - condition ->(opts) do - opts[:noteable] && - opts[:noteable].persisted? && - opts[:current_user] && - !TodosFinder.new(opts[:current_user]).execute.exists?(target: opts[:noteable]) + condition do + noteable.persisted? && + current_user && + !TodosFinder.new(current_user).execute.exists?(target: noteable) end command :todo do @updates[:todo_event] = 'add' end desc 'Mark todo as done' - condition ->(opts) do - opts[:noteable] && - opts[:current_user] && - TodosFinder.new(opts[:current_user]).execute.exists?(target: opts[:noteable]) + condition do + current_user && + TodosFinder.new(current_user).execute.exists?(target: noteable) end command :done do @updates[:todo_event] = 'done' end desc 'Subscribe' - condition ->(opts) do - opts[:noteable] && - opts[:current_user] && - opts[:noteable].persisted? && - !opts[:noteable].subscribed?(opts[:current_user]) + condition do + noteable.persisted? && + !noteable.subscribed?(current_user) end command :subscribe do @updates[:subscription_event] = 'subscribe' end desc 'Unsubscribe' - condition ->(opts) do - opts[:noteable] && - opts[:current_user] && - opts[:noteable].persisted? && - opts[:noteable].subscribed?(opts[:current_user]) + condition do + noteable.persisted? && + noteable.subscribed?(current_user) end command :unsubscribe do @updates[:subscription_event] = 'unsubscribe' @@ -207,12 +175,9 @@ module SlashCommands desc 'Set due date' params 'a date in natural language' - condition ->(opts) do - opts[:noteable] && - opts[:noteable].respond_to?(:due_date) && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"update_#{opts[:noteable].to_ability_name}", opts[:project]) + condition do + noteable.respond_to?(:due_date) && + current_user.can?(:"update_#{noteable.to_ability_name}", project) end command :due_date, :due do |due_date_param| due_date = Chronic.parse(due_date_param).try(:to_date) @@ -221,13 +186,10 @@ module SlashCommands end desc 'Remove due date' - condition ->(opts) do - opts[:noteable] && - opts[:noteable].respond_to?(:due_date) && - opts[:noteable].due_date? && - opts[:current_user] && - opts[:project] && - opts[:current_user].can?(:"update_#{opts[:noteable].to_ability_name}", opts[:project]) + condition do + noteable.respond_to?(:due_date) && + noteable.due_date? && + current_user.can?(:"update_#{noteable.to_ability_name}", project) end command :clear_due_date do @updates[:due_date] = nil @@ -236,10 +198,7 @@ module SlashCommands # This is a dummy command, so that it appears in the autocomplete commands desc 'CC' params '@user' - noop true - command :cc do - return - end + command :cc, noop: true def find_label_ids(labels_param) extract_references(labels_param, :label).map(&:id) diff --git a/lib/gitlab/slash_commands/dsl.rb b/lib/gitlab/slash_commands/dsl.rb index 20e1d071d06..3affd6253e9 100644 --- a/lib/gitlab/slash_commands/dsl.rb +++ b/lib/gitlab/slash_commands/dsl.rb @@ -8,20 +8,27 @@ module Gitlab end module ClassMethods + # 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| - next if cmd_def[:cond_lambda] && !cmd_def[:cond_lambda].call(opts) + context = OpenStruct.new(opts) + next if cmd_def[:cond_block] && !context.instance_exec(&cmd_def[:cond_block]) cmd_def = cmd_def.dup if cmd_def[:description].present? && cmd_def[:description].respond_to?(:call) - cmd_def[:description] = cmd_def[:description].call(opts) rescue '' + cmd_def[:description] = context.instance_exec(&cmd_def[:description]) rescue '' end cmd_def end.compact end + # This method is used to generate a list of valid commands in the current + # context of `opts`. + # 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] @@ -30,59 +37,88 @@ module Gitlab end.compact end - # Allows to give a description to the next slash command - def desc(text) - @description = text + # 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 + # `.command_definitions` or `.command_names`. + # + # Example: + # + # desc do + # "This is a dynamic description for #{noteable.to_ability_name}" + # end + # command :command_key do |arguments| + # # Awesome code block + # end + def desc(text = '', &block) + @description = block_given? ? block : text end - # Allows to define params for the next slash command + # Allows to define params for the next slash command. + # These params are shown in the autocomplete menu. + # + # Example: + # + # params "~label ~label2" + # command :command_key do |arguments| + # # Awesome code block + # end def params(*params) @params = params end - # Allows to define if a command is a no-op, but should appear in autocomplete - def noop(noop) - @noop = noop - end - - # Allows to define if a lambda to conditionally return an action - def condition(cond_lambda) - @cond_lambda = cond_lambda - end - - # Registers a new command which is recognizeable - # from body of email or comment. + # Allows to define conditions that must be met in order for the command + # to be returned by `.command_names` & `.command_definitions`. + # It accepts a block that will be evaluated with the context given to + # `.command_definitions`, `.command_names`, and the actual command method. + # # Example: # + # condition do + # project.public? + # end # command :command_key do |arguments| # # Awesome code block # end + def condition(&block) + @cond_block = block + end + + # Registers a new command which is recognizeable from body of email or + # comment. + # It accepts aliases and takes a block. # + # Example: + # + # command :my_command, :alias_for_my_command do |arguments| + # # Awesome code block + # end def command(*command_names, &block) + opts = command_names.extract_options! command_name, *aliases = command_names proxy_method_name = "__#{command_name}__" - # 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) + 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| - unless @cond_lambda.nil? || @cond_lambda.call(project: project, current_user: current_user, noteable: noteable) - return - end + define_method(command_name) do |*args| + return if @cond_block && !instance_exec(&@cond_block) - proxy_method = method(proxy_method_name) + proxy_method = method(proxy_method_name) - if proxy_method.arity == -1 || proxy_method.arity == args.size - instance_exec(*args, &proxy_method) + if proxy_method.arity == -1 || proxy_method.arity == args.size + instance_exec(*args, &proxy_method) + end end - end - private command_name - aliases.each do |alias_command| - alias_method alias_command, command_name - private alias_command + private command_name + aliases.each do |alias_command| + alias_method alias_command, command_name + private alias_command + end end command_definition = { @@ -91,14 +127,13 @@ module Gitlab description: @description || '', params: @params || [] } - command_definition[:noop] = @noop unless @noop.nil? - command_definition[:cond_lambda] = @cond_lambda unless @cond_lambda.nil? + command_definition[:noop] = opts[:noop] || false + command_definition[:cond_block] = @cond_block @command_definitions << command_definition @description = nil @params = nil - @noop = nil - @cond_lambda = nil + @cond_block = nil end end end diff --git a/spec/lib/gitlab/slash_commands/dsl_spec.rb b/spec/lib/gitlab/slash_commands/dsl_spec.rb index 893a7692f11..7c946313ae1 100644 --- a/spec/lib/gitlab/slash_commands/dsl_spec.rb +++ b/spec/lib/gitlab/slash_commands/dsl_spec.rb @@ -1,9 +1,8 @@ require 'spec_helper' describe Gitlab::SlashCommands::Dsl do - COND_LAMBDA = ->(opts) { opts[:project] == 'foo' } before :all do - DummyClass = Class.new do + DummyClass = Struct.new(:project) do include Gitlab::SlashCommands::Dsl desc 'A command with no args' @@ -21,20 +20,21 @@ describe Gitlab::SlashCommands::Dsl do arg1 end - desc ->(opts) { "A dynamic description for #{opts.fetch(:noteable)}" } + desc do + "A dynamic description for #{noteable.upcase}" + end params 'The first argument', 'The second argument' command :two_args do |arg1, arg2| [arg1, arg2] end - noop true - command :cc do |*args| - args - end + command :cc, noop: true - condition COND_LAMBDA - command :cond_action do |*args| - args + condition do + project == 'foo' + end + command :cond_action do |arg| + arg end command :wildcard do |*args| @@ -42,17 +42,16 @@ describe Gitlab::SlashCommands::Dsl do end end end - let(:dummy) { DummyClass.new } describe '.command_definitions' do let(:base_expected) do [ - { name: :no_args, aliases: [:none], description: 'A command with no args', params: [] }, - { name: :returning, aliases: [], description: 'A command returning a value', params: [] }, - { name: :one_arg, aliases: [:once, :first], description: '', params: ['The first argument'] }, - { name: :two_args, aliases: [], description: '', params: ['The first argument', 'The second argument'] }, - { name: :cc, aliases: [], description: '', params: [], noop: true }, - { name: :wildcard, aliases: [], description: '', params: [] } + { 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} ] end @@ -62,7 +61,7 @@ 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: [], cond_lambda: COND_LAMBDA } } + let(:expected) { base_expected << { name: :cond_action, aliases: [], description: '', params: [], noop: false, cond_block: a_kind_of(Proc) } } it 'returns an array with commands definitions' do expect(DummyClass.command_definitions(project: 'foo')).to match_array expected @@ -77,7 +76,7 @@ describe Gitlab::SlashCommands::Dsl do context 'when description can be generated dynamically' do it 'returns an array with commands definitions with dynamic descriptions' do - base_expected[3][:description] = 'A dynamic description for merge request' + base_expected[3][:description] = 'A dynamic description for MERGE REQUEST' expect(DummyClass.command_definitions(noteable: 'merge request')).to match_array base_expected end @@ -114,6 +113,8 @@ describe Gitlab::SlashCommands::Dsl do end end + let(:dummy) { DummyClass.new(nil) } + describe 'command with no args' do context 'called with no args' do it 'succeeds' do @@ -146,6 +147,28 @@ describe Gitlab::SlashCommands::Dsl do 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 + it 'returns nil' do + expect(dummy.__send__(:cond_action)).to be_nil + end + end + + context 'when condition is met' do + let(:dummy) { DummyClass.new('foo') } + + it 'succeeds' do + expect(dummy.__send__(:cond_action, 42)).to eq 42 + end + end + end + describe 'command with wildcard' do context 'called with no args' do it 'succeeds' do |