Revert "DEV: Add configurable? helper to Plugin::Instance (#20767)" (#21469)

This reverts commit 3073e5cfb0721b3b8009963c5fc7f61d414db317.

app/controllers/admin/site_settings_controller.rb

@@ -259,18 +259,10 @@ class Admin::SiteSettingsController < Admin::AdminController
   end
 
   def raise_access_hidden_setting(id)
-    id = id.to_sym
+    # note, as of Ruby 2.3 symbols are GC'd so this is considered safe
-
+    if SiteSetting.hidden_settings.include?(id.to_sym)
-    if SiteSetting.hidden_settings.include?(id)
       raise Discourse::InvalidParameters, "You are not allowed to change hidden settings"
     end
-
-    if SiteSetting.plugins[id]
-      plugin = Discourse.plugins_by_name[SiteSetting.plugins[id]]
-      if !plugin.configurable?
-        raise Discourse::InvalidParameters, "You are not allowed to change unconfigurable settings"
-      end
-    end
   end
 
   def tag_notification_level(id)

app/controllers/application_controller.rb

@@ -346,11 +346,7 @@ class ApplicationController < ActionController::Base
   # disabled. This allows plugins to be disabled programmatically.
   def self.requires_plugin(plugin_name)
     before_action do
-      if plugin = Discourse.plugins_by_name[plugin_name]
+      raise PluginDisabled.new if Discourse.disabled_plugin_names.include?(plugin_name)
-        raise PluginDisabled.new if !plugin.enabled?
-      else
-        Rails.logger.warn("Required plugin '#{plugin_name}' not found")
-      end
     end
   end
 

lib/discourse.rb

@@ -344,7 +344,6 @@ module Discourse
 
   def self.activate_plugins!
     @plugins = []
-    @plugins_by_name = {}
     Plugin::Instance
       .find_all("#{Rails.root}/plugins")
       .each do |p|
@@ -352,7 +351,6 @@ module Discourse
         if Discourse.has_needed_version?(Discourse::VERSION::STRING, v)
           p.activate!
           @plugins << p
-          @plugins_by_name[p.name] = p
         else
           STDERR.puts "Could not activate #{p.metadata.name}, discourse does not meet required version (#{v})"
         end
@@ -360,16 +358,20 @@ module Discourse
     DiscourseEvent.trigger(:after_plugin_activation)
   end
 
+  def self.disabled_plugin_names
+    plugins.select { |p| !p.enabled? }.map(&:name)
+  end
+
   def self.plugins
     @plugins ||= []
   end
 
-  def self.plugins_by_name
+  def self.hidden_plugins
-    @plugins_by_name ||= {}
+    @hidden_plugins ||= []
   end
 
   def self.visible_plugins
-    plugins.filter(&:visible?)
+    self.plugins - self.hidden_plugins
   end
 
   def self.plugin_themes

lib/plugin/instance.rb

@@ -103,16 +103,7 @@ class Plugin::Instance
     @admin_route = { label: label, location: location }
   end
 
-  def configurable?
-    true
-  end
-
-  def visible?
-    configurable? && !@hidden
-  end
-
   def enabled?
-    return false if !configurable?
     @enabled_site_setting ? SiteSetting.get(@enabled_site_setting) : true
   end
 
@@ -834,7 +825,11 @@ class Plugin::Instance
   end
 
   def hide_plugin
-    @hidden = true
+    Discourse.hidden_plugins << self
+  end
+
+  def enabled_site_setting_filter(filter = nil)
+    STDERR.puts("`enabled_site_setting_filter` is deprecated")
   end
 
   def enabled_site_setting(setting = nil)

lib/site_setting_extension.rb

@@ -166,20 +166,6 @@ module SiteSettingExtension
     end
   end
 
-  def remove_setting(name_arg)
-    raise if !Rails.env.test?
-
-    name = name_arg.to_sym
-
-    categories.delete(name)
-    hidden_settings.delete(name)
-    refresh_settings.delete(name)
-    client_settings.delete(name)
-    previews.delete(name)
-    secret_settings.delete(name)
-    plugins.delete(name)
-  end
-
   def settings_hash
     result = {}
     deprecated_settings = Set.new
@@ -238,9 +224,6 @@ module SiteSettingExtension
 
     defaults
       .all(default_locale)
-      .filter do |setting_name, _|
-        !plugins[setting_name] || Discourse.plugins_by_name[plugins[setting_name]].configurable?
-      end
       .reject { |setting_name, _| !include_hidden && hidden_settings.include?(setting_name) }
       .map do |s, v|
         type_hash = type_supervisor.type_hash(s)
@@ -561,11 +544,6 @@ module SiteSettingExtension
       end
     else
       define_singleton_method clean_name do
-        if plugins[name]
-          plugin = Discourse.plugins_by_name[plugins[name]]
-          return false if !plugin.configurable? && plugin.enabled_site_setting == name
-        end
-
         if (c = current[name]).nil?
           refresh!
           current[name]

plugins/poll/plugin.rb

@@ -20,7 +20,7 @@ hide_plugin
 
 after_initialize do
   module ::DiscoursePoll
-    PLUGIN_NAME ||= "poll"
+    PLUGIN_NAME ||= "discourse_poll"
     DATA_PREFIX ||= "data-poll-"
     HAS_POLLS ||= "has_polls"
     DEFAULT_POLL_NAME ||= "poll"

spec/requests/admin/site_settings_controller_spec.rb

@@ -608,53 +608,6 @@ RSpec.describe Admin::SiteSettingsController do
         expect(response.status).to eq(422)
       end
 
-      it "does not allow changing of hidden settings" do
-        SiteSetting.setting(:hidden_setting, "hidden", hidden: true)
-        SiteSetting.refresh!
-
-        put "/admin/site_settings/hidden_setting.json", params: { hidden_setting: "not allowed" }
-
-        expect(SiteSetting.hidden_setting).to eq("hidden")
-        expect(response.status).to eq(422)
-      end
-
-      context "with an plugin" do
-        let(:plugin) do
-          metadata = Plugin::Metadata.new
-          metadata.name = "discourse-plugin"
-          Plugin::Instance.new(metadata)
-        end
-
-        before do
-          Discourse.plugins_by_name[plugin.name] = plugin
-          SiteSetting.setting(:plugin_setting, "default value", plugin: "discourse-plugin")
-          SiteSetting.refresh!
-        end
-
-        after do
-          Discourse.plugins_by_name.delete(plugin.name)
-          SiteSetting.remove_setting(:plugin_setting)
-        end
-
-        it "allows changing settings of configurable plugins" do
-          plugin.stubs(:configurable?).returns(true)
-
-          put "/admin/site_settings/plugin_setting.json", params: { plugin_setting: "new value" }
-
-          expect(SiteSetting.plugin_setting).to eq("new value")
-          expect(response.status).to eq(200)
-        end
-
-        it "does not allow changing of unconfigurable settings" do
-          plugin.stubs(:configurable?).returns(false)
-
-          put "/admin/site_settings/plugin_setting.json", params: { plugin_setting: "not allowed" }
-
-          expect(SiteSetting.plugin_setting).to eq("default value")
-          expect(response.status).to eq(422)
-        end
-      end
-
       it "fails when a setting does not exist" do
         put "/admin/site_settings/provider.json", params: { provider: "gotcha" }
         expect(response.status).to eq(422)