First steps on refactoring Mattermost Slash commands

Now, each subcommand has its own service, plus I've introduced
presenters to be able to delegate the generation of the views.

app/models/project_services/mattermost_chat_service.rb

@@ -33,31 +33,10 @@ class MattermostChatService < ChatService
   end
 
   def trigger(params)
-    return nil unless valid_token?(params[:token])
-
-    user = find_chat_user(params)
-    return authorize_chat_name(params) unless user
+    user = ChatNames::FindUserService.new(chat_names, params).execute
+    return Mattermost::Presenter.authorize_chat_name(params) unless user
 
     Mattermost::CommandService.new(project, user, params.slice(:command, :text)).
       execute
   end
-
-  private
-
-  def find_chat_user(params)
-    params = params.slice(:team_id, :user_id)
-    ChatNames::FindUserService.
-      new(chat_names, params).
-      execute
-  end
-
-  def authorize_chat_name(params)
-    params = params.slice(:team_id, :team_domain, :user_id, :user_name)
-    url = ChatNames::AuthorizeUserService.new(self, params).execute
-
-    {
-      response_type: :ephemeral,
-      message: "You are not authorized. Click this [link](#{url}) to authorize."
-    }
-  end
 end

app/services/mattermost/command_service.rb

-module Mattermost
-  class CommandService < BaseService
-    SERVICES = [
-      Mattermost::Commands::IssueService,
-      Mattermost::Commands::MergeRequestService
-    ]
-
-    def execute
-      return unknown_user unless current_user
-      return not_found_404 unless can?(current_user, :read_project, project)
-
-      triggered_command = command
-      service = SERVICES.find do |service|
-        service.triggered_by?(triggered_command) && service.available?(project)
-      end
-
-      if service
-        present service.new(project, current_user, params).execute
-      else
-        help_message
-      end
-    end
-
-    private
-
-    def command
-      params[:text].split.first
-    end
-
-    def present(result)
-      return not_found_404 unless result
-
-      if result.respond_to?(:count)
-        if count > 1
-          # TODO
-          return resource_list(result)
-        else
-          result = result.first
-        end
-      end
-
-      message = "### [#{result.to_reference} #{result.title}](link(result))"
-      message << "\n\n#{result.description}" if result.description
-
-      {
-        response_type: :in_channel,
-        text: message
-      }
-    end
-
-    def unknown_user
-      {
-        response_type: :ephemeral,
-        text: 'Hi there! I have not yet had the pleasure to get acquainted!' # TODO allow user to authenticate and authorize
-      }
-    end
-
-    def not_found_404
-      {
-        response_type: :ephemeral,
-        text: "404 not found! GitLab couldn't find what your were looking for! :boom:",
-      }
-    end
-
-    def help_message
-      command_help_messages = SERVICES.map { |service| service.help_message(project) }
-
-      {
-        response_type: :ephemeral,
-        text: "Sadly, the used command does not exist, lets take a look at your options here:\n\n#{command_help_messages.join("\n")}"
-      }
-    end
-  end
-end

app/services/mattermost/commands/base_service.rb

 module Mattermost
   module Commands
     class BaseService < ::BaseService
-      class << self
-        def triggered_by?(_)
+      QUERY_LIMIT = 5
+
+      def execute
         raise NotImplementedError
       end
 
-        def available?(_)
+      def available?
         raise NotImplementedError
       end
 
-        def help_message(_)
-          NotImplementedError
-        end
+      def collection
+        raise NotImplementedError
       end
 
-      QUERY_LIMIT = 5
+      private
 
-      def execute
-        subcommand, args = parse_command
+      def find_by_iid(iid)
+        resource = collection.find_by(iid: iid)
 
-        if subcommands.include?(subcommand)
-          send(subcommand, args)
-        else
-          nil
-        end
+        readable?(resource) ? resource : nil
       end
 
-      private
-
-      # This method can only be used by a resource that has an iid. Also, the
-      # class should implement #collection itself. Probably project.resource
-      # would suffice
-      def show(args)
-        iid = args.first
-
-        result = collection.find_by(iid: iid)
-        if readable?(result)
-          result
-        else
-          nil
+      def search
+        collection.search(query).limit(QUERY_LIMIT).select do |resource|
+          readable?(resource)
         end
       end
 
-      # Child class should implement #collection
-      def search(args)
-        query = args.join(' ')
-
-        collection.search(query).limit(QUERY_LIMIT).select do |issuable|
-          readable?(issuable)
-        end
+      # params[:text] = issue search <search query>
+      def query
+        params[:text].split[2..-1].join(' ')
       end
 
-      def command
-        params[:text]
+      # params[:text] = 'mergerequest show 123'
+      def iid
+        params[:text].split[2]
       end
     end
   end

app/services/mattermost/commands/issue_create_service.rb

