diff --git a/app/services/user_destroyer.rb b/app/services/user_destroyer.rb index e95d4ca60dd..b245bd32d8c 100644 --- a/app/services/user_destroyer.rb +++ b/app/services/user_destroyer.rb @@ -80,7 +80,10 @@ class UserDestroyer end end - StaffActionLogger.new(@actor == user ? Discourse.system_user : @actor).log_user_deletion(user, opts.slice(:context)) + unless opts[:quiet] + StaffActionLogger.new(@actor == user ? Discourse.system_user : @actor).log_user_deletion(user, opts.slice(:context)) + end + MessageBus.publish "/file-change", ["refresh"], user_ids: [u.id] end end diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 9eb6a69cd8b..6cb0adf8eb2 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -58,6 +58,7 @@ module Email error = e.to_s error = e.class.name if error.blank? @incoming_email.update_columns(error: error) if @incoming_email + delete_staged_users raise end end @@ -752,6 +753,12 @@ module Email message = SubscriptionMailer.send(action, user) Email::Sender.new(message, :subscription).send end + + def delete_staged_users + @staged_users.each do |user| + UserDestroyer.new(Discourse.system_user).destroy(user, quiet: true) if user.posts.count == 0 + end + end end end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index c6eee47b72a..69338c3b16a 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -648,10 +648,10 @@ describe Email::Receiver do SiteSetting.enable_staged_users = true end - shared_examples "no staged users" do |email_name| + shared_examples "no staged users" do |email_name, expected_exception| it "does not create staged users" do staged_user_count = User.where(staged: true).count - process(email_name) rescue nil + expect { process(email_name) }.to raise_error(expected_exception) expect(User.where(staged: true).count).to eq(staged_user_count) end end @@ -661,23 +661,23 @@ describe Email::Receiver do ScreenedEmail.expects(:should_block?).with("screened@mail.com").returns(true) end - include_examples "no staged users", :screened_email + include_examples "no staged users", :screened_email, Email::Receiver::ScreenedEmailError end context "when the mail is auto generated" do - include_examples "no staged users", :auto_generated_header + include_examples "no staged users", :auto_generated_header, Email::Receiver::AutoGeneratedEmailError end context "when email is a bounced email" do - include_examples "no staged users", :bounced_email + include_examples "no staged users", :bounced_email, Email::Receiver::BouncedEmailError end context "when the body is blank" do - include_examples "no staged users", :no_body + include_examples "no staged users", :no_body, Email::Receiver::NoBodyDetectedError end context "when unsubscribe via email is not allowed" do - include_examples "no staged users", :unsubscribe_new_user + include_examples "no staged users", :unsubscribe_new_user, Email::Receiver::UnsubscribeNotAllowed end context "when email address is not on whitelist" do @@ -685,7 +685,7 @@ describe Email::Receiver do SiteSetting.email_domains_whitelist = "example.com|bar.com" end - include_examples "no staged users", :blacklist_whitelist_email + include_examples "no staged users", :blacklist_whitelist_email, Email::Receiver::EmailNotAllowed end context "when email address is on blacklist" do @@ -693,7 +693,71 @@ describe Email::Receiver do SiteSetting.email_domains_blacklist = "email.com|mail.com" end - include_examples "no staged users", :blacklist_whitelist_email + include_examples "no staged users", :blacklist_whitelist_email, Email::Receiver::EmailNotAllowed + end + + context "when destinations aren't matching any of the incoming emails" do + include_examples "no staged users", :bad_destinations, Email::Receiver::BadDestinationAddress + end + + context "when email is sent to category" do + context "when email is sent by a new user and category does not allow strangers" do + let!(:category) { Fabricate(:category, email_in: "category@foo.com", email_in_allow_strangers: false) } + + include_examples "no staged users", :new_user, Email::Receiver::StrangersNotAllowedError + end + + context "when email has no date" do + let!(:category) { Fabricate(:category, email_in: "category@foo.com", email_in_allow_strangers: true) } + + include_examples "no staged users", :no_date, Email::Receiver::InvalidPost + end + end + + context "email is a reply" do + let(:reply_key) { "4f97315cc828096c9cb34c6f1a0d6fe8" } + let(:category) { Fabricate(:category) } + let(:user) { Fabricate(:user, email: "discourse@bar.com") } + let(:topic) { create_topic(category: category, user: user) } + let(:post) { create_post(topic: topic, user: user) } + let!(:email_log) { Fabricate(:email_log, reply_key: reply_key, user: user, topic: topic, post: post) } + + context "when the email address isn't matching the one we sent the notification to" do + include_examples "no staged users", :reply_user_not_matching, Email::Receiver::ReplyUserNotMatchingError + end + end + + context "replying without key is allowed" do + let!(:group) { Fabricate(:group, incoming_email: "team@bar.com") } + let!(:topic) do + SiteSetting.find_related_post_with_key = false + process(:email_reply_1) + Topic.last + end + + context "when the topic was deleted" do + before do + topic.update_columns(deleted_at: 1.day.ago) + end + + include_examples "no staged users", :email_reply_staged, Email::Receiver::TopicNotFoundError + end + + context "when the topic was closed" do + before do + topic.update_columns(closed: true) + end + + include_examples "no staged users", :email_reply_staged, Email::Receiver::TopicClosedError + end + + context "when they aren't allowed to like a post" do + before do + topic.update_columns(archived: true) + end + + include_examples "no staged users", :email_reply_like, Email::Receiver::InvalidPostAction + end end end diff --git a/spec/fixtures/emails/email_reply_like.eml b/spec/fixtures/emails/email_reply_like.eml new file mode 100644 index 00000000000..0e1a5ef93c8 --- /dev/null +++ b/spec/fixtures/emails/email_reply_like.eml @@ -0,0 +1,13 @@ +Return-Path: <four@foo.com> +From: Four <four@foo.com> +To: one@foo.com +Cc: team@bar.com +Subject: RE: Testing email threading +Date: Fri, 15 Jan 2016 00:12:43 +0100 +Message-ID: <38@foo.bar.mail> +In-Reply-To: <34@foo.bar.mail> +Mime-Version: 1.0 +Content-Type: text/plain +Content-Transfer-Encoding: 7bit + ++1 diff --git a/spec/fixtures/emails/email_reply_staged.eml b/spec/fixtures/emails/email_reply_staged.eml new file mode 100644 index 00000000000..b25041e77bc --- /dev/null +++ b/spec/fixtures/emails/email_reply_staged.eml @@ -0,0 +1,13 @@ +Return-Path: <staged@foo.com> +From: Staged <staged@foo.com> +To: one@foo.com +Cc: team@bar.com, three@foo.com +Subject: RE: Testing email threading +Date: Fri, 15 Jan 2016 00:12:43 +0100 +Message-ID: <35@foo.bar.mail> +In-Reply-To: <34@foo.bar.mail> +Mime-Version: 1.0 +Content-Type: text/plain +Content-Transfer-Encoding: 7bit + +This is email reply **2**. diff --git a/spec/fixtures/emails/no_date.eml b/spec/fixtures/emails/no_date.eml new file mode 100644 index 00000000000..79989dcbfc1 --- /dev/null +++ b/spec/fixtures/emails/no_date.eml @@ -0,0 +1,10 @@ +Return-Path: <discourse@bar.com> +From: Foo Bar <discourse@bar.com> +To: category@foo.com +Subject: This is a topic from a complete stranger +Message-ID: <31@foo.bar.mail> +Mime-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: quoted-printable + +Hey, this is a topic from a complete stranger ;) diff --git a/spec/services/user_destroyer_spec.rb b/spec/services/user_destroyer_spec.rb index 838607a45a1..3c829d89fdb 100644 --- a/spec/services/user_destroyer_spec.rb +++ b/spec/services/user_destroyer_spec.rb @@ -45,6 +45,12 @@ describe UserDestroyer do StaffActionLogger.any_instance.expects(:log_user_deletion).with(@user, anything).once destroy end + + it "should not log the action if quiet is true" do + expect { + UserDestroyer.new(@admin).destroy(@user, destroy_opts.merge(quiet: true)) + }.to_not change { UserHistory.where(action: UserHistory.actions[:delete_user]).count } + end end shared_examples "email block list" do @@ -202,6 +208,7 @@ describe UserDestroyer do # out of sync user_stat data shouldn't break UserDestroyer @user.user_stat.update_attribute(:post_count, 1) end + let(:destroy_opts) { {} } subject(:destroy) { UserDestroyer.new(@user).destroy(@user, delete_posts: false) } include_examples "successfully destroy a user"