github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-05-22 21:31:04 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: failed messages posted via email silently ignored

Browse Source 
also... test suite over mocking
This commit is contained in:
Sam
2014-07-31 18:46:02 +10:00
parent 138d013e56
commit dcc9923e4b
6 changed files with 107 additions and 212 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
app/jobs/scheduled/poll_mailbox.rb
View File

@ -38,6 +38,9 @@ module Jobs
message_template = :email_reject_reply_key message_template = :email_reject_reply_key
when ActiveRecord::Rollback when ActiveRecord::Rollback
message_template = :email_reject_post_error message_template = :email_reject_post_error
when Email::Receiver::InvalidPost
# TODO there is a message in this exception, place it in email
message_template = :email_reject_post_error
else else
message_template = nil message_template = nil
end end

10
lib/email/receiver.rb
View File

@ -14,6 +14,7 @@ module Email
class UserNotFoundError < ProcessingError; end class UserNotFoundError < ProcessingError; end
class UserNotSufficientTrustLevelError < ProcessingError; end class UserNotSufficientTrustLevelError < ProcessingError; end
class EmailLogNotFound < ProcessingError; end class EmailLogNotFound < ProcessingError; end
class InvalidPost < ProcessingError; end
attr_reader :body, :reply_key, :email_log attr_reader :body, :reply_key, :email_log
@ -40,6 +41,8 @@ module Email
@user = User.find_by_email(@message.from.first) @user = User.find_by_email(@message.from.first)
if @user.blank? && @allow_strangers if @user.blank? && @allow_strangers
wrap_body_in_quote wrap_body_in_quote
# TODO This is WRONG it should register an account
# and email the user details on how to log in / activate
@user = Discourse.system_user @user = Discourse.system_user
end end
@ -213,7 +216,12 @@ module Email
end end
def create_post(user, options) def create_post(user, options)
PostCreator.new(user, options).create creator = PostCreator.new(user, options)
post = creator.create
if creator.errors.present?
raise InvalidPost, creator.errors.full_messages.join("\n")
end
post
end end
end end

290
spec/components/email/receiver_spec.rb
View File

