diff --git a/app/models/blocked_email.rb b/app/models/blocked_email.rb index 6bc8a48570e..0fc451a1223 100644 --- a/app/models/blocked_email.rb +++ b/app/models/blocked_email.rb @@ -8,18 +8,24 @@ class BlockedEmail < ActiveRecord::Base @actions ||= Enum.new(:block, :do_nothing) end + def self.block(email, opts={}) + find_by_email(email) || create(opts.slice(:action_type).merge({email: email})) + end + def self.should_block?(email) - record = BlockedEmail.where(email: email).first - if record - record.match_count += 1 - record.last_match_at = Time.zone.now - record.save - end - record && record.action_type == actions[:block] + blocked_email = BlockedEmail.where(email: email).first + blocked_email.record_match! if blocked_email + blocked_email && blocked_email.action_type == actions[:block] end def set_defaults self.action_type ||= BlockedEmail.actions[:block] end + def record_match! + self.match_count += 1 + self.last_match_at = Time.zone.now + save + end + end diff --git a/lib/user_destroyer.rb b/lib/user_destroyer.rb index be949bea14b..8b15e1dd6d7 100644 --- a/lib/user_destroyer.rb +++ b/lib/user_destroyer.rb @@ -23,6 +23,10 @@ class UserDestroyer end user.destroy.tap do |u| if u + if opts[:block_email] + b = BlockedEmail.block(u.email) + b.record_match! if b + end Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true") StaffActionLogger.new(@admin).log_user_deletion(user) DiscourseHub.unregister_nickname(user.username) if SiteSetting.call_discourse_hub? diff --git a/spec/components/user_destroyer_spec.rb b/spec/components/user_destroyer_spec.rb index c23cb7c2730..1b90c1c8ed4 100644 --- a/spec/components/user_destroyer_spec.rb +++ b/spec/components/user_destroyer_spec.rb @@ -72,6 +72,20 @@ describe UserDestroyer do end end + shared_examples "email block list" do + it "doesn't add email to block list by default" do + BlockedEmail.expects(:block).never + destroy + end + + it "adds email to block list if block_email is true" do + b = Fabricate.build(:blocked_email, email: @user.email) + BlockedEmail.expects(:block).with(@user.email).returns(b) + b.expects(:record_match!).once.returns(true) + UserDestroyer.new(@admin).destroy(@user, destroy_opts.merge({block_email: true})) + end + end + context 'user has posts' do let!(:post) { Fabricate(:post, user: @user) } @@ -98,9 +112,11 @@ describe UserDestroyer do end context "delete_posts is true" do - subject(:destroy) { UserDestroyer.new(@admin).destroy(@user, delete_posts: true) } + let(:destroy_opts) { {delete_posts: true} } + subject(:destroy) { UserDestroyer.new(@admin).destroy(@user, destroy_opts) } include_examples "successfully destroy a user" + include_examples "email block list" it "deletes the posts" do destroy @@ -113,9 +129,11 @@ describe UserDestroyer do context 'user has no posts' do context 'and destroy succeeds' do + let(:destroy_opts) { {} } subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) } include_examples "successfully destroy a user" + include_examples "email block list" it "should mark the user's deleted posts as belonging to a nuked user" do post = Fabricate(:post, user: @user, deleted_at: 1.hour.ago) diff --git a/spec/models/blocked_email_spec.rb b/spec/models/blocked_email_spec.rb index d1a6fe81ebb..ea473291a72 100644 --- a/spec/models/blocked_email_spec.rb +++ b/spec/models/blocked_email_spec.rb @@ -6,18 +6,48 @@ describe BlockedEmail do describe "new record" do it "sets a default action_type" do - BlockedEmail.create(email: email).action_type.should == BlockedEmail.actions[:block] + described_class.create(email: email).action_type.should == described_class.actions[:block] end it "last_match_at is null" do # If we manually load the table with some emails, we can see whether those emails # have ever been blocked by looking at last_match_at. - BlockedEmail.create(email: email).last_match_at.should be_nil + described_class.create(email: email).last_match_at.should be_nil end end - describe "#should_block?" do - subject { BlockedEmail.should_block?(email) } + describe '#block' do + context 'email is not being blocked' do + it 'creates a new record with default action of :block' do + record = described_class.block(email) + record.should_not be_new_record + record.email.should == email + record.action_type.should == described_class.actions[:block] + end + + it 'lets action_type be overriden' do + record = described_class.block(email, action_type: described_class.actions[:do_nothing]) + record.should_not be_new_record + record.email.should == email + record.action_type.should == described_class.actions[:do_nothing] + end + end + + context 'email is already being blocked' do + let!(:existing) { Fabricate(:blocked_email, email: email) } + + it "doesn't create a new record" do + expect { described_class.block(email) }.to_not change { described_class.count } + end + + it "returns the existing record" do + described_class.block(email).should == existing + end + end + end + + describe '#should_block?' do + subject { described_class.should_block?(email) } it "returns false if a record with the email doesn't exist" do subject.should be_false