DEV: Promote block problem checks to ProblemCheck (#26193)

In #26122 we promoted all problem checks defined as class methods on AdminDashboardData to their own first-class ProblemCheck instances.

This PR continues that by promoting problem checks that are implemented as blocks as well. This includes updating a couple plugins that have problem checks.

app/models/admin_dashboard_data.rb

@@ -3,7 +3,7 @@
 class AdminDashboardData
   include StatsCacheable
 
-  cattr_reader :problem_blocks, :problem_messages
+  cattr_reader :problem_messages
 
   # kept for backward compatibility
   GLOBAL_REPORTS ||= []
@@ -25,10 +25,6 @@ class AdminDashboardData
 
   def problems
     problems = []
-    self.class.problem_blocks.each do |blk|
-      message = instance_exec(&blk)
-      problems << ProblemCheck::Problem.new(message) if message.present?
-    end
     self.class.problem_messages.each do |i18n_key|
       message = self.class.problem_message_check(i18n_key)
       problems << ProblemCheck::Problem.new(message) if message.present?
@@ -48,7 +44,10 @@ class AdminDashboardData
   end
 
   def self.add_problem_check(*syms, &blk)
-    @@problem_blocks << blk if blk
+    Discourse.deprecate(
+      "`AdminDashboardData#add_problem_check` is deprecated. Implement a class that inherits `ProblemCheck` instead.",
+      drop_from: "3.3",
+    )
   end
 
   def self.add_found_scheduled_check_problem(problem)
@@ -101,15 +100,11 @@ class AdminDashboardData
   # tests. It will also fire multiple times in development mode because
   # classes are not cached.
   def self.reset_problem_checks
-    @@problem_blocks = []
-
     @@problem_messages = %w[
       dashboard.bad_favicon_url
       dashboard.poll_pop3_timeout
       dashboard.poll_pop3_auth_error
     ]
-
-    add_problem_check { sidekiq_check || queue_size_check }
   end
   reset_problem_checks
 
@@ -175,16 +170,4 @@ class AdminDashboardData
   def self.problem_message_key(i18n_key)
     "#{PROBLEM_MESSAGE_PREFIX}#{i18n_key}"
   end
-
-  def sidekiq_check
-    last_job_performed_at = Jobs.last_job_performed_at
-    if Jobs.queued > 0 && (last_job_performed_at.nil? || last_job_performed_at < 2.minutes.ago)
-      I18n.t("dashboard.sidekiq_warning")
-    end
-  end
-
-  def queue_size_check
-    queue_size = Jobs.queued
-    I18n.t("dashboard.queue_size_warning", queue_size: queue_size) if queue_size >= 100_000
-  end
 end

app/models/problem_check.rb

@@ -70,14 +70,14 @@ class ProblemCheck
 
   private
 
-  def problem(override_key = nil)
+  def problem(override_key = nil, override_data = {})
     [
       Problem.new(
         message ||
           I18n.t(
             override_key || translation_key,
             base_path: Discourse.base_path,
-            **translation_data.symbolize_keys,
+            **override_data.merge(translation_data).symbolize_keys,
           ),
         priority: self.config.priority,
         identifier:,

app/services/problem_check/sidekiq_check.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+class ProblemCheck::SidekiqCheck < ProblemCheck
+  self.priority = "low"
+
+  def call
+    return problem("dashboard.sidekiq_warning") if jobs_in_queue? && !jobs_performed_recently?
+    return problem("dashboard.queue_size_warning", queue_size: Jobs.queued) if massive_queue?
+
+    no_problem
+  end
+
+  private
+
+  def massive_queue?
+    Jobs.queued >= 100_000
+  end
+
+  def jobs_in_queue?
+    Jobs.queued > 0
+  end
+
+  def jobs_performed_recently?
+    Jobs.last_job_performed_at.present? && Jobs.last_job_performed_at > 2.minutes.ago
+  end
+end

spec/models/admin_dashboard_data_spec.rb

@@ -22,15 +22,6 @@ RSpec.describe AdminDashboardData do
         expect(problems).not_to include(I18n.t("errors.messages.invalid"))
       end
     end
-
-    describe "adding new checks" do
-      it "calls the passed block" do
-        AdminDashboardData.add_problem_check { "a problem was found" }
-
-        problems = AdminDashboardData.fetch_problems
-        expect(problems.map(&:to_s)).to include("a problem was found")
-      end
-    end
   end
 
   describe "adding scheduled checks" do
@@ -91,38 +82,4 @@ RSpec.describe AdminDashboardData do
       )
     end
   end
-
-  describe "sidekiq_check" do
-    subject(:check) { described_class.new.sidekiq_check }
-
-    it "returns nil when sidekiq processed a job recently" do
-      Jobs.stubs(:last_job_performed_at).returns(1.minute.ago)
-      Jobs.stubs(:queued).returns(0)
-      expect(check).to be_nil
-    end
-
-    it "returns nil when last job processed was a long time ago, but no jobs are queued" do
-      Jobs.stubs(:last_job_performed_at).returns(7.days.ago)
-      Jobs.stubs(:queued).returns(0)
-      expect(check).to be_nil
-    end
-
-    it "returns nil when no jobs have ever been processed, but no jobs are queued" do
-      Jobs.stubs(:last_job_performed_at).returns(nil)
-      Jobs.stubs(:queued).returns(0)
-      expect(check).to be_nil
-    end
-
-    it "returns a string when no jobs were processed recently and some jobs are queued" do
-      Jobs.stubs(:last_job_performed_at).returns(20.minutes.ago)
-      Jobs.stubs(:queued).returns(1)
-      expect(check).to_not be_nil
-    end
-
-    it "returns a string when no jobs have ever been processed, and some jobs are queued" do
-      Jobs.stubs(:last_job_performed_at).returns(nil)
-      Jobs.stubs(:queued).returns(1)
-      expect(check).to_not be_nil
-    end
-  end
 end

spec/services/problem_check/sidekiq_check_spec.rb

@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+RSpec.describe ProblemCheck::SidekiqCheck do
+  subject(:check) { described_class.new }
+
+  describe ".call" do
+    context "when Sidekiq processed a job recently" do
+      before do
+        Jobs.stubs(:last_job_performed_at).returns(1.minute.ago)
+        Jobs.stubs(:queued).returns(1)
+      end
+
+      it { expect(check).to be_chill_about_it }
+    end
+
+    context "when last job processed was a long time ago, but no jobs are queued" do
+      before do
+        Jobs.stubs(:last_job_performed_at).returns(7.days.ago)
+        Jobs.stubs(:queued).returns(0)
+      end
+
+      it { expect(check).to be_chill_about_it }
+    end
+
+    context "when no jobs have ever been processed, but no jobs are queued" do
+      before do
+        Jobs.stubs(:last_job_performed_at).returns(nil)
+        Jobs.stubs(:queued).returns(0)
+      end
+
+      it { expect(check).to be_chill_about_it }
+    end
+
+    context "when no jobs were processed recently and some jobs are queued" do
+      before do
+        Jobs.stubs(:last_job_performed_at).returns(20.minutes.ago)
+        Jobs.stubs(:queued).returns(1)
+      end
+
+      it do
+        expect(check).to have_a_problem.with_priority("low").with_message(
+          'Sidekiq is not running. Many tasks, like sending emails, are executed asynchronously by Sidekiq. Please ensure at least one Sidekiq process is running. <a href="https://github.com/mperham/sidekiq" target="_blank">Learn about Sidekiq here</a>.',
+        )
+      end
+    end
+
+    context "when no jobs have ever been processed, and some jobs are queued" do
+      before do
+        Jobs.stubs(:last_job_performed_at).returns(nil)
+        Jobs.stubs(:queued).returns(1)
+      end
+
+      it do
+        expect(check).to have_a_problem.with_priority("low").with_message(
+          'Sidekiq is not running. Many tasks, like sending emails, are executed asynchronously by Sidekiq. Please ensure at least one Sidekiq process is running. <a href="https://github.com/mperham/sidekiq" target="_blank">Learn about Sidekiq here</a>.',
+        )
+      end
+    end
+
+    context "when there's a massive pile-up in the queue" do
+      before do
+        Jobs.stubs(:last_job_performed_at).returns(1.second.ago)
+        Jobs.stubs(:queued).returns(100_000)
+      end
+
+      it do
+        expect(check).to have_a_problem.with_priority("low").with_message(
+          "The number of queued jobs is 100000, which is high. This could indicate a problem with the Sidekiq process(es), or you may need to add more Sidekiq workers.",
+        )
+      end
+    end
+  end
+end