DEV: Promote inline problem checks to problem check classes

app/controllers/static_controller.rb

@@ -162,7 +162,7 @@ def favicon
 
               file&.read || ""
             rescue => e
-              AdminDashboardData.add_problem_message("dashboard.bad_favicon_url", 1800)
+              ProblemCheckTracker[:bad_favicon_url].problem!
               Rails.logger.debug("Failed to fetch favicon #{favicon.url}: #{e}\n#{e.backtrace}")
               ""
             ensure

app/jobs/scheduled/poll_mailbox.rb

@@ -68,7 +68,7 @@ def poll_pop3
       if count > 3
         Discourse.redis.del(POLL_MAILBOX_TIMEOUT_ERROR_KEY)
         mark_as_errored!
-        add_admin_dashboard_problem_message("dashboard.poll_pop3_timeout")
+        track_problem(:poll_pop3_timeout)
         Discourse.handle_job_exception(
           e,
           error_context(
@@ -79,7 +79,7 @@ def poll_pop3
       end
     rescue Net::POPAuthenticationError => e
       mark_as_errored!
-      add_admin_dashboard_problem_message("dashboard.poll_pop3_auth_error")
+      track_problem(:poll_pop3_auth_error)
       Discourse.handle_job_exception(e, error_context(@args, "Signing in to poll incoming emails."))
     end
 
@@ -104,11 +104,8 @@ def mark_as_errored!
       Discourse.redis.zadd(POLL_MAILBOX_ERRORS_KEY, now, now.to_s)
     end
 
-    def add_admin_dashboard_problem_message(i18n_key)
-      AdminDashboardData.add_problem_message(
-        i18n_key,
-        SiteSetting.pop3_polling_period_mins.minutes + 5.minutes,
-      )
+    def track_problem(identifier)
+      ProblemCheckTracker[identifier].problem!
     end
   end
 end

app/models/admin_dashboard_data.rb

@@ -90,24 +90,6 @@ def self.load_found_scheduled_check_problems
     end
   end
 
-  ##
-  # We call this method in the class definition below
-  # so all of the problem checks in this class are registered on
-  # boot. These problem checks are run when the problems are loaded in
-  # the admin dashboard controller.
-  #
-  # This method also can be used in testing to reset checks between
-  # tests. It will also fire multiple times in development mode because
-  # classes are not cached.
-  def self.reset_problem_checks
-    @@problem_messages = %w[
-      dashboard.bad_favicon_url
-      dashboard.poll_pop3_timeout
-      dashboard.poll_pop3_auth_error
-    ]
-  end
-  reset_problem_checks
-
   def self.fetch_stats
     new.as_json
   end

app/models/problem_check.rb

@@ -33,6 +33,7 @@ class ProblemCheck
   # Note: This list must come after the `config_accessor` declarations.
   #
   CORE_PROBLEM_CHECKS = [
+    ProblemCheck::BadFaviconUrl,
     ProblemCheck::EmailPollingErroredRecently,
     ProblemCheck::FacebookConfig,
     ProblemCheck::FailingEmails,
@@ -45,6 +46,8 @@ class ProblemCheck
     ProblemCheck::ImageMagick,
     ProblemCheck::MissingMailgunApiKey,
     ProblemCheck::OutOfDateThemes,
+    ProblemCheck::PollPop3Timeout,
+    ProblemCheck::PollPop3AuthError,
     ProblemCheck::RailsEnv,
     ProblemCheck::Ram,
     ProblemCheck::S3BackupConfig,

app/services/problem_check/bad_favicon_url.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class ProblemCheck::BadFaviconUrl < ProblemCheck::InlineProblemCheck
+  self.priority = "low"
+end

app/services/problem_check/inline_problem_check.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class ProblemCheck::InlineProblemCheck < ProblemCheck
+  def call
+    # The logic of this problem check is performed inline, so this class is
+    # purely here to support its configuration.
+    #
+    no_problem
+  end
+end

app/services/problem_check/poll_pop3_auth_error.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class ProblemCheck::PollPop3AuthError < ProblemCheck::InlineProblemCheck
+  self.priority = "low"
+end

app/services/problem_check/poll_pop3_timeout.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class ProblemCheck::PollPop3Timeout < ProblemCheck::InlineProblemCheck
+  self.priority = "low"
+end

spec/jobs/poll_mailbox_spec.rb

@@ -47,9 +47,7 @@
 
         i18n_key = "dashboard.poll_pop3_auth_error"
 
-        expect(AdminDashboardData.problem_message_check(i18n_key)).to eq(
-          I18n.t(i18n_key, base_path: Discourse.base_path),
-        )
+        expect(ProblemCheckTracker[:poll_pop3_auth_error].blips).to eq(1)
       end
 
       it "logs an error on pop connection timeout error" do
@@ -59,9 +57,7 @@
 
         i18n_key = "dashboard.poll_pop3_timeout"
 
-        expect(AdminDashboardData.problem_message_check(i18n_key)).to eq(
-          I18n.t(i18n_key, base_path: Discourse.base_path),
-        )
+        expect(ProblemCheckTracker[:poll_pop3_timeout].blips).to eq(1)
       end
 
       it "logs an error when pop fails and continues with next message" do

spec/requests/static_controller_spec.rb

@@ -54,6 +54,14 @@
         expect(response.media_type).to eq("image/png")
         expect(response.body.bytesize).to eq(upload.filesize)
       end
+
+      context "when favicon fails to load" do
+        before { FileHelper.stubs(:download).raises(SocketError) }
+
+        it "creates an admin notice" do
+          expect { get "/favicon/proxied" }.to change { AdminNotice.problem.count }.by(1)
+        end
+      end
     end
   end