From a91afa0e78a5fa3e54aee38ec64c43fcea0b7abb Mon Sep 17 00:00:00 2001
From: Andy Allan <git@gravitystorm.co.uk>
Date: Wed, 15 Mar 2023 18:08:59 +0000
Subject: [PATCH 1/3] Use .with_locale for about page

This simplifies the code, since we can avoid having to specify the
locale for every translation.

The test was added since I originally developed another approach,
but that caused exceptions on invalid locales.
---
 app/views/site/_about_section.html.erb   |  2 +-
 app/views/site/about.html.erb            | 83 ++++++++++++------------
 test/controllers/site_controller_test.rb |  5 ++
 3 files changed, 48 insertions(+), 42 deletions(-)

diff --git a/app/views/site/_about_section.html.erb b/app/views/site/_about_section.html.erb
index ead28f9fd..60fd3f069 100644
--- a/app/views/site/_about_section.html.erb
+++ b/app/views/site/_about_section.html.erb
@@ -1,7 +1,7 @@
 <%= tag.div :class => "section", :id => local_assigns[:id] do %>
   <div class='d-flex align-items-center gap-2 mb-2'>
     <div class='flex-shrink-0 icon <%= icon %>'></div>
-    <h2 class='flex-grow-1 mb-0'><%= t "site.about.#{title}_title", :locale => @locale %></h2>
+    <h2 class='flex-grow-1 mb-0'><%= t "site.about.#{title}_title" %></h2>
   </div>
   <%= yield %>
 <% end %>
diff --git a/app/views/site/about.html.erb b/app/views/site/about.html.erb
index 5011523e1..12d9a0608 100644
--- a/app/views/site/about.html.erb
+++ b/app/views/site/about.html.erb
@@ -1,45 +1,46 @@
-<%= tag.div :lang => @locale, :dir => t("html.dir", :locale => @locale) do %>
-  <div class="container-lg attr">
-    <div class='row'>
-      <div class='col-sm-7 user-image'></div>
-      <div class='col-sm-5 px-5 py-3 byosm'>
-        <p class='h5 text-white text-nowrap'><%= t ".copyright_html", :locale => @locale %></p>
+<% I18n.with_locale @locale do %>
+  <%= tag.div :lang => @locale, :dir => t("html.dir") do %>
+    <div class="container-lg attr">
+      <div class='row'>
+        <div class='col-sm-7 user-image'></div>
+        <div class='col-sm-5 px-5 py-3 byosm'>
+          <p class='h5 text-white text-nowrap'><%= t ".copyright_html" %></p>
+        </div>
+      </div>
+      <div class='row'>
+        <div class="w-100 px-5 py-4 bg-dark">
+          <h1 class="text-white fw-light"><%= t ".used_by_html", :name => tag.span("OpenStreetMap", :class => "user-name") %></h1>
+        </div>
       </div>
     </div>
-    <div class='row'>
-      <div class="w-100 px-5 py-4 bg-dark">
-        <h1 class="text-white fw-light"><%= t ".used_by_html", :name => tag.span("OpenStreetMap", :class => "user-name"), :locale => @locale %></h1>
-      </div>
+
+    <div class='bg-white px-5 py-4'>
+      <p class="lead"><%= t ".lede_text" %></p>
+
+      <%= render :layout => "about_section", :locals => { :icon => "local", :title => "local_knowledge" } do %>
+        <p><%= t "site.about.local_knowledge_html" %></p>
+      <% end %>
+
+      <%= render :layout => "about_section", :locals => { :icon => "community", :title => "community_driven" } do %>
+        <p><%= t "site.about.community_driven_html", :diary_path => diary_entries_path %></p>
+      <% end %>
+
+      <%= render :layout => "about_section", :locals => { :id => "open-data", :icon => "open", :title => "open_data" } do %>
+        <p><%= t "site.about.open_data_html", :copyright_path => copyright_path %></p>
+      <% end %>
+
+      <%= render :layout => "about_section", :locals => { :id => "legal", :icon => "legal", :title => "legal" } do %>
+        <p><%= t "site.about.legal_1_html" %></p>
+        <p><%= t "site.about.legal_2_html" %></p>
+      <% end %>
+
+      <%= render :layout => "about_section", :locals => { :id => "partners", :icon => "partners", :title => "partners" } do %>
+        <p><%= t "layouts.hosting_partners_html", :ucl => link_to(t("layouts.partners_ucl"), "https://www.ucl.ac.uk"),
+                                                  :fastly => link_to(t("layouts.partners_fastly"), "https://www.fastly.com/"),
+                                                  :bytemark => link_to(t("layouts.partners_bytemark"), "https://www.bytemark.co.uk"),
+                                                  :partners => link_to(t("layouts.partners_partners"), "https://hardware.openstreetmap.org/thanks/") %>
+        </p>
+      <% end %>
     </div>
