From dad43b9853538195cd9b763a1ad6521a6aa4229d Mon Sep 17 00:00:00 2001 From: Allen Hancock Date: Mon, 24 Feb 2014 00:01:37 -0600 Subject: [PATCH 1/5] Optionally allow discourse to create new topics from email. --- app/jobs/scheduled/poll_mailbox.rb | 9 ++++++ app/mailers/rejection_mailer.rb | 9 ++++++ config/discourse_defaults.conf | 8 +++++ config/locales/server.en.yml | 12 ++++++++ docs/MAILING-LIST-SETUP.MD | 48 ++++++++++++++++++++++++++++++ lib/email/receiver.rb | 32 ++++++++++++++++---- 6 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 app/mailers/rejection_mailer.rb create mode 100644 docs/MAILING-LIST-SETUP.MD diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index 3dd1445e9..edd0852e0 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -3,11 +3,14 @@ # require 'net/pop' require_dependency 'email/receiver' +require_dependency 'email/sender' +require_dependency 'email/message_builder' module Jobs class PollMailbox < Jobs::Scheduled every 5.minutes sidekiq_options retry: false + include Email::BuildEmailHelper def execute(args) if SiteSetting.pop3s_polling_enabled? @@ -25,6 +28,12 @@ module Jobs pop.each do |mail| if Email::Receiver.new(mail.pop).process == Email::Receiver.results[:processed] mail.delete + else + @message = Mail::Message.new(mail.pop) + # One for you (mod), and one for me (sender) + GroupMessage.create(Group[:moderators].name, :email_reject_notification, {limit_once_per: false, message_params: {from: @message.from, body: @message.body}}) + clientMessage = RejectionMailer.send_rejection(@message.from, @message.body) + Email::Sender.new(clientMessage, :email_reject_notification).send end end end diff --git a/app/mailers/rejection_mailer.rb b/app/mailers/rejection_mailer.rb new file mode 100644 index 000000000..fa9957361 --- /dev/null +++ b/app/mailers/rejection_mailer.rb @@ -0,0 +1,9 @@ +require_dependency 'email/message_builder' + +class RejectionMailer < ActionMailer::Base + include Email::BuildEmailHelper + + def send_rejection(from, body) + build_email(from, template: 'email_reject_notification', from: from, body: body) + end +end diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf index f4ede17d4..5fa0b3c9f 100644 --- a/config/discourse_defaults.conf +++ b/config/discourse_defaults.conf @@ -90,3 +90,11 @@ cors_origin = '*' # enable if you really need to serve assets in prd serve_static_assets = false + +# Enable new topic creation via email by setting this value to "true" +allow_new_topics_from_email = + +# Set the default category for new threads by entering the category number here +default_categories_id = + + diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index fe50842f5..a5b70719e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1107,6 +1107,18 @@ en: subject_template: "Import completed successfully" text_body_template: "The import was successful." + email_reject_notification: + subject_template: "Message posting failed" + text_body_template: | + This is an automated message to inform you that the user a message failed to meet topic criteria. + + Please review the following message. + + From - %{from} + + Contents - %{body}. + + too_many_spam_flags: subject_template: "New account blocked" text_body_template: | diff --git a/docs/MAILING-LIST-SETUP.MD b/docs/MAILING-LIST-SETUP.MD new file mode 100644 index 000000000..f400f2453 --- /dev/null +++ b/docs/MAILING-LIST-SETUP.MD @@ -0,0 +1,48 @@ +## App Setup +> TODO build an admin UI for this, get it out of discourse_defaults.conf + +Enable new topic creation via email by setting this value to "true" +`allow_new_topics_from_email = ` + +Set the default category for new threads by entering the category number here +`default_categories_id = ` + + +## Admin UI Setup + +Let's tell discourse to check for emails +![enable-reply-by-email](https://f.cloud.github.com/assets/2879972/2242953/97d5dd52-9d17-11e3-915e-037758cc68a7.png) +Be sure to setup email as you would for POP3 based replies. + +If users will be using discourse as a mailing list, allow them to opt-in +![enable-mailing-list-mode](https://f.cloud.github.com/assets/2879972/2242954/994ac2a6-9d17-11e3-8f1f-31e570905394.png) +#TODO Document how to set this true by default + +No way to enforce subject lines, so lower minimum topic length +![lower-min-topic-length](https://f.cloud.github.com/assets/2879972/2242956/9b20df84-9d17-11e3-917b-d91c17fd88c3.png) + +Emails may have the same subject, allow duplicate titles +![allow-duplicate-titles](https://f.cloud.github.com/assets/2879972/2242957/9ce3ed70-9d17-11e3-88ae-b7f9b63145bf.png) + +## Suggested User Preferences +![suggested-user-prefs](https://f.cloud.github.com/assets/2879972/2242958/9e866356-9d17-11e3-815d-164c78794b01.png) + + +## FAQ + +Q: Why is this needed? + +A: No matter how good a forum is, sometimes members need to ask a question and all they have is their mail client. + + +Q: What if a message is received from an email address which doesn't belong to an approved, registered user? + +A: It will be rejected, and a notification email sent to the moderator. Check your POP mailbox to see the rejected email content. + + + + +Q: Who did this? + +A: @therealx and @yesthatallen + diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 819b1508f..4c161f046 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -39,12 +39,19 @@ module Email @reply_key.gsub!(t, "") if t.present? end - # Look up the email log for the reply key + # Look up the email log for the reply key, or create a new post if there is none + # Enabled when config/discourse.conf contains "allow_new_topics_from_email = true" @email_log = EmailLog.for(reply_key) - return Email::Receiver.results[:missing] if @email_log.blank? - - create_reply - + if @email_log.blank? + return Email::Receiver.results[:unprocessable] if GlobalSetting.allow_new_topics_from_email == false + @subject = @message.subject + @user_info = User.find_by_email(@message.from.first) + return Email::Receiver.results[:unprocessable] if @user_info.blank? + Rails.logger.debug "Creating post from #{@message.from.first} with subject #{@subject}" + create_new + else + create_reply + end Email::Receiver.results[:processed] rescue Email::Receiver.results[:error] @@ -130,6 +137,21 @@ module Email creator.create end + def create_new + # Try to create a new topic with the body and subject + # looking to config/discourse.conf to set category + if defined? GlobalSetting.default_categories_id + @categoryID = 1 + else + @categoryID = GlobalSetting.default_categories_id + end + creator = PostCreator.new(@user_info, + title: @subject, + raw: @body, + category: @categoryID, + cooking_options: {traditional_markdown_linebreaks: true}) + creator.create + end end end From 4af2cf3f233eb3afb89770ce8de9a0d999b875d0 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 24 Feb 2014 17:36:53 +0100 Subject: [PATCH 2/5] Refactor and clean up New-Topic via Email With the new email_in admin configuration setting, emails to the email_in_address fetched via POP will now be processed and posted as new topics to the forum. With the email_in_min_trust you can control the trust level the user needs to have at least to be able to post an email as a new topic. Also contains tests for the email-in feature and minor clean ups --- config/discourse_defaults.conf | 8 --- config/locales/server.en.yml | 4 ++ config/site_settings.yml | 6 ++ docs/MAILING-LIST-SETUP.MD | 8 --- lib/email/receiver.rb | 59 ++++++++------- spec/components/email/receiver_spec.rb | 96 +++++++++++++++++++++++++ spec/fixtures/emails/valid_incoming.eml | 25 +++++++ 7 files changed, 164 insertions(+), 42 deletions(-) create mode 100644 spec/fixtures/emails/valid_incoming.eml diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf index 5fa0b3c9f..f4ede17d4 100644 --- a/config/discourse_defaults.conf +++ b/config/discourse_defaults.conf @@ -90,11 +90,3 @@ cors_origin = '*' # enable if you really need to serve assets in prd serve_static_assets = false - -# Enable new topic creation via email by setting this value to "true" -allow_new_topics_from_email = - -# Set the default category for new threads by entering the category number here -default_categories_id = - - diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index a5b70719e..2361259da 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -791,6 +791,10 @@ en: pop3s_polling_host: "The host to poll for email via POP3S" pop3s_polling_username: "The username for the POP3S account to poll for email" pop3s_polling_password: "The password for the POP3S account to poll for email" + email_in: "Allow users to post new topics via email" + email_in_address: "The email address the users can post new topics to. None means users can't post globally." + email_in_min_trust: "The minimum trust level an users needs to have to be allowed to post new topics via email" + email_in_category: "The category new emails are posted into" enable_mailing_list_mode: "Allow users to (optionally) opt-in to mailing list mode via a user preference" minimum_topics_similar: "How many topics need to exist in the database before similar topics are presented." diff --git a/config/site_settings.yml b/config/site_settings.yml index ef00962fb..b02d9c776 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -235,6 +235,12 @@ email: pop3s_polling_port: 995 pop3s_polling_username: '' pop3s_polling_password: '' + email_in: false + email_in_address: '' + email_in_min_trust: + default: 3 + enum: 'MinTrustToCreateTopicSetting' + email_in_category: -1 enable_mailing_list_mode: default: false client: true diff --git a/docs/MAILING-LIST-SETUP.MD b/docs/MAILING-LIST-SETUP.MD index f400f2453..ed5aded10 100644 --- a/docs/MAILING-LIST-SETUP.MD +++ b/docs/MAILING-LIST-SETUP.MD @@ -1,12 +1,4 @@ ## App Setup -> TODO build an admin UI for this, get it out of discourse_defaults.conf - -Enable new topic creation via email by setting this value to "true" -`allow_new_topics_from_email = ` - -Set the default category for new threads by entering the category number here -`default_categories_id = ` - ## Admin UI Setup diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 4c161f046..da8d16027 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -31,6 +31,15 @@ module Email discourse_email_parser return Email::Receiver.results[:unprocessable] if @body.blank? + + if SiteSetting.email_in and @message.to.first == SiteSetting.email_in_address + @user = User.find_by_email(@message.from.first) + return Email::Receiver.results[:unprocessable] if @user.blank? or not @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) + + create_new_topic + return Email::Receiver.results[:processed] + end + @reply_key = @message.to.first # Extract the `reply_key` from the format the site has specified @@ -39,19 +48,12 @@ module Email @reply_key.gsub!(t, "") if t.present? end - # Look up the email log for the reply key, or create a new post if there is none - # Enabled when config/discourse.conf contains "allow_new_topics_from_email = true" + # Look up the email log for the reply key @email_log = EmailLog.for(reply_key) - if @email_log.blank? - return Email::Receiver.results[:unprocessable] if GlobalSetting.allow_new_topics_from_email == false - @subject = @message.subject - @user_info = User.find_by_email(@message.from.first) - return Email::Receiver.results[:unprocessable] if @user_info.blank? - Rails.logger.debug "Creating post from #{@message.from.first} with subject #{@subject}" - create_new - else - create_reply - end + return Email::Receiver.results[:missing] if @email_log.blank? + + create_reply + Email::Receiver.results[:processed] rescue Email::Receiver.results[:error] @@ -137,20 +139,25 @@ module Email creator.create end - def create_new - # Try to create a new topic with the body and subject - # looking to config/discourse.conf to set category - if defined? GlobalSetting.default_categories_id - @categoryID = 1 - else - @categoryID = GlobalSetting.default_categories_id - end - creator = PostCreator.new(@user_info, - title: @subject, - raw: @body, - category: @categoryID, - cooking_options: {traditional_markdown_linebreaks: true}) - creator.create + + def create_new_topic + # Try to post the body as a reply + topic_creator = TopicCreator.new(@user, + Guardian.new(@user), + category: SiteSetting.email_in_category.to_i, + title: @message.subject) + + topic = topic_creator.create + post_creator = PostCreator.new(@user, + raw: @body, + topic_id: topic.id, + cooking_options: {traditional_markdown_linebreaks: true}) + + post_creator.create + EmailLog.create(email_type: "topic_via_incoming_email", + to_address: SiteSetting.email_in_address, + topic_id: topic.id, user_id: @user.id) + topic end end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index ad3dcb7b5..2e6235101 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -7,6 +7,7 @@ describe Email::Receiver do before do SiteSetting.stubs(:reply_by_email_address).returns("reply+%{reply_key}@appmail.adventuretime.ooo") + SiteSetting.stubs(:email_in).returns(false) end describe "exception raised" do @@ -204,5 +205,100 @@ greatest show ever created. Everyone should watch it. end + describe "processes a valid incoming email" do + before do + SiteSetting.stubs(:email_in_address).returns("discourse-in@appmail.adventuretime.ooo") + SiteSetting.stubs(:email_in_category).returns("42") + SiteSetting.stubs(:email_in).returns(true) + end + + let(:incoming_email) { File.read("#{Rails.root}/spec/fixtures/emails/valid_incoming.eml") } + let(:receiver) { Email::Receiver.new(incoming_email) } + let(:user) { Fabricate.build(:user, id: 3456) } + 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! + +Jakie" } + + describe "email from non user" do + + before do + User.expects(:find_by_email).returns(nil) + end + + let!(:result) { receiver.process } + + it "returns unprocessable" do + expect(result).to eq(Email::Receiver.results[:unprocessable]) + end + + end + + describe "email from untrusted user" do + before do + User.expects(:find_by_email).with( + "jake@adventuretime.ooo").returns(user) + SiteSetting.stubs(:email_in_min_trust).returns(TrustLevel.levels[:elder].to_s) + end + + let!(:result) { receiver.process } + + it "returns unprocessable" do + expect(result).to eq(Email::Receiver.results[:unprocessable]) + 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) + + topic_creator = mock() + TopicCreator.expects(:new).with(instance_of(User), + instance_of(Guardian), + has_entries(title: subject, + category: 42)) + .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 "returns a processed result" do + expect(result).to eq(Email::Receiver.results[:processed]) + end + + it "extracts the body" do + expect(receiver.body).to eq(email_body) + end + + end + + end end diff --git a/spec/fixtures/emails/valid_incoming.eml b/spec/fixtures/emails/valid_incoming.eml new file mode 100644 index 000000000..329e6cc64 --- /dev/null +++ b/spec/fixtures/emails/valid_incoming.eml @@ -0,0 +1,25 @@ +Return-Path: +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 ; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; 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 +To: discourse-in@appmail.adventuretime.ooo +Message-ID: +Subject: We should have a post-by-email-feature. +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 + +Hey folks, + +I was thinking. Wouldn't it be great if we could post topics via email? Yes it would! + +Jakie + From 37cea4945980445cc30b4e41656779b44df4ede5 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Thu, 27 Feb 2014 13:44:21 +0100 Subject: [PATCH 3/5] Add Email-In-Per-Category - allow the configuration of an inbox-email-address per category - post emails to that email into that category instead of global - Adds UI for configuration - Adds Documentation for configuration - Adds Tests for new feature --- .../controllers/edit_category_controller.js | 4 + .../javascripts/discourse/models/category.js | 1 + .../modal/edit_category.js.handlebars | 12 ++ app/controllers/categories_controller.rb | 2 +- app/models/category.rb | 4 + app/serializers/category_serializer.rb | 5 + config/locales/client.en.yml | 1 + config/site_settings.yml | 4 +- ...04930_add_custom_email_in_to_categories.rb | 10 ++ docs/MAILING-LIST-SETUP.MD | 23 ++-- lib/email/receiver.rb | 22 +++- spec/components/email/receiver_spec.rb | 119 ++++++++++++++++++ 12 files changed, 193 insertions(+), 14 deletions(-) create mode 100644 db/migrate/20140227104930_add_custom_email_in_to_categories.rb diff --git a/app/assets/javascripts/discourse/controllers/edit_category_controller.js b/app/assets/javascripts/discourse/controllers/edit_category_controller.js index 5d64811c1..4175eb630 100644 --- a/app/assets/javascripts/discourse/controllers/edit_category_controller.js +++ b/app/assets/javascripts/discourse/controllers/edit_category_controller.js @@ -58,6 +58,10 @@ Discourse.EditCategoryController = Discourse.ObjectController.extend(Discourse.M return false; }.property('saving', 'name', 'color', 'deleting'), + emailInEnabled: function() { + return Discourse.SiteSettings.email_in; + }, + deleteDisabled: function() { return (this.get('deleting') || this.get('saving') || false); }.property('disabled', 'saving', 'deleting'), diff --git a/app/assets/javascripts/discourse/models/category.js b/app/assets/javascripts/discourse/models/category.js index 2df38bcdb..6ee646400 100644 --- a/app/assets/javascripts/discourse/models/category.js +++ b/app/assets/javascripts/discourse/models/category.js @@ -66,6 +66,7 @@ Discourse.Category = Discourse.Model.extend({ permissions: this.get('permissionsForUpdate'), auto_close_hours: this.get('auto_close_hours'), position: this.get('position'), + email_in: this.get('email_in'), parent_category_id: this.get('parent_category_id') }, type: this.get('id') ? 'PUT' : 'POST' diff --git a/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars b/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars index 4770195e7..6d1ae5765 100644 --- a/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars @@ -105,6 +105,18 @@ + {{#if controller.emailInEnabled}} +
+ +
+ {{/if}} +
{{textField value=position disabled=defaultPosition class="position-input"}} diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 72d86fb77..140e9c115 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -90,7 +90,7 @@ class CategoriesController < ApplicationController end end - params.permit(*required_param_keys, :position, :parent_category_id, :auto_close_hours, :permissions => [*p.try(:keys)]) + params.permit(*required_param_keys, :position, :email_in, :parent_category_id, :auto_close_hours, :permissions => [*p.try(:keys)]) end end diff --git a/app/models/category.rb b/app/models/category.rb index 4449c8da5..6b918dd0b 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -335,6 +335,10 @@ SQL self.where(id: slug.to_i, parent_category_id: parent_category_id).includes(:featured_users).first end + def self.find_by_email(email) + self.where(email_in: Email.downcase(email)).first + end + def has_children? id && Category.where(parent_category_id: id).exists? end diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb index caa26fa95..c95377063 100644 --- a/app/serializers/category_serializer.rb +++ b/app/serializers/category_serializer.rb @@ -5,6 +5,7 @@ class CategorySerializer < BasicCategorySerializer :auto_close_hours, :group_permissions, :position, + :email_in, :can_delete def group_permissions @@ -35,4 +36,8 @@ class CategorySerializer < BasicCategorySerializer scope && scope.can_delete?(object) end + def include_email_in? + scope && scope.can_edit?(object) + end + end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 14dc7e130..b50dad0dc 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1078,6 +1078,7 @@ en: security: "Security" auto_close_label: "Auto-close topics after:" auto_close_units: "hours" + email_in: "Custom incoming email address:" edit_permissions: "Edit Permissions" add_permission: "Add Permission" this_year: "this year" diff --git a/config/site_settings.yml b/config/site_settings.yml index b02d9c776..e740fdf86 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -235,7 +235,9 @@ email: pop3s_polling_port: 995 pop3s_polling_username: '' pop3s_polling_password: '' - email_in: false + email_in: + default: false + client: true email_in_address: '' email_in_min_trust: default: 3 diff --git a/db/migrate/20140227104930_add_custom_email_in_to_categories.rb b/db/migrate/20140227104930_add_custom_email_in_to_categories.rb new file mode 100644 index 000000000..c6a98a97f --- /dev/null +++ b/db/migrate/20140227104930_add_custom_email_in_to_categories.rb @@ -0,0 +1,10 @@ +class AddCustomEmailInToCategories < ActiveRecord::Migration + def up + add_column :categories, :email_in, :string, null: true + add_index :categories, :email_in, unique: true + end + def down + remove_column :categories, :email_in + remove_index :categories, :email_in + end +end diff --git a/docs/MAILING-LIST-SETUP.MD b/docs/MAILING-LIST-SETUP.MD index ed5aded10..73e0315e3 100644 --- a/docs/MAILING-LIST-SETUP.MD +++ b/docs/MAILING-LIST-SETUP.MD @@ -1,19 +1,27 @@ ## App Setup +Acting like a Mailing list is disabled per default in Discourse. This guide shows you through the way to enable and configure it. + ## Admin UI Setup -Let's tell discourse to check for emails +First of, you need a POP3s enabled server receiving your email. Then make sure to enable "reply_by_email_enabled" and configured the server appropriately in your Admin-Settings under "Email": ![enable-reply-by-email](https://f.cloud.github.com/assets/2879972/2242953/97d5dd52-9d17-11e3-915e-037758cc68a7.png) -Be sure to setup email as you would for POP3 based replies. -If users will be using discourse as a mailing list, allow them to opt-in +Once that is in place, you can enable the "email_in"-feature globally in the same email-section. If you provide another "email_in_address" all emails arriving in the inbox to that address will be handeled and posted to the "email_in_category" (defaults to "uncategorised"). For spam protection only users of a high trust level can post via email per default. You can change this via the "email_in_min_trust" setting. + +### Per category email address + +Once "email_in" is enabled globally a new configuration option appears in your category settings dialog allowing you to specify an email-address for that category. Emails going to the previously configured inbox to that email-address will be posted in this category instead of the default configuration. **Attention** User-Permissions and the minimum trust levels still apply. + +### Troubleshooting + +You might want to allow users to opt-in to receive all posts via email with the option on the bottom: ![enable-mailing-list-mode](https://f.cloud.github.com/assets/2879972/2242954/994ac2a6-9d17-11e3-8f1f-31e570905394.png) -#TODO Document how to set this true by default -No way to enforce subject lines, so lower minimum topic length +As there is no way to enforce subject lines, you might want to lower minimum topic length, too ![lower-min-topic-length](https://f.cloud.github.com/assets/2879972/2242956/9b20df84-9d17-11e3-917b-d91c17fd88c3.png) -Emails may have the same subject, allow duplicate titles +And as some emails may have the same subject, allow duplicate titles might be another option you want to look at ![allow-duplicate-titles](https://f.cloud.github.com/assets/2879972/2242957/9ce3ed70-9d17-11e3-88ae-b7f9b63145bf.png) ## Suggested User Preferences @@ -33,8 +41,7 @@ A: It will be rejected, and a notification email sent to the moderator. Check yo - Q: Who did this? -A: @therealx and @yesthatallen +A: @therealx, @yesthatallen and @ligthyear diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index da8d16027..0997e105f 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -15,6 +15,20 @@ module Email @raw = raw end + def is_in_email? + if SiteSetting.email_in and SiteSetting.email_in_address == @message.to.first + @category_id = SiteSetting.email_in_category.to_i + return true + end + + category = Category.find_by_email(@message.to.first) + return false if not category + + @category_id = category.id + return true + + end + def process return Email::Receiver.results[:unprocessable] if @raw.blank? @@ -32,7 +46,7 @@ module Email return Email::Receiver.results[:unprocessable] if @body.blank? - if SiteSetting.email_in and @message.to.first == SiteSetting.email_in_address + if is_in_email? @user = User.find_by_email(@message.from.first) return Email::Receiver.results[:unprocessable] if @user.blank? or not @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) @@ -51,7 +65,7 @@ module Email # Look up the email log for the reply key @email_log = EmailLog.for(reply_key) return Email::Receiver.results[:missing] if @email_log.blank? - + create_reply Email::Receiver.results[:processed] @@ -144,7 +158,7 @@ module Email # Try to post the body as a reply topic_creator = TopicCreator.new(@user, Guardian.new(@user), - category: SiteSetting.email_in_category.to_i, + category: @category_id, title: @message.subject) topic = topic_creator.create @@ -155,7 +169,7 @@ module Email post_creator.create EmailLog.create(email_type: "topic_via_incoming_email", - to_address: SiteSetting.email_in_address, + to_address: @message.to.first, topic_id: topic.id, user_id: @user.id) topic end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 2e6235101..cba9208a0 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -301,4 +301,123 @@ Jakie" } end + + describe "processes an email to a category" do + before do + SiteSetting.stubs(:email_in_address).returns("") + SiteSetting.stubs(:email_in_category).returns("42") + SiteSetting.stubs(:email_in).returns(true) + end + + let(:incoming_email) { File.read("#{Rails.root}/spec/fixtures/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! + +Jakie" } + + describe "category not found" do + + before do + Category.expects(:find_by_email).returns(nil) + end + + let!(:result) { receiver.process } + + it "returns missing" do + expect(result).to eq(Email::Receiver.results[:missing]) + end + + end + + describe "email from non user" do + + before do + User.expects(:find_by_email).returns(nil) + Category.expects(:find_by_email).with( + "discourse-in@appmail.adventuretime.ooo").returns(category) + end + + let!(:result) { receiver.process } + + it "returns unprocessable" do + expect(result).to eq(Email::Receiver.results[:unprocessable]) + end + + end + + 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 + + let!(:result) { receiver.process } + + it "returns unprocessable" do + expect(result).to eq(Email::Receiver.results[:unprocessable]) + 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 "returns a processed result" do + expect(result).to eq(Email::Receiver.results[:processed]) + end + + it "extracts the body" do + expect(receiver.body).to eq(email_body) + end + + end + + end + end From d32cb558376e2e6adf791d666483acd99f241400 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Thu, 27 Feb 2014 16:36:33 +0100 Subject: [PATCH 4/5] Add public-inbox to Email-In-Feature - Adds the advanced option to accept email from non-users per category email-address - Adds tests covering the new feature - Adds UI to configure this feature in the frontend --- .../javascripts/discourse/models/category.js | 1 + .../modal/edit_category.js.handlebars | 6 ++ app/controllers/categories_controller.rb | 2 +- app/serializers/category_serializer.rb | 5 ++ config/locales/client.en.yml | 1 + ...04930_add_custom_email_in_to_categories.rb | 2 + docs/MAILING-LIST-SETUP.MD | 2 + lib/email/receiver.rb | 13 +++ spec/components/email/receiver_spec.rb | 87 +++++++++++++++++++ spec/fabricators/category_fabricator.rb | 1 - 10 files changed, 118 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/models/category.js b/app/assets/javascripts/discourse/models/category.js index 6ee646400..a6383f8ca 100644 --- a/app/assets/javascripts/discourse/models/category.js +++ b/app/assets/javascripts/discourse/models/category.js @@ -67,6 +67,7 @@ Discourse.Category = Discourse.Model.extend({ auto_close_hours: this.get('auto_close_hours'), position: this.get('position'), email_in: this.get('email_in'), + email_in_allow_strangers: this.get('email_in_allow_strangers'), parent_category_id: this.get('parent_category_id') }, type: this.get('id') ? 'PUT' : 'POST' diff --git a/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars b/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars index 6d1ae5765..6077c9e00 100644 --- a/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars @@ -113,6 +113,12 @@ {{i18n category.email_in}} {{textField value=email_in}} +
+ +
{{/if}} diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 140e9c115..ed2a9fcbb 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -90,7 +90,7 @@ class CategoriesController < ApplicationController end end - params.permit(*required_param_keys, :position, :email_in, :parent_category_id, :auto_close_hours, :permissions => [*p.try(:keys)]) + params.permit(*required_param_keys, :position, :email_in, :email_in_allow_strangers, :parent_category_id, :auto_close_hours, :permissions => [*p.try(:keys)]) end end diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb index c95377063..4ed3e552a 100644 --- a/app/serializers/category_serializer.rb +++ b/app/serializers/category_serializer.rb @@ -6,6 +6,7 @@ class CategorySerializer < BasicCategorySerializer :group_permissions, :position, :email_in, + :email_in_allow_strangers, :can_delete def group_permissions @@ -40,4 +41,8 @@ class CategorySerializer < BasicCategorySerializer scope && scope.can_edit?(object) end + def include_email_in_allow_strangers? + scope && scope.can_edit?(object) + end + end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b50dad0dc..a1cd6dc0f 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1079,6 +1079,7 @@ en: auto_close_label: "Auto-close topics after:" auto_close_units: "hours" email_in: "Custom incoming email address:" + email_in_allow_strangers: "Accept emails from non-users (aka strangers)" edit_permissions: "Edit Permissions" add_permission: "Add Permission" this_year: "this year" diff --git a/db/migrate/20140227104930_add_custom_email_in_to_categories.rb b/db/migrate/20140227104930_add_custom_email_in_to_categories.rb index c6a98a97f..7d3cac2c5 100644 --- a/db/migrate/20140227104930_add_custom_email_in_to_categories.rb +++ b/db/migrate/20140227104930_add_custom_email_in_to_categories.rb @@ -1,10 +1,12 @@ class AddCustomEmailInToCategories < ActiveRecord::Migration def up add_column :categories, :email_in, :string, null: true + add_column :categories, :email_in_allow_strangers, :boolean, default: false add_index :categories, :email_in, unique: true end def down remove_column :categories, :email_in + remove_column :categories, :email_in_allow_strangers remove_index :categories, :email_in end end diff --git a/docs/MAILING-LIST-SETUP.MD b/docs/MAILING-LIST-SETUP.MD index 73e0315e3..44ce15144 100644 --- a/docs/MAILING-LIST-SETUP.MD +++ b/docs/MAILING-LIST-SETUP.MD @@ -13,6 +13,8 @@ Once that is in place, you can enable the "email_in"-feature globally in the sam Once "email_in" is enabled globally a new configuration option appears in your category settings dialog allowing you to specify an email-address for that category. Emails going to the previously configured inbox to that email-address will be posted in this category instead of the default configuration. **Attention** User-Permissions and the minimum trust levels still apply. +Additionally, by checking the "accept non-user emails"-checkbox in the category settings, emails to the given email but from unknown email-addresses will be posted in the category by the System-User in a quoted fashion, showing the original email-address and content in the quotes. + ### Troubleshooting You might want to allow users to opt-in to receive all posts via email with the option on the bottom: diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 0997e105f..f1a6e279b 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -16,6 +16,7 @@ module Email end def is_in_email? + @allow_strangers = false if SiteSetting.email_in and SiteSetting.email_in_address == @message.to.first @category_id = SiteSetting.email_in_category.to_i return true @@ -25,6 +26,7 @@ module Email return false if not category @category_id = category.id + @allow_strangers = category.email_in_allow_strangers return true end @@ -48,6 +50,11 @@ module Email if is_in_email? @user = User.find_by_email(@message.from.first) + if @user.blank? and @allow_strangers + wrap_body_in_quote + @user = Discourse.system_user + end + return Email::Receiver.results[:unprocessable] if @user.blank? or not @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) create_new_topic @@ -75,6 +82,12 @@ module Email private + def wrap_body_in_quote + @body = "[quote=\"#{@message.from.first}\"] +#{@body} +[/quote]" + end + def parse_body html = nil diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index cba9208a0..9217020b3 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -420,4 +420,91 @@ Jakie" } end + + describe "processes an unknown email sender to category" do + before do + SiteSetting.stubs(:email_in_address).returns("") + SiteSetting.stubs(:email_in_category).returns("42") + SiteSetting.stubs(:email_in).returns(true) + end + + let(:incoming_email) { File.read("#{Rails.root}/spec/fixtures/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 + + let!(:result) { receiver.process } + + it "returns unprocessable" do + expect(result).to eq(Email::Receiver.results[:unprocessable]) + end + + end + + describe "to enabled 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(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 "returns a processed result" do + expect(result).to eq(Email::Receiver.results[:processed]) + end + + it "extracts the body" do + expect(receiver.body).to eq(email_body) + end + + end + + end + end diff --git a/spec/fabricators/category_fabricator.rb b/spec/fabricators/category_fabricator.rb index 7ab4c4b75..6ef5a89a9 100644 --- a/spec/fabricators/category_fabricator.rb +++ b/spec/fabricators/category_fabricator.rb @@ -2,4 +2,3 @@ Fabricator(:category) do name { sequence(:name) { |n| "Amazing Category #{n}" } } user end - From 024597e643fb5d70799c28d3a29f8607ee3270ff Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 28 Feb 2014 13:05:09 +0100 Subject: [PATCH 5/5] Switch to proper exception handling system for better user feedback - Replace implicit return code-system in Email::Receiver with proper exception system - Update tests to check for exceptions instead - Test the PollMailbox for expected failures - Add proper email-handling of problematic emails " --- app/jobs/scheduled/poll_mailbox.rb | 30 ++++-- app/mailers/rejection_mailer.rb | 4 + config/locales/server.en.yml | 20 ++-- lib/email/receiver.rb | 51 +++++----- spec/components/email/poll_mailbox_spec.rb | 107 +++++++++++++++++++++ spec/components/email/receiver_spec.rb | 96 ++++++------------ 6 files changed, 198 insertions(+), 110 deletions(-) create mode 100644 spec/components/email/poll_mailbox_spec.rb diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index edd0852e0..a113a35da 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -18,6 +18,26 @@ module Jobs end end + def handle_mail(mail) + begin + Email::Receiver.new(mail).process + rescue Email::Receiver::UserNotSufficientTrustLevelError => e + # inform the user about the rejection + @message = Mail::Message.new(mail) + clientMessage = RejectionMailer.send_trust_level(@message.from, @message.body) + email_sender = Email::Sender.new(clientMessage, :email_reject_trust_level) + email_sender.send + rescue Email::Receiver::ProcessingError + # all other ProcessingErrors are ok to be dropped + rescue StandardError => e + # Inform Admins about error + GroupMessage.create(Group[:admins].name, :email_error_notification, + {limit_once_per: false, message_params: {source: mail, error: e}}) + ensure + mail.delete + end + end + def poll_pop3s Net::POP3.enable_ssl(OpenSSL::SSL::VERIFY_NONE) Net::POP3.start(SiteSetting.pop3s_polling_host, @@ -26,15 +46,7 @@ module Jobs SiteSetting.pop3s_polling_password) do |pop| unless pop.mails.empty? pop.each do |mail| - if Email::Receiver.new(mail.pop).process == Email::Receiver.results[:processed] - mail.delete - else - @message = Mail::Message.new(mail.pop) - # One for you (mod), and one for me (sender) - GroupMessage.create(Group[:moderators].name, :email_reject_notification, {limit_once_per: false, message_params: {from: @message.from, body: @message.body}}) - clientMessage = RejectionMailer.send_rejection(@message.from, @message.body) - Email::Sender.new(clientMessage, :email_reject_notification).send - end + handle_mail mail.pop end end end diff --git a/app/mailers/rejection_mailer.rb b/app/mailers/rejection_mailer.rb index fa9957361..1e060aa55 100644 --- a/app/mailers/rejection_mailer.rb +++ b/app/mailers/rejection_mailer.rb @@ -6,4 +6,8 @@ class RejectionMailer < ActionMailer::Base def send_rejection(from, body) build_email(from, template: 'email_reject_notification', from: from, body: body) end + + def send_trust_level(from, body, to) + build_email(from, template: 'email_reject_trust_level', to: to) + end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2361259da..fa5c2f17c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1111,17 +1111,23 @@ en: subject_template: "Import completed successfully" text_body_template: "The import was successful." - email_reject_notification: - subject_template: "Message posting failed" + email_error_notification: + subject_template: "Error parsing email" text_body_template: | - This is an automated message to inform you that the user a message failed to meet topic criteria. + This is an automated message to inform you that parsing the following incoming email failed. Please review the following message. - From - %{from} - - Contents - %{body}. - + Error - %{error} + + %{source} + + email_reject_trust_level: + subject_template: "Message rejected" + text_body_template: | + The message you've send to %{to} was rejected by the system. + + You do not have the required trust to post new topics to this email address. too_many_spam_flags: subject_template: "New account blocked" diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index f1a6e279b..7ce6102ca 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -5,9 +5,12 @@ module Email class Receiver - def self.results - @results ||= Enum.new(:unprocessable, :missing, :processed, :error) - end + class ProcessingError < StandardError; end + class EmailUnparsableError < ProcessingError; end + class EmptyEmailError < ProcessingError; end + class UserNotFoundError < ProcessingError; end + class UserNotSufficientTrustLevelError < ProcessingError; end + class EmailLogNotFound < ProcessingError; end attr_reader :body, :reply_key, :email_log @@ -32,21 +35,21 @@ module Email end def process - return Email::Receiver.results[:unprocessable] if @raw.blank? + raise EmptyEmailError if @raw.blank? @message = Mail::Message.new(@raw) # First remove the known discourse stuff. parse_body - return Email::Receiver.results[:unprocessable] if @body.blank? + raise EmptyEmailError if @body.blank? # Then run the github EmailReplyParser on it in case we didn't catch it @body = EmailReplyParser.read(@body).visible_text.force_encoding('UTF-8') discourse_email_parser - return Email::Receiver.results[:unprocessable] if @body.blank? + raise EmailUnparsableError if @body.blank? if is_in_email? @user = User.find_by_email(@message.from.first) @@ -55,29 +58,25 @@ module Email @user = Discourse.system_user end - return Email::Receiver.results[:unprocessable] if @user.blank? or not @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) + raise UserNotFoundError if @user.blank? + raise UserNotSufficientTrustLevelError.new @user if not @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i]) create_new_topic - return Email::Receiver.results[:processed] + else + @reply_key = @message.to.first + + # Extract the `reply_key` from the format the site has specified + tokens = SiteSetting.reply_by_email_address.split("%{reply_key}") + tokens.each do |t| + @reply_key.gsub!(t, "") if t.present? + end + + # Look up the email log for the reply key + @email_log = EmailLog.for(reply_key) + raise EmailLogNotFound if @email_log.blank? + + create_reply end - - @reply_key = @message.to.first - - # Extract the `reply_key` from the format the site has specified - tokens = SiteSetting.reply_by_email_address.split("%{reply_key}") - tokens.each do |t| - @reply_key.gsub!(t, "") if t.present? - end - - # Look up the email log for the reply key - @email_log = EmailLog.for(reply_key) - return Email::Receiver.results[:missing] if @email_log.blank? - - create_reply - - Email::Receiver.results[:processed] - rescue - Email::Receiver.results[:error] end private diff --git a/spec/components/email/poll_mailbox_spec.rb b/spec/components/email/poll_mailbox_spec.rb new file mode 100644 index 000000000..1465ca1c7 --- /dev/null +++ b/spec/components/email/poll_mailbox_spec.rb @@ -0,0 +1,107 @@ +# -*- encoding : utf-8 -*- + +require 'spec_helper' +require 'email/receiver' +require 'jobs/scheduled/poll_mailbox' +require 'email/message_builder' + +describe Jobs::PollMailbox do + + describe "processing email" do + + let!(:poller) { Jobs::PollMailbox.new } + let!(:receiver) { mock } + let!(:email) { mock } + + before do + Email::Receiver.expects(:new).with(email).returns(receiver) + end + + describe "all goes fine" do + + it "email gets deleted" do + receiver.expects(:process) + email.expects(:delete) + + poller.handle_mail(email) + end + end + + describe "raises Untrusted error" do + + before do + receiver.expects(:process).raises(Email::Receiver::UserNotSufficientTrustLevelError) + email.expects(:delete) + + Mail::Message.expects(:new).returns(email) + + email.expects(:from) + email.expects(:body) + + clientMessage = mock + senderMock = mock + RejectionMailer.expects(:send_trust_level).returns(clientMessage) + Email::Sender.expects(:new).with( + clientMessage, :email_reject_trust_level).returns(senderMock) + senderMock.expects(:send) + end + + it "sends a reply and deletes the email" do + poller.handle_mail(email) + end + end + + describe "raises error" do + + it "deletes email on ProcessingError" do + receiver.expects(:process).raises(Email::Receiver::ProcessingError) + email.expects(:delete) + + poller.handle_mail(email) + end + + it "deletes email on EmailUnparsableError" do + receiver.expects(:process).raises(Email::Receiver::EmailUnparsableError) + email.expects(:delete) + + poller.handle_mail(email) + end + + it "deletes email on EmptyEmailError" do + receiver.expects(:process).raises(Email::Receiver::EmptyEmailError) + email.expects(:delete) + + poller.handle_mail(email) + end + + it "deletes email on UserNotFoundError" do + receiver.expects(:process).raises(Email::Receiver::UserNotFoundError) + email.expects(:delete) + + poller.handle_mail(email) + end + + it "deletes email on EmailLogNotFound" do + receiver.expects(:process).raises(Email::Receiver::EmailLogNotFound) + email.expects(:delete) + + poller.handle_mail(email) + end + + + it "informs admins on any other error" do + receiver.expects(:process).raises(TypeError) + email.expects(:delete) + GroupMessage.expects(:create) do |args| + args[0].should eq "admins" + args[1].shouled eq :email_error_notification + args[2].message_params.source.should eq email + args[2].message_params.error.should_be instance_of(TypeError) + end + + poller.handle_mail(email) + end + end + end + +end \ No newline at end of file diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 9217020b3..ad0697111 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -10,23 +10,13 @@ describe Email::Receiver do SiteSetting.stubs(:email_in).returns(false) end - describe "exception raised" do - it "returns error if it encountered an error processing" do - receiver = Email::Receiver.new("some email") - def receiver.parse_body - raise "ERROR HAPPENED!" - end - expect(receiver.process).to eq(Email::Receiver.results[:error]) - end - end - describe 'invalid emails' do - it "returns unprocessable if the message is blank" do - expect(Email::Receiver.new("").process).to eq(Email::Receiver.results[:unprocessable]) + it "raises EmptyEmailError if the message is blank" do + expect { Email::Receiver.new("").process }.to raise_error(Email::Receiver::EmptyEmailError) end - it "returns unprocessable if the message is not an email" do - expect(Email::Receiver.new("asdf" * 30).process).to eq(Email::Receiver.results[:unprocessable]) + it "raises EmailUnparsableError if the message is not an email" do + expect { Email::Receiver.new("asdf" * 30).process}.to raise_error(Email::Receiver::EmptyEmailError) end end @@ -35,7 +25,7 @@ describe Email::Receiver do let(:receiver) { Email::Receiver.new(reply_below) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq( "So presumably all the quoted garbage and my (proper) signature will get stripped from my reply?") @@ -47,7 +37,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(reply_below) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("The EC2 instance - I've seen that there tends to be odd and " + "unrecommended settings on the Bitnami installs that I've checked out.") end @@ -58,7 +48,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(attachment) } it "processes correctly" do - expect(receiver.process).to eq(Email::Receiver.results[:unprocessable]) + expect { receiver.process}.to raise_error(Email::Receiver::EmptyEmailError) expect(receiver.body).to be_blank end end @@ -68,7 +58,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(dutch) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("Dit is een antwoord in het Nederlands.") end end @@ -79,7 +69,7 @@ stripped from my reply?") it "processes correctly" do I18n.expects(:t).with('user_notifications.previous_discussion').returns('כלטוב') - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("שלום") end end @@ -90,7 +80,7 @@ stripped from my reply?") it "processes correctly" do I18n.expects(:t).with('user_notifications.previous_discussion').returns('媽!我上電視了!') - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("媽!我上電視了!") end end @@ -100,7 +90,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(wrote) } it "removes via lines if we know them" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("Hello this email has content!") end end @@ -110,7 +100,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(wrote) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("Thanks!") end end @@ -120,7 +110,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(previous) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq("This will not include the previous discussion that is present in this email.") end end @@ -130,7 +120,7 @@ stripped from my reply?") let(:receiver) { Email::Receiver.new(paragraphs) } it "processes correctly" do - receiver.process + expect { receiver.process}.to raise_error(Email::Receiver::ProcessingError) expect(receiver.body).to eq( "Is there any reason the *old* candy can't be be kept in silos while the new candy is imported into *new* silos? @@ -161,10 +151,8 @@ greatest show ever created. Everyone should watch it. EmailLog.expects(:for).returns(nil) end - let!(:result) { receiver.process } - - it "returns missing" do - expect(result).to eq(Email::Receiver.results[:missing]) + it "raises EmailLogNotFoundError" do + expect{ receiver.process }.to raise_error(Email::Receiver::EmailLogNotFound) end end @@ -185,10 +173,6 @@ greatest show ever created. Everyone should watch it. let!(:result) { receiver.process } - it "returns a processed result" do - expect(result).to eq(Email::Receiver.results[:processed]) - end - it "extracts the body" do expect(receiver.body).to eq(reply_body) end @@ -229,10 +213,8 @@ Jakie" } User.expects(:find_by_email).returns(nil) end - let!(:result) { receiver.process } - - it "returns unprocessable" do - expect(result).to eq(Email::Receiver.results[:unprocessable]) + it "raises user not found error" do + expect { receiver.process }.to raise_error(Email::Receiver::UserNotFoundError) end end @@ -244,10 +226,8 @@ Jakie" } SiteSetting.stubs(:email_in_min_trust).returns(TrustLevel.levels[:elder].to_s) end - let!(:result) { receiver.process } - - it "returns unprocessable" do - expect(result).to eq(Email::Receiver.results[:unprocessable]) + it "raises untrusted user error" do + expect { receiver.process }.to raise_error(Email::Receiver::UserNotSufficientTrustLevelError) end end @@ -289,10 +269,6 @@ Jakie" } let!(:result) { receiver.process } - it "returns a processed result" do - expect(result).to eq(Email::Receiver.results[:processed]) - end - it "extracts the body" do expect(receiver.body).to eq(email_body) end @@ -327,10 +303,8 @@ Jakie" } Category.expects(:find_by_email).returns(nil) end - let!(:result) { receiver.process } - - it "returns missing" do - expect(result).to eq(Email::Receiver.results[:missing]) + it "raises EmailLogNotFoundError" do + expect{ receiver.process }.to raise_error(Email::Receiver::EmailLogNotFound) end end @@ -343,10 +317,8 @@ Jakie" } "discourse-in@appmail.adventuretime.ooo").returns(category) end - let!(:result) { receiver.process } - - it "returns unprocessable" do - expect(result).to eq(Email::Receiver.results[:unprocessable]) + it "raises UserNotFoundError" do + expect{ receiver.process }.to raise_error(Email::Receiver::UserNotFoundError) end end @@ -360,10 +332,8 @@ Jakie" } SiteSetting.stubs(:email_in_min_trust).returns(TrustLevel.levels[:elder].to_s) end - let!(:result) { receiver.process } - - it "returns unprocessable" do - expect(result).to eq(Email::Receiver.results[:unprocessable]) + it "raises untrusted user error" do + expect { receiver.process }.to raise_error(Email::Receiver::UserNotSufficientTrustLevelError) end end @@ -408,10 +378,6 @@ Jakie" } let!(:result) { receiver.process } - it "returns a processed result" do - expect(result).to eq(Email::Receiver.results[:processed]) - end - it "extracts the body" do expect(receiver.body).to eq(email_body) end @@ -449,10 +415,8 @@ Jakie "discourse-in@appmail.adventuretime.ooo").returns(non_inbox_email_category) end - let!(:result) { receiver.process } - - it "returns unprocessable" do - expect(result).to eq(Email::Receiver.results[:unprocessable]) + it "raises UserNotFoundError" do + expect{ receiver.process }.to raise_error(Email::Receiver::UserNotFoundError) end end @@ -495,10 +459,6 @@ Jakie let!(:result) { receiver.process } - it "returns a processed result" do - expect(result).to eq(Email::Receiver.results[:processed]) - end - it "extracts the body" do expect(receiver.body).to eq(email_body) end