diff --git a/app/models/about.rb b/app/models/about.rb index 1901fccc663..7252c71ab07 100644 --- a/app/models/about.rb +++ b/app/models/about.rb @@ -1,24 +1,13 @@ # frozen_string_literal: true class About - cattr_reader :plugin_stat_groups - - def self.add_plugin_stat_group(prefix, show_in_ui: false, &block) - @@displayed_plugin_stat_groups << prefix if show_in_ui - @@plugin_stat_groups[prefix] = block - end - - def self.clear_plugin_stat_groups - @@displayed_plugin_stat_groups = Set.new - @@plugin_stat_groups = {} - end - def self.displayed_plugin_stat_groups - @@displayed_plugin_stat_groups.to_a + DiscoursePluginRegistry + .about_stat_groups + .select { |stat_group| stat_group[:show_in_ui] } + .map { |stat_group| stat_group[:name] } end - clear_plugin_stat_groups - class CategoryMods include ActiveModel::Serialization attr_reader :category_id, :moderators @@ -103,13 +92,13 @@ class About def plugin_stats final_plugin_stats = {} - @@plugin_stat_groups.each do |plugin_stat_group_name, stat_group| + DiscoursePluginRegistry.about_stat_groups.each do |stat_group| begin - stats = stat_group.call + stats = stat_group[:block].call rescue StandardError => err Discourse.warn_exception( err, - message: "Unexpected error when collecting #{plugin_stat_group_name} About stats.", + message: "Unexpected error when collecting #{stat_group[:name]} About stats.", ) next end @@ -117,11 +106,11 @@ class About if !stats.key?(:last_day) || !stats.key?("7_days") || !stats.key?("30_days") || !stats.key?(:count) Rails.logger.warn( - "Plugin stat group #{plugin_stat_group_name} for About stats does not have all required keys, skipping.", + "Plugin stat group #{stat_group[:name]} for About stats does not have all required keys, skipping.", ) else final_plugin_stats.merge!( - stats.transform_keys { |key| "#{plugin_stat_group_name}_#{key}".to_sym }, + stats.transform_keys { |key| "#{stat_group[:name]}_#{key}".to_sym }, ) end end diff --git a/lib/discourse_plugin_registry.rb b/lib/discourse_plugin_registry.rb index 40cd5a061c5..ef05d3845ab 100644 --- a/lib/discourse_plugin_registry.rb +++ b/lib/discourse_plugin_registry.rb @@ -108,6 +108,8 @@ class DiscoursePluginRegistry define_filtered_register :search_groups_set_query_callbacks + define_filtered_register :about_stat_groups + 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 77f9e4ff2c7..26f9b4c5091 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -1129,7 +1129,17 @@ class Plugin::Instance # table. Some stats may be needed purely for reporting purposes and thus # do not need to be shown in the UI to admins/users. def register_about_stat_group(plugin_stat_group_name, show_in_ui: false, &block) - About.add_plugin_stat_group(plugin_stat_group_name, show_in_ui: show_in_ui, &block) + # We do not want to register and display the same group multiple times. + if DiscoursePluginRegistry.about_stat_groups.any? { |stat_group| + stat_group[:name] == plugin_stat_group_name + } + return + end + + DiscoursePluginRegistry.register_about_stat_group( + { name: plugin_stat_group_name, show_in_ui: show_in_ui, block: block }, + self, + ) end ## diff --git a/spec/lib/plugin/instance_spec.rb b/spec/lib/plugin/instance_spec.rb index cb9314464ad..4f69c994f39 100644 --- a/spec/lib/plugin/instance_spec.rb +++ b/spec/lib/plugin/instance_spec.rb @@ -769,7 +769,7 @@ RSpec.describe Plugin::Instance do describe "#register_about_stat_group" do let(:plugin) { Plugin::Instance.new } - after { About.clear_plugin_stat_groups } + after { DiscoursePluginRegistry.reset! } it "registers an about stat group correctly" do stats = { :last_day => 1, "7_days" => 10, "30_days" => 100, :count => 1000 } @@ -789,6 +789,13 @@ RSpec.describe Plugin::Instance do plugin.register_about_stat_group("some_group") { stats } expect(About.displayed_plugin_stat_groups).to eq([]) end + + it "does not allow duplicate named stat groups" do + stats = { :last_day => 1, "7_days" => 10, "30_days" => 100, :count => 1000 } + plugin.register_about_stat_group("some_group") { stats } + plugin.register_about_stat_group("some_group") { stats } + expect(DiscoursePluginRegistry.about_stat_groups.count).to eq(1) + end end describe "#register_user_destroyer_on_content_deletion_callback" do diff --git a/spec/models/about_spec.rb b/spec/models/about_spec.rb index 0bb92230c25..79c1a1ea4c4 100644 --- a/spec/models/about_spec.rb +++ b/spec/models/about_spec.rb @@ -5,12 +5,19 @@ RSpec.describe About do include_examples "stats cacheable" end - describe "#stats" do - after { About.clear_plugin_stat_groups } + def register_about_stat_group(name, stats_block, show_in_ui: true) + DiscoursePluginRegistry.register_about_stat_group( + { name: name, show_in_ui: show_in_ui, block: stats_block }, + stub(enabled?: true), + ) + end + after { DiscoursePluginRegistry.reset! } + + describe "#stats" do it "adds plugin stats to the output" do stats = { :last_day => 1, "7_days" => 10, "30_days" => 100, :count => 1000 } - About.add_plugin_stat_group("some_group", show_in_ui: true) { stats } + register_about_stat_group("some_group", Proc.new { stats }) expect(described_class.new.stats.with_indifferent_access).to match( hash_including( some_group_last_day: 1, @@ -23,7 +30,7 @@ RSpec.describe About do it "does not add plugin stats to the output if they are missing one of the required keys" do stats = { "7_days" => 10, "30_days" => 100, :count => 1000 } - About.add_plugin_stat_group("some_group", show_in_ui: true) { stats } + register_about_stat_group("some_group", Proc.new { stats }) expect(described_class.new.stats).not_to match( hash_including( some_group_last_day: 1, @@ -36,8 +43,8 @@ RSpec.describe About do it "does not error if any of the plugin stat blocks throw an error and still adds the non-errored stats to output" do stats = { :last_day => 1, "7_days" => 10, "30_days" => 100, :count => 1000 } - About.add_plugin_stat_group("some_group", show_in_ui: true) { stats } - About.add_plugin_stat_group("other_group", show_in_ui: true) { raise StandardError } + register_about_stat_group("some_group", Proc.new { stats }) + register_about_stat_group("other_group", Proc.new { raise StandardError }) expect(described_class.new.stats.with_indifferent_access).to match( hash_including( some_group_last_day: 1, @@ -48,13 +55,6 @@ RSpec.describe About do ) expect { described_class.new.stats.with_indifferent_access }.not_to raise_error end - - it "does not allow duplicate displayed stat groups" do - stats = { :last_day => 1, "7_days" => 10, "30_days" => 100, :count => 1000 } - About.add_plugin_stat_group("some_group", show_in_ui: true) { stats } - About.add_plugin_stat_group("some_group", show_in_ui: true) { stats } - expect(described_class.displayed_plugin_stat_groups).to eq(["some_group"]) - end end describe "#category_moderators" do