Commit f749fb7f authored by Z.J. van de Weg's avatar Z.J. van de Weg

Improve style, add more tests

parent b8607576
--- ---
title: Add first slash commands title: Added Mattermost slash command
merge_request: 7438 merge_request: 7438
author: author:
...@@ -85,20 +85,11 @@ module API ...@@ -85,20 +85,11 @@ module API
end end
end end
def project_service def project_service(project = user_project)
@project_service ||= user_project.find_or_initialize_service(params[:service_slug].underscore) @project_service ||= project.find_or_initialize_service(params[:service_slug].underscore)
@project_service || not_found!("Service") @project_service || not_found!("Service")
end end
def service_by_slug(project, slug)
underscored_service = slug.underscore
not_found!('Service') unless Service.available_services_names.include?(underscored_service)
service_method = "#{underscored_service}_service"
service = project.public_send(service_method)
end
def service_attributes def service_attributes
@service_attributes ||= project_service.fields.inject([]) do |arr, hash| @service_attributes ||= project_service.fields.inject([]) do |arr, hash|
arr << hash[:name].to_sym arr << hash[:name].to_sym
......
...@@ -67,7 +67,9 @@ module API ...@@ -67,7 +67,9 @@ module API
post ':id/services/:service_slug/trigger' do post ':id/services/:service_slug/trigger' do
project = Project.find_with_namespace(params[:id]) || Project.find_by(id: params[:id]) project = Project.find_with_namespace(params[:id]) || Project.find_by(id: params[:id])
service = service_by_slug(project, params[:service_slug]) not_found! unless project
service = project_service(project)
result = service.try(:active?) && service.try(:trigger, params) result = service.try(:active?) && service.try(:trigger, params)
...@@ -75,7 +77,7 @@ module API ...@@ -75,7 +77,7 @@ module API
status result[:status] || 200 status result[:status] || 200
present result present result
else else
not_found!('Service') not_found!
end end
end end
end end
......
...@@ -3,7 +3,7 @@ module Gitlab ...@@ -3,7 +3,7 @@ module Gitlab
class BaseCommand class BaseCommand
QUERY_LIMIT = 5 QUERY_LIMIT = 5
def self.match(_) def self.match(_text)
raise NotImplementedError raise NotImplementedError
end end
...@@ -11,11 +11,11 @@ module Gitlab ...@@ -11,11 +11,11 @@ module Gitlab
raise NotImplementedError raise NotImplementedError
end end
def self.available?(_) def self.available?(_project)
raise NotImplementedError raise NotImplementedError
end end
def self.allowed?(_, _) def self.allowed?(_user, _ability)
true true
end end
......
module Mattermost module Mattermost
class Presenter class Presenter
class << self class << self
include Rails.application.routes.url_helpers include Gitlab::Routing.url_helpers
def authorize_chat_name(url) def authorize_chat_name(url)
message = if url message = if url
...@@ -14,7 +14,7 @@ module Mattermost ...@@ -14,7 +14,7 @@ module Mattermost
end end
def help(commands, trigger) def help(commands, trigger)
if commands.count == 0 if commands.empty?
ephemeral_response("No commands configured") unless messages.count > 1 ephemeral_response("No commands configured") unless messages.count > 1
else else
message = header_with_list("Available commands", commands) message = header_with_list("Available commands", commands)
...@@ -50,7 +50,7 @@ module Mattermost ...@@ -50,7 +50,7 @@ module Mattermost
end end
def single_resource(resource) def single_resource(resource)
return error(resource) if resource.errors.any? return error(resource) if resource.errors.any? || !resource.persisted?
message = "### #{title(resource)}" message = "### #{title(resource)}"
message << "\n\n#{resource.description}" if resource.description message << "\n\n#{resource.description}" if resource.description
......
...@@ -7,7 +7,7 @@ describe Gitlab::ChatCommands::Command, service: true do ...@@ -7,7 +7,7 @@ describe Gitlab::ChatCommands::Command, service: true do
subject { described_class.new(project, user, params).execute } subject { described_class.new(project, user, params).execute }
describe '#execute' do describe '#execute' do
context 'when no command is not available' do context 'when no command is available' do
let(:params) { { text: 'issue show 1' } } let(:params) { { text: 'issue show 1' } }
let(:project) { create(:project, has_external_issue_tracker: true) } let(:project) { create(:project, has_external_issue_tracker: true) }
...@@ -45,6 +45,10 @@ describe Gitlab::ChatCommands::Command, service: true do ...@@ -45,6 +45,10 @@ describe Gitlab::ChatCommands::Command, service: true do
it 'presents the issue' do it 'presents the issue' do
expect(subject[:text]).to match("my new issue") expect(subject[:text]).to match("my new issue")
end end
it 'shows a link to the new issue' do
expect(subject[:text]).to match(/\/issues\/\d+/)
end
end end
end end
end end
...@@ -34,7 +34,7 @@ describe Gitlab::ChatCommands::IssueCreate, service: true do ...@@ -34,7 +34,7 @@ describe Gitlab::ChatCommands::IssueCreate, service: true do
end end
end end
describe 'self.match' do describe '.match' do
it 'matches the title without description' do it 'matches the title without description' do
match = described_class.match("issue create my title") match = described_class.match("issue create my title")
......
...@@ -116,6 +116,7 @@ project: ...@@ -116,6 +116,7 @@ project:
- base_tags - base_tags
- tag_taggings - tag_taggings
- tags - tags
- chat_services
- creator - creator
- group - group
- namespace - namespace
...@@ -127,6 +128,7 @@ project: ...@@ -127,6 +128,7 @@ project:
- emails_on_push_service - emails_on_push_service
- builds_email_service - builds_email_service
- pipelines_email_service - pipelines_email_service
- mattermost_command_service
- irker_service - irker_service
- pivotaltracker_service - pivotaltracker_service
- hipchat_service - hipchat_service
......
...@@ -102,6 +102,8 @@ describe API::API, api: true do ...@@ -102,6 +102,8 @@ describe API::API, api: true do
end end
context 'the service exists' do context 'the service exists' do
let(:params) { { token: 'token' } }
context 'the service is not active' do context 'the service is not active' do
let!(:inactive_service) do let!(:inactive_service) do
project.create_mattermost_command_service( project.create_mattermost_command_service(
...@@ -124,7 +126,6 @@ describe API::API, api: true do ...@@ -124,7 +126,6 @@ describe API::API, api: true do
properties: { token: 'token' } properties: { token: 'token' }
) )
end end
let(:params) { { token: 'token' } }
it 'retusn status 200' do it 'retusn status 200' do
post api("/projects/#{project.id}/services/mattermost_command/trigger"), params post api("/projects/#{project.id}/services/mattermost_command/trigger"), params
...@@ -132,6 +133,15 @@ describe API::API, api: true do ...@@ -132,6 +133,15 @@ describe API::API, api: true do
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
end end
end end
context 'when the project can not be found' do
it 'returns a generic 404' do
post api("/projects/404/services/mattermost_command/trigger"), params
expect(response).to have_http_status(404)
expect(json_response["message"]).to eq '404 Not Found'
end
end
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment