Incorporate feedback

9 files changed, 279 insertions, 180 deletions

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