Merge pull request #2530 from riking/email-2

Remove email_in_address and email_in_category site settings

app/assets/javascripts/discourse/templates/modal/edit-category.js.handlebars

@@ -3,9 +3,9 @@
     {{edit-category-tab selectedTab=selectedTab tab="general"}}
     {{#unless isUncategorizedCategory}}
       {{edit-category-tab selectedTab=selectedTab tab="security"}}
-      {{edit-category-tab selectedTab=selectedTab tab="settings"}}
-      {{edit-category-tab selectedTab=selectedTab tab="images"}}
     {{/unless}}
+    {{edit-category-tab selectedTab=selectedTab tab="settings"}}
+    {{edit-category-tab selectedTab=selectedTab tab="images"}}
   </ul>
 
   <div class="modal-body">

app/jobs/scheduled/poll_mailbox.rb

@@ -44,7 +44,7 @@ module Jobs
         if message_template
           # inform the user about the rejection
           message = Mail::Message.new(mail_string)
-          client_message = RejectionMailer.send_rejection(message.from, message.body, message.to, message_template)
+          client_message = RejectionMailer.send_rejection(message.from, message.body, message.subject, message.to, message_template)
           Email::Sender.new(client_message, message_template).send
         else
           Discourse.handle_exception(e, { code: "unknown error for incoming email", mail: mail_string} )

app/mailers/rejection_mailer.rb

@@ -3,11 +3,12 @@ require_dependency 'email/message_builder'
 class RejectionMailer < ActionMailer::Base
   include Email::BuildEmailHelper
 
-  def send_rejection(from, body, to_address, template)
+  def send_rejection(message_from, message_body, message_subject, forum_address, template)
-    build_email(from, template: "system_messages.#{template}", source: body, destination: to_address)
+    build_email(message_from,
+                template: "system_messages.#{template}",
+                source: message_body,
+                former_title: message_subject,
+                destination: forum_address)
   end
 
-  def send_trust_level(from, template)
-    build_email(from, template: 'email_reject_trust_level')
-  end
 end

config/locales/server.en.yml

@@ -912,10 +912,8 @@ 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: "Allow users to post new topics via email (requires pop3 polling). Configure the addresses in the \"Settings\" tab of each category."
-    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 a user needs to have to be allowed to post new topics via email."
-    email_in_category: "The category new emails are posted into."
     email_prefix: "The [label] used in the subject of emails. It will default to 'title' if not set."
     email_site_title: "The title of the site used as the sender of emails from the site. Default to 'title' if not set. If your 'title' contains characters that are not allowed in email sender strings, use this setting."
 
@@ -1358,42 +1356,43 @@ en:
     email_reject_trust_level:
       subject_template: "Email issue -- Insufficient Trust Level"
       text_body_template: |
-        We're sorry, but your email message to %{destination} didn't work.
+        We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work.
 
         Your account does not have the required trust level to post new topics to this email address. If you believe this is in error, contact a staff member.
 
     email_reject_no_account:
       subject_template: "Email issue -- No Account"
       text_body_template: |
-        We're sorry, but your email message to %{destination} didn't work.
+        We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work.
 
         There is no known account with this email address. Try sending from a different email address, or contact a staff member.
 
     email_reject_empty:
       subject_template: "Email issue -- No Content"
       text_body_template: |
-        We're sorry, but your email message to %{destination} didn't work.
+        We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work.
 
-        We couldn't find any content in the email.
+        We couldn't find any content in the email. Make sure that you wrote something at the top of the email - we can't parse inline replies.
+        If you're getting this and you did include content, try again in UTF-8 plain text (no HTML).
 
     email_reject_parsing:
       subject_template: "Email issue -- Content unrecognized"
       text_body_template: |
-        We're sorry, but your email message to %{destination} didn't work.
+        We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work.
 
-        The email encoding was unknown or not supported. Try again with UTF-8 plain text.
+        We couldn't find where your reply was in the email. **Make sure to write your entire reply at the top of the email** -- everything below the start of the replied-to message is discarded.
 
     email_reject_post_error:
       subject_template: "Email issue -- Posting error"
       text_body_template: |
-        We're sorry, but your email message to %{destination} didn't work.
+        We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work.
 
         Some possible causes are: complex formatting, message too large, message too small. Please try again.
 
     email_reject_reply_key:
       subject_template: "Email issue -- Bad Reply Key"
       text_body_template: |
-        We're sorry, but your email message to %{destination} didn't work.
+        We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work.
 
         The provided reply key is invalid or unknown, so we don't know what this email is in reply to. Contact a staff member.
 

config/site_settings.yml

@@ -384,13 +384,9 @@ email:
   email_in:
     default: false
     client: true
-  email_in_address:
-    default: ''
-    type: email
   email_in_min_trust:
     default: 2
     enum: 'TrustLevelSetting'
-  email_in_category: -1
   email_prefix: ''
   email_site_title: ''
 

db/migrate/20140711193923_remove_email_in_address_setting.rb

@@ -0,0 +1,19 @@
+class RemoveEmailInAddressSetting < ActiveRecord::Migration
+  def up
+    uncat_id = ActiveRecord::Base.exec_sql("SELECT value FROM site_settings WHERE name = 'uncategorized_category_id'").first
+    cat_id_r = ActiveRecord::Base.exec_sql("SELECT value FROM site_settings WHERE name = 'email_in_category'").first
+    email_r = ActiveRecord::Base.exec_sql("SELECT value FROM site_settings WHERE name = 'email_in_address'").first
+    if email_r
+      category_id = uncat_id["value"].to_i
+      category_id = cat_id_r["value"].to_i if cat_id_r
+      email = email_r["value"]
+      ActiveRecord::Base.exec_sql("UPDATE categories SET email_in = ? WHERE id = ?", email, category_id)
+    end
+
+    ActiveRecord::Base.exec_sql("DELETE FROM site_settings WHERE name = 'email_in_category' OR name = 'email_in_address'")
+  end
+
+  def down
+    # this change is backwards-compatible
+  end
+end

docs/MAILING-LIST-SETUP.md

@@ -7,13 +7,13 @@ Acting like a Mailing list is disabled per default in Discourse. This guide show
 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)
 
-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.
+Once that is in place, you can enable the "email_in"-feature globally in the same email-section.
 
 ### 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.
+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.
 
-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.
+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. 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
 

lib/email/receiver.rb

@@ -44,7 +44,7 @@ module Email
         end
 
         raise UserNotFoundError if @user.blank?
-        raise UserNotSufficientTrustLevelError.new @user unless @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i])
+        raise UserNotSufficientTrustLevelError.new @user unless @allow_strangers || @user.has_trust_level?(TrustLevel.levels[SiteSetting.email_in_min_trust.to_i])
 
         create_new_topic
       else
