From f0275f159189913317599d0a59cd0a6fa2519d1f Mon Sep 17 00:00:00 2001
From: Ted Johansson <drenmi@gmail.com>
Date: Fri, 15 Mar 2024 13:21:11 +0800
Subject: [PATCH] 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  | 25 +++++++++++++
 lib/discourse_chat_integration/provider.rb    |  2 ++
 plugin.rb                                     | 17 +--------
 spec/fabricators/channel_fabricator.rb        |  3 ++
 .../problem_check/channel_errors_spec.rb      | 35 +++++++++++++++++++
 5 files changed, 66 insertions(+), 16 deletions(-)
 create mode 100644 app/services/problem_check/channel_errors.rb
 create mode 100644 spec/fabricators/channel_fabricator.rb
 create mode 100644 spec/services/problem_check/channel_errors_spec.rb

diff --git a/app/services/problem_check/channel_errors.rb b/app/services/problem_check/channel_errors.rb
new file mode 100644
index 0000000..bd1b755
--- /dev/null
+++ b/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
diff --git a/lib/discourse_chat_integration/provider.rb b/lib/discourse_chat_integration/provider.rb
index 6251cd3..eba0f98 100644
--- a/lib/discourse_chat_integration/provider.rb
+++ b/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
diff --git a/plugin.rb b/plugin.rb
index 6be516b..80204ee 100644
--- a/plugin.rb
+++ b/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)
diff --git a/spec/fabricators/channel_fabricator.rb b/spec/fabricators/channel_fabricator.rb
new file mode 100644
index 0000000..f5bf73b
--- /dev/null
+++ b/spec/fabricators/channel_fabricator.rb
@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+Fabricator(:channel, class_name: "DiscourseChatIntegration::Channel") { error_key { nil } }
diff --git a/spec/services/problem_check/channel_errors_spec.rb b/spec/services/problem_check/channel_errors_spec.rb
new file mode 100644
index 0000000..00e463a
--- /dev/null
+++ b/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