Move apend reply logic to ReplyProcessing

- Update specs

lib/gitlab/email/handler/reply_processing.rb

@@ -40,15 +40,14 @@ module Gitlab
         end
 
         def process_message(**kwargs)
-          message = ReplyParser.new(mail, **kwargs).execute.strip
-          message_with_attachments = add_attachments(message)
+          message, stripped_text = ReplyParser.new(mail, **kwargs).execute
+          message = message.strip
 
           # Support bot is specifically forbidden from using slash commands.
-          message = strip_quick_actions(message_with_attachments)
-          return message unless kwargs[:append_reply].present?
+          message = strip_quick_actions(message)
+          message = append_reply(message, stripped_text) if kwargs[:append_reply]
 
-          # Appended details section should be removed if the message only contains slash commands.
-          strip_details_section(message)
+          add_attachments(message)
         end
 
         def add_attachments(reply)
@@ -102,18 +101,20 @@ module Gitlab
           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>)/
-
-          content.sub(body, '').chomp
-        end
-
         def quick_actions_extractor
           command_definitions = ::QuickActions::InterpretService.command_definitions
           ::Gitlab::QuickActions::Extractor.new(command_definitions)
         end
+
+        def append_reply(message, reply)
+          return message if message.blank? || reply.blank?
+
+          # Do not append if message only contains slash commands
+          body, _commands = quick_actions_extractor.extract_commands(message)
+          return message if body.empty?
+
+          message + "\n\n<details><summary>...</summary>\n\n#{reply}\n\n</details>"
+        end
       end
     end
   end

lib/gitlab/email/reply_parser.rb

@@ -16,7 +16,7 @@ module Gitlab
         body = select_body(message)
 
         encoding = body.encoding
-        body = trim_reply(body, append_trimmed_reply: @append_reply) if @trim_reply
+        body, stripped_text = EmailReplyTrimmer.trim(body, @append_reply) if @trim_reply
         return '' unless body
 
         # not using /\s+$/ here because that deletes empty lines
@@ -27,7 +27,9 @@ module Gitlab
         # so we detect it manually here.
         return "" if body.lines.all? { |l| l.strip.empty? || l.start_with?('>') }
 
-        body.force_encoding(encoding).encode("UTF-8")
+        encoded_body = body.force_encoding(encoding).encode("UTF-8")
+
+        @append_reply ? [encoded_body, stripped_text] : encoded_body
       end
 
       private
@@ -69,14 +71,12 @@ module Gitlab
         nil
       end
 
-      def trim_reply(text, append_trimmed_reply: false)
-        trimmed_body, stripped_text = EmailReplyTrimmer.trim(text, true)
-
-        return '' unless trimmed_body.present?
-        return trimmed_body unless stripped_text.present? && append_trimmed_reply
+      # def trim_reply(text, append_trimmed_reply: false)
+      #   trimmed_body, stripped_text = EmailReplyTrimmer.trim(text, append_trimmed_reply)
+      #   return trimmed_body if trimmed_body.blank? || stripped_text.blank?
 
-        trimmed_body + "\n\n<details><summary>...</summary>\n\n#{stripped_text}\n\n</details>"
-      end
+      #   trimmed_body + "\n\n<details><summary>...</summary>\n\n#{stripped_text}\n\n</details>"
+      # end
     end
   end
 end

spec/lib/gitlab/email/reply_parser_spec.rb

@@ -230,21 +230,19 @@ RSpec.describe Gitlab::Email::ReplyParser do
     end
 
     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
+      body = <<-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:
+      BODY
 
-          <details><summary>...</summary>
-
+      reply = <<-BODY.strip_heredoc.chomp
       > this is a quote
-
-          </details>
       BODY
-        )
+
+      expect(test_parse_body(fixture_file("emails/valid_new_issue_with_quote.eml"), { append_reply: true }))
+        .to contain_exactly(body, reply)
     end
   end
 end