From d7f298c177555a09ac06acc9ad037611f664cc9e Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Mon, 30 Jan 2017 12:12:57 +0100 Subject: [PATCH] Incorporate feedback --- lib/gitlab/chat_commands/command.rb | 2 +- lib/gitlab/chat_commands/help.rb | 4 ++-- lib/gitlab/chat_commands/presenters/access.rb | 14 ++++++++++++++ lib/gitlab/chat_commands/presenters/help.rb | 12 +++++++----- lib/gitlab/chat_commands/presenters/issue_new.rb | 4 +++- .../chat_commands/presenters/issue_search.rb | 4 +++- lib/gitlab/chat_commands/presenters/issue_show.rb | 11 ++++++++--- spec/lib/gitlab/chat_commands/command_spec.rb | 6 +----- .../chat_commands/presenters/issue_show_spec.rb | 10 ++++++++++ 9 files changed, 49 insertions(+), 18 deletions(-) diff --git a/lib/gitlab/chat_commands/command.rb b/lib/gitlab/chat_commands/command.rb index e7baa20356c..f34ed0f4cf2 100644 --- a/lib/gitlab/chat_commands/command.rb +++ b/lib/gitlab/chat_commands/command.rb @@ -18,7 +18,7 @@ module Gitlab Gitlab::ChatCommands::Presenters::Access.new.access_denied end else - Gitlab::ChatCommands::Help.new(project, current_user, params).execute(available_commands) + Gitlab::ChatCommands::Help.new(project, current_user, params).execute(available_commands, params[:text]) end end diff --git a/lib/gitlab/chat_commands/help.rb b/lib/gitlab/chat_commands/help.rb index e76733f5445..6c0e4d304a4 100644 --- a/lib/gitlab/chat_commands/help.rb +++ b/lib/gitlab/chat_commands/help.rb @@ -16,8 +16,8 @@ module Gitlab true end - def execute(commands) - Gitlab::ChatCommands::Presenters::Help.new(commands).present(trigger) + def execute(commands, text) + Gitlab::ChatCommands::Presenters::Help.new(commands).present(trigger, text) end def trigger diff --git a/lib/gitlab/chat_commands/presenters/access.rb b/lib/gitlab/chat_commands/presenters/access.rb index b66ef48d6a8..92f4fa17f78 100644 --- a/lib/gitlab/chat_commands/presenters/access.rb +++ b/lib/gitlab/chat_commands/presenters/access.rb @@ -20,6 +20,20 @@ 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/chat_commands/presenters/help.rb b/lib/gitlab/chat_commands/presenters/help.rb index 39ad3249f5b..cd47b7f4c6a 100644 --- a/lib/gitlab/chat_commands/presenters/help.rb +++ b/lib/gitlab/chat_commands/presenters/help.rb @@ -2,17 +2,19 @@ module Gitlab module ChatCommands module Presenters class Help < Presenters::Base - def present(trigger) - ephemeral_response(text: help_message(trigger)) + def present(trigger, text) + ephemeral_response(text: help_message(trigger, text)) end private - def help_message(trigger) - if @resource.present? + def help_message(trigger, text) + return "No commands available :thinking_face:" unless @resource.present? + + if text.start_with?('help') header_with_list("Available commands", full_commands(trigger)) else - "No commands available :thinking_face:" + header_with_list("Unknown command, these commands are available", full_commands(trigger)) end end diff --git a/lib/gitlab/chat_commands/presenters/issue_new.rb b/lib/gitlab/chat_commands/presenters/issue_new.rb index d26dd22b2a0..6e88e0574a3 100644 --- a/lib/gitlab/chat_commands/presenters/issue_new.rb +++ b/lib/gitlab/chat_commands/presenters/issue_new.rb @@ -24,7 +24,9 @@ module Gitlab fields: fields, mrkdwn_in: [ :title, - :text + :pretext, + :text, + :fields ] } ] diff --git a/lib/gitlab/chat_commands/presenters/issue_search.rb b/lib/gitlab/chat_commands/presenters/issue_search.rb index d58a6d6114a..3478359b91d 100644 --- a/lib/gitlab/chat_commands/presenters/issue_search.rb +++ b/lib/gitlab/chat_commands/presenters/issue_search.rb @@ -7,6 +7,8 @@ module Gitlab def present text = if @resource.count >= 5 "Here are the first 5 issues I found:" + elsif @resource.one? + "Here is the only issue I found:" else "Here are the #{@resource.count} issues I found:" end @@ -26,7 +28,7 @@ module Gitlab text: "#{url} · #{issue.title} (#{status_text(issue)})", mrkdwn_in: [ - "text" + :text ] } end diff --git a/lib/gitlab/chat_commands/presenters/issue_show.rb b/lib/gitlab/chat_commands/presenters/issue_show.rb index 2fc671f13a6..fe5847ccd15 100644 --- a/lib/gitlab/chat_commands/presenters/issue_show.rb +++ b/lib/gitlab/chat_commands/presenters/issue_show.rb @@ -5,7 +5,11 @@ module Gitlab include Presenters::Issuable def present - in_channel_response(show_issue) + if @resource.confidential? + ephemeral_response(show_issue) + else + in_channel_response(show_issue) + end end private @@ -25,7 +29,8 @@ module Gitlab fields: fields, mrkdwn_in: [ :pretext, - :text + :text, + :fields ] } ] @@ -48,7 +53,7 @@ module Gitlab end def pretext - "Issue *#{@resource.to_reference} from #{project.name_with_namespace}" + "Issue *#{@resource.to_reference}* from #{project.name_with_namespace}" end end end diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb index 0acf40de1d3..b6e924d67be 100644 --- a/spec/lib/gitlab/chat_commands/command_spec.rb +++ b/spec/lib/gitlab/chat_commands/command_spec.rb @@ -5,7 +5,6 @@ describe Gitlab::ChatCommands::Command, service: true do let(:user) { create(:user) } describe '#execute' do -<<<<<<< HEAD subject do described_class.new(project, user, params).execute end @@ -19,16 +18,13 @@ describe Gitlab::ChatCommands::Command, service: true do expect(subject[:text]).to start_with('404 not found') end end -======= - subject { described_class.new(project, user, params).execute } ->>>>>>> Chat Commands have presenters context 'when an unknown command is triggered' do let(:params) { { command: '/gitlab', text: "unknown command 123" } } it 'displays the help message' do expect(subject[:response_type]).to be(:ephemeral) - expect(subject[:text]).to start_with('Available commands') + expect(subject[:text]).to start_with('Unknown command') expect(subject[:text]).to match('/gitlab issue show') 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 index 89d154e26e4..5b678d31fce 100644 --- a/spec/lib/gitlab/chat_commands/presenters/issue_show_spec.rb +++ b/spec/lib/gitlab/chat_commands/presenters/issue_show_spec.rb @@ -21,7 +21,17 @@ describe Gitlab::ChatCommands::Presenters::IssueShow do end it 'shows the upvote count' do + expect(subject[:response_type]).to be(:in_channel) expect(attachment[:text]).to start_with("**Open** · :+1: 1") end end + + context 'confidential issue' do + let(:issue) { create(:issue, project: project) } + + it 'shows an ephemeral response' do + expect(subject[:response_type]).to be(:in_channel) + expect(attachment[:text]).to start_with("**Open**") + end + end end