From 37cea4945980445cc30b4e41656779b44df4ede5 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Thu, 27 Feb 2014 13:44:21 +0100 Subject: [PATCH] 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