Browse Source

Maintenance: Make it possible to translate Rails error messages.

Martin Gruner 2 years ago
parent
commit
999e0b4b06

+ 1 - 1
app/controllers/application_controller/handles_errors.rb

@@ -95,7 +95,7 @@ module ApplicationController::HandlesErrors
     elsif (first_error = e.try(:record)&.errors&.full_messages&.first)
       data[:error_human] = first_error
     elsif e.message.match?(%r{(already exists|duplicate key|duplicate entry)}i)
-      data[:error_human] = __('Object already exists!')
+      data[:error_human] = __('This object already exists.')
     elsif e.message =~ %r{null value in column "(.+?)" violates not-null constraint}i || e.message =~ %r{Field '(.+?)' doesn't have a default value}i
       data[:error_human] = "Attribute '#{$1}' required!"
     elsif e.message == 'Exceptions::Forbidden'

+ 4 - 3
app/graphql/gql/zammad_schema.rb

@@ -70,15 +70,16 @@ class Gql::ZammadSchema < GraphQL::Schema
   RETHROWABLE_ERRORS = [ArgumentError, IndexError, NameError, RangeError, RegexpError, SystemCallError, ThreadError, TypeError, ZeroDivisionError].freeze
 
   # Post-process errors and enrich them with meta information for processing on the client side.
-  rescue_from(StandardError) do |err, _obj, _args, _ctx, field|
+  rescue_from(StandardError) do |err, _obj, _args, ctx, field|
 
     if field.path.start_with?('Mutations.')
+      user_locale = ctx.current_user?&.locale
       if err.is_a? ActiveRecord::RecordInvalid
-        user_errors = err.record.errors.map { |error| { field: error.attribute.to_s.camelize(:lower), message: error.full_message } }
+        user_errors = err.record.errors.map { |e| { field: e.attribute.to_s.camelize(:lower), message: e.localized_full_message(locale: user_locale, no_field_name: true) } }
         next { errors: user_errors }
       end
       if err.is_a? ActiveRecord::RecordNotUnique
-        next { errors: [ message: __('Object already exists!') ] }
+        next { errors: [ message: Translation.translate(user_locale, 'This object already exists.') ] }
       end
     end
 

+ 47 - 0
config/initializers/activemodel_error.rb

@@ -0,0 +1,47 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+class ActiveModel::Error
+
+  # Make it possible to retrieve errors that are translated with a Zammad locale.
+  def localized_full_message(locale:, no_field_name: false)
+    errors_hash = I18n.backend.translations[:en][:errors]
+    orig_format = errors_hash[:format]
+    errors_hash[:format] = Translation.translate(locale, 'This field %s', '%<message>s') if no_field_name
+    I18n.with_zammad_locale(locale) do
+      full_message
+    end
+  ensure
+    errors_hash[:format] = orig_format
+  end
+end
+
+module I18n
+  def self.with_zammad_locale(locale)
+    backend.zammad_locale = locale
+    yield
+  ensure
+    backend.zammad_locale = nil
+  end
+end
+
+class I18n::Backend::Simple
+
+  attr_accessor :zammad_locale
+
+  if !method_defined?(:orig_lookup)
+
+    alias orig_lookup lookup
+
+    # Allow I18n to load the default rails error messages from the YAML files,
+    #   but translate them to the target locale before the error messages get generated
+    #   including placeholder subsitition.
+    def lookup(...)
+      result = orig_lookup(...)
+      if result.is_a?(String) && zammad_locale
+        return Translation.translate(zammad_locale, result)
+      end
+
+      result
+    end
+  end
+end

+ 96 - 5
i18n/zammad.pot

@@ -1717,6 +1717,12 @@ msgstr ""
 msgid "Cannot delete category"
 msgstr ""
 
+msgid "Cannot delete record because a dependent %{record} exists"
+msgstr ""
+
+msgid "Cannot delete record because dependent %{record} exist"
+msgstr ""
+
 #: app/policies/ticket_policy.rb
 msgid "Cannot follow-up on a closed ticket. Please create a new ticket."
 msgstr ""
@@ -7108,11 +7114,6 @@ msgstr ""
 msgid "Object Manager"
 msgstr ""
 
-#: app/controllers/application_controller/handles_errors.rb
-#: app/graphql/gql/zammad_schema.rb
-msgid "Object already exists!"
-msgstr ""
-
 #: app/assets/javascripts/app/controllers/object_manager.coffee
 #: db/seeds/permissions.rb
 msgid "Objects"
@@ -10111,6 +10112,10 @@ msgstr ""
 msgid "This entry already exists!"
 msgstr ""
 
+#: config/initializers/activemodel_error.rb
+msgid "This field %s"
+msgstr ""
+
 #: app/frontend/shared/form/i18n/locales.ts
 msgid "This field can only contain alphabetical characters."
 msgstr ""
@@ -10239,6 +10244,11 @@ msgstr ""
 msgid "This might take some time during which the system can't be used."
 msgstr ""
 
+#: app/controllers/application_controller/handles_errors.rb
+#: app/graphql/gql/zammad_schema.rb
+msgid "This object already exists."
+msgstr ""
+
 #: app/views/knowledge_base/public/_top_banner.html.erb
 msgid "This page is invisible to the public."
 msgstr ""
@@ -11213,6 +11223,9 @@ msgstr ""
 msgid "VIP"
 msgstr ""
 
+msgid "Validation failed: %{errors}"
+msgstr ""
+
 #: app/assets/javascripts/app/views/integration/clearbit.jst.eco
 #: app/assets/javascripts/app/views/integration/cti.jst.eco
 #: app/assets/javascripts/app/views/integration/exchange.jst.eco
@@ -12014,6 +12027,12 @@ msgstr ""
 msgid "busy"
 msgstr ""
 
+msgid "can't be blank"
+msgstr ""
+
+msgid "can't be empty"
+msgstr ""
+
 #: app/assets/javascripts/app/controllers/_ui_element/core_workflow_condition.coffee
 msgid "changed to"
 msgstr ""
@@ -12143,6 +12162,9 @@ msgstr ""
 msgid "does not exist"
 msgstr ""
 
+msgid "doesn't match %{attribute}"
+msgstr ""
+
 #: app/assets/javascripts/app/views/import/freshdesk.jst.eco
 #: app/assets/javascripts/app/views/import/kayako.jst.eco
 #: app/assets/javascripts/app/views/import/otrs.jst.eco
@@ -12262,6 +12284,9 @@ msgstr ""
 msgid "h"
 msgstr ""
 
+msgid "has already been taken"
+msgstr ""
+
 #: app/assets/javascripts/app/controllers/_ui_element/_application_selector.coffee
 #: app/assets/javascripts/app/controllers/_ui_element/core_workflow_condition.coffee
 #: app/assets/javascripts/app/views/object_manager/index.jst.eco
@@ -12385,19 +12410,31 @@ msgstr ""
 msgid "is in working time"
 msgstr ""
 
+msgid "is invalid"
+msgstr ""
+
 #: app/assets/javascripts/app/controllers/_ui_element/_application_selector.coffee
 #: app/assets/javascripts/app/controllers/_ui_element/core_workflow_condition.coffee
 msgid "is not"
 msgstr ""
 
+msgid "is not a number"
+msgstr ""
+
 #: app/assets/javascripts/app/controllers/_ui_element/_application_selector.coffee
 msgid "is not in working time"
 msgstr ""
 
+msgid "is not included in the list"
+msgstr ""
+
 #: app/assets/javascripts/app/models/_application_model.coffee
 msgid "is required"
 msgstr ""
 
+msgid "is reserved"
+msgstr ""
+
 #: app/assets/javascripts/app/controllers/_ui_element/core_workflow_condition.coffee
 msgid "is set"
 msgstr ""
@@ -12406,6 +12443,24 @@ msgstr ""
 msgid "is the same"
 msgstr ""
 
+msgid "is the wrong length (should be %{count} characters)"
+msgstr ""
+
+msgid "is the wrong length (should be 1 character)"
+msgstr ""
+
+msgid "is too long (maximum is %{count} characters)"
+msgstr ""
+
+msgid "is too long (maximum is 1 character)"
+msgstr ""
+
+msgid "is too short (minimum is %{count} characters)"
+msgstr ""
+
+msgid "is too short (minimum is 1 character)"
+msgstr ""
+
 #: app/assets/javascripts/app/lib/app_post/pretty_date.coffee
 msgid "just now"
 msgstr ""
@@ -12462,6 +12517,42 @@ msgstr ""
 msgid "minutes"
 msgstr ""
 
+msgid "must be accepted"
+msgstr ""
+
+msgid "must be an integer"
+msgstr ""
+
+msgid "must be blank"
+msgstr ""
+
+msgid "must be equal to %{count}"
+msgstr ""
+
+msgid "must be even"
+msgstr ""
+
+msgid "must be greater than %{count}"
+msgstr ""
+
+msgid "must be greater than or equal to %{count}"
+msgstr ""
+
+msgid "must be less than %{count}"
+msgstr ""
+
+msgid "must be less than or equal to %{count}"
+msgstr ""
+
+msgid "must be odd"
+msgstr ""
+
+msgid "must be other than %{count}"
+msgstr ""
+
+msgid "must exist"
+msgstr ""
+
 #: app/assets/javascripts/app/views/integration/cti.jst.eco
 #: app/assets/javascripts/app/views/integration/placetel.jst.eco
 #: app/assets/javascripts/app/views/integration/sipgate.jst.eco

+ 31 - 0
lib/generators/zammad/translation_catalog/extractor/rails_error_messages.rb

@@ -0,0 +1,31 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+class Zammad::TranslationCatalog::Extractor::RailsErrorMessages < Zammad::TranslationCatalog::Extractor::Base
+
+  def extract_translatable_strings
+    I18n.backend.load_translations
+    find_error_messages(I18n.backend.translations[:en]).each do |error_message|
+      extracted_strings << Zammad::TranslationCatalog::ExtractedString.new(string: error_message, references: [])
+    end
+  end
+
+  # Messages from doorkeeper are very technical and don't seem to be shown to the users.
+  IGNORE_KEYS = %r{doorkeeper}
+
+  def find_error_messages(hash)
+    hash.reduce([]) do |result, (key, value)|
+      next result if !value.is_a?(Hash)
+      next result if key.match?(IGNORE_KEYS)
+
+      result + (key == :errors ? flattened_values(value.fetch(:messages, {})) : find_error_messages(value))
+    end.uniq
+  end
+
+  def flattened_values(hash)
+    hash.values.reduce([]) do |result, value|
+      next result + flattened_values(value) if value.is_a?(Hash)
+
+      result.push(value)
+    end
+  end
+end

+ 1 - 1
lib/generators/zammad/translation_catalog/extractor/ruby.rb

@@ -31,7 +31,7 @@ class Zammad::TranslationCatalog::Extractor::Ruby < Zammad::TranslationCatalog::
 
   def find_files
     files = []
-    %w[lib db app].each do |dir|
+    %w[config app db lib].each do |dir|
       files += Dir.glob("#{base_path}/#{dir}/**/*.rb")
     end
     files

+ 7 - 4
spec/graphql/gql/mutations/organization/update_spec.rb

@@ -4,7 +4,7 @@ require 'rails_helper'
 
 RSpec.describe Gql::Mutations::Organization::Update, type: :graphql do
   context 'when updating organizations', authenticated_as: :user do
-    let(:user)               { create(:agent) }
+    let(:user)               { create(:agent, preferences: { locale: 'de-de' }) }
     let(:organization)       { create(:organization) }
     let(:variables)          { { id: gql.id(organization), input: input_payload } }
     let(:input_payload)      { {} }
@@ -31,7 +31,10 @@ RSpec.describe Gql::Mutations::Organization::Update, type: :graphql do
       QUERY
     end
 
+    let(:custom_translations) { { "can't be blank" => 'darf nicht leer sein', 'This field %s' => 'Dieses Feld %{message}', 'This object already exists.' => 'Dieses Objekt existiert bereits.' } } # rubocop:disable Style/FormatStringToken
+
     before do
+      allow(Translation).to receive(:translate) { |_locale, string| custom_translations[string] || string }
       gql.execute(query, variables: variables)
     end
 
@@ -47,16 +50,16 @@ RSpec.describe Gql::Mutations::Organization::Update, type: :graphql do
       let(:input_payload) { { name: '' } }
 
       it 'returns a user error' do
-        expect(gql.result.data['errors'].first).to include('field' => 'name', 'message' => "Name can't be blank")
+        expect(gql.result.data['errors'].first).to include('field' => 'name', 'message' => 'Dieses Feld darf nicht leer sein')
       end
     end
 
     context 'when updating organization with name of another organization' do
       let(:input_payload) { { name: other_org.name } }
-      let(:other_org) { create(:organization) }
+      let(:other_org)     { create(:organization) }
 
       it 'returns a user error' do
-        expect(gql.result.data['errors'].first).to include('message' => 'Object already exists!')
+        expect(gql.result.data['errors'].first).to include('message' => 'Dieses Objekt existiert bereits.')
       end
     end
 

+ 25 - 0
spec/lib/active_model/errors_spec.rb

@@ -0,0 +1,25 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe ActiveModel::Error, :aggregate_failures do
+
+  let(:error) { User.first.errors.add(:firstname, :blank) }
+
+  context 'when using the standard error format and the default locale' do
+    it 'produces a standard Rails error including the field name' do
+      expect(error.message).to eq("can't be blank")
+      expect(error.full_message).to eq("Firstname can't be blank")
+    end
+  end
+
+  context 'when using a custom error format and a custom locale' do
+    let(:custom_translations) { { "can't be blank" => 'darf nicht leer sein', 'This field %s' => 'Dieses Feld %{message}' } } # rubocop:disable Style/FormatStringToken
+
+    it 'produces a custom error NOT including the field name' do
+      allow(Translation).to receive(:translate) { |_locale, string| custom_translations[string] || string }
+      expect(error.message).to eq("can't be blank")
+      expect(error.localized_full_message(no_field_name: true, locale: 'de-de')).to eq('Dieses Feld darf nicht leer sein')
+    end
+  end
+end

+ 19 - 0
spec/lib/generators/zammad/translation_catalog/extractor/rails_error_messages_spec.rb

@@ -0,0 +1,19 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe Zammad::TranslationCatalog::Extractor::RailsErrorMessages do
+  subject(:extractor_module) { described_class.new(options: {}) }
+
+  let(:result_strings) do
+    extractor_module.extract_translatable_strings
+  end
+
+  it 'finds strings from activemodel' do
+    expect(result_strings).to include("can't be empty")
+  end
+
+  it 'finds strings from activerecord' do
+    expect(result_strings).to include('Cannot delete record because dependent %{record} exist') # rubocop:disable Style/FormatStringToken
+  end
+end

+ 1 - 1
spec/system/system/object_manager_spec.rb

@@ -23,7 +23,7 @@ RSpec.describe 'System > Objects', type: :system, mariadb: true do
       end
     end
 
-    include_examples 'cannot create new object attribute', 'customer_id', 'Object already exists!'
+    include_examples 'cannot create new object attribute', 'customer_id', 'This object already exists.'
     ['some_other_id', 'some_other_ids', 'some spaces'].each do |name|
       include_examples 'cannot create new object attribute', name, 'are not allowed'
     end