From 0d54c18c8bd88f769a38694e806b1f8be958cb82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 24 Nov 2015 16:58:26 +0100 Subject: [PATCH] new hidden 'allow_staged_accounts' setting --- app/jobs/regular/user_email.rb | 2 +- app/mailers/user_notifications.rb | 2 +- app/models/user_email_observer.rb | 4 +- app/services/post_alerter.rb | 9 +- config/site_settings.yml | 3 + lib/email/message_builder.rb | 14 +- lib/email/receiver.rb | 25 ++-- lib/new_post_manager.rb | 4 +- lib/post_jobs_enqueuer.rb | 4 +- spec/components/email/receiver_spec.rb | 1 - spec/models/user_email_observer_spec.rb | 185 +++++++----------------- 11 files changed, 94 insertions(+), 159 deletions(-) diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index da5703e360c..7795d4bbac5 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -23,7 +23,7 @@ module Jobs return if @user.staged && args[:type] == :digest seen_recently = (@user.last_seen_at.present? && @user.last_seen_at > SiteSetting.email_time_window_mins.minutes.ago) - seen_recently = false if @user.email_always + seen_recently = false if @user.email_always || @user.staged email_args = {} diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index e48a604f8c2..aa2aa049292 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -293,7 +293,7 @@ class UserNotifications < ActionMailer::Base topic_id: post.topic_id, context: context, username: username, - add_unsubscribe_link: true, + add_unsubscribe_link: !user.staged, unsubscribe_url: post.topic.unsubscribe_url, allow_reply_by_email: allow_reply_by_email, use_site_subject: use_site_subject, diff --git a/app/models/user_email_observer.rb b/app/models/user_email_observer.rb index 0da32b73f0f..41707ec0c9a 100644 --- a/app/models/user_email_observer.rb +++ b/app/models/user_email_observer.rb @@ -39,7 +39,7 @@ class UserEmailObserver < ActiveRecord::Observer private def enqueue(type) - return unless (notification.user.email_direct? && notification.user.active?) + return unless notification.user.email_direct? && (notification.user.active? || notification.user.staged?) Jobs.enqueue_in(delay, :user_email, @@ -49,7 +49,7 @@ class UserEmailObserver < ActiveRecord::Observer end def enqueue_private(type) - return unless (notification.user.email_private_messages? && notification.user.active?) + return unless notification.user.email_private_messages? && (notification.user.active? || notification.user.staged?) Jobs.enqueue_in(delay, :user_email, diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 2631ccf06b8..a85d66b4ba1 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -224,10 +224,11 @@ class PostAlerter exclude_user_ids << reply_to_user.id if reply_to_user.present? exclude_user_ids.flatten! - TopicUser - .where(topic_id: post.topic_id, notification_level: TopicUser.notification_levels[:watching]) - .includes(:user).each do |tu| - create_notification(tu.user, Notification.types[:posted], post) unless exclude_user_ids.include?(tu.user_id) + TopicUser.where(topic_id: post.topic_id) + .where(notification_level: TopicUser.notification_levels[:watching]) + .where("user_id NOT IN (?)", exclude_user_ids) + .includes(:user).each do |tu| + create_notification(tu.user, Notification.types[:posted], post) end end end diff --git a/config/site_settings.yml b/config/site_settings.yml index 356aeeb776c..a88829f748b 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -792,6 +792,9 @@ developer: default: 500 client: true hidden: true + allow_staged_accounts: + default: false + hidden: true embedding: feed_polling_enabled: diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb index f8053287956..2e875a69e59 100644 --- a/lib/email/message_builder.rb +++ b/lib/email/message_builder.rb @@ -59,13 +59,17 @@ module Email return unless html_override = @opts[:html_override] if @opts[:add_unsubscribe_link] - if response_instructions = @template_args[:respond_instructions] - respond_instructions = PrettyText.cook(response_instructions).html_safe - html_override.gsub!("%{respond_instructions}", respond_instructions) - end - unsubscribe_link = PrettyText.cook(I18n.t('unsubscribe_link', template_args)).html_safe html_override.gsub!("%{unsubscribe_link}", unsubscribe_link) + else + html_override.gsub!("%{unsubscribe_link}", "") + end + + if response_instructions = @template_args[:respond_instructions] + respond_instructions = PrettyText.cook(response_instructions).html_safe + html_override.gsub!("%{respond_instructions}", respond_instructions) + else + html_override.gsub!("%{respond_instructions}", "") end styled = Email::Styles.new(html_override, @opts) diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 142e83d4e19..cf3265a9648 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -58,12 +58,17 @@ module Email user_email = @message.from.first @user = User.find_by_email(user_email) - if @user.blank? && @allow_strangers - wrap_body_in_quote user_email - # TODO This is WRONG it should register an account - # and email the user details on how to log in / activate - @user = Discourse.system_user + # create staged account when user doesn't exist + if @user.blank? && @allow_strangers + if SiteSetting.allow_staged_accounts + username = UserNameSuggester.suggest(user_email) + name = User.suggest_name(user_email) + @user = User.create(email: user_email, username: username, name: name, staged: true) + else + wrap_body_in_quote(user_email) + @user = Discourse.system_user + end end raise UserNotFoundError if @user.blank? @@ -196,14 +201,12 @@ module Email lines[range_start..range_end].join.strip end - def wrap_body_in_quote(user_email) - @body = "[quote=\"#{user_email}\"] -#{@body} -[/quote]" - end - private + def wrap_body_in_quote(user_email) + @body = "[quote=\"#{user_email}\"]\n#{@body}\n[/quote]" + end + def create_reply create_post_with_attachments(@email_log.user, raw: @body, diff --git a/lib/new_post_manager.rb b/lib/new_post_manager.rb index 901e7417e6f..f66421d6671 100644 --- a/lib/new_post_manager.rb +++ b/lib/new_post_manager.rb @@ -69,7 +69,7 @@ class NewPostManager def self.user_needs_approval?(manager) user = manager.user - return false if user.staff? + return false if user.staff? || user.staged (user.post_count < SiteSetting.approve_post_count) || (user.trust_level < SiteSetting.approve_unless_trust_level.to_i) || @@ -105,7 +105,6 @@ class NewPostManager end def perform - # We never queue private messages return perform_create_post if @args[:archetype] == Archetype.private_message if args[:topic_id] && Topic.where(id: args[:topic_id], archetype: Archetype.private_message).exists? @@ -145,7 +144,6 @@ class NewPostManager def perform_create_post result = NewPostResult.new(:create_post) - creator = PostCreator.new(@user, @args) post = creator.create result.check_errors_from(creator) diff --git a/lib/post_jobs_enqueuer.rb b/lib/post_jobs_enqueuer.rb index d4e1354aaae..c5f1f6a9d15 100644 --- a/lib/post_jobs_enqueuer.rb +++ b/lib/post_jobs_enqueuer.rb @@ -7,8 +7,8 @@ class PostJobsEnqueuer end def enqueue_jobs - # We need to enqueue jobs after the transaction. Otherwise they might begin before the data has - # been comitted. + # We need to enqueue jobs after the transaction. + # Otherwise they might begin before the data has been comitted. enqueue_post_alerts unless @opts[:import_mode] feature_topic_users unless @opts[:import_mode] trigger_post_post_process diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 82533078990..fe021624e2e 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -604,7 +604,6 @@ greatest show ever created. Everyone should watch it. SiteSetting.email_in = true 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) diff --git a/spec/models/user_email_observer_spec.rb b/spec/models/user_email_observer_spec.rb index f9307e1c4cf..a54fcbb34a5 100644 --- a/spec/models/user_email_observer_spec.rb +++ b/spec/models/user_email_observer_spec.rb @@ -3,184 +3,111 @@ require 'spec_helper' describe UserEmailObserver do # something is off with fabricator - def create_notification(type=nil, user=nil) + def create_notification(type, user=nil) user ||= Fabricate(:user) - type ||= Notification.types[:mentioned] Notification.create(data: '', user: user, notification_type: type) end - context 'user_mentioned' do - let!(:notification) do - create_notification - end + shared_examples "enqueue" do it "enqueues a job for the email" do - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_mentioned, user_id: notification.user_id, notification_id: notification.id) + Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: type, user_id: notification.user_id, notification_id: notification.id) UserEmailObserver.send(:new).after_commit(notification) end - it "enqueue a delayed job for users that are online" do - notification.user.last_seen_at = 1.minute.ago - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_mentioned, user_id: notification.user_id, notification_id: notification.id) - UserEmailObserver.send(:new).after_commit(notification) - end + context "inactive user" do - it "doesn't enqueue an email if the user has mention emails disabled" do - notification.user.expects(:email_direct?).returns(false) - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_mentioned)).never - UserEmailObserver.send(:new).after_commit(notification) - end + before { notification.user.active = false } + + it "doesn't enqueue a job" do + Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: type)).never + UserEmailObserver.send(:new).after_commit(notification) + end + + it "enqueues a job if the user is staged" do + notification.user.staged = true + Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: type, user_id: notification.user_id, notification_id: notification.id) + UserEmailObserver.send(:new).after_commit(notification) + end - it "doesn't enqueue an email if the user account is deactivated" do - notification.user.active = false - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_mentioned)).never - UserEmailObserver.send(:new).after_commit(notification) end end - context 'posted' do + shared_examples "enqueue_public" do + include_examples "enqueue" - let!(:notification) { create_notification(9) } - let(:user) { notification.user } - - it "enqueues a job for the email" do - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_posted, user_id: notification.user_id, notification_id: notification.id) + it "doesn't enqueue a job if the user has mention emails disabled" do + notification.user.expects(:email_direct?).returns(false) + Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: type)).never UserEmailObserver.send(:new).after_commit(notification) end + end - it "doesn't enqueue an email if the user has mention emails disabled" do - user.expects(:email_direct?).returns(false) - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_posted)).never + shared_examples "enqueue_private" do + include_examples "enqueue" + + it "doesn't enqueue a job if the user has private message emails disabled" do + notification.user.expects(:email_private_messages?).returns(false) + Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: type)).never UserEmailObserver.send(:new).after_commit(notification) end + end - it "doesn't enqueue an email if the user account is deactivated" do - user.active = false - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_posted)).never + context 'user_mentioned' do + let(:type) { :user_mentioned } + let!(:notification) { create_notification(1) } + + include_examples "enqueue_public" + + it "enqueue a delayed job for users that are online" do + notification.user.last_seen_at = 1.minute.ago + Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: type, user_id: notification.user_id, notification_id: notification.id) UserEmailObserver.send(:new).after_commit(notification) end end context 'user_replied' do - + let(:type) { :user_replied } let!(:notification) { create_notification(2) } - let(:user) { notification.user } - - it "enqueues a job for the email" do - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_replied, user_id: notification.user_id, notification_id: notification.id) - UserEmailObserver.send(:new).after_commit(notification) - end - - it "doesn't enqueue an email if the user has mention emails disabled" do - user.expects(:email_direct?).returns(false) - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_replied)).never - UserEmailObserver.send(:new).after_commit(notification) - end - - it "doesn't enqueue an email if the user account is deactivated" do - user.active = false - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_replied)).never - UserEmailObserver.send(:new).after_commit(notification) - end + include_examples "enqueue_public" end context 'user_quoted' do - + let(:type) { :user_quoted } let!(:notification) { create_notification(3) } - let(:user) { notification.user } - - it "enqueues a job for the email" do - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_quoted, user_id: notification.user_id, notification_id: notification.id) - UserEmailObserver.send(:new).after_commit(notification) - end - - it "doesn't enqueue an email if the user has mention emails disabled" do - user.expects(:email_direct?).returns(false) - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_quoted)).never - UserEmailObserver.send(:new).after_commit(notification) - end - - it "doesn't enqueue an email if the user account is deactivated" do - user.active = false - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_quoted)).never - UserEmailObserver.send(:new).after_commit(notification) - end + include_examples "enqueue_public" end - context 'email_user_invited_to_private_message' do - - let!(:notification) { create_notification(7) } - let(:user) { notification.user } - - it "enqueues a job for the email" do - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_invited_to_private_message, user_id: notification.user_id, notification_id: notification.id) - UserEmailObserver.send(:new).after_commit(notification) - end - - it "doesn't enqueue an email if the user has mention emails disabled" do - user.expects(:email_direct?).returns(false) - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_invited_to_private_message)).never - UserEmailObserver.send(:new).after_commit(notification) - end - - it "doesn't enqueue an email if the user account is deactivated" do - user.active = false - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_invited_to_private_message)).never - UserEmailObserver.send(:new).after_commit(notification) - end + context 'user_posted' do + let(:type) { :user_posted } + let!(:notification) { create_notification(9) } + include_examples "enqueue_public" end - context 'private_message' do - + context 'user_private_message' do + let(:type) { :user_private_message } let!(:notification) { create_notification(6) } - let(:user) { notification.user } - it "enqueues a job for the email" do - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_private_message, user_id: notification.user_id, notification_id: notification.id) - UserEmailObserver.send(:new).after_commit(notification) - end + include_examples "enqueue_private" + end - it "doesn't enqueue an email if the user has private message emails disabled" do - user.expects(:email_private_messages?).returns(false) - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_private_message)).never - UserEmailObserver.send(:new).after_commit(notification) - end - - it "doesn't enqueue an email if the user account is deactivated" do - user.active = false - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_private_message)).never - UserEmailObserver.send(:new).after_commit(notification) - end + context 'user_invited_to_private_message' do + let(:type) { :user_invited_to_private_message } + let!(:notification) { create_notification(7) } + include_examples "enqueue_public" end context 'user_invited_to_topic' do - + let(:type) { :user_invited_to_topic } let!(:notification) { create_notification(13) } - let(:user) { notification.user } - - it "enqueues a job for the email" do - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_invited_to_topic, user_id: notification.user_id, notification_id: notification.id) - UserEmailObserver.send(:new).after_commit(notification) - end - - it "doesn't enqueue an email if the user has mention emails disabled" do - user.expects(:email_direct?).returns(false) - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_invited_to_topic)).never - UserEmailObserver.send(:new).after_commit(notification) - end - - it "doesn't enqueue an email if the user account is deactivated" do - user.active = false - Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_invited_to_topic)).never - UserEmailObserver.send(:new).after_commit(notification) - end + include_examples "enqueue_public" end end