From 9d8ca60d221a1500e31ef05ecf8c4db56adde3c0 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Sat, 12 Nov 2016 11:17:24 +0100 Subject: [PATCH] issue and mergerequest slash command for mattermost This commit includes a couple of thing: - A chatops controller - Mattermost::CommandService - Mattermost::Commands::(IssueService|MergeRequestService) The controller is the point where mattermost, and later slack will have to fire their payload to. This in turn will execute the CommandService. Thats where the authentication and authorization should happen. So far this is not yet implemented. This should happen in later commits. Per subcommand, in case of `/gitlab issue show 123` issue whould be the subcommand, there is a service to parse the data, and fetch the resource. The resource is passed back to the CommandService which structures the data. --- app/controllers/chat_ops_controller.rb | 10 +++ app/models/project.rb | 4 +- app/models/project_feature.rb | 4 + app/services/mattermost/command_service.rb | 74 +++++++++++++++++++ .../mattermost/commands/base_service.rb | 60 +++++++++++++++ .../mattermost/commands/issue_service.rb | 49 ++++++++++++ .../commands/merge_request_service.rb | 46 ++++++++++++ config/routes.rb | 5 ++ spec/controllers/chat_ops_controller_spec.rb | 11 +++ .../mattermost/command_service_spec.rb | 55 ++++++++++++++ .../mattermost/commands/issue_service_spec.rb | 70 ++++++++++++++++++ .../commands/merge_request_service_spec.rb | 68 +++++++++++++++++ 12 files changed, 455 insertions(+), 1 deletion(-) create mode 100644 app/controllers/chat_ops_controller.rb create mode 100644 app/services/mattermost/command_service.rb create mode 100644 app/services/mattermost/commands/base_service.rb create mode 100644 app/services/mattermost/commands/issue_service.rb create mode 100644 app/services/mattermost/commands/merge_request_service.rb create mode 100644 spec/controllers/chat_ops_controller_spec.rb create mode 100644 spec/services/mattermost/command_service_spec.rb create mode 100644 spec/services/mattermost/commands/issue_service_spec.rb create mode 100644 spec/services/mattermost/commands/merge_request_service_spec.rb diff --git a/app/controllers/chat_ops_controller.rb b/app/controllers/chat_ops_controller.rb new file mode 100644 index 00000000000..2754de5e710 --- /dev/null +++ b/app/controllers/chat_ops_controller.rb @@ -0,0 +1,10 @@ +class ChatOpsController < ApplicationController + respond_to :json + + skip_before_action :verify_authenticity_token + skip_before_action :authenticate_user! + + def trigger + render json: { ok: true } + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 4aedc91dc34..1c392eb7460 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -23,7 +23,9 @@ class Project < ActiveRecord::Base cache_markdown_field :description, pipeline: :description - delegate :feature_available?, :builds_enabled?, :wiki_enabled?, :merge_requests_enabled?, to: :project_feature, allow_nil: true + delegate :feature_available?, :builds_enabled?, :wiki_enabled?, + :merge_requests_enabled?, :issues_enabled?, to: :project_feature, + allow_nil: true default_value_for :archived, false default_value_for :visibility_level, gitlab_config_features.visibility_level diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index 5c53c8f1ee5..03194fc2141 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -60,6 +60,10 @@ class ProjectFeature < ActiveRecord::Base merge_requests_access_level > DISABLED end + def issues_enabled? + issues_access_level > DISABLED + end + private # Validates builds and merge requests access level diff --git a/app/services/mattermost/command_service.rb b/app/services/mattermost/command_service.rb new file mode 100644 index 00000000000..c8329a2dca4 --- /dev/null +++ b/app/services/mattermost/command_service.rb @@ -0,0 +1,74 @@ +module Mattermost + class CommandService < BaseService + SERVICES = [ + Mattermost::Commands::IssueService, + Mattermost::Commands::MergeRequestService + ] + + def execute + return unknown_user unless current_user + return not_found_404 unless can?(current_user, :read_project, project) + + triggered_command = command + service = SERVICES.find do |service| + service.triggered_by?(triggered_command) && service.available?(project) + end + + if service + present service.new(project, current_user, params).execute + else + help_message + end + end + + private + + def command + params[:text].match(/\A(?\S+)/)[:command] + end + + def present(result) + return not_found_404 unless result + + if result.respond_to?(:count) + if count > 1 + #TODO + return resource_list(result) + else + result = result.first + end + end + + message = "### [#{result.to_reference} #{result.title}](link(result))" + message << "\n\n#{result.description}" if result.description + + { + response_type: :in_channel, + text: message + } + end + + def unknown_user + { + response_type: :ephemeral, + text: 'Hi there! I have not yet had the pleasure to get acquainted!' # TODO allow user to authenticate and authorize + } + end + + def not_found_404 + { + response_type: :ephemeral, + text: "404 not found! GitLab couldn't find what your were looking for! :boom:", + } + end + + def help_message + command_help_messages = SERVICES.map { |service| service.help_message(project) } + + { + response_type: :ephemeral, + text: "Sadly, the used command does not exist, lets take a look at your options here:\n\n#{command_help_messages.join("\n")}" + } + end + end +end diff --git a/app/services/mattermost/commands/base_service.rb b/app/services/mattermost/commands/base_service.rb new file mode 100644 index 00000000000..54d8fa088b8 --- /dev/null +++ b/app/services/mattermost/commands/base_service.rb @@ -0,0 +1,60 @@ +module Mattermost + module Commands + class BaseService < ::BaseService + class << self + def triggered_by?(_) + raise NotImplementedError + end + + def available?(_) + raise NotImplementedError + end + + def help_message(_) + NotImplementedError + end + end + + QUERY_LIMIT = 5 + + def execute + subcommand, args = parse_command + + if subcommands.include?(subcommand) + send(subcommand, args) + else + nil + end + end + + private + + # This method can only be used by a resource that has an iid. Also, the + # class should implement #collection itself. Probably project.resource + # would suffice + def show(args) + iid = args.first + + result = collection.find_by(iid: iid) + if readable?(result) + result + else + nil + end + end + + # Child class should implement #collection + def search(args) + query = args.join(' ') + + collection.search(query).limit(QUERY_LIMIT).select do |issuable| + readable?(issuable) + end + end + + def command + params[:text] + end + end + end +end diff --git a/app/services/mattermost/commands/issue_service.rb b/app/services/mattermost/commands/issue_service.rb new file mode 100644 index 00000000000..879f0c2eb21 --- /dev/null +++ b/app/services/mattermost/commands/issue_service.rb @@ -0,0 +1,49 @@ +module Mattermost + module Commands + class IssueService < Mattermost::Commands::BaseService + class << self + def triggered_by?(command) + command == 'issue' + end + + def available?(project) + project.issues_enabled? && project.default_issues_tracker? + end + + def help_message(project) + return nil unless available?(project) + + message = "issue show " + message << "issue search " + end + end + + private + + #TODO implement create + def subcommands + %w[creates search show] + end + + def collection + project.issues + end + + def readable?(issue) + can?(current_user, :read_issue, issue) + end + + # 'issue create my new title\nmy new description + # => 'create', ['my', 'new', 'title, ['my new description']] + # 'issue show 123' + # => 'show', ['123'] + def parse_command + split = command.split + subcommand = split[1] + args = split[2..-1] + + return subcommand, args + end + end + end +end diff --git a/app/services/mattermost/commands/merge_request_service.rb b/app/services/mattermost/commands/merge_request_service.rb new file mode 100644 index 00000000000..907834ee0c2 --- /dev/null +++ b/app/services/mattermost/commands/merge_request_service.rb @@ -0,0 +1,46 @@ +module Mattermost + module Commands + class MergeRequestService < Mattermost::Commands::BaseService + class << self + def triggered_by?(command) + command == 'mergerequest' + end + + def available?(project) + project.merge_requests_enabled? + end + + def help_message(project) + return nil unless available?(project) + + message = "mergerequest show \n" + message << "mergerequest search " + end + end + + private + + def subcommands + %w[show search] + end + + def collection + project.merge_requests + end + + def readable?(_) + can?(current_user, :read_merge_request, project) + end + + # 'mergerequest show 123' => 'show', ['123'] + # 'mergerequest search my query' => 'search',['my', 'query'] + def parse_command + split = command.split + subcommand = split[1] + args = split[2..-1] + + return subcommand, args + end + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 7bf6c03e69b..b1de4ba3821 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -68,6 +68,11 @@ Rails.application.routes.draw do # Notification settings resources :notification_settings, only: [:create, :update] + # For slash commands to POST + namespace :chat_ops do + post :trigger + end + draw :import draw :uploads draw :explore diff --git a/spec/controllers/chat_ops_controller_spec.rb b/spec/controllers/chat_ops_controller_spec.rb new file mode 100644 index 00000000000..7303c981ba2 --- /dev/null +++ b/spec/controllers/chat_ops_controller_spec.rb @@ -0,0 +1,11 @@ +require 'rails_helper' + +RSpec.describe ChatOpsController, type: :controller do + describe "POST #trigger" do + it "returns http success" do + post :trigger + + expect(response).to have_http_status(:success) + end + end +end diff --git a/spec/services/mattermost/command_service_spec.rb b/spec/services/mattermost/command_service_spec.rb new file mode 100644 index 00000000000..35051c09f8d --- /dev/null +++ b/spec/services/mattermost/command_service_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Mattermost::CommandService, service: true do + let(:project) { build(:project) } + let(:user) { build(:user) } + let(:params) { { text: 'issue show 1' } } + + subject { described_class.new(project, user, params).execute } + + describe '#execute' do + context 'no user could be found' do + let(:user) { nil } + + it 'asks the user to introduce him/herself' do + expect(subject[:response_type]).to be :ephemeral + expect(subject[:text]).to start_with 'Hi there!' + end + end + + context 'no project could be found' do + it 'shows a 404 not found message' do + expect(subject[:response_type]).to be :ephemeral + expect(subject[:text]).to start_with '404 not found!' + end + end + + context 'the user has access to the project' do + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + project.team << [user, :master] + end + + context 'no command service is triggered' do + let(:params) { { text: 'unknown_command' } } + + it 'shows the help messages' do + expect(subject[:response_type]).to be :ephemeral + expect(subject[:text]).to start_with 'Sadly, the used command' + end + end + + context 'a valid command is executed' do + let(:issue) { create(:issue, project: project) } + let(:params) { { text: "issue show #{issue.iid}" } } + + it 'a resource is presented to the user' do + expect(subject[:response_type]).to be :in_channel + expect(subject[:text]).to match issue.title + end + end + end + end +end diff --git a/spec/services/mattermost/commands/issue_service_spec.rb b/spec/services/mattermost/commands/issue_service_spec.rb new file mode 100644 index 00000000000..67629d26bc0 --- /dev/null +++ b/spec/services/mattermost/commands/issue_service_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe Mattermost::Commands::IssueService do + let(:project) { create(:project) } + let(:issue ) { create(:issue, :confidential, title: 'Bird is the word', project: project) } + let(:user) { issue.author } + + subject { described_class.new(project, user, params).execute } + + before do + project.team << [user, :developer] + end + + describe '#execute' do + context 'show as subcommand' do + context 'issue can be found' do + let(:params) { { text: "issue show #{issue.iid}" } } + + it 'returns the merge request' do + expect(subject).to eq issue + end + + context 'the user has no access' do + let(:non_member) { create(:user) } + subject { described_class.new(project, non_member, params).execute } + + it 'returns nil' do + expect(subject).to eq nil + end + end + end + + context 'issue can not be found' do + let(:params) { { text: 'issue show 12345' } } + + it 'returns nil' do + expect(subject).to eq nil + end + end + end + + context 'search as a subcommand' do + context 'with results' do + let(:params) { { text: "issue search is the word" } } + + it 'returns the issue' do + expect(subject).to eq [issue] + end + end + + context 'without results' do + let(:params) { { text: 'issue search mepmep' } } + + it 'returns an empty collection' do + expect(subject).to eq [] + end + end + end + end + + describe 'help_message' do + context 'issues are disabled' do + it 'returns nil' do + allow(described_class).to receive(:available?).and_return false + + expect(described_class.help_message(project)).to eq nil + end + end + end +end diff --git a/spec/services/mattermost/commands/merge_request_service_spec.rb b/spec/services/mattermost/commands/merge_request_service_spec.rb new file mode 100644 index 00000000000..39381520a99 --- /dev/null +++ b/spec/services/mattermost/commands/merge_request_service_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +describe Mattermost::Commands::MergeRequestService do + let(:project) { create(:project, :private) } + let(:merge_request) { create(:merge_request, title: 'Bird is the word', source_project: project) } + let(:user) { merge_request.author } + + subject { described_class.new(project, user, params).execute } + + before do + project.team << [user, :developer] + end + + context 'show as subcommand' do + context 'merge request can be found' do + let(:params) { { text: "mergerequest show #{merge_request.iid}" } } + + it 'returns the merge request' do + expect(subject).to eq merge_request + end + + context 'the user has no access' do + let(:non_member) { create(:user) } + subject { described_class.new(project, non_member, params).execute } + + it 'returns nil' do + expect(subject).to eq nil + end + end + end + + context 'merge request can not be found' do + let(:params) { { text: 'mergerequest show 12345' } } + + it 'returns nil' do + expect(subject).to eq nil + end + end + end + + context 'search as a subcommand' do + context 'with results' do + let(:params) { { text: "mergerequest search is the word" } } + + it 'returns the merge_request' do + expect(subject).to eq [merge_request] + end + end + + context 'without results' do + let(:params) { { text: 'mergerequest search mepmep' } } + + it 'returns an empty collection' do + expect(subject).to eq [] + end + end + end + + describe 'help_message' do + context 'issues are disabled' do + it 'returns nil' do + allow(described_class).to receive(:available?).and_return false + + expect(described_class.help_message(project)).to eq nil + end + end + end +end