+module Mattermost
+  module Commands
+    class IssueShowService < Mattermost::Commands::BaseService
+      def execute
+        return Mattermost::Messages::Issues.not_available unless available?
+
+        issue = find_by_iid(iid)
+
+        present issue
+      end
+    end
+  end
+end

app/services/mattermost/commands/issue_search_service.rb

+module Mattermost
+  module Commands
+    class IssueShowService < IssueService
+      def execute
+        return Mattermost::Messages::Issues.not_available unless available?
+
+        present search_results
+      end
+    end
+  end
+end

app/services/mattermost/commands/issue_service.rb

 module Mattermost
   module Commands
     class IssueService < Mattermost::Commands::BaseService
-      class << self
-        def triggered_by?(command)
-          command == 'issue'
-        end
-
-        def available?(project)
+      def available?
         project.issues_enabled? && project.default_issues_tracker?
       end
 
-        def help_message(project)
-          return nil unless available?(project)
-
-          message = "issue show <issue_id>"
-          message << "issue search <query>"
-        end
-      end
-
-      private
-
-      def create(_)
-        return nil unless can?(current_user, :create_issue, project)
-
-        # We parse again as the previous split splits on continues whitespace
-        # per the ruby spec, but we loose information on where the new lines were
-        match = command.match(/\Aissue create (?<title>.*)\n*/)
-        title = match[:title]
-        description = match.post_match
-
-        Issues::CreateService.new(project, current_user, title: title, description: description).execute
-      end
-
-      def subcommands
-        %w[create search show]
-      end
-
       def collection
         project.issues
       end
@@ -44,12 +13,8 @@ module Mattermost
         can?(current_user, :read_issue, issue)
       end
 
-      def parse_command
-        split = command.split
-        subcommand = split[1]
-        args = split[2..-1]
-
-        [subcommand, args]
+      def present
+        Mattermost::Presenter.issue
       end
     end
   end

app/services/mattermost/commands/issue_show_service.rb

+module Mattermost
+  module Commands
+    class IssueShowService < IssueService
+      def execute
+        return Mattermost::Messages.not_available unless available?
+
+        issue = find_by_iid(iid)
+        present issue
+      end
+    end
+  end
+end

app/services/mattermost/commands/merge_request_search_service.rb

+module Mattermost
+  module Commands
+    class MergeRequestService < Mattermost::Commands::BaseService
+      def execute
+        return Mattermost::Messages::MergeRequests.not_available unless available?
+
+        Mattermost::Messages::IssuePresenter.present search_results
+      end
+    end
+  end
+end

app/services/mattermost/commands/merge_request_service.rb

 module Mattermost
   module Commands
     class MergeRequestService < Mattermost::Commands::BaseService
-      class << self
-        def triggered_by?(command)
-          command == 'mergerequest'
-        end
-
-        def available?(project)
-          project.merge_requests_enabled?
-        end
-
-        def help_message(project)
-          return nil unless available?(project)
-
-          message = "mergerequest show <merge request id>\n"
-          message << "mergerequest search <query>"
-        end
-      end
-
-      private
-
-      def subcommands
-        %w[show search]
+      def available?
+        project.issues_enabled? && project.default_issues_tracker?
       end
 
       def collection
@@ -32,14 +13,8 @@ module Mattermost
         can?(current_user, :read_merge_request, project)
       end
 
-      # 'mergerequest show 123' => 'show', ['123']
-      # 'mergerequest search my query' => 'search',['my', 'query']
-      def parse_command
-        split = command.split
-        subcommand = split[1]
-        args = split[2..-1]
-
-        [subcommand, args]
+      def present
+        Mattermost::Presenter.merge_request
       end
     end
   end

app/services/mattermost/commands/merge_request_show_service.rb

+module Mattermost
+  module Commands
+    class MergeRequestShowService < Mattermost::Commands::BaseService
+      def execute
+        return Mattermost::Messages.not_available unless available?
+
+        merge_request = find_by_iid(iid)
+        present merge_request
+      end
+    end
+  end
+end

app/services/mattermost/slash_command_service.rb

+module Mattermost
+  class SlashCommandService < BaseService
+    def self.command(command, sub_command, klass, help_message)
+      registry[command][sub_command] = { klass: klass, help_message: help_message }
+    end
+
+    command 'issue', 'show',    Mattermost::Commands::IssueShowService,   'issue show <id>'
+    command 'issue', 'search',  Mattermost::Commands::IssueSearchService, 'issue search <query>'
+    command 'issue', 'create',  Mattermost::Commands::IssueCreateService, 'issue create my title'
+
+    command 'mergerequest', 'show',   Mattermost::Commands::MergeRequestShowService,    'mergerequest show <id>'
+    command 'mergerequest', 'search', Mattermost::Commands::MergeRequestSearchService,  'mergerequest search <query>'
+
+    def execute
+      service = registry[command][subcommand]
+
+      return help_messages(registry) unless service.try(:available?)
+
+      service.new(project, current_user, params).execute
+    end
+
+    private
+
+    def self.registry
+      @registry ||= Hash.new({})
+    end
+  end
+end

