FEATURE: Add ProblemCheck for google (#167)

This commit brings error messages for Google Translate to the admin
dashboard. And adds errors for missing Google API keys to the
ProblemCheck

ref: /t/136397

* update translation

* small change about translation

* Update app/services/problem_check/missing_translator_api_key.rb

Co-authored-by: Ted Johansson <[email protected]>

app/services/problem_check/missing_translator_api_key.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+class ProblemCheck::MissingTranslatorApiKey < ProblemCheck
+  self.priority = "high"
+
+  def call
+    return no_problem unless SiteSetting.translator_enabled
+    name = api_key_site_setting_name
+    return no_problem if name.nil?
+    return no_problem if SiteSetting.get(name).present?
+
+    problem
+  end
+
+  private
+
+  def translation_data
+    {
+      provider: SiteSetting.translator,
+      key: I18n.t("site_settings.#{api_key_site_setting_name}"),
+      key_name: api_key_site_setting_name,
+    }
+  end
+
+  def api_key_site_setting_name
+    case SiteSetting.translator
+    when "Google"
+      "translator_google_api_key"
+    when "Microsoft"
+      "translator_azure_subscription_key"
+    end
+  end
+end

config/locales/server.en.yml

@@ -32,5 +32,5 @@ en:
     poster_not_in_group: "Post wasn't made by an user in an allowed group."
   dashboard:
     problem:
-      microsoft_azure_key: "Microsoft was used as the translator, but no Azure Subscription Key was provided."
+      missing_translator_api_key: "%{provider} was used as the translator, but no %{key} was provided. See <a href=\"/admin/site_settings/category/discourse_translator?filter=%{key_name}\">site settings</a>."
       translator_error: "%{provider} reported a translation error with code %{code}: %{message}"

plugin.rb

@@ -35,9 +35,9 @@ class Engine < ::Rails::Engine
     end
   end
 
-  require_relative "app/services/problem_check/microsoft_azure_key"
+  require_relative "app/services/problem_check/missing_translator_api_key"
   require_relative "app/services/problem_check/translator_error"
-  register_problem_check ProblemCheck::MicrosoftAzureKey
+  register_problem_check ProblemCheck::MissingTranslatorApiKey
   register_problem_check ProblemCheck::TranslatorError
 
   class DiscourseTranslator::TranslatorController < ::ApplicationController

services/discourse_translator/google.rb

@@ -71,8 +71,8 @@ def self.access_token_key
     end
 
     def self.access_token
-      SiteSetting.translator_google_api_key ||
-        (raise TranslatorError.new("NotFound: Google Api Key not set."))
+      return SiteSetting.translator_google_api_key if SiteSetting.translator_google_api_key.present?
+      raise ProblemCheckedTranslationError.new("NotFound: Google Api Key not set.")
     end
 
     def self.detect(topic_or_post)
@@ -144,11 +144,19 @@ def self.result(url, body)
 
       if response.status != 200
         if body && body["error"]
-          raise TranslatorError.new(body["error"]["message"])
+          ProblemCheckTracker[:translator_error].problem!(
+            details: {
+              provider: "Google",
+              code: body["error"]["code"],
+              message: body["error"]["message"],
+            },
+          )
+          raise ProblemCheckedTranslationError.new(body["error"]["message"])
         else
           raise TranslatorError.new(response.inspect)
         end
       else
+        ProblemCheckTracker[:translator_error].no_problem!
         body["data"]
       end
     end

spec/services/google_spec.rb

@@ -3,12 +3,15 @@
 require "rails_helper"
 
 RSpec.describe DiscourseTranslator::Google do
+  let(:api_key) { "12345" }
+  before do
+    SiteSetting.translator_enabled = true
+    SiteSetting.translator_google_api_key = api_key
+  end
   let(:mock_response) { Struct.new(:status, :body) }
 
   describe ".access_token" do
     describe "when set" do
-      api_key = "12345"
-      before { SiteSetting.translator_google_api_key = api_key }
       it "should return back translator_google_api_key" do
         expect(described_class.access_token).to eq(api_key)
       end
@@ -20,7 +23,7 @@
 
     it "should store the detected language in a custom field" do
       detected_lang = "en"
-      described_class.expects(:access_token).returns("12345")
+      described_class.expects(:access_token).returns(api_key)
       Excon
         .expects(:post)
         .returns(
@@ -48,7 +51,7 @@
       request_url = "#{DiscourseTranslator::Google::DETECT_URI}"
       body = {
         q: post.cooked.truncate(DiscourseTranslator::Google::MAXLENGTH, omission: nil),
-        key: "",
+        key: api_key,
       }
 
       Excon
@@ -94,25 +97,41 @@
   describe ".translate" do
     let(:post) { Fabricate(:post) }
 
-    it "raises an error on failure" do
-      described_class.expects(:access_token).returns("12345")
+    it "raise an error and warns admin on failure" do
+      described_class.expects(:access_token).returns(api_key)
       described_class.expects(:detect).returns("__")
 
       Excon.expects(:post).returns(
         mock_response.new(
           400,
           {
-            error: "something went wrong",
-            error_description: "you passed in a wrong param",
+            error: {
+              code: "400",
+              message: "API key not valid. Please pass a valid API key.",
+            },
           }.to_json,
         ),
       )
 
-      expect { described_class.translate(post) }.to raise_error DiscourseTranslator::TranslatorError
+      ProblemCheckTracker[:translator_error].no_problem!
+
+      expect { described_class.translate(post) }.to raise_error(
+        DiscourseTranslator::ProblemCheckedTranslationError,
+      )
+
+      expect(AdminNotice.problem.last.message).to eq(
+        I18n.t(
+          "dashboard.problem.translator_error",
+          locale: "en",
+          provider: "Google",
+          code: 400,
+          message: "API key not valid. Please pass a valid API key.",
+        ),
+      )
     end
 
     it "raises an error when the response is not JSON" do
-      described_class.expects(:access_token).returns("12345")
+      described_class.expects(:access_token).returns(api_key)
       described_class.expects(:detect).returns("__")
 
       Excon.expects(:post).returns(mock_response.new(413, "<html><body>some html</body></html>"))

spec/services/problem_check/missing_translator_api_key_spec.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+RSpec.describe ProblemCheck::MissingTranslatorApiKey do
+  subject(:check) { described_class.new }
+
+  describe ".call" do
+    before { SiteSetting.stubs(translator_enabled: enabled) }
+
+    shared_examples "missing key checker" do |provider, key|
+      context "when translator is #{provider}" do
+        before { SiteSetting.translator = provider }
+
+        it "when #{provider} is not provided" do
+          SiteSetting.set(key, "")
+
+          expect(check).to have_a_problem.with_priority("high").with_message(
+            I18n.t(
+              "dashboard.problem.missing_translator_api_key",
+              locale: "en",
+              provider:,
+              key: I18n.t("site_settings.#{key}"),
+              key_name: key,
+            ),
+          )
+        end
+
+        it "when #{provider} is provided" do
+          SiteSetting.set(key, "foo")
+
+          expect(check).to be_chill_about_it
+        end
+      end
+    end
+
+    context "when plugin is disabled" do
+      let(:enabled) { false }
+
+      it { expect(check).to be_chill_about_it }
+    end
+
+    context "when plugin is enabled" do
+      let(:enabled) { true }
+
+      include_examples "missing key checker", "Google", "translator_google_api_key"
+      include_examples "missing key checker", "Microsoft", "translator_azure_subscription_key"
+    end
+  end
+end