diff --git a/spec/services/user_destroyer_spec.rb b/spec/services/user_destroyer_spec.rb index f1b37f7bf18..5582a05b2fe 100644 --- a/spec/services/user_destroyer_spec.rb +++ b/spec/services/user_destroyer_spec.rb @@ -4,7 +4,7 @@ require 'rails_helper' describe UserDestroyer do - fab!(:user) { Fabricate(:user) } + fab!(:user) { Fabricate(:user_with_secondary_email) } fab!(:admin) { Fabricate(:admin) } describe 'new' do @@ -18,21 +18,16 @@ describe UserDestroyer do end describe 'destroy' do - before do - @admin = Fabricate(:admin) - @user = Fabricate(:user_with_secondary_email) - end - it 'raises an error when user is nil' do - expect { UserDestroyer.new(@admin).destroy(nil) }.to raise_error(Discourse::InvalidParameters) + expect { UserDestroyer.new(admin).destroy(nil) }.to raise_error(Discourse::InvalidParameters) end it 'raises an error when user is not a User' do - expect { UserDestroyer.new(@admin).destroy('nothing') }.to raise_error(Discourse::InvalidParameters) + expect { UserDestroyer.new(admin).destroy('nothing') }.to raise_error(Discourse::InvalidParameters) end it 'raises an error when regular user tries to delete another user' do - expect { UserDestroyer.new(@user).destroy(Fabricate(:user)) }.to raise_error(Discourse::InvalidAccess) + expect { UserDestroyer.new(user).destroy(Fabricate(:user)) }.to raise_error(Discourse::InvalidAccess) end shared_examples "successfully destroy a user" do @@ -42,18 +37,18 @@ describe UserDestroyer do it 'should return the deleted user record' do return_value = destroy - expect(return_value).to eq(@user) + expect(return_value).to eq(user) expect(return_value).to be_destroyed end it 'should log the action' do - StaffActionLogger.any_instance.expects(:log_user_deletion).with(@user, anything).once + 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)) + UserDestroyer.new(admin).destroy(user, destroy_opts.merge(quiet: true)) }.to_not change { UserHistory.where(action: UserHistory.actions[:delete_user]).count } end @@ -61,7 +56,7 @@ describe UserDestroyer do event = DiscourseEvent.track_events { destroy }.last expect(event[:event_name]).to eq(:user_destroyed) - expect(event[:params].first).to eq(@user) + expect(event[:params].first).to eq(user) end end @@ -73,14 +68,14 @@ describe UserDestroyer do it "adds emails to block list if block_email is true" do expect { - UserDestroyer.new(@admin).destroy(@user, destroy_opts.merge(block_email: true)) + UserDestroyer.new(admin).destroy(user, destroy_opts.merge(block_email: true)) }.to change { ScreenedEmail.count }.by(2) end end context 'user deletes self' do let(:destroy_opts) { { delete_posts: true, context: "/u/username/preferences/account" } } - subject(:destroy) { UserDestroyer.new(@user).destroy(@user, destroy_opts) } + subject(:destroy) { UserDestroyer.new(user).destroy(user, destroy_opts) } include_examples "successfully destroy a user" @@ -139,13 +134,13 @@ describe UserDestroyer do let!(:topic_starter) { Fabricate(:user) } let!(:topic) { Fabricate(:topic, user: topic_starter) } let!(:first_post) { Fabricate(:post, user: topic_starter, topic: topic) } - let!(:post) { Fabricate(:post, user: @user, topic: topic) } + let!(:post) { Fabricate(:post, user: user, topic: topic) } context "delete_posts is false" do - subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) } + subject(:destroy) { UserDestroyer.new(admin).destroy(user) } before do - @user.stubs(:post_count).returns(1) - @user.stubs(:first_post_created_at).returns(Time.zone.now) + user.stubs(:post_count).returns(1) + user.stubs(:first_post_created_at).returns(Time.zone.now) end it 'should raise the right error' do @@ -159,7 +154,7 @@ describe UserDestroyer do let(:destroy_opts) { { delete_posts: true } } context "staff deletes user" do - subject(:destroy) { UserDestroyer.new(@admin).destroy(@user, destroy_opts) } + subject(:destroy) { UserDestroyer.new(admin).destroy(user, destroy_opts) } include_examples "successfully destroy a user" include_examples "email block list" @@ -177,8 +172,8 @@ describe UserDestroyer do end it "deletes topics started by the deleted user" do - spammer_topic = Fabricate(:topic, user: @user) - Fabricate(:post, user: @user, topic: spammer_topic) + spammer_topic = Fabricate(:topic, user: user) + Fabricate(:post, user: user, topic: spammer_topic) destroy expect(spammer_topic.reload.deleted_at).not_to eq(nil) expect(spammer_topic.user_id).to eq(nil) @@ -189,8 +184,8 @@ describe UserDestroyer do before { destroy_opts[:delete_as_spammer] = true } it "approves reviewable flags" do - spammer_post = Fabricate(:post, user: @user) - reviewable = PostActionCreator.inappropriate(@admin, spammer_post).reviewable + spammer_post = Fabricate(:post, user: user) + reviewable = PostActionCreator.inappropriate(admin, spammer_post).reviewable expect(reviewable).to be_pending destroy @@ -203,7 +198,7 @@ describe UserDestroyer do end context "users deletes self" do - subject(:destroy) { UserDestroyer.new(@user).destroy(@user, destroy_opts) } + subject(:destroy) { UserDestroyer.new(user).destroy(user, destroy_opts) } include_examples "successfully destroy a user" include_examples "email block list" @@ -220,18 +215,18 @@ describe UserDestroyer do context 'user has no posts, but user_stats table has post_count > 0' do before do # out of sync user_stat data shouldn't break UserDestroyer - @user.user_stat.update_attribute(:post_count, 1) + user.user_stat.update_attribute(:post_count, 1) end let(:destroy_opts) { {} } - subject(:destroy) { UserDestroyer.new(@user).destroy(@user, delete_posts: false) } + subject(:destroy) { UserDestroyer.new(user).destroy(user, delete_posts: false) } include_examples "successfully destroy a user" end context 'user has deleted posts' do - let!(:deleted_post) { Fabricate(:post, user: @user, deleted_at: 1.hour.ago) } + let!(:deleted_post) { Fabricate(:post, user: user, deleted_at: 1.hour.ago) } it "should mark the user's deleted posts as belonging to a nuked user" do - expect { UserDestroyer.new(@admin).destroy(@user) }.to change { User.count }.by(-1) + expect { UserDestroyer.new(admin).destroy(user) }.to change { User.count }.by(-1) expect(deleted_post.reload.user_id).to eq(nil) end end @@ -239,17 +234,17 @@ describe UserDestroyer do context 'user has no posts' do context 'and destroy succeeds' do let(:destroy_opts) { {} } - subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) } + subject(:destroy) { UserDestroyer.new(admin).destroy(user) } include_examples "successfully destroy a user" include_examples "email block list" end context 'and destroy fails' do - subject(:destroy) { UserDestroyer.new(@admin).destroy(@user) } + subject(:destroy) { UserDestroyer.new(admin).destroy(user) } it 'should not log the action' do - @user.stubs(:destroy).returns(false) + user.stubs(:destroy).returns(false) StaffActionLogger.any_instance.expects(:log_user_deletion).never destroy end @@ -259,43 +254,43 @@ describe UserDestroyer do context 'user has posts with links' do context 'external links' do before do - @post = Fabricate(:post_with_external_links, user: @user) + @post = Fabricate(:post_with_external_links, user: user) TopicLink.extract_from(@post) end it "doesn't add ScreenedUrl records by default" do ScreenedUrl.expects(:watch).never - UserDestroyer.new(@admin).destroy(@user, delete_posts: true) + UserDestroyer.new(admin).destroy(user, delete_posts: true) end it "adds ScreenedUrl records when :block_urls is true" do ScreenedUrl.expects(:watch).with(anything, anything, has_key(:ip_address)).at_least_once - UserDestroyer.new(@admin).destroy(@user, delete_posts: true, block_urls: true) + UserDestroyer.new(admin).destroy(user, delete_posts: true, block_urls: true) end end context 'internal links' do before do - @post = Fabricate(:post_with_external_links, user: @user) + @post = Fabricate(:post_with_external_links, user: user) TopicLink.extract_from(@post) TopicLink.any_instance.stubs(:internal).returns(true) end it "doesn't add ScreenedUrl records" do ScreenedUrl.expects(:watch).never - UserDestroyer.new(@admin).destroy(@user, delete_posts: true, block_urls: true) + UserDestroyer.new(admin).destroy(user, delete_posts: true, block_urls: true) end end context 'with oneboxed links' do before do - @post = Fabricate(:post_with_youtube, user: @user) + @post = Fabricate(:post_with_youtube, user: user) TopicLink.extract_from(@post) end it "doesn't add ScreenedUrl records" do ScreenedUrl.expects(:watch).never - UserDestroyer.new(@admin).destroy(@user, delete_posts: true, block_urls: true) + UserDestroyer.new(admin).destroy(user, delete_posts: true, block_urls: true) end end end @@ -303,29 +298,29 @@ describe UserDestroyer do context 'ip address screening' do it "doesn't create screened_ip_address records by default" do ScreenedIpAddress.expects(:watch).never - UserDestroyer.new(@admin).destroy(@user) + UserDestroyer.new(admin).destroy(user) end context "block_ip is true" do it "creates a new screened_ip_address record" do - ScreenedIpAddress.expects(:watch).with(@user.ip_address).returns(stub_everything) - UserDestroyer.new(@admin).destroy(@user, block_ip: true) + ScreenedIpAddress.expects(:watch).with(user.ip_address).returns(stub_everything) + UserDestroyer.new(admin).destroy(user, block_ip: true) end it "creates two new screened_ip_address records when registration_ip_address is different than last ip_address" do - @user.registration_ip_address = '12.12.12.12' - ScreenedIpAddress.expects(:watch).with(@user.ip_address).returns(stub_everything) - ScreenedIpAddress.expects(:watch).with(@user.registration_ip_address).returns(stub_everything) - UserDestroyer.new(@admin).destroy(@user, block_ip: true) + user.registration_ip_address = '12.12.12.12' + ScreenedIpAddress.expects(:watch).with(user.ip_address).returns(stub_everything) + ScreenedIpAddress.expects(:watch).with(user.registration_ip_address).returns(stub_everything) + UserDestroyer.new(admin).destroy(user, block_ip: true) end end end context 'user created a category' do - let!(:category) { Fabricate(:category_with_definition, user: @user) } + let!(:category) { Fabricate(:category_with_definition, user: user) } it "assigns the system user to the categories" do - UserDestroyer.new(@admin).destroy(@user, delete_posts: true) + UserDestroyer.new(admin).destroy(user, delete_posts: true) expect(category.reload.user_id).to eq(Discourse.system_user.id) expect(category.topic).to be_present expect(category.topic.user_id).to eq(Discourse.system_user.id) @@ -357,7 +352,7 @@ describe UserDestroyer do it "deletes the email log" do expect { - UserDestroyer.new(@admin).destroy(user, delete_posts: true) + UserDestroyer.new(admin).destroy(user, delete_posts: true) }.to change { EmailLog.count }.by(-1) end end @@ -366,12 +361,12 @@ describe UserDestroyer do before do @topic = Fabricate(:topic, user: Fabricate(:user)) @post = Fabricate(:post, user: @topic.user, topic: @topic) - PostActionCreator.like(@user, @post) + PostActionCreator.like(user, @post) end it 'should destroy the like' do expect { - UserDestroyer.new(@admin).destroy(@user, delete_posts: true) + UserDestroyer.new(admin).destroy(user, delete_posts: true) }.to change { PostAction.count }.by(-1) expect(@post.reload.like_count).to eq(0) end