From 7577231ba220f68c590b4344234f6f0bcc9e9033 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Tue, 3 Sep 2024 09:25:45 +1000 Subject: [PATCH] DEV: the ability to define setting areas (#28570) A new setting attribute is used to define the areas (separated by `|`). In addition, endpoint `/admin/config/site_settings.json` accepts new `filter_area` data. --- .../admin-config-areas/flags-settings.gjs | 23 ++++++++---- .../admin/config/site_settings_controller.rb | 5 ++- app/models/site_setting.rb | 2 + config/site_settings.yml | 37 ++++++++++++++----- lib/site_setting_extension.rb | 23 +++++++++++- plugins/chat/config/settings.yml | 2 + spec/lib/site_setting_extension_spec.rb | 22 +++++++++++ .../config/site_settings_controller_spec.rb | 24 ++++++++++++ spec/system/admin_flags_spec.rb | 8 ++-- 9 files changed, 123 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/admin/addon/components/admin-config-areas/flags-settings.gjs b/app/assets/javascripts/admin/addon/components/admin-config-areas/flags-settings.gjs index 5d465480370..c4937c76c78 100644 --- a/app/assets/javascripts/admin/addon/components/admin-config-areas/flags-settings.gjs +++ b/app/assets/javascripts/admin/addon/components/admin-config-areas/flags-settings.gjs @@ -3,7 +3,7 @@ import { tracked } from "@glimmer/tracking"; import didInsert from "@ember/render-modifiers/modifiers/did-insert"; import { service } from "@ember/service"; import DBreadcrumbsItem from "discourse/components/d-breadcrumbs-item"; -import SiteSettingFilter from "discourse/lib/site-setting-filter"; +import { ajax } from "discourse/lib/ajax"; import i18n from "discourse-common/helpers/i18n"; import { bind } from "discourse-common/utils/decorators"; import AdminFilteredSiteSettings from "admin/components/admin-filtered-site-settings"; @@ -15,13 +15,20 @@ export default class AdminConfigAreasFlagsSettings extends Component { @bind loadSettings() { - SiteSetting.findAll({ - categories: ["spam", "rate_limits", "chat"], - }).then((settings) => { - this.settings = new SiteSettingFilter(settings).performSearch( - "flags", - {} - ); + ajax("/admin/config/site_settings.json", { + data: { + filter_area: "flags", + }, + }).then((result) => { + this.settings = [ + { + name: "All", + nameKey: "all_results", + siteSettings: result.site_settings.map((setting) => + SiteSetting.create(setting) + ), + }, + ]; }); } diff --git a/app/controllers/admin/config/site_settings_controller.rb b/app/controllers/admin/config/site_settings_controller.rb index cebfc0f17c4..4dfb9782e1b 100644 --- a/app/controllers/admin/config/site_settings_controller.rb +++ b/app/controllers/admin/config/site_settings_controller.rb @@ -12,12 +12,15 @@ class Admin::Config::SiteSettingsController < Admin::AdminController # UI itself uses the Admin::SiteSettingsController#index endpoint, # which also supports a `category` and `plugin` filter. def index - params.require(:filter_names) + if params[:filter_names].blank? && SiteSetting::VALID_AREAS.exclude?(params[:filter_area]) + raise Discourse::InvalidParameters + end render_json_dump( site_settings: SiteSetting.all_settings( filter_names: params[:filter_names], + filter_area: params[:filter_area], include_locale_setting: false, include_hidden: true, filter_allowed_hidden: ADMIN_CONFIG_AREA_ALLOWLISTED_HIDDEN_SETTINGS, diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index f92bb55fcb4..2d5e6bba294 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class SiteSetting < ActiveRecord::Base + VALID_AREAS = %w[flags] + extend GlobalPath extend SiteSettingExtension diff --git a/config/site_settings.yml b/config/site_settings.yml index 03c5cadcfae..a76186e8273 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2153,9 +2153,14 @@ spam: type: enum enum: "ReviewableSensitivitySetting" default: 3 - num_users_to_silence_new_user: 3 + area: "flags" + num_users_to_silence_new_user: + default: 3 + area: "flags" notify_mods_when_user_silenced: false - flag_sockpuppets: false + flag_sockpuppets: + default: false + area: "flags" newuser_spam_host_threshold: 3 allowed_spam_host_domains: default: "" @@ -2168,7 +2173,9 @@ spam: min_ban_entries_for_roll_up: 5 max_age_unmatched_emails: 365 max_age_unmatched_ips: 365 - num_flaggers_to_close_topic: 5 + num_flaggers_to_close_topic: + default: 5 + area: "flags" auto_close_topic_sensitivity: type: enum enum: "ReviewableSensitivitySetting" @@ -2176,14 +2183,18 @@ spam: num_hours_to_close_topic: default: 4 min: 1 - auto_respond_to_flag_actions: true + auto_respond_to_flag_actions: + default: true + area: "flags" min_first_post_typing_time: 3000 auto_silence_fast_typers_on_first_post: true auto_silence_fast_typers_max_trust_level: default: 0 enum: "TrustLevelSetting" auto_silence_first_post_regex: "" - high_trust_flaggers_auto_hide_posts: true + high_trust_flaggers_auto_hide_posts: + default: true + area: "flags" cooldown_hours_until_reflag: default: 24 min: 0 @@ -2238,7 +2249,9 @@ rate_limits: max_personal_messages_per_day: 20 max_likes_per_day: 50 max_bookmarks_per_day: 20 - max_flags_per_day: 20 + max_flags_per_day: + default: 20 + area: "flags" max_edits_per_day: 30 max_invites_per_day: 10 max_topic_invitations_per_day: 30 @@ -2251,9 +2264,15 @@ rate_limits: tl2_additional_edits_per_day_multiplier: 1.5 tl3_additional_edits_per_day_multiplier: 2 tl4_additional_edits_per_day_multiplier: 3 - tl2_additional_flags_per_day_multiplier: 1.5 - tl3_additional_flags_per_day_multiplier: 2 - tl4_additional_flags_per_day_multiplier: 3 + tl2_additional_flags_per_day_multiplier: + default: 1.5 + area: "flags" + tl3_additional_flags_per_day_multiplier: + default: 2 + area: "flags" + tl4_additional_flags_per_day_multiplier: + default: 3 + area: "flags" alert_admins_if_errors_per_minute: client: true default: 0 diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 4bfe937d120..5971d9cac4d 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -87,6 +87,10 @@ module SiteSettingExtension @categories ||= {} end + def areas + @areas ||= {} + end + def mandatory_values @mandatory_values ||= {} end @@ -194,7 +198,8 @@ module SiteSettingExtension filter_categories: nil, filter_plugin: nil, filter_names: nil, - filter_allowed_hidden: nil + filter_allowed_hidden: nil, + filter_area: nil ) locale_setting_hash = { setting: "default_locale", @@ -231,6 +236,13 @@ module SiteSettingExtension true end end + .select do |setting_name, _| + if filter_area + Array.wrap(areas[setting_name]).include?(filter_area) + else + true + end + end .select do |setting_name, _| if filter_plugin plugins[setting_name] == filter_plugin @@ -681,6 +693,15 @@ module SiteSettingExtension categories[name] = opts[:category] || :uncategorized + if opts[:area] + split_areas = opts[:area].split("|") + if split_areas.any? { |area| !SiteSetting::VALID_AREAS.include?(area) } + raise Discourse::InvalidParameters.new( + "Area is incorrect. Valid areas: #{SiteSetting::VALID_AREAS.join(", ")}", + ) + end + areas[name] = split_areas + end hidden_settings_provider.add_hidden(name) if opts[:hidden] if GlobalSetting.respond_to?(name) diff --git a/plugins/chat/config/settings.yml b/plugins/chat/config/settings.yml index bd0ccff7ab3..6fe491e7fc4 100644 --- a/plugins/chat/config/settings.yml +++ b/plugins/chat/config/settings.yml @@ -53,6 +53,7 @@ chat: chat_auto_silence_from_flags_duration: default: 60 min: 0 + area: "flags" chat_allow_archiving_channels: default: false client: true @@ -106,6 +107,7 @@ chat: type: group_list allow_any: false refresh: true + area: "flags" max_mentions_per_chat_message: client: true type: integer diff --git a/spec/lib/site_setting_extension_spec.rb b/spec/lib/site_setting_extension_spec.rb index d511ba1f18e..ccde4c2a853 100644 --- a/spec/lib/site_setting_extension_spec.rb +++ b/spec/lib/site_setting_extension_spec.rb @@ -490,6 +490,28 @@ RSpec.describe SiteSettingExtension do end end + describe "a setting with an area" do + before do + settings.setting(:test_setting, 88, area: "flags") + settings.setting(:test_setting2, 89, area: "flags") + settings.setting(:test_setting4, 90) + settings.refresh! + end + + it "should allow to filter by area" do + expect(settings.all_settings(filter_area: "flags").map { |s| s[:setting].to_sym }).to eq( + %i[default_locale test_setting test_setting2], + ) + end + + it "raised an error when area is invalid" do + expect { + settings.setting(:test_setting, 89, area: "invalid") + settings.refresh! + }.to raise_error(Discourse::InvalidParameters) + end + end + describe "setting with a validator" do before do settings.setting(:validated_setting, "info@example.com", type: "email") diff --git a/spec/requests/admin/config/site_settings_controller_spec.rb b/spec/requests/admin/config/site_settings_controller_spec.rb index 0a7d0fe019c..602f082ee2a 100644 --- a/spec/requests/admin/config/site_settings_controller_spec.rb +++ b/spec/requests/admin/config/site_settings_controller_spec.rb @@ -29,6 +29,11 @@ RSpec.describe Admin::SiteSettingsController do expect(response.status).to eq(400) end + it "returns 400 when no filter_area is invalid" do + get "/admin/config/site_settings.json", params: { filter_area: "invalid area" } + expect(response.status).to eq(400) + end + it "includes only certain allowed hidden settings" do get "/admin/config/site_settings.json", params: { @@ -60,6 +65,25 @@ RSpec.describe Admin::SiteSettingsController do %w[site_description enforce_second_factor], ) end + + it "returns site settings by area" do + get "/admin/config/site_settings.json", params: { filter_area: "flags" } + expect(response.status).to eq(200) + expect(response.parsed_body["site_settings"].map { |s| s["setting"] }).to match_array( + %w[ + silence_new_user_sensitivity + num_users_to_silence_new_user + flag_sockpuppets + num_flaggers_to_close_topic + auto_respond_to_flag_actions + high_trust_flaggers_auto_hide_posts + max_flags_per_day + tl2_additional_flags_per_day_multiplier + tl3_additional_flags_per_day_multiplier + tl4_additional_flags_per_day_multiplier + ], + ) + end end end end diff --git a/spec/system/admin_flags_spec.rb b/spec/system/admin_flags_spec.rb index c858a966141..d7aec68d877 100644 --- a/spec/system/admin_flags_spec.rb +++ b/spec/system/admin_flags_spec.rb @@ -160,14 +160,14 @@ describe "Admin Flags Page", type: :system do [ "silence new user sensitivity", "num users to silence new user", + "flag sockpuppets", + "num flaggers to close topic", + "auto respond to flag actions", + "high trust flaggers auto hide posts", "max flags per day", "tl2 additional flags per day multiplier", "tl3 additional flags per day multiplier", "tl4 additional flags per day multiplier", - "flag sockpuppets", - "auto respond to flag actions", - "num flaggers to close topic", - "high trust flaggers auto hide posts", ], ) end