diff --git a/app/serializers/reviewable_score_serializer.rb b/app/serializers/reviewable_score_serializer.rb index bbf26b62ce8..8a29b3dfda8 100644 --- a/app/serializers/reviewable_score_serializer.rb +++ b/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", diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index deb1e17f260..5f46a9bb6f4 100644 --- a/config/locales/server.en.yml +++ b/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." diff --git a/config/site_settings.yml b/config/site_settings.yml index dc311d065c8..cfba458e508 100644 --- a/config/site_settings.yml +++ b/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" diff --git a/db/migrate/20231117182638_fill_approve_unless_allowed_groups_based_on_deprecated_settings.rb b/db/migrate/20231117182638_fill_approve_unless_allowed_groups_based_on_deprecated_settings.rb new file mode 100644 index 00000000000..371773c5ebd --- /dev/null +++ b/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 diff --git a/lib/new_post_manager.rb b/lib/new_post_manager.rb index 970cb943b92..24a8e138fc5 100644 --- a/lib/new_post_manager.rb +++ b/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_new_topics_unless_trust_level.to_i > 0 || + 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 end diff --git a/lib/site_settings/deprecated_settings.rb b/lib/site_settings/deprecated_settings.rb index 43973637bb1..730963252d8 100644 --- a/lib/site_settings/deprecated_settings.rb +++ b/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 diff --git a/spec/integration/watched_words_spec.rb b/spec/integration/watched_words_spec.rb index ea1326c2930..2e80312d0fe 100644 --- a/spec/integration/watched_words_spec.rb +++ b/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) diff --git a/spec/lib/email/processor_spec.rb b/spec/lib/email/processor_spec.rb index 392e0644624..cfd5d659767 100644 --- a/spec/lib/email/processor_spec.rb +++ b/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( diff --git a/spec/lib/email/receiver_spec.rb b/spec/lib/email/receiver_spec.rb index 42f3a6b5b0f..5fd60d37e16 100644 --- a/spec/lib/email/receiver_spec.rb +++ b/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: "tl4@bar.com", trust_level: TrustLevel[4]) + Fabricate(:user, email: "tl3@bar.com", trust_level: TrustLevel[3], refresh_auto_groups: true) + Fabricate(:user, email: "tl4@bar.com", trust_level: TrustLevel[4], refresh_auto_groups: true) category.set_permissions(Group[:trust_level_4] => :full) category.save! diff --git a/spec/lib/new_post_manager_spec.rb b/spec/lib/new_post_manager_spec.rb index 00eb5633199..d01479f6c04 100644 --- a/spec/lib/new_post_manager_spec.rb +++ b/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 - user = Fabricate.build(:user, trust_level: 0) - user_stat = UserStat.new(post_count: 0) - user.user_stat = user_stat - user - end + fab!(:user) { Fabricate(:user, trust_level: TrustLevel[0], refresh_auto_groups: true) } 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 diff --git a/spec/lib/topic_view_spec.rb b/spec/lib/topic_view_spec.rb index b355f53d2e0..dce4faf6026 100644 --- a/spec/lib/topic_view_spec.rb +++ b/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, diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index b861f2c1f78..f4fa93ae957 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/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: { diff --git a/spec/system/composer/category_templates_spec.rb b/spec/system/composer/category_templates_spec.rb index 4f499fad41a..c571c40f281 100644 --- a/spec/system/composer/category_templates_spec.rb +++ b/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, diff --git a/spec/system/composer/review_media_unless_trust_level_spec.rb b/spec/system/composer/review_media_unless_trust_level_spec.rb index 9c9c4c96eb1..01005d35b71 100644 --- a/spec/system/composer/review_media_unless_trust_level_spec.rb +++ b/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 } diff --git a/spec/system/hashtag_autocomplete_spec.rb b/spec/system/hashtag_autocomplete_spec.rb index b5b83fef430..750e4d279a8 100644 --- a/spec/system/hashtag_autocomplete_spec.rb +++ b/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