DEV: Allow fetching specific site settings and introduce a service for updating site settings (#27481)

This commit adds ability to fetch a subset of site settings from the `/admin/site_settings` endpoint so that it can be used in all places where the client app needs access to a subset of the site settings.

Additionally, this commit also introduces a new service class called `UpdateSiteSetting` that encapsulates all the logic that surrounds updating a site setting so that it can be used to update site setting(s) anywhere in the backend. This service comes in handy with, for example, the controller for the flags admin config area which may need to update some site settings related to flags.

Internal topic: t/130713.
This commit is contained in:
Osama Sayegh 2024-06-14 13:07:27 +03:00 committed by GitHub
parent 86e768f9e9
commit 4aea12fdcb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 351 additions and 180 deletions

View File

@ -7,12 +7,15 @@ class Admin::SiteSettingsController < Admin::AdminController
def index
params.permit(:categories, :plugin)
params.permit(:filter_names, [])
render_json_dump(
site_settings:
SiteSetting.all_settings(
filter_categories: params[:categories],
filter_plugin: params[:plugin],
filter_names: params[:filter_names],
include_locale_setting: params[:filter_names].blank?,
),
)
end
@ -20,8 +23,8 @@ class Admin::SiteSettingsController < Admin::AdminController
def update
params.require(:id)
id = params[:id]
update_existing_users = params[:update_existing_user].present?
value = params[id]
value.strip! if value.is_a?(String)
new_setting_name =
SiteSettings::DeprecatedSettings::SETTINGS.find do |old_name, new_name, override, _|
@ -37,127 +40,25 @@ class Admin::SiteSettingsController < Admin::AdminController
id = new_setting_name if new_setting_name
raise_access_hidden_setting(id)
case SiteSetting.type_supervisor.get_type(id)
when :integer
value = value.tr("^-0-9", "")
when :file_size_restriction
value = value.tr("^0-9", "").to_i
when :uploaded_image_list
value = value.blank? ? "" : Upload.get_from_urls(value.split("|")).to_a
end
value = Upload.get_from_url(value) || "" if SiteSetting.type_supervisor.get_type(id) == :upload
update_existing_users = params[:update_existing_user].present?
previous_value = value_or_default(SiteSetting.get(id)) if update_existing_users
SiteSetting.set_and_log(id, value, current_user)
with_service(UpdateSiteSetting, setting_name: id, new_value: value) do
on_success do
value = result.new_value
SiteSettingUpdateExistingUsers.call(id, value, previous_value) if update_existing_users
if update_existing_users
new_value = value_or_default(value)
render body: nil
end
if (user_option = user_options[id.to_sym]).present?
if user_option == "text_size_key"
previous_value = UserOption.text_sizes[previous_value.to_sym]
new_value = UserOption.text_sizes[new_value.to_sym]
elsif user_option == "title_count_mode_key"
previous_value = UserOption.title_count_modes[previous_value.to_sym]
new_value = UserOption.title_count_modes[new_value.to_sym]
end
on_failed_policy(:setting_is_visible) do
raise Discourse::InvalidParameters, I18n.t("errors.site_settings.site_setting_is_hidden")
end
attrs = { user_option => new_value }
attrs[:email_digests] = (new_value.to_i != 0) if id == "default_email_digest_frequency"
UserOption.human_users.where(user_option => previous_value).update_all(attrs)
elsif id.start_with?("default_categories_")
previous_category_ids = previous_value.split("|")
new_category_ids = new_value.split("|")
notification_level = category_notification_level(id)
categories_to_unwatch = previous_category_ids - new_category_ids
CategoryUser.where(
category_id: categories_to_unwatch,
notification_level: notification_level,
).delete_all
TopicUser
.joins(:topic)
.where(
notification_level: TopicUser.notification_levels[:watching],
notifications_reason_id: TopicUser.notification_reasons[:auto_watch_category],
topics: {
category_id: categories_to_unwatch,
},
)
.update_all(notification_level: TopicUser.notification_levels[:regular])
(new_category_ids - previous_category_ids).each do |category_id|
skip_user_ids = CategoryUser.where(category_id: category_id).pluck(:user_id)
User
.real
.where(staged: false)
.where.not(id: skip_user_ids)
.select(:id)
.find_in_batches do |users|
category_users = []
users.each do |user|
category_users << {
category_id: category_id,
user_id: user.id,
notification_level: notification_level,
}
end
CategoryUser.insert_all!(category_users)
end
end
elsif id.start_with?("default_tags_")
previous_tag_ids = Tag.where(name: previous_value.split("|")).pluck(:id)
new_tag_ids = Tag.where(name: new_value.split("|")).pluck(:id)
now = Time.zone.now
notification_level = tag_notification_level(id)
TagUser.where(
tag_id: (previous_tag_ids - new_tag_ids),
notification_level: notification_level,
).delete_all
(new_tag_ids - previous_tag_ids).each do |tag_id|
skip_user_ids = TagUser.where(tag_id: tag_id).pluck(:user_id)
User
.real
.where(staged: false)
.where.not(id: skip_user_ids)
.select(:id)
.find_in_batches do |users|
tag_users = []
users.each do |user|
tag_users << {
tag_id: tag_id,
user_id: user.id,
notification_level: notification_level,
created_at: now,
updated_at: now,
}
end
TagUser.insert_all!(tag_users)
end
end
elsif is_sidebar_default_setting?(id)
Jobs.enqueue(
:backfill_sidebar_site_settings,
setting_name: id,
previous_value: previous_value,
new_value: new_value,
)
on_failed_policy(:setting_is_configurable) do
raise Discourse::InvalidParameters,
I18n.t("errors.site_settings.site_setting_is_unconfigurable")
end
end
render body: nil
end
def user_count
@ -170,7 +71,7 @@ class Admin::SiteSettingsController < Admin::AdminController
previous_value = value_or_default(SiteSetting.public_send(id))
json = {}
if (user_option = user_options[id.to_sym]).present?
if (user_option = SiteSettingUpdateExistingUsers.user_options[id.to_sym]).present?
if user_option == "text_size_key"
previous_value = UserOption.text_sizes[previous_value.to_sym]
elsif user_option == "title_count_mode_key"
@ -182,7 +83,7 @@ class Admin::SiteSettingsController < Admin::AdminController
previous_category_ids = previous_value.split("|")
new_category_ids = new_value.split("|")
notification_level = category_notification_level(id)
notification_level = SiteSettingUpdateExistingUsers.category_notification_level(id)
user_ids =
CategoryUser
@ -210,7 +111,7 @@ class Admin::SiteSettingsController < Admin::AdminController
previous_tag_ids = Tag.where(name: previous_value.split("|")).pluck(:id)
new_tag_ids = Tag.where(name: new_value.split("|")).pluck(:id)
notification_level = tag_notification_level(id)
notification_level = SiteSettingUpdateExistingUsers.tag_notification_level(id)
user_ids =
TagUser
@ -228,7 +129,7 @@ class Admin::SiteSettingsController < Admin::AdminController
.pluck("users.id")
json[:user_count] = user_ids.uniq.count
elsif is_sidebar_default_setting?(id)
elsif SiteSettingUpdateExistingUsers.is_sidebar_default_setting?(id)
json[:user_count] = SidebarSiteSettingsBackfiller.new(
id,
previous_value: previous_value,
@ -241,38 +142,6 @@ class Admin::SiteSettingsController < Admin::AdminController
private
def is_sidebar_default_setting?(setting_name)
%w[default_navigation_menu_categories default_navigation_menu_tags].include?(setting_name.to_s)
end
def user_options
{
default_email_mailing_list_mode: "mailing_list_mode",
default_email_mailing_list_mode_frequency: "mailing_list_mode_frequency",
default_email_level: "email_level",
default_email_messages_level: "email_messages_level",
default_topics_automatic_unpin: "automatically_unpin_topics",
default_email_previous_replies: "email_previous_replies",
default_email_in_reply_to: "email_in_reply_to",
default_other_enable_quoting: "enable_quoting",
default_other_enable_defer: "enable_defer",
default_other_external_links_in_new_tab: "external_links_in_new_tab",
default_other_dynamic_favicon: "dynamic_favicon",
default_other_new_topic_duration_minutes: "new_topic_duration_minutes",
default_other_auto_track_topics_after_msecs: "auto_track_topics_after_msecs",
default_other_notification_level_when_replying: "notification_level_when_replying",
default_other_like_notification_frequency: "like_notification_frequency",
default_other_skip_new_user_tips: "skip_new_user_tips",
default_email_digest_frequency: "digest_after_minutes",
default_include_tl0_in_digests: "include_tl0_in_digests",
default_text_size: "text_size_key",
default_title_count_mode: "title_count_mode_key",
default_hide_profile_and_presence: "hide_profile_and_presence",
default_sidebar_link_to_filtered_list: "sidebar_link_to_filtered_list",
default_sidebar_show_count_of_new_items: "sidebar_show_count_of_new_items",
}
end
def raise_access_hidden_setting(id)
id = id.to_sym
@ -285,34 +154,6 @@ class Admin::SiteSettingsController < Admin::AdminController
end
end
def tag_notification_level(id)
case id
when "default_tags_watching"
NotificationLevels.all[:watching]
when "default_tags_tracking"
NotificationLevels.all[:tracking]
when "default_tags_muted"
NotificationLevels.all[:muted]
when "default_tags_watching_first_post"
NotificationLevels.all[:watching_first_post]
end
end
def category_notification_level(id)
case id
when "default_categories_watching"
NotificationLevels.all[:watching]
when "default_categories_tracking"
NotificationLevels.all[:tracking]
when "default_categories_muted"
NotificationLevels.all[:muted]
when "default_categories_watching_first_post"
NotificationLevels.all[:watching_first_post]
when "default_categories_normal"
NotificationLevels.all[:regular]
end
end
def value_or_default(value)
value.nil? ? "" : value
end

View File

@ -0,0 +1,165 @@
# frozen_string_literal: true
class SiteSettingUpdateExistingUsers
def self.call(id, value, previous_value)
new_value = value.nil? ? "" : value
if (user_option = self.user_options[id.to_sym]).present?
if user_option == "text_size_key"
previous_value = UserOption.text_sizes[previous_value.to_sym]
new_value = UserOption.text_sizes[new_value.to_sym]
elsif user_option == "title_count_mode_key"
previous_value = UserOption.title_count_modes[previous_value.to_sym]
new_value = UserOption.title_count_modes[new_value.to_sym]
end
attrs = { user_option => new_value }
attrs[:email_digests] = (new_value.to_i != 0) if id == "default_email_digest_frequency"
UserOption.human_users.where(user_option => previous_value).update_all(attrs)
elsif id.start_with?("default_categories_")
previous_category_ids = previous_value.split("|")
new_category_ids = new_value.split("|")
notification_level = self.category_notification_level(id)
categories_to_unwatch = previous_category_ids - new_category_ids
CategoryUser.where(
category_id: categories_to_unwatch,
notification_level: notification_level,
).delete_all
TopicUser
.joins(:topic)
.where(
notification_level: TopicUser.notification_levels[:watching],
notifications_reason_id: TopicUser.notification_reasons[:auto_watch_category],
topics: {
category_id: categories_to_unwatch,
},
)
.update_all(notification_level: TopicUser.notification_levels[:regular])
(new_category_ids - previous_category_ids).each do |category_id|
skip_user_ids = CategoryUser.where(category_id: category_id).pluck(:user_id)
User
.real
.where(staged: false)
.where.not(id: skip_user_ids)
.select(:id)
.find_in_batches do |users|
category_users = []
users.each do |user|
category_users << {
category_id: category_id,
user_id: user.id,
notification_level: notification_level,
}
end
CategoryUser.insert_all!(category_users)
end
end
elsif id.start_with?("default_tags_")
previous_tag_ids = Tag.where(name: previous_value.split("|")).pluck(:id)
new_tag_ids = Tag.where(name: new_value.split("|")).pluck(:id)
now = Time.zone.now
notification_level = self.tag_notification_level(id)
TagUser.where(
tag_id: (previous_tag_ids - new_tag_ids),
notification_level: notification_level,
).delete_all
(new_tag_ids - previous_tag_ids).each do |tag_id|
skip_user_ids = TagUser.where(tag_id: tag_id).pluck(:user_id)
User
.real
.where(staged: false)
.where.not(id: skip_user_ids)
.select(:id)
.find_in_batches do |users|
tag_users = []
users.each do |user|
tag_users << {
tag_id: tag_id,
user_id: user.id,
notification_level: notification_level,
created_at: now,
updated_at: now,
}
end
TagUser.insert_all!(tag_users)
end
end
elsif self.is_sidebar_default_setting?(id)
Jobs.enqueue(
:backfill_sidebar_site_settings,
setting_name: id,
previous_value: previous_value,
new_value: new_value,
)
end
end
def self.user_options
{
default_email_mailing_list_mode: "mailing_list_mode",
default_email_mailing_list_mode_frequency: "mailing_list_mode_frequency",
default_email_level: "email_level",
default_email_messages_level: "email_messages_level",
default_topics_automatic_unpin: "automatically_unpin_topics",
default_email_previous_replies: "email_previous_replies",
default_email_in_reply_to: "email_in_reply_to",
default_other_enable_quoting: "enable_quoting",
default_other_enable_defer: "enable_defer",
default_other_external_links_in_new_tab: "external_links_in_new_tab",
default_other_dynamic_favicon: "dynamic_favicon",
default_other_new_topic_duration_minutes: "new_topic_duration_minutes",
default_other_auto_track_topics_after_msecs: "auto_track_topics_after_msecs",
default_other_notification_level_when_replying: "notification_level_when_replying",
default_other_like_notification_frequency: "like_notification_frequency",
default_other_skip_new_user_tips: "skip_new_user_tips",
default_email_digest_frequency: "digest_after_minutes",
default_include_tl0_in_digests: "include_tl0_in_digests",
default_text_size: "text_size_key",
default_title_count_mode: "title_count_mode_key",
default_hide_profile_and_presence: "hide_profile_and_presence",
default_sidebar_link_to_filtered_list: "sidebar_link_to_filtered_list",
default_sidebar_show_count_of_new_items: "sidebar_show_count_of_new_items",
}
end
def self.category_notification_level(id)
case id
when "default_categories_watching"
NotificationLevels.all[:watching]
when "default_categories_tracking"
NotificationLevels.all[:tracking]
when "default_categories_muted"
NotificationLevels.all[:muted]
when "default_categories_watching_first_post"
NotificationLevels.all[:watching_first_post]
when "default_categories_normal"
NotificationLevels.all[:regular]
end
end
def self.tag_notification_level(id)
case id
when "default_tags_watching"
NotificationLevels.all[:watching]
when "default_tags_tracking"
NotificationLevels.all[:tracking]
when "default_tags_muted"
NotificationLevels.all[:muted]
when "default_tags_watching_first_post"
NotificationLevels.all[:watching_first_post]
end
end
def self.is_sidebar_default_setting?(setting_name)
%w[default_navigation_menu_categories default_navigation_menu_tags].include?(setting_name.to_s)
end
end

View File

@ -0,0 +1,65 @@
# frozen_string_literal: true
class UpdateSiteSetting
include Service::Base
policy :current_user_is_admin
contract
step :convert_name_to_sym
policy :setting_is_visible
policy :setting_is_configurable
step :cleanup_value
step :save
class Contract
attribute :setting_name
attribute :new_value
attribute :allow_changing_hidden, :boolean, default: false
validates :setting_name, presence: true
end
private
def convert_name_to_sym(setting_name:)
context.setting_name = setting_name.to_sym
end
def current_user_is_admin(guardian:)
guardian.is_admin?
end
def setting_is_visible(setting_name:)
context.allow_changing_hidden || !SiteSetting.hidden_settings.include?(setting_name)
end
def setting_is_configurable(setting_name:)
return true if !SiteSetting.plugins[setting_name]
Discourse.plugins_by_name[SiteSetting.plugins[setting_name]].configurable?
end
def cleanup_value(setting_name:, new_value:)
new_value = new_value.strip if new_value.is_a?(String)
case SiteSetting.type_supervisor.get_type(setting_name)
when :integer
new_value = new_value.tr("^-0-9", "").to_i if new_value.is_a?(String)
when :file_size_restriction
new_value = new_value.tr("^0-9", "").to_i if new_value.is_a?(String)
when :uploaded_image_list
new_value = new_value.blank? ? "" : Upload.get_from_urls(new_value.split("|")).to_a
when :upload
new_value = Upload.get_from_url(new_value) || ""
end
context.new_value = new_value
end
def save(setting_name:, new_value:, guardian:)
SiteSetting.set_and_log(setting_name, new_value, guardian.user)
end
end

View File

@ -333,6 +333,8 @@ en:
site_settings:
invalid_site_setting: "No setting named '%{name}' exists"
invalid_category_id: "You specified a category that does not exist"
site_setting_is_hidden: "You are not allowed to change hidden settings"
site_setting_is_unconfigurable: "You are not allowed to change unconfigurable settings"
invalid_choice:
one: "'%{name}' is an invalid choice."
other: "'%{name}' are invalid choices."

View File

@ -185,7 +185,8 @@ module SiteSettingExtension
include_locale_setting: true,
only_overridden: false,
filter_categories: nil,
filter_plugin: nil
filter_plugin: nil,
filter_names: nil
)
locale_setting_hash = {
setting: "default_locale",
@ -256,6 +257,13 @@ module SiteSettingExtension
true
end
end
.select do |setting|
if filter_names
filter_names.include?(setting[:setting].to_s)
else
true
end
end
.unshift(include_locale_setting && !only_overridden ? locale_setting_hash : nil)
.compact
end

View File

@ -20,6 +20,25 @@ RSpec.describe Admin::SiteSettingsController do
expect(locale.length).to eq(1)
end
describe "the filter_names param" do
it "only returns settings that are specified in the filter_names param" do
get "/admin/site_settings.json",
params: {
filter_names: %w[title site_description notification_email],
}
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["site_settings"].size).to eq(3)
expect(json["site_settings"].map { |s| s["setting"] }).to contain_exactly(
"title",
"site_description",
"notification_email",
)
end
end
end
shared_examples "site settings inaccessible" do

View File

@ -0,0 +1,71 @@
# frozen_string_literal: true
describe(UpdateSiteSetting) do
fab!(:admin)
def call_service(name, value, user: admin, allow_changing_hidden: false)
described_class.call(
setting_name: name,
new_value: value,
guardian: user.guardian,
allow_changing_hidden:,
)
end
context "when setting_name is blank" do
it "fails the service contract" do
expect(call_service(nil, "blah whatever")).to fail_a_contract
expect(call_service(:"", "blah whatever")).to fail_a_contract
end
end
context "when a non-admin user tries to change a setting" do
it "fails the current_user_is_admin policy" do
expect(call_service(:title, "some new title", user: Fabricate(:moderator))).to fail_a_policy(
:current_user_is_admin,
)
expect(SiteSetting.title).not_to eq("some new title")
end
end
context "when the user changes a hidden setting" do
context "when allow_changing_hidden is false" do
it "fails the setting_is_visible policy" do
expect(call_service(:max_category_nesting, 3)).to fail_a_policy(:setting_is_visible)
expect(SiteSetting.max_category_nesting).not_to eq(3)
end
end
context "when allow_changing_hidden is true" do
it "updates the specified setting" do
expect(call_service(:max_category_nesting, 3, allow_changing_hidden: true)).to be_success
expect(SiteSetting.max_category_nesting).to eq(3)
end
end
end
context "when the user changes a visible setting" do
it "updates the specified setting" do
expect(call_service(:title, "hello this is title")).to be_success
expect(SiteSetting.title).to eq("hello this is title")
end
it "cleans up the new setting value before using it" do
expect(call_service(:suggested_topics, "308viu")).to be_success
expect(SiteSetting.suggested_topics).to eq(308)
expect(call_service(:max_image_size_kb, "8zf843")).to be_success
expect(SiteSetting.max_image_size_kb).to eq(8843)
end
it "creates an entry in the staff action logs" do
expect do expect(call_service(:max_image_size_kb, 44_543)).to be_success end.to change {
UserHistory.where(
action: UserHistory.actions[:change_site_setting],
subject: "max_image_size_kb",
).count
}.by(1)
end
end
end