mainio / decidim-module-term_customizer

Decidim module that allows customizing the localization terms in the system for specific contexts.
GNU Affero General Public License v3.0
16 stars 21 forks source link

Unable to translate devise mails #93

Open Kagemaru opened 2 years ago

Kagemaru commented 2 years ago

Steps to reproduce

  1. Generate development_app
  2. Update config/locales/en.yml with:
    devise:
     mailer:
       confirmation_instructions:
         subject: "Base Subject"
  3. Create Translation with: Translation key: devise.mailer.confirmation_instructions.subject Customized term: Overriden Subject
  4. Register a new user and get the confirmation mail

Expected Result Get Overriden Subject as subject

Actual Result Get Base Subject as subject

paulinebessoles commented 1 year ago

Hi @Kagemaru I also spotted this on initiatives emails, for example with the following keys:

I have no idea where it comes from but I'm gonna investigate, do you have any infos? I see that you had some conversations about this on other repositories by my German is quite bad 😄

Kagemaru commented 1 year ago

Hi @paulinebessoles

We've hacked around this to fit our needs for now, but it's a big pain point. We've overridden the mail templates to enable tenant specific translations in our locale files.

The discussions in German were mostly just explaining what the Problem is and how we work around it. We didn't find out what the underlying Problem seems to be.

@carlobeltrame and I would love to help if we can.

Quentinchampenois commented 1 year ago

Hello,

I am currently investigating this issue, I tried to load the term customizer just before the eager_load! in the engine and it allows to overrides Devise I18n translations

Replaced : https://github.com/mainio/decidim-module-term_customizer/blob/abbf0c69e1bcaafebc5aa4f8da22fdb64ce62149/lib/decidim/term_customizer/engine.rb#L8 by initializer "decidim_term_customizer.setup", :before => :eager_load! do

But it is still inconsistent, I noticed Decidim::TermCustomizer::I18nBackend does not find translations because resolver is called without organization, just an hypothesis for now

ahukkanen commented 1 year ago

@Quentinchampenois There are a couple of potential problems that we are aware currently:

  1. The dynamic loading of the translations does not always apply to sending the email. There is a hook to be run before any jobs or any controllers but it may not apply to all mailers.
  2. Devise does not necessarily carry the context normally available in Decidim, particularly current_organization. If the context is not available, Term Customizer won't find any translations because the translations added through the admin panel are always attached to that particular organization.
  3. There may be also additional problems with this when running under multithreaded environments, typically production environments. I would also suggest testing and debugging this under the actual production configuration (e.g. Sidekiq/Resque as job runner, multiple threads, etc.).

Just to let you know if you're putting time into this, this is what we have figured out. There is no solution in any of these points but it may help further debugging.

I am unsure if changing the initialization order would fix the whole problem, as it is not the only problem related to this issue as a whole.

Quentinchampenois commented 1 year ago

I took a look on this issue, there is no issue with initialization order. Also, hook perform_start.active_job is called when sending devise mails.

What I think is that Devise mails have a different format than expected by the Decidim::TermCustomizer::Context::JobContext. For example, here is the data[:job] for a Decidim::FindAndUpdateDescendantsJob and a ActionMailer::MailDeliveryJob

To resume: JobContext does not find organization for MailDeliveryJob because it is defined in the Hash args

I am looking for a fix in the resolve! method to work with the MailDeliveryJob arguments, I didn't find an elegant way for now

larsUE commented 1 month ago

@Quentinchampenois we just stumbled on this issue again. :) Were you able to find a fix?

Quentinchampenois commented 1 week ago

Hello @larsUE,

I forgot my draft PR #109, I actually use this fix in production and it seems to work ! I will finish this PR once I find the time,

Sorry for the answer delay