Simplify the slash commands DSL to store action blocks instead of creating methods

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>

2 files changed, 36 insertions, 23 deletions

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