diff --git a/app/controllers/admin/configs_controller.rb b/app/controllers/admin/configs_controller.rb deleted file mode 100644 index ed0d9ee2d..000000000 --- a/app/controllers/admin/configs_controller.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Admin - class ConfigsController < Admin::SettingsController - include SettingsParams - - def create - result = ::Settings::Upsert.call(settings_params) - if result.success? - Audit::Logger.log(:internal, current_user, params.dup) - bust_content_change_caches - redirect_to admin_config_path, notice: "Successfully updated settings." - else - redirect_to admin_config_path, alert: "😠#{result.errors.to_sentence}" - end - end - end -end diff --git a/app/controllers/admin/settings/authentications_controller.rb b/app/controllers/admin/settings/authentications_controller.rb index 495dcc767..31db9030c 100644 --- a/app/controllers/admin/settings/authentications_controller.rb +++ b/app/controllers/admin/settings/authentications_controller.rb @@ -1,15 +1,10 @@ module Admin module Settings - class AuthenticationsController < Admin::ApplicationController - def create - result = ::Authentication::SettingsUpsert.call(settings_params) + class AuthenticationsController < Admin::Settings::BaseController + private - if result.success? - Audit::Logger.log(:internal, current_user, params.dup) - redirect_to admin_config_path, notice: "Successfully updated settings." - else - redirect_to admin_config_path, alert: "😠#{result.errors.to_sentence}" - end + def upsert_config(settings) + ::Settings::Authentication::Upsert.call(settings) end def settings_params diff --git a/app/controllers/admin/settings/base_controller.rb b/app/controllers/admin/settings/base_controller.rb new file mode 100644 index 000000000..3fbc35833 --- /dev/null +++ b/app/controllers/admin/settings/base_controller.rb @@ -0,0 +1,51 @@ +module Admin + module Settings + class BaseController < Admin::ApplicationController + MISMATCH_ERROR = "The confirmation key does not match".freeze + + before_action :extra_authorization_and_confirmation, only: [:create] + + def create + result = upsert_config(settings_params) + + if result.success? + Audit::Logger.log(:internal, current_user, params.dup) + redirect_to admin_config_path, notice: "Successfully updated settings." + else + redirect_to admin_config_path, alert: "😠#{result.errors.to_sentence}" + end + end + + private + + def extra_authorization_and_confirmation + not_authorized unless current_user.has_role?(:super_admin) + raise_confirmation_mismatch_error unless confirmation_text_valid? + end + + def confirmation_text_valid? + params.require(:confirmation) == + "My username is @#{current_user.username} and this action is 100% safe and appropriate." + end + + def raise_confirmation_mismatch_error + raise ActionController::BadRequest.new, MISMATCH_ERROR + end + + # Override this method if you need to call a custom class for upserting. + # Ideally such a class eventually calls out to Settings::Upsert and returns + # the result of that service. + def upsert_config(settings) + ::Settings::Upsert.call(settings, authorization_resource) + end + + # Override this if you need additional params or need to make other changes, + # e.g. a different require key. + def settings_params + params + .require(:"settings_#{authorization_resource.name.demodulize.underscore}") + .permit(*authorization_resource.keys) + end + end + end +end diff --git a/app/controllers/admin/settings/campaigns_controller.rb b/app/controllers/admin/settings/campaigns_controller.rb index 2e5995e20..f8afc10b5 100644 --- a/app/controllers/admin/settings/campaigns_controller.rb +++ b/app/controllers/admin/settings/campaigns_controller.rb @@ -1,22 +1,6 @@ module Admin module Settings - class CampaignsController < Admin::ApplicationController - def create - result = ::Campaigns::SettingsUpsert.call(settings_params) - - if result.success? - Audit::Logger.log(:internal, current_user, params.dup) - redirect_to admin_config_path, notice: "Successfully updated settings." - else - redirect_to admin_config_path, alert: "😠#{result.errors.to_sentence}" - end - end - - def settings_params - params - .require(:settings_campaign) - .permit(*::Settings::Campaign.keys) - end + class CampaignsController < Admin::Settings::BaseController end end end diff --git a/app/controllers/admin/settings/communities_controller.rb b/app/controllers/admin/settings/communities_controller.rb index e511b9a0b..8657315ed 100644 --- a/app/controllers/admin/settings/communities_controller.rb +++ b/app/controllers/admin/settings/communities_controller.rb @@ -1,36 +1,6 @@ module Admin module Settings - class CommunitiesController < Admin::ApplicationController - def create - errors = upsert_config(settings_params) - - if errors.none? - Audit::Logger.log(:internal, current_user, params.dup) - redirect_to admin_config_path, notice: "Successfully updated settings." - else - redirect_to admin_config_path, alert: "😠#{errors.to_sentence}" - end - end - - private - - def settings_params - params - .require(:settings_community) - .permit(*::Settings::Community.keys) - end - - def upsert_config(configs) - errors = [] - configs.each do |key, value| - ::Settings::Community.public_send("#{key}=", value) if value.present? - rescue ActiveRecord::RecordInvalid => e - errors << e.message - next - end - - errors - end + class CommunitiesController < Admin::Settings::BaseController end end end diff --git a/app/controllers/admin/settings/general_settings_controller.rb b/app/controllers/admin/settings/general_settings_controller.rb new file mode 100644 index 000000000..3a7b2f039 --- /dev/null +++ b/app/controllers/admin/settings/general_settings_controller.rb @@ -0,0 +1,47 @@ +module Admin + module Settings + class GeneralSettingsController < Admin::Settings::BaseController + SPECIAL_PARAMS_TO_ADD = %w[ + credit_prices_in_cents + email_addresses + meta_keywords + ].freeze + + def create + result = ::Settings::General::Upsert.call(settings_params) + if result.success? + Audit::Logger.log(:internal, current_user, params.dup) + bust_content_change_caches + redirect_to admin_config_path, notice: "Successfully updated settings." + else + redirect_to admin_config_path, alert: "😠#{result.errors.to_sentence}" + end + end + + private + + # NOTE: we need to override this since the controller name doesn't reflect + # the model name + def authorization_resource + ::Settings::General + end + + def settings_params + has_emails = params.dig(:settings_general, :email_addresses).present? + params[:settings_general][:email_addresses][:default] = ApplicationConfig["DEFAULT_EMAIL"] if has_emails + + params.require(:settings_general)&.permit( + settings_keys.map(&:to_sym), + social_media_handles: ::Settings::General.social_media_handles.keys, + email_addresses: ::Settings::General.email_addresses.keys, + meta_keywords: ::Settings::General.meta_keywords.keys, + credit_prices_in_cents: ::Settings::General.credit_prices_in_cents.keys, + ) + end + + def settings_keys + ::Settings::General.keys + SPECIAL_PARAMS_TO_ADD + end + end + end +end diff --git a/app/controllers/admin/settings/mandatory_settings_controller.rb b/app/controllers/admin/settings/mandatory_settings_controller.rb index 71c214bce..e49be3482 100644 --- a/app/controllers/admin/settings/mandatory_settings_controller.rb +++ b/app/controllers/admin/settings/mandatory_settings_controller.rb @@ -1,17 +1,14 @@ module Admin module Settings - class MandatorySettingsController < Admin::ApplicationController - def create - errors = upsert_config(settings_params) - - if errors.none? - Audit::Logger.log(:internal, current_user, params.dup) - redirect_to admin_config_path, notice: "Successfully updated settings." - else - redirect_to admin_config_path, alert: "😠#{errors.to_sentence}" + class MandatorySettingsController < Admin::Settings::BaseController + Result = Struct.new(:errors) do + def success? + errors.none? end end + private + def upsert_config(configs) errors = [] configs.each do |key, value| @@ -25,12 +22,9 @@ module Admin errors << e.message next end - - errors + Result.new(errors) end - private - # NOTE: we need to override this since the controller name doesn't reflect # the model name def authorization_resource diff --git a/app/controllers/admin/settings/rate_limits_controller.rb b/app/controllers/admin/settings/rate_limits_controller.rb index 6a95f3570..ce0ca3947 100644 --- a/app/controllers/admin/settings/rate_limits_controller.rb +++ b/app/controllers/admin/settings/rate_limits_controller.rb @@ -1,22 +1,6 @@ module Admin module Settings - class RateLimitsController < Admin::ApplicationController - def create - result = ::RateLimits::SettingsUpsert.call(settings_params) - - if result.success? - Audit::Logger.log(:internal, current_user, params.dup) - redirect_to admin_config_path, notice: "Successfully updated settings." - else - redirect_to admin_config_path, alert: "😠#{result.errors.to_sentence}" - end - end - - def settings_params - params - .require(:settings_rate_limit) - .permit(*::Settings::RateLimit.keys) - end + class RateLimitsController < Admin::Settings::BaseController end end end diff --git a/app/controllers/admin/settings/user_experiences_controller.rb b/app/controllers/admin/settings/user_experiences_controller.rb index cba921581..324c543ce 100644 --- a/app/controllers/admin/settings/user_experiences_controller.rb +++ b/app/controllers/admin/settings/user_experiences_controller.rb @@ -1,34 +1,6 @@ module Admin module Settings - class UserExperiencesController < Admin::ApplicationController - def create - errors = upsert_config(settings_params) - - if errors.none? - Audit::Logger.log(:internal, current_user, params.dup) - redirect_to admin_config_path, notice: "Successfully updated settings." - else - redirect_to admin_config_path, alert: "😠#{errors.to_sentence}" - end - end - - def settings_params - params - .require(:settings_user_experience) - .permit(*::Settings::UserExperience.keys) - end - - def upsert_config(configs) - errors = [] - configs.each do |key, value| - ::Settings::UserExperience.public_send("#{key}=", value) if value.present? - rescue ActiveRecord::RecordInvalid => e - errors << e.message - next - end - - errors - end + class UserExperiencesController < Admin::Settings::BaseController end end end diff --git a/app/controllers/admin/settings_controller.rb b/app/controllers/admin/settings_controller.rb index 6cd297df6..b4bb604ae 100644 --- a/app/controllers/admin/settings_controller.rb +++ b/app/controllers/admin/settings_controller.rb @@ -1,30 +1,19 @@ module Admin + # This controller is solely responsible for rendering the settings page at + # /admin/customization/config. The actual updates get handled by the settings + # controllers in the Admin::Settings namespace. class SettingsController < Admin::ApplicationController - MISMATCH_ERROR = "The confirmation key does not match".freeze - - before_action :extra_authorization_and_confirmation, only: [:create] - layout "admin" - def create; end - def show - @confirmation_text = confirmation_text + @confirmation_text = + "My username is @#{current_user.username} and this action is 100% safe and appropriate." end private - def extra_authorization_and_confirmation - not_authorized unless current_user.has_role?(:super_admin) - raise_confirmation_mismatch_error if params.require(:confirmation) != confirmation_text - end - - def confirmation_text - "My username is @#{current_user.username} and this action is 100% safe and appropriate." - end - - def raise_confirmation_mismatch_error - raise ActionController::BadRequest.new, MISMATCH_ERROR - end + # We need to override this method from Admin::ApplicationController since + # there is no resource to authorize. + def authorization_resource; end end end diff --git a/app/controllers/api/v0/admin/configs_controller.rb b/app/controllers/api/v0/admin/configs_controller.rb index f535a91bf..79a801039 100644 --- a/app/controllers/api/v0/admin/configs_controller.rb +++ b/app/controllers/api/v0/admin/configs_controller.rb @@ -2,7 +2,11 @@ module Api module V0 module Admin class ConfigsController < ApiController - include SettingsParams + SPECIAL_PARAMS_TO_ADD = %w[ + credit_prices_in_cents + email_addresses + meta_keywords + ].freeze before_action :authenticate_with_api_key_or_current_user! before_action :authorize_super_admin @@ -39,6 +43,23 @@ module Api private + def settings_params + has_emails = params.dig(:settings_general, :email_addresses).present? + params[:settings_general][:email_addresses][:default] = ApplicationConfig["DEFAULT_EMAIL"] if has_emails + + params.require(:settings_general)&.permit( + settings_keys.map(&:to_sym), + social_media_handles: ::Settings::General.social_media_handles.keys, + email_addresses: ::Settings::General.email_addresses.keys, + meta_keywords: ::Settings::General.meta_keywords.keys, + credit_prices_in_cents: ::Settings::General.credit_prices_in_cents.keys, + ) + end + + def settings_keys + ::Settings::General.keys + SPECIAL_PARAMS_TO_ADD + end + def auth_settings_params params .require(:site_config) diff --git a/app/controllers/concerns/settings_params.rb b/app/controllers/concerns/settings_params.rb deleted file mode 100644 index ba7d06e04..000000000 --- a/app/controllers/concerns/settings_params.rb +++ /dev/null @@ -1,27 +0,0 @@ -# Helper method for controllers interacting with Settings::General -module SettingsParams - SPECIAL_PARAMS_TO_ADD = %w[ - credit_prices_in_cents - email_addresses - meta_keywords - ].freeze - - def settings_params - has_emails = params.dig(:settings_general, :email_addresses).present? - params[:settings_general][:email_addresses][:default] = ApplicationConfig["DEFAULT_EMAIL"] if has_emails - - params.require(:settings_general)&.permit( - settings_keys.map(&:to_sym), - social_media_handles: Settings::General.social_media_handles.keys, - email_addresses: Settings::General.email_addresses.keys, - meta_keywords: Settings::General.meta_keywords.keys, - credit_prices_in_cents: Settings::General.credit_prices_in_cents.keys, - ) - end - - private - - def settings_keys - Settings::General.keys + SPECIAL_PARAMS_TO_ADD - end -end diff --git a/app/services/authentication/settings_upsert.rb b/app/services/authentication/settings_upsert.rb deleted file mode 100644 index e712e40be..000000000 --- a/app/services/authentication/settings_upsert.rb +++ /dev/null @@ -1,68 +0,0 @@ -module Authentication - class SettingsUpsert - attr_reader :errors - - def self.call(configs) - new(configs).call - end - - def initialize(configs) - @configs = configs - @errors = [] - end - - def call - upsert_configs - self - end - - def success? - @errors.none? - end - - private - - # NOTE: @citizen428 - This was adapted from Settings::Upsert. I'll see if - # a pattern for refactoring emerges in the future, but for now I'll leave - # this as-is. - def upsert_configs - @configs.each do |key, value| - if key == "auth_providers_to_enable" - update_enabled_providers(value) unless value.class.name != "String" - elsif value.is_a?(Array) && value.any? - Settings::Authentication.public_send("#{key}=", value.reject(&:blank?)) - elsif value.present? - Settings::Authentication.public_send("#{key}=", value.strip) - end - rescue ActiveRecord::RecordInvalid => e - @errors << e.message - next - end - end - - def update_enabled_providers(value) - enabled_providers = value.split(",").filter_map do |entry| - entry unless invalid_provider_entry(entry) - end - return if email_login_disabled_with_one_or_less_auth_providers(enabled_providers) - - Settings::Authentication.providers = enabled_providers - end - - def invalid_provider_entry(entry) - entry.blank? || - Authentication::Providers.available.map(&:to_s).exclude?(entry) || - provider_keys_missing(entry) - end - - def provider_keys_missing(entry) - Settings::Authentication.public_send("#{entry}_key").blank? || - Settings::Authentication.public_send("#{entry}_secret").blank? - end - - def email_login_disabled_with_one_or_less_auth_providers(enabled_providers) - !Settings::Authentication.allow_email_password_login && - enabled_providers.count <= 1 - end - end -end diff --git a/app/services/campaigns/settings_upsert.rb b/app/services/campaigns/settings_upsert.rb deleted file mode 100644 index e9c8dc6af..000000000 --- a/app/services/campaigns/settings_upsert.rb +++ /dev/null @@ -1,41 +0,0 @@ -module Campaigns - class SettingsUpsert - attr_reader :errors - - def self.call(configs) - new(configs).call - end - - def initialize(configs) - @configs = configs - @errors = [] - end - - def call - upsert_configs - self - end - - def success? - @errors.none? - end - - private - - # NOTE: @citizen428 - This was adapted from Settings::Upsert. I'll see if - # a pattern for refactoring emerges in the future, but for now I'll leave - # this as-is. - def upsert_configs - @configs.each do |key, value| - if value.is_a?(Array) && value.any? - Settings::Campaign.public_send("#{key}=", value.reject(&:blank?)) - elsif value.present? - Settings::Campaign.public_send("#{key}=", value.strip) - end - rescue ActiveRecord::RecordInvalid => e - @errors << e.message - next - end - end - end -end diff --git a/app/services/rate_limits/settings_upsert.rb b/app/services/rate_limits/settings_upsert.rb deleted file mode 100644 index 27f86c41d..000000000 --- a/app/services/rate_limits/settings_upsert.rb +++ /dev/null @@ -1,41 +0,0 @@ -module RateLimits - class SettingsUpsert - attr_reader :errors - - def self.call(configs) - new(configs).call - end - - def initialize(configs) - @configs = configs - @errors = [] - end - - def call - upsert_configs - self - end - - def success? - @errors.none? - end - - private - - # NOTE: @citizen428 - This was adapted from Settings::Upsert. I'll see if - # a pattern for refactoring emerges in the future, but for now I'll leave - # this as-is. - def upsert_configs - @configs.each do |key, value| - if value.is_a?(Array) && value.any? - Settings::RateLimit.public_send("#{key}=", value.reject(&:blank?)) - elsif value.present? - Settings::RateLimit.public_send("#{key}=", value.strip) - end - rescue ActiveRecord::RecordInvalid => e - @errors << e.message - next - end - end - end -end diff --git a/app/services/settings/authentication/upsert.rb b/app/services/settings/authentication/upsert.rb new file mode 100644 index 000000000..114947df3 --- /dev/null +++ b/app/services/settings/authentication/upsert.rb @@ -0,0 +1,41 @@ +module Settings + class Authentication + module Upsert + attr_reader :errors + + def self.call(settings) + auth_providers_to_enable = settings.delete("auth_providers_to_enable") + result = Settings::Upsert.call(settings, ::Settings::Authentication) + update_enabled_providers(auth_providers_to_enable) + result + end + + def self.update_enabled_providers(value) + return if value.blank? + + enabled_providers = value.split(",").filter_map do |entry| + entry unless invalid_provider_entry(entry) + end + return if email_login_disabled_with_one_or_less_auth_providers(enabled_providers) + + Settings::Authentication.providers = enabled_providers + end + + def self.invalid_provider_entry(entry) + entry.blank? || + ::Authentication::Providers.available.map(&:to_s).exclude?(entry) || + provider_keys_missing(entry) + end + + def self.provider_keys_missing(entry) + ::Settings::Authentication.public_send("#{entry}_key").blank? || + ::Settings::Authentication.public_send("#{entry}_secret").blank? + end + + def self.email_login_disabled_with_one_or_less_auth_providers(enabled_providers) + !::Settings::Authentication.allow_email_password_login && + enabled_providers.count <= 1 + end + end + end +end diff --git a/app/services/settings/general/upsert.rb b/app/services/settings/general/upsert.rb new file mode 100644 index 000000000..6bdbf9bd6 --- /dev/null +++ b/app/services/settings/general/upsert.rb @@ -0,0 +1,34 @@ +module Settings + class General + module Upsert + PARAMS_TO_BE_CLEANED = %i[sidebar_tags suggested_tags suggested_users].freeze + TAG_PARAMS = %w[sidebar_tags suggested_tags].freeze + + def self.call(settings) + cleaned_params = clean_params(settings) + result = ::Settings::Upsert.call(cleaned_params, ::Settings::General) + return result unless result.success? + + create_tags_if_necessary(settings) + result + end + + def self.clean_params(settings) + PARAMS_TO_BE_CLEANED.each do |param| + settings[param] = settings[param]&.downcase&.delete(" ") if settings[param] + end + settings[:credit_prices_in_cents]&.transform_values!(&:to_i) + settings + end + + # Bulk create tags if they should exist. + # This is an acts-as-taggable-on as used on saving of an Article, etc. + def self.create_tags_if_necessary(settings) + return unless (settings.keys & TAG_PARAMS).any? + + tags = Settings::General.suggested_tags + Settings::General.sidebar_tags + Tag.find_or_create_all_with_like_by_name(tags) + end + end + end +end diff --git a/app/services/settings/upsert.rb b/app/services/settings/upsert.rb index cdd44aca8..d9e427215 100644 --- a/app/services/settings/upsert.rb +++ b/app/services/settings/upsert.rb @@ -1,68 +1,42 @@ module Settings + # This service ensures that settings upserts to the database happen in a + # standardized way. Instead of subclassing this service I recommend wrapping + # it, see e.g. Settings::General::Upsert. class Upsert - VALID_DOMAIN = /^[a-zA-Z0-9]{1,61}[a-zA-Z0-9](?:\.[a-zA-Z]{2,})+$/.freeze + attr_reader :errors, :settings_class - PARAMS_TO_BE_CLEANED = %i[sidebar_tags suggested_tags suggested_users].freeze - - attr_reader :errors - - def self.call(configs) - new(configs).call + def self.call(settings, settings_class) + new(settings, settings_class).call end - def initialize(configs) - @configs = configs - @success = false + def initialize(settings, settings_class) + @settings = settings + @settings_class = settings_class + @errors = [] end def call - clean_up_params - - @errors = [] - upsert_configs - return self if @errors.any? - - @success = true - after_upsert_tasks + upsert_settings self end def success? - @success + @errors.none? end - def upsert_configs - @configs.each do |key, value| - if value.is_a?(Array) - Settings::General.public_send("#{key}=", value.reject(&:blank?)) unless value.empty? - elsif value.respond_to?(:to_h) - Settings::General.public_send("#{key}=", value.to_h) unless value.empty? - else - Settings::General.public_send("#{key}=", value.strip) unless value.nil? + def upsert_settings + @settings.each do |key, value| + if value.is_a?(Array) && value.any? + settings_class.public_send("#{key}=", value.reject(&:blank?)) + elsif value.respond_to?(:to_h) && value.present? + settings_class.public_send("#{key}=", value.to_h) + elsif value.present? + settings_class.public_send("#{key}=", value.strip) end rescue ActiveRecord::RecordInvalid => e @errors << e.message next end end - - def after_upsert_tasks - create_tags_if_not_created - end - - def clean_up_params - PARAMS_TO_BE_CLEANED.each do |param| - @configs[param] = @configs[param]&.downcase&.delete(" ") if @configs[param] - end - @configs[:credit_prices_in_cents]&.transform_values!(&:to_i) - end - - def create_tags_if_not_created - # Bulk create tags if they should exist. - # This is an acts-as-taggable-on as used on saving of an Article, etc. - return unless (@configs.keys & %w[suggested_tags sidebar_tags]).any? - - Tag.find_or_create_all_with_like_by_name(Settings::General.suggested_tags + Settings::General.sidebar_tags) - end end end diff --git a/app/views/admin/configs/_apple_auth_provider_settings.html.erb b/app/views/admin/settings/_apple_auth_provider_settings.html.erb similarity index 100% rename from app/views/admin/configs/_apple_auth_provider_settings.html.erb rename to app/views/admin/settings/_apple_auth_provider_settings.html.erb diff --git a/app/views/admin/configs/_auth_provider_settings.html.erb b/app/views/admin/settings/_auth_provider_settings.html.erb similarity index 100% rename from app/views/admin/configs/_auth_provider_settings.html.erb rename to app/views/admin/settings/_auth_provider_settings.html.erb diff --git a/app/views/admin/configs/_form_submission.html.erb b/app/views/admin/settings/_form_submission.html.erb similarity index 100% rename from app/views/admin/configs/_form_submission.html.erb rename to app/views/admin/settings/_form_submission.html.erb diff --git a/app/views/admin/configs/show.html.erb b/app/views/admin/settings/show.html.erb similarity index 97% rename from app/views/admin/configs/show.html.erb rename to app/views/admin/settings/show.html.erb index 6dd4a93b4..3c463fe23 100644 --- a/app/views/admin/configs/show.html.erb +++ b/app/views/admin/settings/show.html.erb @@ -115,7 +115,7 @@