From 7fab92d0748575b82c812a34dca6c361d444f32e Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Mon, 27 Jan 2020 10:28:17 +0100 Subject: [PATCH] Fixes issue #2169 - Time format always english if generated from system. --- app/controllers/first_steps_controller.rb | 2 +- app/models/locale.rb | 8 +++ app/models/ticket.rb | 4 -- app/models/transaction/notification.rb | 2 +- app/models/transaction/slack.rb | 4 +- app/models/translation.rb | 7 ++- app/models/user.rb | 4 +- lib/calendar_subscriptions/tickets.rb | 6 +- lib/excel_sheet.rb | 2 +- lib/notification_factory.rb | 2 +- lib/notification_factory/mailer.rb | 2 +- lib/notification_factory/renderer.rb | 4 +- lib/notification_factory/slack.rb | 2 +- spec/models/locale_spec.rb | 23 ++++++++ spec/models/translation_spec.rb | 4 +- spec/models/trigger/sms_spec.rb | 70 +++++++++++++++++------ spec/models/user_spec.rb | 16 +++++- 17 files changed, 117 insertions(+), 45 deletions(-) create mode 100644 spec/models/locale_spec.rb diff --git a/app/controllers/first_steps_controller.rb b/app/controllers/first_steps_controller.rb index 53a350ef1..4af07aed1 100644 --- a/app/controllers/first_steps_controller.rb +++ b/app/controllers/first_steps_controller.rb @@ -177,7 +177,7 @@ class FirstStepsController < ApplicationController original_user_id = UserInfo.current_user_id result = NotificationFactory::Mailer.template( template: 'test_ticket', - locale: agent.preferences[:locale] || Setting.get('locale_default') || 'en-us', + locale: agent.locale, objects: { agent: agent, customer: customer, diff --git a/app/models/locale.rb b/app/models/locale.rb index 26dbcd1e7..a00258174 100644 --- a/app/models/locale.rb +++ b/app/models/locale.rb @@ -119,6 +119,14 @@ all: result.data end + # Default system locale + # + # @example + # Locale.default + def self.default + Setting.get('locale_default') || 'en-us' + end + private_class_method def self.to_database(data) ActiveRecord::Base.transaction do data.each do |locale| diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 873bf2757..cfd11bfde 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -1462,7 +1462,6 @@ result # get subject subject = NotificationFactory::Mailer.template( templateInline: value['subject'], - locale: 'en-en', objects: objects, quote: false, ) @@ -1470,7 +1469,6 @@ result body = NotificationFactory::Mailer.template( templateInline: value['body'], - locale: 'en-en', objects: objects, quote: true, ) @@ -1561,8 +1559,6 @@ result objects = build_notification_template_objects(article) body = NotificationFactory::Renderer.new( objects: objects, - locale: 'en-en', - timezone: Setting.get('timezone_default'), template: value['body'], escape: false ).render.html2text.tr(' ', ' ') # convert non-breaking space to simple space diff --git a/app/models/transaction/notification.rb b/app/models/transaction/notification.rb index d045ee5f6..dcf9bd57b 100644 --- a/app/models/transaction/notification.rb +++ b/app/models/transaction/notification.rb @@ -235,7 +235,7 @@ class Transaction::Notification return {} if !@item[:changes] - locale = user.preferences[:locale] || Setting.get('locale_default') || 'en-us' + locale = user.locale # only show allowed attributes attribute_list = ObjectManager::Attribute.by_object_as_hash('Ticket', user) diff --git a/app/models/transaction/slack.rb b/app/models/transaction/slack.rb index 19c984922..ab915301c 100644 --- a/app/models/transaction/slack.rb +++ b/app/models/transaction/slack.rb @@ -86,8 +86,8 @@ class Transaction::Slack result = NotificationFactory::Slack.template( template: template, - locale: user[:preferences][:locale] || Setting.get('locale_default'), - timezone: user[:preferences][:timezone] || Setting.get('timezone_default'), + locale: user.locale, + timezone: Setting.get('timezone_default'), objects: { ticket: ticket, article: article, diff --git a/app/models/translation.rb b/app/models/translation.rb index aa0ae6d96..4dd8d2432 100644 --- a/app/models/translation.rb +++ b/app/models/translation.rb @@ -237,14 +237,15 @@ or end end - record = Translation.where(locale: locale, source: 'timestamp', format: 'time').pluck(:target).first - return timestamp.to_s if !record - begin timestamp = timestamp.in_time_zone(timezone) rescue return timestamp.to_s end + + record = Translation.where(locale: locale, source: 'timestamp', format: 'time').pluck(:target).first + return timestamp.to_s if !record + record.sub!('dd', format('%02d', timestamp.day)) record.sub!('d', timestamp.day.to_s) record.sub!('mm', format('%02d', timestamp.month)) diff --git a/app/models/user.rb b/app/models/user.rb index 1ee4631a8..151a01a98 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -916,9 +916,9 @@ try to find correct name firstname.blank? && lastname.blank? end - # get locale of user or system if user's own is not set + # get locale identifier of user or system if user's own is not set def locale - preferences.fetch(:locale) { Setting.get('locale_default') } + preferences.fetch(:locale) { Locale.default } end private diff --git a/lib/calendar_subscriptions/tickets.rb b/lib/calendar_subscriptions/tickets.rb index 7c10af8e2..91058f990 100644 --- a/lib/calendar_subscriptions/tickets.rb +++ b/lib/calendar_subscriptions/tickets.rb @@ -76,7 +76,7 @@ class CalendarSubscriptions::Tickets condition: condition, ) - user_locale = @user.preferences['locale'] || Setting.get('locale_default') || 'en-us' + user_locale = @user.locale translated_ticket = Translation.translate(user_locale, 'ticket') events_data = [] @@ -126,7 +126,7 @@ class CalendarSubscriptions::Tickets condition: condition, ) - user_locale = @user.preferences['locale'] || Setting.get('locale_default') || 'en-us' + user_locale = @user.locale translated_ticket = Translation.translate(user_locale, 'ticket') customer = Translation.translate(user_locale, 'customer') @@ -183,7 +183,7 @@ class CalendarSubscriptions::Tickets condition: condition, ) - user_locale = @user.preferences['locale'] || Setting.get('locale_default') || 'en-us' + user_locale = @user.locale translated_ticket_escalation = Translation.translate(user_locale, 'ticket escalation') customer = Translation.translate(user_locale, 'customer') diff --git a/lib/excel_sheet.rb b/lib/excel_sheet.rb index 99e3ff710..ecf87b012 100644 --- a/lib/excel_sheet.rb +++ b/lib/excel_sheet.rb @@ -5,7 +5,7 @@ class ExcelSheet @header = header @records = records @timezone = timezone.presence || Setting.get('timezone_default') - @locale = locale || 'en-en' + @locale = locale || Locale.default @tempfile = Tempfile.new('excel-export.xls') @workbook = WriteExcel.new(@tempfile) @worksheet = @workbook.add_worksheet diff --git a/lib/notification_factory.rb b/lib/notification_factory.rb index 931799516..5831d0f5e 100644 --- a/lib/notification_factory.rb +++ b/lib/notification_factory.rb @@ -53,7 +53,7 @@ returns private_class_method :template_path def self.template_filenames(data) - locale = data[:locale] || Setting.get('locale_default') || 'en-us' + locale = data[:locale] || Locale.default [locale, locale[0, 2], 'en'] .uniq diff --git a/lib/notification_factory/mailer.rb b/lib/notification_factory/mailer.rb index bf201f924..b6760839f 100644 --- a/lib/notification_factory/mailer.rb +++ b/lib/notification_factory/mailer.rb @@ -295,7 +295,7 @@ returns end template = NotificationFactory.template_read( - locale: data[:locale] || Setting.get('locale_default') || 'en-us', + locale: data[:locale] || Locale.default, template: data[:template], format: data[:format] || 'html', type: 'mailer', diff --git a/lib/notification_factory/renderer.rb b/lib/notification_factory/renderer.rb index 2127c2473..37ba85a83 100644 --- a/lib/notification_factory/renderer.rb +++ b/lib/notification_factory/renderer.rb @@ -26,8 +26,8 @@ examples how to use =end def initialize(objects:, locale: nil, timezone: nil, template:, escape: true) - @objects = objects - @locale = locale || Setting.get('locale_default') || 'en-us' + @objects = objects + @locale = locale || Locale.default @timezone = timezone || Setting.get('timezone_default') @template = NotificationFactory::Template.new(template, escape) @escape = escape diff --git a/lib/notification_factory/slack.rb b/lib/notification_factory/slack.rb index 85542268e..51e918a4e 100644 --- a/lib/notification_factory/slack.rb +++ b/lib/notification_factory/slack.rb @@ -33,7 +33,7 @@ returns end template = NotificationFactory.template_read( - locale: data[:locale] || Setting.get('locale_default') || 'en-us', + locale: data[:locale] || Locale.default, template: data[:template], format: 'md', type: 'slack', diff --git a/spec/models/locale_spec.rb b/spec/models/locale_spec.rb new file mode 100644 index 000000000..f11225b81 --- /dev/null +++ b/spec/models/locale_spec.rb @@ -0,0 +1,23 @@ +require 'rails_helper' + +RSpec.describe Locale, type: :model do + describe 'Class methods:' do + describe '.default' do + context 'with default locale' do + before { Setting.set('locale_default', 'foo') } + + it 'returns the system-wide default locale' do + expect(described_class.default).to eq('foo') + end + end + + context 'without default locale' do + before { Setting.set('locale_default', nil) } + + it 'returns en-us' do + expect(described_class.default).to eq('en-us') + end + end + end + end +end diff --git a/spec/models/translation_spec.rb b/spec/models/translation_spec.rb index f823b735a..360a0864b 100644 --- a/spec/models/translation_spec.rb +++ b/spec/models/translation_spec.rb @@ -38,11 +38,11 @@ RSpec.describe Translation do end it 'not_existing with timestamp as string' do - expect(described_class.timestamp('not_existing', 'Europe/Berlin', '2018-10-10T10:00:00Z0')).to eq('2018-10-10 10:00:00 UTC') + expect(described_class.timestamp('not_existing', 'Europe/Berlin', '2018-10-10T10:00:00Z0')).to eq('2018-10-10 12:00:00 +0200') end it 'not_existing with time object' do - expect(described_class.timestamp('not_existing', 'Europe/Berlin', Time.zone.parse('2018-10-10T10:00:00Z0'))).to eq('2018-10-10 10:00:00 UTC') + expect(described_class.timestamp('not_existing', 'Europe/Berlin', Time.zone.parse('2018-10-10T10:00:00Z0'))).to eq('2018-10-10 12:00:00 +0200') end it 'not_existing with invalid timestamp string' do diff --git a/spec/models/trigger/sms_spec.rb b/spec/models/trigger/sms_spec.rb index 8c1854056..86bdffc51 100644 --- a/spec/models/trigger/sms_spec.rb +++ b/spec/models/trigger/sms_spec.rb @@ -3,29 +3,63 @@ require 'rails_helper' RSpec.describe Trigger do describe 'sms' do + before do + Translation.fetch(locale) + Setting.set('locale_default', locale) + Setting.set('timezone_default', time_zone) + end - it 'sends interpolated, html-free SMS' do - agent = create(:agent_user) - another_agent = create(:admin_user, mobile: '+37061010000') - Group.lookup(id: 1).users << another_agent + let(:time_zone) { 'Europe/Vilnius' } + let(:locale) { 'de-de' } - create(:channel, area: 'Sms::Notification') - create(:trigger, - disable_notification: false, - perform: { - 'notification.sms': { - recipient: 'ticket_agents', - body: 'space between #{ticket.title}', # rubocop:disable Lint/InterpolationCheck - } - }) + context 'sends interpolated, html-free SMS' do + before do + another_agent = create(:admin_user, mobile: '+37061010000') + Group.lookup(id: 1).users << another_agent - ticket = create(:ticket, group: Group.lookup(id: 1), created_by_id: agent.id) - Observer::Transaction.commit + create(:channel, area: 'Sms::Notification') + create(:trigger, + disable_notification: false, + perform: { + 'notification.sms': { + recipient: 'ticket_agents', + body: message_body, + } + }) + end - triggered_article = Ticket::Article.last + let(:message_body) { 'space between #{ticket.title} #{ticket.created_at}' } # rubocop:disable Lint/InterpolationCheck - expect(triggered_article.body).to match(/space between/) - expect(triggered_article.body).to match(ticket.title) + let(:agent) { create(:agent_user) } + let(:ticket) do + ticket = create(:ticket, group: Group.lookup(id: 1), created_by_id: agent.id) + Observer::Transaction.commit + ticket + end + + let(:triggered_article) do + ticket.articles.last + end + + it 'renders HTML chars' do + expect(triggered_article.body).to match(/space between/) + end + + it 'interpolates ticket properties' do + expect(triggered_article.body).to match(ticket.title) + end + + it 'interpolates time in selected time zone' do + time_in_zone = triggered_article.ticket.created_at.in_time_zone(time_zone) + + expect(triggered_article.body).to match(time_in_zone.strftime('%H:%M')) + end + + it 'interpolates date in selected locale format' do + time_in_zone = triggered_article.ticket.created_at.in_time_zone(time_zone) + + expect(triggered_article.body).to match(time_in_zone.strftime('%d.%m.%y')) + end end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c48892b2e..8c515559f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -647,10 +647,20 @@ RSpec.describe User, type: :model do context 'with no #preferences[:locale]' do let(:preferences) { {} } - before { Setting.set('locale_default', 'foo') } + context 'with default locale' do + before { Setting.set('locale_default', 'foo') } - it 'returns the system-wide default locale' do - expect(user.locale).to eq('foo') + it 'returns the system-wide default locale' do + expect(user.locale).to eq('foo') + end + end + + context 'without default locale' do + before { Setting.set('locale_default', nil) } + + it 'returns en-us' do + expect(user.locale).to eq('en-us') + end end end