-  </div>
-
-  <div class='bg-white px-5 py-4'>
-    <p class="lead"><%= t ".lede_text", :locale => @locale %></p>
-
-    <%= render :layout => "about_section", :locals => { :icon => "local", :title => "local_knowledge" } do %>
-      <p><%= t "site.about.local_knowledge_html", :locale => @locale %></p>
-    <% end %>
-
-    <%= render :layout => "about_section", :locals => { :icon => "community", :title => "community_driven" } do %>
-      <p><%= t "site.about.community_driven_html", :locale => @locale, :diary_path => diary_entries_path %></p>
-    <% end %>
-
-    <%= render :layout => "about_section", :locals => { :id => "open-data", :icon => "open", :title => "open_data" } do %>
-      <p><%= t "site.about.open_data_html", :locale => @locale, :copyright_path => copyright_path %></p>
-    <% end %>
-
-    <%= render :layout => "about_section", :locals => { :id => "legal", :icon => "legal", :title => "legal" } do %>
-      <p><%= t "site.about.legal_1_html", :locale => @locale %></p>
-      <p><%= t "site.about.legal_2_html", :locale => @locale %></p>
-    <% end %>
-
-    <%= render :layout => "about_section", :locals => { :id => "partners", :icon => "partners", :title => "partners" } do %>
-      <p><%= t "layouts.hosting_partners_html", :locale => @locale,
-                                                :ucl => link_to(t("layouts.partners_ucl", :locale => @locale), "https://www.ucl.ac.uk"),
-                                                :fastly => link_to(t("layouts.partners_fastly", :locale => @locale), "https://www.fastly.com/"),
-                                                :bytemark => link_to(t("layouts.partners_bytemark", :locale => @locale), "https://www.bytemark.co.uk"),
-                                                :partners => link_to(t("layouts.partners_partners", :locale => @locale), "https://hardware.openstreetmap.org/thanks/") %>
-      </p>
-    <% end %>
-  </div>
+  <% end %>
 <% end %>
diff --git a/test/controllers/site_controller_test.rb b/test/controllers/site_controller_test.rb
index 8a38da5c0..75a042bfe 100644
--- a/test/controllers/site_controller_test.rb
+++ b/test/controllers/site_controller_test.rb
@@ -476,6 +476,11 @@ class SiteControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
     assert_template "about"
     assert_select "div[lang='ar'][dir='rtl']"
+
+    # Page should still render even with incorrect locale
+    get about_path(:about_locale => "zzz")
+    assert_response :success
+    assert_template "about"
   end
 
   # Test the export page

From d41d52877ed68902c45231a89715470774e7f944 Mon Sep 17 00:00:00 2001
From: Andy Allan <git@gravitystorm.co.uk>
Date: Wed, 22 Mar 2023 09:49:34 +0000
Subject: [PATCH 2/3] Use with_locale for the copyright page

This avoids having to override the locale on each and every translation
---
 app/views/site/copyright.html.erb | 84 ++++++++++++++++---------------
 1 file changed, 44 insertions(+), 40 deletions(-)

