From 0c59c2dfb4996c15fd118c9426bd9607c8fb74a5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 10 Apr 2024 03:38:00 +0200
Subject: [PATCH] Extract model to handle locales settings

This way we can simplify the view, particularly the form. However, we're
still adding some complexity to the form so inputs are inside labels and
so the collection is easier to style with CSS.
---
 .../stylesheets/admin/locales/form.scss       |  4 ++
 .../admin/locales/form_component.html.erb     | 37 +++++++++----------
 .../admin/locales/form_component.rb           | 16 ++++----
 .../admin/locales/show_component.html.erb     |  2 +-
 .../admin/locales/show_component.rb           |  7 ++--
 app/controllers/admin/locales_controller.rb   | 24 +++++++++---
 app/models/abilities/administrator.rb         |  2 +
 app/models/setting/locales_settings.rb        | 23 ++++++++++++
 app/views/admin/locales/show.html.erb         |  2 +-
 config/locales/en/activemodel.yml             |  3 ++
 config/locales/en/admin.yml                   |  2 -
 config/locales/es/activemodel.yml             |  3 ++
 config/locales/es/admin.yml                   |  2 -
 .../admin/locales/form_component_spec.rb      |  6 +--
 spec/models/setting/locales_settings_spec.rb  | 13 +++++++
 15 files changed, 99 insertions(+), 47 deletions(-)
 create mode 100644 app/models/setting/locales_settings.rb
 create mode 100644 spec/models/setting/locales_settings_spec.rb

diff --git a/app/assets/stylesheets/admin/locales/form.scss b/app/assets/stylesheets/admin/locales/form.scss
index 7c1b36384..c834ee28b 100644
--- a/app/assets/stylesheets/admin/locales/form.scss
+++ b/app/assets/stylesheets/admin/locales/form.scss
@@ -7,6 +7,10 @@
     width: auto;
   }
 
