Commit 421dbb17 authored by Cameron Crockett's avatar Cameron Crockett

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
parent 76e276cb
---
title: Don't trim incoming emails that create new issues
merge_request:
author: Cameron Crockett
type: fixed
...@@ -47,7 +47,7 @@ module Gitlab ...@@ -47,7 +47,7 @@ module Gitlab
project, project,
author, author,
title: mail.subject, title: mail.subject,
description: message description: message_including_reply
).execute ).execute
end end
end end
......
...@@ -16,8 +16,12 @@ module Gitlab ...@@ -16,8 +16,12 @@ module Gitlab
@message ||= process_message @message ||= process_message
end end
def process_message def message_including_reply
message = ReplyParser.new(mail).execute.strip @message_with_reply ||= process_message(trim_reply: false)
end
def process_message(**kwargs)
message = ReplyParser.new(mail, **kwargs).execute.strip
add_attachments(message) add_attachments(message)
end end
......
...@@ -4,8 +4,9 @@ module Gitlab ...@@ -4,8 +4,9 @@ module Gitlab
class ReplyParser class ReplyParser
attr_accessor :message attr_accessor :message
def initialize(message) def initialize(message, trim_reply: true)
@message = message @message = message
@trim_reply = trim_reply
end end
def execute def execute
...@@ -13,7 +14,9 @@ module Gitlab ...@@ -13,7 +14,9 @@ module Gitlab
encoding = body.encoding encoding = body.encoding
if @trim_reply
body = EmailReplyTrimmer.trim(body) body = EmailReplyTrimmer.trim(body)
end
return '' unless body return '' unless body
......
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
...@@ -46,6 +46,20 @@ describe Gitlab::Email::Handler::CreateIssueHandler do ...@@ -46,6 +46,20 @@ describe Gitlab::Email::Handler::CreateIssueHandler do
expect(issue.description).to eq('') expect(issue.description).to eq('')
end end
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 end
context "something is wrong" do context "something is wrong" do
......
...@@ -3,8 +3,8 @@ require "spec_helper" ...@@ -3,8 +3,8 @@ require "spec_helper"
# Inspired in great part by Discourse's Email::Receiver # Inspired in great part by Discourse's Email::Receiver
describe Gitlab::Email::ReplyParser do describe Gitlab::Email::ReplyParser do
describe '#execute' do describe '#execute' do
def test_parse_body(mail_string) def test_parse_body(mail_string, params = {})
described_class.new(Mail::Message.new(mail_string)).execute described_class.new(Mail::Message.new(mail_string), params).execute
end end
it "returns an empty string if the message is blank" do it "returns an empty string if the message is blank" do
...@@ -212,5 +212,19 @@ describe Gitlab::Email::ReplyParser do ...@@ -212,5 +212,19 @@ describe Gitlab::Email::ReplyParser do
it "does not wrap links with no href in unnecessary brackets" 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!") expect(test_parse_body(fixture_file("emails/html_empty_link.eml"))).to eq("no brackets!")
end 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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment