FEATURE: Filter settings by plugin (#9692)

app/assets/javascripts/admin/controllers/admin-site-settings.js

@@ -15,12 +15,32 @@ export default Controller.extend({
     // If we have no content, don't bother filtering anything
     if (!!isEmpty(this.allSiteSettings)) return;
 
-    let filter;
+    let filter, pluginFilter;
     if (this.filter) {
-      filter = this.filter.toLowerCase().trim();
+      filter = this.filter
+        .toLowerCase()
+        .split(" ")
+        .filter(word => {
+          if (word.length === 0) {
+            return false;
+          }
+
+          if (word.startsWith("plugin:")) {
+            pluginFilter = word.substr("plugin:".length).trim();
+            return false;
+          }
+
+          return true;
+        })
+        .join(" ")
+        .trim();
     }
 
-    if ((!filter || 0 === filter.length) && !this.onlyOverridden) {
+    if (
+      (!filter || 0 === filter.length) &&
+      (!pluginFilter || 0 === pluginFilter.length) &&
+      !this.onlyOverridden
+    ) {
       this.set("visibleSiteSettings", this.allSiteSettings);
       if (this.categoryNameKey === "all_results") {
         this.transitionToRoute("adminSiteSettings");
@@ -39,6 +59,7 @@ export default Controller.extend({
     this.allSiteSettings.forEach(settingsCategory => {
       const siteSettings = settingsCategory.siteSettings.filter(item => {
         if (this.onlyOverridden && !item.get("overridden")) return false;
+        if (pluginFilter && item.plugin !== pluginFilter) return false;
         if (filter) {
           const setting = item.get("setting").toLowerCase();
           return (

app/assets/javascripts/admin/routes/admin-plugins.js

@@ -9,18 +9,12 @@ export default Route.extend({
       const controller = this.controllerFor("adminSiteSettings");
       this.transitionTo("adminSiteSettingsCategory", "plugins").then(() => {
         if (plugin) {
-          const siteSettingFilter = plugin.get("enabled_setting_filter");
+          // filterContent() is normally on a debounce from typing.
-          const match = /^(.*)_enabled/.exec(plugin.get("enabled_setting"));
+          // Because we don't want the default of "All Results", we tell it
-          const filter = siteSettingFilter || match[1];
+          // to skip the next debounce.
-
+          controller.set("filter", `plugin:${plugin.id}`);
-          if (filter) {
+          controller.set("_skipBounce", true);
-            // filterContent() is normally on a debounce from typing.
+          controller.filterContentNow("plugins");
-            // Because we don't want the default of "All Results", we tell it
-            // to skip the next debounce.
-            controller.set("filter", filter);
-            controller.set("_skipBounce", true);
-            controller.filterContentNow("plugins");
-          }
         }
       });
     }

app/models/site_setting.rb

@@ -12,9 +12,9 @@ class SiteSetting < ActiveRecord::Base
     true
   end
 
-  def self.load_settings(file)
+  def self.load_settings(file, plugin: nil)
     SiteSettings::YamlLoader.new(file).load do |category, name, default, opts|
-      setting(name, default, opts.merge(category: category))
+      setting(name, default, opts.merge(category: category, plugin: plugin))
     end
   end
 
@@ -22,7 +22,7 @@ class SiteSetting < ActiveRecord::Base
 
   unless Rails.env.test? && ENV['LOAD_PLUGINS'] != "1"
     Dir[File.join(Rails.root, "plugins", "*", "config", "settings.yml")].each do |file|
-      load_settings(file)
+      load_settings(file, plugin: file.split("/")[-3])
     end
   end
 

app/serializers/admin_plugin_serializer.rb

@@ -8,11 +8,10 @@ class AdminPluginSerializer < ApplicationSerializer
              :admin_route,
              :enabled,
              :enabled_setting,
-             :is_official,
+             :is_official
-             :enabled_setting_filter
 
   def id
-    object.metadata.name
+    object.directory_name
   end
 
   def name
@@ -39,14 +38,6 @@ class AdminPluginSerializer < ApplicationSerializer
     object.enabled_site_setting
   end
 
-  def include_enabled_setting_filter?
-    object.enabled_site_setting_filter.present?
-  end
-
-  def enabled_setting_filter
-    object.enabled_site_setting_filter
-  end
-
   def include_url?
     url.present?
   end

lib/discourse.rb

@@ -801,7 +801,7 @@ module Discourse
     redis_key = "deprecate-notice-#{digest}"
 
     if !Discourse.redis.without_namespace.get(redis_key)
-      Rails.logger.warn(warning)
+      Rails.logger.warn(warning) if Rails.logger
       begin
         Discourse.redis.without_namespace.setex(redis_key, 3600, "x")
       rescue Redis::CommandError => e

lib/plugin/instance.rb

@@ -626,11 +626,7 @@ class Plugin::Instance
   end
 
   def enabled_site_setting_filter(filter = nil)
-    if filter
+    Discourse.deprecate("`enabled_site_setting_filter` is deprecated", output_in_test: true)
-      @enabled_setting_filter = filter
-    else
-      @enabled_setting_filter
-    end
   end
 
   def enabled_site_setting(setting = nil)

lib/site_setting_extension.rb

@@ -110,6 +110,10 @@ module SiteSettingExtension
     @secret_settings ||= []
   end
 
+  def plugins
+    @plugins ||= {}
+  end
+
   def setting(name_arg, default = nil, opts = {})
     name = name_arg.to_sym
 
@@ -158,6 +162,10 @@ module SiteSettingExtension
         secret_settings << name
       end
 
+      if opts[:plugin]
+        plugins[name] = opts[:plugin]
+      end
+
       type_supervisor.load_setting(
         name,
         opts.extract!(*SiteSettings::TypeSupervisor::CONSUMED_OPTS)
@@ -246,6 +254,8 @@ module SiteSettingExtension
         placeholder: placeholder(s)
       }.merge!(type_hash)
 
+      opts[:plugin] = plugins[s] if plugins[s]
+
       opts
     end.unshift(locale_setting_hash)
   end

spec/components/plugin/instance_spec.rb

@@ -502,21 +502,6 @@ describe Plugin::Instance do
     end
   end
 
-  describe '#enabled_site_setting_filter' do
-    describe 'when filter is blank' do
-      it 'should return the right value' do
-        expect(Plugin::Instance.new.enabled_site_setting_filter).to eq(nil)
-      end
-    end
-
-    it 'should set the right value' do
-      instance = Plugin::Instance.new
-      instance.enabled_site_setting_filter('test')
-
-      expect(instance.enabled_site_setting_filter).to eq('test')
-    end
-  end
-
   describe '#register_reviewable_types' do
     it 'Overrides the existing Reviewable types adding new ones' do
       current_types = Reviewable.types

test/javascripts/acceptance/admin-site-settings-test.js

@@ -10,7 +10,7 @@ acceptance("Admin - Site Settings", {
   },
 
   pretend(server, helper) {
-    server.put("/admin/site_settings/title", body => {
+    server.put("/admin/site_settings/title", (body) => {
       titleOverride = body.requestBody.split("=")[1];
       return helper.response({ success: "OK" });
     });
@@ -22,14 +22,14 @@ acceptance("Admin - Site Settings", {
         titleSetting.value = titleOverride;
       }
       const response = {
-        site_settings: [titleSetting, ...fixtures.slice(1)]
+        site_settings: [titleSetting, ...fixtures.slice(1)],
       };
       return helper.response(response);
     });
-  }
+  },
 });
 
-QUnit.test("upload site setting", async assert => {
+QUnit.test("upload site setting", async (assert) => {
   await visit("/admin/site_settings");
 
   assert.ok(
@@ -40,7 +40,7 @@ QUnit.test("upload site setting", async assert => {
   assert.ok(exists(".row.setting.upload .undo"), "undo button is present");
 });
 
-QUnit.test("changing value updates dirty state", async assert => {
+QUnit.test("changing value updates dirty state", async (assert) => {
   await visit("/admin/site_settings");
   await fillIn("#setting-filter", " title ");
   assert.equal(count(".row.setting"), 1, "filter returns 1 site setting");
@@ -89,7 +89,7 @@ QUnit.test("changing value updates dirty state", async assert => {
 
 QUnit.test(
   "always shows filtered site settings if a filter is set",
-  async assert => {
+  async (assert) => {
     await visit("/admin/site_settings");
     await fillIn("#setting-filter", "title");
     assert.equal(count(".row.setting"), 1);
@@ -103,3 +103,14 @@ QUnit.test(
     assert.equal(count(".row.setting"), 1);
   }
 );
+
+QUnit.test("filter settings by plugin name", async (assert) => {
+  await visit("/admin/site_settings");
+
+  await fillIn("#setting-filter", "plugin:discourse-logo");
+  assert.equal(count(".row.setting"), 1);
+
+  // inexistent plugin
+  await fillIn("#setting-filter", "plugin:discourse-plugin");
+  assert.equal(count(".row.setting"), 0);
+});

test/javascripts/fixtures/site_settings.js

@@ -9,7 +9,7 @@ export default {
         category: "required",
         preview: null,
         secret: false,
-        type: "string"
+        type: "string",
       },
       {
         setting: "contact_email",
@@ -20,7 +20,7 @@ export default {
         category: "required",
         preview: null,
         secret: false,
-        type: "email"
+        type: "email",
       },
       {
         setting: "site_contact_username",
@@ -31,7 +31,7 @@ export default {
         category: "required",
         preview: null,
         secret: false,
-        type: "username"
+        type: "username",
       },
       {
         setting: "logo",
@@ -41,7 +41,7 @@ export default {
         category: "required",
         preview: null,
         secret: false,
-        type: "upload"
+        type: "upload",
       },
       {
         setting: "top_menu",
@@ -61,13 +61,24 @@ export default {
           "categories",
           "read",
           "posted",
-          "bookmarks"
+          "bookmarks",
         ],
-        list_type: "compact"
+        list_type: "compact",
-      }
+      },
+      {
+        setting: "plugin_logo",
+        description: "Some plugin logo",
+        default: "",
+        value: "/some/image",
+        category: "required",
+        preview: null,
+        secret: false,
+        type: "upload",
+        plugin: "discourse-logo",
+      },
     ],
     diags: {
-      last_message_processed: null
+      last_message_processed: null,
-    }
+    },
-  }
+  },
 };