From 0ce0c1be95728191e976f8f72843fd59719b06c7 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 7 Oct 2016 11:47:21 +0200 Subject: Improve and test /issue command --- app/services/mattermost/base_service.rb | 36 ++------- app/services/mattermost/issue_service.rb | 62 +++++++++++---- spec/services/mattermost/issue_service_spec.rb | 100 +++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 44 deletions(-) create mode 100644 spec/services/mattermost/issue_service_spec.rb 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(?(#{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 -- cgit v1.2.1