DEV: Add framework for filtered plugin registers (#9763)

DEV: Add framework for filtered plugin registers (#9763)

  • DEV: Add framework for filtered plugin registers

Plugins often need to add values to a list, and we need to filter those lists at runtime to ignore values from disabled plugins. This commit provides a re-usable way to do that, which should make it easier to add new registers in future, and also reduce repeated code.

Follow-up commits will migrate existing registers to use this new system

  • DEV: Migrate user and group custom field APIs to plugin registry

This gives us a consistent system for checking plugin enabled state, so we are repeating less logic. API changes are backwards compatible

diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb
index a8413eb..50cef29 100644
--- a/app/controllers/admin/groups_controller.rb
+++ b/app/controllers/admin/groups_controller.rb
@@ -177,7 +177,7 @@ class Admin::GroupsController < Admin::AdminController
       :usernames,
       :publish_read_state
     ]
-    custom_fields = Group.editable_group_custom_fields
+    custom_fields = DiscoursePluginRegistry.editable_group_custom_fields
     permitted << { custom_fields: custom_fields } unless custom_fields.blank?
 
     params.require(:group).permit(permitted)
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index bb76381..2716282 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -545,7 +545,7 @@ class GroupsController < ApplicationController
             :publish_read_state
           ])
 
-          custom_fields = Group.editable_group_custom_fields
+          custom_fields = DiscoursePluginRegistry.editable_group_custom_fields
           default_params << { custom_fields: custom_fields } unless custom_fields.blank?
         end
 
diff --git a/app/models/group.rb b/app/models/group.rb
index 5bc41d7..80c6080 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -266,19 +266,9 @@ class Group < ActiveRecord::Base
     end
   end
 
-  def self.plugin_editable_group_custom_fields
-    @plugin_editable_group_custom_fields ||= {}
-  end
-
   def self.register_plugin_editable_group_custom_field(custom_field_name, plugin)
-    plugin_editable_group_custom_fields[custom_field_name] = plugin
-  end
-
-  def self.editable_group_custom_fields
-    plugin_editable_group_custom_fields.reduce([]) do |fields, (k, v)|
-      next(fields) unless v.enabled?
-      fields << k
-    end.uniq
+    Discourse.deprecate("Editable group custom fields should be registered using the plugin API", since: "v2.4.0.beta4", drop_from: "v2.5.0")
+    DiscoursePluginRegistry.register_editable_group_custom_field(custom_field_name, plugin)
   end
 
   def downcase_incoming_email
diff --git a/app/models/user.rb b/app/models/user.rb
index f932f15..9d682a6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -267,72 +267,44 @@ class User < ActiveRecord::Base
     end
   end
 
-  def self.plugin_editable_user_custom_fields
-    @plugin_editable_user_custom_fields ||= {}
-  end
-
-  def self.plugin_staff_editable_user_custom_fields
-    @plugin_staff_editable_user_custom_fields ||= {}
-  end
-
-  def self.register_plugin_editable_user_custom_field(custom_field_name, plugin, staff_only: false)
-    if staff_only
-      plugin_staff_editable_user_custom_fields[custom_field_name] = plugin
-    else
-      plugin_editable_user_custom_fields[custom_field_name] = plugin
-    end
-  end
-
   def self.editable_user_custom_fields(by_staff: false)
     fields = []
-
-    plugin_editable_user_custom_fields.each do |k, v|
-      fields << k if v.enabled?
-    end
-
-    if by_staff
-      plugin_staff_editable_user_custom_fields.each do |k, v|
-        fields << k if v.enabled?
-      end
-    end
+    fields.push *DiscoursePluginRegistry.self_editable_user_custom_fields
+    fields.push *DiscoursePluginRegistry.staff_editable_user_custom_fields if by_staff
 
     fields.uniq
   end
 
-  def self.plugin_staff_user_custom_fields
-    @plugin_staff_user_custom_fields ||= {}
+  def self.register_plugin_editable_user_custom_field(custom_field_name, plugin, staff_only: false)
+    Discourse.deprecate("Editable user custom fields should be registered using the plugin API", since: "v2.4.0.beta4", drop_from: "v2.5.0")
+    DiscoursePluginRegistry.register_editable_user_custom_field(custom_field_name, plugin, staff_only: staff_only)
   end
 
   def self.register_plugin_staff_custom_field(custom_field_name, plugin)
