DEV: Convert approve_unless_trust_level to groups (#24357)

This change converts the `approve_unless_trust_level` site setting to
`approve_unless_allowed_groups`.

See: https://meta.discourse.org/t/283408

- Adds the new site setting
- Adds a deprecation warning
- Updates core to use the new settings.
- Adds a migration to fill in the new setting of the old setting was
  changed
- Adds an entry to the site_setting.keywords section
- Updates many tests to account for the new change

After a couple of months we will remove the `approve_unless_trust_level`
setting entirely.

Internal ref: /t/115696

app/serializers/reviewable_score_serializer.rb

@@ -4,6 +4,7 @@ class ReviewableScoreSerializer < ApplicationSerializer
   REASONS_AND_SETTINGS = {
     post_count: "approve_post_count",
     trust_level: "approve_unless_trust_level",
+    group: "approve_unless_allowed_groups",
     new_topics_unless_trust_level: "approve_new_topics_unless_trust_level",
     fast_typer: "min_first_post_typing_time",
     auto_silence_regex: "auto_silence_first_post_regex",

config/locales/server.en.yml

@@ -2319,6 +2319,7 @@ en:
 
     approve_post_count: "The amount of posts from a new or basic user that must be approved"
     approve_unless_trust_level: "Posts for users below this trust level must be approved"
+    approve_unless_allowed_groups: "Posts for users not in these groups must be approved"
     approve_new_topics_unless_trust_level: "New topics for users below this trust level must be approved"
     approve_unless_staged: "New topics and posts for staged users must be approved"
     notify_about_queued_posts_after: "If there are posts that have been waiting to be reviewed for more than this many hours, send a notification to all moderators. Set to 0 to disable these notifications."
@@ -2543,6 +2544,7 @@ en:
       anonymous_posting_allowed_groups: "anonymous_posting_min_trust_level"
       here_mention_allowed_groups: "min_trust_level_for_here_mention"
       shared_drafts_allowed_groups: "shared_drafts_min_trust_level"
+      approve_unless_allowed_groups: "approve_unless_trust_level"
 
     placeholder:
       discourse_connect_provider_secrets:
@@ -5234,6 +5236,7 @@ en:
     reasons:
       post_count: "The first few posts from every user must be approved by staff. See %{link}."
       trust_level: "Users at low trust levels must have replies approved by staff. See %{link}."
+      group: "Users not in the specified groups must have replies approved by staff. See %{link}."
       new_topics_unless_trust_level: "Users at low trust levels must have topics approved by staff. See %{link}."
       fast_typer: "New user typed their first post suspiciously fast, suspected bot or spammer behavior. See %{link}."
       auto_silence_regex: "New user whose first post matches the %{link} setting."

config/site_settings.yml

@@ -1065,6 +1065,13 @@ posting:
   approve_unless_trust_level:
     default: 0
     enum: "TrustLevelSetting"
+    hidden: true
+  approve_unless_allowed_groups:
+    default: 10
+    type: group_list
+    allow_any: false
+    refresh: true
+    validator: "AtLeastOneGroupValidator"
   approve_new_topics_unless_trust_level:
     default: 0
     enum: "TrustLevelSetting"

db/migrate/20231117182638_fill_approve_unless_allowed_groups_based_on_deprecated_settings.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+class FillApproveUnlessAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0]
+  def up
+    approve_unless_trust_level_raw =
+      DB.query_single(
+        "SELECT value FROM site_settings WHERE name = 'approve_unless_trust_level' LIMIT 1",
+      ).first
+
+    # Default for old setting is TL0, we only need to do anything if it's been changed in the DB.
+    if approve_unless_trust_level_raw.present?
+      # Matches Group::AUTO_GROUPS to the trust levels.
+      approve_unless_allowed_groups = "1#{approve_unless_trust_level_raw}"
+
+      # Data_type 20 is group_list
+      DB.exec(
+        "INSERT INTO site_settings(name, value, data_type, created_at, updated_at)
+        VALUES('approve_unless_allowed_groups', :setting, '20', NOW(), NOW())",
+        setting: approve_unless_allowed_groups,
+      )
+    end
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/new_post_manager.rb

@@ -92,7 +92,9 @@ class NewPostManager
       return :post_count
     end
 
-    return :trust_level if user.trust_level < SiteSetting.approve_unless_trust_level.to_i
+    if user.groups.any? && !user.in_any_groups?(SiteSetting.approve_unless_allowed_groups_map)
+      return :group
+    end
 
     if (
          manager.args[:title].present? &&
@@ -200,8 +202,10 @@ class NewPostManager
   end
 
   def self.queue_enabled?
-    SiteSetting.approve_post_count > 0 || SiteSetting.approve_unless_trust_level.to_i > 0 ||
+    SiteSetting.approve_post_count > 0 ||
-      SiteSetting.approve_new_topics_unless_trust_level.to_i > 0 ||
+      !(
+        SiteSetting.approve_unless_allowed_groups_map.include?(Group::AUTO_GROUPS[:trust_level_0])
+      ) || SiteSetting.approve_new_topics_unless_trust_level.to_i > 0 ||
       SiteSetting.approve_unless_staged || WordWatcher.words_for_action_exist?(:require_approval) ||
       handlers.size > 1
   end

lib/site_settings/deprecated_settings.rb

@@ -11,6 +11,7 @@ module SiteSettings::DeprecatedSettings
     ["anonymous_posting_min_trust_level", "anonymous_posting_allowed_groups", false, "3.3"],
     ["shared_drafts_min_trust_level", "shared_drafts_allowed_groups", false, "3.3"],
     ["min_trust_level_for_here_mention", "here_mention_allowed_groups", false, "3.3"],
+    ["approve_unless_trust_level", "approve_unless_allowed_groups", false, "3.3"],
   ]
 
   def setup_deprecated_methods

spec/integration/watched_words_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 RSpec.describe WatchedWord do
-  fab!(:tl2_user) { Fabricate(:user, trust_level: TrustLevel[2]) }
+  fab!(:tl2_user) { Fabricate(:user, trust_level: TrustLevel[2], refresh_auto_groups: true) }
   fab!(:admin)
   fab!(:moderator)
 

spec/lib/email/processor_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe Email::Processor do
   context "when reply via email is too short" do
     let(:mail) { file_from_fixtures("chinese_reply.eml", "emails").read }
     fab!(:post)
-    fab!(:user) { Fabricate(:user, email: "discourse@bar.com") }
+    fab!(:user) { Fabricate(:user, email: "discourse@bar.com", refresh_auto_groups: true) }
 
     fab!(:post_reply_key) do
       Fabricate(

spec/lib/email/receiver_spec.rb

@@ -832,7 +832,7 @@ RSpec.describe Email::Receiver do
     end
 
     it "accepts emails with wrong reply key if the system knows about the forwarded email" do
-      Fabricate(:user, email: "bob@bar.com")
+      Fabricate(:user, email: "bob@bar.com", refresh_auto_groups: true)
       Fabricate(
         :incoming_email,
         raw: <<~RAW,
@@ -1555,7 +1555,12 @@ RSpec.describe Email::Receiver do
       DiscourseEvent.on(:topic_created, &handler)
 
       user =
-        Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust)
+        Fabricate(
+          :user,
+          email: "existing@bar.com",
+          trust_level: SiteSetting.email_in_min_trust,
+          refresh_auto_groups: true,
+        )
       group = Fabricate(:group)
 
       group.add(user)
@@ -1642,10 +1647,10 @@ RSpec.describe Email::Receiver do
     end
 
     it "works when approving is enabled" do
-      SiteSetting.approve_unless_trust_level = 4
+      SiteSetting.approve_unless_allowed_groups = Group::AUTO_GROUPS[:trust_level_4]
 
-      Fabricate(:user, email: "tl3@bar.com", trust_level: TrustLevel[3])
+      Fabricate(:user, email: "tl3@bar.com", trust_level: TrustLevel[3], refresh_auto_groups: true)
-      Fabricate(:user, email: "tl4@bar.com", trust_level: TrustLevel[4])
+      Fabricate(:user, email: "tl4@bar.com", trust_level: TrustLevel[4], refresh_auto_groups: true)
 
       category.set_permissions(Group[:trust_level_4] => :full)
       category.save!

spec/lib/new_post_manager_spec.rb

@@ -3,7 +3,7 @@
 require "new_post_manager"
 
 RSpec.describe NewPostManager do
-  fab!(:user)
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
   fab!(:topic)
 
   describe "default action" do
@@ -111,7 +111,7 @@ RSpec.describe NewPostManager do
     context "with the settings zeroed out" do
       before do
         SiteSetting.approve_post_count = 0
-        SiteSetting.approve_unless_trust_level = 0
+        SiteSetting.approve_unless_allowed_groups = Group::AUTO_GROUPS[:trust_level_0]
       end
 
       it "doesn't return a result action" do
@@ -203,25 +203,25 @@ RSpec.describe NewPostManager do
     end
 
     context "with a high trust level setting" do
-      before { SiteSetting.approve_unless_trust_level = 4 }
+      before { SiteSetting.approve_unless_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] }
       it "will return an enqueue result" do
         result = NewPostManager.default_handler(manager)
         expect(NewPostManager.queue_enabled?).to eq(true)
         expect(result.action).to eq(:enqueued)
-        expect(result.reason).to eq(:trust_level)
+        expect(result.reason).to eq(:group)
       end
     end
 
     context "with uncategorized disabled, and approval" do
       before do
         SiteSetting.allow_uncategorized_topics = false
-        SiteSetting.approve_unless_trust_level = 4
+        SiteSetting.approve_unless_allowed_groups = Group::AUTO_GROUPS[:trust_level_4]
       end
 
       it "will return an enqueue result" do
         npm =
           NewPostManager.new(
-            Fabricate(:user),
+            user,
             title: "this is a new topic title",
             raw: "this is the raw content",
             category: Fabricate(:category).id,
@@ -328,7 +328,10 @@ RSpec.describe NewPostManager do
       NewPostManager.new(user, raw: "this is new topic content", title: "new topic title")
     end
     context "with a high trust level setting for new topics" do
-      before { SiteSetting.approve_new_topics_unless_trust_level = 4 }
+      before do
+        SiteSetting.approve_new_topics_unless_trust_level = 4
+        Group.refresh_automatic_groups!
+      end
       it "will return an enqueue result" do
         result = NewPostManager.default_handler(manager)
         expect(NewPostManager.queue_enabled?).to eq(true)
@@ -452,6 +455,7 @@ RSpec.describe NewPostManager do
     end
 
     it "if nothing returns a result it creates a post" do
+      Group.refresh_automatic_groups!
       manager = NewPostManager.new(user, raw: "this is a new post", topic_id: topic.id)
 
       result = manager.perform
@@ -464,14 +468,10 @@ RSpec.describe NewPostManager do
   end
 
   describe "user needs approval?" do
-    let :user do
+    fab!(:user) { Fabricate(:user, trust_level: TrustLevel[0], refresh_auto_groups: true) }
-      user = Fabricate.build(:user, trust_level: 0)
-      user_stat = UserStat.new(post_count: 0)
-      user.user_stat = user_stat
-      user
-    end
 
     it "handles post_needs_approval? correctly" do
+      user.user_stat = UserStat.new(post_count: 0, new_since: DateTime.now)
       u = user
       default = NewPostManager.new(u, {})
       expect(NewPostManager.post_needs_approval?(default)).to eq(:skip)
@@ -500,6 +500,7 @@ RSpec.describe NewPostManager do
         SiteSetting.tagging_enabled = true
         category.require_topic_approval = true
         category.save
+        Group.refresh_automatic_groups!
       end
 
       it "enqueues new topics" do
@@ -627,6 +628,7 @@ RSpec.describe NewPostManager do
       before do
         category.require_reply_approval = true
         category.save
+        Group.refresh_automatic_groups!
       end
 
       it "enqueues new posts" do

spec/lib/topic_view_spec.rb

@@ -1004,6 +1004,7 @@ RSpec.describe TopicView do
 
   describe "#reviewable_counts" do
     it "exclude posts queued because the category needs approval" do
+      Group.refresh_automatic_groups!
       category =
         Fabricate.create(
           :category,

spec/requests/posts_controller_spec.rb

@@ -827,6 +827,7 @@ RSpec.describe PostsController do
     before do
       SiteSetting.min_first_post_typing_time = 0
       SiteSetting.whispers_allowed_groups = "#{Group::AUTO_GROUPS[:staff]}"
+      Group.refresh_automatic_groups!
     end
 
     context "with api" do
@@ -866,7 +867,7 @@ RSpec.describe PostsController do
       end
 
       it "returns a valid JSON response when the post is enqueued" do
-        SiteSetting.approve_unless_trust_level = 4
+        SiteSetting.approve_unless_allowed_groups = Group::AUTO_GROUPS[:trust_level_4]
 
         master_key = Fabricate(:api_key).key
 
@@ -1652,6 +1653,7 @@ RSpec.describe PostsController do
         it "it triggers flag_linked_posts_as_spam when the post creator returns spam" do
           SiteSetting.newuser_spam_host_threshold = 1
           sign_in(Fabricate(:user, trust_level: 0))
+          Group.refresh_automatic_groups!
 
           post "/posts.json",
                params: {

spec/system/composer/category_templates_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 describe "Composer Form Templates", type: :system do
-  fab!(:user)
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
   fab!(:form_template_1) do
     Fabricate(
       :form_template,

spec/system/composer/review_media_unless_trust_level_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 describe "Composer using review_media", type: :system do
-  fab!(:user)
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
   fab!(:topic) { Fabricate(:topic, category: Category.find(SiteSetting.uncategorized_category_id)) }
   fab!(:post) { Fabricate(:post, topic: topic) }
   let(:topic_page) { PageObjects::Pages::Topic.new }

spec/system/hashtag_autocomplete_spec.rb

@@ -100,6 +100,7 @@ describe "Using #hashtag autocompletion to search for and lookup categories and
   end
 
   it "cooks the hashtags for tag and category correctly serverside when the post is saved to the database" do
+    Group.refresh_automatic_groups!
     topic_page.visit_topic_and_open_composer(topic)
 
     expect(topic_page).to have_expanded_composer