FEATURE: Staff can receive pending user reminders more frequently. (#13422)

* FEATURE: Staff can receive pending user reminders more frequently.

We now express the "pending_users_reminder_delay"  in minutes instead of hours so staff can have finer control over the delay.

We need to keep in mind that the reminders could still take up to 20 minutes, even when using a lower value. We send them from a scheduled job.

* Migrate to a new site setting for the reminders delay
This commit is contained in:
Roman Rizzi 2021-06-24 10:02:56 -03:00 committed by GitHub
parent 20070f5089
commit 2c918a3161
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 39 additions and 16 deletions

View File

@ -3,13 +3,13 @@
module Jobs module Jobs
class PendingUsersReminder < ::Jobs::Scheduled class PendingUsersReminder < ::Jobs::Scheduled
every 1.hour every 5.minutes
def execute(args) def execute(args)
if SiteSetting.must_approve_users && SiteSetting.pending_users_reminder_delay >= 0 if SiteSetting.must_approve_users && SiteSetting.pending_users_reminder_delay_minutes >= 0
query = AdminUserIndexQuery.new(query: 'pending', stats: false).find_users_query # default order is: users.created_at DESC query = AdminUserIndexQuery.new(query: 'pending', stats: false).find_users_query # default order is: users.created_at DESC
if SiteSetting.pending_users_reminder_delay > 0 if SiteSetting.pending_users_reminder_delay_minutes > 0
query = query.where('users.created_at < ?', SiteSetting.pending_users_reminder_delay.hours.ago) query = query.where('users.created_at < ?', SiteSetting.pending_users_reminder_delay_minutes.minutes.ago)
end end
newest_username = query.limit(1).select(:username).first&.username newest_username = query.limit(1).select(:username).first&.username

View File

@ -1601,7 +1601,7 @@ en:
invite_code: "User must type this code to be allowed account registration, ignored when empty (case-insensitive)" invite_code: "User must type this code to be allowed account registration, ignored when empty (case-insensitive)"
approve_suspect_users: "Add suspicious users to the review queue. Suspicious users have entered a bio/website but have no reading activity." approve_suspect_users: "Add suspicious users to the review queue. Suspicious users have entered a bio/website but have no reading activity."
review_every_post: "All posts must be reviewed. WARNING! NOT RECOMMENDED FOR BUSY SITES." review_every_post: "All posts must be reviewed. WARNING! NOT RECOMMENDED FOR BUSY SITES."
pending_users_reminder_delay: "Notify moderators if new users have been waiting for approval for longer than this many hours. Set to -1 to disable notifications." pending_users_reminder_delay_minutes: "Notify moderators if new users have been waiting for approval for longer than this many minutes. Set to -1 to disable notifications."
persistent_sessions: "Users will remain logged in when the web browser is closed" persistent_sessions: "Users will remain logged in when the web browser is closed"
maximum_session_age: "User will remain logged in for n hours since last visit" maximum_session_age: "User will remain logged in for n hours since last visit"
ga_version: "Version of Google Universal Analytics to use: v3 (analytics.js), v4 (gtag)" ga_version: "Version of Google Universal Analytics to use: v3 (analytics.js), v4 (gtag)"

View File

@ -500,9 +500,9 @@ login:
list_type: simple list_type: simple
hide_email_address_taken: false hide_email_address_taken: false
log_out_strict: false log_out_strict: false
pending_users_reminder_delay: pending_users_reminder_delay_minutes:
min: -1 min: -1
default: 8 default: 480
persistent_sessions: true persistent_sessions: true
maximum_session_age: maximum_session_age:
default: 1440 default: 1440

View File

@ -0,0 +1,22 @@
# frozen_string_literal: true
class MigratePendingUsersReminderDelaySetting < ActiveRecord::Migration[6.1]
def up
setting_value = DB.query_single("SELECT value FROM site_settings WHERE name = 'pending_users_reminder_delay'").first
if setting_value.present?
new_value = setting_value.to_i
new_value = new_value > 0 ? new_value * 60 : new_value
DB.exec(<<~SQL, delay: new_value)
INSERT INTO site_settings (name, data_type, value, created_at, updated_at)
VALUES ('pending_users_reminder_delay_minutes', 3, :delay, NOW(), NOW())
SQL
DB.exec("DELETE FROM site_settings WHERE name = 'pending_users_reminder_delay'")
end
end
def down
end
end

View File

@ -21,29 +21,30 @@ describe Jobs::PendingUsersReminder do
Group.refresh_automatic_group!(:moderators) Group.refresh_automatic_group!(:moderators)
end end
it "sends a message if user was created more than pending_users_reminder_delay hours ago" do it "sends a message if user was created more than pending_users_reminder_delay minutes ago" do
SiteSetting.pending_users_reminder_delay = 8 SiteSetting.pending_users_reminder_delay_minutes = 8
Fabricate(:user, created_at: 9.hours.ago) Fabricate(:user, created_at: 9.minutes.ago)
PostCreator.expects(:create).once PostCreator.expects(:create).once
Jobs::PendingUsersReminder.new.execute({}) Jobs::PendingUsersReminder.new.execute({})
end end
it "doesn't send a message if user was created less than pending_users_reminder_delay hours ago" do it "doesn't send a message if user was created less than pending_users_reminder_delay minutes ago" do
SiteSetting.pending_users_reminder_delay = 8 SiteSetting.pending_users_reminder_delay_minutes = 8
Fabricate(:user, created_at: 2.hours.ago) Fabricate(:user, created_at: 2.minutes.ago)
PostCreator.expects(:create).never PostCreator.expects(:create).never
Jobs::PendingUsersReminder.new.execute({}) Jobs::PendingUsersReminder.new.execute({})
end end
it "doesn't send a message if pending_users_reminder_delay is -1" do it "doesn't send a message if pending_users_reminder_delay is -1" do
SiteSetting.pending_users_reminder_delay = -1 SiteSetting.pending_users_reminder_delay_minutes = -1
Fabricate(:user, created_at: 24.hours.ago)
PostCreator.expects(:create).never PostCreator.expects(:create).never
Jobs::PendingUsersReminder.new.execute({}) Jobs::PendingUsersReminder.new.execute({})
end end
it "sets the correct pending user count in the notification" do it "sets the correct pending user count in the notification" do
SiteSetting.pending_users_reminder_delay = 8 SiteSetting.pending_users_reminder_delay_minutes = 8
Fabricate(:user, created_at: 9.hours.ago) Fabricate(:user, created_at: 9.minutes.ago)
PostCreator.expects(:create).with(Discourse.system_user, has_entries(title: '1 user waiting for approval')) PostCreator.expects(:create).with(Discourse.system_user, has_entries(title: '1 user waiting for approval'))
Jobs::PendingUsersReminder.new.execute({}) Jobs::PendingUsersReminder.new.execute({})
end end