@@ -134,11 +134,7 @@ module Email
 
     def is_in_email?
       @allow_strangers = false
-
+      return false unless SiteSetting.email_in
-      if SiteSetting.email_in && 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 unless category

spec/components/email/receiver_spec.rb

@@ -19,8 +19,8 @@ describe Email::Receiver do
       expect { Email::Receiver.new("asdf" * 30).process}.to raise_error(Email::Receiver::EmptyEmailError)
     end
 
-    pending "raises EmailUnparsableError in some situation" do
+    it "raises EmailUnparsableError if there is no reply content" do
-      expect { Email::Receiver.new("something").process}.to raise_error(Email::Receiver::EmailUnparsableError)
+      expect { Email::Receiver.new(fixture_file("emails/no_content_reply.eml")).process}.to raise_error(Email::Receiver::EmailUnparsableError)
     end
   end
 
@@ -198,99 +198,8 @@ 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) { fixture_file("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
-
-      it "raises user not found error" do
-        expect { receiver.process }.to raise_error(Email::Receiver::UserNotFoundError)
-      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
-
-      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)
-
-        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 "extracts the body" do
-        expect(receiver.body).to eq(email_body)
-      end
-
-    end
-
-  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
 
@@ -398,8 +307,6 @@ Jakie" }
 
   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
 

spec/fixtures/emails/no_content_reply.eml

@@ -0,0 +1,34 @@
+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 <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400
+Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@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: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo
+Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
+Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
+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
+
+On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta
+<reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo> wrote:
+>
+>
+>
+> eviltrout posted in 'Adventure Time Sux' on Discourse Meta:
+>
+> ---
+> hey guys everyone knows adventure time sucks!
+>
+> ---
+> Please visit this link to respond: http://localhost:3000/t/adventure-time-sux/1234/3
+>
+> To unsubscribe from these emails, visit your [user preferences](http://localhost:3000/user_preferences).
+>

spec/jobs/poll_mailbox_spec.rb

@@ -44,14 +44,7 @@ describe Jobs::PollMailbox do
   describe "processing email" do
 
     let!(:receiver) { mock }
-    let!(:email_string) { <<MAIL
+    let!(:email_string) { fixture_file("emails/valid_incoming.eml") }
-From: user@example.com
-To: reply+32@discourse.example.net
-Subject: Hi
-
-Email As a String
-MAIL
-    }
     let!(:email) { mock }
 
     before do
@@ -82,7 +75,7 @@ MAIL
         sender_object = mock
 
         RejectionMailer.expects(:send_rejection).with(
-            message.from, message.body, message.to, :email_reject_trust_level
+            message.from, message.body, message.subject, message.to, :email_reject_trust_level
         ).returns(client_message)
         Email::Sender.expects(:new).with(client_message, :email_reject_trust_level).returns(sender_object)
         sender_object.expects(:send)