lib/mattermost/presenter.rb

+module Mattermost
+  class Presenter
+    class << self
+      COMMAND_PREFIX = '/gitlab'.freeze
+
+      def authorize_chat_name(params)
+        url = ChatNames::RequestService.new(service, params).execute
+
+        {
+          response_type: :ephemeral,
+          message: "You are not authorized. Click this [link](#{url}) to authorize."
+        }
+      end
+
+      # TODO figure out how I know which are available or not
+      def help_message(commands)
+        messages = ["Available commands:"]
+
+        commands.each do |sub_command, attrs|
+          messages << "\t#{COMMAND_PREFIX} #{attrs[:help_message]}"
+        end
+
+        {
+          response_type: :ephemeral,
+          text: messages.join("\n")
+        }
+      end
+
+      def not_found
+        {
+          response_type: :ephemeral,
+          text: "404 not found! GitLab couldn't find what your were looking for! :boom:",
+        }
+      end
+    end
+
+    attr_reader :result
+
+    def initialize(result)
+      @result = result
+    end
+
+    def present
+      if result.respond_to?(:count)
+        if result.count > 1
+          return respond_collection(result)
+        elsif result.count == 0
+          return not_found
+        else
+          result = result.first
+        end
+      end
+
+      single_resource
+    end
+
+    private
+
+    def single_resource
+      message = title(resource)
+      message << "\n\n#{resource.description}" if resource.description
+
+      {
+        response_type: :in_channel,
+        text: message
+      }
+    end
+
+    def multiple_resources(resources)
+      message = "Multiple results were found:\n"
+      message << resource.map { |resource| "  #{title(resource)}" }.join("\n")
+
+      {
+        response_type: :ephemeral,
+        text: message
+      }
+    end
+
+    def title(resource)
+      url = url_for([resource.project.namespace.becomes(Namespace), resource.project, resource])
+      "### [#{resource.to_reference} #{resource.title}](#{url})"
+    end
+  end
+end

spec/services/mattermost/command_service_spec.rb

-require 'spec_helper'
-
-describe Mattermost::CommandService, service: true do
-  let(:project)         { build(:project) }
-  let(:user)            { build(:user) }
-  let(:params)          { { text: 'issue show 1' } }
-
-  subject { described_class.new(project, user, params).execute }
-
-  describe '#execute' do
-    context 'no user could be found' do
-      let(:user) { nil }
-
-      it 'asks the user to introduce him/herself' do
-        expect(subject[:response_type]).to be :ephemeral
-        expect(subject[:text]).to start_with 'Hi there!'
-      end
-    end
-
-    context 'no project could be found' do
-      it 'shows a 404 not found message' do
-        expect(subject[:response_type]).to be :ephemeral
-        expect(subject[:text]).to start_with '404 not found!'
-      end
-    end
-
-    context 'the user has access to the project' do
-      let(:project) { create(:project) }
-      let(:user)    { create(:user) }
-
-      before do
-        project.team << [user, :master]
-      end
-
-      context 'no command service is triggered' do
-        let(:params) { { text: 'unknown_command' } }
-
-        it 'shows the help messages' do
-          expect(subject[:response_type]).to be :ephemeral
-          expect(subject[:text]).to start_with 'Sadly, the used command'
-        end
-      end
-
-      context 'a valid command is executed' do
-        let(:issue)   { create(:issue, project: project) }
-        let(:params)  { { text: "issue show #{issue.iid}" } }
-
-        it 'a resource is presented to the user' do
-          expect(subject[:response_type]).to be :in_channel
-          expect(subject[:text]).to match issue.title
-        end
-      end
-    end
-  end
-end

spec/services/mattermost/slash_command_service_spec.rb

+require 'spec_helper'
+
+describe Mattermost::SlashCommandService, service: true do
+  let(:project)         { build(:project) }
+  let(:user)            { build(:user) }
+  let(:params)          { { text: 'issue show 1' } }
+
+  subject { described_class.new(project, user, params).execute }
+
+  describe '#execute' do
+    context 'no command service is triggered' do
+      let(:params) { { text: 'unknown_command' } }
+
+      it 'shows the help messages' do
+        expect(subject[:response_type]).to be :ephemeral
+        expect(subject[:text]).to start_with 'Sadly, the used command'
+      end
+    end
+
+    context 'a valid command is executed' do
+      let(:issue)   { create(:issue, project: project) }
+      let(:params)  { { text: "issue show #{issue.iid}" } }
+
+      it 'a resource is presented to the user' do
+        expect(subject[:response_type]).to be :in_channel
+        expect(subject[:text]).to match issue.title
+      end
+    end
+  end
+end