Maintenance: Improve handling and translatability of custom Rails model error messages.

.rubocop/cop/zammad/exists_condition.rb

@@ -31,7 +31,7 @@ module RuboCop
           }
         PATTERN
 
-        MSG = 'Use `%<prefer>s` instead of `%<current>s`.'.freeze
+        MSG = 'Use `%{prefer}` instead of `%{current}`.'.freeze
 
         def on_if(node)
           check_for_find_by(node)

.rubocop/default.yml

@@ -93,6 +93,11 @@ Style/RescueStandardError:
   Enabled: true
   EnforcedStyle: implicit
 
+Style/FormatStringToken:
+  Description: Use a consistent style for named format string tokens.
+  EnforcedStyle: template
+  # Prefer %{...} over %<...>s to make it easier for translators.
+
 Layout/SpaceInsideReferenceBrackets:
   Description: 'Checks the spacing inside referential brackets.'
   Enabled: false

app/jobs/issue_2715_fix_broken_twitter_urls_job.rb

@@ -1,8 +1,8 @@
 # Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
 
 class Issue2715FixBrokenTwitterUrlsJob < ApplicationJob
-  STATUS_TEMPLATE = 'https://twitter.com/_/status/%<message_id>s'.freeze
-  DM_TEMPLATE = 'https://twitter.com/messages/%<recipient_id>s-%<sender_id>s'.freeze
+  STATUS_TEMPLATE = 'https://twitter.com/_/status/%{message_id}'.freeze
+  DM_TEMPLATE = 'https://twitter.com/messages/%{recipient_id}-%{sender_id}'.freeze
 
   def perform
     Ticket::Article.joins(:type)

app/models/concerns/can_be_published.rb

@@ -99,19 +99,19 @@ module CanBePublished
   def archived_after_internal
     return if internal_at.nil? || archived_at.nil? || archived_at >= internal_at
 
-    errors.add(:archived_at, 'date must be no earlier than internal at date')
+    errors.add(:archived_at, __('date must be no earlier than internal date'))
   end
 
   def archived_after_published
     return if published_at.nil? || archived_at.nil? || archived_at >= published_at
 
-    errors.add(:archived_at, 'date must be no earlier than published at date')
+    errors.add(:archived_at, __('date must be no earlier than published date'))
   end
 
   def published_after_internal
     return if published_at.nil? || internal_at.nil? || published_at >= internal_at
 
-    errors.add(:published_at, 'date must be no earlier than internal at date')
+    errors.add(:published_at, __('date must be no earlier than internal date'))
   end
 
   def schedule_touch_for(attr)

app/models/concerns/has_group_relation_definition.rb

@@ -48,7 +48,7 @@ module HasGroupRelationDefinition
 
     return if !query.exists?
 
-    errors.add(:access, "#{group_relation_model_identifier.to_s.capitalize} can have full or granular access to group")
+    errors.add(:access, __('%{model} can have full or granular access to group'), model: group_relation_model_identifier.to_s.capitalize)
   end
 
   # methods defined here are going to extend the class, not the instance of it

app/models/data_privacy_task/validation.rb

@@ -20,7 +20,7 @@ class DataPrivacyTask::Validation < ActiveModel::Validator
     return if !record.deletable_type_changed?
     return if deletable_is_user?
 
-    invalid_because(:deletable, 'is not a User')
+    invalid_because(:deletable, __('is not a User'))
   end
 
   def check_for_system_user
@@ -28,7 +28,7 @@ class DataPrivacyTask::Validation < ActiveModel::Validator
     return if !deletable_is_user?
     return if deletable.id != 1
 
-    invalid_because(:deletable, 'is undeletable system User with ID 1')
+    invalid_because(:deletable, __('is undeletable system User with ID 1'))
   end
 
   def check_for_current_user
@@ -36,7 +36,7 @@ class DataPrivacyTask::Validation < ActiveModel::Validator
     return if !deletable_is_user?
     return if deletable.id != UserInfo.current_user_id
 
-    invalid_because(:deletable, 'is your current account')
+    invalid_because(:deletable, __('is your current account'))
   end
 
   def check_for_last_admin
@@ -44,7 +44,7 @@ class DataPrivacyTask::Validation < ActiveModel::Validator
     return if !deletable_is_user?
     return if !last_admin?
 
-    invalid_because(:deletable, 'is last account with admin permissions')
+    invalid_because(:deletable, __('is last account with admin permissions'))
   end
 
   def check_for_existing_task
@@ -52,7 +52,7 @@ class DataPrivacyTask::Validation < ActiveModel::Validator
     return if !deletable_is_user?
     return if !tasks_exists?
 
-    invalid_because(:deletable, 'has an existing DataPrivacyTask queued')
+    invalid_because(:deletable, __('has an existing DataPrivacyTask queued'))
   end
 
   def deletable_is_user?
@@ -63,8 +63,8 @@ class DataPrivacyTask::Validation < ActiveModel::Validator
     record.deletable
   end
 
-  def invalid_because(attribute, message)
-    record.errors.add attribute, message
+  def invalid_because(attribute, message, **options)
+    record.errors.add attribute, message, **options
   end
 
   def tasks_exists?

app/models/knowledge_base.rb

@@ -210,19 +210,19 @@ class KnowledgeBase < ApplicationModel
 
     # not domain, but no leading slash
     if custom_address.exclude?('.') && custom_address[0] != '/'
-      errors.add(:custom_address, 'must begin with a slash ("/").')
+      errors.add(:custom_address, __('must begin with a slash ("/")'))
     end
 
     if custom_address.include?('://')
-      errors.add(:custom_address, 'must not include a protocol (e.g., "http://" or "https://").')
+      errors.add(:custom_address, __('must not include a protocol (e.g., "http://" or "https://")'))
     end
 
     if custom_address.last == '/'
-      errors.add(:custom_address, 'must not end with a slash ("/").')
+      errors.add(:custom_address, __('must not end with a slash ("/")'))
     end
 
     if custom_address == '/' # rubocop:disable Style/GuardClause
-      errors.add(:custom_address, __('Please enter valid path or domain'))
+      errors.add(:custom_address, __('must be a valid path or domain'))
     end
   end
 

app/models/knowledge_base/category.rb

@@ -134,7 +134,7 @@ class KnowledgeBase::Category < ApplicationModel
   private
 
   def cannot_be_child_of_parent
-    errors.add(:parent_id, 'cannot be a child of the parent') if self_parent?(self)
+    errors.add(:parent_id, __('cannot be a child of the parent')) if self_parent?(self)
   end
   validate :cannot_be_child_of_parent
 

app/models/knowledge_base/has_unique_title.rb

@@ -19,7 +19,7 @@ class KnowledgeBase
            .neighbours_of(self)
            .none?
 
-      errors.add(:title, 'is already used')
+      errors.add(:title, __('is already used'))
     end
   end
 end

app/models/mention/validation.rb

@@ -4,6 +4,6 @@ class Mention::Validation < ActiveModel::Validator
   def validate(record)
     return if Mention.mentionable? record.mentionable, record.user
 
-    record.errors.add :user, 'has no agent access to this ticket'
+    record.errors.add :user, __('has no agent access to this ticket')
   end
 end