From 1f5d1bf8d3a65943047d780ac4fee90ff7b62850 Mon Sep 17 00:00:00 2001
From: Martin Gruner <mg@zammad.com>
Date: Fri, 19 Nov 2021 13:11:20 +0100
Subject: [PATCH] Maintenance: Improve handling of translation entries in
 extractor and on importing.

---
 .../javascripts/app/controllers/sla.coffee    |  2 +-
 app/assets/javascripts/app/models/sla.coffee  |  4 +--
 .../javascripts/app/models/text_module.coffee | 10 +++---
 .../javascripts/app/models/trigger.coffee     |  4 +--
 .../translation/synchronizes_from_po.rb       | 22 +++++++++---
 i18n/zammad.pot                               | 34 +++++++++++++++++--
 .../translation_catalog/extractor/base.rb     |  8 +++++
 .../translation_catalog/extractor/erb.rb      |  1 +
 .../translation_catalog/extractor/frontend.rb |  1 +
 .../translation_catalog/extractor/ruby.rb     |  1 +
 .../translation_catalog/extractor/erb_spec.rb | 12 +++++++
 .../extractor/frontend_spec.rb                | 12 +++++++
 .../extractor/ruby_spec.rb                    | 12 +++++++
 .../translation_synchronizes_from_po_spec.rb  |  7 ++++
 14 files changed, 111 insertions(+), 19 deletions(-)

diff --git a/app/assets/javascripts/app/controllers/sla.coffee b/app/assets/javascripts/app/controllers/sla.coffee
index e34779b3b..1d03e8910 100644
--- a/app/assets/javascripts/app/controllers/sla.coffee
+++ b/app/assets/javascripts/app/controllers/sla.coffee
@@ -89,7 +89,7 @@ class Sla extends App.ControllerSubContent
 
   description: (e) =>
     new App.ControllerGenericDescription(
-      description: App.Calendar.description
+      description: App.Sla.description
       container:   @el.closest('.content')
     )
 
diff --git a/app/assets/javascripts/app/models/sla.coffee b/app/assets/javascripts/app/models/sla.coffee
index dc3db3cf5..77f1dbb19 100644
--- a/app/assets/javascripts/app/models/sla.coffee
+++ b/app/assets/javascripts/app/models/sla.coffee
@@ -23,8 +23,8 @@ class App.Sla extends App.Model
 
   @description = __('''
 ** Service Level Agreements **, abbreviated ** SLAs **, help you to meet certain customers' time-related responses. Thus, for example, you can say customers should always get a response from you after 8 hours at the latest. In the event of an imminent violation or a breach, Zammad will alert you to such events.
-
+''') + '\n\n' + __('''
 It can be ** response time ** (time between the creation of a ticket and the first reaction of an agent), ** update time ** (time between a customer's request and an agent's reaction) and ** solution time ** (time between creation and closing a ticket ) To be defined.
-
+''') + '\n\n' + __('''
 Any violations are displayed in a separate view in the overviews. You can also configure ** e-mail notifications **.
 ''')
diff --git a/app/assets/javascripts/app/models/text_module.coffee b/app/assets/javascripts/app/models/text_module.coffee
index 48c2c8702..4f4eff5ee 100644
--- a/app/assets/javascripts/app/models/text_module.coffee
+++ b/app/assets/javascripts/app/models/text_module.coffee
@@ -39,24 +39,22 @@ class App.TextModule extends App.Model
   # coffeelint: disable=no_interpolation_in_single_quotes
   @description = __('''
 Create Text Modules to **spend less time writing responses**. Text Modules can include smart variables like the users name or email address.
-
+''') + '\n\n' + __('''
 Examples of snippets are:
-
 * Hello Mrs. #{ticket.customer.lastname},
 * Hello Mr. #{ticket.customer.lastname},
 * Hello #{ticket.customer.firstname},
 * My Name is #{user.firstname},
-
+''') + '\n\n' + __('''
 Of course you can also use multi line snippets.
-
+''') + '\n\n' + __('''
 Available objects are:
 * ticket (e. g. ticket.state, ticket.group)
 * ticket.customer (e. g. ticket.customer.firstname, ticket.customer.lastname)
 * ticket.owner (e. g. ticket.owner.firstname, ticket.owner.lastname)
 * ticket.organization (e. g. ticket.organization.name)
 * user (e. g. user.firstname, user.email)
-
+''') + '\n\n' + __('''
 To select placeholders from a list, just enter "::".
-
 ''')
   # coffeelint: enable=no_interpolation_in_single_quotes
