From 0d3e24358b88ce41848c97f3ac37bc813074f260 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Wed, 30 Nov 2016 15:51:48 +0100 Subject: Create Slack Slash command service --- Gemfile | 2 +- Gemfile.lock | 4 +- app/models/project.rb | 1 + .../slack_slash_commands_service.rb | 49 ++++++++ app/models/service.rb | 3 +- .../mattermost_slash_commands/_help.html.haml | 2 +- changelogs/unreleased/zj-slack-slash-commands.yml | 4 + lib/gitlab/chat_commands/deploy.rb | 2 +- lib/gitlab/chat_commands/help.rb | 28 +++++ lib/gitlab/chat_commands/presenters/mattermost.rb | 131 +++++++++++++++++++++ lib/mattermost/presenter.rb | 131 --------------------- spec/lib/gitlab/chat_commands/command_spec.rb | 26 +++- .../chat_message/build_message_spec.rb | 8 +- .../chat_message/issue_message_spec.rb | 10 +- .../chat_message/merge_message_spec.rb | 12 +- .../chat_message/note_message_spec.rb | 22 ++-- .../chat_message/push_message_spec.rb | 26 ++-- .../chat_message/wiki_page_message_spec.rb | 8 +- 18 files changed, 287 insertions(+), 182 deletions(-) create mode 100644 app/models/project_services/slack_slash_commands_service.rb create mode 100644 changelogs/unreleased/zj-slack-slash-commands.yml create mode 100644 lib/gitlab/chat_commands/help.rb create mode 100644 lib/gitlab/chat_commands/presenters/mattermost.rb delete mode 100644 lib/mattermost/presenter.rb diff --git a/Gemfile b/Gemfile index 5eb8c32b168..a59b874248b 100644 --- a/Gemfile +++ b/Gemfile @@ -170,7 +170,7 @@ gem 'gitlab-flowdock-git-hook', '~> 1.0.1' gem 'gemnasium-gitlab-service', '~> 0.2' # Slack integration -gem 'slack-notifier', '~> 1.2.0' +gem 'slack-notifier', '~> 1.5.1' # Asana integration gem 'asana', '~> 0.4.0' diff --git a/Gemfile.lock b/Gemfile.lock index 23e45ddc16f..f33b171e1d2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -687,7 +687,7 @@ GEM json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.0) - slack-notifier (1.2.1) + slack-notifier (1.5.1) slop (3.6.0) spinach (0.8.10) colorize @@ -957,7 +957,7 @@ DEPENDENCIES sidekiq-cron (~> 0.4.4) sidekiq-limit_fetch (~> 3.4) simplecov (= 0.12.0) - slack-notifier (~> 1.2.0) + slack-notifier (~> 1.5.1) spinach-rails (~> 0.2.1) spinach-rerun-reporter (~> 0.0.2) spring (~> 1.7.0) diff --git a/app/models/project.rb b/app/models/project.rb index 5d092ca42c2..7f3a4debfc6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -96,6 +96,7 @@ class Project < ActiveRecord::Base has_one :gemnasium_service, dependent: :destroy has_one :mattermost_slash_commands_service, dependent: :destroy has_one :mattermost_notification_service, dependent: :destroy + has_one :slack_slash_commands_service, dependent: :destroy has_one :slack_notification_service, dependent: :destroy has_one :buildkite_service, dependent: :destroy has_one :bamboo_service, dependent: :destroy diff --git a/app/models/project_services/slack_slash_commands_service.rb b/app/models/project_services/slack_slash_commands_service.rb new file mode 100644 index 00000000000..9c66f95eef9 --- /dev/null +++ b/app/models/project_services/slack_slash_commands_service.rb @@ -0,0 +1,49 @@ +class SlackSlashCommandsService < ChatService + include TriggersHelper + + prop_accessor :token + + def can_test? + false + end + + def title + 'Slack Slash Command' + end + + def description + "Perform common operations on GitLab in Slack" + end + + def to_param + 'slack_slash_commands' + end + + def fields + [ + { type: 'text', name: 'token', placeholder: '' } + ] + end + + def trigger(params) + return nil unless valid_token?(params[:token]) + + user = find_chat_user(params) + unless user + url = authorize_chat_name_url(params) + return Gitlab::ChatCommands::Presenters::Access.new(url).authorize + end + + Gitlab::ChatCommands::Command.new(project, user, params).execute + end + + private + + def find_chat_user(params) + ChatNames::FindUserService.new(self, params).execute + end + + def authorize_chat_name_url(params) + ChatNames::AuthorizeUserService.new(self, params).execute + end +end diff --git a/app/models/service.rb b/app/models/service.rb index 0bbab078cf6..8abd8e73e43 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -216,11 +216,12 @@ class Service < ActiveRecord::Base jira kubernetes mattermost_slash_commands + mattermost_notification pipelines_email pivotaltracker pushover redmine - mattermost_notification + slack_slash_commands slack_notification teamcity ] diff --git a/app/views/projects/services/mattermost_slash_commands/_help.html.haml b/app/views/projects/services/mattermost_slash_commands/_help.html.haml index a676c0290a0..2bdd4cd148c 100644 --- a/app/views/projects/services/mattermost_slash_commands/_help.html.haml +++ b/app/views/projects/services/mattermost_slash_commands/_help.html.haml @@ -1,4 +1,4 @@ -- pretty_path_with_namespace = "#{@project ? @project.namespace.name : 'namespace'} / #{@project ? @project.name : 'name'}" +- pretty_path_with_namespace = "#{@project.namespace ? @project.namespace.name : 'namespace'} / #{@project ? @project.name : 'name'}" - run_actions_text = "Perform common operations on this project: #{pretty_path_with_namespace}" .well diff --git a/changelogs/unreleased/zj-slack-slash-commands.yml b/changelogs/unreleased/zj-slack-slash-commands.yml new file mode 100644 index 00000000000..9f4c8681ad0 --- /dev/null +++ b/changelogs/unreleased/zj-slack-slash-commands.yml @@ -0,0 +1,4 @@ +--- +title: Refactor presenters ChatCommands +merge_request: 7846 +author: diff --git a/lib/gitlab/chat_commands/deploy.rb b/lib/gitlab/chat_commands/deploy.rb index 0eed1fce0dc..6bb854dc080 100644 --- a/lib/gitlab/chat_commands/deploy.rb +++ b/lib/gitlab/chat_commands/deploy.rb @@ -4,7 +4,7 @@ module Gitlab include Gitlab::Routing.url_helpers def self.match(text) - /\Adeploy\s+(?.*)\s+to+\s+(?.*)\z/.match(text) + /\Adeploy\s+(?\S+.*)\s+to+\s+(?\S+.*)\z/.match(text) end def self.help_message diff --git a/lib/gitlab/chat_commands/help.rb b/lib/gitlab/chat_commands/help.rb new file mode 100644 index 00000000000..e76733f5445 --- /dev/null +++ b/lib/gitlab/chat_commands/help.rb @@ -0,0 +1,28 @@ +module Gitlab + module ChatCommands + class Help < BaseCommand + # This class has to be used last, as it always matches. It has to match + # because other commands were not triggered and we want to show the help + # command + def self.match(_text) + true + end + + def self.help_message + 'help' + end + + def self.allowed?(_project, _user) + true + end + + def execute(commands) + Gitlab::ChatCommands::Presenters::Help.new(commands).present(trigger) + end + + def trigger + params[:command] + end + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/mattermost.rb b/lib/gitlab/chat_commands/presenters/mattermost.rb new file mode 100644 index 00000000000..67eda983a74 --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/mattermost.rb @@ -0,0 +1,131 @@ +module Mattermost + class Presenter + class << self + include Gitlab::Routing.url_helpers + + def authorize_chat_name(url) + message = if url + ":wave: Hi there! Before I do anything for you, please [connect your GitLab account](#{url})." + else + ":sweat_smile: Couldn't identify you, nor can I autorize you!" + end + + ephemeral_response(message) + end + + def help(commands, trigger) + if commands.none? + ephemeral_response("No commands configured") + else + commands.map! { |command| "#{trigger} #{command}" } + message = header_with_list("Available commands", commands) + + ephemeral_response(message) + end + end + + def present(subject) + return not_found unless subject + + if subject.is_a?(Gitlab::ChatCommands::Result) + show_result(subject) + elsif subject.respond_to?(:count) + if subject.many? + multiple_resources(subject) + elsif subject.none? + not_found + else + single_resource(subject) + end + else + single_resource(subject) + end + end + + def access_denied + ephemeral_response("Whoops! That action is not allowed. This incident will be [reported](https://xkcd.com/838/).") + end + + private + + def show_result(result) + case result.type + when :success + in_channel_response(result.message) + else + ephemeral_response(result.message) + end + end + + def not_found + ephemeral_response("404 not found! GitLab couldn't find what you were looking for! :boom:") + end + + def single_resource(resource) + return error(resource) if resource.errors.any? || !resource.persisted? + + message = "### #{title(resource)}" + message << "\n\n#{resource.description}" if resource.try(:description) + + in_channel_response(message) + end + + def multiple_resources(resources) + resources.map! { |resource| title(resource) } + + message = header_with_list("Multiple results were found:", resources) + + ephemeral_response(message) + end + + def error(resource) + message = header_with_list("The action was not successful, because:", resource.errors.messages) + + ephemeral_response(message) + end + + def title(resource) + reference = resource.try(:to_reference) || resource.try(:id) + title = resource.try(:title) || resource.try(:name) + + "[#{reference} #{title}](#{url(resource)})" + end + + def header_with_list(header, items) + message = [header] + + items.each do |item| + message << "- #{item}" + end + + message.join("\n") + end + + def url(resource) + url_for( + [ + resource.project.namespace.becomes(Namespace), + resource.project, + resource + ] + ) + end + + def ephemeral_response(message) + { + response_type: :ephemeral, + text: message, + status: 200 + } + end + + def in_channel_response(message) + { + response_type: :in_channel, + text: message, + status: 200 + } + end + end + end +end diff --git a/lib/mattermost/presenter.rb b/lib/mattermost/presenter.rb deleted file mode 100644 index 67eda983a74..00000000000 --- a/lib/mattermost/presenter.rb +++ /dev/null @@ -1,131 +0,0 @@ -module Mattermost - class Presenter - class << self - include Gitlab::Routing.url_helpers - - def authorize_chat_name(url) - message = if url - ":wave: Hi there! Before I do anything for you, please [connect your GitLab account](#{url})." - else - ":sweat_smile: Couldn't identify you, nor can I autorize you!" - end - - ephemeral_response(message) - end - - def help(commands, trigger) - if commands.none? - ephemeral_response("No commands configured") - else - commands.map! { |command| "#{trigger} #{command}" } - message = header_with_list("Available commands", commands) - - ephemeral_response(message) - end - end - - def present(subject) - return not_found unless subject - - if subject.is_a?(Gitlab::ChatCommands::Result) - show_result(subject) - elsif subject.respond_to?(:count) - if subject.many? - multiple_resources(subject) - elsif subject.none? - not_found - else - single_resource(subject) - end - else - single_resource(subject) - end - end - - def access_denied - ephemeral_response("Whoops! That action is not allowed. This incident will be [reported](https://xkcd.com/838/).") - end - - private - - def show_result(result) - case result.type - when :success - in_channel_response(result.message) - else - ephemeral_response(result.message) - end - end - - def not_found - ephemeral_response("404 not found! GitLab couldn't find what you were looking for! :boom:") - end - - def single_resource(resource) - return error(resource) if resource.errors.any? || !resource.persisted? - - message = "### #{title(resource)}" - message << "\n\n#{resource.description}" if resource.try(:description) - - in_channel_response(message) - end - - def multiple_resources(resources) - resources.map! { |resource| title(resource) } - - message = header_with_list("Multiple results were found:", resources) - - ephemeral_response(message) - end - - def error(resource) - message = header_with_list("The action was not successful, because:", resource.errors.messages) - - ephemeral_response(message) - end - - def title(resource) - reference = resource.try(:to_reference) || resource.try(:id) - title = resource.try(:title) || resource.try(:name) - - "[#{reference} #{title}](#{url(resource)})" - end - - def header_with_list(header, items) - message = [header] - - items.each do |item| - message << "- #{item}" - end - - message.join("\n") - end - - def url(resource) - url_for( - [ - resource.project.namespace.becomes(Namespace), - resource.project, - resource - ] - ) - end - - def ephemeral_response(message) - { - response_type: :ephemeral, - text: message, - status: 200 - } - end - - def in_channel_response(message) - { - response_type: :in_channel, - text: message, - status: 200 - } - end - end - end -end diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb index bfc6818ac08..ed8d25a526a 100644 --- a/spec/lib/gitlab/chat_commands/command_spec.rb +++ b/spec/lib/gitlab/chat_commands/command_spec.rb @@ -64,7 +64,7 @@ describe Gitlab::ChatCommands::Command, service: true do context 'and user can not create deployment' do it 'returns action' do expect(subject[:response_type]).to be(:ephemeral) - expect(subject[:text]).to start_with('Whoops! That action is not allowed') + expect(subject[:text]).to start_with('Whoops! This action is not allowed') end end @@ -74,7 +74,7 @@ describe Gitlab::ChatCommands::Command, service: true do end it 'returns action' do - expect(subject[:text]).to include('Deployment from staging to production started') + expect(subject[:text]).to include('Deployment started from staging to production') expect(subject[:response_type]).to be(:in_channel) end @@ -91,4 +91,26 @@ describe Gitlab::ChatCommands::Command, service: true do end end end + + describe '#match_command' do + subject { described_class.new(project, user, params).match_command.first } + + context 'IssueShow is triggered' do + let(:params) { { text: 'issue show 123' } } + + it { is_expected.to eq(Gitlab::ChatCommands::IssueShow) } + end + + context 'IssueCreate is triggered' do + let(:params) { { text: 'issue create my title' } } + + it { is_expected.to eq(Gitlab::ChatCommands::IssueCreate) } + end + + context 'IssueSearch is triggered' do + let(:params) { { text: 'issue search my query' } } + + it { is_expected.to eq(Gitlab::ChatCommands::IssueSearch) } + end + end end diff --git a/spec/models/project_services/chat_message/build_message_spec.rb b/spec/models/project_services/chat_message/build_message_spec.rb index b71d153f814..50ad5013df9 100644 --- a/spec/models/project_services/chat_message/build_message_spec.rb +++ b/spec/models/project_services/chat_message/build_message_spec.rb @@ -10,7 +10,7 @@ describe ChatMessage::BuildMessage do tag: false, project_name: 'project_name', - project_url: 'example.gitlab.com', + project_url: 'http://example.gitlab.com', commit: { status: status, @@ -48,10 +48,10 @@ describe ChatMessage::BuildMessage do end def build_message(status_text = status) - ":" \ - " Commit :" \ + " Commit " \ - " of branch" \ + " of branch" \ " by hacker #{status_text} in #{duration} #{'second'.pluralize(duration)}" end end diff --git a/spec/models/project_services/chat_message/issue_message_spec.rb b/spec/models/project_services/chat_message/issue_message_spec.rb index ebe0ead4408..190ff4c535d 100644 --- a/spec/models/project_services/chat_message/issue_message_spec.rb +++ b/spec/models/project_services/chat_message/issue_message_spec.rb @@ -10,14 +10,14 @@ describe ChatMessage::IssueMessage, models: true do username: 'test.user' }, project_name: 'project_name', - project_url: 'somewhere.com', + project_url: 'http://somewhere.com', object_attributes: { title: 'Issue title', id: 10, iid: 100, assignee_id: 1, - url: 'url', + url: 'http://url.com', action: 'open', state: 'opened', description: 'issue description' @@ -40,11 +40,11 @@ describe ChatMessage::IssueMessage, models: true do context 'open' do it 'returns a message regarding opening of issues' do expect(subject.pretext).to eq( - '] Issue opened by test.user') + '[] Issue opened by test.user') expect(subject.attachments).to eq([ { title: "#100 Issue title", - title_link: "url", + title_link: "http://url.com", text: "issue description", color: color, } @@ -60,7 +60,7 @@ describe ChatMessage::IssueMessage, models: true do it 'returns a message regarding closing of issues' do expect(subject.pretext). to eq( - '] Issue closed by test.user') + '[] Issue closed by test.user') expect(subject.attachments).to be_empty end end diff --git a/spec/models/project_services/chat_message/merge_message_spec.rb b/spec/models/project_services/chat_message/merge_message_spec.rb index 07c414c6ca4..cc154112e90 100644 --- a/spec/models/project_services/chat_message/merge_message_spec.rb +++ b/spec/models/project_services/chat_message/merge_message_spec.rb @@ -10,14 +10,14 @@ describe ChatMessage::MergeMessage, models: true do username: 'test.user' }, project_name: 'project_name', - project_url: 'somewhere.com', + project_url: 'http://somewhere.com', object_attributes: { title: "Issue title\nSecond line", id: 10, iid: 100, assignee_id: 1, - url: 'url', + url: 'http://url.com', state: 'opened', description: 'issue description', source_branch: 'source_branch', @@ -31,8 +31,8 @@ describe ChatMessage::MergeMessage, models: true do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'test.user opened '\ - 'in : *Issue title*') + 'test.user opened '\ + 'in : *Issue title*') expect(subject.attachments).to be_empty end end @@ -43,8 +43,8 @@ describe ChatMessage::MergeMessage, models: true do end it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'test.user closed '\ - 'in : *Issue title*') + 'test.user closed '\ + 'in : *Issue title*') expect(subject.attachments).to be_empty end end diff --git a/spec/models/project_services/chat_message/note_message_spec.rb b/spec/models/project_services/chat_message/note_message_spec.rb index 31936da40a2..da700a08e57 100644 --- a/spec/models/project_services/chat_message/note_message_spec.rb +++ b/spec/models/project_services/chat_message/note_message_spec.rb @@ -11,15 +11,15 @@ describe ChatMessage::NoteMessage, models: true do avatar_url: 'http://fakeavatar' }, project_name: 'project_name', - project_url: 'somewhere.com', + project_url: 'http://somewhere.com', repository: { name: 'project_name', - url: 'somewhere.com', + url: 'http://somewhere.com', }, object_attributes: { id: 10, note: 'comment on a commit', - url: 'url', + url: 'http://url.com', noteable_type: 'Commit' } } @@ -37,8 +37,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on commits' do message = described_class.new(@args) - expect(message.pretext).to eq("test.user in : " \ + expect(message.pretext).to eq("test.user in : " \ "*Added a commit message*") expected_attachments = [ { @@ -63,8 +63,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on a merge request' do message = described_class.new(@args) - expect(message.pretext).to eq("test.user in : " \ + expect(message.pretext).to eq("test.user in : " \ "*merge request title*") expected_attachments = [ { @@ -90,8 +90,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on an issue' do message = described_class.new(@args) expect(message.pretext).to eq( - "test.user in : " \ + "test.user in : " \ "*issue title*") expected_attachments = [ { @@ -115,8 +115,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on a project snippet' do message = described_class.new(@args) - expect(message.pretext).to eq("test.user in : " \ + expect(message.pretext).to eq("test.user in : " \ "*snippet title*") expected_attachments = [ { diff --git a/spec/models/project_services/chat_message/push_message_spec.rb b/spec/models/project_services/chat_message/push_message_spec.rb index b781c4505db..24928873bad 100644 --- a/spec/models/project_services/chat_message/push_message_spec.rb +++ b/spec/models/project_services/chat_message/push_message_spec.rb @@ -10,7 +10,7 @@ describe ChatMessage::PushMessage, models: true do project_name: 'project_name', ref: 'refs/heads/master', user_name: 'test.user', - project_url: 'url' + project_url: 'http://url.com' } end @@ -19,20 +19,20 @@ describe ChatMessage::PushMessage, models: true do context 'push' do before do args[:commits] = [ - { message: 'message1', url: 'url1', id: 'abcdefghijkl', author: { name: 'author1' } }, - { message: 'message2', url: 'url2', id: '123456789012', author: { name: 'author2' } }, + { message: 'message1', url: 'http://url1.com', id: 'abcdefghijkl', author: { name: 'author1' } }, + { message: 'message2', url: 'http://url2.com', id: '123456789012', author: { name: 'author2' } }, ] end it 'returns a message regarding pushes' do expect(subject.pretext).to eq( - 'test.user pushed to branch of '\ - ' ()' + 'test.user pushed to branch of '\ + ' ()' ) expect(subject.attachments).to eq([ { - text: ": message1 - author1\n"\ - ": message2 - author2", + text: ": message1 - author1\n"\ + ": message2 - author2", color: color, } ]) @@ -47,14 +47,14 @@ describe ChatMessage::PushMessage, models: true do project_name: 'project_name', ref: 'refs/tags/new_tag', user_name: 'test.user', - project_url: 'url' + project_url: 'http://url.com' } end it 'returns a message regarding pushes' do expect(subject.pretext).to eq('test.user pushed new tag ' \ - ' to ' \ - '') + ' to ' \ + '') expect(subject.attachments).to be_empty end end @@ -66,8 +66,8 @@ describe ChatMessage::PushMessage, models: true do it 'returns a message regarding a new branch' do expect(subject.pretext).to eq( - 'test.user pushed new branch to '\ - '' + 'test.user pushed new branch to '\ + '' ) expect(subject.attachments).to be_empty end @@ -80,7 +80,7 @@ describe ChatMessage::PushMessage, models: true do it 'returns a message regarding a removed branch' do expect(subject.pretext).to eq( - 'test.user removed branch master from ' + 'test.user removed branch master from ' ) expect(subject.attachments).to be_empty end diff --git a/spec/models/project_services/chat_message/wiki_page_message_spec.rb b/spec/models/project_services/chat_message/wiki_page_message_spec.rb index 94c04dc0865..a2ad61e38e7 100644 --- a/spec/models/project_services/chat_message/wiki_page_message_spec.rb +++ b/spec/models/project_services/chat_message/wiki_page_message_spec.rb @@ -10,10 +10,10 @@ describe ChatMessage::WikiPageMessage, models: true do username: 'test.user' }, project_name: 'project_name', - project_url: 'somewhere.com', + project_url: 'http://somewhere.com', object_attributes: { title: 'Wiki page title', - url: 'url', + url: 'http://url.com', content: 'Wiki page description' } } @@ -25,7 +25,7 @@ describe ChatMessage::WikiPageMessage, models: true do it 'returns a message that a new wiki page was created' do expect(subject.pretext).to eq( - 'test.user created in : '\ + 'test.user created in : '\ '*Wiki page title*') end end @@ -35,7 +35,7 @@ describe ChatMessage::WikiPageMessage, models: true do it 'returns a message that a wiki page was updated' do expect(subject.pretext).to eq( - 'test.user edited in : '\ + 'test.user edited in : '\ '*Wiki page title*') end end -- cgit v1.2.1 From ed880e4954803f9753cfafe0dd8106852245d10d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 15 Dec 2016 23:45:10 +0100 Subject: Rename ChatService into ChatSlashCommandsService --- app/models/project.rb | 1 - app/models/project_services/chat_service.rb | 21 --------- .../chat_slash_commands_service.rb | 55 ++++++++++++++++++++++ .../mattermost_slash_commands_service.rb | 28 +---------- .../slack_slash_commands_service.rb | 36 ++------------ 5 files changed, 60 insertions(+), 81 deletions(-) delete mode 100644 app/models/project_services/chat_service.rb create mode 100644 app/models/project_services/chat_slash_commands_service.rb diff --git a/app/models/project.rb b/app/models/project.rb index 7f3a4debfc6..9d8351399f9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -79,7 +79,6 @@ class Project < ActiveRecord::Base has_one :last_event, -> {order 'events.created_at DESC'}, class_name: 'Event' has_many :boards, before_add: :validate_board_limit, dependent: :destroy - has_many :chat_services # Project services has_one :campfire_service, dependent: :destroy diff --git a/app/models/project_services/chat_service.rb b/app/models/project_services/chat_service.rb deleted file mode 100644 index 574788462de..00000000000 --- a/app/models/project_services/chat_service.rb +++ /dev/null @@ -1,21 +0,0 @@ -# Base class for Chat services -# This class is not meant to be used directly, but only to inherit from. -class ChatService < Service - default_value_for :category, 'chat' - - has_many :chat_names, foreign_key: :service_id - - def valid_token?(token) - self.respond_to?(:token) && - self.token.present? && - ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token) - end - - def supported_events - [] - end - - def trigger(params) - raise NotImplementedError - end -end diff --git a/app/models/project_services/chat_slash_commands_service.rb b/app/models/project_services/chat_slash_commands_service.rb new file mode 100644 index 00000000000..b21a1730285 --- /dev/null +++ b/app/models/project_services/chat_slash_commands_service.rb @@ -0,0 +1,55 @@ +# Base class for Chat services +# This class is not meant to be used directly, but only to inherrit from. +class ChatSlashCommandsService < Service + default_value_for :category, 'chat' + + prop_accessor :token + + has_many :chat_names, foreign_key: :service_id + + def valid_token?(token) + self.respond_to?(:token) && + self.token.present? && + ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token) + end + + def supported_events + [] + end + + def can_test? + false + end + + def fields + [ + { type: 'text', name: 'token', placeholder: '' } + ] + end + + def trigger(params) + return nil unless valid_token?(params[:token]) + + user = find_chat_user(params) + unless user + url = authorize_chat_name_url(params) + return presenter.authorize_chat_name(url) + end + + Gitlab::ChatCommands::Command.new(presenter, project, user, params).execute + end + + private + + def find_chat_user(params) + ChatNames::FindUserService.new(self, params).execute + end + + def authorize_chat_name_url(params) + ChatNames::AuthorizeUserService.new(self, params).execute + end + + def presenter + throw NotImplementedError + end +end diff --git a/app/models/project_services/mattermost_slash_commands_service.rb b/app/models/project_services/mattermost_slash_commands_service.rb index 33431f41dc2..4c1e27cafbb 100644 --- a/app/models/project_services/mattermost_slash_commands_service.rb +++ b/app/models/project_services/mattermost_slash_commands_service.rb @@ -19,31 +19,7 @@ class MattermostSlashCommandsService < ChatService 'mattermost_slash_commands' end - def fields - [ - { type: 'text', name: 'token', placeholder: '' } - ] - end - - def trigger(params) - return nil unless valid_token?(params[:token]) - - user = find_chat_user(params) - unless user - url = authorize_chat_name_url(params) - return Mattermost::Presenter.authorize_chat_name(url) - end - - Gitlab::ChatCommands::Command.new(project, user, params).execute - end - - private - - def find_chat_user(params) - ChatNames::FindUserService.new(self, params).execute - end - - def authorize_chat_name_url(params) - ChatNames::AuthorizeUserService.new(self, params).execute + def presenter + Gitlab::ChatCommands::Presenters::Mattermost.new end end diff --git a/app/models/project_services/slack_slash_commands_service.rb b/app/models/project_services/slack_slash_commands_service.rb index 9c66f95eef9..978d48e8c18 100644 --- a/app/models/project_services/slack_slash_commands_service.rb +++ b/app/models/project_services/slack_slash_commands_service.rb @@ -1,12 +1,6 @@ -class SlackSlashCommandsService < ChatService +class SlackSlashCommandsService < ChatSlashCommandsService include TriggersHelper - prop_accessor :token - - def can_test? - false - end - def title 'Slack Slash Command' end @@ -19,31 +13,7 @@ class SlackSlashCommandsService < ChatService 'slack_slash_commands' end - def fields - [ - { type: 'text', name: 'token', placeholder: '' } - ] - end - - def trigger(params) - return nil unless valid_token?(params[:token]) - - user = find_chat_user(params) - unless user - url = authorize_chat_name_url(params) - return Gitlab::ChatCommands::Presenters::Access.new(url).authorize - end - - Gitlab::ChatCommands::Command.new(project, user, params).execute - end - - private - - def find_chat_user(params) - ChatNames::FindUserService.new(self, params).execute - end - - def authorize_chat_name_url(params) - ChatNames::AuthorizeUserService.new(self, params).execute + def presenter + Gitlab::ChatCommands::Presenters::Mattermost.new end end -- cgit v1.2.1 From 37057870a6b4bf4bf42cc7210a6ae17d68ae5448 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 15 Dec 2016 23:45:46 +0100 Subject: Rename Mattermost::Presenter to Presenter --- lib/gitlab/chat_commands/presenter.rb | 133 ++++++++++++++++++++++ lib/gitlab/chat_commands/presenters/mattermost.rb | 131 --------------------- 2 files changed, 133 insertions(+), 131 deletions(-) create mode 100644 lib/gitlab/chat_commands/presenter.rb delete mode 100644 lib/gitlab/chat_commands/presenters/mattermost.rb diff --git a/lib/gitlab/chat_commands/presenter.rb b/lib/gitlab/chat_commands/presenter.rb new file mode 100644 index 00000000000..3143dc092a1 --- /dev/null +++ b/lib/gitlab/chat_commands/presenter.rb @@ -0,0 +1,133 @@ +module Gitlab + class ChatCommands + class Presenter + class << self + include Gitlab::Routing.url_helpers + + def authorize_chat_name(url) + message = if url + ":wave: Hi there! Before I do anything for you, please [connect your GitLab account](#{url})." + else + ":sweat_smile: Couldn't identify you, nor can I autorize you!" + end + + ephemeral_response(message) + end + + def help(commands, trigger) + if commands.none? + ephemeral_response("No commands configured") + else + commands.map! { |command| "#{trigger} #{command}" } + message = header_with_list("Available commands", commands) + + ephemeral_response(message) + end + end + + def present(subject) + return not_found unless subject + + if subject.is_a?(Gitlab::ChatCommands::Result) + show_result(subject) + elsif subject.respond_to?(:count) + if subject.many? + multiple_resources(subject) + elsif subject.none? + not_found + else + single_resource(subject) + end + else + single_resource(subject) + end + end + + def access_denied + ephemeral_response("Whoops! That action is not allowed. This incident will be [reported](https://xkcd.com/838/).") + end + + private + + def show_result(result) + case result.type + when :success + in_channel_response(result.message) + else + ephemeral_response(result.message) + end + end + + def not_found + ephemeral_response("404 not found! GitLab couldn't find what you were looking for! :boom:") + end + + def single_resource(resource) + return error(resource) if resource.errors.any? || !resource.persisted? + + message = "### #{title(resource)}" + message << "\n\n#{resource.description}" if resource.try(:description) + + in_channel_response(message) + end + + def multiple_resources(resources) + resources.map! { |resource| title(resource) } + + message = header_with_list("Multiple results were found:", resources) + + ephemeral_response(message) + end + + def error(resource) + message = header_with_list("The action was not successful, because:", resource.errors.messages) + + ephemeral_response(message) + end + + def title(resource) + reference = resource.try(:to_reference) || resource.try(:id) + title = resource.try(:title) || resource.try(:name) + + "[#{reference} #{title}](#{url(resource)})" + end + + def header_with_list(header, items) + message = [header] + + items.each do |item| + message << "- #{item}" + end + + message.join("\n") + end + + def url(resource) + url_for( + [ + resource.project.namespace.becomes(Namespace), + resource.project, + resource + ] + ) + end + + def ephemeral_response(message) + { + response_type: :ephemeral, + text: message, + status: 200 + } + end + + def in_channel_response(message) + { + response_type: :in_channel, + text: message, + status: 200 + } + end + end + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/mattermost.rb b/lib/gitlab/chat_commands/presenters/mattermost.rb deleted file mode 100644 index 67eda983a74..00000000000 --- a/lib/gitlab/chat_commands/presenters/mattermost.rb +++ /dev/null @@ -1,131 +0,0 @@ -module Mattermost - class Presenter - class << self - include Gitlab::Routing.url_helpers - - def authorize_chat_name(url) - message = if url - ":wave: Hi there! Before I do anything for you, please [connect your GitLab account](#{url})." - else - ":sweat_smile: Couldn't identify you, nor can I autorize you!" - end - - ephemeral_response(message) - end - - def help(commands, trigger) - if commands.none? - ephemeral_response("No commands configured") - else - commands.map! { |command| "#{trigger} #{command}" } - message = header_with_list("Available commands", commands) - - ephemeral_response(message) - end - end - - def present(subject) - return not_found unless subject - - if subject.is_a?(Gitlab::ChatCommands::Result) - show_result(subject) - elsif subject.respond_to?(:count) - if subject.many? - multiple_resources(subject) - elsif subject.none? - not_found - else - single_resource(subject) - end - else - single_resource(subject) - end - end - - def access_denied - ephemeral_response("Whoops! That action is not allowed. This incident will be [reported](https://xkcd.com/838/).") - end - - private - - def show_result(result) - case result.type - when :success - in_channel_response(result.message) - else - ephemeral_response(result.message) - end - end - - def not_found - ephemeral_response("404 not found! GitLab couldn't find what you were looking for! :boom:") - end - - def single_resource(resource) - return error(resource) if resource.errors.any? || !resource.persisted? - - message = "### #{title(resource)}" - message << "\n\n#{resource.description}" if resource.try(:description) - - in_channel_response(message) - end - - def multiple_resources(resources) - resources.map! { |resource| title(resource) } - - message = header_with_list("Multiple results were found:", resources) - - ephemeral_response(message) - end - - def error(resource) - message = header_with_list("The action was not successful, because:", resource.errors.messages) - - ephemeral_response(message) - end - - def title(resource) - reference = resource.try(:to_reference) || resource.try(:id) - title = resource.try(:title) || resource.try(:name) - - "[#{reference} #{title}](#{url(resource)})" - end - - def header_with_list(header, items) - message = [header] - - items.each do |item| - message << "- #{item}" - end - - message.join("\n") - end - - def url(resource) - url_for( - [ - resource.project.namespace.becomes(Namespace), - resource.project, - resource - ] - ) - end - - def ephemeral_response(message) - { - response_type: :ephemeral, - text: message, - status: 200 - } - end - - def in_channel_response(message) - { - response_type: :in_channel, - text: message, - status: 200 - } - end - end - end -end -- cgit v1.2.1 From 8ca5fef9d472e11b4a9ff5d5ab47bbba3e7e670d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 15 Dec 2016 23:48:30 +0100 Subject: Use single presenter for everything --- app/models/project_services/chat_slash_commands_service.rb | 8 ++------ app/models/project_services/mattermost_slash_commands_service.rb | 4 ---- app/models/project_services/slack_slash_commands_service.rb | 4 ---- 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/app/models/project_services/chat_slash_commands_service.rb b/app/models/project_services/chat_slash_commands_service.rb index b21a1730285..7ff80447a1c 100644 --- a/app/models/project_services/chat_slash_commands_service.rb +++ b/app/models/project_services/chat_slash_commands_service.rb @@ -33,10 +33,10 @@ class ChatSlashCommandsService < Service user = find_chat_user(params) unless user url = authorize_chat_name_url(params) - return presenter.authorize_chat_name(url) + return Gitlab::ChatCommands::Presenter.authorize_chat_name(url) end - Gitlab::ChatCommands::Command.new(presenter, project, user, params).execute + Gitlab::ChatCommands::Command.new(project, user, params).execute end private @@ -48,8 +48,4 @@ class ChatSlashCommandsService < Service def authorize_chat_name_url(params) ChatNames::AuthorizeUserService.new(self, params).execute end - - def presenter - throw NotImplementedError - end end diff --git a/app/models/project_services/mattermost_slash_commands_service.rb b/app/models/project_services/mattermost_slash_commands_service.rb index 4c1e27cafbb..6aac7c2788b 100644 --- a/app/models/project_services/mattermost_slash_commands_service.rb +++ b/app/models/project_services/mattermost_slash_commands_service.rb @@ -18,8 +18,4 @@ class MattermostSlashCommandsService < ChatService def to_param 'mattermost_slash_commands' end - - def presenter - Gitlab::ChatCommands::Presenters::Mattermost.new - end end diff --git a/app/models/project_services/slack_slash_commands_service.rb b/app/models/project_services/slack_slash_commands_service.rb index 978d48e8c18..197d8eb7bca 100644 --- a/app/models/project_services/slack_slash_commands_service.rb +++ b/app/models/project_services/slack_slash_commands_service.rb @@ -12,8 +12,4 @@ class SlackSlashCommandsService < ChatSlashCommandsService def to_param 'slack_slash_commands' end - - def presenter - Gitlab::ChatCommands::Presenters::Mattermost.new - end end -- cgit v1.2.1 From dc995daf90e67b5531dd7fbb8a958507f46e4eb6 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 15 Dec 2016 23:51:57 +0100 Subject: Use Slack compatible syntax --- lib/gitlab/chat_commands/presenter.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/gitlab/chat_commands/presenter.rb b/lib/gitlab/chat_commands/presenter.rb index 3143dc092a1..14c78ac39aa 100644 --- a/lib/gitlab/chat_commands/presenter.rb +++ b/lib/gitlab/chat_commands/presenter.rb @@ -6,7 +6,7 @@ module Gitlab def authorize_chat_name(url) message = if url - ":wave: Hi there! Before I do anything for you, please [connect your GitLab account](#{url})." + ":wave: Hi there! Before I do anything for you, please <#{url}|connect your GitLab account>." else ":sweat_smile: Couldn't identify you, nor can I autorize you!" end @@ -44,7 +44,7 @@ module Gitlab end def access_denied - ephemeral_response("Whoops! That action is not allowed. This incident will be [reported](https://xkcd.com/838/).") + ephemeral_response("Whoops! That action is not allowed. This incident will be .") end private @@ -65,7 +65,7 @@ module Gitlab def single_resource(resource) return error(resource) if resource.errors.any? || !resource.persisted? - message = "### #{title(resource)}" + message = "#{title(resource)}:" message << "\n\n#{resource.description}" if resource.try(:description) in_channel_response(message) @@ -89,7 +89,7 @@ module Gitlab reference = resource.try(:to_reference) || resource.try(:id) title = resource.try(:title) || resource.try(:name) - "[#{reference} #{title}](#{url(resource)})" + "<#{url(resource)}|#{reference} #{title}>" end def header_with_list(header, items) -- cgit v1.2.1 From cc83aded33471a3e2f943bd9a760f689d30f901e Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 16 Dec 2016 00:00:54 +0100 Subject: Render format dependent links --- .../chat_slash_commands_service.rb | 13 +- .../mattermost_slash_commands_service.rb | 4 + .../slack_slash_commands_service.rb | 4 + lib/gitlab/chat_commands/base_command.rb | 4 + lib/gitlab/chat_commands/command.rb | 6 +- lib/gitlab/chat_commands/presenter.rb | 200 +++++++++++---------- 6 files changed, 132 insertions(+), 99 deletions(-) diff --git a/app/models/project_services/chat_slash_commands_service.rb b/app/models/project_services/chat_slash_commands_service.rb index 7ff80447a1c..f11d257e6c1 100644 --- a/app/models/project_services/chat_slash_commands_service.rb +++ b/app/models/project_services/chat_slash_commands_service.rb @@ -33,10 +33,11 @@ class ChatSlashCommandsService < Service user = find_chat_user(params) unless user url = authorize_chat_name_url(params) - return Gitlab::ChatCommands::Presenter.authorize_chat_name(url) + return presenter.authorize_chat_name(url) end - Gitlab::ChatCommands::Command.new(project, user, params).execute + Gitlab::ChatCommands::Command.new(project, user, + params.merge(presenter_format: presenter_format)).execute end private @@ -48,4 +49,12 @@ class ChatSlashCommandsService < Service def authorize_chat_name_url(params) ChatNames::AuthorizeUserService.new(self, params).execute end + + def presenter + Gitlab::ChatCommands::Presenter.new(presenter_format) + end + + def presenter_format + throw NotImplementedError + end end diff --git a/app/models/project_services/mattermost_slash_commands_service.rb b/app/models/project_services/mattermost_slash_commands_service.rb index 6aac7c2788b..f9d4b29f4ea 100644 --- a/app/models/project_services/mattermost_slash_commands_service.rb +++ b/app/models/project_services/mattermost_slash_commands_service.rb @@ -18,4 +18,8 @@ class MattermostSlashCommandsService < ChatService def to_param 'mattermost_slash_commands' end + + def presenter_format + 'mattermost' + end end diff --git a/app/models/project_services/slack_slash_commands_service.rb b/app/models/project_services/slack_slash_commands_service.rb index 197d8eb7bca..6bf10ff6572 100644 --- a/app/models/project_services/slack_slash_commands_service.rb +++ b/app/models/project_services/slack_slash_commands_service.rb @@ -12,4 +12,8 @@ class SlackSlashCommandsService < ChatSlashCommandsService def to_param 'slack_slash_commands' end + + def presenter_format + 'slack' + end end diff --git a/lib/gitlab/chat_commands/base_command.rb b/lib/gitlab/chat_commands/base_command.rb index 25da8474e95..156bb826f86 100644 --- a/lib/gitlab/chat_commands/base_command.rb +++ b/lib/gitlab/chat_commands/base_command.rb @@ -42,6 +42,10 @@ module Gitlab def find_by_iid(iid) collection.find_by(iid: iid) end + + def presenter + Gitlab::ChatCommands::Presenter.new(params[:presenter_format]) + end end end end diff --git a/lib/gitlab/chat_commands/command.rb b/lib/gitlab/chat_commands/command.rb index b0d3fdbc48a..c5c54cf7cfc 100644 --- a/lib/gitlab/chat_commands/command.rb +++ b/lib/gitlab/chat_commands/command.rb @@ -48,15 +48,15 @@ module Gitlab end def help(messages) - Mattermost::Presenter.help(messages, params[:command]) + presenter.help(messages, params[:command]) end def access_denied - Mattermost::Presenter.access_denied + presenter.access_denied end def present(resource) - Mattermost::Presenter.present(resource) + presenter.present(resource) end end end diff --git a/lib/gitlab/chat_commands/presenter.rb b/lib/gitlab/chat_commands/presenter.rb index 14c78ac39aa..e151513cbd5 100644 --- a/lib/gitlab/chat_commands/presenter.rb +++ b/lib/gitlab/chat_commands/presenter.rb @@ -1,131 +1,143 @@ module Gitlab class ChatCommands class Presenter - class << self - include Gitlab::Routing.url_helpers + include Gitlab::Routing.url_helpers - def authorize_chat_name(url) - message = if url - ":wave: Hi there! Before I do anything for you, please <#{url}|connect your GitLab account>." - else - ":sweat_smile: Couldn't identify you, nor can I autorize you!" - end + attr_reader :format - ephemeral_response(message) - end + def initialize(format) + @format = format + end - def help(commands, trigger) - if commands.none? - ephemeral_response("No commands configured") - else - commands.map! { |command| "#{trigger} #{command}" } - message = header_with_list("Available commands", commands) + def authorize_chat_name(url) + message = if url + ":wave: Hi there! Before I do anything for you, please #{link(url, 'connect your GitLab account')}." + else + ":sweat_smile: Couldn't identify you, nor can I autorize you!" + end - ephemeral_response(message) - end + ephemeral_response(message) + end + + def help(commands, trigger) + if commands.none? + ephemeral_response("No commands configured") + else + commands.map! { |command| "#{trigger} #{command}" } + message = header_with_list("Available commands", commands) + + ephemeral_response(message) end + end - def present(subject) - return not_found unless subject - - if subject.is_a?(Gitlab::ChatCommands::Result) - show_result(subject) - elsif subject.respond_to?(:count) - if subject.many? - multiple_resources(subject) - elsif subject.none? - not_found - else - single_resource(subject) - end + def present(subject) + return not_found unless subject + + if subject.is_a?(Gitlab::ChatCommands::Result) + show_result(subject) + elsif subject.respond_to?(:count) + if subject.many? + multiple_resources(subject) + elsif subject.none? + not_found else single_resource(subject) end + else + single_resource(subject) end + end - def access_denied - ephemeral_response("Whoops! That action is not allowed. This incident will be .") - end + def access_denied + ephemeral_response("Whoops! That action is not allowed. This incident will be #{link('https://xkcd.com/838/', 'reported')}.") + end - private + private - def show_result(result) - case result.type - when :success - in_channel_response(result.message) - else - ephemeral_response(result.message) - end + def show_result(result) + case result.type + when :success + in_channel_response(result.message) + else + ephemeral_response(result.message) end + end - def not_found - ephemeral_response("404 not found! GitLab couldn't find what you were looking for! :boom:") - end + def not_found + ephemeral_response("404 not found! GitLab couldn't find what you were looking for! :boom:") + end - def single_resource(resource) - return error(resource) if resource.errors.any? || !resource.persisted? + def single_resource(resource) + return error(resource) if resource.errors.any? || !resource.persisted? - message = "#{title(resource)}:" - message << "\n\n#{resource.description}" if resource.try(:description) + message = "#{title(resource)}:" + message << "\n\n#{resource.description}" if resource.try(:description) - in_channel_response(message) - end + in_channel_response(message) + end - def multiple_resources(resources) - resources.map! { |resource| title(resource) } + def multiple_resources(resources) + resources.map! { |resource| title(resource) } - message = header_with_list("Multiple results were found:", resources) + message = header_with_list("Multiple results were found:", resources) - ephemeral_response(message) - end + ephemeral_response(message) + end - def error(resource) - message = header_with_list("The action was not successful, because:", resource.errors.messages) + def error(resource) + message = header_with_list("The action was not successful, because:", resource.errors.messages) - ephemeral_response(message) - end + ephemeral_response(message) + end - def title(resource) - reference = resource.try(:to_reference) || resource.try(:id) - title = resource.try(:title) || resource.try(:name) + def title(resource) + reference = resource.try(:to_reference) || resource.try(:id) + title = resource.try(:title) || resource.try(:name) - "<#{url(resource)}|#{reference} #{title}>" - end + link(url(resource), "#{reference} #{title}") + end - def header_with_list(header, items) - message = [header] + def header_with_list(header, items) + message = [header] - items.each do |item| - message << "- #{item}" - end - - message.join("\n") + items.each do |item| + message << "- #{item}" end - def url(resource) - url_for( - [ - resource.project.namespace.becomes(Namespace), - resource.project, - resource - ] - ) - end + message.join("\n") + end - def ephemeral_response(message) - { - response_type: :ephemeral, - text: message, - status: 200 - } - end + def url(resource) + url_for( + [ + resource.project.namespace.becomes(Namespace), + resource.project, + resource + ] + ) + end + + def ephemeral_response(message) + { + response_type: :ephemeral, + text: message, + status: 200 + } + end + + def in_channel_response(message) + { + response_type: :in_channel, + text: message, + status: 200 + } + end - def in_channel_response(message) - { - response_type: :in_channel, - text: message, - status: 200 - } + def link(url, title) + case format + when 'slack' then "<#{url}|#{title}>" + when 'mattermost' then "[#{title}](#{url})" + else then title end end end -- cgit v1.2.1 From ebc3f62be52e386b4550e01ce589744e880ad443 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 16 Dec 2016 13:32:05 +0100 Subject: Fix specs --- .../services/mattermost_slash_commands/_help.html.haml | 5 ++--- lib/gitlab/chat_commands/command.rb | 4 ++-- lib/gitlab/chat_commands/presenter.rb | 11 ++++++++--- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/app/views/projects/services/mattermost_slash_commands/_help.html.haml b/app/views/projects/services/mattermost_slash_commands/_help.html.haml index 2bdd4cd148c..01a77a952d1 100644 --- a/app/views/projects/services/mattermost_slash_commands/_help.html.haml +++ b/app/views/projects/services/mattermost_slash_commands/_help.html.haml @@ -1,5 +1,4 @@ -- pretty_path_with_namespace = "#{@project.namespace ? @project.namespace.name : 'namespace'} / #{@project ? @project.name : 'name'}" -- run_actions_text = "Perform common operations on this project: #{pretty_path_with_namespace}" +- run_actions_text = "Perform common operations on this project: #{@project.name_with_namespace}" .well This service allows GitLab users to perform common operations on this @@ -27,7 +26,7 @@ .form-group = label_tag :display_name, 'Display name', class: 'col-sm-2 col-xs-12 control-label' .col-sm-10.col-xs-12.input-group - = text_field_tag :display_name, "GitLab / #{pretty_path_with_namespace}", class: 'form-control input-sm', readonly: 'readonly' + = text_field_tag :display_name, "GitLab / #{@project.name_with_namespace}", class: 'form-control input-sm', readonly: 'readonly' .input-group-btn = clipboard_button(clipboard_target: '#display_name') diff --git a/lib/gitlab/chat_commands/command.rb b/lib/gitlab/chat_commands/command.rb index c5c54cf7cfc..145086755e4 100644 --- a/lib/gitlab/chat_commands/command.rb +++ b/lib/gitlab/chat_commands/command.rb @@ -22,8 +22,6 @@ module Gitlab end end - private - def match_command match = nil service = available_commands.find do |klass| @@ -33,6 +31,8 @@ module Gitlab [service, match] end + private + def help_messages available_commands.map(&:help_message) end diff --git a/lib/gitlab/chat_commands/presenter.rb b/lib/gitlab/chat_commands/presenter.rb index e151513cbd5..c86efc0c3f8 100644 --- a/lib/gitlab/chat_commands/presenter.rb +++ b/lib/gitlab/chat_commands/presenter.rb @@ -135,9 +135,14 @@ module Gitlab def link(url, title) case format - when 'slack' then "<#{url}|#{title}>" - when 'mattermost' then "[#{title}](#{url})" - else then title + when 'slack' + "<#{url}|#{title}>" + + when 'mattermost' + "[#{title}](#{url})" + + else + title end end end -- cgit v1.2.1 From f9f1a508c6a4bdb2fcee98d18394e28e1cc663c3 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 16 Dec 2016 14:21:06 +0100 Subject: Fix SlackSlashCommands tests --- .../chat_slash_commands_service.rb | 2 +- .../mattermost_slash_commands_service.rb | 2 +- lib/gitlab/chat_commands/presenter.rb | 2 +- spec/lib/gitlab/chat_commands/command_spec.rb | 20 +++- spec/models/project_services/chat_service_spec.rb | 15 --- .../chat_slash_commands_service_spec.rb | 103 +++++++++++++++++++++ .../mattermost_slash_commands_service_spec.rb | 96 +------------------ .../slack_slash_commands_service.rb | 5 + 8 files changed, 127 insertions(+), 118 deletions(-) delete mode 100644 spec/models/project_services/chat_service_spec.rb create mode 100644 spec/models/project_services/chat_slash_commands_service_spec.rb create mode 100644 spec/models/project_services/slack_slash_commands_service.rb diff --git a/app/models/project_services/chat_slash_commands_service.rb b/app/models/project_services/chat_slash_commands_service.rb index f11d257e6c1..e58c96f5094 100644 --- a/app/models/project_services/chat_slash_commands_service.rb +++ b/app/models/project_services/chat_slash_commands_service.rb @@ -5,7 +5,7 @@ class ChatSlashCommandsService < Service prop_accessor :token - has_many :chat_names, foreign_key: :service_id + has_many :chat_names, foreign_key: :service_id, dependent: :destroy def valid_token?(token) self.respond_to?(:token) && diff --git a/app/models/project_services/mattermost_slash_commands_service.rb b/app/models/project_services/mattermost_slash_commands_service.rb index f9d4b29f4ea..72a7b9c8f3a 100644 --- a/app/models/project_services/mattermost_slash_commands_service.rb +++ b/app/models/project_services/mattermost_slash_commands_service.rb @@ -1,4 +1,4 @@ -class MattermostSlashCommandsService < ChatService +class MattermostSlashCommandsService < ChatSlashCommandsService include TriggersHelper prop_accessor :token diff --git a/lib/gitlab/chat_commands/presenter.rb b/lib/gitlab/chat_commands/presenter.rb index c86efc0c3f8..98356ebebb3 100644 --- a/lib/gitlab/chat_commands/presenter.rb +++ b/lib/gitlab/chat_commands/presenter.rb @@ -1,5 +1,5 @@ module Gitlab - class ChatCommands + module ChatCommands class Presenter include Gitlab::Routing.url_helpers diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb index ed8d25a526a..dec98d990b2 100644 --- a/spec/lib/gitlab/chat_commands/command_spec.rb +++ b/spec/lib/gitlab/chat_commands/command_spec.rb @@ -3,9 +3,13 @@ require 'spec_helper' describe Gitlab::ChatCommands::Command, service: true do let(:project) { create(:empty_project) } let(:user) { create(:user) } + let(:format) { nil } describe '#execute' do - subject { described_class.new(project, user, params).execute } + subject do + described_class.new(project, user, + params.merge(presenter_format: format)).execute + end context 'when no command is available' do let(:params) { { text: 'issue show 1' } } @@ -47,8 +51,14 @@ describe Gitlab::ChatCommands::Command, service: true 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+/) + %w(slack mattermost).each do |format| + context "for #{format}" do + let(:format) { format } + + it 'shows a link to the new issue' do + expect(subject[:text]).to match(/\/issues\/\d+/) + end + end end end @@ -64,7 +74,7 @@ describe Gitlab::ChatCommands::Command, service: true do context 'and user can not create deployment' do it 'returns action' do expect(subject[:response_type]).to be(:ephemeral) - expect(subject[:text]).to start_with('Whoops! This action is not allowed') + expect(subject[:text]).to start_with('Whoops! That action is not allowed') end end @@ -74,7 +84,7 @@ describe Gitlab::ChatCommands::Command, service: true do end it 'returns action' do - expect(subject[:text]).to include('Deployment started from staging to production') + expect(subject[:text]).to include('Deployment from staging to production started.') expect(subject[:response_type]).to be(:in_channel) end diff --git a/spec/models/project_services/chat_service_spec.rb b/spec/models/project_services/chat_service_spec.rb deleted file mode 100644 index c6a45a3e1be..00000000000 --- a/spec/models/project_services/chat_service_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require 'spec_helper' - -describe ChatService, models: true do - describe "Associations" do - it { is_expected.to have_many :chat_names } - end - - describe '#valid_token?' do - subject { described_class.new } - - it 'is false as it has no token' do - expect(subject.valid_token?('wer')).to be_falsey - end - end -end diff --git a/spec/models/project_services/chat_slash_commands_service_spec.rb b/spec/models/project_services/chat_slash_commands_service_spec.rb new file mode 100644 index 00000000000..64fdd4d570b --- /dev/null +++ b/spec/models/project_services/chat_slash_commands_service_spec.rb @@ -0,0 +1,103 @@ +require 'spec_helper' + +describe ChatSlashCommandsService, models: true do + describe "Associations" do + it { is_expected.to respond_to :token } + it { is_expected.to have_many :chat_names } + end + + describe '#valid_token?' do + subject { described_class.new } + + context 'when the token is empty' do + it 'is false' do + expect(subject.valid_token?('wer')).to be_falsey + end + end + + context 'when there is a token' do + before do + subject.token = '123' + end + + it 'accepts equal tokens' do + expect(subject.valid_token?('123')).to be_truthy + end + end + end + + describe '#trigger' do + subject { described_class.new } + + before do + allow(subject).to receive(:presenter_format).and_return('unknown') + end + + context 'no token is passed' do + let(:params) { Hash.new } + + it 'returns nil' do + expect(subject.trigger(params)).to be_nil + end + end + + context 'with a token passed' do + let(:project) { create(:empty_project) } + let(:params) { { token: 'token' } } + + before do + allow(subject).to receive(:token).and_return('token') + end + + context 'no user can be found' do + context 'when no url can be generated' do + it 'responds with the authorize url' do + response = subject.trigger(params) + + expect(response[:response_type]).to eq :ephemeral + expect(response[:text]).to start_with ":sweat_smile: Couldn't identify you" + end + end + + context 'when an auth url can be generated' do + let(:params) do + { + team_domain: 'http://domain.tld', + team_id: 'T3423423', + user_id: 'U234234', + user_name: 'mepmep', + token: 'token' + } + end + + let(:service) do + project.create_mattermost_slash_commands_service( + properties: { token: 'token' } + ) + end + + it 'generates the url' do + response = service.trigger(params) + + expect(response[:text]).to start_with(':wave: Hi there!') + end + end + end + + context 'when the user is authenticated' do + let!(:chat_name) { create(:chat_name, service: subject) } + let(:params) { { token: 'token', team_id: chat_name.team_id, user_id: chat_name.chat_id } } + + subject do + described_class.create(project: project, properties: { token: 'token' }) + end + + it 'triggers the command' do + expect_any_instance_of(Gitlab::ChatCommands::Command).to receive(:execute) + + subject.trigger(params) + end + end + end + end +end diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/project_services/mattermost_slash_commands_service_spec.rb index 4a1037e950b..b9deb0201e1 100644 --- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb +++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb @@ -1,99 +1,5 @@ require 'spec_helper' describe MattermostSlashCommandsService, models: true do - describe "Associations" do - it { is_expected.to respond_to :token } - end - - describe '#valid_token?' do - subject { described_class.new } - - context 'when the token is empty' do - it 'is false' do - expect(subject.valid_token?('wer')).to be_falsey - end - end - - context 'when there is a token' do - before do - subject.token = '123' - end - - it 'accepts equal tokens' do - expect(subject.valid_token?('123')).to be_truthy - end - end - end - - describe '#trigger' do - subject { described_class.new } - - context 'no token is passed' do - let(:params) { Hash.new } - - it 'returns nil' do - expect(subject.trigger(params)).to be_nil - end - end - - context 'with a token passed' do - let(:project) { create(:empty_project) } - let(:params) { { token: 'token' } } - - before do - allow(subject).to receive(:token).and_return('token') - end - - context 'no user can be found' do - context 'when no url can be generated' do - it 'responds with the authorize url' do - response = subject.trigger(params) - - expect(response[:response_type]).to eq :ephemeral - expect(response[:text]).to start_with ":sweat_smile: Couldn't identify you" - end - end - - context 'when an auth url can be generated' do - let(:params) do - { - team_domain: 'http://domain.tld', - team_id: 'T3423423', - user_id: 'U234234', - user_name: 'mepmep', - token: 'token' - } - end - - let(:service) do - project.create_mattermost_slash_commands_service( - properties: { token: 'token' } - ) - end - - it 'generates the url' do - response = service.trigger(params) - - expect(response[:text]).to start_with(':wave: Hi there!') - end - end - end - - context 'when the user is authenticated' do - let!(:chat_name) { create(:chat_name, service: service) } - let(:service) do - project.create_mattermost_slash_commands_service( - properties: { token: 'token' } - ) - end - let(:params) { { token: 'token', team_id: chat_name.team_id, user_id: chat_name.chat_id } } - - it 'triggers the command' do - expect_any_instance_of(Gitlab::ChatCommands::Command).to receive(:execute) - - service.trigger(params) - end - end - end - end + it { is_expected.to respond_to :presenter_format } end diff --git a/spec/models/project_services/slack_slash_commands_service.rb b/spec/models/project_services/slack_slash_commands_service.rb new file mode 100644 index 00000000000..5ef97b9a2ed --- /dev/null +++ b/spec/models/project_services/slack_slash_commands_service.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe SlackSlashCommandsService, models: true do + it { is_expected.to respond_to :presenter_format } +end -- cgit v1.2.1 From 0f2776287a7d9b0fde9ff54ef8d9f74e2f844a09 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 16 Dec 2016 15:08:10 +0100 Subject: Use Slack::Notifier::LinkFormatter to convert markdown links to slack compat --- .../chat_slash_commands_service.rb | 8 +- .../mattermost_slash_commands_service.rb | 4 - .../slack_slash_commands_service.rb | 11 +- lib/gitlab/chat_commands/base_command.rb | 2 +- lib/gitlab/chat_commands/presenter.rb | 25 +- spec/lib/gitlab/chat_commands/command_spec.rb | 14 +- .../chat_slash_commands_service_spec.rb | 103 ------- .../mattermost_notification_service_spec.rb | 2 +- .../mattermost_slash_commands_service_spec.rb | 2 +- .../slack_notification_service_spec.rb | 2 +- .../slack_slash_commands_service.rb | 35 ++- .../support/chat_slash_commands_shared_examples.rb | 97 ++++++ ...ack_mattermost_notifications_shared_examples.rb | 328 +++++++++++++++++++++ spec/support/slack_mattermost_shared_examples.rb | 328 --------------------- 14 files changed, 480 insertions(+), 481 deletions(-) delete mode 100644 spec/models/project_services/chat_slash_commands_service_spec.rb create mode 100644 spec/support/chat_slash_commands_shared_examples.rb create mode 100644 spec/support/slack_mattermost_notifications_shared_examples.rb delete mode 100644 spec/support/slack_mattermost_shared_examples.rb diff --git a/app/models/project_services/chat_slash_commands_service.rb b/app/models/project_services/chat_slash_commands_service.rb index e58c96f5094..12261e9821e 100644 --- a/app/models/project_services/chat_slash_commands_service.rb +++ b/app/models/project_services/chat_slash_commands_service.rb @@ -37,7 +37,7 @@ class ChatSlashCommandsService < Service end Gitlab::ChatCommands::Command.new(project, user, - params.merge(presenter_format: presenter_format)).execute + params).execute end private @@ -51,10 +51,6 @@ class ChatSlashCommandsService < Service end def presenter - Gitlab::ChatCommands::Presenter.new(presenter_format) - end - - def presenter_format - throw NotImplementedError + Gitlab::ChatCommands::Presenter.new end end diff --git a/app/models/project_services/mattermost_slash_commands_service.rb b/app/models/project_services/mattermost_slash_commands_service.rb index 72a7b9c8f3a..10740275669 100644 --- a/app/models/project_services/mattermost_slash_commands_service.rb +++ b/app/models/project_services/mattermost_slash_commands_service.rb @@ -18,8 +18,4 @@ class MattermostSlashCommandsService < ChatSlashCommandsService def to_param 'mattermost_slash_commands' end - - def presenter_format - 'mattermost' - end end diff --git a/app/models/project_services/slack_slash_commands_service.rb b/app/models/project_services/slack_slash_commands_service.rb index 6bf10ff6572..8413c657099 100644 --- a/app/models/project_services/slack_slash_commands_service.rb +++ b/app/models/project_services/slack_slash_commands_service.rb @@ -13,7 +13,14 @@ class SlackSlashCommandsService < ChatSlashCommandsService 'slack_slash_commands' end - def presenter_format - 'slack' + def trigger(params) + result = super + + # Format messages to be Slack-compatible + if result && result[:text] + result[:text] = Slack::Notifier::LinkFormatter.format(result[:text]) + end + + result end end diff --git a/lib/gitlab/chat_commands/base_command.rb b/lib/gitlab/chat_commands/base_command.rb index 156bb826f86..4fe53ce93a9 100644 --- a/lib/gitlab/chat_commands/base_command.rb +++ b/lib/gitlab/chat_commands/base_command.rb @@ -44,7 +44,7 @@ module Gitlab end def presenter - Gitlab::ChatCommands::Presenter.new(params[:presenter_format]) + Gitlab::ChatCommands::Presenter.new end end end diff --git a/lib/gitlab/chat_commands/presenter.rb b/lib/gitlab/chat_commands/presenter.rb index 98356ebebb3..e94d0ce2470 100644 --- a/lib/gitlab/chat_commands/presenter.rb +++ b/lib/gitlab/chat_commands/presenter.rb @@ -3,15 +3,9 @@ module Gitlab class Presenter include Gitlab::Routing.url_helpers - attr_reader :format - - def initialize(format) - @format = format - end - def authorize_chat_name(url) message = if url - ":wave: Hi there! Before I do anything for you, please #{link(url, 'connect your GitLab account')}." + ":wave: Hi there! Before I do anything for you, please [connect your GitLab account](#{url})." else ":sweat_smile: Couldn't identify you, nor can I autorize you!" end @@ -49,7 +43,7 @@ module Gitlab end def access_denied - ephemeral_response("Whoops! That action is not allowed. This incident will be #{link('https://xkcd.com/838/', 'reported')}.") + ephemeral_response("Whoops! That action is not allowed. This incident will be [reported](https://xkcd.com/838/).") end private @@ -94,7 +88,7 @@ module Gitlab reference = resource.try(:to_reference) || resource.try(:id) title = resource.try(:title) || resource.try(:name) - link(url(resource), "#{reference} #{title}") + "[#{reference} #{title}](#{url(resource)})" end def header_with_list(header, items) @@ -132,19 +126,6 @@ module Gitlab status: 200 } end - - def link(url, title) - case format - when 'slack' - "<#{url}|#{title}>" - - when 'mattermost' - "[#{title}](#{url})" - - else - title - end - end end end end diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb index dec98d990b2..a0ec8884635 100644 --- a/spec/lib/gitlab/chat_commands/command_spec.rb +++ b/spec/lib/gitlab/chat_commands/command_spec.rb @@ -3,12 +3,10 @@ require 'spec_helper' describe Gitlab::ChatCommands::Command, service: true do let(:project) { create(:empty_project) } let(:user) { create(:user) } - let(:format) { nil } describe '#execute' do subject do - described_class.new(project, user, - params.merge(presenter_format: format)).execute + described_class.new(project, user, params).execute end context 'when no command is available' do @@ -51,14 +49,8 @@ describe Gitlab::ChatCommands::Command, service: true do expect(subject[:text]).to match("my new issue") end - %w(slack mattermost).each do |format| - context "for #{format}" do - let(:format) { format } - - it 'shows a link to the new issue' do - expect(subject[:text]).to match(/\/issues\/\d+/) - end - end + it 'shows a link to the new issue' do + expect(subject[:text]).to match(/\/issues\/\d+/) end end diff --git a/spec/models/project_services/chat_slash_commands_service_spec.rb b/spec/models/project_services/chat_slash_commands_service_spec.rb deleted file mode 100644 index 64fdd4d570b..00000000000 --- a/spec/models/project_services/chat_slash_commands_service_spec.rb +++ /dev/null @@ -1,103 +0,0 @@ -require 'spec_helper' - -describe ChatSlashCommandsService, models: true do - describe "Associations" do - it { is_expected.to respond_to :token } - it { is_expected.to have_many :chat_names } - end - - describe '#valid_token?' do - subject { described_class.new } - - context 'when the token is empty' do - it 'is false' do - expect(subject.valid_token?('wer')).to be_falsey - end - end - - context 'when there is a token' do - before do - subject.token = '123' - end - - it 'accepts equal tokens' do - expect(subject.valid_token?('123')).to be_truthy - end - end - end - - describe '#trigger' do - subject { described_class.new } - - before do - allow(subject).to receive(:presenter_format).and_return('unknown') - end - - context 'no token is passed' do - let(:params) { Hash.new } - - it 'returns nil' do - expect(subject.trigger(params)).to be_nil - end - end - - context 'with a token passed' do - let(:project) { create(:empty_project) } - let(:params) { { token: 'token' } } - - before do - allow(subject).to receive(:token).and_return('token') - end - - context 'no user can be found' do - context 'when no url can be generated' do - it 'responds with the authorize url' do - response = subject.trigger(params) - - expect(response[:response_type]).to eq :ephemeral - expect(response[:text]).to start_with ":sweat_smile: Couldn't identify you" - end - end - - context 'when an auth url can be generated' do - let(:params) do - { - team_domain: 'http://domain.tld', - team_id: 'T3423423', - user_id: 'U234234', - user_name: 'mepmep', - token: 'token' - } - end - - let(:service) do - project.create_mattermost_slash_commands_service( - properties: { token: 'token' } - ) - end - - it 'generates the url' do - response = service.trigger(params) - - expect(response[:text]).to start_with(':wave: Hi there!') - end - end - end - - context 'when the user is authenticated' do - let!(:chat_name) { create(:chat_name, service: subject) } - let(:params) { { token: 'token', team_id: chat_name.team_id, user_id: chat_name.chat_id } } - - subject do - described_class.create(project: project, properties: { token: 'token' }) - end - - it 'triggers the command' do - expect_any_instance_of(Gitlab::ChatCommands::Command).to receive(:execute) - - subject.trigger(params) - end - end - end - end -end diff --git a/spec/models/project_services/mattermost_notification_service_spec.rb b/spec/models/project_services/mattermost_notification_service_spec.rb index c01e64b4c8e..7832d6f50cf 100644 --- a/spec/models/project_services/mattermost_notification_service_spec.rb +++ b/spec/models/project_services/mattermost_notification_service_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' describe MattermostNotificationService, models: true do - it_behaves_like "slack or mattermost" + it_behaves_like "slack or mattermost notifications" end diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/project_services/mattermost_slash_commands_service_spec.rb index b9deb0201e1..5c34cb6b4cf 100644 --- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb +++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' describe MattermostSlashCommandsService, models: true do - it { is_expected.to respond_to :presenter_format } + it_behaves_like "chat slash commands" end diff --git a/spec/models/project_services/slack_notification_service_spec.rb b/spec/models/project_services/slack_notification_service_spec.rb index 59ddddf7454..110b5bf2115 100644 --- a/spec/models/project_services/slack_notification_service_spec.rb +++ b/spec/models/project_services/slack_notification_service_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' describe SlackNotificationService, models: true do - it_behaves_like "slack or mattermost" + it_behaves_like "slack or mattermost notifications" end diff --git a/spec/models/project_services/slack_slash_commands_service.rb b/spec/models/project_services/slack_slash_commands_service.rb index 5ef97b9a2ed..c3fa80caebe 100644 --- a/spec/models/project_services/slack_slash_commands_service.rb +++ b/spec/models/project_services/slack_slash_commands_service.rb @@ -1,5 +1,38 @@ require 'spec_helper' describe SlackSlashCommandsService, models: true do - it { is_expected.to respond_to :presenter_format } + it_behaves_like "chat slash commands" + + describe '#trigger' do + context 'when an auth url is generated' do + let(:project) { create(:empty_project) } + let(:params) do + { + team_domain: 'http://domain.tld', + team_id: 'T3423423', + user_id: 'U234234', + user_name: 'mepmep', + token: 'token' + } + end + let(:service) do + project.create_slack_slash_commands_service( + properties: { token: 'token' } + ) + end + let(:authorize_url) do + 'http://authorize.example.com/' + end + + before do + allow(service).to receive(:authorize_chat_name_url).and_return(authorize_url) + end + + it 'uses slack compatible links' do + response = service.trigger(params) + + expect(response[:text]).to include("<#{authorize_url}|connect your GitLab account>") + end + end + end end diff --git a/spec/support/chat_slash_commands_shared_examples.rb b/spec/support/chat_slash_commands_shared_examples.rb new file mode 100644 index 00000000000..96130b45235 --- /dev/null +++ b/spec/support/chat_slash_commands_shared_examples.rb @@ -0,0 +1,97 @@ +RSpec.shared_examples 'chat slash commands' do + describe "Associations" do + it { is_expected.to respond_to :token } + it { is_expected.to have_many :chat_names } + end + + describe '#valid_token?' do + subject { described_class.new } + + context 'when the token is empty' do + it 'is false' do + expect(subject.valid_token?('wer')).to be_falsey + end + end + + context 'when there is a token' do + before do + subject.token = '123' + end + + it 'accepts equal tokens' do + expect(subject.valid_token?('123')).to be_truthy + end + end + end + + describe '#trigger' do + subject { described_class.new } + + context 'no token is passed' do + let(:params) { Hash.new } + + it 'returns nil' do + expect(subject.trigger(params)).to be_nil + end + end + + context 'with a token passed' do + let(:project) { create(:empty_project) } + let(:params) { { token: 'token' } } + + before do + allow(subject).to receive(:token).and_return('token') + end + + context 'no user can be found' do + context 'when no url can be generated' do + it 'responds with the authorize url' do + response = subject.trigger(params) + + expect(response[:response_type]).to eq :ephemeral + expect(response[:text]).to start_with ":sweat_smile: Couldn't identify you" + end + end + + context 'when an auth url can be generated' do + let(:params) do + { + team_domain: 'http://domain.tld', + team_id: 'T3423423', + user_id: 'U234234', + user_name: 'mepmep', + token: 'token' + } + end + + let(:service) do + project.create_mattermost_slash_commands_service( + properties: { token: 'token' } + ) + end + + it 'generates the url' do + response = service.trigger(params) + + expect(response[:text]).to start_with(':wave: Hi there!') + end + end + end + + context 'when the user is authenticated' do + let!(:chat_name) { create(:chat_name, service: subject) } + let(:params) { { token: 'token', team_id: chat_name.team_id, user_id: chat_name.chat_id } } + + subject do + described_class.create(project: project, properties: { token: 'token' }) + end + + it 'triggers the command' do + expect_any_instance_of(Gitlab::ChatCommands::Command).to receive(:execute) + + subject.trigger(params) + end + end + end + end +end diff --git a/spec/support/slack_mattermost_notifications_shared_examples.rb b/spec/support/slack_mattermost_notifications_shared_examples.rb new file mode 100644 index 00000000000..8582aea5fe5 --- /dev/null +++ b/spec/support/slack_mattermost_notifications_shared_examples.rb @@ -0,0 +1,328 @@ +Dir[Rails.root.join("app/models/project_services/chat_message/*.rb")].each { |f| require f } + +RSpec.shared_examples 'slack or mattermost notifications' do + let(:chat_service) { described_class.new } + let(:webhook_url) { 'https://example.gitlab.com/' } + + describe "Associations" do + it { is_expected.to belong_to :project } + it { is_expected.to have_one :service_hook } + end + + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:webhook) } + it_behaves_like 'issue tracker service URL attribute', :webhook + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:webhook) } + end + end + + describe "#execute" do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:username) { 'slack_username' } + let(:channel) { 'slack_channel' } + + let(:push_sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end + + before do + allow(chat_service).to receive_messages( + project: project, + project_id: project.id, + service_hook: true, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url) + + opts = { + title: 'Awesome issue', + description: 'please fix' + } + + issue_service = Issues::CreateService.new(project, user, opts) + @issue = issue_service.execute + @issues_sample_data = issue_service.hook_data(@issue, 'open') + + opts = { + title: 'Awesome merge_request', + description: 'please fix', + source_branch: 'feature', + target_branch: 'master' + } + merge_service = MergeRequests::CreateService.new(project, + user, opts) + @merge_request = merge_service.execute + @merge_sample_data = merge_service.hook_data(@merge_request, + 'open') + + opts = { + title: "Awesome wiki_page", + content: "Some text describing some thing or another", + format: "md", + message: "user created page: Awesome wiki_page" + } + + wiki_page_service = WikiPages::CreateService.new(project, user, opts) + @wiki_page = wiki_page_service.execute + @wiki_page_sample_data = wiki_page_service.hook_data(@wiki_page, 'create') + end + + it "calls Slack/Mattermost API for push events" do + chat_service.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + + it "calls Slack/Mattermost API for issue events" do + chat_service.execute(@issues_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + + it "calls Slack/Mattermost API for merge requests events" do + chat_service.execute(@merge_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + + it "calls Slack/Mattermost API for wiki page events" do + chat_service.execute(@wiki_page_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + + it 'uses the username as an option for slack when configured' do + allow(chat_service).to receive(:username).and_return(username) + + expect(Slack::Notifier).to receive(:new). + with(webhook_url, username: username, channel: chat_service.default_channel). + and_return( + double(:slack_service).as_null_object + ) + + chat_service.execute(push_sample_data) + end + + it 'uses the channel as an option when it is configured' do + allow(chat_service).to receive(:channel).and_return(channel) + expect(Slack::Notifier).to receive(:new). + with(webhook_url, channel: channel). + and_return( + double(:slack_service).as_null_object + ) + chat_service.execute(push_sample_data) + end + + context "event channels" do + it "uses the right channel for push event" do + chat_service.update_attributes(push_channel: "random") + + expect(Slack::Notifier).to receive(:new). + with(webhook_url, channel: "random"). + and_return( + double(:slack_service).as_null_object + ) + + chat_service.execute(push_sample_data) + end + + it "uses the right channel for merge request event" do + chat_service.update_attributes(merge_request_channel: "random") + + expect(Slack::Notifier).to receive(:new). + with(webhook_url, channel: "random"). + and_return( + double(:slack_service).as_null_object + ) + + chat_service.execute(@merge_sample_data) + end + + it "uses the right channel for issue event" do + chat_service.update_attributes(issue_channel: "random") + + expect(Slack::Notifier).to receive(:new). + with(webhook_url, channel: "random"). + and_return( + double(:slack_service).as_null_object + ) + + chat_service.execute(@issues_sample_data) + end + + it "uses the right channel for wiki event" do + chat_service.update_attributes(wiki_page_channel: "random") + + expect(Slack::Notifier).to receive(:new). + with(webhook_url, channel: "random"). + and_return( + double(:slack_service).as_null_object + ) + + chat_service.execute(@wiki_page_sample_data) + end + + context "note event" do + let(:issue_note) do + create(:note_on_issue, project: project, note: "issue note") + end + + it "uses the right channel" do + chat_service.update_attributes(note_channel: "random") + + note_data = Gitlab::DataBuilder::Note.build(issue_note, user) + + expect(Slack::Notifier).to receive(:new). + with(webhook_url, channel: "random"). + and_return( + double(:slack_service).as_null_object + ) + + chat_service.execute(note_data) + end + end + end + end + + describe "Note events" do + let(:user) { create(:user) } + let(:project) { create(:project, creator_id: user.id) } + + before do + allow(chat_service).to receive_messages( + project: project, + project_id: project.id, + service_hook: true, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url) + end + + context 'when commit comment event executed' do + let(:commit_note) do + create(:note_on_commit, author: user, + project: project, + commit_id: project.repository.commit.id, + note: 'a comment on a commit') + end + + it "calls Slack/Mattermost API for commit comment events" do + data = Gitlab::DataBuilder::Note.build(commit_note, user) + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when merge request comment event executed' do + let(:merge_request_note) do + create(:note_on_merge_request, project: project, + note: "merge request note") + end + + it "calls Slack API for merge request comment events" do + data = Gitlab::DataBuilder::Note.build(merge_request_note, user) + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when issue comment event executed' do + let(:issue_note) do + create(:note_on_issue, project: project, note: "issue note") + end + + it "calls Slack API for issue comment events" do + data = Gitlab::DataBuilder::Note.build(issue_note, user) + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when snippet comment event executed' do + let(:snippet_note) do + create(:note_on_project_snippet, project: project, + note: "snippet note") + end + + it "calls Slack API for snippet comment events" do + data = Gitlab::DataBuilder::Note.build(snippet_note, user) + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + end + + describe 'Pipeline events' do + let(:user) { create(:user) } + let(:project) { create(:project) } + + let(:pipeline) do + create(:ci_pipeline, + project: project, status: status, + sha: project.commit.sha, ref: project.default_branch) + end + + before do + allow(chat_service).to receive_messages( + project: project, + service_hook: true, + webhook: webhook_url + ) + end + + shared_examples 'call Slack/Mattermost API' do + before do + WebMock.stub_request(:post, webhook_url) + end + + it 'calls Slack/Mattermost API for pipeline events' do + data = Gitlab::DataBuilder::Pipeline.build(pipeline) + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'with failed pipeline' do + let(:status) { 'failed' } + + it_behaves_like 'call Slack/Mattermost API' + end + + context 'with succeeded pipeline' do + let(:status) { 'success' } + + context 'with default to notify_only_broken_pipelines' do + it 'does not call Slack/Mattermost API for pipeline events' do + data = Gitlab::DataBuilder::Pipeline.build(pipeline) + result = chat_service.execute(data) + + expect(result).to be_falsy + end + end + + context 'with setting notify_only_broken_pipelines to false' do + before do + chat_service.notify_only_broken_pipelines = false + end + + it_behaves_like 'call Slack/Mattermost API' + end + end + end +end diff --git a/spec/support/slack_mattermost_shared_examples.rb b/spec/support/slack_mattermost_shared_examples.rb deleted file mode 100644 index 56d4965f74d..00000000000 --- a/spec/support/slack_mattermost_shared_examples.rb +++ /dev/null @@ -1,328 +0,0 @@ -Dir[Rails.root.join("app/models/project_services/chat_message/*.rb")].each { |f| require f } - -RSpec.shared_examples 'slack or mattermost' do - let(:chat_service) { described_class.new } - let(:webhook_url) { 'https://example.gitlab.com/' } - - describe "Associations" do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end - - describe 'Validations' do - context 'when service is active' do - before { subject.active = true } - - it { is_expected.to validate_presence_of(:webhook) } - it_behaves_like 'issue tracker service URL attribute', :webhook - end - - context 'when service is inactive' do - before { subject.active = false } - - it { is_expected.not_to validate_presence_of(:webhook) } - end - end - - describe "#execute" do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:username) { 'slack_username' } - let(:channel) { 'slack_channel' } - - let(:push_sample_data) do - Gitlab::DataBuilder::Push.build_sample(project, user) - end - - before do - allow(chat_service).to receive_messages( - project: project, - project_id: project.id, - service_hook: true, - webhook: webhook_url - ) - - WebMock.stub_request(:post, webhook_url) - - opts = { - title: 'Awesome issue', - description: 'please fix' - } - - issue_service = Issues::CreateService.new(project, user, opts) - @issue = issue_service.execute - @issues_sample_data = issue_service.hook_data(@issue, 'open') - - opts = { - title: 'Awesome merge_request', - description: 'please fix', - source_branch: 'feature', - target_branch: 'master' - } - merge_service = MergeRequests::CreateService.new(project, - user, opts) - @merge_request = merge_service.execute - @merge_sample_data = merge_service.hook_data(@merge_request, - 'open') - - opts = { - title: "Awesome wiki_page", - content: "Some text describing some thing or another", - format: "md", - message: "user created page: Awesome wiki_page" - } - - wiki_page_service = WikiPages::CreateService.new(project, user, opts) - @wiki_page = wiki_page_service.execute - @wiki_page_sample_data = wiki_page_service.hook_data(@wiki_page, 'create') - end - - it "calls Slack/Mattermost API for push events" do - chat_service.execute(push_sample_data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - - it "calls Slack/Mattermost API for issue events" do - chat_service.execute(@issues_sample_data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - - it "calls Slack/Mattermost API for merge requests events" do - chat_service.execute(@merge_sample_data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - - it "calls Slack/Mattermost API for wiki page events" do - chat_service.execute(@wiki_page_sample_data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - - it 'uses the username as an option for slack when configured' do - allow(chat_service).to receive(:username).and_return(username) - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, username: username, channel: chat_service.default_channel). - and_return( - double(:slack_service).as_null_object - ) - - chat_service.execute(push_sample_data) - end - - it 'uses the channel as an option when it is configured' do - allow(chat_service).to receive(:channel).and_return(channel) - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: channel). - and_return( - double(:slack_service).as_null_object - ) - chat_service.execute(push_sample_data) - end - - context "event channels" do - it "uses the right channel for push event" do - chat_service.update_attributes(push_channel: "random") - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: "random"). - and_return( - double(:slack_service).as_null_object - ) - - chat_service.execute(push_sample_data) - end - - it "uses the right channel for merge request event" do - chat_service.update_attributes(merge_request_channel: "random") - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: "random"). - and_return( - double(:slack_service).as_null_object - ) - - chat_service.execute(@merge_sample_data) - end - - it "uses the right channel for issue event" do - chat_service.update_attributes(issue_channel: "random") - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: "random"). - and_return( - double(:slack_service).as_null_object - ) - - chat_service.execute(@issues_sample_data) - end - - it "uses the right channel for wiki event" do - chat_service.update_attributes(wiki_page_channel: "random") - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: "random"). - and_return( - double(:slack_service).as_null_object - ) - - chat_service.execute(@wiki_page_sample_data) - end - - context "note event" do - let(:issue_note) do - create(:note_on_issue, project: project, note: "issue note") - end - - it "uses the right channel" do - chat_service.update_attributes(note_channel: "random") - - note_data = Gitlab::DataBuilder::Note.build(issue_note, user) - - expect(Slack::Notifier).to receive(:new). - with(webhook_url, channel: "random"). - and_return( - double(:slack_service).as_null_object - ) - - chat_service.execute(note_data) - end - end - end - end - - describe "Note events" do - let(:user) { create(:user) } - let(:project) { create(:project, creator_id: user.id) } - - before do - allow(chat_service).to receive_messages( - project: project, - project_id: project.id, - service_hook: true, - webhook: webhook_url - ) - - WebMock.stub_request(:post, webhook_url) - end - - context 'when commit comment event executed' do - let(:commit_note) do - create(:note_on_commit, author: user, - project: project, - commit_id: project.repository.commit.id, - note: 'a comment on a commit') - end - - it "calls Slack/Mattermost API for commit comment events" do - data = Gitlab::DataBuilder::Note.build(commit_note, user) - chat_service.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end - - context 'when merge request comment event executed' do - let(:merge_request_note) do - create(:note_on_merge_request, project: project, - note: "merge request note") - end - - it "calls Slack API for merge request comment events" do - data = Gitlab::DataBuilder::Note.build(merge_request_note, user) - chat_service.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end - - context 'when issue comment event executed' do - let(:issue_note) do - create(:note_on_issue, project: project, note: "issue note") - end - - it "calls Slack API for issue comment events" do - data = Gitlab::DataBuilder::Note.build(issue_note, user) - chat_service.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end - - context 'when snippet comment event executed' do - let(:snippet_note) do - create(:note_on_project_snippet, project: project, - note: "snippet note") - end - - it "calls Slack API for snippet comment events" do - data = Gitlab::DataBuilder::Note.build(snippet_note, user) - chat_service.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end - end - - describe 'Pipeline events' do - let(:user) { create(:user) } - let(:project) { create(:project) } - - let(:pipeline) do - create(:ci_pipeline, - project: project, status: status, - sha: project.commit.sha, ref: project.default_branch) - end - - before do - allow(chat_service).to receive_messages( - project: project, - service_hook: true, - webhook: webhook_url - ) - end - - shared_examples 'call Slack/Mattermost API' do - before do - WebMock.stub_request(:post, webhook_url) - end - - it 'calls Slack/Mattermost API for pipeline events' do - data = Gitlab::DataBuilder::Pipeline.build(pipeline) - chat_service.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once - end - end - - context 'with failed pipeline' do - let(:status) { 'failed' } - - it_behaves_like 'call Slack/Mattermost API' - end - - context 'with succeeded pipeline' do - let(:status) { 'success' } - - context 'with default to notify_only_broken_pipelines' do - it 'does not call Slack/Mattermost API for pipeline events' do - data = Gitlab::DataBuilder::Pipeline.build(pipeline) - result = chat_service.execute(data) - - expect(result).to be_falsy - end - end - - context 'with setting notify_only_broken_pipelines to false' do - before do - chat_service.notify_only_broken_pipelines = false - end - - it_behaves_like 'call Slack/Mattermost API' - end - end - end -end -- cgit v1.2.1 From 3f60a276fc36fc7d1c5323c38b33fdbc774cfbbf Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Tue, 13 Dec 2016 16:53:01 +0000 Subject: Added slack slash commands frontend help well Added tests --- app/assets/stylesheets/framework/forms.scss | 4 + .../services/slack_slash_commands/_help.html.haml | 93 ++++++++++++++++++++++ .../projects/services/slack_slash_command_spec.rb | 48 +++++++++++ 3 files changed, 145 insertions(+) create mode 100644 app/views/projects/services/slack_slash_commands/_help.html.haml create mode 100644 spec/features/projects/services/slack_slash_command_spec.rb diff --git a/app/assets/stylesheets/framework/forms.scss b/app/assets/stylesheets/framework/forms.scss index 940807fc399..8726a69867b 100644 --- a/app/assets/stylesheets/framework/forms.scss +++ b/app/assets/stylesheets/framework/forms.scss @@ -96,6 +96,10 @@ label { code { line-height: 1.8; } + + img { + margin-right: $gl-padding; + } } @media(max-width: $screen-xs-max) { diff --git a/app/views/projects/services/slack_slash_commands/_help.html.haml b/app/views/projects/services/slack_slash_commands/_help.html.haml new file mode 100644 index 00000000000..c45052e3954 --- /dev/null +++ b/app/views/projects/services/slack_slash_commands/_help.html.haml @@ -0,0 +1,93 @@ +- run_actions_text = "Perform common operations on this project: #{@project.name_with_namespace}" + +.well + This service allows GitLab users to perform common operations on this + project by entering slash commands in Slack. + %br + See list of available commands in Slack after setting up this service, + by entering + %code /<command> help + %br + %br + To setup this service: + %ul.list-unstyled + %li + 1. + = link_to 'Add a slash command', 'https://my.slack.com/services/new/slash-commands' + in your Slack team with these options: + + %hr + + .help-form + .form-group + = label_tag nil, 'Command', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.text-block + %p Fill in the word that works best for your team. + %p + Suggestions: + %code= 'gitlab' + %code= @project.path # Path contains no spaces, but dashes + %code= @project.path_with_namespace + + .form-group + = label_tag :url, 'URL', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.input-group + = text_field_tag :url, service_trigger_url(subject), class: 'form-control input-sm', readonly: 'readonly' + .input-group-btn + = clipboard_button(clipboard_target: '#url') + + .form-group + = label_tag nil, 'Method', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.text-block POST + + .form-group + = label_tag :customize_name, 'Customize name', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.input-group + = text_field_tag :customize_name, 'GitLab', class: 'form-control input-sm', readonly: 'readonly' + .input-group-btn + = clipboard_button(clipboard_target: '#customize_name') + + .form-group + = label_tag nil, 'Customize icon', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.text-block + = image_tag(asset_url('gitlab_logo.png'), width: 36, height: 36) + = link_to('Download image', asset_url('gitlab_logo.png'), class: 'btn btn-sm', target: '_blank') + + .form-group + = label_tag nil, 'Autocomplete', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.text-block Show this command in the autocomplete list + + .form-group + = label_tag :autocomplete_description, 'Autocomplete description', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.input-group + = text_field_tag :autocomplete_description, run_actions_text, class: 'form-control input-sm', readonly: 'readonly' + .input-group-btn + = clipboard_button(clipboard_target: '#autocomplete_description') + + .form-group + = label_tag :autocomplete_usage_hint, 'Autocomplete usage hint', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.input-group + = text_field_tag :autocomplete_usage_hint, '[help]', class: 'form-control input-sm', readonly: 'readonly' + .input-group-btn + = clipboard_button(clipboard_target: '#autocomplete_usage_hint') + + .form-group + = label_tag :descriptive_label, 'Descriptive label', class: 'col-sm-2 col-xs-12 control-label' + .col-sm-10.col-xs-12.input-group + = text_field_tag :descriptive_label, 'Perform common operations on GitLab project', class: 'form-control input-sm', readonly: 'readonly' + .input-group-btn + = clipboard_button(clipboard_target: '#descriptive_label') + + %hr + + %ul.list-unstyled + %li + 2. Paste the + %strong Token + into the field below + %li + 3. Select the + %strong Active + checkbox, press + %strong Save changes + and start using GitLab inside Slack! diff --git a/spec/features/projects/services/slack_slash_command_spec.rb b/spec/features/projects/services/slack_slash_command_spec.rb new file mode 100644 index 00000000000..dee43d69895 --- /dev/null +++ b/spec/features/projects/services/slack_slash_command_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +feature 'Setup Slack slash commands', feature: true do + include WaitForAjax + + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:service) { project.create_slack_slash_commands_service } + + before do + project.team << [user, :master] + login_as(user) + end + + describe 'user visits the slack slash command config page', js: true do + it 'shows a help message' do + visit edit_namespace_project_service_path(project.namespace, project, service) + + wait_for_ajax + + expect(page).to have_content('This service allows GitLab users to perform common') + end + end + + describe 'saving a token' do + let(:token) { ('a'..'z').to_a.join } + + it 'shows the token after saving' do + visit edit_namespace_project_service_path(project.namespace, project, service) + + fill_in 'service_token', with: token + click_on 'Save' + + value = find_field('service_token').value + + expect(value).to eq(token) + end + end + + describe 'the trigger url' do + it 'shows the correct url' do + visit edit_namespace_project_service_path(project.namespace, project, service) + + value = find_field('url').value + expect(value).to match("api/v3/projects/#{project.id}/services/slack_slash_commands/trigger") + end + end +end -- cgit v1.2.1 From 676b79b94d33e007b34d70c00900a52983ce1106 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 18 Dec 2016 23:30:37 +0100 Subject: Fix Rubocop --- lib/gitlab/chat_commands/presenter.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/chat_commands/presenter.rb b/lib/gitlab/chat_commands/presenter.rb index e94d0ce2470..b4c4dc252ca 100644 --- a/lib/gitlab/chat_commands/presenter.rb +++ b/lib/gitlab/chat_commands/presenter.rb @@ -43,7 +43,7 @@ module Gitlab end def access_denied - ephemeral_response("Whoops! That action is not allowed. This incident will be [reported](https://xkcd.com/838/).") + ephemeral_response("Whoops! That action is not allowed. This incident will be [reported](https://xkcd.com/838/).") end private @@ -88,7 +88,7 @@ module Gitlab reference = resource.try(:to_reference) || resource.try(:id) title = resource.try(:title) || resource.try(:name) - "[#{reference} #{title}](#{url(resource)})" + "[#{reference} #{title}](#{url(resource)})" end def header_with_list(header, items) -- cgit v1.2.1 From 9e3153dbf556b9b9397806bedcf0a195e8ee3fa4 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 18 Dec 2016 23:30:48 +0100 Subject: Remove not related spec changes --- .../chat_message/build_message_spec.rb | 8 +++---- .../chat_message/issue_message_spec.rb | 10 ++++----- .../chat_message/merge_message_spec.rb | 12 +++++----- .../chat_message/note_message_spec.rb | 22 +++++++++--------- .../chat_message/push_message_spec.rb | 26 +++++++++++----------- .../chat_message/wiki_page_message_spec.rb | 8 +++---- 6 files changed, 43 insertions(+), 43 deletions(-) diff --git a/spec/models/project_services/chat_message/build_message_spec.rb b/spec/models/project_services/chat_message/build_message_spec.rb index 50ad5013df9..b71d153f814 100644 --- a/spec/models/project_services/chat_message/build_message_spec.rb +++ b/spec/models/project_services/chat_message/build_message_spec.rb @@ -10,7 +10,7 @@ describe ChatMessage::BuildMessage do tag: false, project_name: 'project_name', - project_url: 'http://example.gitlab.com', + project_url: 'example.gitlab.com', commit: { status: status, @@ -48,10 +48,10 @@ describe ChatMessage::BuildMessage do end def build_message(status_text = status) - ":" \ - " Commit :" \ + " Commit " \ - " of branch" \ + " of branch" \ " by hacker #{status_text} in #{duration} #{'second'.pluralize(duration)}" end end diff --git a/spec/models/project_services/chat_message/issue_message_spec.rb b/spec/models/project_services/chat_message/issue_message_spec.rb index 190ff4c535d..ebe0ead4408 100644 --- a/spec/models/project_services/chat_message/issue_message_spec.rb +++ b/spec/models/project_services/chat_message/issue_message_spec.rb @@ -10,14 +10,14 @@ describe ChatMessage::IssueMessage, models: true do username: 'test.user' }, project_name: 'project_name', - project_url: 'http://somewhere.com', + project_url: 'somewhere.com', object_attributes: { title: 'Issue title', id: 10, iid: 100, assignee_id: 1, - url: 'http://url.com', + url: 'url', action: 'open', state: 'opened', description: 'issue description' @@ -40,11 +40,11 @@ describe ChatMessage::IssueMessage, models: true do context 'open' do it 'returns a message regarding opening of issues' do expect(subject.pretext).to eq( - '[] Issue opened by test.user') + '] Issue opened by test.user') expect(subject.attachments).to eq([ { title: "#100 Issue title", - title_link: "http://url.com", + title_link: "url", text: "issue description", color: color, } @@ -60,7 +60,7 @@ describe ChatMessage::IssueMessage, models: true do it 'returns a message regarding closing of issues' do expect(subject.pretext). to eq( - '[] Issue closed by test.user') + '] Issue closed by test.user') expect(subject.attachments).to be_empty end end diff --git a/spec/models/project_services/chat_message/merge_message_spec.rb b/spec/models/project_services/chat_message/merge_message_spec.rb index cc154112e90..07c414c6ca4 100644 --- a/spec/models/project_services/chat_message/merge_message_spec.rb +++ b/spec/models/project_services/chat_message/merge_message_spec.rb @@ -10,14 +10,14 @@ describe ChatMessage::MergeMessage, models: true do username: 'test.user' }, project_name: 'project_name', - project_url: 'http://somewhere.com', + project_url: 'somewhere.com', object_attributes: { title: "Issue title\nSecond line", id: 10, iid: 100, assignee_id: 1, - url: 'http://url.com', + url: 'url', state: 'opened', description: 'issue description', source_branch: 'source_branch', @@ -31,8 +31,8 @@ describe ChatMessage::MergeMessage, models: true do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'test.user opened '\ - 'in : *Issue title*') + 'test.user opened '\ + 'in : *Issue title*') expect(subject.attachments).to be_empty end end @@ -43,8 +43,8 @@ describe ChatMessage::MergeMessage, models: true do end it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'test.user closed '\ - 'in : *Issue title*') + 'test.user closed '\ + 'in : *Issue title*') expect(subject.attachments).to be_empty end end diff --git a/spec/models/project_services/chat_message/note_message_spec.rb b/spec/models/project_services/chat_message/note_message_spec.rb index da700a08e57..31936da40a2 100644 --- a/spec/models/project_services/chat_message/note_message_spec.rb +++ b/spec/models/project_services/chat_message/note_message_spec.rb @@ -11,15 +11,15 @@ describe ChatMessage::NoteMessage, models: true do avatar_url: 'http://fakeavatar' }, project_name: 'project_name', - project_url: 'http://somewhere.com', + project_url: 'somewhere.com', repository: { name: 'project_name', - url: 'http://somewhere.com', + url: 'somewhere.com', }, object_attributes: { id: 10, note: 'comment on a commit', - url: 'http://url.com', + url: 'url', noteable_type: 'Commit' } } @@ -37,8 +37,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on commits' do message = described_class.new(@args) - expect(message.pretext).to eq("test.user in : " \ + expect(message.pretext).to eq("test.user in : " \ "*Added a commit message*") expected_attachments = [ { @@ -63,8 +63,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on a merge request' do message = described_class.new(@args) - expect(message.pretext).to eq("test.user in : " \ + expect(message.pretext).to eq("test.user in : " \ "*merge request title*") expected_attachments = [ { @@ -90,8 +90,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on an issue' do message = described_class.new(@args) expect(message.pretext).to eq( - "test.user in : " \ + "test.user in : " \ "*issue title*") expected_attachments = [ { @@ -115,8 +115,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on a project snippet' do message = described_class.new(@args) - expect(message.pretext).to eq("test.user in : " \ + expect(message.pretext).to eq("test.user in : " \ "*snippet title*") expected_attachments = [ { diff --git a/spec/models/project_services/chat_message/push_message_spec.rb b/spec/models/project_services/chat_message/push_message_spec.rb index 24928873bad..b781c4505db 100644 --- a/spec/models/project_services/chat_message/push_message_spec.rb +++ b/spec/models/project_services/chat_message/push_message_spec.rb @@ -10,7 +10,7 @@ describe ChatMessage::PushMessage, models: true do project_name: 'project_name', ref: 'refs/heads/master', user_name: 'test.user', - project_url: 'http://url.com' + project_url: 'url' } end @@ -19,20 +19,20 @@ describe ChatMessage::PushMessage, models: true do context 'push' do before do args[:commits] = [ - { message: 'message1', url: 'http://url1.com', id: 'abcdefghijkl', author: { name: 'author1' } }, - { message: 'message2', url: 'http://url2.com', id: '123456789012', author: { name: 'author2' } }, + { message: 'message1', url: 'url1', id: 'abcdefghijkl', author: { name: 'author1' } }, + { message: 'message2', url: 'url2', id: '123456789012', author: { name: 'author2' } }, ] end it 'returns a message regarding pushes' do expect(subject.pretext).to eq( - 'test.user pushed to branch of '\ - ' ()' + 'test.user pushed to branch of '\ + ' ()' ) expect(subject.attachments).to eq([ { - text: ": message1 - author1\n"\ - ": message2 - author2", + text: ": message1 - author1\n"\ + ": message2 - author2", color: color, } ]) @@ -47,14 +47,14 @@ describe ChatMessage::PushMessage, models: true do project_name: 'project_name', ref: 'refs/tags/new_tag', user_name: 'test.user', - project_url: 'http://url.com' + project_url: 'url' } end it 'returns a message regarding pushes' do expect(subject.pretext).to eq('test.user pushed new tag ' \ - ' to ' \ - '') + ' to ' \ + '') expect(subject.attachments).to be_empty end end @@ -66,8 +66,8 @@ describe ChatMessage::PushMessage, models: true do it 'returns a message regarding a new branch' do expect(subject.pretext).to eq( - 'test.user pushed new branch to '\ - '' + 'test.user pushed new branch to '\ + '' ) expect(subject.attachments).to be_empty end @@ -80,7 +80,7 @@ describe ChatMessage::PushMessage, models: true do it 'returns a message regarding a removed branch' do expect(subject.pretext).to eq( - 'test.user removed branch master from ' + 'test.user removed branch master from ' ) expect(subject.attachments).to be_empty end diff --git a/spec/models/project_services/chat_message/wiki_page_message_spec.rb b/spec/models/project_services/chat_message/wiki_page_message_spec.rb index a2ad61e38e7..94c04dc0865 100644 --- a/spec/models/project_services/chat_message/wiki_page_message_spec.rb +++ b/spec/models/project_services/chat_message/wiki_page_message_spec.rb @@ -10,10 +10,10 @@ describe ChatMessage::WikiPageMessage, models: true do username: 'test.user' }, project_name: 'project_name', - project_url: 'http://somewhere.com', + project_url: 'somewhere.com', object_attributes: { title: 'Wiki page title', - url: 'http://url.com', + url: 'url', content: 'Wiki page description' } } @@ -25,7 +25,7 @@ describe ChatMessage::WikiPageMessage, models: true do it 'returns a message that a new wiki page was created' do expect(subject.pretext).to eq( - 'test.user created in : '\ + 'test.user created in : '\ '*Wiki page title*') end end @@ -35,7 +35,7 @@ describe ChatMessage::WikiPageMessage, models: true do it 'returns a message that a wiki page was updated' do expect(subject.pretext).to eq( - 'test.user edited in : '\ + 'test.user edited in : '\ '*Wiki page title*') end end -- cgit v1.2.1 From f9023adbad4939ba597d509e319105659e61734b Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 18 Dec 2016 23:32:53 +0100 Subject: Fix spec failures --- app/models/project_services/slack_slash_commands_service.rb | 2 +- spec/features/admin/admin_settings_spec.rb | 4 ++-- spec/lib/gitlab/import_export/all_models.yml | 1 + spec/models/project_spec.rb | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/models/project_services/slack_slash_commands_service.rb b/app/models/project_services/slack_slash_commands_service.rb index 8413c657099..6782ba5ad0a 100644 --- a/app/models/project_services/slack_slash_commands_service.rb +++ b/app/models/project_services/slack_slash_commands_service.rb @@ -2,7 +2,7 @@ class SlackSlashCommandsService < ChatSlashCommandsService include TriggersHelper def title - 'Slack Slash Command' + 'Slack Command' end def description diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 8cd66f189be..e7a23746244 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -17,9 +17,9 @@ feature 'Admin updates settings', feature: true do expect(page).to have_content "Application settings saved successfully" end - scenario 'Change Slack Service template settings' do + scenario 'Change Slack Notifications Service template settings' do click_link 'Service Templates' - click_link 'Slack' + click_link 'Slack notifications' fill_in 'Webhook', with: 'http://localhost' fill_in 'Username', with: 'test_user' fill_in 'service_push_channel', with: '#test_channel' diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 9b49d6837c3..7e618e2fcf5 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -129,6 +129,7 @@ project: - builds_email_service - pipelines_email_service - mattermost_slash_commands_service +- slack_slash_commands_service - irker_service - pivotaltracker_service - hipchat_service diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index bab3c3dbb02..4b39dc77f29 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -20,7 +20,6 @@ describe Project, models: true do it { is_expected.to have_many(:deploy_keys) } it { is_expected.to have_many(:hooks).dependent(:destroy) } it { is_expected.to have_many(:protected_branches).dependent(:destroy) } - it { is_expected.to have_many(:chat_services) } it { is_expected.to have_one(:forked_project_link).dependent(:destroy) } it { is_expected.to have_one(:slack_notification_service).dependent(:destroy) } it { is_expected.to have_one(:mattermost_notification_service).dependent(:destroy) } @@ -37,6 +36,7 @@ describe Project, models: true do it { is_expected.to have_one(:hipchat_service).dependent(:destroy) } it { is_expected.to have_one(:flowdock_service).dependent(:destroy) } it { is_expected.to have_one(:assembla_service).dependent(:destroy) } + it { is_expected.to have_one(:slack_slash_commands_service).dependent(:destroy) } it { is_expected.to have_one(:mattermost_slash_commands_service).dependent(:destroy) } it { is_expected.to have_one(:gemnasium_service).dependent(:destroy) } it { is_expected.to have_one(:buildkite_service).dependent(:destroy) } -- cgit v1.2.1 From f5ff372140d066d7bcedc0ad0799c723a9012bb0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 18 Dec 2016 23:34:40 +0100 Subject: Fix failures --- spec/features/admin/admin_settings_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index e7a23746244..47fa2f14307 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -30,7 +30,7 @@ feature 'Admin updates settings', feature: true do expect(page).to have_content 'Application settings saved successfully' - click_link 'Slack' + click_link 'Slack notifications' page.all('input[type=checkbox]').each do |checkbox| expect(checkbox).to be_checked -- cgit v1.2.1 From e6842ff5643c7fc88a65af24b56682f47f4be2de Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 19 Dec 2016 13:32:37 +0100 Subject: Improve code design --- app/models/project_services/chat_slash_commands_service.rb | 2 +- app/models/project_services/slack_slash_commands_service.rb | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/models/project_services/chat_slash_commands_service.rb b/app/models/project_services/chat_slash_commands_service.rb index 12261e9821e..0bc160af604 100644 --- a/app/models/project_services/chat_slash_commands_service.rb +++ b/app/models/project_services/chat_slash_commands_service.rb @@ -28,7 +28,7 @@ class ChatSlashCommandsService < Service end def trigger(params) - return nil unless valid_token?(params[:token]) + return unless valid_token?(params[:token]) user = find_chat_user(params) unless user diff --git a/app/models/project_services/slack_slash_commands_service.rb b/app/models/project_services/slack_slash_commands_service.rb index 6782ba5ad0a..cb19ebf4cad 100644 --- a/app/models/project_services/slack_slash_commands_service.rb +++ b/app/models/project_services/slack_slash_commands_service.rb @@ -14,13 +14,15 @@ class SlackSlashCommandsService < ChatSlashCommandsService end def trigger(params) - result = super - # Format messages to be Slack-compatible - if result && result[:text] - result[:text] = Slack::Notifier::LinkFormatter.format(result[:text]) + super.tap do |result| + result[:text] = format(result[:text]) end + end + + private - result + def format(text) + Slack::Notifier::LinkFormatter.format(text) if text end end -- cgit v1.2.1 From b1ccf99e87605216f7d5733d6a4ffb4530d4cfc9 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 19 Dec 2016 13:34:03 +0100 Subject: Fix previously reverted spec failures --- .../chat_message/build_message_spec.rb | 8 +++---- .../chat_message/issue_message_spec.rb | 10 ++++----- .../chat_message/merge_message_spec.rb | 12 +++++----- .../chat_message/note_message_spec.rb | 22 +++++++++--------- .../chat_message/pipeline_message_spec.rb | 8 +++---- .../chat_message/push_message_spec.rb | 26 +++++++++++----------- .../chat_message/wiki_page_message_spec.rb | 8 +++---- 7 files changed, 47 insertions(+), 47 deletions(-) diff --git a/spec/models/project_services/chat_message/build_message_spec.rb b/spec/models/project_services/chat_message/build_message_spec.rb index b71d153f814..50ad5013df9 100644 --- a/spec/models/project_services/chat_message/build_message_spec.rb +++ b/spec/models/project_services/chat_message/build_message_spec.rb @@ -10,7 +10,7 @@ describe ChatMessage::BuildMessage do tag: false, project_name: 'project_name', - project_url: 'example.gitlab.com', + project_url: 'http://example.gitlab.com', commit: { status: status, @@ -48,10 +48,10 @@ describe ChatMessage::BuildMessage do end def build_message(status_text = status) - ":" \ - " Commit :" \ + " Commit " \ - " of branch" \ + " of branch" \ " by hacker #{status_text} in #{duration} #{'second'.pluralize(duration)}" end end diff --git a/spec/models/project_services/chat_message/issue_message_spec.rb b/spec/models/project_services/chat_message/issue_message_spec.rb index ebe0ead4408..190ff4c535d 100644 --- a/spec/models/project_services/chat_message/issue_message_spec.rb +++ b/spec/models/project_services/chat_message/issue_message_spec.rb @@ -10,14 +10,14 @@ describe ChatMessage::IssueMessage, models: true do username: 'test.user' }, project_name: 'project_name', - project_url: 'somewhere.com', + project_url: 'http://somewhere.com', object_attributes: { title: 'Issue title', id: 10, iid: 100, assignee_id: 1, - url: 'url', + url: 'http://url.com', action: 'open', state: 'opened', description: 'issue description' @@ -40,11 +40,11 @@ describe ChatMessage::IssueMessage, models: true do context 'open' do it 'returns a message regarding opening of issues' do expect(subject.pretext).to eq( - '] Issue opened by test.user') + '[] Issue opened by test.user') expect(subject.attachments).to eq([ { title: "#100 Issue title", - title_link: "url", + title_link: "http://url.com", text: "issue description", color: color, } @@ -60,7 +60,7 @@ describe ChatMessage::IssueMessage, models: true do it 'returns a message regarding closing of issues' do expect(subject.pretext). to eq( - '] Issue closed by test.user') + '[] Issue closed by test.user') expect(subject.attachments).to be_empty end end diff --git a/spec/models/project_services/chat_message/merge_message_spec.rb b/spec/models/project_services/chat_message/merge_message_spec.rb index 07c414c6ca4..cc154112e90 100644 --- a/spec/models/project_services/chat_message/merge_message_spec.rb +++ b/spec/models/project_services/chat_message/merge_message_spec.rb @@ -10,14 +10,14 @@ describe ChatMessage::MergeMessage, models: true do username: 'test.user' }, project_name: 'project_name', - project_url: 'somewhere.com', + project_url: 'http://somewhere.com', object_attributes: { title: "Issue title\nSecond line", id: 10, iid: 100, assignee_id: 1, - url: 'url', + url: 'http://url.com', state: 'opened', description: 'issue description', source_branch: 'source_branch', @@ -31,8 +31,8 @@ describe ChatMessage::MergeMessage, models: true do context 'open' do it 'returns a message regarding opening of merge requests' do expect(subject.pretext).to eq( - 'test.user opened '\ - 'in : *Issue title*') + 'test.user opened '\ + 'in : *Issue title*') expect(subject.attachments).to be_empty end end @@ -43,8 +43,8 @@ describe ChatMessage::MergeMessage, models: true do end it 'returns a message regarding closing of merge requests' do expect(subject.pretext).to eq( - 'test.user closed '\ - 'in : *Issue title*') + 'test.user closed '\ + 'in : *Issue title*') expect(subject.attachments).to be_empty end end diff --git a/spec/models/project_services/chat_message/note_message_spec.rb b/spec/models/project_services/chat_message/note_message_spec.rb index 31936da40a2..da700a08e57 100644 --- a/spec/models/project_services/chat_message/note_message_spec.rb +++ b/spec/models/project_services/chat_message/note_message_spec.rb @@ -11,15 +11,15 @@ describe ChatMessage::NoteMessage, models: true do avatar_url: 'http://fakeavatar' }, project_name: 'project_name', - project_url: 'somewhere.com', + project_url: 'http://somewhere.com', repository: { name: 'project_name', - url: 'somewhere.com', + url: 'http://somewhere.com', }, object_attributes: { id: 10, note: 'comment on a commit', - url: 'url', + url: 'http://url.com', noteable_type: 'Commit' } } @@ -37,8 +37,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on commits' do message = described_class.new(@args) - expect(message.pretext).to eq("test.user in : " \ + expect(message.pretext).to eq("test.user in : " \ "*Added a commit message*") expected_attachments = [ { @@ -63,8 +63,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on a merge request' do message = described_class.new(@args) - expect(message.pretext).to eq("test.user in : " \ + expect(message.pretext).to eq("test.user in : " \ "*merge request title*") expected_attachments = [ { @@ -90,8 +90,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on an issue' do message = described_class.new(@args) expect(message.pretext).to eq( - "test.user in : " \ + "test.user in : " \ "*issue title*") expected_attachments = [ { @@ -115,8 +115,8 @@ describe ChatMessage::NoteMessage, models: true do it 'returns a message regarding notes on a project snippet' do message = described_class.new(@args) - expect(message.pretext).to eq("test.user in : " \ + expect(message.pretext).to eq("test.user in : " \ "*snippet title*") expected_attachments = [ { diff --git a/spec/models/project_services/chat_message/pipeline_message_spec.rb b/spec/models/project_services/chat_message/pipeline_message_spec.rb index eca71db07b6..bf2a9616455 100644 --- a/spec/models/project_services/chat_message/pipeline_message_spec.rb +++ b/spec/models/project_services/chat_message/pipeline_message_spec.rb @@ -15,7 +15,7 @@ describe ChatMessage::PipelineMessage do duration: duration }, project: { path_with_namespace: 'project_name', - web_url: 'example.gitlab.com' }, + web_url: 'http://example.gitlab.com' }, user: user } end @@ -59,9 +59,9 @@ describe ChatMessage::PipelineMessage do end def build_message(status_text = status, name = user[:name]) - ":" \ - " Pipeline " \ - " of branch" \ + ":" \ + " Pipeline " \ + " of branch" \ " by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" end end diff --git a/spec/models/project_services/chat_message/push_message_spec.rb b/spec/models/project_services/chat_message/push_message_spec.rb index b781c4505db..24928873bad 100644 --- a/spec/models/project_services/chat_message/push_message_spec.rb +++ b/spec/models/project_services/chat_message/push_message_spec.rb @@ -10,7 +10,7 @@ describe ChatMessage::PushMessage, models: true do project_name: 'project_name', ref: 'refs/heads/master', user_name: 'test.user', - project_url: 'url' + project_url: 'http://url.com' } end @@ -19,20 +19,20 @@ describe ChatMessage::PushMessage, models: true do context 'push' do before do args[:commits] = [ - { message: 'message1', url: 'url1', id: 'abcdefghijkl', author: { name: 'author1' } }, - { message: 'message2', url: 'url2', id: '123456789012', author: { name: 'author2' } }, + { message: 'message1', url: 'http://url1.com', id: 'abcdefghijkl', author: { name: 'author1' } }, + { message: 'message2', url: 'http://url2.com', id: '123456789012', author: { name: 'author2' } }, ] end it 'returns a message regarding pushes' do expect(subject.pretext).to eq( - 'test.user pushed to branch of '\ - ' ()' + 'test.user pushed to branch of '\ + ' ()' ) expect(subject.attachments).to eq([ { - text: ": message1 - author1\n"\ - ": message2 - author2", + text: ": message1 - author1\n"\ + ": message2 - author2", color: color, } ]) @@ -47,14 +47,14 @@ describe ChatMessage::PushMessage, models: true do project_name: 'project_name', ref: 'refs/tags/new_tag', user_name: 'test.user', - project_url: 'url' + project_url: 'http://url.com' } end it 'returns a message regarding pushes' do expect(subject.pretext).to eq('test.user pushed new tag ' \ - ' to ' \ - '') + ' to ' \ + '') expect(subject.attachments).to be_empty end end @@ -66,8 +66,8 @@ describe ChatMessage::PushMessage, models: true do it 'returns a message regarding a new branch' do expect(subject.pretext).to eq( - 'test.user pushed new branch to '\ - '' + 'test.user pushed new branch to '\ + '' ) expect(subject.attachments).to be_empty end @@ -80,7 +80,7 @@ describe ChatMessage::PushMessage, models: true do it 'returns a message regarding a removed branch' do expect(subject.pretext).to eq( - 'test.user removed branch master from ' + 'test.user removed branch master from ' ) expect(subject.attachments).to be_empty end diff --git a/spec/models/project_services/chat_message/wiki_page_message_spec.rb b/spec/models/project_services/chat_message/wiki_page_message_spec.rb index 94c04dc0865..a2ad61e38e7 100644 --- a/spec/models/project_services/chat_message/wiki_page_message_spec.rb +++ b/spec/models/project_services/chat_message/wiki_page_message_spec.rb @@ -10,10 +10,10 @@ describe ChatMessage::WikiPageMessage, models: true do username: 'test.user' }, project_name: 'project_name', - project_url: 'somewhere.com', + project_url: 'http://somewhere.com', object_attributes: { title: 'Wiki page title', - url: 'url', + url: 'http://url.com', content: 'Wiki page description' } } @@ -25,7 +25,7 @@ describe ChatMessage::WikiPageMessage, models: true do it 'returns a message that a new wiki page was created' do expect(subject.pretext).to eq( - 'test.user created in : '\ + 'test.user created in : '\ '*Wiki page title*') end end @@ -35,7 +35,7 @@ describe ChatMessage::WikiPageMessage, models: true do it 'returns a message that a wiki page was updated' do expect(subject.pretext).to eq( - 'test.user edited in : '\ + 'test.user edited in : '\ '*Wiki page title*') end end -- cgit v1.2.1 From 298d05a5c3cc3c2f1daa4d77c45f9c90b53248df Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 19 Dec 2016 15:40:06 +0100 Subject: Improve after feedback --- features/project/service.feature | 6 ++--- features/steps/project/services.rb | 8 +++---- lib/api/services.rb | 9 ++++++- lib/gitlab/chat_commands/help.rb | 28 ---------------------- lib/gitlab/chat_commands/presenter.rb | 2 +- .../projects/services/slack_slash_command_spec.rb | 18 +++++++------- .../mattermost_slash_commands_service_spec.rb | 4 ++-- .../slack_slash_commands_service.rb | 6 +++-- .../support/chat_slash_commands_shared_examples.rb | 2 +- 9 files changed, 32 insertions(+), 51 deletions(-) delete mode 100644 lib/gitlab/chat_commands/help.rb diff --git a/features/project/service.feature b/features/project/service.feature index 3a7b8308524..892db48d785 100644 --- a/features/project/service.feature +++ b/features/project/service.feature @@ -39,9 +39,9 @@ Feature: Project Services Scenario: Activate Slack service When I visit project "Shop" services page - And I click Slack service link - And I fill Slack settings - Then I should see Slack service settings saved + And I click Slack Notifications service link + And I fill Slack Notifications settings + Then I should see Slack Notifications service settings saved Scenario: Activate Pushover service When I visit project "Shop" services page diff --git a/features/steps/project/services.rb b/features/steps/project/services.rb index bd6466f3686..06a1afedbd9 100644 --- a/features/steps/project/services.rb +++ b/features/steps/project/services.rb @@ -137,17 +137,17 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps expect(find_field('Colorize messages').value).to eq '1' end - step 'I click Slack service link' do - click_link 'Slack' + step 'I click Slack Notifications service link' do + click_link 'Slack Notifications' end - step 'I fill Slack settings' do + step 'I fill Slack Notifications settings' do check 'Active' fill_in 'Webhook', with: 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' click_button 'Save' end - step 'I should see Slack service settings saved' do + step 'I should see Slack Notifications service settings saved' do expect(find_field('Webhook').value).to eq 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' end diff --git a/lib/api/services.rb b/lib/api/services.rb index 59232c84c24..aa97f6af0b2 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -378,7 +378,6 @@ module API desc: 'A custom certificate authority bundle to verify the Kubernetes cluster with (PEM format)' }, ], - 'mattermost-slash-commands' => [ { required: true, @@ -387,6 +386,14 @@ module API desc: 'The Mattermost token' } ], + 'slack-slash-commands' => [ + { + required: true, + name: :token, + type: String, + desc: 'The Slack token' + } + ], 'pipelines-email' => [ { required: true, diff --git a/lib/gitlab/chat_commands/help.rb b/lib/gitlab/chat_commands/help.rb deleted file mode 100644 index e76733f5445..00000000000 --- a/lib/gitlab/chat_commands/help.rb +++ /dev/null @@ -1,28 +0,0 @@ -module Gitlab - module ChatCommands - class Help < BaseCommand - # This class has to be used last, as it always matches. It has to match - # because other commands were not triggered and we want to show the help - # command - def self.match(_text) - true - end - - def self.help_message - 'help' - end - - def self.allowed?(_project, _user) - true - end - - def execute(commands) - Gitlab::ChatCommands::Presenters::Help.new(commands).present(trigger) - end - - def trigger - params[:command] - end - end - end -end diff --git a/lib/gitlab/chat_commands/presenter.rb b/lib/gitlab/chat_commands/presenter.rb index b4c4dc252ca..caceaa25391 100644 --- a/lib/gitlab/chat_commands/presenter.rb +++ b/lib/gitlab/chat_commands/presenter.rb @@ -1,7 +1,7 @@ module Gitlab module ChatCommands class Presenter - include Gitlab::Routing.url_helpers + include Gitlab::Routing def authorize_chat_name(url) message = if url diff --git a/spec/features/projects/services/slack_slash_command_spec.rb b/spec/features/projects/services/slack_slash_command_spec.rb index dee43d69895..70e203efcf5 100644 --- a/spec/features/projects/services/slack_slash_command_spec.rb +++ b/spec/features/projects/services/slack_slash_command_spec.rb @@ -1,18 +1,18 @@ require 'spec_helper' -feature 'Setup Slack slash commands', feature: true do +feature 'Slack slash commands', feature: true do include WaitForAjax - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:service) { project.create_slack_slash_commands_service } + given(:user) { create(:user) } + given(:project) { create(:project) } + given(:service) { project.create_slack_slash_commands_service } - before do + background do project.team << [user, :master] login_as(user) end - describe 'user visits the slack slash command config page', js: true do + scenario 'user visits the slack slash command config page', js: true do it 'shows a help message' do visit edit_namespace_project_service_path(project.namespace, project, service) @@ -22,8 +22,8 @@ feature 'Setup Slack slash commands', feature: true do end end - describe 'saving a token' do - let(:token) { ('a'..'z').to_a.join } + scenario 'saving a token' do + given(:token) { ('a'..'z').to_a.join } it 'shows the token after saving' do visit edit_namespace_project_service_path(project.namespace, project, service) @@ -37,7 +37,7 @@ feature 'Setup Slack slash commands', feature: true do end end - describe 'the trigger url' do + scenario 'the trigger url' do it 'shows the correct url' do visit edit_namespace_project_service_path(project.namespace, project, service) diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/project_services/mattermost_slash_commands_service_spec.rb index 5c34cb6b4cf..1ae1483e2a4 100644 --- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb +++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' -describe MattermostSlashCommandsService, models: true do - it_behaves_like "chat slash commands" +describe MattermostSlashCommandsService, :models do + it_behaves_like "chat slash commands service" end diff --git a/spec/models/project_services/slack_slash_commands_service.rb b/spec/models/project_services/slack_slash_commands_service.rb index c3fa80caebe..5775e439906 100644 --- a/spec/models/project_services/slack_slash_commands_service.rb +++ b/spec/models/project_services/slack_slash_commands_service.rb @@ -1,7 +1,7 @@ require 'spec_helper' -describe SlackSlashCommandsService, models: true do - it_behaves_like "chat slash commands" +describe SlackSlashCommandsService, :models do + it_behaves_like "chat slash commands service" describe '#trigger' do context 'when an auth url is generated' do @@ -15,11 +15,13 @@ describe SlackSlashCommandsService, models: true do token: 'token' } end + let(:service) do project.create_slack_slash_commands_service( properties: { token: 'token' } ) end + let(:authorize_url) do 'http://authorize.example.com/' end diff --git a/spec/support/chat_slash_commands_shared_examples.rb b/spec/support/chat_slash_commands_shared_examples.rb index 96130b45235..4dfa29849ee 100644 --- a/spec/support/chat_slash_commands_shared_examples.rb +++ b/spec/support/chat_slash_commands_shared_examples.rb @@ -1,4 +1,4 @@ -RSpec.shared_examples 'chat slash commands' do +RSpec.shared_examples 'chat slash commands service' do describe "Associations" do it { is_expected.to respond_to :token } it { is_expected.to have_many :chat_names } -- cgit v1.2.1 From e06f88effa842c73d3827593f8d28846207bfca0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 19 Dec 2016 22:12:30 +0100 Subject: Fix specs --- features/project/service.feature | 6 ++-- features/steps/project/services.rb | 6 ++-- .../projects/services/slack_slash_command_spec.rb | 36 +++++++++------------- 3 files changed, 20 insertions(+), 28 deletions(-) diff --git a/features/project/service.feature b/features/project/service.feature index 892db48d785..cce5f58adec 100644 --- a/features/project/service.feature +++ b/features/project/service.feature @@ -37,10 +37,10 @@ Feature: Project Services And I fill Assembla settings Then I should see Assembla service settings saved - Scenario: Activate Slack service + Scenario: Activate Slack notifications service When I visit project "Shop" services page - And I click Slack Notifications service link - And I fill Slack Notifications settings + And I click Slack notifications service link + And I fill Slack notifications settings Then I should see Slack Notifications service settings saved Scenario: Activate Pushover service diff --git a/features/steps/project/services.rb b/features/steps/project/services.rb index 06a1afedbd9..a4d29770922 100644 --- a/features/steps/project/services.rb +++ b/features/steps/project/services.rb @@ -137,11 +137,11 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps expect(find_field('Colorize messages').value).to eq '1' end - step 'I click Slack Notifications service link' do - click_link 'Slack Notifications' + step 'I click Slack notifications service link' do + click_link 'Slack notifications' end - step 'I fill Slack Notifications settings' do + step 'I fill Slack notifications settings' do check 'Active' fill_in 'Webhook', with: 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' click_button 'Save' diff --git a/spec/features/projects/services/slack_slash_command_spec.rb b/spec/features/projects/services/slack_slash_command_spec.rb index 70e203efcf5..32b32f7ae8e 100644 --- a/spec/features/projects/services/slack_slash_command_spec.rb +++ b/spec/features/projects/services/slack_slash_command_spec.rb @@ -12,37 +12,29 @@ feature 'Slack slash commands', feature: true do login_as(user) end - scenario 'user visits the slack slash command config page', js: true do - it 'shows a help message' do - visit edit_namespace_project_service_path(project.namespace, project, service) + scenario 'user visits the slack slash command config page and shows a help message', js: true do + visit edit_namespace_project_service_path(project.namespace, project, service) - wait_for_ajax + wait_for_ajax - expect(page).to have_content('This service allows GitLab users to perform common') - end + expect(page).to have_content('This service allows GitLab users to perform common') end - scenario 'saving a token' do - given(:token) { ('a'..'z').to_a.join } + scenario 'shows the token after saving' do + visit edit_namespace_project_service_path(project.namespace, project, service) - it 'shows the token after saving' do - visit edit_namespace_project_service_path(project.namespace, project, service) + fill_in 'service_token', with: 'token' + click_on 'Save' - fill_in 'service_token', with: token - click_on 'Save' + value = find_field('service_token').value - value = find_field('service_token').value - - expect(value).to eq(token) - end + expect(value).to eq('token') end - scenario 'the trigger url' do - it 'shows the correct url' do - visit edit_namespace_project_service_path(project.namespace, project, service) + scenario 'shows the correct trigger url' do + visit edit_namespace_project_service_path(project.namespace, project, service) - value = find_field('url').value - expect(value).to match("api/v3/projects/#{project.id}/services/slack_slash_commands/trigger") - end + value = find_field('url').value + expect(value).to match("api/v3/projects/#{project.id}/services/slack_slash_commands/trigger") end end -- cgit v1.2.1