diff --git a/app/assets/javascripts/admin/controllers/admin-site-settings.js b/app/assets/javascripts/admin/controllers/admin-site-settings.js index ac84553a71c..e3225b163dd 100644 --- a/app/assets/javascripts/admin/controllers/admin-site-settings.js +++ b/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 ( diff --git a/app/assets/javascripts/admin/routes/admin-plugins.js b/app/assets/javascripts/admin/routes/admin-plugins.js index c4af286fafb..261610a97e6 100644 --- a/app/assets/javascripts/admin/routes/admin-plugins.js +++ b/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"); - const match = /^(.*)_enabled/.exec(plugin.get("enabled_setting")); - const filter = siteSettingFilter || match[1]; - - if (filter) { - // filterContent() is normally on a debounce from typing. - // 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"); - } + // filterContent() is normally on a debounce from typing. + // Because we don't want the default of "All Results", we tell it + // to skip the next debounce. + controller.set("filter", `plugin:${plugin.id}`); + controller.set("_skipBounce", true); + controller.filterContentNow("plugins"); } }); } diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 7925f91ef1c..bc966333598 100644 --- a/app/models/site_setting.rb +++ b/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 diff --git a/app/serializers/admin_plugin_serializer.rb b/app/serializers/admin_plugin_serializer.rb index 3b8fc0e323c..843ca584087 100644 --- a/app/serializers/admin_plugin_serializer.rb +++ b/app/serializers/admin_plugin_serializer.rb @@ -8,11 +8,10 @@ class AdminPluginSerializer < ApplicationSerializer :admin_route, :enabled, :enabled_setting, - :is_official, - :enabled_setting_filter + :is_official 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 diff --git a/lib/discourse.rb b/lib/discourse.rb index 9bdbc2b3024..649fb00c630 100644 --- a/lib/discourse.rb +++ b/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 diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 13064a7e238..006aa310b04 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -626,11 +626,7 @@ class Plugin::Instance end def enabled_site_setting_filter(filter = nil) - if filter - @enabled_setting_filter = filter - else - @enabled_setting_filter - end + Discourse.deprecate("`enabled_site_setting_filter` is deprecated", output_in_test: true) end def enabled_site_setting(setting = nil) diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 64fe9d6d443..09b616eda99 100644 --- a/lib/site_setting_extension.rb +++ b/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 diff --git a/spec/components/plugin/instance_spec.rb b/spec/components/plugin/instance_spec.rb index 26dd0d39c7f..1c5bee70669 100644 --- a/spec/components/plugin/instance_spec.rb +++ b/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 diff --git a/test/javascripts/acceptance/admin-site-settings-test.js b/test/javascripts/acceptance/admin-site-settings-test.js index b0975a9ae78..f02d093b392 100644 --- a/test/javascripts/acceptance/admin-site-settings-test.js +++ b/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); +}); diff --git a/test/javascripts/fixtures/site_settings.js b/test/javascripts/fixtures/site_settings.js index cf48b6c10be..c31d4c84120 100644 --- a/test/javascripts/fixtures/site_settings.js +++ b/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, + }, + }, };