diff --git a/app/serializers/admin_plugin_serializer.rb b/app/serializers/admin_plugin_serializer.rb index a462f9d16f7..73efef681d7 100644 --- a/app/serializers/admin_plugin_serializer.rb +++ b/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? diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index aa282453812..58e62079037 100644 --- a/lib/plugin/instance.rb +++ b/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? diff --git a/spec/lib/plugin/instance_spec.rb b/spec/lib/plugin/instance_spec.rb index 1691d279a21..02d64d8d6ba 100644 --- a/spec/lib/plugin/instance_spec.rb +++ b/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