diff --git a/app/assets/javascripts/app/models/trigger.coffee b/app/assets/javascripts/app/models/trigger.coffee
index c67c19460..623b573c3 100644
--- a/app/assets/javascripts/app/models/trigger.coffee
+++ b/app/assets/javascripts/app/models/trigger.coffee
@@ -17,8 +17,8 @@ class App.Trigger extends App.Model
 
   @description = __('''
 Every time a customer creates a new ticket, they automatically receive a confirmation email to assure them that their issue has been submitted successfully. This behavior is built into Zammad, but it’s also highly customizable, and you can set up other automated actions just like it.
-
+''') + '\n\n' + __('''
 Maybe you want to set a higher priority on any ticket with the word “urgent” in the title. Maybe you want to avoid sending auto-reply emails to customers from certain organizations. Maybe you want mark a ticket as “pending” whenever someone adds an internal note to a ticket.
-
+''') + '\n\n' + __('''
 Whatever it is, you can do it with triggers: actions that watch tickets for certain changes, and then fire off whenever those changes occur.
 ''')
diff --git a/app/models/translation/synchronizes_from_po.rb b/app/models/translation/synchronizes_from_po.rb
index 35ba40256..697869db0 100644
--- a/app/models/translation/synchronizes_from_po.rb
+++ b/app/models/translation/synchronizes_from_po.rb
@@ -9,28 +9,38 @@ module Translation::SynchronizesFromPo
 
     def sync
       Locale.to_sync.each do |locale|
-        sync_locale_from_po locale.locale
+        ActiveRecord::Base.transaction do
+          sync_locale_from_po locale.locale
+        end
       end
     end
 
-    def sync_locale_from_po(locale) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity
+    def sync_locale_from_po(locale) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
 
       previous_unmodified_translations = Translation.where(locale: locale, is_synchronized_from_codebase: true).select { |t| t.target.eql?(t.target_initial) }
       updated_translation_ids = Set[]
+      importable_translations = []
+
+      strings_for_locale(locale).each_pair do |source, entry| # rubocop:disable Metrics/BlockLength
+
+        if source.length > 500 || entry.translation.length > 500
+          Rails.logger.error "Cannot import translation for locale #{locale} because it exceeds maximum string length of 500: source: '#{source}', translation: '#{entry.translation}'"
+          next
+        end
 
-      strings_for_locale(locale).each_pair do |source, entry|
         t = Translation.find_source(locale, source)
         # New string
         if !t
-          Translation.new(
+          importable_translations << Translation.new(
             locale:                             locale,
             source:                             source,
             target:                             entry.translation,
+            target_initial:                     entry.translation,
             is_synchronized_from_codebase:      true,
             synchronized_from_translation_file: entry.translation_file,
             created_by_id:                      1,
             updated_by_id:                      1
-          ).save!
+          )
           next
         end
 
@@ -48,6 +58,8 @@ module Translation::SynchronizesFromPo
         end
         updated_translation_ids.add t.id
       end
+
+      Translation.import importable_translations
       # Remove any unmodified & synchronized strings that are not present in the data any more.
       previous_unmodified_translations.reject { |t| updated_translation_ids.member? t.id }.each(&:destroy!)
       true
diff --git a/i18n/zammad.pot b/i18n/zammad.pot
index 6299b46d2..e04939c88 100644
--- a/i18n/zammad.pot
+++ b/i18n/zammad.pot
@@ -245,7 +245,7 @@ msgid "** Data Privacy **, helps you to delete and verify the removal of existin
 msgstr ""
 
 #: app/assets/javascripts/app/models/sla.coffee
