Handle commands only text

- When the email body contains only quick commands
 a trimmed trply should not be appended
 - Update specs

lib/gitlab/email/handler/create_note_handler.rb

@@ -25,7 +25,7 @@ module Gitlab
           validate_permission!(:create_note)
 
           raise NoteableNotFoundError unless noteable
-          raise EmptyEmailError if message_including_truncated_reply.blank?
+          raise EmptyEmailError if message_with_appended_reply.blank?
 
           verify_record!(
             record: create_note,
@@ -48,7 +48,7 @@ module Gitlab
         end
 
         def create_note
-          sent_notification.create_reply(message_including_truncated_reply)
+          sent_notification.create_reply(message_with_appended_reply)
         end
       end
     end

lib/gitlab/email/handler/reply_processing.rb

@@ -35,17 +35,20 @@ module Gitlab
           @message_with_reply ||= process_message(trim_reply: false)
         end
 
-        def message_including_truncated_reply
-          @message_including_truncated_reply ||= process_message(trim_reply: false, append_reply: true)
+        def message_with_appended_reply
+          @message_with_appended_reply ||= process_message(append_reply: true)
         end
 
         def process_message(**kwargs)
           message = ReplyParser.new(mail, **kwargs).execute.strip
           message_with_attachments = add_attachments(message)
 
-          # Support bot is specifically forbidden
-          # from using slash commands.
-          strip_quick_actions(message_with_attachments)
+          # Support bot is specifically forbidden from using slash commands.
+          message = strip_quick_actions(message_with_attachments)
+          return message unless kwargs[:append_reply].present?
+
+          # Appended details section should be removed if the message only contains slash commands.
+          strip_details_section(message)
         end
 
         def add_attachments(reply)
@@ -96,10 +99,20 @@ module Gitlab
         def strip_quick_actions(content)
           return content unless author.support_bot?
 
-          command_definitions = ::QuickActions::InterpretService.command_definitions
-          extractor = ::Gitlab::QuickActions::Extractor.new(command_definitions)
+          quick_actions_extractor.redact_commands(content)
+        end
+
+        def strip_details_section(content)
+          body, commands = quick_actions_extractor.extract_commands(content)
+          return content if commands.empty?
+          return content unless body =~ /\A(<details>)/
 
-          extractor.redact_commands(content)
+          content.sub(body, '').chomp
+        end
+
+        def quick_actions_extractor
+          command_definitions = ::QuickActions::InterpretService.command_definitions
+          ::Gitlab::QuickActions::Extractor.new(command_definitions)
         end
       end
     end

lib/gitlab/email/reply_parser.rb

@@ -16,13 +16,7 @@ module Gitlab
         body = select_body(message)
 
         encoding = body.encoding
-
-        if @trim_reply
-          body = trim_reply(body)
-        elsif @append_reply
-          body = trim_reply(body, append_trimmed_reply: true)
-        end
-
+        body = trim_reply(body, append_trimmed_reply: @append_reply) if @trim_reply
         return '' unless body
 
         # not using /\s+$/ here because that deletes empty lines
@@ -77,12 +71,11 @@ module Gitlab
 
       def trim_reply(text, append_trimmed_reply: false)
         trimmed_body = EmailReplyTrimmer.trim(text)
+        return '' unless trimmed_body
         return trimmed_body unless append_trimmed_reply
 
-        partitioned_body = text.partition(trimmed_body)
-        main_body = partitioned_body[1]
-        stripped_text = partitioned_body[2].chomp
-        return main_body if stripped_text.empty?
+        _, main_body, stripped_text = text.partition(trimmed_body)
+        return main_body if stripped_text.chomp.empty?
 
         main_body + "\n<details><summary>...</summary>\n#{stripped_text}\n</details>"
       end

spec/fixtures/emails/valid_note_with_quote.eml

-Return-Path: <jake@adventuretime.ooo>
-Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400
-Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400
-Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+gitlabhq-gitlabhq-project_id-auth_token-issue-issue_iid@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700
-Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700
-Date: Thu, 13 Jun 2013 17:03:48 -0400
-From: Jake the Dog <jake@adventuretime.ooo>
-To: incoming+gitlabhq-gitlabhq-project_id-auth_token-issue-issue_iid@appmail.adventuretime.ooo
-Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
-Subject: New Issue comment by email
-Mime-Version: 1.0
-Content-Type: text/plain;
- charset=ISO-8859-1
-Content-Transfer-Encoding: 7bit
-X-Sieve: CMU Sieve 2.2
-X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu,
- 13 Jun 2013 14:03:48 -0700 (PDT)
-X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1
-
-> This is a quote
-
--- This is a line with 2 dashes
-
-*This is bold text*
-
-- This is a signature
-
-> Final quote
\ No newline at end of file

spec/lib/gitlab/email/handler/create_note_handler_spec.rb

@@ -110,6 +110,38 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do
         it_behaves_like 'an email that contains a mail key', 'References'
       end
     end
+
+    context 'when email contains quoted text' do
+      context 'when email contains quoted text only' do
+        let(:email_raw) { fixture_file('emails/no_content_reply.eml') }
+
+        it 'creates a discussion without appended reply' do
+          expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+          new_note = noteable.notes.last
+
+          expect(new_note.note).not_to include('<details><summary>...</summary>')
+        end
+      end
+
+      context 'when email contains quoted text and quick commands only' do
+        let(:email_raw) { fixture_file('emails/commands_only_reply.eml') }
+
+        it 'does not create a discussion' do
+          expect { receiver.execute }.not_to change { noteable.notes.count }
+        end
+      end
+
+      context 'when email contains text, quoted text and quick commands' do
+        let(:email_raw) { fixture_file('emails/commands_in_reply.eml') }
+
+        it 'creates a discussion with appended reply' do
+          expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+          new_note = noteable.notes.last
+
+          expect(new_note.note).to include('<details><summary>...</summary>')
+        end
+      end
+    end
   end
 
   context 'when note is not a discussion' do

spec/lib/gitlab/email/reply_parser_spec.rb

@@ -229,43 +229,21 @@ RSpec.describe Gitlab::Email::ReplyParser do
         )
     end
 