-    plugin_staff_user_custom_fields[custom_field_name] = plugin
-  end
-
-  def self.plugin_public_user_custom_fields
-    @plugin_public_user_custom_fields ||= {}
+    Discourse.deprecate("Staff user custom fields should be registered using the plugin API",  since: "v2.4.0.beta4", drop_from: "v2.5.0")
+    DiscoursePluginRegistry.register_staff_user_custom_field(custom_field_name, plugin)
   end
 
   def self.register_plugin_public_custom_field(custom_field_name, plugin)
-    plugin_public_user_custom_fields[custom_field_name] = plugin
+    Discourse.deprecate("Public user custom fields should be registered using the plugin API", since: "v2.4.0.beta4", drop_from: "v2.5.0")
+    DiscoursePluginRegistry.register_public_user_custom_field(custom_field_name, plugin)
   end
 
   def self.whitelisted_user_custom_fields(guardian)
     fields = []
 
-    plugin_public_user_custom_fields.each do |k, v|
-      fields << k if v.enabled?
-    end
+    fields.push *DiscoursePluginRegistry.public_user_custom_fields
 
     if SiteSetting.public_user_custom_fields.present?
-      fields += SiteSetting.public_user_custom_fields.split('|')
+      fields.push *SiteSetting.public_user_custom_fields.split('|')
     end
 
     if guardian.is_staff?
       if SiteSetting.staff_user_custom_fields.present?
-        fields += SiteSetting.staff_user_custom_fields.split('|')
-      end
-      plugin_staff_user_custom_fields.each do |k, v|
-        fields << k if v.enabled?
+        fields.push *SiteSetting.staff_user_custom_fields.split('|')
       end
+
+      fields.push *DiscoursePluginRegistry.staff_user_custom_fields
     end
 
     fields.uniq
diff --git a/lib/discourse_plugin_registry.rb b/lib/discourse_plugin_registry.rb
index acd35ec..ae260e2 100644
--- a/lib/discourse_plugin_registry.rb
+++ b/lib/discourse_plugin_registry.rb
@@ -24,6 +24,29 @@ class DiscoursePluginRegistry
     end
   end
 
+  # Create a new register (see `define_register`) with some additions:
+  #   - Register is created in a class variable using the specified name/type
+  #   - Defines singleton method to access the register
+  #   - Defines instance method as a shortcut to the singleton method
+  #   - Automatically deletes the register on ::clear!
+  def self.define_filtered_register(register_name)
+    define_register(register_name, Array)
+
+    singleton_class.alias_method :"_raw_#{register_name}", :"#{register_name}"
+
+    define_singleton_method(register_name) do
+      unfiltered = public_send(:"_raw_#{register_name}")
+      unfiltered
+        .filter { |v| v[:plugin].enabled? }
+        .map { |v| v[:value] }
+        .uniq
+    end
+
+    define_singleton_method("register_#{register_name.to_s.singularize}") do |value, plugin|
+      public_send(:"_raw_#{register_name}") << { plugin: plugin, value: value }
+    end
+  end
+
   define_register :javascripts, Set
   define_register :auth_providers, Set
   define_register :service_workers, Set
@@ -44,6 +67,14 @@ class DiscoursePluginRegistry
   define_register :vendored_pretty_text, Set
   define_register :vendored_core_pretty_text, Set
 
+  define_filtered_register :staff_user_custom_fields
+  define_filtered_register :public_user_custom_fields
+
+  define_filtered_register :self_editable_user_custom_fields
+  define_filtered_register :staff_editable_user_custom_fields
+
+  define_filtered_register :editable_group_custom_fields
+
   def self.register_auth_provider(auth_provider)
     self.auth_providers << auth_provider
   end
diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb
index 647a453..a993ce5 100644
--- a/lib/plugin/instance.rb
+++ b/lib/plugin/instance.rb
@@ -146,27 +146,23 @@ class Plugin::Instance
   end
 
   def whitelist_staff_user_custom_field(field)
-    reloadable_patch do |plugin|
-      ::User.register_plugin_staff_custom_field(field, plugin) # plugin.enabled? is checked at runtime
-    end
+    DiscoursePluginRegistry.register_staff_user_custom_field(field, self)
   end
 
   def whitelist_public_user_custom_field(field)
-    reloadable_patch do |plugin|

[... diff too long, it was truncated ...]

GitHub sha: 461b4e5c

1 Like

This commit appears in #9763 which was approved by eviltrout. It was merged by davidtaylorhq.