Page MenuHomePhabricator

email notifications aren't properly localized
Closed, ResolvedPublic

Description

Currently all emails are going out with the locale of en-us.

While there is a language preference control in the user account area, that preference only persists for the duration of a user's web session. Apparently, this has always been the way in which this preference control operated. The pre-SUL version of the site leveraged a user's home wiki as a proxy for their locale, as the locale for a user was mapped to their home wiki. This would have been correct often, but not always. When we moved to SUL, we also moved to setting the default for a session to the locale requested by a user's browser, because that's how the web should work.

But: emails are not typically fired off from within the web session of the recipient. Because of that, emails are getting generated with the system locale, which is en-us. We need to persistently store each users' preferred language, and apply that preference to emails sent to them.

We should probably have two states:

  1. the user hasn't actually used the preference control. In this case, we should default the setting to their browser's locale. The ui should make it clear that this is an overridable default. In this case emails will go out using whatever locale the user had set when they last visited.
  2. the user went out of their way to make a choice. In this case we should always respect that choice. This should always override the detected locale, but it should allow the user to clear out the selected value and return to auto-defaulting.

Event Timeline

This issue should be resolved, though the implementation looks a little different than what I had originally envisioned. I made the following changes:

  • added a user field for language preference
  • added a hook on update_from_wikipedia to set that field to the current browser language if it's empty
  • re-implemented the django i18n form to also set that field for a user if they are logged in
  • added the value of that field to the context loaded into all of the email templates

I made some discoveries while working on this:

  • I found a bug in the way django migrations were getting applied by the puppet module. I mostly worked around it, but there are some lingering questions that I need to write up
  • I found that we are really not testing emails at all right now. I hand tested various scenarios (eg, what happens when we email someone with no langauge preference) to verify that I'm not doing anything too silly. Django has some decent email testing tools that we should probably be using. I'll need to write that up somewhere.