The fix might be something as simple as enclosing the affected code block in a with_locale wrapper (it seems to be fine with being passed nil):

Example from: RequestMailer#new_response

  # Tell the requester that a new response has arrived
  def new_response(info_request, incoming_message)
    @incoming_message, @info_request = incoming_message, info_request

    set_reply_to_headers(info_request.user)
    set_auto_generated_headers

+    AlaveteliLocalization.with_locale(info_request.user.locale) do
      mail(
        :from => contact_for_user(info_request.user),
        :to => info_request.user.name_and_email,
        :subject => _("New response to your FOI request - {{request_title}}",
                      :request_title => info_request.title.html_safe),
        :charset => "UTF-8"
      )
+    end
  end

(Would need tests to show what happens with the default language used, the user having something other than the default set (off the top of my head, 'es' should be available in the test suite) - including nil and an invalid/unavailable choice - and for code and tests to be repeated for all the cases where this could happen.)

We also have User#locale that we could use – though I don't know how it gets populated.

Effectively that is usingUser#locale (I don't think we store a locale for the request itself).

It's automatically populated via ApplicationController#set_gettext_locale so it's (probably) the last language you viewed the site in while logged in rather than a deliberate user preference along the lines of "please send me emails in Swedish where possible"

Oh yes, durr!

Yeah, it looks like what you suggest sounds like how we'd want to approach fixing this – send the email in the locale the user last used, which we're storing anyway.

Presumed without testing to also affect (but may not be limited to):

• RequestMailer#new_response_reminder_alert
• RequestMailer#comment_on_alert_plural
• RequestMailer#comment_on_alert
• RequestMailer#not_clarified_alert
• RequestMailer#old_unclassified_updated
• RequestMailer#very_overdue_alert
• RequestMailer#overdue_alert

(Although we could probably call the request alerts feature complete by adding locale awareness to all the above, and dealing with notification mailers etc separately)

Since 2019, the method mail_user has been added to ApplicationMailer. I think the best solution is to let mail_user handle the localisation. To enable this, I suggest that the mail options, including subject, that are now args to mail_user are moved to a block instead. Like so:

-  def mail_user(user, subject:, **opts)                                        
+  def mail_user(user, &block)                                                  
     if user.is_a?(User)                                                        
+      opts = {}                                                                
+      AlaveteliLocalization.with_locale(user.locale) do                        
+        opts = yield                                                           
+      end                                                                      
       opts[:to] = user.name_and_email                                          
     else                                                                       
+      opts = yield                                                             
       opts[:to] = user                                                         
     end                                                                        
                                                                                
     if opts[:from].is_a?(User)                                                 
       set_reply_to_headers('Reply-To' => opts[:from].name_and_email)           
       opts[:from] = MailHandler.address_from_name_and_email(                   
         opts[:from].name, blackhole_email                                      
       )                                                                        
                                                                                
     else                                                                       
       set_reply_to_headers                                                     
       opts[:from] ||= blackhole_email                                          
     end                                                                        
                                                                                
     set_auto_generated_headers                                                 
                                                                                
-    default_opts = {                                                           
-      subject: subject                                                         
-    }                                                                          
-    default_opts.merge!(opts)                                                  
-    mail(default_opts)                                                         
+    mail(opts)                                                                 
   end

Then all calls to mail_user would have to be rewritten, the suggested example from 2019 would e.g. be

  # Tell the requester that a new response has arrived
  def new_response(info_request, incoming_message)
    @incoming_message = incoming_message
    @info_request = info_request                                                
                                                                                
    mail_user(info_request.user) do {                                       
      subject: _("New response to your FOI request - {{request_title}}",        
                 request_title: info_request.title.html_safe),                  
      charset: "UTF-8",                                                         
    } end                                                                       
  end

There are about 25 uses of mail_user in the code. All of these will have to be rewritten, and tests (it_behaves_like?) written. I have a working experiment, so this is all very possible but rather tedious. So I am awaiting approval.

Would a PR along these lines be accepted?

@gbp What do you think about the suggestion from @andersjl ?

@andersjl, @mattiasaxell ah yes I see the issues here.

Moving the subject attribute into a block so it can be parsed as the locale set by the user seems like it would work.

Alternatively I think we could pass an optional proc for the subject, like: mail_user(subject: -> { _('My localised subject') }).

And in mail_user we can check for this, like:

diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb
index c8cd1469f..2261efe00 100644
--- a/app/mailers/application_mailer.rb
+++ b/app/mailers/application_mailer.rb
@@ -45,7 +45,7 @@ def mail_user(user, subject:, **opts)
     set_auto_generated_headers
 
     default_opts = {
-      subject: subject
+      subject: subject.respond_to?(:call) ? subject.call : subject
     }
     default_opts.merge!(opts)
     mail(default_opts)

If it's just the subject attribute which needs to be localised I would prefer using a proc this rather than a block.

@gbp I see your point. More obvious to the reader.

A drawback would possibly be that a future contributor may by mistake localise the subject before calling mail_user. So while I agree that a proc is better than a block in this case, I suggest making it required rather than optional. With a doc comment on mail_user explaining why. I can see no reason ever to localise the subject with the default locale.

I will check the mail_user invocations and see if localising the subject is enough for all of them (I expect so).

@andersjl yep good point, happy to require the subject attribute to always be a proc.

This is desirable, but unlikely to be worked on in the next 12 months so closing for now.

I have an almost finished PR solving this. Actually, if it were not for a severe migrain this morning I would have sent it today.

@andersjl sorry, this wasn't meant to be closed. We would welcome the contribution.