PERF: Admin plugin preload settings routes (#31215)

Followup 503f9b6f02ac5c4918d41611848c886b8755e5a0

This previous commit introduced an autogenerated
settings route for every plugin with more than one
setting defined. Plugins with only one setting
only have enabled_site_settings defined, which are
handled using the toggle in the admin plugin list,
so we don't need a dedicated setting page for them.

However in production this introduced a performance
issue, since we were looking through SiteSetting.all_settings
for every plugin, which could be quite slow in some
cases especially on our hosting.

Instead, we already have all the plugin settings cached
inside `SiteSetting.plugins`. We can instead use this to
count how many settings the plugin has, then if there is > 1
for a plugin we use the settings route. This is a much faster lookup
than
searching through SiteSetting.all_settings.

app/serializers/admin_plugin_serializer.rb

@@ -61,15 +61,15 @@ class AdminPluginSerializer < ApplicationSerializer
   end
 
   def plugin_settings
-    @plugin_settings ||= SiteSetting.plugins.select { |_, v| v == id }
+    object.plugin_settings
   end
 
   def has_settings
-    plugin_settings.values.any?
+    object.any_settings?
   end
 
   def has_only_enabled_setting
-    plugin_settings.keys.length == 1 && plugin_settings.keys.first == enabled_setting
+    object.has_only_enabled_setting?
   end
 
   def include_url?

lib/plugin/instance.rb

@@ -123,8 +123,7 @@ class Plugin::Instance
     route = self.admin_route
 
     if route.blank?
-      return if !any_settings?
-
+      return if !any_settings? || has_only_enabled_setting?
       route = default_admin_route
     end
 
@@ -137,11 +136,15 @@ class Plugin::Instance
   end
 
   def any_settings?
-    return false if !configurable?
+    configurable? && plugin_settings.values.length.positive?
+  end
 
-    SiteSetting
-      .all_settings(filter_plugin: self.name)
-      .any? { |s| s[:setting] != @enabled_site_setting }
+  def has_only_enabled_setting?
+    any_settings? && plugin_settings.values.one?
+  end
+
+  def plugin_settings
+    @plugin_settings ||= SiteSetting.plugins.select { |_, plugin_name| plugin_name == self.name }
   end
 
   def configurable?

spec/lib/plugin/instance_spec.rb

@@ -1103,4 +1103,80 @@ TEXT
       )
     end
   end
+
+  describe "#full_admin_route" do
+    context "when there is no admin route defined for the plugin" do
+      context "if the plugin has more than one setting" do
+        before do
+          plugin_instance.stubs(:plugin_settings).returns(
+            { enabled_setting: plugin_instance.name, other_setting: plugin_instance.name },
+          )
+        end
+
+        it "returns the default settings route" do
+          expect(plugin_instance.full_admin_route).to eq(
+            {
+              auto_generated: true,
+              full_location: "adminPlugins.show",
+              label: "discourse_sample_plugin.title",
+              location: "discourse-sample-plugin",
+              use_new_show_route: true,
+            },
+          )
+        end
+      end
+
+      context "if the plugin has only one setting (which is the enabled setting)" do
+        before do
+          plugin_instance.stubs(:plugin_settings).returns({ enabled_setting: plugin_instance.name })
+        end
+
+        it "returns nothing" do
+          expect(plugin_instance.full_admin_route).to be_nil
+        end
+      end
+
+      context "if the plugin is not configurable" do
+        before { plugin_instance.stubs(:configurable?).returns(false) }
+
+        it "returns nothing" do
+          expect(plugin_instance.full_admin_route).to be_nil
+        end
+      end
+    end
+
+    context "when there is an admin route defined for the plugin" do
+      context "when using the new show route" do
+        before { plugin_instance.add_admin_route("test", "testIndex", use_new_show_route: true) }
+
+        it "returns the correct details" do
+          expect(plugin_instance.full_admin_route).to eq(
+            {
+              auto_generated: false,
+              full_location: "adminPlugins.show",
+              label: "test",
+              location: "testIndex",
+              use_new_show_route: true,
+            },
+          )
+        end
+      end
+
+      context "when not using the new show route" do
+        before { plugin_instance.add_admin_route("test", "testIndex") }
+
+        it "returns the correct details" do
+          expect(plugin_instance.full_admin_route).to eq(
+            {
+              auto_generated: false,
+              full_location: "adminPlugins.testIndex",
+              label: "test",
+              location: "testIndex",
+              use_new_show_route: false,
+            },
+          )
+        end
+      end
+    end
+  end
 end