From 4855fee3d515f8dac7dc6f52b8b0a54353f56abf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 3 Apr 2024 22:08:57 +0200 Subject: [PATCH 01/22] Remove unused code in globalize helper This code isn't used since commit 041abe904. --- app/helpers/globalize_helper.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/helpers/globalize_helper.rb b/app/helpers/globalize_helper.rb index b18762550..2aab2c593 100644 --- a/app/helpers/globalize_helper.rb +++ b/app/helpers/globalize_helper.rb @@ -58,10 +58,6 @@ module GlobalizeHelper end end - def translations_for_locale?(resource) - resource.locales_not_marked_for_destruction.any? - end - def selected_languages_description(resource) sanitize(t("shared.translations.languages_in_use", count: active_languages_count(resource))) end From 161eaaff8b28f0452e6edc383f2b317ceeee5b96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 3 Apr 2024 22:31:12 +0200 Subject: [PATCH 02/22] Remove redundant condition rendering globalize locales The `manage_languages` variables is never defined when calling the site customization information texts globalize locales partial. --- .../information_texts/_globalize_locales.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb b/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb index da4297ad1..a8afbbebc 100644 --- a/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb +++ b/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb @@ -1,4 +1,4 @@ <%= render "shared/common_globalize_locales", resource: nil, display_style: lambda { |locale| site_customization_display_translation_style(locale) }, - manage_languages: defined?(manage_languages) ? manage_languages : true %> + manage_languages: true %> From 9a4a7bd56e208b9a4eba4ffb84d6282513d60652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 3 Apr 2024 22:42:44 +0200 Subject: [PATCH 03/22] Remove obsolete parameter rendering globalize locales This parameter isn't used since commit b86c0d3c3, which deleted a file that wasn't used since commit 146c09adb. Further proof that this code wasn't used is the fact that the `enable_translation_style` method, which this code called, was removed in commit 5ada97544. --- .../information_texts/form_field_component.html.erb | 2 +- .../information_texts/form_field_component.rb | 6 +++++- app/helpers/site_customization_helper.rb | 4 ---- app/views/admin/legislation/homepages/_form.html.erb | 5 +---- .../admin/legislation/milestones/_summary_form.html.erb | 5 +---- .../information_texts/_globalize_locales.html.erb | 5 +---- app/views/shared/_globalize_locales.html.erb | 1 - 7 files changed, 9 insertions(+), 19 deletions(-) diff --git a/app/components/admin/site_customization/information_texts/form_field_component.html.erb b/app/components/admin/site_customization/information_texts/form_field_component.html.erb index 2bda944d7..1852afba0 100644 --- a/app/components/admin/site_customization/information_texts/form_field_component.html.erb +++ b/app/components/admin/site_customization/information_texts/form_field_component.html.erb @@ -4,6 +4,6 @@ text, rows: 5, class: "js-globalize-attribute", - style: site_customization_display_translation_style(locale), + style: site_customization_display_translation_style, data: { locale: locale } %> <% end %> diff --git a/app/components/admin/site_customization/information_texts/form_field_component.rb b/app/components/admin/site_customization/information_texts/form_field_component.rb index f85c1c2ca..69ce896ef 100644 --- a/app/components/admin/site_customization/information_texts/form_field_component.rb +++ b/app/components/admin/site_customization/information_texts/form_field_component.rb @@ -1,6 +1,6 @@ class Admin::SiteCustomization::InformationTexts::FormFieldComponent < ApplicationComponent attr_reader :i18n_content, :locale - use_helpers :globalize, :site_customization_display_translation_style + use_helpers :globalize, :site_customization_enable_translation? def initialize(i18n_content, locale:) @i18n_content = i18n_content @@ -22,4 +22,8 @@ class Admin::SiteCustomization::InformationTexts::FormFieldComponent < Applicati def i18n_text I18n.translate(i18n_content.key, locale: locale) end + + def site_customization_display_translation_style + site_customization_enable_translation?(locale) ? "" : "display: none;" + end end diff --git a/app/helpers/site_customization_helper.rb b/app/helpers/site_customization_helper.rb index 72f00951f..a002071dd 100644 --- a/app/helpers/site_customization_helper.rb +++ b/app/helpers/site_customization_helper.rb @@ -3,10 +3,6 @@ module SiteCustomizationHelper I18nContentTranslation.existing_languages.include?(locale) || locale == I18n.locale end - def site_customization_display_translation_style(locale) - site_customization_enable_translation?(locale) ? "" : "display: none;" - end - def information_texts_tabs [:basic, :debates, :community, :proposals, :polls, :legislation, :budgets, :layouts, :mailers, :management, :welcome, :machine_learning] diff --git a/app/views/admin/legislation/homepages/_form.html.erb b/app/views/admin/legislation/homepages/_form.html.erb index 4a752569c..b2a3e42e5 100644 --- a/app/views/admin/legislation/homepages/_form.html.erb +++ b/app/views/admin/legislation/homepages/_form.html.erb @@ -1,7 +1,4 @@ -<%= render "shared/globalize_locales", - resource: @process, - display_style: lambda { |locale| enable_translation_style(@process, locale) }, - manage_languages: false %> +<%= render "shared/globalize_locales", resource: @process, manage_languages: false %> <%= translatable_form_for [:admin, @process], url: url do |f| %> diff --git a/app/views/admin/legislation/milestones/_summary_form.html.erb b/app/views/admin/legislation/milestones/_summary_form.html.erb index 27442d377..1e6b0b4a9 100644 --- a/app/views/admin/legislation/milestones/_summary_form.html.erb +++ b/app/views/admin/legislation/milestones/_summary_form.html.erb @@ -1,7 +1,4 @@ -<%= render "shared/globalize_locales", - resource: @process, - display_style: lambda { |locale| enable_translation_style(@process, locale) }, - manage_languages: false %> +<%= render "shared/globalize_locales", resource: @process, manage_languages: false %> <%= translatable_form_for [:admin, @process] do |f| %>
diff --git a/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb b/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb index a8afbbebc..8462c141a 100644 --- a/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb +++ b/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb @@ -1,4 +1 @@ -<%= render "shared/common_globalize_locales", - resource: nil, - display_style: lambda { |locale| site_customization_display_translation_style(locale) }, - manage_languages: true %> +<%= render "shared/common_globalize_locales", resource: nil, manage_languages: true %> diff --git a/app/views/shared/_globalize_locales.html.erb b/app/views/shared/_globalize_locales.html.erb index 88aa4064b..32f1b9ce0 100644 --- a/app/views/shared/_globalize_locales.html.erb +++ b/app/views/shared/_globalize_locales.html.erb @@ -1,6 +1,5 @@ <% if translations_interface_enabled? %> <%= render "shared/common_globalize_locales", resource: resource, - display_style: lambda { |locale| enable_translation_style(resource, locale) }, manage_languages: defined?(manage_languages) ? manage_languages : true %> <% end %> From 5f09718e77c996b77dafb6d6ab5fee0eba5f7185 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 3 Apr 2024 22:17:01 +0200 Subject: [PATCH 04/22] Move globalize locales partial to a component --- .../globalize_locales_component.html.erb} | 8 +- .../shared/globalize_locales_component.rb | 91 +++++++++++++++++++ app/helpers/globalize_helper.rb | 79 ---------------- .../_globalize_locales.html.erb | 2 +- app/views/shared/_globalize_locales.html.erb | 7 +- 5 files changed, 100 insertions(+), 87 deletions(-) rename app/{views/shared/_common_globalize_locales.html.erb => components/shared/globalize_locales_component.html.erb} (83%) create mode 100644 app/components/shared/globalize_locales_component.rb diff --git a/app/views/shared/_common_globalize_locales.html.erb b/app/components/shared/globalize_locales_component.html.erb similarity index 83% rename from app/views/shared/_common_globalize_locales.html.erb rename to app/components/shared/globalize_locales_component.html.erb index 2ead5fd8c..aa73c9b24 100644 --- a/app/views/shared/_common_globalize_locales.html.erb +++ b/app/components/shared/globalize_locales_component.html.erb @@ -3,19 +3,19 @@ "> - <%= selected_languages_description(resource) %> + <%= selected_languages_description %> <%= select_tag :select_language, - options_for_select_language(resource), + options_for_select_language, prompt: t("shared.translations.select_language_prompt"), class: "js-select-language" %> - <%= select_language_error(resource) %> + <%= select_language_error %>
<% if manage_languages %> <% I18n.available_locales.each do |locale| %> <%= link_to t("shared.translations.remove_language"), "#", - style: display_destroy_locale_style(resource, locale), + style: display_destroy_locale_style(locale), class: "delete js-delete-language js-delete-#{locale}", data: { locale: locale } %> <% end %> diff --git a/app/components/shared/globalize_locales_component.rb b/app/components/shared/globalize_locales_component.rb new file mode 100644 index 000000000..8060cadc2 --- /dev/null +++ b/app/components/shared/globalize_locales_component.rb @@ -0,0 +1,91 @@ +class Shared::GlobalizeLocalesComponent < ApplicationComponent + attr_reader :resource, :manage_languages + use_helpers :first_translation, :first_marked_for_destruction_translation, + :enabled_locale?, :name_for_locale, :highlight_translation_html_class + + def initialize(resource = nil, manage_languages: true) + @resource = resource + @manage_languages = manage_languages + end + + private + + def options_for_select_language + options_for_select(available_locales, selected_locale) + end + + def available_locales + I18n.available_locales.select { |locale| enabled_locale?(resource, locale) }.map do |locale| + [name_for_locale(locale), locale, { data: { locale: locale }}] + end + end + + def selected_locale + return first_i18n_content_translation_locale if resource.blank? + + if resource.locales_not_marked_for_destruction.any? + first_translation(resource) + elsif resource.locales_persisted_and_marked_for_destruction.any? + first_marked_for_destruction_translation(resource) + else + I18n.locale + end + end + + def first_i18n_content_translation_locale + if I18nContentTranslation.existing_languages.count == 0 || + I18nContentTranslation.existing_languages.include?(I18n.locale) + I18n.locale + else + I18nContentTranslation.existing_languages.first + end + end + + def selected_languages_description + sanitize(t("shared.translations.languages_in_use", count: active_languages_count)) + end + + def select_language_error + return if resource.blank? + + current_translation = resource.translation_for(selected_locale) + if current_translation.errors.added? :base, :translations_too_short + tag.div class: "small error" do + current_translation.errors[:base].join(", ") + end + end + end + + def active_languages_count + if resource.blank? + no_resource_languages_count + elsif resource.locales_not_marked_for_destruction.size > 0 + resource.locales_not_marked_for_destruction.size + else + 1 + end + end + + def no_resource_languages_count + count = I18nContentTranslation.existing_languages.count + count > 0 ? count : 1 + end + + def display_destroy_locale_style(locale) + "display: none;" unless display_destroy_locale_link?(locale) + end + + def display_destroy_locale_link?(locale) + selected_locale == locale + end + + def options_for_add_language + options_for_select(all_language_options, nil) + end + + def all_language_options + I18n.available_locales.map do |locale| + [name_for_locale(locale), locale] + end + end +end diff --git a/app/helpers/globalize_helper.rb b/app/helpers/globalize_helper.rb index 2aab2c593..1d824fd06 100644 --- a/app/helpers/globalize_helper.rb +++ b/app/helpers/globalize_helper.rb @@ -1,14 +1,4 @@ module GlobalizeHelper - def options_for_select_language(resource) - options_for_select(available_locales(resource), selected_locale(resource)) - end - - def available_locales(resource) - I18n.available_locales.select { |locale| enabled_locale?(resource, locale) }.map do |locale| - [name_for_locale(locale), locale, { data: { locale: locale }}] - end - end - def enabled_locale?(resource, locale) return site_customization_enable_translation?(locale) if resource.blank? @@ -21,27 +11,6 @@ module GlobalizeHelper end end - def selected_locale(resource) - return first_i18n_content_translation_locale if resource.blank? - - if resource.locales_not_marked_for_destruction.any? - first_translation(resource) - elsif resource.locales_persisted_and_marked_for_destruction.any? - first_marked_for_destruction_translation(resource) - else - I18n.locale - end - end - - def first_i18n_content_translation_locale - if I18nContentTranslation.existing_languages.count == 0 || - I18nContentTranslation.existing_languages.include?(I18n.locale) - I18n.locale - else - I18nContentTranslation.existing_languages.first - end - end - def first_translation(resource) if resource.locales_not_marked_for_destruction.include? I18n.locale I18n.locale @@ -58,36 +27,6 @@ module GlobalizeHelper end end - def selected_languages_description(resource) - sanitize(t("shared.translations.languages_in_use", count: active_languages_count(resource))) - end - - def select_language_error(resource) - return if resource.blank? - - current_translation = resource.translation_for(selected_locale(resource)) - if current_translation.errors.added? :base, :translations_too_short - tag.div class: "small error" do - current_translation.errors[:base].join(", ") - end - end - end - - def active_languages_count(resource) - if resource.blank? - no_resource_languages_count - elsif resource.locales_not_marked_for_destruction.size > 0 - resource.locales_not_marked_for_destruction.size - else - 1 - end - end - - def no_resource_languages_count - count = I18nContentTranslation.existing_languages.count - count > 0 ? count : 1 - end - def display_translation_style(resource, locale) "display: none;" unless display_translation?(resource, locale) end @@ -104,24 +43,6 @@ module GlobalizeHelper end end - def display_destroy_locale_style(resource, locale) - "display: none;" unless display_destroy_locale_link?(resource, locale) - end - - def display_destroy_locale_link?(resource, locale) - selected_locale(resource) == locale - end - - def options_for_add_language - options_for_select(all_language_options, nil) - end - - def all_language_options - I18n.available_locales.map do |locale| - [name_for_locale(locale), locale] - end - end - def translation_enabled_tag(locale, enabled) hidden_field_tag("enabled_translations[#{locale}]", (enabled ? 1 : 0)) end diff --git a/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb b/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb index 8462c141a..1d38894fb 100644 --- a/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb +++ b/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb @@ -1 +1 @@ -<%= render "shared/common_globalize_locales", resource: nil, manage_languages: true %> +<%= render Shared::GlobalizeLocalesComponent.new %> diff --git a/app/views/shared/_globalize_locales.html.erb b/app/views/shared/_globalize_locales.html.erb index 32f1b9ce0..e35125c6f 100644 --- a/app/views/shared/_globalize_locales.html.erb +++ b/app/views/shared/_globalize_locales.html.erb @@ -1,5 +1,6 @@ <% if translations_interface_enabled? %> - <%= render "shared/common_globalize_locales", - resource: resource, - manage_languages: defined?(manage_languages) ? manage_languages : true %> + <%= render Shared::GlobalizeLocalesComponent.new( + resource, + manage_languages: defined?(manage_languages) ? manage_languages : true + ) %> <% end %> From b451a251fc4f00657aecc98d87575ca3cdcdf91a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 24 May 2024 17:16:22 +0200 Subject: [PATCH 05/22] Rename methods that returns an array of locales Having `_languages` in the method name made it harder to know what that method was returning. --- .../shared/globalize_locales_component.rb | 16 ++++++++-------- app/helpers/site_customization_helper.rb | 2 +- app/models/i18n_content_translation.rb | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/components/shared/globalize_locales_component.rb b/app/components/shared/globalize_locales_component.rb index 8060cadc2..49034ecc4 100644 --- a/app/components/shared/globalize_locales_component.rb +++ b/app/components/shared/globalize_locales_component.rb @@ -33,16 +33,16 @@ class Shared::GlobalizeLocalesComponent < ApplicationComponent end def first_i18n_content_translation_locale - if I18nContentTranslation.existing_languages.count == 0 || - I18nContentTranslation.existing_languages.include?(I18n.locale) + if I18nContentTranslation.existing_locales.count == 0 || + I18nContentTranslation.existing_locales.include?(I18n.locale) I18n.locale else - I18nContentTranslation.existing_languages.first + I18nContentTranslation.existing_locales.first end end def selected_languages_description - sanitize(t("shared.translations.languages_in_use", count: active_languages_count)) + sanitize(t("shared.translations.languages_in_use", count: active_locales_count)) end def select_language_error @@ -56,9 +56,9 @@ class Shared::GlobalizeLocalesComponent < ApplicationComponent end end - def active_languages_count + def active_locales_count if resource.blank? - no_resource_languages_count + no_resource_locales_count elsif resource.locales_not_marked_for_destruction.size > 0 resource.locales_not_marked_for_destruction.size else @@ -66,8 +66,8 @@ class Shared::GlobalizeLocalesComponent < ApplicationComponent end end - def no_resource_languages_count - count = I18nContentTranslation.existing_languages.count + def no_resource_locales_count + count = I18nContentTranslation.existing_locales.count count > 0 ? count : 1 end diff --git a/app/helpers/site_customization_helper.rb b/app/helpers/site_customization_helper.rb index a002071dd..61dd678e9 100644 --- a/app/helpers/site_customization_helper.rb +++ b/app/helpers/site_customization_helper.rb @@ -1,6 +1,6 @@ module SiteCustomizationHelper def site_customization_enable_translation?(locale) - I18nContentTranslation.existing_languages.include?(locale) || locale == I18n.locale + I18nContentTranslation.existing_locales.include?(locale) || locale == I18n.locale end def information_texts_tabs diff --git a/app/models/i18n_content_translation.rb b/app/models/i18n_content_translation.rb index 4e754a3a5..e15d3e54b 100644 --- a/app/models/i18n_content_translation.rb +++ b/app/models/i18n_content_translation.rb @@ -1,5 +1,5 @@ class I18nContentTranslation < ApplicationRecord - def self.existing_languages + def self.existing_locales self.select(:locale).distinct.map { |l| l.locale.to_sym }.to_a end end From b188c5cae0a0c36aa9dee8136e466bf6b6c975d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 24 May 2024 17:23:57 +0200 Subject: [PATCH 06/22] Simplify method to get existing locales We can get the same results using `pluck`. --- app/models/i18n_content_translation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/i18n_content_translation.rb b/app/models/i18n_content_translation.rb index e15d3e54b..165dd8a60 100644 --- a/app/models/i18n_content_translation.rb +++ b/app/models/i18n_content_translation.rb @@ -1,5 +1,5 @@ class I18nContentTranslation < ApplicationRecord def self.existing_locales - self.select(:locale).distinct.map { |l| l.locale.to_sym }.to_a + distinct.pluck(:locale).map(&:to_sym) end end From a2177b457596e61f760f984f66c52b8924089531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 24 May 2024 17:35:50 +0200 Subject: [PATCH 07/22] Refactor methods to get active locales count The code is now a bit more readable. --- .../shared/globalize_locales_component.rb | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/app/components/shared/globalize_locales_component.rb b/app/components/shared/globalize_locales_component.rb index 49034ecc4..8b551ceb6 100644 --- a/app/components/shared/globalize_locales_component.rb +++ b/app/components/shared/globalize_locales_component.rb @@ -21,7 +21,7 @@ class Shared::GlobalizeLocalesComponent < ApplicationComponent end def selected_locale - return first_i18n_content_translation_locale if resource.blank? + return first_i18n_content_locale if resource.blank? if resource.locales_not_marked_for_destruction.any? first_translation(resource) @@ -32,12 +32,11 @@ class Shared::GlobalizeLocalesComponent < ApplicationComponent end end - def first_i18n_content_translation_locale - if I18nContentTranslation.existing_locales.count == 0 || - I18nContentTranslation.existing_locales.include?(I18n.locale) + def first_i18n_content_locale + if i18n_content_locales.empty? || i18n_content_locales.include?(I18n.locale) I18n.locale else - I18nContentTranslation.existing_locales.first + i18n_content_locales.first end end @@ -57,18 +56,19 @@ class Shared::GlobalizeLocalesComponent < ApplicationComponent end def active_locales_count - if resource.blank? - no_resource_locales_count - elsif resource.locales_not_marked_for_destruction.size > 0 - resource.locales_not_marked_for_destruction.size + if active_locales.size > 0 + active_locales.size else 1 end end - def no_resource_locales_count - count = I18nContentTranslation.existing_locales.count - count > 0 ? count : 1 + def active_locales + if resource.present? + resource.locales_not_marked_for_destruction + else + i18n_content_locales + end end def display_destroy_locale_style(locale) @@ -88,4 +88,8 @@ class Shared::GlobalizeLocalesComponent < ApplicationComponent [name_for_locale(locale), locale] end end + + def i18n_content_locales + I18nContentTranslation.existing_locales + end end From 3e13f93ebdd1886f7296343e8c15932a69169170 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 22 May 2024 00:01:18 +0200 Subject: [PATCH 08/22] Add controller tests for switch_locale This way it'll be easier to change it while checking we haven't broken existing behavior. While writing the tests, I noticed we were sometimes storing a symbol in the session while sometimes we were storing a string. So we're adding a `to_s` call so we always store a string in the session. --- app/controllers/application_controller.rb | 2 +- app/controllers/management/base_controller.rb | 4 +- app/controllers/subscriptions_controller.rb | 2 +- .../application_controller_spec.rb | 54 +++++++++++++++++++ .../management/base_controller_spec.rb | 44 +++++++++++++++ .../subscriptions_controller_spec.rb | 8 +++ 6 files changed, 110 insertions(+), 4 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 082c74754..f05714b53 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -46,7 +46,7 @@ class ApplicationController < ActionController::Base current_user.update(locale: locale) end - session[:locale] = locale + session[:locale] = locale.to_s I18n.with_locale(locale, &action) end diff --git a/app/controllers/management/base_controller.rb b/app/controllers/management/base_controller.rb index b408d01fa..185a0fc07 100644 --- a/app/controllers/management/base_controller.rb +++ b/app/controllers/management/base_controller.rb @@ -41,10 +41,10 @@ class Management::BaseController < ActionController::Base def switch_locale(&action) if params[:locale] && I18n.available_locales.include?(params[:locale].to_sym) - session[:locale] = params[:locale] + session[:locale] = params[:locale].to_s end - session[:locale] ||= I18n.default_locale + session[:locale] ||= I18n.default_locale.to_s I18n.with_locale(session[:locale], &action) end diff --git a/app/controllers/subscriptions_controller.rb b/app/controllers/subscriptions_controller.rb index 163b913b8..acbdb02d8 100644 --- a/app/controllers/subscriptions_controller.rb +++ b/app/controllers/subscriptions_controller.rb @@ -32,7 +32,7 @@ class SubscriptionsController < ApplicationController def set_user_locale(&action) if params[:locale].blank? - session[:locale] = I18n.available_locales.find { |locale| locale == @user.locale&.to_sym } + session[:locale] = I18n.available_locales.find { |locale| locale == @user.locale&.to_sym }.to_s end I18n.with_locale(session[:locale], &action) end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index c6d77bfe4..6e46ab7d7 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -1,6 +1,14 @@ require "rails_helper" describe ApplicationController do + controller do + skip_authorization_check + + def index + render plain: I18n.locale + end + end + describe "#current_budget" do it "returns the last budget that is not in draft phase" do create(:budget, :finished, created_at: 2.years.ago, name: "Old") @@ -12,4 +20,50 @@ describe ApplicationController do expect(budget.name).to eq("Current") end end + + describe "#switch_locale" do + it "uses the default locale by default" do + get :index + + expect(response.body).to eq "en" + end + + it "uses the locale in the parameters when it's there" do + get :index, params: { locale: :es } + + expect(response.body).to eq "es" + end + + it "uses the locale in the session if there are no parameters" do + get :index, params: { locale: :es } + + expect(response.body).to eq "es" + + get :index + + expect(response.body).to eq "es" + end + + it "uses the locale in the parameters even when it's in the session" do + get :index + + expect(response.body).to eq "en" + + get :index, params: { locale: :es } + + expect(response.body).to eq "es" + end + + context "authenticated user" do + let(:user) { create(:user) } + before { sign_in(user) } + + it "updates the prefered locale when it's in the parameters" do + get :index, params: { locale: :es } + + expect(user.reload.locale).to eq "es" + expect(response.body).to eq "es" + end + end + end end diff --git a/spec/controllers/management/base_controller_spec.rb b/spec/controllers/management/base_controller_spec.rb index da0a15cbb..0aa4202fe 100644 --- a/spec/controllers/management/base_controller_spec.rb +++ b/spec/controllers/management/base_controller_spec.rb @@ -1,6 +1,16 @@ require "rails_helper" describe Management::BaseController do + before { session[:manager] = double } + + controller do + skip_authorization_check + + def index + render plain: I18n.locale + end + end + describe "managed_user" do it "returns existent user with session document info if present" do session[:document_type] = "1" @@ -21,4 +31,38 @@ describe Management::BaseController do expect(managed_user.document_number).to eq "333333333E" end end + + describe "#switch_locale" do + it "uses the default locale by default" do + get :index + + expect(response.body).to eq "en" + end + + it "uses the locale in the parameters when it's there" do + get :index, params: { locale: :es } + + expect(response.body).to eq "es" + end + + it "uses the locale in the session if there are no parameters" do + get :index, params: { locale: :es } + + expect(response.body).to eq "es" + + get :index + + expect(response.body).to eq "es" + end + + it "uses the locale in the parameters even when it's in the session" do + get :index + + expect(response.body).to eq "en" + + get :index, params: { locale: :es } + + expect(response.body).to eq "es" + end + end end diff --git a/spec/controllers/subscriptions_controller_spec.rb b/spec/controllers/subscriptions_controller_spec.rb index 3c1d39414..dd1934dd6 100644 --- a/spec/controllers/subscriptions_controller_spec.rb +++ b/spec/controllers/subscriptions_controller_spec.rb @@ -19,5 +19,13 @@ describe SubscriptionsController do expect(response).to redirect_to "/" expect(flash[:alert]).to eq "No tienes permiso para acceder a esta página." end + + it "uses the user locale where there's no locale in the parameters" do + create(:user, locale: "es", subscriptions_token: "mytoken") + + get :edit, params: { token: "mytoken" } + + expect(session[:locale]).to eq "es" + end end end From 709f39c6ce6bb265269c4c6486d30d84e4f774a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 22 May 2024 16:53:37 +0200 Subject: [PATCH 09/22] Handle unavailable locales in subscriptions There was an edge case where a user could configure a locale and then the application would change the locales to that one would no longer be available. In that case, we were getting a `I18n::InvalidLocale` exception when accessing the subscriptions page. So now, we're defaulting to `I18n.locale`. Note we're using `I18n.locale`instead of `I18n.default_locale` because `set_user_locale` is called inside the `switch_locale` block in `ApplicationController`, which already sets `I18n.locale` based on `I18n.default_locale`. --- app/controllers/subscriptions_controller.rb | 6 +++++- spec/controllers/subscriptions_controller_spec.rb | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/controllers/subscriptions_controller.rb b/app/controllers/subscriptions_controller.rb index acbdb02d8..315905001 100644 --- a/app/controllers/subscriptions_controller.rb +++ b/app/controllers/subscriptions_controller.rb @@ -32,8 +32,12 @@ class SubscriptionsController < ApplicationController def set_user_locale(&action) if params[:locale].blank? - session[:locale] = I18n.available_locales.find { |locale| locale == @user.locale&.to_sym }.to_s + session[:locale] = find_locale.to_s end I18n.with_locale(session[:locale], &action) end + + def find_locale + I18n.available_locales.find { |locale| locale == @user.locale&.to_sym } || I18n.locale + end end diff --git a/spec/controllers/subscriptions_controller_spec.rb b/spec/controllers/subscriptions_controller_spec.rb index dd1934dd6..1e1124125 100644 --- a/spec/controllers/subscriptions_controller_spec.rb +++ b/spec/controllers/subscriptions_controller_spec.rb @@ -27,5 +27,13 @@ describe SubscriptionsController do expect(session[:locale]).to eq "es" end + + it "only accepts available locales" do + create(:user, locale: "wl", subscriptions_token: "mytoken") + + get :edit, params: { token: "mytoken" } + + expect(session[:locale]).to eq "en" + end end end From e7d2cfbf232dc5d83cbc79736f5abc7e1671c02b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 23 May 2024 19:18:36 +0200 Subject: [PATCH 10/22] Add missing action to content blocks controller The code works without this action because both the route and the view are specified, and by default Rails renders the view when there's no action defined. However, the code is easier to understand if the action is present in the controller, which is what we do most of the time. --- .../admin/site_customization/content_blocks_controller.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/controllers/admin/site_customization/content_blocks_controller.rb b/app/controllers/admin/site_customization/content_blocks_controller.rb index 7384551f9..9334ee656 100644 --- a/app/controllers/admin/site_customization/content_blocks_controller.rb +++ b/app/controllers/admin/site_customization/content_blocks_controller.rb @@ -11,6 +11,9 @@ class Admin::SiteCustomization::ContentBlocksController < Admin::SiteCustomizati @headings_content_blocks = Budget::ContentBlock.all end + def new + end + def create if is_heading_content_block?(@content_block.name) heading_content_block = new_heading_content_block From 1898bdec56c0ef782adcb103e803c48e40d7d2fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 23 May 2024 19:45:29 +0200 Subject: [PATCH 11/22] Remove redundant conditions in content blocks controller In the case of the `edit` action, we're using `load_and_authorize_resource`, which will always return a `SiteCustomization::ContentBlock`. In the case of `edit_heading_content_block`, we're using `Budget::ContentBlock.find`, which always returns a `Budget::ContentBlock` (or raises an exception). So, in both cases, the condition to assign `@selected_content_block` can be removed. --- .../site_customization/content_blocks_controller.rb | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/site_customization/content_blocks_controller.rb b/app/controllers/admin/site_customization/content_blocks_controller.rb index 9334ee656..d4a01c01b 100644 --- a/app/controllers/admin/site_customization/content_blocks_controller.rb +++ b/app/controllers/admin/site_customization/content_blocks_controller.rb @@ -34,11 +34,7 @@ class Admin::SiteCustomization::ContentBlocksController < Admin::SiteCustomizati end def edit - if @content_block.is_a? SiteCustomization::ContentBlock - @selected_content_block = @content_block.name - else - @selected_content_block = "hcb_#{@content_block.heading_id}" - end + @selected_content_block = @content_block.name end def update @@ -75,11 +71,7 @@ class Admin::SiteCustomization::ContentBlocksController < Admin::SiteCustomizati def edit_heading_content_block @content_block = Budget::ContentBlock.find(params[:id]) - if @content_block.is_a? Budget::ContentBlock - @selected_content_block = "hcb_#{@content_block.heading_id}" - else - @selected_content_block = @content_block.name - end + @selected_content_block = "hcb_#{@content_block.heading_id}" @is_heading_content_block = true render :edit end From 2a5edbb5dd281df1a961807ef752f7378fd1fe0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 23 May 2024 19:30:26 +0200 Subject: [PATCH 12/22] Move content blocks forms partials to components This way we can simplify the controller a little bit, and it'll be easier to write tests for them when we change the code. --- .../form_content_block_component.html.erb} | 6 +++--- .../content_blocks/form_content_block_component.rb | 14 ++++++++++++++ .../form_heading_content_block_component.html.erb} | 10 +++++----- .../form_heading_content_block_component.rb | 14 ++++++++++++++ .../content_blocks_controller.rb | 2 -- .../content_blocks/_form.html.erb | 4 ++-- 6 files changed, 38 insertions(+), 12 deletions(-) rename app/{views/admin/site_customization/content_blocks/_form_content_block.html.erb => components/admin/site_customization/content_blocks/form_content_block_component.html.erb} (61%) create mode 100644 app/components/admin/site_customization/content_blocks/form_content_block_component.rb rename app/{views/admin/site_customization/content_blocks/_form_heading_content_block.html.erb => components/admin/site_customization/content_blocks/form_heading_content_block_component.html.erb} (64%) create mode 100644 app/components/admin/site_customization/content_blocks/form_heading_content_block_component.rb diff --git a/app/views/admin/site_customization/content_blocks/_form_content_block.html.erb b/app/components/admin/site_customization/content_blocks/form_content_block_component.html.erb similarity index 61% rename from app/views/admin/site_customization/content_blocks/_form_content_block.html.erb rename to app/components/admin/site_customization/content_blocks/form_content_block_component.html.erb index 01f61438e..0f36892b6 100644 --- a/app/views/admin/site_customization/content_blocks/_form_content_block.html.erb +++ b/app/components/admin/site_customization/content_blocks/form_content_block_component.html.erb @@ -1,9 +1,9 @@ -<%= form_for [:admin, @content_block], html: { class: "edit_page" } do |f| %> +<%= form_for [:admin, content_block], html: { class: "edit_page" } do |f| %> - <%= render "shared/errors", resource: @content_block %> + <%= render "shared/errors", resource: content_block %>
- <%= f.select :name, options_for_select(valid_blocks, @selected_content_block) %> + <%= f.select :name, options_for_select(valid_blocks, selected_content_block) %>
<%= f.select :locale, I18n.available_locales %> diff --git a/app/components/admin/site_customization/content_blocks/form_content_block_component.rb b/app/components/admin/site_customization/content_blocks/form_content_block_component.rb new file mode 100644 index 000000000..59218499c --- /dev/null +++ b/app/components/admin/site_customization/content_blocks/form_content_block_component.rb @@ -0,0 +1,14 @@ +class Admin::SiteCustomization::ContentBlocks::FormContentBlockComponent < ApplicationComponent + attr_reader :content_block + use_helpers :valid_blocks + + def initialize(content_block) + @content_block = content_block + end + + private + + def selected_content_block + content_block.name + end +end diff --git a/app/views/admin/site_customization/content_blocks/_form_heading_content_block.html.erb b/app/components/admin/site_customization/content_blocks/form_heading_content_block_component.html.erb similarity index 64% rename from app/views/admin/site_customization/content_blocks/_form_heading_content_block.html.erb rename to app/components/admin/site_customization/content_blocks/form_heading_content_block_component.html.erb index d3415c053..862794114 100644 --- a/app/views/admin/site_customization/content_blocks/_form_heading_content_block.html.erb +++ b/app/components/admin/site_customization/content_blocks/form_heading_content_block_component.html.erb @@ -1,17 +1,17 @@ -<%= form_tag(admin_site_customization_update_heading_content_block_path(@content_block.id), method: "put") do %> - <%= render "shared/errors", resource: @content_block %> +<%= form_tag(admin_site_customization_update_heading_content_block_path(content_block.id), method: "put") do %> + <%= render "shared/errors", resource: content_block %>
<%= label_tag :name %> - <%= select_tag :name, options_for_select(valid_blocks, @selected_content_block) %> + <%= select_tag :name, options_for_select(valid_blocks, selected_content_block) %>
<%= label_tag :locale %> - <%= select_tag :locale, options_for_select(I18n.available_locales, @content_block.locale.to_sym) %> + <%= select_tag :locale, options_for_select(I18n.available_locales, content_block.locale.to_sym) %>
<%= label_tag :body %> - <%= text_area_tag :body, @content_block.body, rows: 10 %> + <%= text_area_tag :body, content_block.body, rows: 10 %>
<%= button_tag t("admin.menu.site_customization.buttons.content_block.update"), class: "button success expanded" %>
diff --git a/app/components/admin/site_customization/content_blocks/form_heading_content_block_component.rb b/app/components/admin/site_customization/content_blocks/form_heading_content_block_component.rb new file mode 100644 index 000000000..6c13a9656 --- /dev/null +++ b/app/components/admin/site_customization/content_blocks/form_heading_content_block_component.rb @@ -0,0 +1,14 @@ +class Admin::SiteCustomization::ContentBlocks::FormHeadingContentBlockComponent < ApplicationComponent + attr_reader :content_block + use_helpers :valid_blocks + + def initialize(content_block) + @content_block = content_block + end + + private + + def selected_content_block + "hcb_#{content_block.heading_id}" + end +end diff --git a/app/controllers/admin/site_customization/content_blocks_controller.rb b/app/controllers/admin/site_customization/content_blocks_controller.rb index d4a01c01b..0b124cf6f 100644 --- a/app/controllers/admin/site_customization/content_blocks_controller.rb +++ b/app/controllers/admin/site_customization/content_blocks_controller.rb @@ -34,7 +34,6 @@ class Admin::SiteCustomization::ContentBlocksController < Admin::SiteCustomizati end def edit - @selected_content_block = @content_block.name end def update @@ -71,7 +70,6 @@ class Admin::SiteCustomization::ContentBlocksController < Admin::SiteCustomizati def edit_heading_content_block @content_block = Budget::ContentBlock.find(params[:id]) - @selected_content_block = "hcb_#{@content_block.heading_id}" @is_heading_content_block = true render :edit end diff --git a/app/views/admin/site_customization/content_blocks/_form.html.erb b/app/views/admin/site_customization/content_blocks/_form.html.erb index 6f2b99a18..7879c6195 100644 --- a/app/views/admin/site_customization/content_blocks/_form.html.erb +++ b/app/views/admin/site_customization/content_blocks/_form.html.erb @@ -1,5 +1,5 @@ <% if @is_heading_content_block %> - <%= render "form_heading_content_block" %> + <%= render Admin::SiteCustomization::ContentBlocks::FormHeadingContentBlockComponent.new(@content_block) %> <% else %> - <%= render "form_content_block" %> + <%= render Admin::SiteCustomization::ContentBlocks::FormContentBlockComponent.new(@content_block) %> <% end %> From 26b48e527a1068ffadc900ef68ba5c777a9fddca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 23 May 2024 20:21:56 +0200 Subject: [PATCH 13/22] Move information texts form partial to a component This way it'll be easierto write tests for it when we change it. --- .../information_texts/form_component.html.erb} | 2 +- .../information_texts/form_component.rb | 14 ++++++++++++++ app/helpers/globalize_helper.rb | 4 ---- .../information_texts/_globalize_locales.html.erb | 1 - .../information_texts/index.html.erb | 3 +-- 5 files changed, 16 insertions(+), 8 deletions(-) rename app/{views/admin/site_customization/information_texts/_form.html.erb => components/admin/site_customization/information_texts/form_component.html.erb} (92%) create mode 100644 app/components/admin/site_customization/information_texts/form_component.rb delete mode 100644 app/views/admin/site_customization/information_texts/_globalize_locales.html.erb diff --git a/app/views/admin/site_customization/information_texts/_form.html.erb b/app/components/admin/site_customization/information_texts/form_component.html.erb similarity index 92% rename from app/views/admin/site_customization/information_texts/_form.html.erb rename to app/components/admin/site_customization/information_texts/form_component.html.erb index b7b4adf5b..b224a0cc1 100644 --- a/app/views/admin/site_customization/information_texts/_form.html.erb +++ b/app/components/admin/site_customization/information_texts/form_component.html.erb @@ -1,4 +1,4 @@ -<%= render "globalize_locales" %> +<%= render Shared::GlobalizeLocalesComponent.new %> <%= form_tag admin_site_customization_information_texts_path do %> <% I18n.available_locales.each do |l| %> diff --git a/app/components/admin/site_customization/information_texts/form_component.rb b/app/components/admin/site_customization/information_texts/form_component.rb new file mode 100644 index 000000000..49521f9e1 --- /dev/null +++ b/app/components/admin/site_customization/information_texts/form_component.rb @@ -0,0 +1,14 @@ +class Admin::SiteCustomization::InformationTexts::FormComponent < ApplicationComponent + attr_reader :contents + use_helpers :site_customization_enable_translation? + + def initialize(contents) + @contents = contents + end + + private + + def translation_enabled_tag(locale, enabled) + hidden_field_tag("enabled_translations[#{locale}]", (enabled ? 1 : 0)) + end +end diff --git a/app/helpers/globalize_helper.rb b/app/helpers/globalize_helper.rb index 1d824fd06..bb6cebb11 100644 --- a/app/helpers/globalize_helper.rb +++ b/app/helpers/globalize_helper.rb @@ -43,10 +43,6 @@ module GlobalizeHelper end end - def translation_enabled_tag(locale, enabled) - hidden_field_tag("enabled_translations[#{locale}]", (enabled ? 1 : 0)) - end - def globalize(locale, &) Globalize.with_locale(locale, &) end diff --git a/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb b/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb deleted file mode 100644 index 1d38894fb..000000000 --- a/app/views/admin/site_customization/information_texts/_globalize_locales.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render Shared::GlobalizeLocalesComponent.new %> diff --git a/app/views/admin/site_customization/information_texts/index.html.erb b/app/views/admin/site_customization/information_texts/index.html.erb index 16d3d7f32..c6a2fea43 100644 --- a/app/views/admin/site_customization/information_texts/index.html.erb +++ b/app/views/admin/site_customization/information_texts/index.html.erb @@ -4,7 +4,6 @@ <%= render "tabs" %> -
From c11780880c54bdbaf5ba56c3c37c28e8b884ab12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 23 May 2024 21:14:10 +0200 Subject: [PATCH 14/22] Move form builders to their own folder We were defining one builder in the `app/lib/` folder and another one inside a helper module. So now we're grouping them together. This way we're following the "one class per file" convention that we follow most of the time. And, by extracting the `TranslatableFormBuilder` class to its own file, it'll be easier to add tests for it. Note that, for consistency, we're renaming the `TranslationsFieldsBuilder` class so it ends in `FormBuilder`. --- .../consul_form_builder.rb | 0 app/form_builders/custom/.keep | 0 .../translatable_form_builder.rb | 71 +++++++++++++++++ .../translations_fields_form_builder.rb | 5 ++ app/helpers/translatable_form_helper.rb | 78 ------------------- config/application.rb | 1 + .../consul_form_builder_spec.rb | 0 7 files changed, 77 insertions(+), 78 deletions(-) rename app/{lib => form_builders}/consul_form_builder.rb (100%) create mode 100644 app/form_builders/custom/.keep create mode 100644 app/form_builders/translatable_form_builder.rb create mode 100644 app/form_builders/translations_fields_form_builder.rb rename spec/{lib => form_builders}/consul_form_builder_spec.rb (100%) diff --git a/app/lib/consul_form_builder.rb b/app/form_builders/consul_form_builder.rb similarity index 100% rename from app/lib/consul_form_builder.rb rename to app/form_builders/consul_form_builder.rb diff --git a/app/form_builders/custom/.keep b/app/form_builders/custom/.keep new file mode 100644 index 000000000..e69de29bb diff --git a/app/form_builders/translatable_form_builder.rb b/app/form_builders/translatable_form_builder.rb new file mode 100644 index 000000000..e185542ff --- /dev/null +++ b/app/form_builders/translatable_form_builder.rb @@ -0,0 +1,71 @@ +class TranslatableFormBuilder < ConsulFormBuilder + attr_accessor :translations + + def translatable_fields(&) + @translations = {} + visible_locales.map do |locale| + @translations[locale] = translation_for(locale) + end + safe_join(visible_locales.map do |locale| + Globalize.with_locale(locale) { fields_for_locale(locale, &) } + end) + end + + private + + def fields_for_locale(locale) + fields_for_translation(@translations[locale]) do |translations_form| + @template.tag.div(**translations_options(translations_form.object, locale)) do + @template.concat translations_form.hidden_field( + :_destroy, + value: !@template.enabled_locale?(translations_form.object.globalized_model, locale), + data: { locale: locale } + ) + + @template.concat translations_form.hidden_field(:locale, value: locale) + + yield translations_form + end + end + end + + def fields_for_translation(translation, &) + fields_for(:translations, translation, builder: TranslationsFieldsFormBuilder, &) + end + + def translation_for(locale) + existing_translation_for(locale) || new_translation_for(locale) + end + + def existing_translation_for(locale) + @object.translations.find { |translation| translation.locale == locale } + end + + def new_translation_for(locale) + @object.translations.new(locale: locale).tap(&:mark_for_destruction) + end + + def highlight_translation_html_class + @template.highlight_translation_html_class + end + + def translations_options(resource, locale) + { + class: "translatable-fields js-globalize-attribute #{highlight_translation_html_class}", + style: @template.display_translation_style(resource.globalized_model, locale), + data: { locale: locale } + } + end + + def no_other_translations?(translation) + (@object.translations - [translation]).reject(&:_destroy).empty? + end + + def visible_locales + if @template.translations_interface_enabled? + @object.globalize_locales + else + [I18n.locale] + end + end +end diff --git a/app/form_builders/translations_fields_form_builder.rb b/app/form_builders/translations_fields_form_builder.rb new file mode 100644 index 000000000..e33d8f15f --- /dev/null +++ b/app/form_builders/translations_fields_form_builder.rb @@ -0,0 +1,5 @@ +class TranslationsFieldsFormBuilder < ConsulFormBuilder + def locale + @object.locale + end +end diff --git a/app/helpers/translatable_form_helper.rb b/app/helpers/translatable_form_helper.rb index f77ca0b00..d67b5fe95 100644 --- a/app/helpers/translatable_form_helper.rb +++ b/app/helpers/translatable_form_helper.rb @@ -14,82 +14,4 @@ module TranslatableFormHelper def highlight_translation_html_class "highlight" if translations_interface_enabled? end - - class TranslatableFormBuilder < ConsulFormBuilder - attr_accessor :translations - - def translatable_fields(&) - @translations = {} - visible_locales.map do |locale| - @translations[locale] = translation_for(locale) - end - safe_join(visible_locales.map do |locale| - Globalize.with_locale(locale) { fields_for_locale(locale, &) } - end) - end - - private - - def fields_for_locale(locale) - fields_for_translation(@translations[locale]) do |translations_form| - @template.tag.div **translations_options(translations_form.object, locale) do - @template.concat translations_form.hidden_field( - :_destroy, - value: !@template.enabled_locale?(translations_form.object.globalized_model, locale), - data: { locale: locale } - ) - - @template.concat translations_form.hidden_field(:locale, value: locale) - - yield translations_form - end - end - end - - def fields_for_translation(translation, &) - fields_for(:translations, translation, builder: TranslationsFieldsBuilder, &) - end - - def translation_for(locale) - existing_translation_for(locale) || new_translation_for(locale) - end - - def existing_translation_for(locale) - @object.translations.find { |translation| translation.locale == locale } - end - - def new_translation_for(locale) - @object.translations.new(locale: locale).tap(&:mark_for_destruction) - end - - def highlight_translation_html_class - @template.highlight_translation_html_class - end - - def translations_options(resource, locale) - { - class: "translatable-fields js-globalize-attribute #{highlight_translation_html_class}", - style: @template.display_translation_style(resource.globalized_model, locale), - data: { locale: locale } - } - end - - def no_other_translations?(translation) - (@object.translations - [translation]).reject(&:_destroy).empty? - end - - def visible_locales - if @template.translations_interface_enabled? - @object.globalize_locales - else - [I18n.locale] - end - end - end - - class TranslationsFieldsBuilder < ConsulFormBuilder - def locale - @object.locale - end - end end diff --git a/config/application.rb b/config/application.rb index deaf238f6..ca6317e38 100644 --- a/config/application.rb +++ b/config/application.rb @@ -136,6 +136,7 @@ module Consul [ "app/components/custom", "app/controllers/custom", + "app/form_builders/custom", "app/graphql/custom", "app/lib/custom", "app/mailers/custom", diff --git a/spec/lib/consul_form_builder_spec.rb b/spec/form_builders/consul_form_builder_spec.rb similarity index 100% rename from spec/lib/consul_form_builder_spec.rb rename to spec/form_builders/consul_form_builder_spec.rb From 790d515afc9fd04cf0abac23e9735876226b6fec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 23 May 2024 22:56:41 +0200 Subject: [PATCH 15/22] Remove unused code in globalizable concern This code was added in commit 041abe904, but it was never used. --- app/models/concerns/globalizable.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/concerns/globalizable.rb b/app/models/concerns/globalizable.rb index 8b9ea9527..d4bb45edb 100644 --- a/app/models/concerns/globalizable.rb +++ b/app/models/concerns/globalizable.rb @@ -13,10 +13,6 @@ module Globalizable translations.reject(&:marked_for_destruction?).map(&:locale) end - def locales_marked_for_destruction - I18n.available_locales - locales_not_marked_for_destruction - end - def locales_persisted_and_marked_for_destruction translations.select { |t| t.persisted? && t.marked_for_destruction? }.map(&:locale) end From 647121d13efb6b7e0b54fd6083872dbf651f26b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 30 Oct 2022 12:37:33 +0100 Subject: [PATCH 16/22] Allow different locales per tenant Note that, currently, we take these settings from the database but we don't provide a way to edit them through the admin interface, so the locales must be manually introduced through a Rails console. While we did consider using a comma-separated list, we're using spaces in order to be consistent with the way we store the allowed content types settings. The `enabled_locales` nomenclature, which contrasts with `available_locales`, is probably subconsciously based on similar patterns like the one Nginx uses to enable sites. Note that we aren't using `Setting.enabled_locales` in the globalize initializer when setting the fallbacks. This means the following test (which we could add to the shared globalizable examples) would fail: ``` it "Falls back to an enabled locale if the fallback is not enabled" do Setting["locales.default"] = "en" Setting["locales.enabled"] = "fr en" allow(I18n.fallbacks).to receive(:[]).and_return([:fr, :es]) Globalize.set_fallbacks_to_all_available_locales I18n.with_locale(:fr) do expect(record.send(attribute)).to eq "In English" end end ``` The reason is that the code making this test pass could be: ``` def Globalize.set_fallbacks_to_all_available_locales Globalize.fallbacks = I18n.available_locales.index_with do |locale| ((I18n.fallbacks[locale] & Setting.enabled_locales) + Setting.enabled_locales).uniq end end ``` However, this would make it impossible to run `rake db:migrate` on new applications because the initializer would try to load the `Setting` model but the `settings` table wouldn't exist at that point. Besides, this is a really rare case that IMHO we don't need to support. For this scenario, an installation would have to enable a locale, create records with contents in that locale, then disable that locale and have that locale as a fallback for a language where content for that record wasn't created. If that happened, it would be solved by creating content for that record in every enabled language. --- .rubocop.yml | 1 + .../form_content_block_component.html.erb | 2 +- ...m_heading_content_block_component.html.erb | 2 +- .../information_texts/form_component.html.erb | 4 +- .../information_texts/form_component.rb | 4 ++ .../layout/locale_switcher_component.rb | 2 +- .../globalize_locales_component.html.erb | 2 +- .../shared/globalize_locales_component.rb | 4 +- app/controllers/application_controller.rb | 4 +- app/controllers/management/base_controller.rb | 2 +- app/controllers/subscriptions_controller.rb | 2 +- .../translatable_form_builder.rb | 2 +- app/models/budget/content_block.rb | 2 +- app/models/i18n_content.rb | 2 +- app/models/setting.rb | 7 +++ .../site_customization/content_block.rb | 2 +- db/dev_seeds.rb | 6 ++- db/dev_seeds/banners.rb | 2 +- db/dev_seeds/polls.rb | 14 +++--- db/pages/accessibility.rb | 2 +- db/pages/census_terms.rb | 2 +- db/pages/conditions.rb | 2 +- db/pages/faq.rb | 2 +- db/pages/privacy.rb | 2 +- db/pages/welcome_level_three_verified.rb | 2 +- db/pages/welcome_level_two_verified.rb | 2 +- db/pages/welcome_not_verified.rb | 2 +- .../form_content_block_component_spec.rb | 18 +++++++ ...rm_heading_content_block_component_spec.rb | 18 +++++++ .../information_texts/form_component_spec.rb | 29 +++++++++++ .../layout/locale_switcher_component_spec.rb | 16 ++++++ .../globalize_locales_component_spec.rb | 49 +++++++++++++++++++ .../application_controller_spec.rb | 16 ++++++ .../management/base_controller_spec.rb | 16 ++++++ .../subscriptions_controller_spec.rb | 6 ++- .../translatable_form_builder_spec.rb | 46 +++++++++++++++++ spec/models/budget/content_block_spec.rb | 8 +++ spec/models/i18n_content_spec.rb | 10 ++++ spec/models/setting_spec.rb | 46 +++++++++++++++++ .../site_customization/content_block_spec.rb | 8 +++ 40 files changed, 333 insertions(+), 35 deletions(-) create mode 100644 spec/components/admin/site_customization/content_blocks/form_content_block_component_spec.rb create mode 100644 spec/components/admin/site_customization/content_blocks/form_heading_content_block_component_spec.rb create mode 100644 spec/components/admin/site_customization/information_texts/form_component_spec.rb create mode 100644 spec/components/shared/globalize_locales_component_spec.rb create mode 100644 spec/form_builders/translatable_form_builder_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index eef2eaac0..96d7572b5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -574,6 +574,7 @@ RSpec/InstanceVariable: Enabled: true Exclude: - spec/controllers/concerns/has_orders_spec.rb + - spec/form_builders/translatable_form_builder_spec.rb RSpec/LetBeforeExamples: Enabled: true diff --git a/app/components/admin/site_customization/content_blocks/form_content_block_component.html.erb b/app/components/admin/site_customization/content_blocks/form_content_block_component.html.erb index 0f36892b6..55ea3f573 100644 --- a/app/components/admin/site_customization/content_blocks/form_content_block_component.html.erb +++ b/app/components/admin/site_customization/content_blocks/form_content_block_component.html.erb @@ -6,7 +6,7 @@ <%= f.select :name, options_for_select(valid_blocks, selected_content_block) %>
- <%= f.select :locale, I18n.available_locales %> + <%= f.select :locale, Setting.enabled_locales %>
diff --git a/app/components/admin/site_customization/content_blocks/form_heading_content_block_component.html.erb b/app/components/admin/site_customization/content_blocks/form_heading_content_block_component.html.erb index 862794114..486b9b4cd 100644 --- a/app/components/admin/site_customization/content_blocks/form_heading_content_block_component.html.erb +++ b/app/components/admin/site_customization/content_blocks/form_heading_content_block_component.html.erb @@ -7,7 +7,7 @@
<%= label_tag :locale %> - <%= select_tag :locale, options_for_select(I18n.available_locales, content_block.locale.to_sym) %> + <%= select_tag :locale, options_for_select(Setting.enabled_locales, content_block.locale.to_sym) %>
<%= label_tag :body %> diff --git a/app/components/admin/site_customization/information_texts/form_component.html.erb b/app/components/admin/site_customization/information_texts/form_component.html.erb index b224a0cc1..b8f8b6444 100644 --- a/app/components/admin/site_customization/information_texts/form_component.html.erb +++ b/app/components/admin/site_customization/information_texts/form_component.html.erb @@ -1,13 +1,13 @@ <%= render Shared::GlobalizeLocalesComponent.new %> <%= form_tag admin_site_customization_information_texts_path do %> - <% I18n.available_locales.each do |l| %> + <% enabled_locales.each do |l| %> <%= translation_enabled_tag l, site_customization_enable_translation?(l) %> <% end %> <% contents.each do |group| %> <% group.each do |content| %> <%= content.key %> - <% content.globalize_locales.each do |locale| %> + <% (content.globalize_locales & enabled_locales.map(&:to_sym)).each do |locale| %> <%= render Admin::SiteCustomization::InformationTexts::FormFieldComponent.new(content, locale: locale) %> <% end %> <% end %> diff --git a/app/components/admin/site_customization/information_texts/form_component.rb b/app/components/admin/site_customization/information_texts/form_component.rb index 49521f9e1..862e5a759 100644 --- a/app/components/admin/site_customization/information_texts/form_component.rb +++ b/app/components/admin/site_customization/information_texts/form_component.rb @@ -11,4 +11,8 @@ class Admin::SiteCustomization::InformationTexts::FormComponent < ApplicationCom def translation_enabled_tag(locale, enabled) hidden_field_tag("enabled_translations[#{locale}]", (enabled ? 1 : 0)) end + + def enabled_locales + Setting.enabled_locales + end end diff --git a/app/components/layout/locale_switcher_component.rb b/app/components/layout/locale_switcher_component.rb index 304bfd119..bce22d522 100644 --- a/app/components/layout/locale_switcher_component.rb +++ b/app/components/layout/locale_switcher_component.rb @@ -12,7 +12,7 @@ class Layout::LocaleSwitcherComponent < ApplicationComponent end def locales - I18n.available_locales + Setting.enabled_locales end def label diff --git a/app/components/shared/globalize_locales_component.html.erb b/app/components/shared/globalize_locales_component.html.erb index aa73c9b24..664d298db 100644 --- a/app/components/shared/globalize_locales_component.html.erb +++ b/app/components/shared/globalize_locales_component.html.erb @@ -13,7 +13,7 @@ <%= select_language_error %>
<% if manage_languages %> - <% I18n.available_locales.each do |locale| %> + <% Setting.enabled_locales.each do |locale| %> <%= link_to t("shared.translations.remove_language"), "#", style: display_destroy_locale_style(locale), class: "delete js-delete-language js-delete-#{locale}", diff --git a/app/components/shared/globalize_locales_component.rb b/app/components/shared/globalize_locales_component.rb index 8b551ceb6..8bc2696ce 100644 --- a/app/components/shared/globalize_locales_component.rb +++ b/app/components/shared/globalize_locales_component.rb @@ -15,7 +15,7 @@ class Shared::GlobalizeLocalesComponent < ApplicationComponent end def available_locales - I18n.available_locales.select { |locale| enabled_locale?(resource, locale) }.map do |locale| + Setting.enabled_locales.select { |locale| enabled_locale?(resource, locale) }.map do |locale| [name_for_locale(locale), locale, { data: { locale: locale }}] end end @@ -84,7 +84,7 @@ class Shared::GlobalizeLocalesComponent < ApplicationComponent end def all_language_options - I18n.available_locales.map do |locale| + Setting.enabled_locales.map do |locale| [name_for_locale(locale), locale] end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f05714b53..28493bd02 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -51,9 +51,9 @@ class ApplicationController < ActionController::Base end def current_locale - if I18n.available_locales.include?(params[:locale]&.to_sym) + if Setting.enabled_locales.include?(params[:locale]&.to_sym) params[:locale] - elsif I18n.available_locales.include?(session[:locale]&.to_sym) + elsif Setting.enabled_locales.include?(session[:locale]&.to_sym) session[:locale] else I18n.default_locale diff --git a/app/controllers/management/base_controller.rb b/app/controllers/management/base_controller.rb index 185a0fc07..2307d5ea3 100644 --- a/app/controllers/management/base_controller.rb +++ b/app/controllers/management/base_controller.rb @@ -40,7 +40,7 @@ class Management::BaseController < ActionController::Base end def switch_locale(&action) - if params[:locale] && I18n.available_locales.include?(params[:locale].to_sym) + if params[:locale] && Setting.enabled_locales.include?(params[:locale].to_sym) session[:locale] = params[:locale].to_s end diff --git a/app/controllers/subscriptions_controller.rb b/app/controllers/subscriptions_controller.rb index 315905001..e1a50316a 100644 --- a/app/controllers/subscriptions_controller.rb +++ b/app/controllers/subscriptions_controller.rb @@ -38,6 +38,6 @@ class SubscriptionsController < ApplicationController end def find_locale - I18n.available_locales.find { |locale| locale == @user.locale&.to_sym } || I18n.locale + Setting.enabled_locales.find { |locale| locale == @user.locale&.to_sym } || I18n.locale end end diff --git a/app/form_builders/translatable_form_builder.rb b/app/form_builders/translatable_form_builder.rb index e185542ff..cc1700ca1 100644 --- a/app/form_builders/translatable_form_builder.rb +++ b/app/form_builders/translatable_form_builder.rb @@ -63,7 +63,7 @@ class TranslatableFormBuilder < ConsulFormBuilder def visible_locales if @template.translations_interface_enabled? - @object.globalize_locales + Setting.enabled_locales & @object.globalize_locales else [I18n.locale] end diff --git a/app/models/budget/content_block.rb b/app/models/budget/content_block.rb index fef73dc9a..95608f30d 100644 --- a/app/models/budget/content_block.rb +++ b/app/models/budget/content_block.rb @@ -1,6 +1,6 @@ class Budget class ContentBlock < ApplicationRecord - validates :locale, presence: true, inclusion: { in: I18n.available_locales.map(&:to_s) } + validates :locale, presence: true, inclusion: { in: ->(*) { Setting.enabled_locales.map(&:to_s) }} validates :heading, presence: true, uniqueness: { scope: :locale } belongs_to :heading diff --git a/app/models/i18n_content.rb b/app/models/i18n_content.rb index c17a7dee2..72c656c95 100644 --- a/app/models/i18n_content.rb +++ b/app/models/i18n_content.rb @@ -119,7 +119,7 @@ class I18nContent < ApplicationRecord end end - def self.update(contents, enabled_translations = I18n.available_locales) + def self.update(contents, enabled_translations = Setting.enabled_locales) contents.each do |content| values = content[:values].slice(*translation_params(enabled_translations)) diff --git a/app/models/setting.rb b/app/models/setting.rb index 5427afccf..0dee92c84 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -95,6 +95,7 @@ class Setting < ApplicationRecord "html.per_page_code_body": "", # Code to be included at the top (inside ) of every page (useful for tracking) "html.per_page_code_head": "", + "locales.enabled": nil, "map.latitude": 51.48, "map.longitude": 0.0, "map.zoom": 10, @@ -219,5 +220,11 @@ class Setting < ApplicationRecord def archived_proposals_date_limit Setting["months_to_archive_proposals"].to_i.months.ago end + + def enabled_locales + locales = Setting["locales.enabled"].to_s.split.map(&:to_sym) + + (locales & I18n.available_locales).presence || I18n.available_locales + end end end diff --git a/app/models/site_customization/content_block.rb b/app/models/site_customization/content_block.rb index 7d42817e6..a7e95d0ab 100644 --- a/app/models/site_customization/content_block.rb +++ b/app/models/site_customization/content_block.rb @@ -1,7 +1,7 @@ class SiteCustomization::ContentBlock < ApplicationRecord VALID_BLOCKS = %w[top_links footer footer_legal subnavigation_left subnavigation_right].freeze - validates :locale, presence: true, inclusion: { in: I18n.available_locales.map(&:to_s) } + validates :locale, presence: true, inclusion: { in: ->(*) { Setting.enabled_locales.map(&:to_s) }} validates :name, presence: true, uniqueness: { scope: :locale }, inclusion: { in: ->(*) { VALID_BLOCKS }} def self.block_for(name, locale) diff --git a/db/dev_seeds.rb b/db/dev_seeds.rb index 00f102f22..5c67ac52f 100644 --- a/db/dev_seeds.rb +++ b/db/dev_seeds.rb @@ -23,7 +23,11 @@ def log(msg) end def random_locales - [I18n.default_locale, *(I18n.available_locales & %i[en es]), *I18n.available_locales.sample(4)].uniq.take(5) + [ + I18n.default_locale, + *(Setting.enabled_locales & %i[en es]), + *Setting.enabled_locales.sample(4) + ].uniq.take(5) end def random_locales_attributes(**attribute_names_with_values) diff --git a/db/dev_seeds/banners.rb b/db/dev_seeds/banners.rb index 345ae4e90..9716978ae 100644 --- a/db/dev_seeds/banners.rb +++ b/db/dev_seeds/banners.rb @@ -9,7 +9,7 @@ section "Creating banners" do post_started_at: rand((1.week.ago)..(1.day.ago)), post_ended_at: rand((1.day.ago)..(1.week.from_now)), created_at: rand((1.week.ago)..Time.current)) - I18n.available_locales.map do |locale| + Setting.enabled_locales.map do |locale| Globalize.with_locale(locale) do banner.description = "Description for locale #{locale}" banner.title = "Title for locale #{locale}" diff --git a/db/dev_seeds/polls.rb b/db/dev_seeds/polls.rb index e0b62a899..44021a782 100644 --- a/db/dev_seeds/polls.rb +++ b/db/dev_seeds/polls.rb @@ -41,7 +41,7 @@ section "Creating polls" do Poll.find_each do |poll| name = poll.name - I18n.available_locales.map do |locale| + Setting.enabled_locales.map do |locale| Globalize.with_locale(locale) do poll.name = "#{name} (#{locale})" poll.summary = "Summary for locale #{locale}" @@ -59,7 +59,7 @@ section "Creating Poll Questions & Answers" do question = Poll::Question.new(author: User.sample, title: question_title, poll: poll) - I18n.available_locales.map do |locale| + Setting.enabled_locales.map do |locale| Globalize.with_locale(locale) do question.title = "#{question_title} (#{locale})" end @@ -71,7 +71,7 @@ section "Creating Poll Questions & Answers" do title: title.capitalize, description: description, given_order: index + 1) - I18n.available_locales.map do |locale| + Setting.enabled_locales.map do |locale| Globalize.with_locale(locale) do answer.title = "#{title} (#{locale})" answer.description = "#{description} (#{locale})" @@ -235,7 +235,7 @@ section "Creating Poll Questions from Proposals" do question = Poll::Question.new(poll: poll) question.copy_attributes_from_proposal(proposal) question_title = question.title - I18n.available_locales.map do |locale| + Setting.enabled_locales.map do |locale| Globalize.with_locale(locale) do question.title = "#{question_title} (#{locale})" end @@ -247,7 +247,7 @@ section "Creating Poll Questions from Proposals" do title: title.capitalize, description: description, given_order: index + 1) - I18n.available_locales.map do |locale| + Setting.enabled_locales.map do |locale| Globalize.with_locale(locale) do answer.title = "#{title} (#{locale})" answer.description = "#{description} (#{locale})" @@ -265,7 +265,7 @@ section "Creating Successful Proposals" do question = Poll::Question.new(poll: poll) question.copy_attributes_from_proposal(proposal) question_title = question.title - I18n.available_locales.map do |locale| + Setting.enabled_locales.map do |locale| Globalize.with_locale(locale) do question.title = "#{question_title} (#{locale})" end @@ -277,7 +277,7 @@ section "Creating Successful Proposals" do title: title.capitalize, description: description, given_order: index + 1) - I18n.available_locales.map do |locale| + Setting.enabled_locales.map do |locale| Globalize.with_locale(locale) do answer.title = "#{title} (#{locale})" answer.description = "#{description} (#{locale})" diff --git a/db/pages/accessibility.rb b/db/pages/accessibility.rb index c499199ca..8d2e2d824 100644 --- a/db/pages/accessibility.rb +++ b/db/pages/accessibility.rb @@ -104,7 +104,7 @@ end if SiteCustomization::Page.find_by(slug: "accessibility").nil? page = SiteCustomization::Page.new(slug: "accessibility", status: "published") - I18n.available_locales.each do |locale| + Setting.enabled_locales.each do |locale| I18n.with_locale(locale) { generate_content(page) } end end diff --git a/db/pages/census_terms.rb b/db/pages/census_terms.rb index b5c3a43c3..27ac3fc45 100644 --- a/db/pages/census_terms.rb +++ b/db/pages/census_terms.rb @@ -7,7 +7,7 @@ end if SiteCustomization::Page.find_by(slug: "census_terms").nil? page = SiteCustomization::Page.new(slug: "census_terms", status: "published") page.print_content_flag = true - I18n.available_locales.each do |locale| + Setting.enabled_locales.each do |locale| I18n.with_locale(locale) { generate_content(page) } end end diff --git a/db/pages/conditions.rb b/db/pages/conditions.rb index 539fc6afe..314d0db61 100644 --- a/db/pages/conditions.rb +++ b/db/pages/conditions.rb @@ -8,7 +8,7 @@ end if SiteCustomization::Page.find_by(slug: "conditions").nil? page = SiteCustomization::Page.new(slug: "conditions", status: "published") page.print_content_flag = true - I18n.available_locales.each do |locale| + Setting.enabled_locales.each do |locale| I18n.with_locale(locale) { generate_content(page) } end end diff --git a/db/pages/faq.rb b/db/pages/faq.rb index 362801a2e..983b78665 100644 --- a/db/pages/faq.rb +++ b/db/pages/faq.rb @@ -5,7 +5,7 @@ def generate_content(page) end if SiteCustomization::Page.find_by(slug: "faq").nil? page = SiteCustomization::Page.new(slug: "faq", status: "published") - I18n.available_locales.each do |locale| + Setting.enabled_locales.each do |locale| I18n.with_locale(locale) { generate_content(page) } end end diff --git a/db/pages/privacy.rb b/db/pages/privacy.rb index 5f215810d..dfa82b492 100644 --- a/db/pages/privacy.rb +++ b/db/pages/privacy.rb @@ -7,7 +7,7 @@ end if SiteCustomization::Page.find_by(slug: "privacy").nil? page = SiteCustomization::Page.new(slug: "privacy", status: "published") page.print_content_flag = true - I18n.available_locales.each do |locale| + Setting.enabled_locales.each do |locale| I18n.with_locale(locale) { generate_content(page) } end end diff --git a/db/pages/welcome_level_three_verified.rb b/db/pages/welcome_level_three_verified.rb index 5467f5917..0fd6a6eac 100644 --- a/db/pages/welcome_level_three_verified.rb +++ b/db/pages/welcome_level_three_verified.rb @@ -17,7 +17,7 @@ end if SiteCustomization::Page.find_by(slug: "welcome_level_three_verified").nil? page = SiteCustomization::Page.new(slug: "welcome_level_three_verified", status: "published") - I18n.available_locales.each do |locale| + Setting.enabled_locales.each do |locale| I18n.with_locale(locale) { generate_content(page) } end end diff --git a/db/pages/welcome_level_two_verified.rb b/db/pages/welcome_level_two_verified.rb index 251fc6b04..27d2fba32 100644 --- a/db/pages/welcome_level_two_verified.rb +++ b/db/pages/welcome_level_two_verified.rb @@ -22,7 +22,7 @@ def generate_content(page) end if SiteCustomization::Page.find_by(slug: "welcome_level_two_verified").nil? page = SiteCustomization::Page.new(slug: "welcome_level_two_verified", status: "published") - I18n.available_locales.each do |locale| + Setting.enabled_locales.each do |locale| I18n.with_locale(locale) { generate_content(page) } end end diff --git a/db/pages/welcome_not_verified.rb b/db/pages/welcome_not_verified.rb index 4263130f0..946570101 100644 --- a/db/pages/welcome_not_verified.rb +++ b/db/pages/welcome_not_verified.rb @@ -22,7 +22,7 @@ def generate_content(page) end if SiteCustomization::Page.find_by(slug: "welcome_not_verified").nil? page = SiteCustomization::Page.new(slug: "welcome_not_verified", status: "published") - I18n.available_locales.each do |locale| + Setting.enabled_locales.each do |locale| I18n.with_locale(locale) { generate_content(page) } end end diff --git a/spec/components/admin/site_customization/content_blocks/form_content_block_component_spec.rb b/spec/components/admin/site_customization/content_blocks/form_content_block_component_spec.rb new file mode 100644 index 000000000..6e44168b6 --- /dev/null +++ b/spec/components/admin/site_customization/content_blocks/form_content_block_component_spec.rb @@ -0,0 +1,18 @@ +require "rails_helper" + +describe Admin::SiteCustomization::ContentBlocks::FormContentBlockComponent do + describe "locale selector" do + let(:content_block) { create(:site_customization_content_block, locale: "de") } + let(:component) do + Admin::SiteCustomization::ContentBlocks::FormContentBlockComponent.new(content_block) + end + + it "only includes enabled settings" do + Setting["locales.enabled"] = "de fr" + + render_inline component + + expect(page).to have_select "locale", options: ["de", "fr"] + end + end +end diff --git a/spec/components/admin/site_customization/content_blocks/form_heading_content_block_component_spec.rb b/spec/components/admin/site_customization/content_blocks/form_heading_content_block_component_spec.rb new file mode 100644 index 000000000..b04c8a64d --- /dev/null +++ b/spec/components/admin/site_customization/content_blocks/form_heading_content_block_component_spec.rb @@ -0,0 +1,18 @@ +require "rails_helper" + +describe Admin::SiteCustomization::ContentBlocks::FormHeadingContentBlockComponent do + describe "locale selector" do + let(:content_block) { create(:heading_content_block, locale: "de") } + let(:component) do + Admin::SiteCustomization::ContentBlocks::FormHeadingContentBlockComponent.new(content_block) + end + + it "only includes enabled settings" do + Setting["locales.enabled"] = "de fr" + + render_inline component + + expect(page).to have_select "locale", options: ["de", "fr"] + end + end +end diff --git a/spec/components/admin/site_customization/information_texts/form_component_spec.rb b/spec/components/admin/site_customization/information_texts/form_component_spec.rb new file mode 100644 index 000000000..614ef28f9 --- /dev/null +++ b/spec/components/admin/site_customization/information_texts/form_component_spec.rb @@ -0,0 +1,29 @@ +require "rails_helper" + +describe Admin::SiteCustomization::InformationTexts::FormComponent do + describe "enabled_translations fields" do + it "renders fields for enabled locales" do + Setting["locales.enabled"] = "en es" + content = create(:i18n_content) + + render_inline Admin::SiteCustomization::InformationTexts::FormComponent.new([[content]]) + + expect(page).to have_css "input[name^='enabled_translations']", count: 2, visible: :all + expect(page).to have_css "input[name='enabled_translations[en]']", visible: :hidden + expect(page).to have_css "input[name='enabled_translations[es]']", visible: :hidden + end + end + + describe "text fields" do + it "renders fields for enabled locales" do + Setting["locales.enabled"] = "en es" + content = create(:i18n_content, key: "system.failure") + + render_inline Admin::SiteCustomization::InformationTexts::FormComponent.new([[content]]) + + expect(page).to have_css "textarea[name^='contents[content_system.failure]']", count: 2, visible: :all + expect(page).to have_field "contents[content_system.failure]values[value_en]" + expect(page).to have_field "contents[content_system.failure]values[value_es]" + end + end +end diff --git a/spec/components/layout/locale_switcher_component_spec.rb b/spec/components/layout/locale_switcher_component_spec.rb index 1063ede2f..f043f3550 100644 --- a/spec/components/layout/locale_switcher_component_spec.rb +++ b/spec/components/layout/locale_switcher_component_spec.rb @@ -97,4 +97,20 @@ describe Layout::LocaleSwitcherComponent do expect(page).to have_css "[href='/?locale=en'][data-turbolinks=true]" end end + + context "when not all available locales are enabled" do + before do + allow(I18n).to receive(:available_locales).and_return(%i[en es fr]) + Setting["locales.enabled"] = "es fr" + end + + it "displays the enabled locales" do + render_inline component + + expect(page).to have_link count: 2 + expect(page).to have_link "Español" + expect(page).to have_link "Français" + expect(page).not_to have_link "English" + end + end end diff --git a/spec/components/shared/globalize_locales_component_spec.rb b/spec/components/shared/globalize_locales_component_spec.rb new file mode 100644 index 000000000..da2dae47f --- /dev/null +++ b/spec/components/shared/globalize_locales_component_spec.rb @@ -0,0 +1,49 @@ +require "rails_helper" + +describe Shared::GlobalizeLocalesComponent do + describe "Language selector" do + it "only includes enabled locales" do + Setting["locales.enabled"] = "en nl" + + I18n.with_locale(:en) do + render_inline Shared::GlobalizeLocalesComponent.new + + expect(page).to have_select options: ["Choose language", "English"] + end + + I18n.with_locale(:es) do + render_inline Shared::GlobalizeLocalesComponent.new + + expect(page).to have_select options: ["Seleccionar idioma"] + end + end + end + + describe "links to destroy languages" do + it "only includes enabled locales" do + Setting["locales.enabled"] = "en nl" + + I18n.with_locale(:en) do + render_inline Shared::GlobalizeLocalesComponent.new + + expect(page).to have_css "a[data-locale]", count: 1 + end + + I18n.with_locale(:es) do + render_inline Shared::GlobalizeLocalesComponent.new + + expect(page).not_to have_css "a[data-locale]" + end + end + end + + describe "Add language selector" do + it "only includes enabled locales" do + Setting["locales.enabled"] = "en nl" + + render_inline Shared::GlobalizeLocalesComponent.new + + expect(page).to have_select options: ["Add language", "English", "Nederlands"] + end + end +end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 6e46ab7d7..f9bc09b65 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -54,6 +54,22 @@ describe ApplicationController do expect(response.body).to eq "es" end + it "only accepts enabled locales" do + Setting["locales.enabled"] = "en es fr" + + get :index, params: { locale: :es } + + expect(response.body).to eq "es" + + get :index, params: { locale: :de } + + expect(response.body).to eq "es" + + get :index, params: { locale: :fr } + + expect(response.body).to eq "fr" + end + context "authenticated user" do let(:user) { create(:user) } before { sign_in(user) } diff --git a/spec/controllers/management/base_controller_spec.rb b/spec/controllers/management/base_controller_spec.rb index 0aa4202fe..9abe44fbb 100644 --- a/spec/controllers/management/base_controller_spec.rb +++ b/spec/controllers/management/base_controller_spec.rb @@ -64,5 +64,21 @@ describe Management::BaseController do expect(response.body).to eq "es" end + + it "only accepts enabled locales" do + Setting["locales.enabled"] = "en es fr" + + get :index, params: { locale: :es } + + expect(response.body).to eq "es" + + get :index, params: { locale: :de } + + expect(response.body).to eq "es" + + get :index, params: { locale: :fr } + + expect(response.body).to eq "fr" + end end end diff --git a/spec/controllers/subscriptions_controller_spec.rb b/spec/controllers/subscriptions_controller_spec.rb index 1e1124125..e675136de 100644 --- a/spec/controllers/subscriptions_controller_spec.rb +++ b/spec/controllers/subscriptions_controller_spec.rb @@ -28,8 +28,10 @@ describe SubscriptionsController do expect(session[:locale]).to eq "es" end - it "only accepts available locales" do - create(:user, locale: "wl", subscriptions_token: "mytoken") + it "only accepts enabled locales" do + Setting["locales.enabled"] = "en nl" + + create(:user, locale: "es", subscriptions_token: "mytoken") get :edit, params: { token: "mytoken" } diff --git a/spec/form_builders/translatable_form_builder_spec.rb b/spec/form_builders/translatable_form_builder_spec.rb new file mode 100644 index 000000000..fd8747f3f --- /dev/null +++ b/spec/form_builders/translatable_form_builder_spec.rb @@ -0,0 +1,46 @@ +require "rails_helper" + +describe TranslatableFormBuilder do + before do + dummy_banner = Class.new(ApplicationRecord) do + def self.name + "DummyBanner" + end + self.table_name = "banners" + + translates :title, touch: true + include Globalizable + has_many :translations, class_name: "DummyBanner::Translation", foreign_key: "banner_id" + end + + stub_const("DummyBanner", dummy_banner) + end + + let(:builder) do + TranslatableFormBuilder.new(:dummy, DummyBanner.new, ApplicationController.new.view_context, {}) + end + + describe "#translatable_fields" do + it "renders fields for the enabled locales when the translation interface is enabled" do + Setting["feature.translation_interface"] = true + Setting["locales.enabled"] = "en fr" + + builder.translatable_fields do |translations_builder| + render translations_builder.text_field :title + end + + expect(page).to have_field "Title", count: 2 + end + end + + attr_reader :content + + def render(content) + @content ||= "" + @content << content + end + + def page + Capybara::Node::Simple.new(content) + end +end diff --git a/spec/models/budget/content_block_spec.rb b/spec/models/budget/content_block_spec.rb index 543035e6e..4ffbd1ca4 100644 --- a/spec/models/budget/content_block_spec.rb +++ b/spec/models/budget/content_block_spec.rb @@ -20,6 +20,14 @@ describe Budget::ContentBlock do expect(valid_block).to be_valid end + it "is not valid with a disabled locale" do + Setting["locales.enabled"] = "nl pt-BR" + + block.locale = "en" + + expect(block).not_to be_valid + end + describe "#name" do it "uses the heading name" do block = Budget::ContentBlock.new(heading: Budget::Heading.new(name: "Central")) diff --git a/spec/models/i18n_content_spec.rb b/spec/models/i18n_content_spec.rb index 6fe557ae6..e3a38816a 100644 --- a/spec/models/i18n_content_spec.rb +++ b/spec/models/i18n_content_spec.rb @@ -160,6 +160,16 @@ RSpec.describe I18nContent do end it "does not store new keys for disabled translations" do + Setting["locales.enabled"] = "es" + + I18nContent.update([{ id: "shared.yes", values: { "value_en" => "Oh, yeah" }}]) + + expect(I18nContent.all).to be_empty + end + + it "uses different enabled translations when given a parameter" do + Setting["locales.enabled"] = "en es" + I18nContent.update([{ id: "shared.yes", values: { "value_en" => "Oh, yeah" }}], [:es]) expect(I18nContent.all).to be_empty diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index d31c4ef3c..5df0fd8bc 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -258,4 +258,50 @@ describe Setting do expect(Setting.force_presence_postal_code?).to be true end end + + describe ".available_locales" do + before { allow(I18n).to receive(:available_locales).and_return(%i[de en es pt-BR]) } + + it "uses I18n available locales by default" do + Setting["locales.enabled"] = "" + + expect(Setting.enabled_locales).to eq %i[de en es pt-BR] + end + + it "defines available locales with a space-separated list" do + Setting["locales.enabled"] = "de es" + + expect(Setting.enabled_locales).to eq %i[de es] + end + + it "handles locales which include a dash" do + Setting["locales.enabled"] = "de en pt-BR" + + expect(Setting.enabled_locales).to eq %i[de en pt-BR] + end + + it "ignores extra whitespace between locales" do + Setting["locales.enabled"] = " de en pt-BR " + + expect(Setting.enabled_locales).to eq %i[de en pt-BR] + end + + it "ignores locales which aren't available" do + Setting["locales.enabled"] = "de es en-US fr zh-CN" + + expect(Setting.enabled_locales).to eq %i[de es] + end + + it "ignores words that don't make sense in this context" do + Setting["locales.enabled"] = "yes es 1234 en SuperCool" + + expect(Setting.enabled_locales).to eq %i[es en] + end + + it "uses I18n available locales when no locale is available" do + Setting["locales.enabled"] = "nl fr zh-CN" + + expect(Setting.enabled_locales).to eq %i[de en es pt-BR] + end + end end diff --git a/spec/models/site_customization/content_block_spec.rb b/spec/models/site_customization/content_block_spec.rb index a61753f12..3be09e812 100644 --- a/spec/models/site_customization/content_block_spec.rb +++ b/spec/models/site_customization/content_block_spec.rb @@ -27,4 +27,12 @@ RSpec.describe SiteCustomization::ContentBlock do block.name = "top_links" expect(block).not_to be_valid end + + it "is not valid with a disabled locale" do + Setting["locales.enabled"] = "nl pt-BR" + + block.locale = "en" + + expect(block).not_to be_valid + end end From 722e50a66998b29ea39c9a8b6846bc7e4fa1432e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 23 May 2024 22:28:41 +0200 Subject: [PATCH 17/22] Simplify code to find a content block We're only calling the `block_for` method from one place: the `content_block` helper, and we're never passing the locale parameter to that helper, which means we're always calling `block_for` using `I18n.locale`. So I'm not sure why we were doing the `locale ||=` assignment, since `I18n.locale` doesn't return nil. In any case, since this was added in commit c1de2dced, Ruby has added support for arguments forwarding, so we can use it here to simplify the code a little bit. --- app/helpers/application_helper.rb | 4 ++-- app/models/site_customization/content_block.rb | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index eeaf6a7ff..801bb8e16 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -46,8 +46,8 @@ module ApplicationHelper end end - def content_block(name, locale = I18n.locale) - SiteCustomization::ContentBlock.block_for(name, locale) + def content_block(...) + SiteCustomization::ContentBlock.block_for(...) end def self.asset_data_base64(path) diff --git a/app/models/site_customization/content_block.rb b/app/models/site_customization/content_block.rb index a7e95d0ab..6009505b7 100644 --- a/app/models/site_customization/content_block.rb +++ b/app/models/site_customization/content_block.rb @@ -4,8 +4,7 @@ class SiteCustomization::ContentBlock < ApplicationRecord validates :locale, presence: true, inclusion: { in: ->(*) { Setting.enabled_locales.map(&:to_s) }} validates :name, presence: true, uniqueness: { scope: :locale }, inclusion: { in: ->(*) { VALID_BLOCKS }} - def self.block_for(name, locale) - locale ||= I18n.default_locale + def self.block_for(name, locale = I18n.locale) find_by(name: name, locale: locale)&.body end end From 6de4737b7059e2620d727e2b8ce2e017bb405659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 4 Mar 2024 20:03:25 +0100 Subject: [PATCH 18/22] Allow different default locales per tenant Note that, for everything to work consistently, we need to make sure that the default locale is one of the available locales. Also note that we aren't overwriting the `#save ` method set by globalize. I didn't feel too comfortable changing a monkey-patch which ideally shouldn't be there in the first place, I haven't found a case where `Globalize.locale` is `nil` (since it defaults to `I18n.locale`, which should never be `nil`), so using `I18n.default_locale` probably doesn't affect us. --- app/controllers/application_controller.rb | 2 +- app/controllers/management/base_controller.rb | 2 +- app/lib/search_dictionary_selector.rb | 2 +- app/mailers/mailer.rb | 2 +- app/models/concerns/globalizable.rb | 2 +- app/models/setting.rb | 12 ++++- app/models/user.rb | 2 +- db/dev_seeds.rb | 2 +- .../form_content_block_component_spec.rb | 1 + ...rm_heading_content_block_component_spec.rb | 1 + .../layout/locale_switcher_component_spec.rb | 1 + .../application_controller_spec.rb | 4 +- .../management/base_controller_spec.rb | 4 +- .../subscriptions_controller_spec.rb | 5 +- spec/lib/search_dictionary_selector_spec.rb | 18 +++---- spec/mailers/mailer_spec.rb | 10 ++++ spec/models/budget/content_block_spec.rb | 1 + spec/models/globalizable_spec.rb | 47 +++++++++++++++++ spec/models/i18n_content_spec.rb | 1 + spec/models/setting_spec.rb | 52 +++++++++++++++++-- .../site_customization/content_block_spec.rb | 1 + spec/models/user_spec.rb | 10 ++++ 22 files changed, 156 insertions(+), 26 deletions(-) create mode 100644 spec/models/globalizable_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 28493bd02..6a70a9d5e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -56,7 +56,7 @@ class ApplicationController < ActionController::Base elsif Setting.enabled_locales.include?(session[:locale]&.to_sym) session[:locale] else - I18n.default_locale + Setting.default_locale end end diff --git a/app/controllers/management/base_controller.rb b/app/controllers/management/base_controller.rb index 2307d5ea3..745688743 100644 --- a/app/controllers/management/base_controller.rb +++ b/app/controllers/management/base_controller.rb @@ -44,7 +44,7 @@ class Management::BaseController < ActionController::Base session[:locale] = params[:locale].to_s end - session[:locale] ||= I18n.default_locale.to_s + session[:locale] ||= Setting.default_locale.to_s I18n.with_locale(session[:locale], &action) end diff --git a/app/lib/search_dictionary_selector.rb b/app/lib/search_dictionary_selector.rb index cc6168317..358204489 100644 --- a/app/lib/search_dictionary_selector.rb +++ b/app/lib/search_dictionary_selector.rb @@ -37,7 +37,7 @@ module SearchDictionarySelector private def find_from_i18n_default - key_to_lookup = I18n.default_locale.to_s.split("-").first.to_sym + key_to_lookup = Setting.default_locale.to_s.split("-").first.to_sym dictionary = I18N_TO_DICTIONARY[key_to_lookup] dictionary ||= "simple" diff --git a/app/mailers/mailer.rb b/app/mailers/mailer.rb index 8398bbf6b..7fcc3d98c 100644 --- a/app/mailers/mailer.rb +++ b/app/mailers/mailer.rb @@ -76,7 +76,7 @@ class Mailer < ApplicationMailer def user_invite(email) @email_to = email - I18n.with_locale(I18n.default_locale) do + I18n.with_locale(Setting.default_locale) do mail(to: @email_to, subject: t("mailers.user_invite.subject", org_name: Setting["org_name"])) end end diff --git a/app/models/concerns/globalizable.rb b/app/models/concerns/globalizable.rb index d4bb45edb..5578c37db 100644 --- a/app/models/concerns/globalizable.rb +++ b/app/models/concerns/globalizable.rb @@ -82,7 +82,7 @@ module Globalizable translation_class.instance_eval do validates method, length: options[:length], - if: lambda { |translation| translation.locale == I18n.default_locale } + if: lambda { |translation| translation.locale == Setting.default_locale } end if options.count > 1 translation_class.instance_eval do diff --git a/app/models/setting.rb b/app/models/setting.rb index 0dee92c84..7a69b569d 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -96,6 +96,7 @@ class Setting < ApplicationRecord # Code to be included at the top (inside ) of every page (useful for tracking) "html.per_page_code_head": "", "locales.enabled": nil, + "locales.default": nil, "map.latitude": 51.48, "map.longitude": 0.0, "map.zoom": 10, @@ -224,7 +225,16 @@ class Setting < ApplicationRecord def enabled_locales locales = Setting["locales.enabled"].to_s.split.map(&:to_sym) - (locales & I18n.available_locales).presence || I18n.available_locales + [ + default_locale, + *((locales & I18n.available_locales).presence || I18n.available_locales) + ].uniq + end + + def default_locale + locale = Setting["locales.default"].to_s.strip.to_sym + + ([locale] & I18n.available_locales).first || I18n.default_locale end end end diff --git a/app/models/user.rb b/app/models/user.rb index d13c27b2f..2fbfd9fed 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -339,7 +339,7 @@ class User < ApplicationRecord end def locale - self[:locale] || I18n.default_locale.to_s + self[:locale] || Setting.default_locale.to_s end def confirmation_required? diff --git a/db/dev_seeds.rb b/db/dev_seeds.rb index 5c67ac52f..4623cdac1 100644 --- a/db/dev_seeds.rb +++ b/db/dev_seeds.rb @@ -24,7 +24,7 @@ end def random_locales [ - I18n.default_locale, + Setting.default_locale, *(Setting.enabled_locales & %i[en es]), *Setting.enabled_locales.sample(4) ].uniq.take(5) diff --git a/spec/components/admin/site_customization/content_blocks/form_content_block_component_spec.rb b/spec/components/admin/site_customization/content_blocks/form_content_block_component_spec.rb index 6e44168b6..f025bd3a5 100644 --- a/spec/components/admin/site_customization/content_blocks/form_content_block_component_spec.rb +++ b/spec/components/admin/site_customization/content_blocks/form_content_block_component_spec.rb @@ -8,6 +8,7 @@ describe Admin::SiteCustomization::ContentBlocks::FormContentBlockComponent do end it "only includes enabled settings" do + Setting["locales.default"] = "de" Setting["locales.enabled"] = "de fr" render_inline component diff --git a/spec/components/admin/site_customization/content_blocks/form_heading_content_block_component_spec.rb b/spec/components/admin/site_customization/content_blocks/form_heading_content_block_component_spec.rb index b04c8a64d..09fe4469d 100644 --- a/spec/components/admin/site_customization/content_blocks/form_heading_content_block_component_spec.rb +++ b/spec/components/admin/site_customization/content_blocks/form_heading_content_block_component_spec.rb @@ -8,6 +8,7 @@ describe Admin::SiteCustomization::ContentBlocks::FormHeadingContentBlockCompone end it "only includes enabled settings" do + Setting["locales.default"] = "de" Setting["locales.enabled"] = "de fr" render_inline component diff --git a/spec/components/layout/locale_switcher_component_spec.rb b/spec/components/layout/locale_switcher_component_spec.rb index f043f3550..8678ef566 100644 --- a/spec/components/layout/locale_switcher_component_spec.rb +++ b/spec/components/layout/locale_switcher_component_spec.rb @@ -101,6 +101,7 @@ describe Layout::LocaleSwitcherComponent do context "when not all available locales are enabled" do before do allow(I18n).to receive(:available_locales).and_return(%i[en es fr]) + Setting["locales.default"] = "es" Setting["locales.enabled"] = "es fr" end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index f9bc09b65..921a3256e 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -23,9 +23,11 @@ describe ApplicationController do describe "#switch_locale" do it "uses the default locale by default" do + Setting["locales.default"] = "pt-BR" + get :index - expect(response.body).to eq "en" + expect(response.body).to eq "pt-BR" end it "uses the locale in the parameters when it's there" do diff --git a/spec/controllers/management/base_controller_spec.rb b/spec/controllers/management/base_controller_spec.rb index 9abe44fbb..35385f73d 100644 --- a/spec/controllers/management/base_controller_spec.rb +++ b/spec/controllers/management/base_controller_spec.rb @@ -34,9 +34,11 @@ describe Management::BaseController do describe "#switch_locale" do it "uses the default locale by default" do + Setting["locales.default"] = "pt-BR" + get :index - expect(response.body).to eq "en" + expect(response.body).to eq "pt-BR" end it "uses the locale in the parameters when it's there" do diff --git a/spec/controllers/subscriptions_controller_spec.rb b/spec/controllers/subscriptions_controller_spec.rb index e675136de..11a96fd8d 100644 --- a/spec/controllers/subscriptions_controller_spec.rb +++ b/spec/controllers/subscriptions_controller_spec.rb @@ -29,13 +29,14 @@ describe SubscriptionsController do end it "only accepts enabled locales" do - Setting["locales.enabled"] = "en nl" + Setting["locales.default"] = "fr" + Setting["locales.enabled"] = "fr nl" create(:user, locale: "es", subscriptions_token: "mytoken") get :edit, params: { token: "mytoken" } - expect(session[:locale]).to eq "en" + expect(session[:locale]).to eq "fr" end end end diff --git a/spec/lib/search_dictionary_selector_spec.rb b/spec/lib/search_dictionary_selector_spec.rb index d9a618b25..b0f2cbc29 100644 --- a/spec/lib/search_dictionary_selector_spec.rb +++ b/spec/lib/search_dictionary_selector_spec.rb @@ -3,27 +3,23 @@ require "rails_helper" describe SearchDictionarySelector do context "from I18n default locale" do before { allow(subject).to receive(:call).and_call_original } - around do |example| - original_i18n_default = I18n.default_locale - begin - example.run - ensure - I18n.default_locale = original_i18n_default - end - end it "returns correct dictionary for simple locale" do - I18n.default_locale = :es + Setting["locales.default"] = "es" + expect(subject.call).to eq("spanish") end it "returns correct dictionary for compound locale" do - I18n.default_locale = :"pt-BR" + Setting["locales.default"] = "pt-BR" + expect(subject.call).to eq("portuguese") end it "returns simple for unsupported locale" do - expect(I18n).to receive(:default_locale).and_return(:pl) # avoiding I18n::InvalidLocale + allow(I18n).to receive(:available_locales).and_return(%i[en pl]) + Setting["locales.default"] = "pl" + expect(subject.call).to eq("simple") end end diff --git a/spec/mailers/mailer_spec.rb b/spec/mailers/mailer_spec.rb index af991075c..aed829ae2 100644 --- a/spec/mailers/mailer_spec.rb +++ b/spec/mailers/mailer_spec.rb @@ -30,6 +30,16 @@ describe Mailer do end end + describe "#user_invite" do + it "uses the default locale setting" do + Setting["locales.default"] = "es" + + Mailer.user_invite("invited@consul.dev").deliver_now + + expect(ActionMailer::Base.deliveries.last.body.to_s).to match " "Oh, yeah" }}]) diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index 5df0fd8bc..f3a87ce22 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -260,7 +260,7 @@ describe Setting do end describe ".available_locales" do - before { allow(I18n).to receive(:available_locales).and_return(%i[de en es pt-BR]) } + before { allow(I18n).to receive_messages(default_locale: :de, available_locales: %i[de en es pt-BR]) } it "uses I18n available locales by default" do Setting["locales.enabled"] = "" @@ -280,6 +280,12 @@ describe Setting do expect(Setting.enabled_locales).to eq %i[de en pt-BR] end + it "adds the default locale to the list of available locales" do + Setting["locales.enabled"] = "en es" + + expect(Setting.enabled_locales).to eq %i[de en es] + end + it "ignores extra whitespace between locales" do Setting["locales.enabled"] = " de en pt-BR " @@ -293,9 +299,9 @@ describe Setting do end it "ignores words that don't make sense in this context" do - Setting["locales.enabled"] = "yes es 1234 en SuperCool" + Setting["locales.enabled"] = "yes de 1234 en SuperCool" - expect(Setting.enabled_locales).to eq %i[es en] + expect(Setting.enabled_locales).to eq %i[de en] end it "uses I18n available locales when no locale is available" do @@ -304,4 +310,44 @@ describe Setting do expect(Setting.enabled_locales).to eq %i[de en es pt-BR] end end + + describe ".default_locale" do + before { allow(I18n).to receive_messages(default_locale: :en, available_locales: %i[de en es pt-BR]) } + + it "uses I18n default locale by default" do + Setting["locales.default"] = "" + + expect(Setting.default_locale).to eq :en + end + + it "allows defining the default locale" do + Setting["locales.default"] = "de" + + expect(Setting.default_locale).to eq :de + end + + it "handles locales which include a dash" do + Setting["locales.default"] = "pt-BR" + + expect(Setting.default_locale).to eq :"pt-BR" + end + + it "ignores extra whitespace in the locale name" do + Setting["locales.default"] = " es " + + expect(Setting.default_locale).to eq :es + end + + it "ignores locales which aren't available" do + Setting["locales.default"] = "fr" + + expect(Setting.default_locale).to eq :en + end + + it "ignores an array of several locales" do + Setting["locales.default"] = "de es" + + expect(Setting.default_locale).to eq :en + end + end end diff --git a/spec/models/site_customization/content_block_spec.rb b/spec/models/site_customization/content_block_spec.rb index 3be09e812..d0b5d276b 100644 --- a/spec/models/site_customization/content_block_spec.rb +++ b/spec/models/site_customization/content_block_spec.rb @@ -29,6 +29,7 @@ RSpec.describe SiteCustomization::ContentBlock do end it "is not valid with a disabled locale" do + Setting["locales.default"] = "nl" Setting["locales.enabled"] = "nl pt-BR" block.locale = "en" diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 62a0663be..0e9d4fe22 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -81,6 +81,16 @@ describe User do end end + describe "#locale" do + it "defaults to the default locale setting" do + Setting["locales.default"] = "nl" + + user = build(:user, locale: nil) + + expect(user.locale).to eq "nl" + end + end + describe "preferences" do describe "email_on_comment" do it "is false by default" do From 999d5c2f67f817aebdbd35c0dc11a7194ff8bf52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 6 Apr 2024 23:21:23 +0200 Subject: [PATCH 19/22] Remove redundant "Manage" in admin menu entries This is the admin section; it's obvious that every link in the menu will take you to a page to manage something. We're going to add a new item to either the "Settings" or the "Site content" section, so it's a good time to improve what's already there. --- config/locales/en/admin.yml | 14 +++++++------- config/locales/es/admin.yml | 10 +++++----- spec/system/admin/banners_spec.rb | 4 ++-- spec/system/admin/geozones_spec.rb | 2 +- spec/system/admin/poll/booths_spec.rb | 6 +++--- spec/system/admin/poll/shifts_spec.rb | 14 +++++++------- 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 66ff65ea1..09d07d33a 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -727,11 +727,11 @@ en: menu: activity: Moderator activity admin: Admin menu - banner: Manage banners + banner: Banners proposals: Proposals proposals_topics: Proposals topics budgets: Participatory budgets - geozones: Manage geozones + geozones: Geozones hidden_comments: Hidden comments hidden_debates: Hidden debates hidden_proposals: Hidden proposals @@ -752,7 +752,7 @@ en: polls: Polls poll_booths: Booths location poll_booth_assignments: Booths Assignments - poll_shifts: Manage shifts + poll_shifts: Shifts Assignments officials: Officials organizations: Organisations settings: Global settings @@ -794,7 +794,7 @@ en: dashboard_actions: Resources and actions debates: "Debates" comments: "Comments" - local_census_records: Manage local census + local_census_records: Local census machine_learning: "AI / Machine learning" multitenancy: Multitenancy administrators: @@ -1232,7 +1232,7 @@ en: show: location: "Location" booth: - shifts: "Manage shifts" + shifts: "Shifts Assignments" officials: edit: destroy: Remove "Official" status @@ -1713,7 +1713,7 @@ en: empty: "There are no changes logged" local_census_records: index: - title: Manage local census + title: Local census create: Create new local census record no_local_census_records: There are no local census records. document_type: Document type @@ -1738,7 +1738,7 @@ en: create: notice: Local census records import process executed successfully! show: - title: Manage local census + title: Local census subtitle: Import process results import: Import again errored: Errored rows diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index 81297a548..e5d11242c 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -727,11 +727,11 @@ es: menu: activity: Actividad de moderadores admin: Menú de administración - banner: Gestionar banners + banner: Banners proposals: Propuestas proposals_topics: Temas de propuestas budgets: Presupuestos participativos - geozones: Gestionar zonas + geozones: Zonas hidden_comments: Comentarios ocultos hidden_debates: Debates ocultos hidden_proposals: Propuestas ocultas @@ -794,7 +794,7 @@ es: dashboard_actions: Recursos y acciones debates: "Debates" comments: "Comentarios" - local_census_records: Gestionar censo local + local_census_records: Censo local machine_learning: "IA / Machine learning" multitenancy: Multientidad administrators: @@ -1713,7 +1713,7 @@ es: empty: "No hay cambios registrados" local_census_records: index: - title: Gestionar censo local + title: Censo local create: Crear nuevo registro en el censo local no_local_census_records: No hay registros de censo local document_type: Tipo de documento @@ -1738,7 +1738,7 @@ es: create: notice: ¡Proceso de import de registros del censo local ejecutado correctamente! show: - title: Gestionar censo local + title: Censo local subtitle: Resultados del proceso de importación import: Importar otro fichero errored: Filas erróneas diff --git a/spec/system/admin/banners_spec.rb b/spec/system/admin/banners_spec.rb index c604f6d6e..33d67c405 100644 --- a/spec/system/admin/banners_spec.rb +++ b/spec/system/admin/banners_spec.rb @@ -65,7 +65,7 @@ describe "Admin banners magement", :admin do within("#side_menu") do click_button "Site content" - click_link "Manage banners" + click_link "Banners" end click_link "Create banner" @@ -134,7 +134,7 @@ describe "Admin banners magement", :admin do within("#side_menu") do click_button "Site content" - click_link "Manage banners" + click_link "Banners" end click_link "Edit" diff --git a/spec/system/admin/geozones_spec.rb b/spec/system/admin/geozones_spec.rb index 384cccc73..3ece9f6f4 100644 --- a/spec/system/admin/geozones_spec.rb +++ b/spec/system/admin/geozones_spec.rb @@ -16,7 +16,7 @@ describe "Admin geozones", :admin do within("#side_menu") do click_button "Settings" - click_link "Manage geozones" + click_link "Geozones" end click_link "Create geozone" diff --git a/spec/system/admin/poll/booths_spec.rb b/spec/system/admin/poll/booths_spec.rb index 392f01b62..9462358df 100644 --- a/spec/system/admin/poll/booths_spec.rb +++ b/spec/system/admin/poll/booths_spec.rb @@ -39,7 +39,7 @@ describe "Admin booths", :admin do within("#side_menu") do click_button "Voting booths" - click_link "Manage shifts" + click_link "Shifts Assignments" end expect(page).to have_css(".booth", count: 1) @@ -80,7 +80,7 @@ describe "Admin booths", :admin do visit admin_booths_path within("#booth_#{booth.id}") do - expect(page).not_to have_link "Manage shifts" + expect(page).not_to have_link "Shifts Assignments" click_link "Edit" end @@ -105,7 +105,7 @@ describe "Admin booths", :admin do visit available_admin_booths_path within("#booth_#{booth.id}") do - click_link "Manage shifts" + click_link "Shifts Assignments" end click_link "Go back" diff --git a/spec/system/admin/poll/shifts_spec.rb b/spec/system/admin/poll/shifts_spec.rb index a5e25a554..ad55359f8 100644 --- a/spec/system/admin/poll/shifts_spec.rb +++ b/spec/system/admin/poll/shifts_spec.rb @@ -44,7 +44,7 @@ describe "Admin shifts", :admin do visit available_admin_booths_path within("#booth_#{booth.id}") do - click_link "Manage shifts" + click_link "Shifts Assignments" end expect(page).to have_content "This booth has no shifts" @@ -71,7 +71,7 @@ describe "Admin shifts", :admin do visit available_admin_booths_path within("#booth_#{booth.id}") do - click_link "Manage shifts" + click_link "Shifts Assignments" end expect(page).to have_css(".shift", count: 1) @@ -117,7 +117,7 @@ describe "Admin shifts", :admin do visit available_admin_booths_path within("#booth_#{booth.id}") do - click_link "Manage shifts" + click_link "Shifts Assignments" end expect(page).to have_css(".shift", count: 2) @@ -159,7 +159,7 @@ describe "Admin shifts", :admin do visit available_admin_booths_path within("#booth_#{booth.id}") do - click_link "Manage shifts" + click_link "Shifts Assignments" end expect(page).to have_content "This booth has no shifts" @@ -182,7 +182,7 @@ describe "Admin shifts", :admin do visit available_admin_booths_path within("#booth_#{booth.id}") do - click_link "Manage shifts" + click_link "Shifts Assignments" end expect(page).to have_css(".shift", count: 1) @@ -208,7 +208,7 @@ describe "Admin shifts", :admin do visit available_admin_booths_path within("#booth_#{booth.id}") do - click_link "Manage shifts" + click_link "Shifts Assignments" end expect(page).to have_css(".shift", count: 1) @@ -237,7 +237,7 @@ describe "Admin shifts", :admin do visit available_admin_booths_path within("#booth_#{booth.id}") do - click_link "Manage shifts" + click_link "Shifts Assignments" end expect(page).to have_css(".shift", count: 1) From 78bbf430d5e2f4da93fa69b1820aecfe05a9b265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 9 Apr 2024 03:18:01 +0200 Subject: [PATCH 20/22] Add a form to edit available locales We're using different controls depending on the number of available locales. When there are only a few locales, the solution is obvious: radio buttons to select the default language, and checkboxes to select the available ones are simple and intuitive. With many languages, showing two consecutive lists of 30 languages could be confusing, though, particularly on small devices, where scrolling through both lists could be hard. So, in this case, we're rendering a