diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index f46905217e3..d7ec2260fca 100644 --- a/app/models/reviewable.rb +++ b/app/models/reviewable.rb @@ -66,14 +66,11 @@ class Reviewable < ActiveRecord::Base end def self.valid_type?(type) - return false if Reviewable.types.exclude?(type) - type.constantize <= Reviewable - rescue NameError - false + type.to_s.safe_constantize.in?(types) end def self.types - %w[ReviewableFlaggedPost ReviewableQueuedPost ReviewableUser ReviewablePost] + [ReviewableFlaggedPost, ReviewableQueuedPost, ReviewableUser, ReviewablePost] end def self.custom_filters diff --git a/lib/discourse_plugin_registry.rb b/lib/discourse_plugin_registry.rb index cd4dd47b589..ee2c9786c2a 100644 --- a/lib/discourse_plugin_registry.rb +++ b/lib/discourse_plugin_registry.rb @@ -4,6 +4,8 @@ # A class that handles interaction between a plugin and the Discourse App. # class DiscoursePluginRegistry + @@register_names = Set.new + # Plugins often need to be able to register additional handlers, data, or # classes that will be used by core classes. This should be used if you # need to control which type the registry is, and if it doesn't need to @@ -15,7 +17,7 @@ class DiscoursePluginRegistry # - Defines instance method as a shortcut to the singleton method # - Automatically deletes the register on registry.reset! def self.define_register(register_name, type) - @@register_names ||= Set.new + return if respond_to?(register_name) @@register_names << register_name define_singleton_method(register_name) do @@ -36,13 +38,13 @@ class DiscoursePluginRegistry # - Defines instance method as a shortcut to the singleton method # - Automatically deletes the register on registry.reset! def self.define_filtered_register(register_name) + return if respond_to?(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 + public_send(:"_raw_#{register_name}").filter_map { |h| h[:value] if h[:plugin].enabled? }.uniq end define_singleton_method("register_#{register_name.to_s.singularize}") do |value, plugin| diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 058248f1241..10bbd34c802 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -831,14 +831,28 @@ class Plugin::Instance end def register_reviewable_type(reviewable_type_class) - extend_list_method Reviewable, :types, [reviewable_type_class.name] + return unless reviewable_type_class < Reviewable + extend_list_method(Reviewable, :types, reviewable_type_class) end def extend_list_method(klass, method, new_attributes) - current_list = klass.public_send(method) - current_list.concat(new_attributes) + register_name = [klass, method].join("_").underscore + DiscoursePluginRegistry.define_filtered_register(register_name) + DiscoursePluginRegistry.public_send( + "register_#{register_name.singularize}", + new_attributes, + self, + ) - reloadable_patch { klass.public_send(:define_singleton_method, method) { current_list } } + original_method_alias = "__original_#{method}__" + return if klass.respond_to?(original_method_alias) + reloadable_patch do + klass.singleton_class.alias_method(original_method_alias, method) + klass.define_singleton_method(method) do + public_send(original_method_alias) | + DiscoursePluginRegistry.public_send(register_name).flatten + end + end end def directory_name diff --git a/spec/lib/plugin/instance_spec.rb b/spec/lib/plugin/instance_spec.rb index 7bf68f1d8b9..2610e269076 100644 --- a/spec/lib/plugin/instance_spec.rb +++ b/spec/lib/plugin/instance_spec.rb @@ -599,25 +599,60 @@ TEXT end end - describe "#register_reviewable_types" do - it "Overrides the existing Reviewable types adding new ones" do - current_types = Reviewable.types - new_type_class = Class + describe "#register_reviewable_type" do + subject(:register_reviewable_type) { plugin_instance.register_reviewable_type(new_type) } - Plugin::Instance.new.register_reviewable_type new_type_class + context "when the provided class inherits from `Reviewable`" do + let(:new_type) { Class.new(Reviewable) } - expect(Reviewable.types).to match_array(current_types << new_type_class.name) + it "adds the provided class to the existing types" do + expect { register_reviewable_type }.to change { Reviewable.types.size }.by(1) + expect(Reviewable.types).to include(new_type) + end + + context "when the plugin is disabled" do + before do + register_reviewable_type + plugin_instance.stubs(:enabled?).returns(false) + end + + it "does not return the new type" do + expect(Reviewable.types).not_to be_blank + expect(Reviewable.types).not_to include(new_type) + end + end + end + + context "when the provided class does not inherit from `Reviewable`" do + let(:new_type) { Class } + + it "does not add the provided class to the existing types" do + expect { register_reviewable_type }.not_to change { Reviewable.types } + expect(Reviewable.types).not_to be_blank + end end end describe "#extend_list_method" do - it "Overrides the existing list appending new elements" do - current_list = Reviewable.types - new_element = Class.name + subject(:extend_list) do + plugin_instance.extend_list_method(UserHistory, :staff_actions, %i[new_action another_action]) + end - Plugin::Instance.new.extend_list_method Reviewable, :types, [new_element] + it "adds the provided values to the provided method on the provided class" do + expect { extend_list }.to change { UserHistory.staff_actions.size }.by(2) + expect(UserHistory.staff_actions).to include(:new_action, :another_action) + end - expect(Reviewable.types).to match_array(current_list << new_element) + context "when the plugin is disabled" do + before do + extend_list + plugin_instance.stubs(:enabled?).returns(false) + end + + it "does not return the provided values" do + expect(UserHistory.staff_actions).not_to be_blank + expect(UserHistory.staff_actions).not_to include(:new_action, :another_action) + end end end