+  .help-text {
+    display: block;
+  }
+
   > select + fieldset,
   > fieldset + fieldset {
     margin-top: $line-height / 2;
diff --git a/app/components/admin/locales/form_component.html.erb b/app/components/admin/locales/form_component.html.erb
index 109960c97..6c8bbb291 100644
--- a/app/components/admin/locales/form_component.html.erb
+++ b/app/components/admin/locales/form_component.html.erb
@@ -1,37 +1,36 @@
-<%= form_tag admin_locales_path, method: :patch, class: "locales-form" do %>
+<%= form_for locales_settings, url: admin_locales_path, html: { class: "locales-form" } do |f| %>
   <% if many_available_locales? %>
-    <%= label_tag "default_locale", t("admin.locales.default") %>
-    <p class="help-text"><%= sanitize(t("admin.locales.default_help_text")) %></p>
-    <%= select_tag "default_locale", locales_options %>
+    <%= f.select :default, locales_options, hint: sanitize(t("admin.locales.default_help_text")) %>
   <% else %>
     <fieldset>
-      <legend><%= t("admin.locales.default") %></legend>
+      <legend><%= attribute_name(:default) %></legend>
       <p class="help-text"><%= sanitize(t("admin.locales.default_help_text")) %></p>
 
       <div class="collection-radio-buttons">
-        <% available_locales.each do |locale| %>
-          <%= label_tag "default_locale_#{locale}" do %>
-            <%= radio_button_tag "default_locale", locale, locale == default %>
-            <%= name_for_locale(locale) %>
-          <% end %>
+        <%= f.collection_radio_buttons(
+          :default,
+          available_locales,
+          :to_sym,
+          ->(locale) { name_for_locale(locale) }
+        ) do |b| %>
+          <%= b.label { b.radio_button + b.text } %>
         <% end %>
       </div>
     </fieldset>
   <% end %>
 
   <fieldset>
-    <legend><%= t("admin.locales.enabled") %></legend>
+    <legend><%= attribute_name(:enabled) %></legend>
     <p class="help-text"><%= t("admin.locales.enabled_help_text") %></p>
 
     <div class="collection-check-boxes">
-      <% available_locales.each do |locale| %>
-        <%= label_tag "enabled_locales_#{locale}" do %>
-          <%= check_box_tag "enabled_locales[]",
-                            locale,
-                            enabled_locales.include?(locale),
-                            id: "enabled_locales_#{locale}" %>
-          <%= name_for_locale(locale) %>
-        <% end %>
+      <%= f.collection_check_boxes(
+        :enabled,
+        available_locales,
+        :to_sym,
+        ->(locale) { name_for_locale(locale) }
+      ) do |b| %>
+        <%= b.label { b.check_box + b.text } %>
       <% end %>
     </div>
   </fieldset>
diff --git a/app/components/admin/locales/form_component.rb b/app/components/admin/locales/form_component.rb
index 6e96e0b8b..4cd16bc85 100644
--- a/app/components/admin/locales/form_component.rb
+++ b/app/components/admin/locales/form_component.rb
@@ -1,10 +1,9 @@
 class Admin::Locales::FormComponent < ApplicationComponent
-  attr_reader :enabled_locales, :default
+  attr_reader :locales_settings
   use_helpers :name_for_locale
 
-  def initialize(enabled_locales, default:)
-    @enabled_locales = enabled_locales
-    @default = default
+  def initialize(locales_settings)
+    @locales_settings = locales_settings
   end
 
   private
@@ -18,13 +17,14 @@ class Admin::Locales::FormComponent < ApplicationComponent
     end
 
     def locales_options
-      options_for_select(
-        available_locales.map { |locale| [name_for_locale(locale), locale] },
-        default
-      )
+      available_locales.map { |locale| [name_for_locale(locale), locale] }
     end
 
     def select_field_threshold
       10
     end
+
+    def attribute_name(...)
+      locales_settings.class.human_attribute_name(...)
+    end
 end
diff --git a/app/components/admin/locales/show_component.html.erb b/app/components/admin/locales/show_component.html.erb
index 399df83cf..6a1463f3e 100644
--- a/app/components/admin/locales/show_component.html.erb
+++ b/app/components/admin/locales/show_component.html.erb
@@ -1,4 +1,4 @@
 <%= header %>
 <% provide :main_class, "admin-locales-show" %>
 
-<%= render Admin::Locales::FormComponent.new(locales, default: default) %>
+<%= render Admin::Locales::FormComponent.new(locales_settings) %>
diff --git a/app/components/admin/locales/show_component.rb b/app/components/admin/locales/show_component.rb
index 2625d049f..e33e66b51 100644
--- a/app/components/admin/locales/show_component.rb
+++ b/app/components/admin/locales/show_component.rb
@@ -1,10 +1,9 @@
 class Admin::Locales::ShowComponent < ApplicationComponent
   include Header
-  attr_reader :locales, :default
+  attr_reader :locales_settings
 
-  def initialize(locales, default:)
-    @locales = locales
-    @default = default
+  def initialize(locales_settings)
+    @locales_settings = locales_settings
   end
 
   def title
diff --git a/app/controllers/admin/locales_controller.rb b/app/controllers/admin/locales_controller.rb
index 5d0088393..df822a29b 100644
--- a/app/controllers/admin/locales_controller.rb
+++ b/app/controllers/admin/locales_controller.rb
@@ -1,15 +1,27 @@
 class Admin::LocalesController < Admin::BaseController
+  before_action :set_locales_settings
+  authorize_resource :locales_settings
+
   def show
-    @enabled_locales = Setting.enabled_locales
-    @default_locale = Setting.default_locale
   end
 
   def update
-    Setting.transaction do
-      Setting["locales.default"] = params["default_locale"]
-      Setting["locales.enabled"] = [params["default_locale"], *params["enabled_locales"]].join(" ")
-    end
+    @locales_settings.update!(locales_settings_params)
 
     redirect_to admin_locales_path, notice: t("admin.locales.update.notice")
   end
+
+  private
+
+    def locales_settings_params
+      params.require(:setting_locales_settings).permit(allowed_params)
+    end
+
+    def allowed_params
+      [:default, enabled: []]
+    end
+
+    def set_locales_settings
+      @locales_settings = Setting::LocalesSettings.new
+    end
 end
diff --git a/app/models/abilities/administrator.rb b/app/models/abilities/administrator.rb
index cd5e4d3f0..c6abe703c 100644
--- a/app/models/abilities/administrator.rb
+++ b/app/models/abilities/administrator.rb
@@ -134,6 +134,8 @@ module Abilities
       can [:deliver], Newsletter, hidden_at: nil
       can [:manage], Dashboard::AdministratorTask
 
+      can :manage, Setting::LocalesSettings
+
       can :manage, LocalCensusRecord
       can [:create, :read], LocalCensusRecords::Import
 
diff --git a/app/models/setting/locales_settings.rb b/app/models/setting/locales_settings.rb
new file mode 100644
index 000000000..26db131a9
--- /dev/null
+++ b/app/models/setting/locales_settings.rb
@@ -0,0 +1,23 @@
+class Setting
+  class LocalesSettings
+    include ActiveModel::Model
+    include ActiveModel::Attributes
+
+    attribute :enabled, array: true, default: -> { Setting.enabled_locales }
+    attribute :default, default: -> { Setting.default_locale }
+
+    def persisted?
+      true
+    end
+
+    def update(attributes)
+      assign_attributes(attributes)
+
+      Setting.transaction do
+        Setting["locales.default"] = default
+        Setting["locales.enabled"] = [default, *enabled].join(" ")
+      end
+    end
+    alias_method :update!, :update
+  end
+end
diff --git a/app/views/admin/locales/show.html.erb b/app/views/admin/locales/show.html.erb
index bb4561ded..994174d9f 100644
--- a/app/views/admin/locales/show.html.erb
+++ b/app/views/admin/locales/show.html.erb
@@ -1 +1 @@
-<%= render Admin::Locales::ShowComponent.new(@enabled_locales, default: @default_locale) %>
+<%= render Admin::Locales::ShowComponent.new(@locales_settings) %>
diff --git a/config/locales/en/activemodel.yml b/config/locales/en/activemodel.yml
index ea83bed5e..346b0e3d5 100644
--- a/config/locales/en/activemodel.yml
+++ b/config/locales/en/activemodel.yml
@@ -38,6 +38,9 @@ en:
         year_of_birth: "Year born"
       local_census_records/import:
         file: File
+      setting/locales_settings:
+        default: Default language
+        enabled: Enabled languages
     errors:
       models:
         local_census_records/import:
diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml
index 736f767fe..3b70a864d 100644
--- a/config/locales/en/admin.yml
+++ b/config/locales/en/admin.yml
@@ -711,9 +711,7 @@ en:
         index:
           title: Following
     locales:
-      default: Default language
       default_help_text: "This is the default language of the application, and changing it will <strong>affect every user</strong> visiting the website for the first time."
-      enabled: Enabled languages
       enabled_help_text: The default language, selected above, will be included automatically.
       update:
         notice: Languages updated successfully
diff --git a/config/locales/es/activemodel.yml b/config/locales/es/activemodel.yml
index e303303d7..eaf66fc74 100644
--- a/config/locales/es/activemodel.yml
+++ b/config/locales/es/activemodel.yml
@@ -38,6 +38,9 @@ es:
         year_of_birth: "Año de nacimiento"
       local_census_records/import:
         file: Archivo
+      setting/locales_settings:
+        default: Idioma por defecto
+        enabled: Idiomas habilitados
     errors:
       models:
         local_census_records/import:
diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml
index 7c90832be..513ae0306 100644
--- a/config/locales/es/admin.yml
+++ b/config/locales/es/admin.yml
@@ -711,9 +711,7 @@ es:
         index:
           title: Seguimiento
     locales:
-      default: Idioma por defecto
       default_help_text: "Cambiar el idioma por defecto <strong>afectará a todos los usuarios</strong> que visiten la página por primera vez."
-      enabled: Idiomas habilitados
       enabled_help_text: El idioma por defecto, seleccionado con anterioridad, se habilitará automáticamente.
       update:
         notice: Idiomas actualizados con éxito
diff --git a/spec/components/admin/locales/form_component_spec.rb b/spec/components/admin/locales/form_component_spec.rb
index 2cb7e4ace..ea2398f01 100644
--- a/spec/components/admin/locales/form_component_spec.rb
+++ b/spec/components/admin/locales/form_component_spec.rb
@@ -3,10 +3,8 @@ require "rails_helper"
 describe Admin::Locales::FormComponent do
   let(:default_locale) { :nl }
   let(:enabled_locales) { %i[en nl] }
-
-  let(:component) do
-    Admin::Locales::FormComponent.new(enabled_locales, default: default_locale)
-  end
+  let(:locales_settings) { Setting::LocalesSettings.new(default: default_locale, enabled: enabled_locales) }
+  let(:component) { Admin::Locales::FormComponent.new(locales_settings) }
 
   describe "default language selector" do
     before { allow(I18n).to receive(:available_locales).and_return(%i[de en es nl]) }
diff --git a/spec/models/setting/locales_settings_spec.rb b/spec/models/setting/locales_settings_spec.rb
new file mode 100644
index 000000000..1b21ec582
--- /dev/null
+++ b/spec/models/setting/locales_settings_spec.rb
@@ -0,0 +1,13 @@
+require "rails_helper"
+
+describe Setting::LocalesSettings do
+  describe "#update!" do
+    it "saves the default locale in the enabled ones when nothing is enabled" do
+      Setting::LocalesSettings.new.update!(default: "es", enabled: %w[])
+      updated_locales_settings = Setting::LocalesSettings.new
+
+      expect(updated_locales_settings.default).to eq :es
+      expect(updated_locales_settings.enabled).to match_array [:es]
+    end
+  end
+end