From e8dd299e7cb4fbb622359d762089367267ed5c09 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 26 Aug 2019 14:23:36 +0200 Subject: [PATCH] Improve chatops help output This improves the output produced when running an unknown command, running the "help" command, and when trying to run a command you are not allowed to run. The new help output includes links to the project of the chatops integration, and a link to the chatops documentation. --- .../slash_commands_service.rb | 4 +- .../unreleased/improve-chatops-help.yml | 5 ++ lib/gitlab/slash_commands/application_help.rb | 7 ++- lib/gitlab/slash_commands/command.rb | 2 +- lib/gitlab/slash_commands/help.rb | 4 +- .../slash_commands/presenters/access.rb | 25 +++----- lib/gitlab/slash_commands/presenters/help.rb | 62 +++++++++++++++++-- .../slash_commands/application_help_spec.rb | 3 +- .../lib/gitlab/slash_commands/command_spec.rb | 6 +- .../slash_commands/presenters/access_spec.rb | 6 +- .../chat_slash_commands_shared_examples.rb | 2 +- 11 files changed, 93 insertions(+), 33 deletions(-) create mode 100644 changelogs/unreleased/improve-chatops-help.yml diff --git a/app/models/project_services/slash_commands_service.rb b/app/models/project_services/slash_commands_service.rb index cb16ad75d14..5bfd06476f0 100644 --- a/app/models/project_services/slash_commands_service.rb +++ b/app/models/project_services/slash_commands_service.rb @@ -35,7 +35,9 @@ class SlashCommandsService < Service chat_user = find_chat_user(params) if chat_user&.user - return Gitlab::SlashCommands::Presenters::Access.new.access_denied unless chat_user.user.can?(:use_slash_commands) + unless chat_user.user.can?(:use_slash_commands) + return Gitlab::SlashCommands::Presenters::Access.new.access_denied(project) + end Gitlab::SlashCommands::Command.new(project, chat_user, params).execute else diff --git a/changelogs/unreleased/improve-chatops-help.yml b/changelogs/unreleased/improve-chatops-help.yml new file mode 100644 index 00000000000..77e6f2e5308 --- /dev/null +++ b/changelogs/unreleased/improve-chatops-help.yml @@ -0,0 +1,5 @@ +--- +title: Improve chatops help output +merge_request: 32208 +author: +type: changed diff --git a/lib/gitlab/slash_commands/application_help.rb b/lib/gitlab/slash_commands/application_help.rb index 0ea7554ba64..1a92346be15 100644 --- a/lib/gitlab/slash_commands/application_help.rb +++ b/lib/gitlab/slash_commands/application_help.rb @@ -3,12 +3,15 @@ module Gitlab module SlashCommands class ApplicationHelp < BaseCommand - def initialize(params) + def initialize(project, params) + @project = project @params = params end def execute - Gitlab::SlashCommands::Presenters::Help.new(commands).present(trigger, params[:text]) + Gitlab::SlashCommands::Presenters::Help + .new(project, commands) + .present(trigger, params[:text]) end private diff --git a/lib/gitlab/slash_commands/command.rb b/lib/gitlab/slash_commands/command.rb index 7c963fcf38a..45c34d8b86b 100644 --- a/lib/gitlab/slash_commands/command.rb +++ b/lib/gitlab/slash_commands/command.rb @@ -21,7 +21,7 @@ module Gitlab if command.allowed?(project, current_user) command.new(project, chat_name, params).execute(match) else - Gitlab::SlashCommands::Presenters::Access.new.access_denied + Gitlab::SlashCommands::Presenters::Access.new.access_denied(project) end else Gitlab::SlashCommands::Help.new(project, chat_name, params) diff --git a/lib/gitlab/slash_commands/help.rb b/lib/gitlab/slash_commands/help.rb index dbe15baa3d7..3eff64192ab 100644 --- a/lib/gitlab/slash_commands/help.rb +++ b/lib/gitlab/slash_commands/help.rb @@ -19,7 +19,9 @@ module Gitlab end def execute(commands, text) - Gitlab::SlashCommands::Presenters::Help.new(commands).present(trigger, text) + Gitlab::SlashCommands::Presenters::Help + .new(project, commands) + .present(trigger, text) end def trigger diff --git a/lib/gitlab/slash_commands/presenters/access.rb b/lib/gitlab/slash_commands/presenters/access.rb index fa163cb098e..b1bfaa6cb59 100644 --- a/lib/gitlab/slash_commands/presenters/access.rb +++ b/lib/gitlab/slash_commands/presenters/access.rb @@ -4,8 +4,15 @@ module Gitlab module SlashCommands 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/).") + def access_denied(project) + ephemeral_response(text: <<~MESSAGE) + You are not allowed to perform the given chatops command. Most + likely you do not have access to the GitLab project for this chatops + integration. + + The GitLab project for this chatops integration can be found at + #{url_for(project)}. + MESSAGE end def not_found @@ -22,20 +29,6 @@ module Gitlab ephemeral_response(text: message) end - - def unknown_command(commands) - ephemeral_response(text: help_message(trigger)) - end - - private - - def help_message(trigger) - header_with_list("Command not found, these are the commands you can use", full_commands(trigger)) - end - - def full_commands(trigger) - @resource.map { |command| "#{trigger} #{command.help_message}" } - end end end end diff --git a/lib/gitlab/slash_commands/presenters/help.rb b/lib/gitlab/slash_commands/presenters/help.rb index 480d7aa6a30..5421b0b9a84 100644 --- a/lib/gitlab/slash_commands/presenters/help.rb +++ b/lib/gitlab/slash_commands/presenters/help.rb @@ -4,6 +4,11 @@ module Gitlab module SlashCommands module Presenters class Help < Presenters::Base + def initialize(project, commands) + @project = project + @commands = commands + end + def present(trigger, text) ephemeral_response(text: help_message(trigger, text)) end @@ -11,17 +16,64 @@ module Gitlab private def help_message(trigger, text) - return "No commands available :thinking_face:" unless @resource.present? + unless @commands.present? + return <<~MESSAGE + This chatops integration does not have any commands that can be + executed. + + #{footer} + MESSAGE + end if text.start_with?('help') - header_with_list("Available commands", full_commands(trigger)) + <<~MESSAGE + #{full_commands_message(trigger)} + + #{help_footer} + MESSAGE else - header_with_list("Unknown command, these commands are available", full_commands(trigger)) + <<~MESSAGE + The specified command is not valid. + + #{full_commands_message(trigger)} + + #{help_footer} + MESSAGE end end - def full_commands(trigger) - @resource.map { |command| "#{trigger} #{command.help_message}" } + def help_footer + <<~MESSAGE + *Project* + + The GitLab project for this chatops integration can be found at + #{url_for(@project)}. + + *Documentation* + + For more information about GitLab chatops, refer to its + documentation: https://docs.gitlab.com/ce/ci/chatops/README.html. + MESSAGE + end + + def full_commands_message(trigger) + list = @commands + .map { |command| "#{trigger} #{command.help_message}" } + .join("\n") + + <<~MESSAGE + *Available commands* + + The following commands are available for this chatops integration: + + #{list} + + If available, the `run` command is used for running GitLab CI jobs + defined in this project's `.gitlab-ci.yml` file. For example, if a + job called "help" is defined you can run it like so: + + `#{trigger} run help` + MESSAGE end end end diff --git a/spec/lib/gitlab/slash_commands/application_help_spec.rb b/spec/lib/gitlab/slash_commands/application_help_spec.rb index b203a1ee79c..afa63c21584 100644 --- a/spec/lib/gitlab/slash_commands/application_help_spec.rb +++ b/spec/lib/gitlab/slash_commands/application_help_spec.rb @@ -4,10 +4,11 @@ require 'spec_helper' describe Gitlab::SlashCommands::ApplicationHelp do let(:params) { { command: '/gitlab', text: 'help' } } + let(:project) { build(:project) } describe '#execute' do subject do - described_class.new(params).execute + described_class.new(project, params).execute end it 'displays the help section' do diff --git a/spec/lib/gitlab/slash_commands/command_spec.rb b/spec/lib/gitlab/slash_commands/command_spec.rb index c4ea8cbf2b1..dc412c80e68 100644 --- a/spec/lib/gitlab/slash_commands/command_spec.rb +++ b/spec/lib/gitlab/slash_commands/command_spec.rb @@ -27,7 +27,7 @@ describe Gitlab::SlashCommands::Command do it 'displays the help message' do expect(subject[:response_type]).to be(:ephemeral) - expect(subject[:text]).to start_with('Unknown command') + expect(subject[:text]).to start_with('The specified command is not valid') expect(subject[:text]).to match('/gitlab issue show') end end @@ -37,7 +37,7 @@ describe Gitlab::SlashCommands::Command do it 'rejects the actions' do expect(subject[:response_type]).to be(:ephemeral) - expect(subject[:text]).to start_with('Whoops! This action is not allowed') + expect(subject[:text]).to start_with('You are not allowed') end end @@ -57,7 +57,7 @@ describe Gitlab::SlashCommands::Command do context 'and user can not create deployment' do it 'returns action' do expect(subject[:response_type]).to be(:ephemeral) - expect(subject[:text]).to start_with('Whoops! This action is not allowed') + expect(subject[:text]).to start_with('You are not allowed') end end diff --git a/spec/lib/gitlab/slash_commands/presenters/access_spec.rb b/spec/lib/gitlab/slash_commands/presenters/access_spec.rb index 286fec892e6..f00039c634f 100644 --- a/spec/lib/gitlab/slash_commands/presenters/access_spec.rb +++ b/spec/lib/gitlab/slash_commands/presenters/access_spec.rb @@ -4,12 +4,14 @@ require 'spec_helper' describe Gitlab::SlashCommands::Presenters::Access do describe '#access_denied' do - subject { described_class.new.access_denied } + let(:project) { build(:project) } + + subject { described_class.new.access_denied(project) } it { is_expected.to be_a(Hash) } it 'displays an error message' do - expect(subject[:text]).to match("is not allowed") + expect(subject[:text]).to match('are not allowed') expect(subject[:response_type]).to be(:ephemeral) end end diff --git a/spec/support/shared_examples/chat_slash_commands_shared_examples.rb b/spec/support/shared_examples/chat_slash_commands_shared_examples.rb index dcc92dda950..a99068ab678 100644 --- a/spec/support/shared_examples/chat_slash_commands_shared_examples.rb +++ b/spec/support/shared_examples/chat_slash_commands_shared_examples.rb @@ -103,7 +103,7 @@ RSpec.shared_examples 'chat slash commands service' do expect_any_instance_of(Gitlab::SlashCommands::Command).not_to receive(:execute) result = subject.trigger(params) - expect(result).to include(text: /^Whoops! This action is not allowed/) + expect(result).to include(text: /^You are not allowed/) end end end