From bc9c245b87375abafd9050648bf020b879172a79 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Tue, 10 Jan 2017 19:43:58 +0100 Subject: Chat Commands have presenters This improves the styling and readability of the code. This is supported by both Mattermost and Slack. --- .../chat_slash_commands_service.rb | 20 +-- lib/gitlab/chat_commands/base_command.rb | 4 - lib/gitlab/chat_commands/command.rb | 22 +-- lib/gitlab/chat_commands/deploy.rb | 24 ++-- lib/gitlab/chat_commands/issue_create.rb | 18 ++- lib/gitlab/chat_commands/issue_search.rb | 10 +- lib/gitlab/chat_commands/issue_show.rb | 8 +- lib/gitlab/chat_commands/presenter.rb | 131 ----------------- lib/gitlab/chat_commands/presenters/access.rb | 22 +++ lib/gitlab/chat_commands/presenters/base.rb | 73 ++++++++++ lib/gitlab/chat_commands/presenters/deploy.rb | 24 ++++ lib/gitlab/chat_commands/presenters/issuable.rb | 33 +++++ lib/gitlab/chat_commands/presenters/list_issues.rb | 32 +++++ lib/gitlab/chat_commands/presenters/show_issue.rb | 38 +++++ lib/mattermost/client.rb | 41 ------ lib/mattermost/command.rb | 10 -- lib/mattermost/error.rb | 3 - lib/mattermost/session.rb | 160 --------------------- lib/mattermost/team.rb | 7 - spec/lib/gitlab/chat_commands/command_spec.rb | 60 +------- spec/lib/gitlab/chat_commands/deploy_spec.rb | 24 ++-- spec/lib/gitlab/chat_commands/issue_create_spec.rb | 12 +- spec/lib/gitlab/chat_commands/issue_search_spec.rb | 12 +- spec/lib/gitlab/chat_commands/issue_show_spec.rb | 25 +++- .../gitlab/chat_commands/presenters/access_spec.rb | 49 +++++++ .../gitlab/chat_commands/presenters/deploy_spec.rb | 47 ++++++ .../chat_commands/presenters/list_issues_spec.rb | 24 ++++ .../chat_commands/presenters/show_issue_spec.rb | 27 ++++ spec/lib/mattermost/client_spec.rb | 24 ---- spec/lib/mattermost/command_spec.rb | 61 -------- spec/lib/mattermost/session_spec.rb | 123 ---------------- spec/lib/mattermost/team_spec.rb | 66 --------- 32 files changed, 476 insertions(+), 758 deletions(-) delete mode 100644 lib/gitlab/chat_commands/presenter.rb create mode 100644 lib/gitlab/chat_commands/presenters/access.rb create mode 100644 lib/gitlab/chat_commands/presenters/base.rb create mode 100644 lib/gitlab/chat_commands/presenters/deploy.rb create mode 100644 lib/gitlab/chat_commands/presenters/issuable.rb create mode 100644 lib/gitlab/chat_commands/presenters/list_issues.rb create mode 100644 lib/gitlab/chat_commands/presenters/show_issue.rb delete mode 100644 lib/mattermost/client.rb delete mode 100644 lib/mattermost/command.rb delete mode 100644 lib/mattermost/error.rb delete mode 100644 lib/mattermost/session.rb delete mode 100644 lib/mattermost/team.rb create mode 100644 spec/lib/gitlab/chat_commands/presenters/access_spec.rb create mode 100644 spec/lib/gitlab/chat_commands/presenters/deploy_spec.rb create mode 100644 spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb create mode 100644 spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb delete mode 100644 spec/lib/mattermost/client_spec.rb delete mode 100644 spec/lib/mattermost/command_spec.rb delete mode 100644 spec/lib/mattermost/session_spec.rb delete mode 100644 spec/lib/mattermost/team_spec.rb diff --git a/app/models/project_services/chat_slash_commands_service.rb b/app/models/project_services/chat_slash_commands_service.rb index 2bcff541cc0..608754f3035 100644 --- a/app/models/project_services/chat_slash_commands_service.rb +++ b/app/models/project_services/chat_slash_commands_service.rb @@ -28,20 +28,24 @@ class ChatSlashCommandsService < Service end def trigger(params) - return unless valid_token?(params[:token]) + return access_presenter unless valid_token?(params[:token]) user = find_chat_user(params) - unless user + + if user + Gitlab::ChatCommands::Command.new(project, user, params).execute + else url = authorize_chat_name_url(params) - return presenter.authorize_chat_name(url) + access_presenter(url).authorize end - - Gitlab::ChatCommands::Command.new(project, user, - params).execute end private + def access_presenter(url = nil) + Gitlab::ChatCommands::Presenters::Access.new(url) + end + def find_chat_user(params) ChatNames::FindUserService.new(self, params).execute end @@ -49,8 +53,4 @@ class ChatSlashCommandsService < Service def authorize_chat_name_url(params) ChatNames::AuthorizeUserService.new(self, params).execute end - - def presenter - Gitlab::ChatCommands::Presenter.new - end end diff --git a/lib/gitlab/chat_commands/base_command.rb b/lib/gitlab/chat_commands/base_command.rb index 4fe53ce93a9..25da8474e95 100644 --- a/lib/gitlab/chat_commands/base_command.rb +++ b/lib/gitlab/chat_commands/base_command.rb @@ -42,10 +42,6 @@ module Gitlab def find_by_iid(iid) collection.find_by(iid: iid) end - - def presenter - Gitlab::ChatCommands::Presenter.new - end end end end diff --git a/lib/gitlab/chat_commands/command.rb b/lib/gitlab/chat_commands/command.rb index 145086755e4..ac7ee868402 100644 --- a/lib/gitlab/chat_commands/command.rb +++ b/lib/gitlab/chat_commands/command.rb @@ -13,9 +13,9 @@ module Gitlab if command if command.allowed?(project, current_user) - present command.new(project, current_user, params).execute(match) + command.new(project, current_user, params).execute(match) else - access_denied + Gitlab::ChatCommands::Presenters::Access.new.access_denied end else help(help_messages) @@ -25,7 +25,7 @@ module Gitlab def match_command match = nil service = available_commands.find do |klass| - match = klass.match(command) + match = klass.match(params[:text]) end [service, match] @@ -42,22 +42,6 @@ module Gitlab klass.available?(project) end end - - def command - params[:text] - end - - def help(messages) - presenter.help(messages, params[:command]) - end - - def access_denied - presenter.access_denied - end - - def present(resource) - presenter.present(resource) - end end end end diff --git a/lib/gitlab/chat_commands/deploy.rb b/lib/gitlab/chat_commands/deploy.rb index 7127d2f6d04..458d90f84e8 100644 --- a/lib/gitlab/chat_commands/deploy.rb +++ b/lib/gitlab/chat_commands/deploy.rb @@ -1,8 +1,6 @@ module Gitlab module ChatCommands class Deploy < BaseCommand - include Gitlab::Routing.url_helpers - def self.match(text) /\Adeploy\s+(?\S+.*)\s+to+\s+(?\S+.*)\z/.match(text) end @@ -24,35 +22,29 @@ module Gitlab to = match[:to] actions = find_actions(from, to) - return unless actions.present? - if actions.one? - play!(from, to, actions.first) + if actions.none? + Gitlab::ChatCommands::Presenters::Deploy.new(nil).no_actions + elsif actions.one? + action = play!(from, to, actions.first) + Gitlab::ChatCommands::Presenters::Deploy.new(action).present(from, to) else - Result.new(:error, 'Too many actions defined') + Gitlab::ChatCommands::Presenters::Deploy.new(actions).too_many_actions end end private def play!(from, to, action) - new_action = action.play(current_user) - - Result.new(:success, "Deployment from #{from} to #{to} started. Follow the progress: #{url(new_action)}.") + action.play(current_user) end def find_actions(from, to) environment = project.environments.find_by(name: from) - return unless environment + return [] unless environment environment.actions_for(to).select(&:starts_environment?) end - - def url(subject) - polymorphic_url( - [subject.project.namespace.becomes(Namespace), subject.project, subject] - ) - end end end end diff --git a/lib/gitlab/chat_commands/issue_create.rb b/lib/gitlab/chat_commands/issue_create.rb index cefb6775db8..a06f13b0f72 100644 --- a/lib/gitlab/chat_commands/issue_create.rb +++ b/lib/gitlab/chat_commands/issue_create.rb @@ -2,7 +2,7 @@ module Gitlab module ChatCommands class IssueCreate < IssueCommand def self.match(text) - # we can not match \n with the dot by passing the m modifier as than + # we can not match \n with the dot by passing the m modifier as than # the title and description are not seperated /\Aissue\s+(new|create)\s+(?[^\n]*)\n*(?<description>(.|\n)*)/.match(text) end @@ -19,8 +19,24 @@ module Gitlab title = match[:title] description = match[:description].to_s.rstrip + issue = create_issue(title: title, description: description) + + if issue.errors.any? + presenter(issue).display_errors + else + presenter(issue).present + end + end + + private + + def create_issue(title:, description:) Issues::CreateService.new(project, current_user, title: title, description: description).execute end + + def presenter(issue) + Gitlab::ChatCommands::Presenters::ShowIssue.new(issue) + end end end end diff --git a/lib/gitlab/chat_commands/issue_search.rb b/lib/gitlab/chat_commands/issue_search.rb index 51bf80c800b..e2d3a0f466a 100644 --- a/lib/gitlab/chat_commands/issue_search.rb +++ b/lib/gitlab/chat_commands/issue_search.rb @@ -10,7 +10,15 @@ module Gitlab end def execute(match) - collection.search(match[:query]).limit(QUERY_LIMIT) + issues = collection.search(match[:query]).limit(QUERY_LIMIT) + + if issues.none? + Presenters::Access.new(issues).not_found + elsif issues.one? + Presenters::ShowIssue.new(issues.first).present + else + Presenters::ListIssues.new(issues).present + end end end end diff --git a/lib/gitlab/chat_commands/issue_show.rb b/lib/gitlab/chat_commands/issue_show.rb index 2a45d49cf6b..9f3e1b9a64b 100644 --- a/lib/gitlab/chat_commands/issue_show.rb +++ b/lib/gitlab/chat_commands/issue_show.rb @@ -10,7 +10,13 @@ module Gitlab end def execute(match) - find_by_iid(match[:iid]) + issue = find_by_iid(match[:iid]) + + if issue + Gitlab::ChatCommands::Presenters::ShowIssue.new(issue).present + else + Gitlab::ChatCommands::Presenters::Access.new.not_found + end end end end diff --git a/lib/gitlab/chat_commands/presenter.rb b/lib/gitlab/chat_commands/presenter.rb deleted file mode 100644 index 8930a21f406..00000000000 --- a/lib/gitlab/chat_commands/presenter.rb +++ /dev/null @@ -1,131 +0,0 @@ -module Gitlab - module ChatCommands - class Presenter - include Gitlab::Routing - - 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.none? - not_found - elsif subject.one? - single_resource(subject.first) - else - multiple_resources(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) - titles = resources.map { |resource| title(resource) } - - message = header_with_list("Multiple results were found:", titles) - - 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/gitlab/chat_commands/presenters/access.rb b/lib/gitlab/chat_commands/presenters/access.rb new file mode 100644 index 00000000000..6d18d745608 --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/access.rb @@ -0,0 +1,22 @@ +module Gitlab::ChatCommands::Presenters + class Access < Gitlab::ChatCommands::Presenters::Base + def access_denied + ephemeral_response(text: "Whoops! This action is not allowed. This incident will be [reported](https://xkcd.com/838/).") + end + + def not_found + ephemeral_response(text: "404 not found! GitLab couldn't find what you were looking for! :boom:") + end + + def authorize + message = + if @resource + ":wave: Hi there! Before I do anything for you, please [connect your GitLab account](#{@resource})." + else + ":sweat_smile: Couldn't identify you, nor can I autorize you!" + end + + ephemeral_response(text: message) + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/base.rb b/lib/gitlab/chat_commands/presenters/base.rb new file mode 100644 index 00000000000..0897025d85f --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/base.rb @@ -0,0 +1,73 @@ +module Gitlab::ChatCommands::Presenters + class Base + include Gitlab::Routing.url_helpers + + def initialize(resource = nil) + @resource = resource + end + + def display_errors + message = header_with_list("The action was not successful, because:", @resource.errors.full_messages) + + ephemeral_response(text: message) + end + + private + + def header_with_list(header, items) + message = [header] + + items.each do |item| + message << "- #{item}" + end + + message.join("\n") + end + + def ephemeral_response(message) + response = { + response_type: :ephemeral, + status: 200 + }.merge(message) + + format_response(response) + end + + def in_channel_response(message) + response = { + response_type: :in_channel, + status: 200 + }.merge(message) + + format_response(response) + end + + def format_response(response) + response[:text] = format(response[:text]) if response.has_key?(:text) + + if response.has_key?(:attachments) + response[:attachments].each do |attachment| + attachment[:pretext] = format(attachment[:pretext]) if attachment[:pretext] + attachment[:text] = format(attachment[:text]) if attachment[:text] + end + end + + response + end + + # Convert Markdown to slacks format + def format(string) + Slack::Notifier::LinkFormatter.format(string) + end + + def resource_url + url_for( + [ + @resource.project.namespace.becomes(Namespace), + @resource.project, + @resource + ] + ) + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/deploy.rb b/lib/gitlab/chat_commands/presenters/deploy.rb new file mode 100644 index 00000000000..4f6333812ff --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/deploy.rb @@ -0,0 +1,24 @@ +module Gitlab::ChatCommands::Presenters + class Deploy < Gitlab::ChatCommands::Presenters::Base + def present(from, to) + message = "Deployment started from #{from} to #{to}. [Follow its progress](#{resource_url})." + in_channel_response(text: message) + end + + def no_actions + ephemeral_response(text: "No action found to be executed") + end + + def too_many_actions + ephemeral_response(text: "Too many actions defined") + end + + private + + def resource_url + polymorphic_url( + [ @resource.project.namespace.becomes(Namespace), @resource.project, @resource] + ) + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/issuable.rb b/lib/gitlab/chat_commands/presenters/issuable.rb new file mode 100644 index 00000000000..9623387f188 --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/issuable.rb @@ -0,0 +1,33 @@ +module Gitlab::ChatCommands::Presenters + class Issuable < Gitlab::ChatCommands::Presenters::Base + private + + def project + @resource.project + end + + def author + @resource.author + end + + def fields + [ + { + title: "Assignee", + value: @resource.assignee ? @resource.assignee.name : "_None_", + short: true + }, + { + title: "Milestone", + value: @resource.milestone ? @resource.milestone.title : "_None_", + short: true + }, + { + title: "Labels", + value: @resource.labels.any? ? @resource.label_names : "_None_", + short: true + } + ] + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/list_issues.rb b/lib/gitlab/chat_commands/presenters/list_issues.rb new file mode 100644 index 00000000000..5a7b3fca5c2 --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/list_issues.rb @@ -0,0 +1,32 @@ +module Gitlab::ChatCommands::Presenters + class ListIssues < Gitlab::ChatCommands::Presenters::Base + def present + ephemeral_response(text: "Here are the issues I found:", attachments: attachments) + end + + private + + def attachments + @resource.map do |issue| + state = issue.open? ? "Open" : "Closed" + + { + fallback: "Issue #{issue.to_reference}: #{issue.title}", + color: "#d22852", + text: "[#{issue.to_reference}](#{url_for([namespace, project, issue])}) · #{issue.title} (#{state})", + mrkdwn_in: [ + "text" + ] + } + end + end + + def project + @project ||= @resource.first.project + end + + def namespace + @namespace ||= project.namespace.becomes(Namespace) + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/show_issue.rb b/lib/gitlab/chat_commands/presenters/show_issue.rb new file mode 100644 index 00000000000..2a89c30b972 --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/show_issue.rb @@ -0,0 +1,38 @@ +module Gitlab::ChatCommands::Presenters + class ShowIssue < Gitlab::ChatCommands::Presenters::Issuable + def present + in_channel_response(show_issue) + end + + private + + def show_issue + { + attachments: [ + { + title: @resource.title, + title_link: resource_url, + author_name: author.name, + author_icon: author.avatar_url, + fallback: "#{@resource.to_reference}: #{@resource.title}", + text: text, + fields: fields, + mrkdwn_in: [ + :title, + :text + ] + } + ] + } + end + + def text + message = "" + message << ":+1: #{@resource.upvotes} " unless @resource.upvotes.zero? + message << ":-1: #{@resource.downvotes} " unless @resource.downvotes.zero? + message << ":speech_balloon: #{@resource.user_notes_count}" unless @resource.user_notes_count.zero? + + message + end + end +end diff --git a/lib/mattermost/client.rb b/lib/mattermost/client.rb deleted file mode 100644 index ec2903b7ec6..00000000000 --- a/lib/mattermost/client.rb +++ /dev/null @@ -1,41 +0,0 @@ -module Mattermost - class ClientError < Mattermost::Error; end - - class Client - attr_reader :user - - def initialize(user) - @user = user - end - - private - - def with_session(&blk) - Mattermost::Session.new(user).with_session(&blk) - end - - def json_get(path, options = {}) - with_session do |session| - json_response session.get(path, options) - end - end - - def json_post(path, options = {}) - with_session do |session| - json_response session.post(path, options) - end - end - - def json_response(response) - json_response = JSON.parse(response.body) - - unless response.success? - raise Mattermost::ClientError.new(json_response['message'] || 'Undefined error') - end - - json_response - rescue JSON::JSONError - raise Mattermost::ClientError.new('Cannot parse response') - end - end -end diff --git a/lib/mattermost/command.rb b/lib/mattermost/command.rb deleted file mode 100644 index d1e4bb0eccf..00000000000 --- a/lib/mattermost/command.rb +++ /dev/null @@ -1,10 +0,0 @@ -module Mattermost - class Command < Client - def create(params) - response = json_post("/api/v3/teams/#{params[:team_id]}/commands/create", - body: params.to_json) - - response['token'] - end - end -end diff --git a/lib/mattermost/error.rb b/lib/mattermost/error.rb deleted file mode 100644 index 014df175be0..00000000000 --- a/lib/mattermost/error.rb +++ /dev/null @@ -1,3 +0,0 @@ -module Mattermost - class Error < StandardError; end -end diff --git a/lib/mattermost/session.rb b/lib/mattermost/session.rb deleted file mode 100644 index 377cb7b1021..00000000000 --- a/lib/mattermost/session.rb +++ /dev/null @@ -1,160 +0,0 @@ -module Mattermost - class NoSessionError < Mattermost::Error - def message - 'No session could be set up, is Mattermost configured with Single Sign On?' - end - end - - class ConnectionError < Mattermost::Error; end - - # This class' prime objective is to obtain a session token on a Mattermost - # instance with SSO configured where this GitLab instance is the provider. - # - # The process depends on OAuth, but skips a step in the authentication cycle. - # For example, usually a user would click the 'login in GitLab' button on - # Mattermost, which would yield a 302 status code and redirects you to GitLab - # to approve the use of your account on Mattermost. Which would trigger a - # callback so Mattermost knows this request is approved and gets the required - # data to create the user account etc. - # - # This class however skips the button click, and also the approval phase to - # speed up the process and keep it without manual action and get a session - # going. - class Session - include Doorkeeper::Helpers::Controller - include HTTParty - - LEASE_TIMEOUT = 60 - - base_uri Settings.mattermost.host - - attr_accessor :current_resource_owner, :token - - def initialize(current_user) - @current_resource_owner = current_user - end - - def with_session - with_lease do - raise Mattermost::NoSessionError unless create - - begin - yield self - rescue Errno::ECONNREFUSED - raise Mattermost::NoSessionError - ensure - destroy - end - end - end - - # Next methods are needed for Doorkeeper - def pre_auth - @pre_auth ||= Doorkeeper::OAuth::PreAuthorization.new( - Doorkeeper.configuration, server.client_via_uid, params) - end - - def authorization - @authorization ||= strategy.request - end - - def strategy - @strategy ||= server.authorization_request(pre_auth.response_type) - end - - def request - @request ||= OpenStruct.new(parameters: params) - end - - def params - Rack::Utils.parse_query(oauth_uri.query).symbolize_keys - end - - def get(path, options = {}) - handle_exceptions do - self.class.get(path, options.merge(headers: @headers)) - end - end - - def post(path, options = {}) - handle_exceptions do - self.class.post(path, options.merge(headers: @headers)) - end - end - - private - - def create - return unless oauth_uri - return unless token_uri - - @token = request_token - @headers = { - Authorization: "Bearer #{@token}" - } - - @token - end - - def destroy - post('/api/v3/users/logout') - end - - def oauth_uri - return @oauth_uri if defined?(@oauth_uri) - - @oauth_uri = nil - - response = get("/api/v3/oauth/gitlab/login", follow_redirects: false) - return unless 300 <= response.code && response.code < 400 - - redirect_uri = response.headers['location'] - return unless redirect_uri - - @oauth_uri = URI.parse(redirect_uri) - end - - def token_uri - @token_uri ||= - if oauth_uri - authorization.authorize.redirect_uri if pre_auth.authorizable? - end - end - - def request_token - response = get(token_uri, follow_redirects: false) - - if 200 <= response.code && response.code < 400 - response.headers['token'] - end - end - - def with_lease - lease_uuid = lease_try_obtain - raise NoSessionError unless lease_uuid - - begin - yield - ensure - Gitlab::ExclusiveLease.cancel(lease_key, lease_uuid) - end - end - - def lease_key - "mattermost:session" - end - - def lease_try_obtain - lease = ::Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT) - lease.try_obtain - end - - def handle_exceptions - yield - rescue HTTParty::Error => e - raise Mattermost::ConnectionError.new(e.message) - rescue Errno::ECONNREFUSED - raise Mattermost::ConnectionError.new(e.message) - end - end -end diff --git a/lib/mattermost/team.rb b/lib/mattermost/team.rb deleted file mode 100644 index 784eca6ab5a..00000000000 --- a/lib/mattermost/team.rb +++ /dev/null @@ -1,7 +0,0 @@ -module Mattermost - class Team < Client - def all - json_get('/api/v3/teams/all') - end - end -end diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb index a2d84977f58..b634df52b68 100644 --- a/spec/lib/gitlab/chat_commands/command_spec.rb +++ b/spec/lib/gitlab/chat_commands/command_spec.rb @@ -5,19 +5,7 @@ describe Gitlab::ChatCommands::Command, service: true do let(:user) { create(:user) } describe '#execute' do - subject do - described_class.new(project, user, params).execute - end - - context 'when no command is available' do - let(:params) { { text: 'issue show 1' } } - let(:project) { create(:project, has_external_issue_tracker: true) } - - it 'displays 404 messages' do - expect(subject[:response_type]).to be(:ephemeral) - expect(subject[:text]).to start_with('404 not found') - end - end + subject { described_class.new(project, user, params).execute } context 'when an unknown command is triggered' do let(:params) { { command: '/gitlab', text: "unknown command 123" } } @@ -34,47 +22,7 @@ describe Gitlab::ChatCommands::Command, service: true do it 'rejects the actions' do expect(subject[:response_type]).to be(:ephemeral) - expect(subject[:text]).to start_with('Whoops! That action is not allowed') - end - end - - context 'issue is successfully created' do - let(:params) { { text: "issue create my new issue" } } - - before do - project.team << [user, :master] - end - - it 'presents the issue' do - expect(subject[:text]).to match("my new issue") - end - - it 'shows a link to the new issue' do - expect(subject[:text]).to match(/\/issues\/\d+/) - end - end - - context 'searching for an issue' do - let(:params) { { text: 'issue search find me' } } - let!(:issue) { create(:issue, project: project, title: 'find me') } - - before do - project.team << [user, :master] - end - - context 'a single issue is found' do - it 'presents the issue' do - expect(subject[:text]).to match(issue.title) - end - end - - context 'multiple issues found' do - let!(:issue2) { create(:issue, project: project, title: "someone find me") } - - it 'shows a link to the new issue' do - expect(subject[:text]).to match(issue.title) - expect(subject[:text]).to match(issue2.title) - end + expect(subject[:text]).to start_with('Whoops! This action is not allowed') end end @@ -90,7 +38,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 @@ -100,7 +48,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 diff --git a/spec/lib/gitlab/chat_commands/deploy_spec.rb b/spec/lib/gitlab/chat_commands/deploy_spec.rb index bd8099c92da..b3358a32161 100644 --- a/spec/lib/gitlab/chat_commands/deploy_spec.rb +++ b/spec/lib/gitlab/chat_commands/deploy_spec.rb @@ -15,8 +15,9 @@ describe Gitlab::ChatCommands::Deploy, service: true do end context 'if no environment is defined' do - it 'returns nil' do - expect(subject).to be_nil + it 'does not execute an action' do + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to eq("No action found to be executed") end end @@ -26,8 +27,9 @@ describe Gitlab::ChatCommands::Deploy, service: true do let!(:deployment) { create(:deployment, environment: staging, deployable: build) } context 'without actions' do - it 'returns nil' do - expect(subject).to be_nil + it 'does not execute an action' do + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to eq("No action found to be executed") end end @@ -37,8 +39,8 @@ describe Gitlab::ChatCommands::Deploy, service: true do end it 'returns success result' do - expect(subject.type).to eq(:success) - expect(subject.message).to include('Deployment from staging to production started') + expect(subject[:response_type]).to be(:in_channel) + expect(subject[:text]).to start_with('Deployment started from staging to production') end context 'when duplicate action exists' do @@ -47,8 +49,8 @@ describe Gitlab::ChatCommands::Deploy, service: true do end it 'returns error' do - expect(subject.type).to eq(:error) - expect(subject.message).to include('Too many actions defined') + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to eq('Too many actions defined') end end @@ -59,9 +61,9 @@ describe Gitlab::ChatCommands::Deploy, service: true do name: 'teardown', environment: 'production') end - it 'returns success result' do - expect(subject.type).to eq(:success) - expect(subject.message).to include('Deployment from staging to production started') + it 'returns the success message' do + expect(subject[:response_type]).to be(:in_channel) + expect(subject[:text]).to start_with('Deployment started from staging to production') end end end diff --git a/spec/lib/gitlab/chat_commands/issue_create_spec.rb b/spec/lib/gitlab/chat_commands/issue_create_spec.rb index 6c71e79ff6d..0f84b19a5a4 100644 --- a/spec/lib/gitlab/chat_commands/issue_create_spec.rb +++ b/spec/lib/gitlab/chat_commands/issue_create_spec.rb @@ -18,7 +18,7 @@ describe Gitlab::ChatCommands::IssueCreate, service: true do it 'creates the issue' do expect { subject }.to change { project.issues.count }.by(1) - expect(subject.title).to eq('bird is the word') + expect(subject[:response_type]).to be(:in_channel) end end @@ -41,6 +41,16 @@ describe Gitlab::ChatCommands::IssueCreate, service: true do expect { subject }.to change { project.issues.count }.by(1) end end + + context 'issue cannot be created' do + let!(:issue) { create(:issue, project: project, title: 'bird is the word') } + let(:regex_match) { described_class.match("issue create #{'a' * 512}}") } + + it 'displays the errors' do + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to match("- Title is too long") + end + end end describe '.match' do diff --git a/spec/lib/gitlab/chat_commands/issue_search_spec.rb b/spec/lib/gitlab/chat_commands/issue_search_spec.rb index 24c06a967fa..04d10ad52a1 100644 --- a/spec/lib/gitlab/chat_commands/issue_search_spec.rb +++ b/spec/lib/gitlab/chat_commands/issue_search_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' describe Gitlab::ChatCommands::IssueSearch, service: true do describe '#execute' do - let!(:issue) { create(:issue, title: 'find me') } + let!(:issue) { create(:issue, project: project, title: 'find me') } let!(:confidential) { create(:issue, :confidential, project: project, title: 'mepmep find') } - let(:project) { issue.project } + let(:project) { create(:empty_project) } let(:user) { issue.author } let(:regex_match) { described_class.match("issue search find") } @@ -14,7 +14,8 @@ describe Gitlab::ChatCommands::IssueSearch, service: true do context 'when the user has no access' do it 'only returns the open issues' do - expect(subject).not_to include(confidential) + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to match("not found") end end @@ -24,13 +25,14 @@ describe Gitlab::ChatCommands::IssueSearch, service: true do end it 'returns all results' do - expect(subject).to include(confidential, issue) + expect(subject).to have_key(:attachments) + expect(subject[:text]).to match("Here are the issues I found:") end end context 'without hits on the query' do it 'returns an empty collection' do - expect(subject).to be_empty + expect(subject[:text]).to match("not found") end end end diff --git a/spec/lib/gitlab/chat_commands/issue_show_spec.rb b/spec/lib/gitlab/chat_commands/issue_show_spec.rb index 2eab73e49e5..89932c395c6 100644 --- a/spec/lib/gitlab/chat_commands/issue_show_spec.rb +++ b/spec/lib/gitlab/chat_commands/issue_show_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe Gitlab::ChatCommands::IssueShow, service: true do describe '#execute' do - let(:issue) { create(:issue) } - let(:project) { issue.project } + let(:issue) { create(:issue, project: project) } + let(:project) { create(:empty_project) } let(:user) { issue.author } let(:regex_match) { described_class.match("issue show #{issue.iid}") } @@ -16,15 +16,19 @@ describe Gitlab::ChatCommands::IssueShow, service: true do end context 'the issue exists' do + let(:title) { subject[:attachments].first[:title] } + it 'returns the issue' do - expect(subject.iid).to be issue.iid + expect(subject[:response_type]).to be(:in_channel) + expect(title).to eq(issue.title) end context 'when its reference is given' do let(:regex_match) { described_class.match("issue show #{issue.to_reference}") } it 'shows the issue' do - expect(subject.iid).to be issue.iid + expect(subject[:response_type]).to be(:in_channel) + expect(title).to eq(issue.title) end end end @@ -32,17 +36,24 @@ describe Gitlab::ChatCommands::IssueShow, service: true do context 'the issue does not exist' do let(:regex_match) { described_class.match("issue show 2343242") } - it "returns nil" do - expect(subject).to be_nil + it "returns not found" do + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to match("not found") end end end - describe 'self.match' do + describe '.match' do it 'matches the iid' do match = described_class.match("issue show 123") expect(match[:iid]).to eq("123") end + + it 'accepts a reference' do + match = described_class.match("issue show #{Issue.reference_prefix}123") + + expect(match[:iid]).to eq("123") + end end end diff --git a/spec/lib/gitlab/chat_commands/presenters/access_spec.rb b/spec/lib/gitlab/chat_commands/presenters/access_spec.rb new file mode 100644 index 00000000000..ae41d75ab0c --- /dev/null +++ b/spec/lib/gitlab/chat_commands/presenters/access_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe Gitlab::ChatCommands::Presenters::Access do + describe '#access_denied' do + subject { described_class.new.access_denied } + + it { is_expected.to be_a(Hash) } + + it 'displays an error message' do + expect(subject[:text]).to match("is not allowed") + expect(subject[:response_type]).to be(:ephemeral) + end + end + + describe '#not_found' do + subject { described_class.new.not_found } + + it { is_expected.to be_a(Hash) } + + it 'tells the user the resource was not found' do + expect(subject[:text]).to match("not found!") + expect(subject[:response_type]).to be(:ephemeral) + end + end + + describe '#authorize' do + context 'with an authorization URL' do + subject { described_class.new('http://authorize.me').authorize } + + it { is_expected.to be_a(Hash) } + + it 'tells the user to authorize' do + expect(subject[:text]).to match("connect your GitLab account") + expect(subject[:response_type]).to be(:ephemeral) + end + end + + context 'without authorization url' do + subject { described_class.new.authorize } + + it { is_expected.to be_a(Hash) } + + it 'tells the user to authorize' do + expect(subject[:text]).to match("Couldn't identify you") + expect(subject[:response_type]).to be(:ephemeral) + end + end + end +end diff --git a/spec/lib/gitlab/chat_commands/presenters/deploy_spec.rb b/spec/lib/gitlab/chat_commands/presenters/deploy_spec.rb new file mode 100644 index 00000000000..1c48c727e30 --- /dev/null +++ b/spec/lib/gitlab/chat_commands/presenters/deploy_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe Gitlab::ChatCommands::Presenters::Deploy do + let(:build) { create(:ci_build) } + + describe '#present' do + subject { described_class.new(build).present('staging', 'prod') } + + it { is_expected.to have_key(:text) } + it { is_expected.to have_key(:response_type) } + it { is_expected.to have_key(:status) } + it { is_expected.not_to have_key(:attachments) } + + it 'messages the channel of the deploy' do + expect(subject[:response_type]).to be(:in_channel) + expect(subject[:text]).to start_with("Deployment started from staging to prod") + end + end + + describe '#no_actions' do + subject { described_class.new(nil).no_actions } + + it { is_expected.to have_key(:text) } + it { is_expected.to have_key(:response_type) } + it { is_expected.to have_key(:status) } + it { is_expected.not_to have_key(:attachments) } + + it 'tells the user there is no action' do + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to eq("No action found to be executed") + end + end + + describe '#too_many_actions' do + subject { described_class.new(nil).too_many_actions } + + it { is_expected.to have_key(:text) } + it { is_expected.to have_key(:response_type) } + it { is_expected.to have_key(:status) } + it { is_expected.not_to have_key(:attachments) } + + it 'tells the user there is no action' do + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to eq("Too many actions defined") + end + end +end diff --git a/spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb b/spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb new file mode 100644 index 00000000000..1852395fc97 --- /dev/null +++ b/spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe Gitlab::ChatCommands::Presenters::ListIssues do + let(:project) { create(:empty_project) } + let(:message) { subject[:text] } + let(:issue) { project.issues.first } + + before { create_list(:issue, 2, project: project) } + + subject { described_class.new(project.issues).present } + + it do + is_expected.to have_key(:text) + is_expected.to have_key(:status) + is_expected.to have_key(:response_type) + is_expected.to have_key(:attachments) + end + + it 'shows a list of results' do + expect(subject[:response_type]).to be(:ephemeral) + + expect(message).to start_with("Here are the issues I found") + end +end diff --git a/spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb b/spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb new file mode 100644 index 00000000000..13a318fe680 --- /dev/null +++ b/spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe Gitlab::ChatCommands::Presenters::ShowIssue do + let(:project) { create(:empty_project) } + let(:issue) { create(:issue, project: project) } + let(:attachment) { subject[:attachments].first } + + subject { described_class.new(issue).present } + + it { is_expected.to be_a(Hash) } + + it 'shows the issue' do + expect(subject[:response_type]).to be(:in_channel) + expect(subject).to have_key(:attachments) + expect(attachment[:title]).to eq(issue.title) + end + + context 'with upvotes' do + before do + create(:award_emoji, :upvote, awardable: issue) + end + + it 'shows the upvote count' do + expect(attachment[:text]).to start_with(":+1: 1") + end + end +end diff --git a/spec/lib/mattermost/client_spec.rb b/spec/lib/mattermost/client_spec.rb deleted file mode 100644 index dc11a414717..00000000000 --- a/spec/lib/mattermost/client_spec.rb +++ /dev/null @@ -1,24 +0,0 @@ -require 'spec_helper' - -describe Mattermost::Client do - let(:user) { build(:user) } - - subject { described_class.new(user) } - - context 'JSON parse error' do - before do - Struct.new("Request", :body, :success?) - end - - it 'yields an error on malformed JSON' do - bad_json = Struct::Request.new("I'm not json", true) - expect { subject.send(:json_response, bad_json) }.to raise_error(Mattermost::ClientError) - end - - it 'shows a client error if the request was unsuccessful' do - bad_request = Struct::Request.new("true", false) - - expect { subject.send(:json_response, bad_request) }.to raise_error(Mattermost::ClientError) - end - end -end diff --git a/spec/lib/mattermost/command_spec.rb b/spec/lib/mattermost/command_spec.rb deleted file mode 100644 index 5ccf1100898..00000000000 --- a/spec/lib/mattermost/command_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -require 'spec_helper' - -describe Mattermost::Command do - let(:params) { { 'token' => 'token', team_id: 'abc' } } - - before do - Mattermost::Session.base_uri('http://mattermost.example.com') - - allow_any_instance_of(Mattermost::Client).to receive(:with_session). - and_yield(Mattermost::Session.new(nil)) - end - - describe '#create' do - let(:params) do - { team_id: 'abc', - trigger: 'gitlab' - } - end - - subject { described_class.new(nil).create(params) } - - context 'for valid trigger word' do - before do - stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create'). - with(body: { - team_id: 'abc', - trigger: 'gitlab' }.to_json). - to_return( - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: { token: 'token' }.to_json - ) - end - - it 'returns a token' do - is_expected.to eq('token') - end - end - - context 'for error message' do - before do - stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create'). - to_return( - status: 500, - headers: { 'Content-Type' => 'application/json' }, - body: { - id: 'api.command.duplicate_trigger.app_error', - message: 'This trigger word is already in use. Please choose another word.', - detailed_error: '', - request_id: 'obc374man7bx5r3dbc1q5qhf3r', - status_code: 500 - }.to_json - ) - end - - it 'raises an error with message' do - expect { subject }.to raise_error(Mattermost::Error, 'This trigger word is already in use. Please choose another word.') - end - end - end -end diff --git a/spec/lib/mattermost/session_spec.rb b/spec/lib/mattermost/session_spec.rb deleted file mode 100644 index 74d12e37181..00000000000 --- a/spec/lib/mattermost/session_spec.rb +++ /dev/null @@ -1,123 +0,0 @@ -require 'spec_helper' - -describe Mattermost::Session, type: :request do - let(:user) { create(:user) } - - let(:gitlab_url) { "http://gitlab.com" } - let(:mattermost_url) { "http://mattermost.com" } - - subject { described_class.new(user) } - - # Needed for doorkeeper to function - it { is_expected.to respond_to(:current_resource_owner) } - it { is_expected.to respond_to(:request) } - it { is_expected.to respond_to(:authorization) } - it { is_expected.to respond_to(:strategy) } - - before do - described_class.base_uri(mattermost_url) - end - - describe '#with session' do - let(:location) { 'http://location.tld' } - let!(:stub) do - WebMock.stub_request(:get, "#{mattermost_url}/api/v3/oauth/gitlab/login"). - to_return(headers: { 'location' => location }, status: 307) - end - - context 'without oauth uri' do - it 'makes a request to the oauth uri' do - expect { subject.with_session }.to raise_error(Mattermost::NoSessionError) - end - end - - context 'with oauth_uri' do - let!(:doorkeeper) do - Doorkeeper::Application.create( - name: "GitLab Mattermost", - redirect_uri: "#{mattermost_url}/signup/gitlab/complete\n#{mattermost_url}/login/gitlab/complete", - scopes: "") - end - - context 'without token_uri' do - it 'can not create a session' do - expect do - subject.with_session - end.to raise_error(Mattermost::NoSessionError) - end - end - - context 'with token_uri' do - let(:state) { "state" } - let(:params) do - { response_type: "code", - client_id: doorkeeper.uid, - redirect_uri: "#{mattermost_url}/signup/gitlab/complete", - state: state } - end - let(:location) do - "#{gitlab_url}/oauth/authorize?#{URI.encode_www_form(params)}" - end - - before do - WebMock.stub_request(:get, "#{mattermost_url}/signup/gitlab/complete"). - with(query: hash_including({ 'state' => state })). - to_return do |request| - post "/oauth/token", - client_id: doorkeeper.uid, - client_secret: doorkeeper.secret, - redirect_uri: params[:redirect_uri], - grant_type: 'authorization_code', - code: request.uri.query_values['code'] - - if response.status == 200 - { headers: { 'token' => 'thisworksnow' }, status: 202 } - end - end - - WebMock.stub_request(:post, "#{mattermost_url}/api/v3/users/logout"). - to_return(headers: { Authorization: 'token thisworksnow' }, status: 200) - end - - it 'can setup a session' do - subject.with_session do |session| - end - - expect(subject.token).not_to be_nil - end - - it 'returns the value of the block' do - result = subject.with_session do |session| - "value" - end - - expect(result).to eq("value") - end - end - end - - context 'with lease' do - before do - allow(subject).to receive(:lease_try_obtain).and_return('aldkfjsldfk') - end - - it 'tries to obtain a lease' do - expect(subject).to receive(:lease_try_obtain) - expect(Gitlab::ExclusiveLease).to receive(:cancel) - - # Cannot setup a session, but we should still cancel the lease - expect { subject.with_session }.to raise_error(Mattermost::NoSessionError) - end - end - - context 'without lease' do - before do - allow(subject).to receive(:lease_try_obtain).and_return(nil) - end - - it 'returns a NoSessionError error' do - expect { subject.with_session }.to raise_error(Mattermost::NoSessionError) - end - end - end -end diff --git a/spec/lib/mattermost/team_spec.rb b/spec/lib/mattermost/team_spec.rb deleted file mode 100644 index 2d14be6bcc2..00000000000 --- a/spec/lib/mattermost/team_spec.rb +++ /dev/null @@ -1,66 +0,0 @@ -require 'spec_helper' - -describe Mattermost::Team do - before do - Mattermost::Session.base_uri('http://mattermost.example.com') - - allow_any_instance_of(Mattermost::Client).to receive(:with_session). - and_yield(Mattermost::Session.new(nil)) - end - - describe '#all' do - subject { described_class.new(nil).all } - - context 'for valid request' do - let(:response) do - [{ - "id" => "xiyro8huptfhdndadpz8r3wnbo", - "create_at" => 1482174222155, - "update_at" => 1482174222155, - "delete_at" => 0, - "display_name" => "chatops", - "name" => "chatops", - "email" => "admin@example.com", - "type" => "O", - "company_name" => "", - "allowed_domains" => "", - "invite_id" => "o4utakb9jtb7imctdfzbf9r5ro", - "allow_open_invite" => false }] - end - - before do - stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all'). - to_return( - status: 200, - headers: { 'Content-Type' => 'application/json' }, - body: response.to_json - ) - end - - it 'returns a token' do - is_expected.to eq(response) - end - end - - context 'for error message' do - before do - stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all'). - to_return( - status: 500, - headers: { 'Content-Type' => 'application/json' }, - body: { - id: 'api.team.list.app_error', - message: 'Cannot list teams.', - detailed_error: '', - request_id: 'obc374man7bx5r3dbc1q5qhf3r', - status_code: 500 - }.to_json - ) - end - - it 'raises an error with message' do - expect { subject }.to raise_error(Mattermost::Error, 'Cannot list teams.') - end - end - end -end -- cgit v1.2.1 From 19c55a47b77f6c63db39a45946dc47f3c95fc744 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" <git@zjvandeweg.nl> Date: Wed, 11 Jan 2017 08:54:44 -0500 Subject: Revert removing of some files --- lib/mattermost/client.rb | 41 ++++++++++++ lib/mattermost/command.rb | 10 +++ lib/mattermost/error.rb | 3 + lib/mattermost/session.rb | 160 ++++++++++++++++++++++++++++++++++++++++++++++ lib/mattermost/team.rb | 7 ++ 5 files changed, 221 insertions(+) create mode 100644 lib/mattermost/client.rb create mode 100644 lib/mattermost/command.rb create mode 100644 lib/mattermost/error.rb create mode 100644 lib/mattermost/session.rb create mode 100644 lib/mattermost/team.rb diff --git a/lib/mattermost/client.rb b/lib/mattermost/client.rb new file mode 100644 index 00000000000..ec2903b7ec6 --- /dev/null +++ b/lib/mattermost/client.rb @@ -0,0 +1,41 @@ +module Mattermost + class ClientError < Mattermost::Error; end + + class Client + attr_reader :user + + def initialize(user) + @user = user + end + + private + + def with_session(&blk) + Mattermost::Session.new(user).with_session(&blk) + end + + def json_get(path, options = {}) + with_session do |session| + json_response session.get(path, options) + end + end + + def json_post(path, options = {}) + with_session do |session| + json_response session.post(path, options) + end + end + + def json_response(response) + json_response = JSON.parse(response.body) + + unless response.success? + raise Mattermost::ClientError.new(json_response['message'] || 'Undefined error') + end + + json_response + rescue JSON::JSONError + raise Mattermost::ClientError.new('Cannot parse response') + end + end +end diff --git a/lib/mattermost/command.rb b/lib/mattermost/command.rb new file mode 100644 index 00000000000..d1e4bb0eccf --- /dev/null +++ b/lib/mattermost/command.rb @@ -0,0 +1,10 @@ +module Mattermost + class Command < Client + def create(params) + response = json_post("/api/v3/teams/#{params[:team_id]}/commands/create", + body: params.to_json) + + response['token'] + end + end +end diff --git a/lib/mattermost/error.rb b/lib/mattermost/error.rb new file mode 100644 index 00000000000..014df175be0 --- /dev/null +++ b/lib/mattermost/error.rb @@ -0,0 +1,3 @@ +module Mattermost + class Error < StandardError; end +end diff --git a/lib/mattermost/session.rb b/lib/mattermost/session.rb new file mode 100644 index 00000000000..377cb7b1021 --- /dev/null +++ b/lib/mattermost/session.rb @@ -0,0 +1,160 @@ +module Mattermost + class NoSessionError < Mattermost::Error + def message + 'No session could be set up, is Mattermost configured with Single Sign On?' + end + end + + class ConnectionError < Mattermost::Error; end + + # This class' prime objective is to obtain a session token on a Mattermost + # instance with SSO configured where this GitLab instance is the provider. + # + # The process depends on OAuth, but skips a step in the authentication cycle. + # For example, usually a user would click the 'login in GitLab' button on + # Mattermost, which would yield a 302 status code and redirects you to GitLab + # to approve the use of your account on Mattermost. Which would trigger a + # callback so Mattermost knows this request is approved and gets the required + # data to create the user account etc. + # + # This class however skips the button click, and also the approval phase to + # speed up the process and keep it without manual action and get a session + # going. + class Session + include Doorkeeper::Helpers::Controller + include HTTParty + + LEASE_TIMEOUT = 60 + + base_uri Settings.mattermost.host + + attr_accessor :current_resource_owner, :token + + def initialize(current_user) + @current_resource_owner = current_user + end + + def with_session + with_lease do + raise Mattermost::NoSessionError unless create + + begin + yield self + rescue Errno::ECONNREFUSED + raise Mattermost::NoSessionError + ensure + destroy + end + end + end + + # Next methods are needed for Doorkeeper + def pre_auth + @pre_auth ||= Doorkeeper::OAuth::PreAuthorization.new( + Doorkeeper.configuration, server.client_via_uid, params) + end + + def authorization + @authorization ||= strategy.request + end + + def strategy + @strategy ||= server.authorization_request(pre_auth.response_type) + end + + def request + @request ||= OpenStruct.new(parameters: params) + end + + def params + Rack::Utils.parse_query(oauth_uri.query).symbolize_keys + end + + def get(path, options = {}) + handle_exceptions do + self.class.get(path, options.merge(headers: @headers)) + end + end + + def post(path, options = {}) + handle_exceptions do + self.class.post(path, options.merge(headers: @headers)) + end + end + + private + + def create + return unless oauth_uri + return unless token_uri + + @token = request_token + @headers = { + Authorization: "Bearer #{@token}" + } + + @token + end + + def destroy + post('/api/v3/users/logout') + end + + def oauth_uri + return @oauth_uri if defined?(@oauth_uri) + + @oauth_uri = nil + + response = get("/api/v3/oauth/gitlab/login", follow_redirects: false) + return unless 300 <= response.code && response.code < 400 + + redirect_uri = response.headers['location'] + return unless redirect_uri + + @oauth_uri = URI.parse(redirect_uri) + end + + def token_uri + @token_uri ||= + if oauth_uri + authorization.authorize.redirect_uri if pre_auth.authorizable? + end + end + + def request_token + response = get(token_uri, follow_redirects: false) + + if 200 <= response.code && response.code < 400 + response.headers['token'] + end + end + + def with_lease + lease_uuid = lease_try_obtain + raise NoSessionError unless lease_uuid + + begin + yield + ensure + Gitlab::ExclusiveLease.cancel(lease_key, lease_uuid) + end + end + + def lease_key + "mattermost:session" + end + + def lease_try_obtain + lease = ::Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT) + lease.try_obtain + end + + def handle_exceptions + yield + rescue HTTParty::Error => e + raise Mattermost::ConnectionError.new(e.message) + rescue Errno::ECONNREFUSED + raise Mattermost::ConnectionError.new(e.message) + end + end +end diff --git a/lib/mattermost/team.rb b/lib/mattermost/team.rb new file mode 100644 index 00000000000..784eca6ab5a --- /dev/null +++ b/lib/mattermost/team.rb @@ -0,0 +1,7 @@ +module Mattermost + class Team < Client + def all + json_get('/api/v3/teams/all') + end + end +end -- cgit v1.2.1 From e96ddef3deaac499bcdd67800839e59f46ae67b5 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" <git@zjvandeweg.nl> Date: Wed, 11 Jan 2017 09:04:49 -0500 Subject: Add help command --- lib/gitlab/chat_commands/command.rb | 13 +++++-------- lib/gitlab/chat_commands/help.rb | 28 ++++++++++++++++++++++++++++ lib/gitlab/chat_commands/presenters/help.rb | 20 ++++++++++++++++++++ 3 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 lib/gitlab/chat_commands/help.rb create mode 100644 lib/gitlab/chat_commands/presenters/help.rb diff --git a/lib/gitlab/chat_commands/command.rb b/lib/gitlab/chat_commands/command.rb index ac7ee868402..4e5031a8a26 100644 --- a/lib/gitlab/chat_commands/command.rb +++ b/lib/gitlab/chat_commands/command.rb @@ -18,25 +18,22 @@ module Gitlab Gitlab::ChatCommands::Presenters::Access.new.access_denied end else - help(help_messages) + Gitlab::ChatCommands::Help.new(project, current_user, params).execute(available_commands) end end def match_command match = nil - service = available_commands.find do |klass| - match = klass.match(params[:text]) - end + service = + available_commands.find do |klass| + match = klass.match(params[:text]) + end [service, match] end private - def help_messages - available_commands.map(&:help_message) - end - def available_commands COMMANDS.select do |klass| klass.available?(project) 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/help.rb b/lib/gitlab/chat_commands/presenters/help.rb new file mode 100644 index 00000000000..133b707231f --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/help.rb @@ -0,0 +1,20 @@ +module Gitlab::ChatCommands::Presenters + class Help < Gitlab::ChatCommands::Presenters::Base + def present(trigger) + message = + if @resource.none? + "No commands available :thinking_face:" + else + header_with_list("Available commands", full_commands(trigger)) + end + + ephemeral_response(text: message) + end + + private + + def full_commands(trigger) + @resource.map { |command| "#{trigger} #{command.help_message}" } + end + end +end -- cgit v1.2.1 From e9d163865bc6f987e7fa277536c6bc3950fbf1e0 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" <git@zjvandeweg.nl> Date: Thu, 12 Jan 2017 09:04:21 -0500 Subject: Fix tests --- app/models/project_services/chat_slash_commands_service.rb | 8 ++------ 1 file changed, 2 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 608754f3035..5eb1bd86e9d 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 access_presenter unless valid_token?(params[:token]) + return unless valid_token?(params[:token]) user = find_chat_user(params) @@ -36,16 +36,12 @@ class ChatSlashCommandsService < Service Gitlab::ChatCommands::Command.new(project, user, params).execute else url = authorize_chat_name_url(params) - access_presenter(url).authorize + Gitlab::ChatCommands::Presenters::Access.new(url).authorize end end private - def access_presenter(url = nil) - Gitlab::ChatCommands::Presenters::Access.new(url) - end - def find_chat_user(params) ChatNames::FindUserService.new(self, params).execute end -- cgit v1.2.1 From 52ca0d2c9ecb7e7d4d0130f19bf0fb7b772a357d Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" <git@zjvandeweg.nl> Date: Thu, 19 Jan 2017 09:22:09 +0100 Subject: Incorporate feedback --- changelogs/unreleased/zj-format-chat-messages.yml | 4 + lib/gitlab/chat_commands/issue_create.rb | 2 +- lib/gitlab/chat_commands/presenters/access.rb | 36 +++--- lib/gitlab/chat_commands/presenters/base.rb | 112 ++++++++++--------- lib/gitlab/chat_commands/presenters/deploy.rb | 39 ++++--- lib/gitlab/chat_commands/presenters/help.rb | 31 +++--- lib/gitlab/chat_commands/presenters/issuable.rb | 66 ++++++----- lib/gitlab/chat_commands/presenters/list_issues.rb | 59 ++++++---- lib/gitlab/chat_commands/presenters/new_issue.rb | 42 +++++++ lib/gitlab/chat_commands/presenters/show_issue.rb | 72 +++++++----- spec/lib/gitlab/chat_commands/issue_search_spec.rb | 2 +- spec/lib/gitlab/chat_commands/issue_show_spec.rb | 4 +- .../gitlab/chat_commands/presenters/deploy_spec.rb | 2 +- .../chat_commands/presenters/list_issues_spec.rb | 5 +- .../chat_commands/presenters/show_issue_spec.rb | 4 +- spec/lib/mattermost/client_spec.rb | 24 ++++ spec/lib/mattermost/command_spec.rb | 61 ++++++++++ spec/lib/mattermost/session_spec.rb | 123 +++++++++++++++++++++ spec/lib/mattermost/team_spec.rb | 66 +++++++++++ 19 files changed, 565 insertions(+), 189 deletions(-) create mode 100644 changelogs/unreleased/zj-format-chat-messages.yml create mode 100644 lib/gitlab/chat_commands/presenters/new_issue.rb create mode 100644 spec/lib/mattermost/client_spec.rb create mode 100644 spec/lib/mattermost/command_spec.rb create mode 100644 spec/lib/mattermost/session_spec.rb create mode 100644 spec/lib/mattermost/team_spec.rb diff --git a/changelogs/unreleased/zj-format-chat-messages.yml b/changelogs/unreleased/zj-format-chat-messages.yml new file mode 100644 index 00000000000..2494884f5c9 --- /dev/null +++ b/changelogs/unreleased/zj-format-chat-messages.yml @@ -0,0 +1,4 @@ +--- +title: Reformat messages ChatOps +merge_request: 8528 +author: diff --git a/lib/gitlab/chat_commands/issue_create.rb b/lib/gitlab/chat_commands/issue_create.rb index a06f13b0f72..3f3d7de8b2e 100644 --- a/lib/gitlab/chat_commands/issue_create.rb +++ b/lib/gitlab/chat_commands/issue_create.rb @@ -35,7 +35,7 @@ module Gitlab end def presenter(issue) - Gitlab::ChatCommands::Presenters::ShowIssue.new(issue) + Gitlab::ChatCommands::Presenters::NewIssue.new(issue) end end end diff --git a/lib/gitlab/chat_commands/presenters/access.rb b/lib/gitlab/chat_commands/presenters/access.rb index 6d18d745608..b66ef48d6a8 100644 --- a/lib/gitlab/chat_commands/presenters/access.rb +++ b/lib/gitlab/chat_commands/presenters/access.rb @@ -1,22 +1,26 @@ -module Gitlab::ChatCommands::Presenters - class Access < Gitlab::ChatCommands::Presenters::Base - def access_denied - ephemeral_response(text: "Whoops! This action is not allowed. This incident will be [reported](https://xkcd.com/838/).") - end - - def not_found - ephemeral_response(text: "404 not found! GitLab couldn't find what you were looking for! :boom:") - end +module Gitlab + module ChatCommands + module Presenters + class Access < Presenters::Base + def access_denied + ephemeral_response(text: "Whoops! This action is not allowed. This incident will be [reported](https://xkcd.com/838/).") + end - def authorize - message = - if @resource - ":wave: Hi there! Before I do anything for you, please [connect your GitLab account](#{@resource})." - else - ":sweat_smile: Couldn't identify you, nor can I autorize you!" + def not_found + ephemeral_response(text: "404 not found! GitLab couldn't find what you were looking for! :boom:") end - ephemeral_response(text: message) + def authorize + message = + if @resource + ":wave: Hi there! Before I do anything for you, please [connect your GitLab account](#{@resource})." + else + ":sweat_smile: Couldn't identify you, nor can I autorize you!" + end + + ephemeral_response(text: message) + end + end end end end diff --git a/lib/gitlab/chat_commands/presenters/base.rb b/lib/gitlab/chat_commands/presenters/base.rb index 0897025d85f..2700a5a2ad5 100644 --- a/lib/gitlab/chat_commands/presenters/base.rb +++ b/lib/gitlab/chat_commands/presenters/base.rb @@ -1,73 +1,77 @@ -module Gitlab::ChatCommands::Presenters - class Base - include Gitlab::Routing.url_helpers +module Gitlab + module ChatCommands + module Presenters + class Base + include Gitlab::Routing.url_helpers + + def initialize(resource = nil) + @resource = resource + end - def initialize(resource = nil) - @resource = resource - end + def display_errors + message = header_with_list("The action was not successful, because:", @resource.errors.full_messages) - def display_errors - message = header_with_list("The action was not successful, because:", @resource.errors.full_messages) + ephemeral_response(text: message) + end - ephemeral_response(text: message) - end + private - private + def header_with_list(header, items) + message = [header] - def header_with_list(header, items) - message = [header] + items.each do |item| + message << "- #{item}" + end - items.each do |item| - message << "- #{item}" - end + message.join("\n") + end - message.join("\n") - end + def ephemeral_response(message) + response = { + response_type: :ephemeral, + status: 200 + }.merge(message) - def ephemeral_response(message) - response = { - response_type: :ephemeral, - status: 200 - }.merge(message) + format_response(response) + end - format_response(response) - end + def in_channel_response(message) + response = { + response_type: :in_channel, + status: 200 + }.merge(message) - def in_channel_response(message) - response = { - response_type: :in_channel, - status: 200 - }.merge(message) + format_response(response) + end - format_response(response) - end + def format_response(response) + response[:text] = format(response[:text]) if response.has_key?(:text) - def format_response(response) - response[:text] = format(response[:text]) if response.has_key?(:text) + if response.has_key?(:attachments) + response[:attachments].each do |attachment| + attachment[:pretext] = format(attachment[:pretext]) if attachment[:pretext] + attachment[:text] = format(attachment[:text]) if attachment[:text] + end + end - if response.has_key?(:attachments) - response[:attachments].each do |attachment| - attachment[:pretext] = format(attachment[:pretext]) if attachment[:pretext] - attachment[:text] = format(attachment[:text]) if attachment[:text] + response end - end - - response - end - # Convert Markdown to slacks format - def format(string) - Slack::Notifier::LinkFormatter.format(string) - end + # Convert Markdown to slacks format + def format(string) + Slack::Notifier::LinkFormatter.format(string) + end - def resource_url - url_for( - [ - @resource.project.namespace.becomes(Namespace), - @resource.project, - @resource - ] - ) + def resource_url + url_for( + [ + @resource.project.namespace.becomes(Namespace), + @resource.project, + @resource + ] + ) + end + end end end end diff --git a/lib/gitlab/chat_commands/presenters/deploy.rb b/lib/gitlab/chat_commands/presenters/deploy.rb index 4f6333812ff..b1cfaac15af 100644 --- a/lib/gitlab/chat_commands/presenters/deploy.rb +++ b/lib/gitlab/chat_commands/presenters/deploy.rb @@ -1,24 +1,29 @@ -module Gitlab::ChatCommands::Presenters - class Deploy < Gitlab::ChatCommands::Presenters::Base - def present(from, to) - message = "Deployment started from #{from} to #{to}. [Follow its progress](#{resource_url})." - in_channel_response(text: message) - end +module Gitlab + module ChatCommands + module Presenters + class Deploy < Presenters::Base + def present(from, to) + message = "Deployment started from #{from} to #{to}. [Follow its progress](#{resource_url})." - def no_actions - ephemeral_response(text: "No action found to be executed") - end + in_channel_response(text: message) + end - def too_many_actions - ephemeral_response(text: "Too many actions defined") - end + def no_actions + ephemeral_response(text: "No action found to be executed") + end + + def too_many_actions + ephemeral_response(text: "Too many actions defined") + end - private + private - def resource_url - polymorphic_url( - [ @resource.project.namespace.becomes(Namespace), @resource.project, @resource] - ) + def resource_url + polymorphic_url( + [ @resource.project.namespace.becomes(Namespace), @resource.project, @resource] + ) + end + end end end end diff --git a/lib/gitlab/chat_commands/presenters/help.rb b/lib/gitlab/chat_commands/presenters/help.rb index 133b707231f..c7a67467b7e 100644 --- a/lib/gitlab/chat_commands/presenters/help.rb +++ b/lib/gitlab/chat_commands/presenters/help.rb @@ -1,20 +1,25 @@ -module Gitlab::ChatCommands::Presenters - class Help < Gitlab::ChatCommands::Presenters::Base - def present(trigger) - message = - if @resource.none? - "No commands available :thinking_face:" - else - header_with_list("Available commands", full_commands(trigger)) +module Gitlab + module ChatCommands + module Presenters + class Help < Presenters::Base + def present(trigger) + ephemeral_response(text: help_message(trigger)) end - ephemeral_response(text: message) - end + private - private + def help_message(trigger) + if @resource.none? + "No commands available :thinking_face:" + else + header_with_list("Available commands", full_commands(trigger)) + end + end - def full_commands(trigger) - @resource.map { |command| "#{trigger} #{command.help_message}" } + def full_commands(trigger) + @resource.map { |command| "#{trigger} #{command.help_message}" } + end + end end end end diff --git a/lib/gitlab/chat_commands/presenters/issuable.rb b/lib/gitlab/chat_commands/presenters/issuable.rb index 9623387f188..2cb6b1525fc 100644 --- a/lib/gitlab/chat_commands/presenters/issuable.rb +++ b/lib/gitlab/chat_commands/presenters/issuable.rb @@ -1,33 +1,45 @@ -module Gitlab::ChatCommands::Presenters - class Issuable < Gitlab::ChatCommands::Presenters::Base - private +module Gitlab + module ChatCommands + module Presenters + class Issuable < Presenters::Base + private - def project - @resource.project - end + def color(issuable) + issuable.open? ? '#38ae67' : '#d22852' + end - def author - @resource.author - end + def status_text(issuable) + issuable.open? ? 'Open' : 'Closed' + end + + def project + @resource.project + end + + def author + @resource.author + end - def fields - [ - { - title: "Assignee", - value: @resource.assignee ? @resource.assignee.name : "_None_", - short: true - }, - { - title: "Milestone", - value: @resource.milestone ? @resource.milestone.title : "_None_", - short: true - }, - { - title: "Labels", - value: @resource.labels.any? ? @resource.label_names : "_None_", - short: true - } - ] + def fields + [ + { + title: "Assignee", + value: @resource.assignee ? @resource.assignee.name : "_None_", + short: true + }, + { + title: "Milestone", + value: @resource.milestone ? @resource.milestone.title : "_None_", + short: true + }, + { + title: "Labels", + value: @resource.labels.any? ? @resource.label_names : "_None_", + short: true + } + ] + end + end end end end diff --git a/lib/gitlab/chat_commands/presenters/list_issues.rb b/lib/gitlab/chat_commands/presenters/list_issues.rb index 5a7b3fca5c2..2458b9356b7 100644 --- a/lib/gitlab/chat_commands/presenters/list_issues.rb +++ b/lib/gitlab/chat_commands/presenters/list_issues.rb @@ -1,32 +1,43 @@ -module Gitlab::ChatCommands::Presenters - class ListIssues < Gitlab::ChatCommands::Presenters::Base - def present - ephemeral_response(text: "Here are the issues I found:", attachments: attachments) - end +module Gitlab + module ChatCommands + module Presenters + class ListIssues < Presenters::Issuable + def present + text = if @resource.count >= 5 + "Here are the first 5 issues I found:" + else + "Here are the #{@resource.count} issues I found:" + end - private + ephemeral_response(text: text, attachments: attachments) + end - def attachments - @resource.map do |issue| - state = issue.open? ? "Open" : "Closed" + private - { - fallback: "Issue #{issue.to_reference}: #{issue.title}", - color: "#d22852", - text: "[#{issue.to_reference}](#{url_for([namespace, project, issue])}) · #{issue.title} (#{state})", - mrkdwn_in: [ - "text" - ] - } - end - end + def attachments + @resource.map do |issue| + url = "[#{issue.to_reference}](#{url_for([namespace, project, issue])})" - def project - @project ||= @resource.first.project - end + { + color: color(issue), + fallback: "#{issue.to_reference} #{issue.title}", + text: "#{url} · #{issue.title} (#{status_text(issue)})", - def namespace - @namespace ||= project.namespace.becomes(Namespace) + mrkdwn_in: [ + "text" + ] + } + end + end + + def project + @project ||= @resource.first.project + end + + def namespace + @namespace ||= project.namespace.becomes(Namespace) + end + end end end end diff --git a/lib/gitlab/chat_commands/presenters/new_issue.rb b/lib/gitlab/chat_commands/presenters/new_issue.rb new file mode 100644 index 00000000000..c7c6febb56e --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/new_issue.rb @@ -0,0 +1,42 @@ +module Gitlab + module ChatCommands + module Presenters + class NewIssue < Presenters::Issuable + def present + in_channel_response(show_issue) + end + + private + + def show_issue + { + attachments: [ + { + title: "#{@resource.title} · #{@resource.to_reference}", + title_link: resource_url, + author_name: author.name, + author_icon: author.avatar_url, + fallback: "New issue #{@resource.to_reference}: #{@resource.title}", + pretext: pretext, + color: color(@resource), + fields: fields, + mrkdwn_in: [ + :title, + :text + ] + } + ] + } + end + + def pretext + "I opened an issue on behalf on #{author_profile_link}: *#{@resource.to_reference}* from #{project.name_with_namespace}" + end + + def author_profile_link + "[#{author.to_reference}](#{url_for(author)})" + end + end + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/show_issue.rb b/lib/gitlab/chat_commands/presenters/show_issue.rb index 2a89c30b972..e5644a4ad7e 100644 --- a/lib/gitlab/chat_commands/presenters/show_issue.rb +++ b/lib/gitlab/chat_commands/presenters/show_issue.rb @@ -1,38 +1,54 @@ -module Gitlab::ChatCommands::Presenters - class ShowIssue < Gitlab::ChatCommands::Presenters::Issuable - def present - in_channel_response(show_issue) - end +module Gitlab + module ChatCommands + module Presenters + class ShowIssue < Presenters::Issuable + def present + in_channel_response(show_issue) + end - private + private - def show_issue - { - attachments: [ + def show_issue { - title: @resource.title, - title_link: resource_url, - author_name: author.name, - author_icon: author.avatar_url, - fallback: "#{@resource.to_reference}: #{@resource.title}", - text: text, - fields: fields, - mrkdwn_in: [ - :title, - :text + attachments: [ + { + title: "#{@resource.title} · #{@resource.to_reference}", + title_link: resource_url, + author_name: author.name, + author_icon: author.avatar_url, + fallback: "New issue #{@resource.to_reference}: #{@resource.title}", + pretext: pretext, + text: text, + color: color(@resource), + fields: fields, + mrkdwn_in: [ + :pretext, + :text + ] + } ] } - ] - } - end + end + + def text + message = "**#{status_text(@resource)}**" + + if @resource.upvotes.zero? && @resource.downvotes.zero? && @resource.user_notes_count.zero? + return message + end + + message << " · " + message << ":+1: #{@resource.upvotes} " unless @resource.upvotes.zero? + message << ":-1: #{@resource.downvotes} " unless @resource.downvotes.zero? + message << ":speech_balloon: #{@resource.user_notes_count}" unless @resource.user_notes_count.zero? - def text - message = "" - message << ":+1: #{@resource.upvotes} " unless @resource.upvotes.zero? - message << ":-1: #{@resource.downvotes} " unless @resource.downvotes.zero? - message << ":speech_balloon: #{@resource.user_notes_count}" unless @resource.user_notes_count.zero? + message + end - message + def pretext + "Issue *#{@resource.to_reference} from #{project.name_with_namespace}" + end + end end end end diff --git a/spec/lib/gitlab/chat_commands/issue_search_spec.rb b/spec/lib/gitlab/chat_commands/issue_search_spec.rb index 04d10ad52a1..551ccb79a58 100644 --- a/spec/lib/gitlab/chat_commands/issue_search_spec.rb +++ b/spec/lib/gitlab/chat_commands/issue_search_spec.rb @@ -26,7 +26,7 @@ describe Gitlab::ChatCommands::IssueSearch, service: true do it 'returns all results' do expect(subject).to have_key(:attachments) - expect(subject[:text]).to match("Here are the issues I found:") + expect(subject[:text]).to eq("Here are the 2 issues I found:") end end diff --git a/spec/lib/gitlab/chat_commands/issue_show_spec.rb b/spec/lib/gitlab/chat_commands/issue_show_spec.rb index 89932c395c6..1f20d0a44ce 100644 --- a/spec/lib/gitlab/chat_commands/issue_show_spec.rb +++ b/spec/lib/gitlab/chat_commands/issue_show_spec.rb @@ -20,7 +20,7 @@ describe Gitlab::ChatCommands::IssueShow, service: true do it 'returns the issue' do expect(subject[:response_type]).to be(:in_channel) - expect(title).to eq(issue.title) + expect(title).to start_with(issue.title) end context 'when its reference is given' do @@ -28,7 +28,7 @@ describe Gitlab::ChatCommands::IssueShow, service: true do it 'shows the issue' do expect(subject[:response_type]).to be(:in_channel) - expect(title).to eq(issue.title) + expect(title).to start_with(issue.title) end end end diff --git a/spec/lib/gitlab/chat_commands/presenters/deploy_spec.rb b/spec/lib/gitlab/chat_commands/presenters/deploy_spec.rb index 1c48c727e30..dc2dd300072 100644 --- a/spec/lib/gitlab/chat_commands/presenters/deploy_spec.rb +++ b/spec/lib/gitlab/chat_commands/presenters/deploy_spec.rb @@ -32,7 +32,7 @@ describe Gitlab::ChatCommands::Presenters::Deploy do end describe '#too_many_actions' do - subject { described_class.new(nil).too_many_actions } + subject { described_class.new([]).too_many_actions } it { is_expected.to have_key(:text) } it { is_expected.to have_key(:response_type) } diff --git a/spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb b/spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb index 1852395fc97..13a1f70fe78 100644 --- a/spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb +++ b/spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb @@ -3,13 +3,12 @@ require 'spec_helper' describe Gitlab::ChatCommands::Presenters::ListIssues do let(:project) { create(:empty_project) } let(:message) { subject[:text] } - let(:issue) { project.issues.first } before { create_list(:issue, 2, project: project) } subject { described_class.new(project.issues).present } - it do + it 'formats the message correct' do is_expected.to have_key(:text) is_expected.to have_key(:status) is_expected.to have_key(:response_type) @@ -19,6 +18,6 @@ describe Gitlab::ChatCommands::Presenters::ListIssues do it 'shows a list of results' do expect(subject[:response_type]).to be(:ephemeral) - expect(message).to start_with("Here are the issues I found") + expect(message).to start_with("Here are the 2 issues I found") end end diff --git a/spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb b/spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb index 13a318fe680..ca4062e692a 100644 --- a/spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb +++ b/spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb @@ -12,7 +12,7 @@ describe Gitlab::ChatCommands::Presenters::ShowIssue do it 'shows the issue' do expect(subject[:response_type]).to be(:in_channel) expect(subject).to have_key(:attachments) - expect(attachment[:title]).to eq(issue.title) + expect(attachment[:title]).to start_with(issue.title) end context 'with upvotes' do @@ -21,7 +21,7 @@ describe Gitlab::ChatCommands::Presenters::ShowIssue do end it 'shows the upvote count' do - expect(attachment[:text]).to start_with(":+1: 1") + expect(attachment[:text]).to start_with("**Open** · :+1: 1") end end end diff --git a/spec/lib/mattermost/client_spec.rb b/spec/lib/mattermost/client_spec.rb new file mode 100644 index 00000000000..dc11a414717 --- /dev/null +++ b/spec/lib/mattermost/client_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe Mattermost::Client do + let(:user) { build(:user) } + + subject { described_class.new(user) } + + context 'JSON parse error' do + before do + Struct.new("Request", :body, :success?) + end + + it 'yields an error on malformed JSON' do + bad_json = Struct::Request.new("I'm not json", true) + expect { subject.send(:json_response, bad_json) }.to raise_error(Mattermost::ClientError) + end + + it 'shows a client error if the request was unsuccessful' do + bad_request = Struct::Request.new("true", false) + + expect { subject.send(:json_response, bad_request) }.to raise_error(Mattermost::ClientError) + end + end +end diff --git a/spec/lib/mattermost/command_spec.rb b/spec/lib/mattermost/command_spec.rb new file mode 100644 index 00000000000..5ccf1100898 --- /dev/null +++ b/spec/lib/mattermost/command_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' + +describe Mattermost::Command do + let(:params) { { 'token' => 'token', team_id: 'abc' } } + + before do + Mattermost::Session.base_uri('http://mattermost.example.com') + + allow_any_instance_of(Mattermost::Client).to receive(:with_session). + and_yield(Mattermost::Session.new(nil)) + end + + describe '#create' do + let(:params) do + { team_id: 'abc', + trigger: 'gitlab' + } + end + + subject { described_class.new(nil).create(params) } + + context 'for valid trigger word' do + before do + stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create'). + with(body: { + team_id: 'abc', + trigger: 'gitlab' }.to_json). + to_return( + status: 200, + headers: { 'Content-Type' => 'application/json' }, + body: { token: 'token' }.to_json + ) + end + + it 'returns a token' do + is_expected.to eq('token') + end + end + + context 'for error message' do + before do + stub_request(:post, 'http://mattermost.example.com/api/v3/teams/abc/commands/create'). + to_return( + status: 500, + headers: { 'Content-Type' => 'application/json' }, + body: { + id: 'api.command.duplicate_trigger.app_error', + message: 'This trigger word is already in use. Please choose another word.', + detailed_error: '', + request_id: 'obc374man7bx5r3dbc1q5qhf3r', + status_code: 500 + }.to_json + ) + end + + it 'raises an error with message' do + expect { subject }.to raise_error(Mattermost::Error, 'This trigger word is already in use. Please choose another word.') + end + end + end +end diff --git a/spec/lib/mattermost/session_spec.rb b/spec/lib/mattermost/session_spec.rb new file mode 100644 index 00000000000..74d12e37181 --- /dev/null +++ b/spec/lib/mattermost/session_spec.rb @@ -0,0 +1,123 @@ +require 'spec_helper' + +describe Mattermost::Session, type: :request do + let(:user) { create(:user) } + + let(:gitlab_url) { "http://gitlab.com" } + let(:mattermost_url) { "http://mattermost.com" } + + subject { described_class.new(user) } + + # Needed for doorkeeper to function + it { is_expected.to respond_to(:current_resource_owner) } + it { is_expected.to respond_to(:request) } + it { is_expected.to respond_to(:authorization) } + it { is_expected.to respond_to(:strategy) } + + before do + described_class.base_uri(mattermost_url) + end + + describe '#with session' do + let(:location) { 'http://location.tld' } + let!(:stub) do + WebMock.stub_request(:get, "#{mattermost_url}/api/v3/oauth/gitlab/login"). + to_return(headers: { 'location' => location }, status: 307) + end + + context 'without oauth uri' do + it 'makes a request to the oauth uri' do + expect { subject.with_session }.to raise_error(Mattermost::NoSessionError) + end + end + + context 'with oauth_uri' do + let!(:doorkeeper) do + Doorkeeper::Application.create( + name: "GitLab Mattermost", + redirect_uri: "#{mattermost_url}/signup/gitlab/complete\n#{mattermost_url}/login/gitlab/complete", + scopes: "") + end + + context 'without token_uri' do + it 'can not create a session' do + expect do + subject.with_session + end.to raise_error(Mattermost::NoSessionError) + end + end + + context 'with token_uri' do + let(:state) { "state" } + let(:params) do + { response_type: "code", + client_id: doorkeeper.uid, + redirect_uri: "#{mattermost_url}/signup/gitlab/complete", + state: state } + end + let(:location) do + "#{gitlab_url}/oauth/authorize?#{URI.encode_www_form(params)}" + end + + before do + WebMock.stub_request(:get, "#{mattermost_url}/signup/gitlab/complete"). + with(query: hash_including({ 'state' => state })). + to_return do |request| + post "/oauth/token", + client_id: doorkeeper.uid, + client_secret: doorkeeper.secret, + redirect_uri: params[:redirect_uri], + grant_type: 'authorization_code', + code: request.uri.query_values['code'] + + if response.status == 200 + { headers: { 'token' => 'thisworksnow' }, status: 202 } + end + end + + WebMock.stub_request(:post, "#{mattermost_url}/api/v3/users/logout"). + to_return(headers: { Authorization: 'token thisworksnow' }, status: 200) + end + + it 'can setup a session' do + subject.with_session do |session| + end + + expect(subject.token).not_to be_nil + end + + it 'returns the value of the block' do + result = subject.with_session do |session| + "value" + end + + expect(result).to eq("value") + end + end + end + + context 'with lease' do + before do + allow(subject).to receive(:lease_try_obtain).and_return('aldkfjsldfk') + end + + it 'tries to obtain a lease' do + expect(subject).to receive(:lease_try_obtain) + expect(Gitlab::ExclusiveLease).to receive(:cancel) + + # Cannot setup a session, but we should still cancel the lease + expect { subject.with_session }.to raise_error(Mattermost::NoSessionError) + end + end + + context 'without lease' do + before do + allow(subject).to receive(:lease_try_obtain).and_return(nil) + end + + it 'returns a NoSessionError error' do + expect { subject.with_session }.to raise_error(Mattermost::NoSessionError) + end + end + end +end diff --git a/spec/lib/mattermost/team_spec.rb b/spec/lib/mattermost/team_spec.rb new file mode 100644 index 00000000000..2d14be6bcc2 --- /dev/null +++ b/spec/lib/mattermost/team_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe Mattermost::Team do + before do + Mattermost::Session.base_uri('http://mattermost.example.com') + + allow_any_instance_of(Mattermost::Client).to receive(:with_session). + and_yield(Mattermost::Session.new(nil)) + end + + describe '#all' do + subject { described_class.new(nil).all } + + context 'for valid request' do + let(:response) do + [{ + "id" => "xiyro8huptfhdndadpz8r3wnbo", + "create_at" => 1482174222155, + "update_at" => 1482174222155, + "delete_at" => 0, + "display_name" => "chatops", + "name" => "chatops", + "email" => "admin@example.com", + "type" => "O", + "company_name" => "", + "allowed_domains" => "", + "invite_id" => "o4utakb9jtb7imctdfzbf9r5ro", + "allow_open_invite" => false }] + end + + before do + stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all'). + to_return( + status: 200, + headers: { 'Content-Type' => 'application/json' }, + body: response.to_json + ) + end + + it 'returns a token' do + is_expected.to eq(response) + end + end + + context 'for error message' do + before do + stub_request(:get, 'http://mattermost.example.com/api/v3/teams/all'). + to_return( + status: 500, + headers: { 'Content-Type' => 'application/json' }, + body: { + id: 'api.team.list.app_error', + message: 'Cannot list teams.', + detailed_error: '', + request_id: 'obc374man7bx5r3dbc1q5qhf3r', + status_code: 500 + }.to_json + ) + end + + it 'raises an error with message' do + expect { subject }.to raise_error(Mattermost::Error, 'Cannot list teams.') + end + end + end +end -- cgit v1.2.1 From dbda72a79999998bfd1d77b3102bc16053a2685e Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" <git@zjvandeweg.nl> Date: Thu, 26 Jan 2017 15:30:34 +0100 Subject: Rename presenters for consitency --- lib/gitlab/chat_commands/command.rb | 2 +- lib/gitlab/chat_commands/issue_create.rb | 42 ------------ lib/gitlab/chat_commands/issue_new.rb | 42 ++++++++++++ lib/gitlab/chat_commands/issue_search.rb | 8 +-- lib/gitlab/chat_commands/issue_show.rb | 2 +- lib/gitlab/chat_commands/presenters/deploy.rb | 8 --- lib/gitlab/chat_commands/presenters/help.rb | 6 +- lib/gitlab/chat_commands/presenters/issuable.rb | 4 +- lib/gitlab/chat_commands/presenters/issue_new.rb | 48 +++++++++++++ .../chat_commands/presenters/issue_search.rb | 45 +++++++++++++ lib/gitlab/chat_commands/presenters/issue_show.rb | 56 ++++++++++++++++ lib/gitlab/chat_commands/presenters/list_issues.rb | 43 ------------ lib/gitlab/chat_commands/presenters/new_issue.rb | 42 ------------ lib/gitlab/chat_commands/presenters/show_issue.rb | 54 --------------- spec/lib/gitlab/chat_commands/command_spec.rb | 2 +- spec/lib/gitlab/chat_commands/issue_create_spec.rb | 78 ---------------------- spec/lib/gitlab/chat_commands/issue_new_spec.rb | 78 ++++++++++++++++++++++ .../chat_commands/presenters/issue_new_spec.rb | 17 +++++ .../chat_commands/presenters/issue_search_spec.rb | 23 +++++++ .../chat_commands/presenters/issue_show_spec.rb | 27 ++++++++ .../chat_commands/presenters/list_issues_spec.rb | 23 ------- .../chat_commands/presenters/show_issue_spec.rb | 27 -------- 22 files changed, 346 insertions(+), 331 deletions(-) delete mode 100644 lib/gitlab/chat_commands/issue_create.rb create mode 100644 lib/gitlab/chat_commands/issue_new.rb create mode 100644 lib/gitlab/chat_commands/presenters/issue_new.rb create mode 100644 lib/gitlab/chat_commands/presenters/issue_search.rb create mode 100644 lib/gitlab/chat_commands/presenters/issue_show.rb delete mode 100644 lib/gitlab/chat_commands/presenters/list_issues.rb delete mode 100644 lib/gitlab/chat_commands/presenters/new_issue.rb delete mode 100644 lib/gitlab/chat_commands/presenters/show_issue.rb delete mode 100644 spec/lib/gitlab/chat_commands/issue_create_spec.rb create mode 100644 spec/lib/gitlab/chat_commands/issue_new_spec.rb create mode 100644 spec/lib/gitlab/chat_commands/presenters/issue_new_spec.rb create mode 100644 spec/lib/gitlab/chat_commands/presenters/issue_search_spec.rb create mode 100644 spec/lib/gitlab/chat_commands/presenters/issue_show_spec.rb delete mode 100644 spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb delete mode 100644 spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb diff --git a/lib/gitlab/chat_commands/command.rb b/lib/gitlab/chat_commands/command.rb index 4e5031a8a26..e7baa20356c 100644 --- a/lib/gitlab/chat_commands/command.rb +++ b/lib/gitlab/chat_commands/command.rb @@ -3,7 +3,7 @@ module Gitlab class Command < BaseCommand COMMANDS = [ Gitlab::ChatCommands::IssueShow, - Gitlab::ChatCommands::IssueCreate, + Gitlab::ChatCommands::IssueNew, Gitlab::ChatCommands::IssueSearch, Gitlab::ChatCommands::Deploy, ].freeze diff --git a/lib/gitlab/chat_commands/issue_create.rb b/lib/gitlab/chat_commands/issue_create.rb deleted file mode 100644 index 3f3d7de8b2e..00000000000 --- a/lib/gitlab/chat_commands/issue_create.rb +++ /dev/null @@ -1,42 +0,0 @@ -module Gitlab - module ChatCommands - class IssueCreate < IssueCommand - def self.match(text) - # we can not match \n with the dot by passing the m modifier as than - # the title and description are not seperated - /\Aissue\s+(new|create)\s+(?<title>[^\n]*)\n*(?<description>(.|\n)*)/.match(text) - end - - def self.help_message - 'issue new <title> *`⇧ Shift`*+*`↵ Enter`* <description>' - end - - def self.allowed?(project, user) - can?(user, :create_issue, project) - end - - def execute(match) - title = match[:title] - description = match[:description].to_s.rstrip - - issue = create_issue(title: title, description: description) - - if issue.errors.any? - presenter(issue).display_errors - else - presenter(issue).present - end - end - - private - - def create_issue(title:, description:) - Issues::CreateService.new(project, current_user, title: title, description: description).execute - end - - def presenter(issue) - Gitlab::ChatCommands::Presenters::NewIssue.new(issue) - end - end - end -end diff --git a/lib/gitlab/chat_commands/issue_new.rb b/lib/gitlab/chat_commands/issue_new.rb new file mode 100644 index 00000000000..016054ecd46 --- /dev/null +++ b/lib/gitlab/chat_commands/issue_new.rb @@ -0,0 +1,42 @@ +module Gitlab + module ChatCommands + class IssueNew < IssueCommand + def self.match(text) + # we can not match \n with the dot by passing the m modifier as than + # the title and description are not seperated + /\Aissue\s+(new|create)\s+(?<title>[^\n]*)\n*(?<description>(.|\n)*)/.match(text) + end + + def self.help_message + 'issue new <title> *`⇧ Shift`*+*`↵ Enter`* <description>' + end + + def self.allowed?(project, user) + can?(user, :create_issue, project) + end + + def execute(match) + title = match[:title] + description = match[:description].to_s.rstrip + + issue = create_issue(title: title, description: description) + + if issue.persisted? + presenter(issue).present + else + presenter(issue).display_errors + end + end + + private + + def create_issue(title:, description:) + Issues::CreateService.new(project, current_user, title: title, description: description).execute + end + + def presenter(issue) + Gitlab::ChatCommands::Presenters::IssueNew.new(issue) + end + end + end +end diff --git a/lib/gitlab/chat_commands/issue_search.rb b/lib/gitlab/chat_commands/issue_search.rb index e2d3a0f466a..3491b53093e 100644 --- a/lib/gitlab/chat_commands/issue_search.rb +++ b/lib/gitlab/chat_commands/issue_search.rb @@ -12,12 +12,10 @@ module Gitlab def execute(match) issues = collection.search(match[:query]).limit(QUERY_LIMIT) - if issues.none? - Presenters::Access.new(issues).not_found - elsif issues.one? - Presenters::ShowIssue.new(issues.first).present + if issues.present? + Presenters::IssueSearch.new(issues).present else - Presenters::ListIssues.new(issues).present + Presenters::Access.new(issues).not_found end end end diff --git a/lib/gitlab/chat_commands/issue_show.rb b/lib/gitlab/chat_commands/issue_show.rb index 9f3e1b9a64b..d6013f4d10c 100644 --- a/lib/gitlab/chat_commands/issue_show.rb +++ b/lib/gitlab/chat_commands/issue_show.rb @@ -13,7 +13,7 @@ module Gitlab issue = find_by_iid(match[:iid]) if issue - Gitlab::ChatCommands::Presenters::ShowIssue.new(issue).present + Gitlab::ChatCommands::Presenters::IssueShow.new(issue).present else Gitlab::ChatCommands::Presenters::Access.new.not_found end diff --git a/lib/gitlab/chat_commands/presenters/deploy.rb b/lib/gitlab/chat_commands/presenters/deploy.rb index b1cfaac15af..863d0bf99ca 100644 --- a/lib/gitlab/chat_commands/presenters/deploy.rb +++ b/lib/gitlab/chat_commands/presenters/deploy.rb @@ -15,14 +15,6 @@ module Gitlab def too_many_actions ephemeral_response(text: "Too many actions defined") end - - private - - def resource_url - polymorphic_url( - [ @resource.project.namespace.becomes(Namespace), @resource.project, @resource] - ) - end end end end diff --git a/lib/gitlab/chat_commands/presenters/help.rb b/lib/gitlab/chat_commands/presenters/help.rb index c7a67467b7e..39ad3249f5b 100644 --- a/lib/gitlab/chat_commands/presenters/help.rb +++ b/lib/gitlab/chat_commands/presenters/help.rb @@ -9,10 +9,10 @@ module Gitlab private def help_message(trigger) - if @resource.none? - "No commands available :thinking_face:" - else + if @resource.present? header_with_list("Available commands", full_commands(trigger)) + else + "No commands available :thinking_face:" end end diff --git a/lib/gitlab/chat_commands/presenters/issuable.rb b/lib/gitlab/chat_commands/presenters/issuable.rb index 2cb6b1525fc..dfb1c8f6616 100644 --- a/lib/gitlab/chat_commands/presenters/issuable.rb +++ b/lib/gitlab/chat_commands/presenters/issuable.rb @@ -1,9 +1,7 @@ module Gitlab module ChatCommands module Presenters - class Issuable < Presenters::Base - private - + module Issuable def color(issuable) issuable.open? ? '#38ae67' : '#d22852' end diff --git a/lib/gitlab/chat_commands/presenters/issue_new.rb b/lib/gitlab/chat_commands/presenters/issue_new.rb new file mode 100644 index 00000000000..d26dd22b2a0 --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/issue_new.rb @@ -0,0 +1,48 @@ +module Gitlab + module ChatCommands + module Presenters + class IssueNew < Presenters::Base + include Presenters::Issuable + + def present + in_channel_response(new_issue) + end + + private + + def new_issue + { + attachments: [ + { + title: "#{@resource.title} · #{@resource.to_reference}", + title_link: resource_url, + author_name: author.name, + author_icon: author.avatar_url, + fallback: "New issue #{@resource.to_reference}: #{@resource.title}", + pretext: pretext, + color: color(@resource), + fields: fields, + mrkdwn_in: [ + :title, + :text + ] + } + ] + } + end + + def pretext + "I opened an issue on behalf on #{author_profile_link}: *#{@resource.to_reference}* from #{project.name_with_namespace}" + end + + def project_link + "[#{project.name_with_namespace}](#{url_for(project)})" + end + + def author_profile_link + "[#{author.to_reference}](#{url_for(author)})" + end + end + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/issue_search.rb b/lib/gitlab/chat_commands/presenters/issue_search.rb new file mode 100644 index 00000000000..d58a6d6114a --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/issue_search.rb @@ -0,0 +1,45 @@ +module Gitlab + module ChatCommands + module Presenters + class IssueSearch < Presenters::Base + include Presenters::Issuable + + def present + text = if @resource.count >= 5 + "Here are the first 5 issues I found:" + else + "Here are the #{@resource.count} issues I found:" + end + + ephemeral_response(text: text, attachments: attachments) + end + + private + + def attachments + @resource.map do |issue| + url = "[#{issue.to_reference}](#{url_for([namespace, project, issue])})" + + { + color: color(issue), + fallback: "#{issue.to_reference} #{issue.title}", + text: "#{url} · #{issue.title} (#{status_text(issue)})", + + mrkdwn_in: [ + "text" + ] + } + end + end + + def project + @project ||= @resource.first.project + end + + def namespace + @namespace ||= project.namespace.becomes(Namespace) + end + end + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/issue_show.rb b/lib/gitlab/chat_commands/presenters/issue_show.rb new file mode 100644 index 00000000000..2fc671f13a6 --- /dev/null +++ b/lib/gitlab/chat_commands/presenters/issue_show.rb @@ -0,0 +1,56 @@ +module Gitlab + module ChatCommands + module Presenters + class IssueShow < Presenters::Base + include Presenters::Issuable + + def present + in_channel_response(show_issue) + end + + private + + def show_issue + { + attachments: [ + { + title: "#{@resource.title} · #{@resource.to_reference}", + title_link: resource_url, + author_name: author.name, + author_icon: author.avatar_url, + fallback: "Issue #{@resource.to_reference}: #{@resource.title}", + pretext: pretext, + text: text, + color: color(@resource), + fields: fields, + mrkdwn_in: [ + :pretext, + :text + ] + } + ] + } + end + + def text + message = "**#{status_text(@resource)}**" + + if @resource.upvotes.zero? && @resource.downvotes.zero? && @resource.user_notes_count.zero? + return message + end + + message << " · " + message << ":+1: #{@resource.upvotes} " unless @resource.upvotes.zero? + message << ":-1: #{@resource.downvotes} " unless @resource.downvotes.zero? + message << ":speech_balloon: #{@resource.user_notes_count}" unless @resource.user_notes_count.zero? + + message + end + + def pretext + "Issue *#{@resource.to_reference} from #{project.name_with_namespace}" + end + end + end + end +end diff --git a/lib/gitlab/chat_commands/presenters/list_issues.rb b/lib/gitlab/chat_commands/presenters/list_issues.rb deleted file mode 100644 index 2458b9356b7..00000000000 --- a/lib/gitlab/chat_commands/presenters/list_issues.rb +++ /dev/null @@ -1,43 +0,0 @@ -module Gitlab - module ChatCommands - module Presenters - class ListIssues < Presenters::Issuable - def present - text = if @resource.count >= 5 - "Here are the first 5 issues I found:" - else - "Here are the #{@resource.count} issues I found:" - end - - ephemeral_response(text: text, attachments: attachments) - end - - private - - def attachments - @resource.map do |issue| - url = "[#{issue.to_reference}](#{url_for([namespace, project, issue])})" - - { - color: color(issue), - fallback: "#{issue.to_reference} #{issue.title}", - text: "#{url} · #{issue.title} (#{status_text(issue)})", - - mrkdwn_in: [ - "text" - ] - } - end - end - - def project - @project ||= @resource.first.project - end - - def namespace - @namespace ||= project.namespace.becomes(Namespace) - end - end - end - end -end diff --git a/lib/gitlab/chat_commands/presenters/new_issue.rb b/lib/gitlab/chat_commands/presenters/new_issue.rb deleted file mode 100644 index c7c6febb56e..00000000000 --- a/lib/gitlab/chat_commands/presenters/new_issue.rb +++ /dev/null @@ -1,42 +0,0 @@ -module Gitlab - module ChatCommands - module Presenters - class NewIssue < Presenters::Issuable - def present - in_channel_response(show_issue) - end - - private - - def show_issue - { - attachments: [ - { - title: "#{@resource.title} · #{@resource.to_reference}", - title_link: resource_url, - author_name: author.name, - author_icon: author.avatar_url, - fallback: "New issue #{@resource.to_reference}: #{@resource.title}", - pretext: pretext, - color: color(@resource), - fields: fields, - mrkdwn_in: [ - :title, - :text - ] - } - ] - } - end - - def pretext - "I opened an issue on behalf on #{author_profile_link}: *#{@resource.to_reference}* from #{project.name_with_namespace}" - end - - def author_profile_link - "[#{author.to_reference}](#{url_for(author)})" - end - end - end - end -end diff --git a/lib/gitlab/chat_commands/presenters/show_issue.rb b/lib/gitlab/chat_commands/presenters/show_issue.rb deleted file mode 100644 index e5644a4ad7e..00000000000 --- a/lib/gitlab/chat_commands/presenters/show_issue.rb +++ /dev/null @@ -1,54 +0,0 @@ -module Gitlab - module ChatCommands - module Presenters - class ShowIssue < Presenters::Issuable - def present - in_channel_response(show_issue) - end - - private - - def show_issue - { - attachments: [ - { - title: "#{@resource.title} · #{@resource.to_reference}", - title_link: resource_url, - author_name: author.name, - author_icon: author.avatar_url, - fallback: "New issue #{@resource.to_reference}: #{@resource.title}", - pretext: pretext, - text: text, - color: color(@resource), - fields: fields, - mrkdwn_in: [ - :pretext, - :text - ] - } - ] - } - end - - def text - message = "**#{status_text(@resource)}**" - - if @resource.upvotes.zero? && @resource.downvotes.zero? && @resource.user_notes_count.zero? - return message - end - - message << " · " - message << ":+1: #{@resource.upvotes} " unless @resource.upvotes.zero? - message << ":-1: #{@resource.downvotes} " unless @resource.downvotes.zero? - message << ":speech_balloon: #{@resource.user_notes_count}" unless @resource.user_notes_count.zero? - - message - end - - def pretext - "Issue *#{@resource.to_reference} from #{project.name_with_namespace}" - 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 b634df52b68..f4441f9f93c 100644 --- a/spec/lib/gitlab/chat_commands/command_spec.rb +++ b/spec/lib/gitlab/chat_commands/command_spec.rb @@ -78,7 +78,7 @@ describe Gitlab::ChatCommands::Command, service: true do context 'IssueCreate is triggered' do let(:params) { { text: 'issue create my title' } } - it { is_expected.to eq(Gitlab::ChatCommands::IssueCreate) } + it { is_expected.to eq(Gitlab::ChatCommands::IssueNew) } end context 'IssueSearch is triggered' do diff --git a/spec/lib/gitlab/chat_commands/issue_create_spec.rb b/spec/lib/gitlab/chat_commands/issue_create_spec.rb deleted file mode 100644 index 0f84b19a5a4..00000000000 --- a/spec/lib/gitlab/chat_commands/issue_create_spec.rb +++ /dev/null @@ -1,78 +0,0 @@ -require 'spec_helper' - -describe Gitlab::ChatCommands::IssueCreate, service: true do - describe '#execute' do - let(:project) { create(:empty_project) } - let(:user) { create(:user) } - let(:regex_match) { described_class.match("issue create bird is the word") } - - before do - project.team << [user, :master] - end - - subject do - described_class.new(project, user).execute(regex_match) - end - - context 'without description' do - it 'creates the issue' do - expect { subject }.to change { project.issues.count }.by(1) - - expect(subject[:response_type]).to be(:in_channel) - end - end - - context 'with description' do - let(:description) { "Surfin bird" } - let(:regex_match) { described_class.match("issue create bird is the word\n#{description}") } - - it 'creates the issue with description' do - subject - - expect(Issue.last.description).to eq(description) - end - end - - context "with more newlines between the title and the description" do - let(:description) { "Surfin bird" } - let(:regex_match) { described_class.match("issue create bird is the word\n\n#{description}\n") } - - it 'creates the issue' do - expect { subject }.to change { project.issues.count }.by(1) - end - end - - context 'issue cannot be created' do - let!(:issue) { create(:issue, project: project, title: 'bird is the word') } - let(:regex_match) { described_class.match("issue create #{'a' * 512}}") } - - it 'displays the errors' do - expect(subject[:response_type]).to be(:ephemeral) - expect(subject[:text]).to match("- Title is too long") - end - end - end - - describe '.match' do - it 'matches the title without description' do - match = described_class.match("issue create my title") - - expect(match[:title]).to eq('my title') - expect(match[:description]).to eq("") - end - - it 'matches the title with description' do - match = described_class.match("issue create my title\n\ndescription") - - expect(match[:title]).to eq('my title') - expect(match[:description]).to eq('description') - end - - it 'matches the alias new' do - match = described_class.match("issue new my title") - - expect(match).not_to be_nil - expect(match[:title]).to eq('my title') - end - end -end diff --git a/spec/lib/gitlab/chat_commands/issue_new_spec.rb b/spec/lib/gitlab/chat_commands/issue_new_spec.rb new file mode 100644 index 00000000000..84c22328064 --- /dev/null +++ b/spec/lib/gitlab/chat_commands/issue_new_spec.rb @@ -0,0 +1,78 @@ +require 'spec_helper' + +describe Gitlab::ChatCommands::IssueNew, service: true do + describe '#execute' do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + let(:regex_match) { described_class.match("issue create bird is the word") } + + before do + project.team << [user, :master] + end + + subject do + described_class.new(project, user).execute(regex_match) + end + + context 'without description' do + it 'creates the issue' do + expect { subject }.to change { project.issues.count }.by(1) + + expect(subject[:response_type]).to be(:in_channel) + end + end + + context 'with description' do + let(:description) { "Surfin bird" } + let(:regex_match) { described_class.match("issue create bird is the word\n#{description}") } + + it 'creates the issue with description' do + subject + + expect(Issue.last.description).to eq(description) + end + end + + context "with more newlines between the title and the description" do + let(:description) { "Surfin bird" } + let(:regex_match) { described_class.match("issue create bird is the word\n\n#{description}\n") } + + it 'creates the issue' do + expect { subject }.to change { project.issues.count }.by(1) + end + end + + context 'issue cannot be created' do + let!(:issue) { create(:issue, project: project, title: 'bird is the word') } + let(:regex_match) { described_class.match("issue create #{'a' * 512}}") } + + it 'displays the errors' do + expect(subject[:response_type]).to be(:ephemeral) + expect(subject[:text]).to match("- Title is too long") + end + end + end + + describe '.match' do + it 'matches the title without description' do + match = described_class.match("issue create my title") + + expect(match[:title]).to eq('my title') + expect(match[:description]).to eq("") + end + + it 'matches the title with description' do + match = described_class.match("issue create my title\n\ndescription") + + expect(match[:title]).to eq('my title') + expect(match[:description]).to eq('description') + end + + it 'matches the alias new' do + match = described_class.match("issue new my title") + + expect(match).not_to be_nil + expect(match[:title]).to eq('my title') + end + end +end diff --git a/spec/lib/gitlab/chat_commands/presenters/issue_new_spec.rb b/spec/lib/gitlab/chat_commands/presenters/issue_new_spec.rb new file mode 100644 index 00000000000..17fcdbc2452 --- /dev/null +++ b/spec/lib/gitlab/chat_commands/presenters/issue_new_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe Gitlab::ChatCommands::Presenters::IssueNew do + let(:project) { create(:empty_project) } + let(:issue) { create(:issue, project: project) } + let(:attachment) { subject[:attachments].first } + + subject { described_class.new(issue).present } + + it { is_expected.to be_a(Hash) } + + it 'shows the issue' do + expect(subject[:response_type]).to be(:in_channel) + expect(subject).to have_key(:attachments) + expect(attachment[:title]).to start_with(issue.title) + end +end diff --git a/spec/lib/gitlab/chat_commands/presenters/issue_search_spec.rb b/spec/lib/gitlab/chat_commands/presenters/issue_search_spec.rb new file mode 100644 index 00000000000..ec6d3e34a96 --- /dev/null +++ b/spec/lib/gitlab/chat_commands/presenters/issue_search_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe Gitlab::ChatCommands::Presenters::IssueSearch do + let(:project) { create(:empty_project) } + let(:message) { subject[:text] } + + before { create_list(:issue, 2, project: project) } + + subject { described_class.new(project.issues).present } + + it 'formats the message correct' do + is_expected.to have_key(:text) + is_expected.to have_key(:status) + is_expected.to have_key(:response_type) + is_expected.to have_key(:attachments) + end + + it 'shows a list of results' do + expect(subject[:response_type]).to be(:ephemeral) + + expect(message).to start_with("Here are the 2 issues I found") + end +end diff --git a/spec/lib/gitlab/chat_commands/presenters/issue_show_spec.rb b/spec/lib/gitlab/chat_commands/presenters/issue_show_spec.rb new file mode 100644 index 00000000000..89d154e26e4 --- /dev/null +++ b/spec/lib/gitlab/chat_commands/presenters/issue_show_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe Gitlab::ChatCommands::Presenters::IssueShow do + let(:project) { create(:empty_project) } + let(:issue) { create(:issue, project: project) } + let(:attachment) { subject[:attachments].first } + + subject { described_class.new(issue).present } + + it { is_expected.to be_a(Hash) } + + it 'shows the issue' do + expect(subject[:response_type]).to be(:in_channel) + expect(subject).to have_key(:attachments) + expect(attachment[:title]).to start_with(issue.title) + end + + context 'with upvotes' do + before do + create(:award_emoji, :upvote, awardable: issue) + end + + it 'shows the upvote count' do + expect(attachment[:text]).to start_with("**Open** · :+1: 1") + end + end +end diff --git a/spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb b/spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb deleted file mode 100644 index 13a1f70fe78..00000000000 --- a/spec/lib/gitlab/chat_commands/presenters/list_issues_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -describe Gitlab::ChatCommands::Presenters::ListIssues do - let(:project) { create(:empty_project) } - let(:message) { subject[:text] } - - before { create_list(:issue, 2, project: project) } - - subject { described_class.new(project.issues).present } - - it 'formats the message correct' do - is_expected.to have_key(:text) - is_expected.to have_key(:status) - is_expected.to have_key(:response_type) - is_expected.to have_key(:attachments) - end - - it 'shows a list of results' do - expect(subject[:response_type]).to be(:ephemeral) - - expect(message).to start_with("Here are the 2 issues I found") - end -end diff --git a/spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb b/spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb deleted file mode 100644 index ca4062e692a..00000000000 --- a/spec/lib/gitlab/chat_commands/presenters/show_issue_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'spec_helper' - -describe Gitlab::ChatCommands::Presenters::ShowIssue do - let(:project) { create(:empty_project) } - let(:issue) { create(:issue, project: project) } - let(:attachment) { subject[:attachments].first } - - subject { described_class.new(issue).present } - - it { is_expected.to be_a(Hash) } - - it 'shows the issue' do - expect(subject[:response_type]).to be(:in_channel) - expect(subject).to have_key(:attachments) - expect(attachment[:title]).to start_with(issue.title) - end - - context 'with upvotes' do - before do - create(:award_emoji, :upvote, awardable: issue) - end - - it 'shows the upvote count' do - expect(attachment[:text]).to start_with("**Open** · :+1: 1") - end - end -end -- cgit v1.2.1