diff --git a/app/models/reviewable_user.rb b/app/models/reviewable_user.rb index 3f05984f11c..93291bfd40b 100644 --- a/app/models/reviewable_user.rb +++ b/app/models/reviewable_user.rb @@ -66,7 +66,7 @@ class ReviewableUser < Reviewable begin self.reject_reason = args[:reject_reason] - if args[:send_email] != false && SiteSetting.must_approve_users? + if args[:send_email] && SiteSetting.must_approve_users? # Execute job instead of enqueue because user has to exists to send email Jobs::CriticalUserEmail.new.execute({ type: :signup_after_reject, @@ -78,11 +78,16 @@ class ReviewableUser < Reviewable delete_args = {} delete_args[:block_ip] = true if args[:block_ip] delete_args[:block_email] = true if args[:block_email] + delete_args[:context] = if performed_by.id == Discourse.system_user.id + I18n.t("user.destroy_reasons.reviewable_reject_auto") + else + I18n.t("user.destroy_reasons.reviewable_reject") + end destroyer.destroy(target, delete_args) rescue UserDestroyer::PostsExistError # If a user has posts, we won't delete them to preserve their content. - # However the reviable record will be "rejected" and they will remain + # However the reviewable record will be "rejected" and they will remain # unapproved in the database. A staff member can still approve them # via the admin. end diff --git a/app/models/user.rb b/app/models/user.rb index 5722ae61f44..166494be039 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -53,7 +53,6 @@ class User < ActiveRecord::Base has_many :bookmarks, dependent: :delete_all has_many :notifications, dependent: :delete_all has_many :topic_users, dependent: :delete_all - has_many :email_logs, dependent: :delete_all has_many :incoming_emails, dependent: :delete_all has_many :user_visits, dependent: :delete_all has_many :user_auth_token_logs, dependent: :delete_all @@ -67,6 +66,7 @@ class User < ActiveRecord::Base has_many :post_actions has_many :post_timings has_many :directory_items + has_many :email_logs has_many :security_keys, -> { where(enabled: true) }, class_name: "UserSecurityKey" diff --git a/app/services/destroy_task.rb b/app/services/destroy_task.rb index 57afc0e3d72..4c5036eca66 100644 --- a/app/services/destroy_task.rb +++ b/app/services/destroy_task.rb @@ -81,7 +81,7 @@ class DestroyTask def destroy_users User.human_users.where(admin: false).find_each do |user| begin - if UserDestroyer.new(Discourse.system_user).destroy(user, delete_posts: true) + if UserDestroyer.new(Discourse.system_user).destroy(user, delete_posts: true, context: "destroy task") @io.puts "#{user.username} deleted" else @io.puts "#{user.username} not deleted" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c7c554cba86..6eb78d05236 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2564,6 +2564,8 @@ en: fixed_primary_email: "Fixed primary email for staged user" same_ip_address: "Same IP address (%{ip_address}) as other users" inactive_user: "Inactive user" + reviewable_reject_auto: "Auto handle queued reviewables" + reviewable_reject: "Reviewable user rejected" email_in_spam_header: "User's first email was flagged as spam" already_silenced: "User was already silenced by %{staff} %{time_ago}." already_suspended: "User was already suspended by %{staff} %{time_ago}." diff --git a/spec/integration/auto_reject_reviewable_users_spec.rb b/spec/integration/auto_reject_reviewable_users_spec.rb new file mode 100644 index 00000000000..1b827c0440d --- /dev/null +++ b/spec/integration/auto_reject_reviewable_users_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe "auto reject reviewable users" do + context "reviewable users" do + fab!(:old_user) { Fabricate(:reviewable, created_at: 80.days.ago) } + + it "does not send email to rejected user" do + SiteSetting.must_approve_users = true + SiteSetting.auto_handle_queued_age = 60 + + Jobs::CriticalUserEmail.any_instance.expects(:execute).never + Jobs::AutoQueueHandler.new.execute({}) + + expect(old_user.reload.rejected?).to eq(true) + expect(UserHistory.last.context).to eq( + I18n.t("user.destroy_reasons.reviewable_reject_auto") + ) + end + end +end diff --git a/spec/models/reviewable_user_spec.rb b/spec/models/reviewable_user_spec.rb index eabc3623388..b63c4901c95 100644 --- a/spec/models/reviewable_user_spec.rb +++ b/spec/models/reviewable_user_spec.rb @@ -105,6 +105,9 @@ RSpec.describe ReviewableUser, type: :model do reviewable.reload expect(reviewable.target).to be_blank expect(reviewable.reject_reason).to eq("reject reason") + expect(UserHistory.last.context).to eq( + I18n.t("user.destroy_reasons.reviewable_reject") + ) end it "allows us to reject and block a user" do @@ -129,7 +132,7 @@ RSpec.describe ReviewableUser, type: :model do it "is not sending email to the user about rejection" do SiteSetting.must_approve_users = true Jobs::CriticalUserEmail.any_instance.expects(:execute).never - reviewable.perform(moderator, :reject_user_block, reject_reason: "reject reason", send_email: false) + reviewable.perform(moderator, :reject_user_block, reject_reason: "reject reason") end it "optionaly sends email with reject reason" do diff --git a/spec/services/user_destroyer_spec.rb b/spec/services/user_destroyer_spec.rb index ea23dbf0b96..9b474bf8e64 100644 --- a/spec/services/user_destroyer_spec.rb +++ b/spec/services/user_destroyer_spec.rb @@ -364,16 +364,6 @@ describe UserDestroyer do end end - context 'user got an email' do - let!(:email_log) { Fabricate(:email_log, user: user) } - - it "deletes the email log" do - expect { - UserDestroyer.new(admin).destroy(user, delete_posts: true) - }.to change { EmailLog.count }.by(-1) - end - end - context 'user liked things' do before do @topic = Fabricate(:topic, user: Fabricate(:user)) @@ -446,6 +436,16 @@ describe UserDestroyer do expect(log.details).to include(username) end end + + context 'user got an email' do + let!(:email_log) { Fabricate(:email_log, user: user) } + + it "does not delete the email log" do + expect { + UserDestroyer.new(admin).destroy(user, delete_posts: true) + }.to_not change { EmailLog.count } + end + end end end