DEV: Move about_stat_groups to DiscoursePluginRegistry (#20496)

Follow up to 098ab29d41. Since
we just used a `cattr_reader` on `About` this was not safe
for multisite, since some sites could have the chat plugin
enabled and some may not. Using `DiscoursePluginRegistry` gets
around this issue, and makes it so the chat stats only show
for a site if `chat_enabled` is true.

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

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

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
 
   ##

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

spec/models/about_spec.rb

@@ -5,12 +5,19 @@ RSpec.describe About do
     include_examples "stats cacheable"
   end
 
-  describe "#stats" do
+  def register_about_stat_group(name, stats_block, show_in_ui: true)
-    after { About.clear_plugin_stat_groups }
+    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 }
+      register_about_stat_group("some_group", Proc.new { stats })
-      About.add_plugin_stat_group("other_group", show_in_ui: true) { raise StandardError }
+      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