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.

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)

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"

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