From 74fd883a89a4492a44f8289fa5f3afdc65b6b484 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 25 Jan 2024 10:45:46 +1000 Subject: [PATCH] DEV: Improve site setting rename generator (#25354) We need to be able to generate these migrations for plugin settings as well. Also, we can use the type supervisor to get the enum data in a nicer way. --- ...ting_move_to_groups_migration_generator.rb | 27 +++++++-- .../fixtures/site_settings/generator_test.yml | 17 ++++++ ...ove_to_groups_migrations_generator_spec.rb | 60 +++++++++---------- 3 files changed, 69 insertions(+), 35 deletions(-) create mode 100644 spec/fixtures/site_settings/generator_test.yml diff --git a/lib/generators/site_setting_move_to_groups_migration/site_setting_move_to_groups_migration_generator.rb b/lib/generators/site_setting_move_to_groups_migration/site_setting_move_to_groups_migration_generator.rb index 31515686886..0391cb23ff7 100644 --- a/lib/generators/site_setting_move_to_groups_migration/site_setting_move_to_groups_migration_generator.rb +++ b/lib/generators/site_setting_move_to_groups_migration/site_setting_move_to_groups_migration_generator.rb @@ -11,6 +11,7 @@ class SiteSettingMoveToGroupsMigrationGenerator < Rails::Generators::Base file_path = "db/migrate/#{migration_version}_fill_#{new_name}_based_on_deprecated_setting.rb" class_name = "Fill#{new_name.classify}BasedOnDeprecatedSetting" + load_all_settings validate_setting_name!(old_name) validate_setting_name!(new_name) validate_setting_type!(old_name) @@ -89,17 +90,35 @@ class SiteSettingMoveToGroupsMigrationGenerator < Rails::Generators::Base private + def load_all_settings + load_settings(File.join(Rails.root, "config", "site_settings.yml")) + + if GlobalSetting.load_plugins? + Dir[File.join(Rails.root, "plugins", "*", "config", "settings.yml")].each do |file| + load_settings(file, plugin: file.split("/")[-3]) + end + end + + if Rails.env.test? + load_settings( + File.join(Rails.root, "spec", "fixtures", "site_settings", "generator_test.yml"), + ) + end + end + def validate_setting_name!(name) - if !SiteSetting.respond_to?(name) + if !self.respond_to?(name) say "Site setting with #{name} does not exist" raise ArgumentError end end def setting_type(name) - @site_settings ||= load_settings(File.join(Rails.root, "config", "site_settings.yml")) - subgroup = @site_settings.values.find { |row| row.keys.include?(name) } - subgroup[name][:enum] + if type_supervisor.get_type(name.to_sym) == :enum + return type_supervisor.get_enum_class(name.to_sym).to_s + end + + nil end def validate_setting_type!(name) diff --git a/spec/fixtures/site_settings/generator_test.yml b/spec/fixtures/site_settings/generator_test.yml new file mode 100644 index 00000000000..cc9bf44986c --- /dev/null +++ b/spec/fixtures/site_settings/generator_test.yml @@ -0,0 +1,17 @@ +category1: + use_https: true + min_trust_level_to_allow_invite: + default: 2 + enum: "TrustLevelSetting" + hidden: true + min_trust_level_to_allow_invite_tl_and_staff: + default: 2 + enum: "TrustLevelAndStaffSetting" + hidden: true + allow_invite_groups: + default: "11" # auto group trust_level_1 + type: group_list + client: true + allow_any: false + refresh: true + validator: "AtLeastOneGroupValidator" diff --git a/spec/generator/site_setting_move_to_groups_migrations_generator_spec.rb b/spec/generator/site_setting_move_to_groups_migrations_generator_spec.rb index dd1454a4e58..8df3277d385 100644 --- a/spec/generator/site_setting_move_to_groups_migrations_generator_spec.rb +++ b/spec/generator/site_setting_move_to_groups_migrations_generator_spec.rb @@ -7,23 +7,22 @@ require "generators/site_setting_move_to_groups_migration/site_setting_move_to_g RSpec.describe SiteSettingMoveToGroupsMigrationGenerator, type: :generator do it "generates the correct migration for TrustLevelSetting" do freeze_time DateTime.parse("2010-01-01 12:00") - described_class - .any_instance - .expects(:load_settings) - .returns({ branding: { "site_description" => { enum: "TrustLevelSetting" } } }) - described_class.start(%w[site_description contact_email], destination_root: "#{Rails.root}/tmp") + described_class.start( + %w[min_trust_level_to_allow_invite allow_invite_groups], + destination_root: "#{Rails.root}/tmp", + ) file_path = - "#{Rails.root}/tmp/db/migrate/20100101120000_fill_contact_email_based_on_deprecated_setting.rb" + "#{Rails.root}/tmp/db/migrate/20100101120000_fill_allow_invite_groups_based_on_deprecated_setting.rb" expected_content = <<~EXPECTED_CONTENT # frozen_string_literal: true - class FillContactEmailBasedOnDeprecatedSetting < ActiveRecord::Migration[7.0] + class FillAllowInviteGroupBasedOnDeprecatedSetting < ActiveRecord::Migration[7.0] def up old_setting_trust_level = DB.query_single( - "SELECT value FROM site_settings WHERE name = 'site_description' LIMIT 1", + "SELECT value FROM site_settings WHERE name = 'min_trust_level_to_allow_invite' LIMIT 1", ).first if old_setting_trust_level.present? @@ -31,7 +30,7 @@ RSpec.describe SiteSettingMoveToGroupsMigrationGenerator, type: :generator do DB.exec( "INSERT INTO site_settings(name, value, data_type, created_at, updated_at) - VALUES('contact_email', :setting, '20', NOW(), NOW())", + VALUES('allow_invite_groups', :setting, '20', NOW(), NOW())", setting: allowed_groups, ) end @@ -44,28 +43,28 @@ RSpec.describe SiteSettingMoveToGroupsMigrationGenerator, type: :generator do EXPECTED_CONTENT expect(File.read(file_path)).to eq(expected_content) + ensure File.delete(file_path) end it "generates the correct migration for TrustLevelAndStaffSetting" do freeze_time DateTime.parse("2010-01-01 12:00") - described_class - .any_instance - .expects(:load_settings) - .returns({ branding: { "title" => { enum: "TrustLevelAndStaffSetting" } } }) - described_class.start(%w[title contact_email], destination_root: "#{Rails.root}/tmp") + described_class.start( + %w[min_trust_level_to_allow_invite_tl_and_staff allow_invite_groups], + destination_root: "#{Rails.root}/tmp", + ) file_path = - "#{Rails.root}/tmp/db/migrate/20100101120000_fill_contact_email_based_on_deprecated_setting.rb" + "#{Rails.root}/tmp/db/migrate/20100101120000_fill_allow_invite_groups_based_on_deprecated_setting.rb" expected_content = <<~EXPECTED_CONTENT # frozen_string_literal: true - class FillContactEmailBasedOnDeprecatedSetting < ActiveRecord::Migration[7.0] + class FillAllowInviteGroupBasedOnDeprecatedSetting < ActiveRecord::Migration[7.0] def up old_setting_trust_level = DB.query_single( - "SELECT value FROM site_settings WHERE name = 'title' LIMIT 1", + "SELECT value FROM site_settings WHERE name = 'min_trust_level_to_allow_invite_tl_and_staff' LIMIT 1", ).first if old_setting_trust_level.present? @@ -89,7 +88,7 @@ RSpec.describe SiteSettingMoveToGroupsMigrationGenerator, type: :generator do DB.exec( "INSERT INTO site_settings(name, value, data_type, created_at, updated_at) - VALUES('contact_email', :setting, '20', NOW(), NOW())", + VALUES('allow_invite_groups', :setting, '20', NOW(), NOW())", setting: allowed_groups, ) end @@ -102,24 +101,23 @@ RSpec.describe SiteSettingMoveToGroupsMigrationGenerator, type: :generator do EXPECTED_CONTENT expect(File.read(file_path)).to eq(expected_content) + ensure File.delete(file_path) end it "raises an error when old name is incorrect" do - expect { described_class.start(%w[wrong_name contact_email]) }.to raise_error(ArgumentError) - end - - it "raises an error when new name is incorrect" do - expect { described_class.start(%w[site_description wrong_name]) }.to raise_error(ArgumentError) - end - - it "raises an error when old setting is incorrect type" do - described_class - .any_instance - .expects(:load_settings) - .returns({ branding: { "site_description" => { enum: "EmojiSetSiteSetting" } } }) - expect { described_class.start(%w[site_description contact_email]) }.to raise_error( + expect { described_class.start(%w[wrong_name allow_invite_groups]) }.to raise_error( ArgumentError, ) end + + it "raises an error when new name is incorrect" do + expect { described_class.start(%w[min_trust_level_to_allow_invite wrong_name]) }.to raise_error( + ArgumentError, + ) + end + + it "raises an error when old setting is incorrect type" do + expect { described_class.start(%w[title allow_invite_groups]) }.to raise_error(ArgumentError) + end end