kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Implement #3243 New Issue by email

Browse source 
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.
This commit is contained in:
Lin Jen-Shin 2016-03-23 22:20:22 +08:00
parent 2375b437bd
commit 6cfd028278
3 changed files with 147 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

87
lib/gitlab/email/receiver.rb
View file

@ -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
elsif message_project
process_create_issue
else
# TODO: could also be project not found
raise SentNotificationNotFoundError
end
end
private
def process_reply
raise AutoGeneratedEmailError if message.header.to_s =~ /auto-(generated|replied)/
author = sent_notification.recipient
raise UserNotFoundError unless author
raise UserBlockedError if author.blocked?
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]}"

23
spec/fixtures/emails/valid_new_issue.eml vendored Normal file
View file

@ -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.

66
spec/lib/gitlab/email/receiver_spec.rb
View file

@ -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) { "![image](uploads/image.png)" }
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) { "![image](uploads/image.png)" }
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