-msgid "** Service Level Agreements **, abbreviated ** SLAs **, help you to meet certain customers' time-related responses. Thus, for example, you can say customers should always get a response from you after 8 hours at the latest. In the event of an imminent violation or a breach, Zammad will alert you to such events.\n\nIt can be ** response time ** (time between the creation of a ticket and the first reaction of an agent), ** update time ** (time between a customer's request and an agent's reaction) and ** solution time ** (time between creation and closing a ticket ) To be defined.\n\nAny violations are displayed in a separate view in the overviews. You can also configure ** e-mail notifications **."
+msgid "** Service Level Agreements **, abbreviated ** SLAs **, help you to meet certain customers' time-related responses. Thus, for example, you can say customers should always get a response from you after 8 hours at the latest. In the event of an imminent violation or a breach, Zammad will alert you to such events."
 msgstr ""
 
 #: app/assets/javascripts/app/controllers/_plugin/keyboard_shortcuts.coffee
@@ -793,6 +793,10 @@ msgstr ""
 msgid "Any Recipient"
 msgstr ""
 
+#: app/assets/javascripts/app/models/sla.coffee
+msgid "Any violations are displayed in a separate view in the overviews. You can also configure ** e-mail notifications **."
+msgstr ""
+
 #: db/seeds/settings.rb
 msgid "App ID"
 msgstr ""
@@ -1074,6 +1078,10 @@ msgstr ""
 msgid "Available for the following roles"
 msgstr ""
 
+#: app/assets/javascripts/app/models/text_module.coffee
+msgid "Available objects are:\n* ticket (e. g. ticket.state, ticket.group)\n* ticket.customer (e. g. ticket.customer.firstname, ticket.customer.lastname)\n* ticket.owner (e. g. ticket.owner.firstname, ticket.owner.lastname)\n* ticket.organization (e. g. ticket.organization.name)\n* user (e. g. user.firstname, user.email)"
+msgstr ""
+
 #: db/seeds/settings.rb
 msgid "Available types for a new ticket"
 msgstr ""
@@ -2077,7 +2085,7 @@ msgid "Create Text Modules"
 msgstr ""
 
 #: app/assets/javascripts/app/models/text_module.coffee
-msgid "Create Text Modules to **spend less time writing responses**. Text Modules can include smart variables like the users name or email address.\n\nExamples of snippets are:\n\n* Hello Mrs. #{ticket.customer.lastname},\n* Hello Mr. #{ticket.customer.lastname},\n* Hello #{ticket.customer.firstname},\n* My Name is #{user.firstname},\n\nOf course you can also use multi line snippets.\n\nAvailable objects are:\n* ticket (e. g. ticket.state, ticket.group)\n* ticket.customer (e. g. ticket.customer.firstname, ticket.customer.lastname)\n* ticket.owner (e. g. ticket.owner.firstname, ticket.owner.lastname)\n* ticket.organization (e. g. ticket.organization.name)\n* user (e. g. user.firstname, user.email)\n\nTo select placeholders from a list, just enter \"::\".\n"
+msgid "Create Text Modules to **spend less time writing responses**. Text Modules can include smart variables like the users name or email address."
 msgstr ""
 
 #: app/controllers/first_steps_controller.rb
@@ -3737,7 +3745,7 @@ msgid "Every category in your knowledge base should be given a unique icon for m
 msgstr ""
 
 #: app/assets/javascripts/app/models/trigger.coffee
-msgid "Every time a customer creates a new ticket, they automatically receive a confirmation email to assure them that their issue has been submitted successfully. This behavior is built into Zammad, but it’s also highly customizable, and you can set up other automated actions just like it.\n\nMaybe you want to set a higher priority on any ticket with the word “urgent” in the title. Maybe you want to avoid sending auto-reply emails to customers from certain organizations. Maybe you want mark a ticket as “pending” whenever someone adds an internal note to a ticket.\n\nWhatever it is, you can do it with triggers: actions that watch tickets for certain changes, and then fire off whenever those changes occur."
+msgid "Every time a customer creates a new ticket, they automatically receive a confirmation email to assure them that their issue has been submitted successfully. This behavior is built into Zammad, but it’s also highly customizable, and you can set up other automated actions just like it."
 msgstr ""
 
 #: app/assets/javascripts/app/views/api.jst.eco
@@ -3757,6 +3765,10 @@ msgstr ""
 msgid "Examples"
 msgstr ""
 
