Automatic merge of gitlab-org/gitlab-ce master

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

changelogs/unreleased/improve-chatops-help.yml

+---
+title: Improve chatops help output
+merge_request: 32208
+author:
+type: changed

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

lib/gitlab/slash_commands/command.rb

@@ -22,7 +22,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)

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

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

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

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

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
 

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

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