[openstreetmap/openstreetmap-website] Change language immediately after updating settings (#1883)

Tom Hughes notifications at github.com
Sun May 27 10:36:58 UTC 2018


tomhughes requested changes on this pull request.



> @@ -126,6 +126,7 @@ def account
          (params[:user][:auth_provider] == current_user.auth_provider &&
           params[:user][:auth_uid] == current_user.auth_uid)
         update_user(current_user, params)
+        @title = t "user.account.title"

Would it not be more sensible just to move setting of the title to later in the method rather than duplicating it here?

> @@ -712,6 +713,7 @@ def update_user(user, params)
     end
 
     if user.save
+      @preferred_languages = current_user.preferred_languages

What is this doing? Does anything actually use that member of the controller?

Was it an early attempt to reset the cache on the model that has since been replaced with the change in the model?

> @@ -189,7 +189,7 @@ def preferred_language
   end
 
   def preferred_languages
-    @preferred_languages ||= Locale.list(languages)
+    Locale.list(languages)

Rather than just getting rid of the memoization maybe we should use an `after_save` filter to set it to `nil` so that it will be recomputed on next read?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/1883#pullrequestreview-123579345
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20180527/7ae0d27c/attachment.html>


More information about the rails-dev mailing list