+#: app/assets/javascripts/app/models/text_module.coffee
+msgid "Examples of snippets are:\n* Hello Mrs. #{ticket.customer.lastname},\n* Hello Mr. #{ticket.customer.lastname},\n* Hello #{ticket.customer.firstname},\n* My Name is #{user.firstname},"
+msgstr ""
+
 #: app/assets/javascripts/app/controllers/_manage/ticket_auto_assignment.coffee
 msgid "Exception users"
 msgstr ""
@@ -5126,6 +5138,10 @@ msgstr ""
 msgid "Ionicons"
 msgstr ""
 
+#: app/assets/javascripts/app/models/sla.coffee
+msgid "It can be ** response time ** (time between the creation of a ticket and the first reaction of an agent), ** update time ** (time between a customer's request and an agent's reaction) and ** solution time ** (time between creation and closing a ticket ) To be defined."
+msgstr ""
+
 #: app/assets/javascripts/app/controllers/ticket_overview.coffee
 msgid "Items per page"
 msgstr ""
@@ -5665,6 +5681,10 @@ msgstr ""
 msgid "May"
 msgstr ""
 
+#: app/assets/javascripts/app/models/trigger.coffee
+msgid "Maybe you want to set a higher priority on any ticket with the word “urgent” in the title. Maybe you want to avoid sending auto-reply emails to customers from certain organizations. Maybe you want mark a ticket as “pending” whenever someone adds an internal note to a ticket."
+msgstr ""
+
 #: app/assets/javascripts/app/views/layout_ref/organization_profile.jst.eco
 #: app/assets/javascripts/app/views/organization_profile/object.jst.eco
 #: app/assets/javascripts/app/views/popover/organization.jst.eco
@@ -6673,6 +6693,10 @@ msgstr ""
 msgid "October"
 msgstr ""
 
+#: app/assets/javascripts/app/models/text_module.coffee
+msgid "Of course you can also use multi line snippets."
+msgstr ""
+
 #: app/assets/javascripts/app/controllers/_profile/linked_accounts.coffee
 #: db/seeds/settings.rb
 msgid "Office 365"
@@ -10340,6 +10364,10 @@ msgstr ""
 msgid "What values of %s should be synced to users."
 msgstr ""
 
+#: app/assets/javascripts/app/models/trigger.coffee
+msgid "Whatever it is, you can do it with triggers: actions that watch tickets for certain changes, and then fire off whenever those changes occur."
+msgstr ""
+
 #: app/assets/javascripts/app/controllers/_integration/slack.coffee
 msgid "When notification is being sent."
 msgstr ""
diff --git a/lib/generators/translation_catalog/extractor/base.rb b/lib/generators/translation_catalog/extractor/base.rb
index eb295cb79..442d1b75e 100644
--- a/lib/generators/translation_catalog/extractor/base.rb
+++ b/lib/generators/translation_catalog/extractor/base.rb
@@ -15,6 +15,14 @@ class Generators::TranslationCatalog::Extractor::Base
     end
   end
 
+  def validate_strings
+    @strings.to_a.each do |s|
+      if s.length > 500
+        raise "Found a string that longer than than the allowed 500 characters: '#{s}'"
+      end
+    end
+  end
+
   def extract_from_string(string, filename)
     raise NotImplementedError
   end
diff --git a/lib/generators/translation_catalog/extractor/erb.rb b/lib/generators/translation_catalog/extractor/erb.rb
index fd36b0e39..3d490d1d2 100644
--- a/lib/generators/translation_catalog/extractor/erb.rb
+++ b/lib/generators/translation_catalog/extractor/erb.rb
@@ -19,6 +19,7 @@ class Generators::TranslationCatalog::Extractor::Erb < Generators::TranslationCa
         references[result] << filename
       end
     end
+    validate_strings
   end
 
   def find_files(base_path)
diff --git a/lib/generators/translation_catalog/extractor/frontend.rb b/lib/generators/translation_catalog/extractor/frontend.rb
index a71be8f7e..8ba8f5c47 100644
--- a/lib/generators/translation_catalog/extractor/frontend.rb
+++ b/lib/generators/translation_catalog/extractor/frontend.rb
@@ -29,6 +29,7 @@ class Generators::TranslationCatalog::Extractor::Frontend < Generators::Translat
         references[result] << filename
       end
     end
+    validate_strings
   end
 
   def find_files(base_path)
