diff options
author | Z.J. van de Weg <git@zjvandeweg.nl> | 2016-11-18 11:38:54 +0100 |
---|---|---|
committer | Z.J. van de Weg <git@zjvandeweg.nl> | 2016-11-18 11:38:54 +0100 |
commit | f749fb7fe0574d07eeb38561b9af62754e518281 (patch) | |
tree | e8647693e60b73f12cd3b9dca0efb0aec1796fcb | |
parent | b8607576c1bb25fe2cbb575b48becdf7bd4aaa8c (diff) | |
download | gitlab-ce-f749fb7fe0574d07eeb38561b9af62754e518281.tar.gz |
Improve style, add more tests
-rw-r--r-- | changelogs/unreleased/zj-slash-commands-mattermost.yml | 2 | ||||
-rw-r--r-- | lib/api/helpers.rb | 13 | ||||
-rw-r--r-- | lib/api/services.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/chat_commands/base_command.rb | 6 | ||||
-rw-r--r-- | lib/mattermost/presenter.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/chat_commands/command_spec.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/chat_commands/issue_create_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/all_models.yml | 4 | ||||
-rw-r--r-- | spec/models/project_services/mattermost_command_service_spec.rb | 10 | ||||
-rw-r--r-- | spec/requests/api/services_spec.rb | 12 |
10 files changed, 38 insertions, 29 deletions
diff --git a/changelogs/unreleased/zj-slash-commands-mattermost.yml b/changelogs/unreleased/zj-slash-commands-mattermost.yml index 84aca57e666..996ffe954f3 100644 --- a/changelogs/unreleased/zj-slash-commands-mattermost.yml +++ b/changelogs/unreleased/zj-slash-commands-mattermost.yml @@ -1,4 +1,4 @@ --- -title: Add first slash commands +title: Added Mattermost slash command merge_request: 7438 author: diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index d6526ec4fdc..2c593dbb4ea 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -85,20 +85,11 @@ module API end end - def project_service - @project_service ||= user_project.find_or_initialize_service(params[:service_slug].underscore) + def project_service(project = user_project) + @project_service ||= project.find_or_initialize_service(params[:service_slug].underscore) @project_service || not_found!("Service") 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 @service_attributes ||= project_service.fields.inject([]) do |arr, hash| arr << hash[:name].to_sym diff --git a/lib/api/services.rb b/lib/api/services.rb index 163187d450d..e3c6a998631 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -67,7 +67,9 @@ module API post ':id/services/:service_slug/trigger' do 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) @@ -75,7 +77,7 @@ module API status result[:status] || 200 present result else - not_found!('Service') + not_found! end end end diff --git a/lib/gitlab/chat_commands/base_command.rb b/lib/gitlab/chat_commands/base_command.rb index f84aca5365d..e59d69b72b9 100644 --- a/lib/gitlab/chat_commands/base_command.rb +++ b/lib/gitlab/chat_commands/base_command.rb @@ -3,7 +3,7 @@ module Gitlab class BaseCommand QUERY_LIMIT = 5 - def self.match(_) + def self.match(_text) raise NotImplementedError end @@ -11,11 +11,11 @@ module Gitlab raise NotImplementedError end - def self.available?(_) + def self.available?(_project) raise NotImplementedError end - def self.allowed?(_, _) + def self.allowed?(_user, _ability) true end diff --git a/lib/mattermost/presenter.rb b/lib/mattermost/presenter.rb index d7455d39bce..b4e7358770f 100644 --- a/lib/mattermost/presenter.rb +++ b/lib/mattermost/presenter.rb @@ -1,7 +1,7 @@ module Mattermost class Presenter class << self - include Rails.application.routes.url_helpers + include Gitlab::Routing.url_helpers def authorize_chat_name(url) message = if url @@ -14,7 +14,7 @@ module Mattermost end def help(commands, trigger) - if commands.count == 0 + if commands.empty? ephemeral_response("No commands configured") unless messages.count > 1 else message = header_with_list("Available commands", commands) @@ -50,7 +50,7 @@ module Mattermost end def single_resource(resource) - return error(resource) if resource.errors.any? + return error(resource) if resource.errors.any? || !resource.persisted? message = "### #{title(resource)}" message << "\n\n#{resource.description}" if resource.description diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb index 11b607a6843..f44c848479e 100644 --- a/spec/lib/gitlab/chat_commands/command_spec.rb +++ b/spec/lib/gitlab/chat_commands/command_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::ChatCommands::Command, service: true do subject { described_class.new(project, user, params).execute } 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(:project) { create(:project, has_external_issue_tracker: true) } @@ -45,6 +45,10 @@ describe Gitlab::ChatCommands::Command, service: true do it 'presents the issue' do expect(subject[:text]).to match("my new issue") end + + it 'shows a link to the new issue' do + expect(subject[:text]).to match(/\/issues\/\d+/) + end end end end diff --git a/spec/lib/gitlab/chat_commands/issue_create_spec.rb b/spec/lib/gitlab/chat_commands/issue_create_spec.rb index 4831f24efed..df0c317ccea 100644 --- a/spec/lib/gitlab/chat_commands/issue_create_spec.rb +++ b/spec/lib/gitlab/chat_commands/issue_create_spec.rb @@ -34,7 +34,7 @@ describe Gitlab::ChatCommands::IssueCreate, service: true do end end - describe 'self.match' do + describe '.match' do it 'matches the title without description' do match = described_class.match("issue create my title") diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 02b11bd999a..bc837299b9e 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -116,6 +116,7 @@ project: - base_tags - tag_taggings - tags +- chat_services - creator - group - namespace @@ -127,6 +128,7 @@ project: - emails_on_push_service - builds_email_service - pipelines_email_service +- mattermost_command_service - irker_service - pivotaltracker_service - hipchat_service @@ -188,4 +190,4 @@ award_emoji: - awardable - user priorities: -- label
\ No newline at end of file +- label diff --git a/spec/models/project_services/mattermost_command_service_spec.rb b/spec/models/project_services/mattermost_command_service_spec.rb index 757ad687bc4..21bf1d35e3f 100644 --- a/spec/models/project_services/mattermost_command_service_spec.rb +++ b/spec/models/project_services/mattermost_command_service_spec.rb @@ -55,11 +55,11 @@ describe MattermostCommandService, models: true do end context 'when an auth url can be generated' do - let(:params) do + let(:params) do { - team_domain: 'http://domain.tld', - team_id: 'T3423423', - user_id: 'U234234', + team_domain: 'http://domain.tld', + team_id: 'T3423423', + user_id: 'U234234', user_name: 'mepmep', token: 'token' } @@ -72,7 +72,7 @@ describe MattermostCommandService, models: true do end it 'generates the url' do - response = service.trigger(params) + response = service.trigger(params) expect(response[:text]).to start_with(':wave: Hi there!') end diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index fb234ab8ed1..765d662e52b 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -102,6 +102,8 @@ describe API::API, api: true do end context 'the service exists' do + let(:params) { { token: 'token' } } + context 'the service is not active' do let!(:inactive_service) do project.create_mattermost_command_service( @@ -124,7 +126,6 @@ describe API::API, api: true do properties: { token: 'token' } ) end - let(:params) { { token: 'token' } } it 'retusn status 200' do post api("/projects/#{project.id}/services/mattermost_command/trigger"), params @@ -132,6 +133,15 @@ describe API::API, api: true do expect(response).to have_http_status(200) 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 |