diff --git a/app/views/site/copyright.html.erb b/app/views/site/copyright.html.erb
index 3253aebe1..977398660 100644
--- a/app/views/site/copyright.html.erb
+++ b/app/views/site/copyright.html.erb
@@ -35,53 +35,57 @@
     <% end %>
   <% end %>
 
-  <%= tag.h1 :lang => @locale, :dir => t("html.dir", :locale => @locale) do %>
-    <%= t ".legal_babble.title_html", :locale => @locale %>
+  <% I18n.with_locale @locale do %>
+    <%= tag.h1 :lang => @locale, :dir => t("html.dir") do %>
+      <%= t ".legal_babble.title_html" %>
+    <% end %>
   <% end %>
 
 <% end %>
 
-<%= tag.div :lang => @locale, :dir => t("html.dir", :locale => @locale) do %>
-  <p><%= t ".legal_babble.intro_1_html", :locale => @locale %></p>
-  <p><%= t ".legal_babble.intro_2_html", :locale => @locale %></p>
-  <p><%= t ".legal_babble.intro_3_1_html", :locale => @locale %></p>
+<% I18n.with_locale @locale do %>
+  <%= tag.div :lang => @locale, :dir => t("html.dir") do %>
+    <p><%= t ".legal_babble.intro_1_html" %></p>
+    <p><%= t ".legal_babble.intro_2_html" %></p>
+    <p><%= t ".legal_babble.intro_3_1_html" %></p>
 
-  <h3><%= t ".legal_babble.credit_title_html", :locale => @locale %></h3>
-  <p><%= t ".legal_babble.credit_1_html", :locale => @locale %></p>
-  <p><%= t ".legal_babble.credit_2_1_html", :locale => @locale %></p>
-  <p><%= t ".legal_babble.credit_3_1_html", :locale => @locale %></p>
-  <p><%= t ".legal_babble.credit_4_html", :locale => @locale %></p>
-  <p><%= image_tag("attribution_example.png",
-                   :alt => t(".legal_babble.attribution_example.alt"),
-                   :border => 0,
-                   :title => t(".legal_babble.attribution_example.title")) %></p>
+    <h3><%= t ".legal_babble.credit_title_html" %></h3>
+    <p><%= t ".legal_babble.credit_1_html" %></p>
+    <p><%= t ".legal_babble.credit_2_1_html" %></p>
+    <p><%= t ".legal_babble.credit_3_1_html" %></p>
+    <p><%= t ".legal_babble.credit_4_html" %></p>
+    <p><%= image_tag("attribution_example.png",
+                     :alt => t(".legal_babble.attribution_example.alt"),
+                     :border => 0,
+                     :title => t(".legal_babble.attribution_example.title")) %></p>
 
-  <h3><%= t ".legal_babble.more_title_html", :locale => @locale %></h3>
-  <p><%= t ".legal_babble.more_1_html", :locale => @locale %></p>
-  <p><%= t ".legal_babble.more_2_html", :locale => @locale %></p>
+    <h3><%= t ".legal_babble.more_title_html" %></h3>
+    <p><%= t ".legal_babble.more_1_html" %></p>
+    <p><%= t ".legal_babble.more_2_html" %></p>
 