diff --git a/lib/generators/translation_catalog/extractor/ruby.rb b/lib/generators/translation_catalog/extractor/ruby.rb
index 10f9a233c..93b2dd7de 100644
--- a/lib/generators/translation_catalog/extractor/ruby.rb
+++ b/lib/generators/translation_catalog/extractor/ruby.rb
@@ -24,6 +24,7 @@ class Generators::TranslationCatalog::Extractor::Ruby < Generators::TranslationC
         references[result] << filename
       end
     end
+    validate_strings
   end
 
   def find_files(base_path)
diff --git a/spec/lib/generators/translation_catalog/extractor/erb_spec.rb b/spec/lib/generators/translation_catalog/extractor/erb_spec.rb
index c8028f68d..f780fba66 100644
--- a/spec/lib/generators/translation_catalog/extractor/erb_spec.rb
+++ b/spec/lib/generators/translation_catalog/extractor/erb_spec.rb
@@ -40,4 +40,16 @@ RSpec.describe Generators::TranslationCatalog::Extractor::Erb do
       expect(result_strings).to eq(Set[])
     end
   end
+
+  context 'with strings too long' do
+    let(:string) do
+      <<~"CODE"
+        <%= t("#{'a' * 501}") %>
+      CODE
+    end
+
+    it 'raises an error' do
+      expect { result_strings }.to raise_error(%r{Found a string that longer than than the allowed 500 characters})
+    end
+  end
 end
diff --git a/spec/lib/generators/translation_catalog/extractor/frontend_spec.rb b/spec/lib/generators/translation_catalog/extractor/frontend_spec.rb
index 2f76080ec..74ad0d3b2 100644
--- a/spec/lib/generators/translation_catalog/extractor/frontend_spec.rb
+++ b/spec/lib/generators/translation_catalog/extractor/frontend_spec.rb
@@ -42,4 +42,16 @@ RSpec.describe Generators::TranslationCatalog::Extractor::Frontend do
       expect(result_strings).to eq(Set[])
     end
   end
+
+  context 'with strings too long' do
+    let(:string) do
+      <<~"CODE"
+        __("#{'a' * 501}")
+      CODE
+    end
+
+    it 'raises an error' do
+      expect { result_strings }.to raise_error(%r{Found a string that longer than than the allowed 500 characters})
+    end
+  end
 end
diff --git a/spec/lib/generators/translation_catalog/extractor/ruby_spec.rb b/spec/lib/generators/translation_catalog/extractor/ruby_spec.rb
index b836c3a83..46d8449f8 100644
--- a/spec/lib/generators/translation_catalog/extractor/ruby_spec.rb
+++ b/spec/lib/generators/translation_catalog/extractor/ruby_spec.rb
@@ -41,4 +41,16 @@ RSpec.describe Generators::TranslationCatalog::Extractor::Ruby do
       expect(result_strings).to eq(Set[])
     end
   end
+
+  context 'with strings too long' do
+    let(:string) do
+      <<~"CODE"
+        __("#{'a' * 501}")
+      CODE
+    end
+
+    it 'raises an error' do
+      expect { result_strings }.to raise_error(%r{Found a string that longer than than the allowed 500 characters})
+    end
+  end
 end
diff --git a/spec/models/translation/translation_synchronizes_from_po_spec.rb b/spec/models/translation/translation_synchronizes_from_po_spec.rb
index 6473e491c..646aea6f2 100644
--- a/spec/models/translation/translation_synchronizes_from_po_spec.rb
+++ b/spec/models/translation/translation_synchronizes_from_po_spec.rb
@@ -177,6 +177,9 @@ RSpec.describe Translation do
         msgid "custom-string-translated"
         msgstr "custom-string-übersetzt"
 
+        msgid "custom-string-too-long"
+        msgstr "custom-string-too-long #{'a' * 501}"
+
         msgid "custom-string-untranslated"
         msgstr ""
 
@@ -221,6 +224,10 @@ RSpec.describe Translation do
     it 'adds the custom fuzzy entry without content' do
       expect(described_class.find_source('de-de', 'custom-string-fuzzy')).to have_attributes(target: '')
     end
+
+    it 'ignores strings that are too long' do
+      expect(described_class.find_source('de-de', 'custom-string-too-long')).to be_nil
+    end
   end
 
 end