@ -6,8 +6,8 @@ require 'email/receiver'
describe Email::Receiver do describe Email::Receiver do
before do before do
SiteSetting.stubs(:reply_by_email_address).returns("reply+%{reply_key}@appmail.adventuretime.ooo") SiteSetting.reply_by_email_address = "reply+%{reply_key}@appmail.adventuretime.ooo"
SiteSetting.stubs(:email_in).returns(false) SiteSetting.email_in = false
end end
describe 'invalid emails' do describe 'invalid emails' do
@ -126,70 +126,66 @@ Thanks for listening.")
end end
end end
def fill_email(mail, from, to, body = nil, subject = nil)
result = mail.gsub("FROM", from).gsub("TO", to)
if body
result.gsub!(/Hey.*/m, body)
end
if subject
result.sub!(/We .*/, subject)
end
result
end
def process_email(opts)
incoming_email = fixture_file("emails/valid_incoming.eml")
email = fill_email(incoming_email, opts[:from], opts[:to], opts[:body], opts[:subject])
Email::Receiver.new(email).process
end
describe "with a valid email" do describe "with a valid email" do
let(:reply_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" } let(:reply_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" }
let(:valid_reply) { fixture_file("emails/valid_reply.eml") } let(:to) { SiteSetting.reply_by_email_address.gsub("%{reply_key}", reply_key) }
let(:valid_reply) {
reply = fixture_file("emails/valid_reply.eml")
to = SiteSetting.reply_by_email_address.gsub("%{reply_key}", reply_key)
fill_email(reply, "test@test.com", to)
}
let(:receiver) { Email::Receiver.new(valid_reply) } let(:receiver) { Email::Receiver.new(valid_reply) }
let(:post) { Fabricate.build(:post) } let(:post) { create_post }
let(:user) { Fabricate.build(:user) } let(:user) { post.user }
let(:email_log) { EmailLog.new(reply_key: reply_key, post_id: 1234, topic_id: 4567, user_id: 6677, post: post, user: user ) } let(:email_log) { EmailLog.new(reply_key: reply_key,
post_id: post.id,
topic_id: post.topic_id,
user_id: post.user_id,
post: post,
user: user,
email_type: 'test',
to_address: 'test@test.com'
) }
let(:reply_body) { let(:reply_body) {
"I could not disagree more. I am obviously biased but adventure time is the "I could not disagree more. I am obviously biased but adventure time is the
greatest show ever created. Everyone should watch it. greatest show ever created. Everyone should watch it.
- Jake out" } - Jake out" }
describe "email with non-existant email log" do
before do
EmailLog.expects(:for).returns(nil)
end
it "raises EmailLogNotFoundError" do
expect{ receiver.process }.to raise_error(Email::Receiver::EmailLogNotFound)
end
end
describe "with an email log" do describe "with an email log" do
before do it "extracts data" do
EmailLog.expects(:for).with(reply_key).returns(email_log) expect{ receiver.process }.to raise_error(Email::Receiver::EmailLogNotFound)
creator = mock email_log.save!
PostCreator.expects(:new).with(instance_of(User), receiver.process
has_entries(raw: reply_body,
cooking_options: {traditional_markdown_linebreaks: true}))
.returns(creator)
creator.expects(:create)
end
let!(:result) { receiver.process }
it "extracts the body" do
expect(receiver.body).to eq(reply_body) expect(receiver.body).to eq(reply_body)
end
it "looks up the email log" do
expect(receiver.email_log).to eq(email_log) expect(receiver.email_log).to eq(email_log)
end
it "extracts the key" do
expect(receiver.reply_key).to eq(reply_key) expect(receiver.reply_key).to eq(reply_key)
end
end
describe "email with attachments" do
it "can find the message and create a post" do
user.id = -1
User.stubs(:find_by_email).returns(user)
EmailLog.stubs(:for).returns(email_log)
attachment_email = fixture_file("emails/attachment.eml") attachment_email = fixture_file("emails/attachment.eml")
attachment_email = fill_email(attachment_email, "test@test.com", to)
r = Email::Receiver.new(attachment_email) r = Email::Receiver.new(attachment_email)
r.expects(:create_post)
expect { r.process }.to_not raise_error expect { r.process }.to_not raise_error
expect(r.body).to match(/here is an image attachment\n<img src='\/uploads\/default\/\d+\/\w{16}\.png' width='289' height='126'>\n/) expect(r.body).to match(/here is an image attachment\n<img src='\/uploads\/default\/\d+\/\w{16}\.png' width='289' height='126'>\n/)
end end
@ -200,106 +196,48 @@ greatest show ever created. Everyone should watch it.
describe "processes an email to a category" do describe "processes an email to a category" do
before do before do
SiteSetting.stubs(:email_in).returns(true) SiteSetting.email_in = true
end end
let(:incoming_email) { fixture_file("emails/valid_incoming.eml") }
let(:receiver) { Email::Receiver.new(incoming_email) }
let(:user) { Fabricate.build(:user, id: 3456) }
let(:category) { Fabricate.build(:category, id: 10) }
let(:subject) { "We should have a post-by-email-feature." }
let(:email_body) {
"Hey folks,
I was thinking. Wouldn't it be great if we could post topics via email? Yes it would! it "correctly can target categories" do
to = "some@email.com"
Jakie" } Fabricate(:category, email_in_allow_strangers: false, email_in: to)
SiteSetting.email_in_min_trust = TrustLevel.levels[:elder].to_s
describe "category not found" do # no email in for user
expect{
process_email(from: "cobb@dob.com", to: "invalid@address.com")
}.to raise_error(Email::Receiver::EmailLogNotFound)
before do # valid target invalid user
Category.expects(:find_by_email).returns(nil) expect{
end process_email(from: "cobb@dob.com", to: to)
}.to raise_error(Email::Receiver::UserNotFoundError)
it "raises EmailLogNotFoundError" do
expect{ receiver.process }.to raise_error(Email::Receiver::EmailLogNotFound) # untrusted
end user = Fabricate(:user)
expect{
end process_email(from: user.email, to: to)
}.to raise_error(Email::Receiver::UserNotSufficientTrustLevelError)
describe "email from non user" do
# trusted
before do user.trust_level = 4
User.expects(:find_by_email).returns(nil) user.save
Category.expects(:find_by_email).with(
"discourse-in@appmail.adventuretime.ooo").returns(category) process_email(from: user.email, to: to)
end user.posts.count.should == 1
it "raises UserNotFoundError" do # email too short
expect{ receiver.process }.to raise_error(Email::Receiver::UserNotFoundError) message = nil
end begin
process_email(from: user.email, to: to, body: "x", subject: "this is my new topic title")
end rescue Email::Receiver::InvalidPost => e
message = e.message
describe "email from untrusted user" do
before do
User.expects(:find_by_email).with(
"jake@adventuretime.ooo").returns(user)
Category.expects(:find_by_email).with(
"discourse-in@appmail.adventuretime.ooo").returns(category)
SiteSetting.stubs(:email_in_min_trust).returns(TrustLevel.levels[:elder].to_s)
end
it "raises untrusted user error" do
expect { receiver.process }.to raise_error(Email::Receiver::UserNotSufficientTrustLevelError)
end
end
describe "with proper user" do
before do
SiteSetting.stubs(:email_in_min_trust).returns(
TrustLevel.levels[:newuser].to_s)
User.expects(:find_by_email).with(
"jake@adventuretime.ooo").returns(user)
Category.expects(:find_by_email).with(
"discourse-in@appmail.adventuretime.ooo").returns(category)
topic_creator = mock()
TopicCreator.expects(:new).with(instance_of(User),
instance_of(Guardian),
has_entries(title: subject,
category: 10)) # Make sure it is posted to the right category
.returns(topic_creator)
topic_creator.expects(:create).returns(topic_creator)
topic_creator.expects(:id).twice.returns(12345)
post_creator = mock
PostCreator.expects(:new).with(instance_of(User),
has_entries(raw: email_body,
topic_id: 12345,
cooking_options: {traditional_markdown_linebreaks: true}))
.returns(post_creator)
post_creator.expects(:create)
EmailLog.expects(:create).with(has_entries(
email_type: 'topic_via_incoming_email',
to_address: "discourse-in@appmail.adventuretime.ooo",
user_id: 3456,
topic_id: 12345
))
end
let!(:result) { receiver.process }
it "extracts the body" do
expect(receiver.body).to eq(email_body)
end end
e.message.should include("too short")
end end
end end
@ -307,77 +245,21 @@ Jakie" }
describe "processes an unknown email sender to category" do describe "processes an unknown email sender to category" do
before do before do
SiteSetting.stubs(:email_in).returns(true) SiteSetting.email_in = true
end end
let(:incoming_email) { fixture_file("emails/valid_incoming.eml") }
let(:receiver) { Email::Receiver.new(incoming_email) }
let(:non_inbox_email_category) { Fabricate.build(:category, id: 20, email_in_allow_strangers: false) }
let(:public_inbox_email_category) { Fabricate.build(:category, id: 25, email_in_allow_strangers: true) }
let(:subject) { "We should have a post-by-email-feature." }
let(:email_body) { "[quote=\"jake@adventuretime.ooo\"]
Hey folks,
I was thinking. Wouldn't it be great if we could post topics via email? Yes it would!
Jakie
[/quote]" }
describe "to disabled category" do
before do
User.expects(:find_by_email).with(
"jake@adventuretime.ooo").returns(nil)
Category.expects(:find_by_email).with(
"discourse-in@appmail.adventuretime.ooo").returns(non_inbox_email_category)
end
it "raises UserNotFoundError" do
expect{ receiver.process }.to raise_error(Email::Receiver::UserNotFoundError)
end
it "rejects anon email" do
Fabricate(:category, email_in_allow_strangers: false, email_in: "bob@bob.com")
expect { process_email(from: "test@test.com", to: "bob@bob.com") }.to raise_error(Email::Receiver::UserNotFoundError)
end end
describe "to enabled category" do it "creates a topic for allowed category" do
Fabricate(:category, email_in_allow_strangers: true, email_in: "bob@bob.com")
process_email(from: "test@test.com", to: "bob@bob.com")
before do # This is the current implementation but it is wrong, it should register an account
User.expects(:find_by_email).with( Discourse.system_user.posts.order("id desc").limit(1).pluck(:raw).first.should include("Hey folks")
"jake@adventuretime.ooo").returns(nil)
Category.expects(:find_by_email).with(
"discourse-in@appmail.adventuretime.ooo").returns(public_inbox_email_category)
topic_creator = mock()
TopicCreator.expects(:new).with(Discourse.system_user,
instance_of(Guardian),
has_entries(title: subject,
category: 25)) # Make sure it is posted to the right category
.returns(topic_creator)
topic_creator.expects(:create).returns(topic_creator)
topic_creator.expects(:id).twice.returns(135)
post_creator = mock
PostCreator.expects(:new).with(Discourse.system_user,
has_entries(raw: email_body,
topic_id: 135,
cooking_options: {traditional_markdown_linebreaks: true}))
.returns(post_creator)
post_creator.expects(:create)
EmailLog.expects(:create).with(has_entries(
email_type: 'topic_via_incoming_email',
to_address: "discourse-in@appmail.adventuretime.ooo",
user_id: Discourse.system_user.id,
topic_id: 135
))
end
let!(:result) { receiver.process }
it "extracts the body" do
expect(receiver.body).to eq(email_body)
end
end end

4
spec/fixtures/emails/attachment.eml vendored
View File

@ -1,9 +1,9 @@
Message-ID: <51C22E52.1030509@darthvader.ca> Message-ID: <51C22E52.1030509@darthvader.ca>
Date: Wed, 19 Jun 2013 18:18:58 -0400 Date: Wed, 19 Jun 2013 18:18:58 -0400
From: Anakin Skywalker <evildad@darthvader.ca> From: Anakin Skywalker <FROM>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6
MIME-Version: 1.0 MIME-Version: 1.0
To: Han Solo via Death Star <discourse+603775f8f5f68006461890a3eadf94cf@darthvader.ca> To: Han Solo via Death Star <TO>
Subject: Re: [Death Star] [PM] re: Regarding your post in "Site Customization Subject: Re: [Death Star] [PM] re: Regarding your post in "Site Customization
not working" not working"
References: <51d23d33f41fb_5f4e4b35d7d60798@xwing.mail> References: <51d23d33f41fb_5f4e4b35d7d60798@xwing.mail>

10
spec/fixtures/emails/valid_incoming.eml vendored
View File

@ -1,11 +1,11 @@
Return-Path: <jake@adventuretime.ooo> Return-Path: <FROM>
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 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 <discourse-in@appmail.adventuretime.ooo>; 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 <TO>; Thu, 13 Jun 2013 17:03:50 -0400
Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <discourse-in@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <TO>; Thu, 13 Jun 2013 14:03:48 -0700
Received: by 10.0.0.1 with HTTP; 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 Date: Thu, 13 Jun 2013 17:03:48 -0400
From: Jake the Dog <jake@adventuretime.ooo> From: Jake the Dog <FROM>
To: discourse-in@appmail.adventuretime.ooo To: <TO>
Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
Subject: We should have a post-by-email-feature. Subject: We should have a post-by-email-feature.
Mime-Version: 1.0 Mime-Version: 1.0

2
spec/spec_helper.rb
View File

@ -57,6 +57,8 @@ Spork.prefork do
config.before(:suite) do config.before(:suite) do
Sidekiq.error_handlers.clear
# Ugly, but needed until we have a user creator # Ugly, but needed until we have a user creator
User.skip_callback(:create, :after, :ensure_in_trust_level_group) User.skip_callback(:create, :after, :ensure_in_trust_level_group)