-  <h3><%= t ".legal_babble.contributors_title_html", :locale => @locale %></h3>
-  <p><%= t ".legal_babble.contributors_intro_html", :locale => @locale %></p>
-  <ul id="contributors">
-    <li><%= t ".legal_babble.contributors_at_html", :locale => @locale %></li>
-    <li><%= t ".legal_babble.contributors_au_html", :locale => @locale %></li>
-    <li><%= t ".legal_babble.contributors_ca_html", :locale => @locale %></li>
-    <li><%= t ".legal_babble.contributors_fi_html", :locale => @locale %></li>
-    <li><%= t ".legal_babble.contributors_fr_html", :locale => @locale %></li>
-    <li><%= t ".legal_babble.contributors_nl_html", :locale => @locale %></li>
-    <li><%= t ".legal_babble.contributors_nz_html", :locale => @locale %></li>
-    <li><%= t ".legal_babble.contributors_si_html", :locale => @locale %></li>
-    <li><%= t ".legal_babble.contributors_es_html", :locale => @locale %></li>
-    <li><%= t ".legal_babble.contributors_za_html", :locale => @locale %></li>
-    <li><%= t ".legal_babble.contributors_gb_html", :locale => @locale %></li>
-  </ul>
-  <p><%= t ".legal_babble.contributors_footer_1_html", :locale => @locale %></p>
-  <p><%= t ".legal_babble.contributors_footer_2_html", :locale => @locale %></p>
+    <h3><%= t ".legal_babble.contributors_title_html" %></h3>
+    <p><%= t ".legal_babble.contributors_intro_html" %></p>
+    <ul id="contributors">
+      <li><%= t ".legal_babble.contributors_at_html" %></li>
+      <li><%= t ".legal_babble.contributors_au_html" %></li>
+      <li><%= t ".legal_babble.contributors_ca_html" %></li>
+      <li><%= t ".legal_babble.contributors_fi_html" %></li>
+      <li><%= t ".legal_babble.contributors_fr_html" %></li>
+      <li><%= t ".legal_babble.contributors_nl_html" %></li>
+      <li><%= t ".legal_babble.contributors_nz_html" %></li>
+      <li><%= t ".legal_babble.contributors_si_html" %></li>
+      <li><%= t ".legal_babble.contributors_es_html" %></li>
+      <li><%= t ".legal_babble.contributors_za_html" %></li>
+      <li><%= t ".legal_babble.contributors_gb_html" %></li>
+    </ul>
+    <p><%= t ".legal_babble.contributors_footer_1_html" %></p>
+    <p><%= t ".legal_babble.contributors_footer_2_html" %></p>
 
-  <h3><%= t ".legal_babble.infringement_title_html", :locale => @locale %></h3>
-  <p><%= t ".legal_babble.infringement_1_html", :locale => @locale %></p>
-  <p><%= t ".legal_babble.infringement_2_html", :locale => @locale %></p>
+    <h3><%= t ".legal_babble.infringement_title_html" %></h3>
+    <p><%= t ".legal_babble.infringement_1_html" %></p>
+    <p><%= t ".legal_babble.infringement_2_html" %></p>
 
-  <h3><%= t ".legal_babble.trademarks_title_html", :locale => @locale %></h3>
-  <p><%= t ".legal_babble.trademarks_1_html", :locale => @locale %></p>
+    <h3><%= t ".legal_babble.trademarks_title_html" %></h3>
+    <p><%= t ".legal_babble.trademarks_1_html" %></p>
+  <% end %>
 <% end %>

From 3ec310b0c8e517d2c0c12281c575d278f35c6f64 Mon Sep 17 00:00:00 2001
From: Andy Allan <git@gravitystorm.co.uk>
Date: Wed, 15 Mar 2023 18:11:22 +0000
Subject: [PATCH 3/3] Fix indentation

---
 app/views/site/copyright.html.erb | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/app/views/site/copyright.html.erb b/app/views/site/copyright.html.erb
index 977398660..581af7d6d 100644
--- a/app/views/site/copyright.html.erb
+++ b/app/views/site/copyright.html.erb
@@ -1,7 +1,6 @@
 <% content_for :heading do %>
-<% if @locale == "en" %>
-  <!-- Maybe ease foreigners back to their native page -->
-
+  <% if @locale == "en" %>
+    <!-- Maybe ease foreigners back to their native page -->
     <% if t(".legal_babble", :locale => I18n.locale) != t(".legal_babble", :locale => :en) %>
       <%= tag.div :lang => @locale, :dir => t("html.dir", :locale => @locale) do %>
         <h1><%= t ".native.title" %></h1>
@@ -40,7 +39,6 @@
       <%= t ".legal_babble.title_html" %>
     <% end %>
   <% end %>
-
 <% end %>
 
 <% I18n.with_locale @locale do %>