DEV: Promote channel error check to ProblemCheck (#190)

We're promoting problem checks to first class citizens in core. This migrates the problem check to the new API.

In the process of adding tests for this check, I discovered what seems like a mistake that likely means this check never worked until now. (See inline comment.)

app/services/problem_check/channel_errors.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class ProblemCheck::ChannelErrors < ProblemCheck
+  self.priority = "low"
+
+  def call
+    return no_problem if !SiteSetting.chat_integration_enabled
+    return no_problem if !channel_errors?
+
+    problem
+  end
+
+  private
+
+  def channel_errors?
+    DiscourseChatIntegration::Channel.find_each.any? do |channel|
+      channel.error_key.present? &&
+        ::DiscourseChatIntegration::Provider.is_enabled(channel.provider)
+    end
+  end
+
+  def translation_key
+    "chat_integration.admin_error"
+  end
+end

lib/discourse_chat_integration/provider.rb

@@ -32,6 +32,8 @@ module DiscourseChatIntegration
     end
 
     def self.is_enabled(provider)
+      provider = get_by_name(provider) if provider.is_a?(String)
+
       if defined?(provider::PROVIDER_ENABLED_SETTING)
         SiteSetting.public_send(provider::PROVIDER_ENABLED_SETTING)
       else

plugin.rb

@@ -19,6 +19,7 @@ require_relative "lib/discourse_chat_integration/provider/slack/slack_enabled_se
 
 after_initialize do
   require_relative "app/initializers/discourse_chat_integration"
+  require_relative "app/services/problem_check/channel_errors"
 
   on(:site_setting_changed) do |setting_name, old_value, new_value|
     is_enabled_setting = setting_name == :chat_integration_telegram_enabled
@@ -44,22 +45,6 @@ after_initialize do
 
   add_admin_route "chat_integration.menu_title", "chat-integration"
 
-  AdminDashboardData.add_problem_check do
-    next if !SiteSetting.chat_integration_enabled
-
-    error = false
-    DiscourseChatIntegration::Channel.find_each do |channel|
-      next if channel.error_key.blank?
-      next if !::DiscourseChatIntegration::Provider.is_enabled(channel.provider)
-      error = true
-    end
-
-    if error
-      base_path = Discourse.respond_to?(:base_path) ? Discourse.base_path : Discourse.base_uri
-      I18n.t("chat_integration.admin_error", base_path: base_path)
-    end
-  end
-
   DiscourseChatIntegration::Provider.mount_engines
 
   if defined?(DiscourseAutomation)

spec/fabricators/channel_fabricator.rb

@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+Fabricator(:channel, class_name: "DiscourseChatIntegration::Channel") { error_key { nil } }

spec/services/problem_check/channel_errors_spec.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require_relative "../../dummy_provider"
+
+RSpec.describe ProblemCheck::ChannelErrors do
+  include_context "with dummy provider"
+
+  let(:check) { described_class.new }
+
+  context "when chat integration is not enabled" do
+    before { SiteSetting.stubs(chat_integration_enabled: false) }
+
+    it { expect(check).to be_chill_about_it }
+  end
+
+  context "when chat integration is enabled" do
+    before { SiteSetting.stubs(chat_integration_enabled: true) }
+
+    context "when an enabled channel has errors" do
+      before { Fabricate(:channel, provider: "dummy", error_key: "whoops") }
+
+      it do
+        expect(check).to have_a_problem.with_priority("low").with_message(
+          "Some chat integration channels have errors. Visit <a href='/admin/plugins/chat-integration'>the chat integration section</a> to find out more.",
+        )
+      end
+    end
+
+    context "when a disabled chanel has errors" do
+      before { Fabricate(:channel, provider: "dummy", error_key: nil) }
+
+      it { expect(check).to be_chill_about_it }
+    end
+  end
+end