Improve and test /issue command

3 files changed, 154 insertions, 44 deletions

diff --git a/app/services/mattermost/base_service.rb b/app/services/mattermost/base_service.rb
index 83de4852ca4..d96d4eff13a 100644
--- a/app/services/mattermost/base_service.rb
+++ b/app/services/mattermost/base_service.rb
@@ -1,13 +1,8 @@
 module Mattermost
   class BaseService < ::BaseService
     def execute
-      resource = if resource_id
-                   find_resource
-                 else
-                   collection.search(params[:text]).limit(5)
-                 end
-
-      generate_response(resource)
+      # Implement in child
+      raise NotImplementedError
     end
 
     private
@@ -18,17 +13,14 @@ module Mattermost
       match ? match[:id] : nil
     end
 
-    def find_resource
-      collection.find_by(iid: resource_id)
-    end
 
     def generate_response(resource)
-      return response_404 if resource.nil?
+      return respond_404 if resource.nil?
       return single_resource(resource) unless resource.respond_to?(:count)
-      return no_results if resource.empty?
+      return no_search_results if resource.empty?
 
       if resource.count == 1
-        single_resource(resource)
+        single_resource(resource.first)
       else
         multiple_resources(resource)
       end
@@ -44,17 +36,7 @@ module Mattermost
     def no_search_results
       {
         response_type: :ephemeral,
-        text: "No search results for \"#{params[:text]}\". :disappointed:"
-      }
-    end
-
-    def single_resource(resource)
-      {
-        response_type: :in_channel,
-        text: %{### #{resource.to_reference} #{resource.title}
-
-        #{resource.description.truncate(256)}
-        }
+        text: "### No search results for \"#{params[:text]}\". :disappointed:"
       }
     end
 
@@ -63,11 +45,7 @@ module Mattermost
 
       {
         response_type: :ephemeral,
-        text: <<-MD
-        #### Search results for \"#{params[:text]}\"
-
-        #{list}
-        MD
+        text: "### Search results for \"#{params[:text]}\"\n\n#{list}"
       }
     end
   end
diff --git a/app/services/mattermost/issue_service.rb b/app/services/mattermost/issue_service.rb
index dc26039ef41..e1429c23438 100644
--- a/app/services/mattermost/issue_service.rb
+++ b/app/services/mattermost/issue_service.rb
@@ -1,30 +1,56 @@
 module Mattermost
   class IssueService < BaseService
+    SUBCOMMANDS = ['create', 'search'].freeze
+
     def execute
-      if params[:text].start_with?('create')
-        create_issue
-      else
-        super
-      end
+      resource  = if resource_id
+                    find_resource
+                  elsif subcommand
+                    send(subcommand)
+                  else
+                    nil
+                  end
+
+      generate_response(resource)
     end
 
     private
 
-    def create_issue
-      issue = Issues::CreateService.new(project, current_user, issue_params).execute
+    def find_resource
+      return nil unless can?(current_user, :read_issue, project)
+
+      collection.find_by(iid: resource_id)
+    end
+
+    def create
+      return nil unless can?(current_user, :create_issue, project)
 
-      byebug
-      if issue.valid?
-        generate_response(issue)
-      else
-        issue_create_error(issue.errors.full_messages)
-      end
+      Issues::CreateService.new(project, current_user, issue_params).execute
+    end
+
+    def search
+      return nil unless can?(current_user, :read_issue, project)
+
+      query = params[:text].gsub(/\Asearch /, '')
+      collection.search(query).limit(5)
     end
 
     def issue_create_error(errors)
       {
         response_type: :ephemeral,
-        text: "An error occured creating your issue: #{errors}"
+        text: "An error occured creating your issue: #{errors}" #TODO; this displays an array
+      }
+    end
+
+    def single_resource(issue)
+      return issue_create_error(issue) if issue.errors.any?
+
+      message = "### [#{issue.to_reference} #{issue.title}](#{link(issue)})"
+      message << "\n\n#{issue.description}" if issue.description
+
+      {
+        response_type: :in_channel,
+        text: message
       }
     end
 
@@ -45,8 +71,14 @@ module Mattermost
 
       {
         title: match[:title],
-        description: params[:text].gsub(/\Acreate .+$\s*/, ''),
+        description: params[:text].gsub(/\Acreate .+$\s*/, '')
       }
     end
+
+    def subcommand
+      match = params[:text].match(/\A(?<subcommand>(#{SUBCOMMANDS.join('|')}))/)
+
+      match ? match[:subcommand] : nil
+    end
   end
 end
diff --git a/spec/services/mattermost/issue_service_spec.rb b/spec/services/mattermost/issue_service_spec.rb
new file mode 100644
index 00000000000..b45971bdf9b
--- /dev/null
+++ b/spec/services/mattermost/issue_service_spec.rb
@@ -0,0 +1,100 @@
+require 'spec_helper'
+
+describe Mattermost::IssueService, services: true do
+  let(:project) { create(:empty_project) }
+  let(:user) { create(:user) }
+
+  let(:service) { described_class.new(project, user, params) }
+
+  shared_examples 'a 404 response' do
+    it 'responds with a 404 message' do
+      expect(subject[:response_type]).to be :ephemeral
+      expect(subject[:text]).to start_with '404 not found!'
+    end
+  end
+
+  describe '#execute' do
+    subject { service.execute }
+
+    context 'Looking up on iid' do
+      let(:params) { { text: 1 } }
+
+      context 'when the user can not read issues' do
+        it_behaves_like 'a 404 response'
+      end
+
+      context 'when the user has access' do
+        context 'when the resource exists' do
+          let(:issue) { create(:issue, project: project) }
+          let(:params) { { text: issue.iid } }
+
+          before do
+            project.team << [user, :master]
+          end
+
+          it 'returns the resource' do
+            expect(subject[:response_type]).to be :in_channel
+            expect(subject[:text]).to start_with "### [#{issue.to_reference} "
+          end
+        end
+
+        context 'when the resource does not exists' do
+          it_behaves_like 'a 404 response'
+        end
+      end
+    end
+
+    context 'searching for issues' do
+      let!(:issue) { create(:issue, project: project, title: 'Bird is the word') }
+      let!(:issue1) { create(:issue, project: project, title: 'Everybody heard about the bird') }
+      let(:params) { { text: 'search bird' } }
+
+      context 'when the user has no access' do
+        it_behaves_like 'a 404 response'
+      end
+
+      context 'when the user has acces' do
+        before do
+          project.team << [user, :master]
+        end
+
+        context 'when there are results' do
+          it 'returns the resource' do
+            expect(subject[:response_type]).to be :ephemeral
+            expect(subject[:text]).to start_with "### Search results for"
+          end
+        end
+
+        context 'when there is only one result' do
+          let(:params) { { text: 'search about the bird' } }
+
+          it 'returns the resource' do
+            expect(subject[:response_type]).to be :in_channel
+            expect(subject[:text]).to start_with "### [#{issue1.to_reference} "
+          end
+        end
+      end
+    end
+
+    context 'creating an issue' do
+      let(:params) { { text: "create The Trashmen\nBird is the word" } }
+
+      context 'when the user has no access' do
+        it_behaves_like 'a 404 response'
+      end
+
+      context 'when the user has acces' do
+        before do
+          project.team << [user, :master]
+        end
+
+        context 'it creates an issue' do
+          it 'returns the resource' do
+            expect(subject[:response_type]).to be :in_channel
+            expect(subject[:text]).to match /The Trashmen/
+          end
+        end
+      end
+    end
+  end
+end