-    context 'when append_reply option is true' do
-      it "appends trimmed reply when body includes a quote at the end" do
-        expect(test_parse_body(fixture_file("emails/valid_new_issue_with_quote.eml"), { trim_reply: false, append_reply: true }))
-          .to eq(
-            <<-BODY.strip_heredoc.chomp
-            The reply by email functionality should be extended to allow creating a new issue by email.
-            even when the email is forwarded to the project which may include lines that begin with ">"
-
-            there should be a quote below this line:
-            <details><summary>...</summary>
-
-
-            > this is a quote
-            </details>
-            BODY
-          )
-      end
-
-      it 'appends trimmed reply when body includes a quote at the beginning' do
-        expect(test_parse_body(fixture_file("emails/valid_note_with_quote.eml"), { trim_reply: false, append_reply: true }))
-          .to eq(
-            <<-BODY.strip_heredoc.chomp
-            > This is a quote
-
-            -- This is a line with 2 dashes
-
-            *This is bold text*
+    it "appends trimmed reply when when append_reply option is true" do
+      expect(test_parse_body(fixture_file("emails/valid_new_issue_with_quote.eml"), { append_reply: true }))
+        .to eq(
+          <<-BODY.strip_heredoc.chomp
+          The reply by email functionality should be extended to allow creating a new issue by email.
+          even when the email is forwarded to the project which may include lines that begin with ">"
 
-            - This is a signature
-            <details><summary>...</summary>
+          there should be a quote below this line:
+          <details><summary>...</summary>
 
 
-            > Final quote
-            </details>
-            BODY
-          )
-      end
+          > this is a quote
+          </details>
+          BODY
+        )
     end
   end
 end