From 6cfd028278e7fe22c2776b9ce70a5b92223115f9 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 23 Mar 2016 22:20:22 +0800
Subject: [PATCH] Implement #3243 New Issue by email
So we extend Gitlab::Email::Receiver for this new behaviour,
however we might want to split it into another class for better
testing it.
Another issue is that, currently it's using this to parse project
identifier:
Gitlab::IncomingEmail.key_from_address
Which is using:
Gitlab.config.incoming_email.address
for the receiver name. This is probably `reply` because it's used
for replying to a specific issue. We might want to introduce another
config for this, or just use `reply` instead of `incoming`.
I'll prefer to introduce a new config for this, or just change
`reply` to `incoming` because it would make sense for replying to
there, too.
The email template used in tests were copied and modified from:
`emails/valid_reply.eml` which I hope is ok.
---
lib/gitlab/email/receiver.rb | 87 +++++++++++++++++++-----
spec/fixtures/emails/valid_new_issue.eml | 23 +++++++
spec/lib/gitlab/email/receiver_spec.rb | 66 ++++++++++++++----
3 files changed, 147 insertions(+), 29 deletions(-)
create mode 100644 spec/fixtures/emails/valid_new_issue.eml
diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb
index 97ef9851d71..2b57b3a20fc 100644
--- a/lib/gitlab/email/receiver.rb
+++ b/lib/gitlab/email/receiver.rb
@@ -12,6 +12,7 @@ module Gitlab
class UserNotAuthorizedError < ProcessingError; end
class NoteableNotFoundError < ProcessingError; end
class InvalidNoteError < ProcessingError; end
+ class InvalidIssueError < ProcessingError; end
def initialize(raw)
@raw = raw
@@ -20,29 +21,30 @@ module Gitlab
def execute
raise EmptyEmailError if @raw.blank?
- raise SentNotificationNotFoundError unless sent_notification
+ if sent_notification
+ process_reply
- raise AutoGeneratedEmailError if message.header.to_s =~ /auto-(generated|replied)/
-
- author = sent_notification.recipient
+ elsif message_project
+ process_create_issue
- raise UserNotFoundError unless author
+ else
+ # TODO: could also be project not found
+ raise SentNotificationNotFoundError
+ end
+ end
- raise UserBlockedError if author.blocked?
+ private
+ def process_reply
+ raise AutoGeneratedEmailError if message.header.to_s =~ /auto-(generated|replied)/
+ author = sent_notification.recipient
project = sent_notification.project
- raise UserNotAuthorizedError unless project && author.can?(:create_note, project)
+ check_input(author, project, :create_note)
raise NoteableNotFoundError unless sent_notification.noteable
- reply = ReplyParser.new(message).execute.strip
-
- raise EmptyEmailError if reply.blank?
-
- reply = add_attachments(reply)
-
- note = create_note(reply)
+ note = create_note(extract_reply(project))
unless note.persisted?
msg = "The comment could not be created for the following reasons:"
@@ -54,7 +56,58 @@ module Gitlab
end
end
- private
+ def process_create_issue
+ check_input(message_sender, message_project, :create_issue)
+
+ issue = Issues::CreateService.new(message_project, message_sender,
+ title: message.subject,
+ description: extract_reply(message_project)).execute
+
+ unless issue.persisted?
+ msg = "The issue could not be created for the following reasons:"
+ issue.errors.full_messages.each do |error|
+ msg << "\n\n- #{error}"
+ end
+
+ raise InvalidIssueError, msg
+ end
+ end
+
+ def check_input(author, project, permission)
+ if author
+ if author.blocked?
+ raise UserBlockedError
+ elsif project.nil? || !author.can?(permission, project)
+ # TODO: Give project not found error if author cannot read project
+ raise UserNotAuthorizedError
+ end
+ else
+ raise UserNotFoundError
+ end
+ end
+
+ # Find the first matched user in database from email From: section
+ def message_sender
+ @message_sender ||= message.from.find do |email|
+ user = User.find_by_any_email(email)
+ break user if user
+ end
+ end
+
+ def message_project
+ @message_project ||=
+ Project.find_with_namespace(reply_key) if reply_key
+ end
+
+ def extract_reply project
+ reply = ReplyParser.new(message).execute.strip
+
+ raise EmptyEmailError if reply.blank?
+
+ add_attachments(reply, project)
+
+ reply
+ end
def message
@message ||= Mail::Message.new(@raw)
@@ -93,8 +146,8 @@ module Gitlab
SentNotification.for(reply_key)
end
- def add_attachments(reply)
- attachments = Email::AttachmentUploader.new(message).execute(sent_notification.project)
+ def add_attachments(reply, project)
+ attachments = Email::AttachmentUploader.new(message).execute(project)
attachments.each do |link|
reply << "\n\n#{link[:markdown]}"
diff --git a/spec/fixtures/emails/valid_new_issue.eml b/spec/fixtures/emails/valid_new_issue.eml
new file mode 100644
index 00000000000..a7e34b9846d
--- /dev/null
+++ b/spec/fixtures/emails/valid_new_issue.eml
@@ -0,0 +1,23 @@
+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@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.
+
+* Allow an admin to specify which project the issue should be created under by checking the sender domain.
+* Possibly allow the use of regular expression matches within the subject/body to specify which project the issue should be created under.
diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb
index f381a3907f3..e7391a33751 100644
--- a/spec/lib/gitlab/email/receiver_spec.rb
+++ b/spec/lib/gitlab/email/receiver_spec.rb
@@ -15,6 +15,20 @@ describe Gitlab::Email::Receiver, lib: true do
let!(:sent_notification) { SentNotification.record(noteable, user.id, reply_key) }
let(:receiver) { described_class.new(email_raw) }
+ let(:markdown) { "" }
+
+ def setup_attachment
+ allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return(
+ [
+ {
+ url: "uploads/image.png",
+ is_image: true,
+ alt: "image",
+ markdown: markdown
+ }
+ ]
+ )
+ end
context "when the recipient address doesn't include a reply key" do
let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(reply_key, "") }
@@ -108,19 +122,8 @@ describe Gitlab::Email::Receiver, lib: true do
end
context "when everything is fine" do
- let(:markdown) { "" }
-
before do
- allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return(
- [
- {
- url: "uploads/image.png",
- is_image: true,
- alt: "image",
- markdown: markdown
- }
- ]
- )
+ setup_attachment
end
it "creates a comment" do
@@ -161,4 +164,43 @@ describe Gitlab::Email::Receiver, lib: true do
end
end
end
+
+ context "when it's trying to create a new issue" do
+ before do
+ setup_attachment
+ stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.adventuretime.ooo")
+ end
+
+ let(:sent_notification) {}
+ let!(:user) { create(:user, email: 'jake@adventuretime.ooo') }
+ let(:namespace) { create(:namespace, path: 'gitlabhq') }
+ let(:project) { create(:project, :public, namespace: namespace) }
+ let(:email_raw) { fixture_file('emails/valid_new_issue.eml') }
+
+ context "when everything is fine" do
+ 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(markdown)
+ end
+ end
+
+ context "something is wrong" do
+ context "when the issue could not be saved" do
+ before do
+ project
+
+ allow_any_instance_of(Issue).to receive(:persisted?).and_return(false)
+ end
+
+ it "raises an InvalidIssueError" do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::InvalidIssueError)
+ end
+ end
+ end
+ end
end
--
GitLab