diff options
author | Cameron Crockett <cameron@ccrockett.com> | 2018-05-09 23:19:16 +0300 |
---|---|---|
committer | Cameron Crockett <cameron@ccrockett.com> | 2018-05-23 07:05:51 +0300 |
commit | 421dbb1746e7944a578d2dc894ae3975dac94ac8 (patch) | |
tree | b7dc90958be9f94ffe0f57bb56b5a590f21c02d1 | |
parent | 76e276cb433a7023cf8154f9d9555725b3b11e67 (diff) |
skip email trim when email is creating new issue
Updates from MR discussion
1. Added test for ReplyParser
2. Changed param to trim_reply with default set as true
Removed keyword param in favor of normal options param
updates for MR discussion
Resolutions for code review comments
more code review fixes
-rw-r--r-- | changelogs/unreleased/issue-25256.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/email/handler/create_issue_handler.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/email/handler/reply_processing.rb | 8 | ||||
-rw-r--r-- | lib/gitlab/email/reply_parser.rb | 7 | ||||
-rw-r--r-- | spec/fixtures/emails/valid_new_issue_with_quote.eml | 25 | ||||
-rw-r--r-- | spec/lib/gitlab/email/handler/create_issue_handler_spec.rb | 14 | ||||
-rw-r--r-- | spec/lib/gitlab/email/reply_parser_spec.rb | 18 |
7 files changed, 72 insertions, 7 deletions
diff --git a/changelogs/unreleased/issue-25256.yml b/changelogs/unreleased/issue-25256.yml new file mode 100644 index 00000000000..e981b5f9a8f --- /dev/null +++ b/changelogs/unreleased/issue-25256.yml @@ -0,0 +1,5 @@ +--- +title: Don't trim incoming emails that create new issues +merge_request: +author: Cameron Crockett +type: fixed diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb index 05a60deb7d3..764f93f6d3d 100644 --- a/lib/gitlab/email/handler/create_issue_handler.rb +++ b/lib/gitlab/email/handler/create_issue_handler.rb @@ -47,7 +47,7 @@ module Gitlab project, author, title: mail.subject, - description: message + description: message_including_reply ).execute end end diff --git a/lib/gitlab/email/handler/reply_processing.rb b/lib/gitlab/email/handler/reply_processing.rb index da5ff350549..38b1425364f 100644 --- a/lib/gitlab/email/handler/reply_processing.rb +++ b/lib/gitlab/email/handler/reply_processing.rb @@ -16,8 +16,12 @@ module Gitlab @message ||= process_message end - def process_message - message = ReplyParser.new(mail).execute.strip + def message_including_reply + @message_with_reply ||= process_message(trim_reply: false) + end + + def process_message(**kwargs) + message = ReplyParser.new(mail, **kwargs).execute.strip add_attachments(message) end diff --git a/lib/gitlab/email/reply_parser.rb b/lib/gitlab/email/reply_parser.rb index 01c28d051ee..ae6b84607d6 100644 --- a/lib/gitlab/email/reply_parser.rb +++ b/lib/gitlab/email/reply_parser.rb @@ -4,8 +4,9 @@ module Gitlab class ReplyParser attr_accessor :message - def initialize(message) + def initialize(message, trim_reply: true) @message = message + @trim_reply = trim_reply end def execute @@ -13,7 +14,9 @@ module Gitlab encoding = body.encoding - body = EmailReplyTrimmer.trim(body) + if @trim_reply + body = EmailReplyTrimmer.trim(body) + end return '' unless body diff --git a/spec/fixtures/emails/valid_new_issue_with_quote.eml b/spec/fixtures/emails/valid_new_issue_with_quote.eml new file mode 100644 index 00000000000..0caf8ed4e9e --- /dev/null +++ b/spec/fixtures/emails/valid_new_issue_with_quote.eml @@ -0,0 +1,25 @@ +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@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+auth_token@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: New Issue 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 + +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: + +> this is a quote
\ No newline at end of file diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb index 452249210b0..154ab4b3856 100644 --- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb @@ -46,6 +46,20 @@ describe Gitlab::Email::Handler::CreateIssueHandler do expect(issue.description).to eq('') end end + + context "when there are quotes in email" do + let(:email_raw) { fixture_file("emails/valid_new_issue_with_quote.eml") } + + it "creates a new issue" do + expect { receiver.execute }.to change { project.issues.count }.by(1) + issue = project.issues.last + + expect(issue.author).to eq(user) + expect(issue.title).to eq('New Issue by email') + expect(issue.description).to include('reply by email') + expect(issue.description).to include('> this is a quote') + end + end end context "something is wrong" do diff --git a/spec/lib/gitlab/email/reply_parser_spec.rb b/spec/lib/gitlab/email/reply_parser_spec.rb index e21a998adfe..0989188f7ee 100644 --- a/spec/lib/gitlab/email/reply_parser_spec.rb +++ b/spec/lib/gitlab/email/reply_parser_spec.rb @@ -3,8 +3,8 @@ require "spec_helper" # Inspired in great part by Discourse's Email::Receiver describe Gitlab::Email::ReplyParser do describe '#execute' do - def test_parse_body(mail_string) - described_class.new(Mail::Message.new(mail_string)).execute + def test_parse_body(mail_string, params = {}) + described_class.new(Mail::Message.new(mail_string), params).execute end it "returns an empty string if the message is blank" do @@ -212,5 +212,19 @@ describe Gitlab::Email::ReplyParser do it "does not wrap links with no href in unnecessary brackets" do expect(test_parse_body(fixture_file("emails/html_empty_link.eml"))).to eq("no brackets!") end + + it "does not trim reply if trim_reply option is false" do + expect(test_parse_body(fixture_file("emails/valid_new_issue_with_quote.eml"), { trim_reply: false })) + .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: + + > this is a quote + BODY + ) + end end end |