From 8a45f84277e6e13bc48b8f4e40350a9cbd5cd2ee Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Wed, 22 Nov 2023 10:44:59 -0700 Subject: [PATCH] DEV: Convert approve_new_topics_unless_trust_level to groups (#24504) * DEV: Convert approve_new_topics_unless_trust_level to groups This change converts the `approve_new_topics_unless_trust_level` site setting to `approve_new_topics_unless_allowed_groups`. See: https://meta.discourse.org/t/283408 - Hides the old setting - Adds the new site setting - Add a deprecation warning - Updates to use the new setting - Adds a migration to fill in the new setting if the old setting was changed - Adds an entry to the site_setting.keywords section - Updates tests to account for the new change After a couple of months we will remove the `approve_new_topics_unless_trust_level` setting entirely. Internal ref: /t/115696 * add missing translation * Add keyword entry * Add migration --- .../reviewable_score_serializer.rb | 1 + config/locales/server.en.yml | 2 ++ config/site_settings.yml | 7 +++++ ...wed_groups_based_on_deprecated_settings.rb | 27 +++++++++++++++++++ lib/new_post_manager.rb | 16 ++++++----- lib/site_settings/deprecated_settings.rb | 6 +++++ spec/lib/new_post_manager_spec.rb | 8 +++--- 7 files changed, 58 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20231122152552_fill_new_topics_unless_allowed_groups_based_on_deprecated_settings.rb diff --git a/app/serializers/reviewable_score_serializer.rb b/app/serializers/reviewable_score_serializer.rb index 8a29b3dfda8..1d406e037c2 100644 --- a/app/serializers/reviewable_score_serializer.rb +++ b/app/serializers/reviewable_score_serializer.rb @@ -6,6 +6,7 @@ class ReviewableScoreSerializer < ApplicationSerializer trust_level: "approve_unless_trust_level", group: "approve_unless_allowed_groups", new_topics_unless_trust_level: "approve_new_topics_unless_trust_level", + new_topics_unless_allowed_groups: "approve_new_topics_unless_allowed_groups", fast_typer: "min_first_post_typing_time", auto_silence_regex: "auto_silence_first_post_regex", staged: "approve_unless_staged", diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 5f46a9bb6f4..2873122775b 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2321,6 +2321,7 @@ en: 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_new_topics_unless_allowed_groups: "New topics for users not in these groups 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." reviewable_revision_reasons: "List of reasons that can be selected when rejecting a reviewable queued post with a revision. Other is always available as well, which allows for a custom reason to be entered." @@ -2545,6 +2546,7 @@ en: 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" + approve_new_topics_unless_allowed_groups: "approve_new_topics_unless_trust_level" placeholder: discourse_connect_provider_secrets: diff --git a/config/site_settings.yml b/config/site_settings.yml index cfba458e508..977c365a564 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1075,6 +1075,13 @@ posting: approve_new_topics_unless_trust_level: default: 0 enum: "TrustLevelSetting" + hidden: true + approve_new_topics_unless_allowed_groups: + default: 10 + type: group_list + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" approve_suspect_users: default: true approve_unless_staged: diff --git a/db/migrate/20231122152552_fill_new_topics_unless_allowed_groups_based_on_deprecated_settings.rb b/db/migrate/20231122152552_fill_new_topics_unless_allowed_groups_based_on_deprecated_settings.rb new file mode 100644 index 00000000000..e6062856e86 --- /dev/null +++ b/db/migrate/20231122152552_fill_new_topics_unless_allowed_groups_based_on_deprecated_settings.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class FillNewTopicsUnlessAllowedGroupsBasedOnDeprecatedSettings < ActiveRecord::Migration[7.0] + def up + approve_new_topics_unless_trust_level_raw = + DB.query_single( + "SELECT value FROM site_settings WHERE name = 'approve_new_topics_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_new_topics_unless_trust_level_raw.present? + # Matches Group::AUTO_GROUPS to the trust levels. + approve_new_topics_unless_allowed_groups = "1#{approve_new_topics_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_new_topics_unless_allowed_groups', :setting, '20', NOW(), NOW())", + setting: approve_new_topics_unless_allowed_groups, + ) + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/new_post_manager.rb b/lib/new_post_manager.rb index 24a8e138fc5..d6716057f48 100644 --- a/lib/new_post_manager.rb +++ b/lib/new_post_manager.rb @@ -97,10 +97,10 @@ class NewPostManager end if ( - manager.args[:title].present? && - user.trust_level < SiteSetting.approve_new_topics_unless_trust_level.to_i + manager.args[:title].present? && user.groups.any? && + !user.in_any_groups?(SiteSetting.approve_new_topics_unless_allowed_groups_map) ) - return :new_topics_unless_trust_level + return :new_topics_unless_allowed_groups end if WordWatcher.new("#{manager.args[:title]} #{manager.args[:raw]}").requires_approval? @@ -205,9 +205,13 @@ class NewPostManager SiteSetting.approve_post_count > 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 + ) || + !( + SiteSetting.approve_new_topics_unless_allowed_groups_map.include?( + Group::AUTO_GROUPS[:trust_level_0], + ) + ) || SiteSetting.approve_unless_staged || + WordWatcher.words_for_action_exist?(:require_approval) || handlers.size > 1 end def initialize(user, args) diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index 730963252d8..72761d4c8cf 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/lib/site_settings/deprecated_settings.rb @@ -12,6 +12,12 @@ module SiteSettings::DeprecatedSettings ["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"], + [ + "approve_new_topics_unless_trust_level", + "approve_new_topics_unless_allowed_groups", + false, + "3.3", + ], ] def setup_deprecated_methods diff --git a/spec/lib/new_post_manager_spec.rb b/spec/lib/new_post_manager_spec.rb index d01479f6c04..429bcc3e891 100644 --- a/spec/lib/new_post_manager_spec.rb +++ b/spec/lib/new_post_manager_spec.rb @@ -249,7 +249,9 @@ RSpec.describe NewPostManager do end context "with a high trust level setting for new topics but post responds to existing topic" do - before { SiteSetting.approve_new_topics_unless_trust_level = 4 } + before do + SiteSetting.approve_new_topics_unless_allowed_groups = Group::AUTO_GROUPS[:trust_level_4] + end it "doesn't return a result action" do result = NewPostManager.default_handler(manager) expect(result).to eq(nil) @@ -329,14 +331,14 @@ RSpec.describe NewPostManager do end context "with a high trust level setting for new topics" do before do - SiteSetting.approve_new_topics_unless_trust_level = 4 + SiteSetting.approve_new_topics_unless_allowed_groups = Group::AUTO_GROUPS[: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) expect(result.action).to eq(:enqueued) - expect(result.reason).to eq(:new_topics_unless_trust_level) + expect(result.reason).to eq(:new_topics_unless_allowed_groups) end end end