From f393f2dde016edf63b5168eb63405f15d65803eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Fri, 12 Aug 2016 11:19:29 +0200
Subject: [PATCH] Simplify the slash commands DSL to store action blocks
instead of creating methods
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Other improvements:
- Ensure slash commands autocomplete doesn't break when noteable_type is not given
- Slash commands: improve autocomplete behavior and /due command
- We don't display slash commands for note edit forms.
- Add tests for reply by email with slash commands
- Be sure to execute slash commands after the note creation in Notes::CreateService
Signed-off-by: Rémy Coutable <remy@rymai.me>
---
.../javascripts/gfm_auto_complete.js.es6 | 4 +-
app/controllers/projects_controller.rb | 8 +-
app/services/issuable_base_service.rb | 4 +-
app/services/notes/create_service.rb | 5 +-
app/services/notes/slash_commands_service.rb | 15 +-
app/services/projects/autocomplete_service.rb | 34 ++--
app/services/projects/participants_service.rb | 25 +--
.../slash_commands/interpret_service.rb | 33 ++--
.../layouts/_init_auto_complete.html.haml | 2 +-
doc/workflow/slash_commands.md | 2 +-
lib/gitlab/slash_commands/dsl.rb | 88 +++++------
.../issues/user_uses_slash_commands_spec.rb | 1 -
spec/fixtures/emails/commands_in_reply.eml | 43 ++++++
spec/fixtures/emails/commands_only_reply.eml | 1 +
.../email/handler/create_note_handler_spec.rb | 45 +++++-
spec/lib/gitlab/slash_commands/dsl_spec.rb | 145 +++++++++++-------
16 files changed, 296 insertions(+), 159 deletions(-)
create mode 100644 spec/fixtures/emails/commands_in_reply.eml
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
--
GitLab