Change format of the user languages_settings #2369 (#2496)

* Change format of user preferred languages

* Make sure user language settings are set by default

* Add missing expect to the user spec

* Fix users/misc template

* Move existing languages to a constant

* Remove commented user code

* Move languages out of User model

* Update preferred languages accordingly to the available languages

* Set user estimated language only if the language is available

* Fix Portuguese spelling #2582

app/controllers/users_controller.rb

@@ -11,6 +11,7 @@ class UsersController < ApplicationController
     @user = current_user
     @tab_list = @user.settings_tab_list
     @tab = params["tab"]
+
     authorize @user
     handle_settings_tab
   end
@@ -21,7 +22,6 @@ class UsersController < ApplicationController
     @tab_list = @user.settings_tab_list
     @tab = params["user"]["tab"] || "profile"
     authorize @user
-    # raise permitted_attributes(@user).to_s
     if @user.update(permitted_attributes(@user))
       RssReader.new.delay.fetch_user(@user) if @user.feed_url.present?
       Streams::TwitchWebhookRegistrationJob.perform_later(@user.id) if @user.twitch_username.present?
@@ -39,6 +39,21 @@ class UsersController < ApplicationController
     end
   end
 
+  def update_language_settings
+    @user = current_user
+    @tab_list = @user.settings_tab_list
+    @tab = "misc"
+    authorize @user
+    @user.language_settings["preferred_languages"] = Languages::LIST.keys & params[:user][:preferred_languages].to_a
+    if @user.save
+      notice = "Your profile was successfully updated."
+      @user.touch(:profile_updated_at)
+      redirect_to "/settings/#{@tab}", notice: notice
+    else
+      render :edit
+    end
+  end
+
   def destroy
     @user = current_user
     @tab_list = @user.settings_tab_list

app/decorators/user_decorator.rb

@@ -83,12 +83,4 @@ class UserDecorator < ApplicationDecorator
     ]
     colors[id % 10]
   end
-
-  def preferred_languages_array
-    languages = []
-    language_settings.keys.each do |setting|
-      languages << setting.split("prefer_language_")[1] if language_settings[setting] && setting.include?("prefer_language_")
-    end
-    languages
-  end
 end

app/models/languages.rb

@@ -0,0 +1,9 @@
+module Languages
+  LIST = { "en" => "English", "ja" => "Japanese", "es" => "Spanish", "fr" => "French", "it" => "Italian", "pt" => "Portuguese" }.freeze
+
+  module_function
+
+  def available?(code)
+    LIST.key? code
+  end
+end

app/models/user.rb

@@ -143,6 +143,7 @@ class User < ApplicationRecord
   after_save  :subscribe_to_mailchimp_newsletter
   after_save  :conditionally_resave_articles
   after_create :estimate_default_language!
+  before_create :set_default_language
   before_update :mentorship_status_update
   before_validation :set_username
   # make sure usernames are not empty, to be able to use the database unique index
@@ -183,15 +184,8 @@ class User < ApplicationRecord
     end
   end
 
-  # Via https://github.com/G5/storext
-  store_attributes :language_settings do
-    estimated_default_language String
-    prefer_language_en Boolean, default: true
-    prefer_language_ja Boolean, default: false
-    prefer_language_es Boolean, default: false
-    prefer_language_fr Boolean, default: false
-    prefer_language_it Boolean, default: false
-    prefer_language_pt Boolean, default: false
+  def estimated_default_language
+    language_settings["estimated_default_language"]
   end
 
   def self.trigger_delayed_index(record, remove)
@@ -258,16 +252,27 @@ class User < ApplicationRecord
 
   def cached_preferred_langs
     Rails.cache.fetch("user-#{id}-#{updated_at}/cached_preferred_langs", expires_in: 24.hours) do
-      langs = []
-      langs << "en" if prefer_language_en
-      langs << "ja" if prefer_language_ja
-      langs << "es" if prefer_language_es
-      langs << "fr" if prefer_language_fr
-      langs << "it" if prefer_language_it
-      langs
+      preferred_languages_array
     end
   end
 
+  # handles both old (prefer_language_*) and new (Array of language codes) formats
+  def preferred_languages_array
+    # return @prefer_languages_array if defined? @preferred_languages_array
+    return @preferred_languages_array if defined?(@preferred_languages_array)
+
+    if language_settings["preferred_languages"].present?
+      @preferred_languages_array = language_settings["preferred_languages"].to_a
+    else
+      languages = []
+      language_settings.keys.each do |setting|
+        languages << setting.split("prefer_language_")[1] if language_settings[setting] && setting.include?("prefer_language_")
+      end
+      @preferred_languages_array = languages
+    end
+    @preferred_languages_array
+  end
+
   def processed_website_url
     website_url.to_s.strip if website_url.present?
   end
@@ -411,6 +416,10 @@ class User < ApplicationRecord
 
   private
 
+  def set_default_language
+    language_settings["preferred_languages"] ||= ["en"]
+  end
+
   def send_welcome_notification
     Notification.send_welcome_notification(id)
   end

app/policies/user_policy.rb

@@ -11,6 +11,10 @@ class UserPolicy < ApplicationPolicy
     current_user?
   end
 
+  def update_language_settings?
+    current_user?
+  end
+
   def destroy?
     current_user?
   end
@@ -105,12 +109,6 @@ class UserPolicy < ApplicationPolicy
        permit_adjacent_sponsors
        password
        password_confirmation
-       prefer_language_en
-       prefer_language_es
-       prefer_language_fr
-       prefer_language_it
-       prefer_language_pt
-       prefer_language_ja
        profile_image
        seeking_mentorship
        stackoverflow_url

app/services/users/estimate_default_language.rb

@@ -9,18 +9,32 @@ module Users
     end
 
     def call
-      identity = user.identities.find_by(provider: "twitter")
-      if user.email.to_s.end_with?(".jp")
-        user.update(estimated_default_language: "ja", prefer_language_ja: true)
-      elsif identity
-        lang = identity.auth_data_dump["extra"]["raw_info"]["lang"]
-        user.update(:estimated_default_language => lang,
-                    "prefer_language_#{lang}" => true)
-      end
+      preferred_languages = ["en"]
+      preferred_languages |= [estimated_default_language] if estimated_default_language
+      user.update_columns(language_settings: { estimated_default_language: estimated_default_language,
+                                               preferred_languages: preferred_languages })
     end
 
     private
 
     attr_reader :user
+
+    def estimated_default_language
+      return @estimated_default_language if defined? @estimated_default_language
+
+      identity = user.identities.find_by(provider: "twitter")
+      @estimated_default_language = user.email.to_s.end_with?(".jp") ? "ja" : language_from_twitter(identity)
+    end
+
+    # available twitter languages
+    # https://developer.twitter.com/en/docs/developer-utilities/supported-languages/api-reference/get-help-languages
+    def language_from_twitter(identity)
+      return nil unless identity
+
+      twitter_lang = identity.auth_data_dump["extra"]["raw_info"]["lang"]
+      return "en" if twitter_lang == "en-gb"
+
+      Languages.available?(twitter_lang) ? twitter_lang : nil
+    end
   end
 end

app/views/users/_language_settings.html.erb

@@ -0,0 +1,24 @@
+<h2>Languages</h2>
+<h3>Select which languages you'd prefer to see in your feed <span style="color:#e05252">(beta)</span></h3>
+<h4 style="font-weight:400">
+  This setting controls which languages you are more likely to see throughout the site, but you may still see other languages, especially English.
+</h4>
+
+<%= form_tag users_update_language_settings_path do |f| %>
+  <div class="checkbox-field">
+    <% Languages::LIST.each do |code, name| %>
+      <div class="sub-field">
+        <%= check_box_tag "user[preferred_languages][]", code, @user.preferred_languages_array.include?(code) %>
+        <%= label_tag "user[preferred_languages][]", name %>
+      </div>
+    <% end %>
+  </div>
+  <div class="field">
+    <label></label>
+    <%= hidden_field_tag :tab, value: @tab %>
+    <%= submit_tag "SUBMIT", class: "cta" %>
+  </div>
+<% end %>
+<h3>
+  This feature is in beta and will improve over time. Another way to tailor your feed is to discover and follow members who write in your preferred language.
+</h3>

app/views/users/_misc.html.erb

@@ -13,7 +13,7 @@
 <%= form_for(@user) do |f| %>
   <div class="sub-field">
     <%= f.label :editor_version, "Editor version: v1 or v2" %>
-    <%= f.select :editor_version, options_for_select(["v1", "v2"], @user.editor_version) %>
+    <%= f.select :editor_version, options_for_select(%w[v1 v2], @user.editor_version) %>
     <sub><em>v2 is currently in beta</em></sub>
   </div>
   <div class="field">
@@ -53,49 +53,8 @@
     <%= f.submit "SUBMIT", class: "cta" %>
   </div>
 <% end %>
-<h2>Languages</h2>
-<h3>Select which languages you'd prefer to see in your feed <span style="color:#e05252">(beta)</span></h3>
-<h4 style="font-weight:400">
-  This setting controls which languages you are more likely to see throughout the site, but you may still see other languages, especially English.
-</h4>
 
-<%= form_for(@user) do |f| %>
-  <div class="checkbox-field">
-    <div class="sub-field">
-      <%= f.check_box :prefer_language_en %>
-      <%= f.label :prefer_language_en, "English" %>
-    </div>
-    <div class="sub-field">
-      <%= f.check_box :prefer_language_ja %>
-      <%= f.label :prefer_language_ja, "Japanese" %>
-    </div>
-    <div class="sub-field">
-      <%= f.check_box :prefer_language_es %>
-      <%= f.label :prefer_language_es, "Spanish" %>
-    </div>
-    <div class="sub-field">
-      <%= f.check_box :prefer_language_fr %>
-      <%= f.label :prefer_language_fr, "French" %>
-    </div>
-    <div class="sub-field">
-      <%= f.check_box :prefer_language_it %>
-      <%= f.label :prefer_language_it, "Italian" %>
-    </div>
-    <div class="sub-field">
-      <%= f.check_box :prefer_language_pt %>
-      <%= f.label :prefer_language_pt, "Portugese" %>
-    </div>
-  </div>
-  <div class="field">
-    <label></label>
-    <%= f.hidden_field :tab, value: @tab %>
-    <%= f.submit "SUBMIT", class: "cta" %>
-  </div>
-<% end %>
-
-<h3>
-  This feature is in beta and will improve over time. Another way to tailor your feed is to discover and follow members who write in your preferred language.
-</h3>
+<%= render "language_settings" %>
 
 <h2>Sponsors</h2>
 <h4 style="font-weight:400">
@@ -157,12 +116,11 @@
   <div class="field checkbox-label-first">
     <%= f.label :inbox_type, "Open your inbox to messages from people you don't follow or keep your inbox private to mutual follows." %>
     <div class="sub-field">
-      <%= f.select :inbox_type, [["Open", "open"], ["Private", "private"]] %>
+      <%= f.select :inbox_type, [%w[Open open], %w[Private private]] %>
     </div>
     <%= f.label :inbox_guidelines, "Open inbox guidelines/instructions (optional)" %>
     <div class="sub-field">
       <%= f.text_area :inbox_guidelines, maxlength: 200, placeholder: "For example:
-      
 - Only contact me about consulting opportunities.
 - I'm happy to help beginners with CSS.
 - Allow for a few days to respond.

config/routes.rb

@@ -198,6 +198,7 @@ Rails.application.routes.draw do
       to: redirect("anotherdevblog/every-developer-should-write-a-personal-automation-api")
 
   # Settings
+  post "users/update_language_settings" => "users#update_language_settings"
   post "users/join_org" => "users#join_org"
   post "users/leave_org" => "users#leave_org"
   post "users/add_org_admin" => "users#add_org_admin"

spec/models/user_spec.rb

@@ -1,7 +1,7 @@
 require "rails_helper"
 
 RSpec.describe User, type: :model do
-  let(:user)            { create(:user) }
+  let!(:user)           { create(:user) }
   let(:returning_user)  { create(:user, signup_cta_variant: nil) }
   let(:second_user)     { create(:user) }
   let(:article)         { create(:article, user_id: user.id) }
@@ -392,6 +392,18 @@ RSpec.describe User, type: :model do
     end
 
     context "when estimating the default language" do
+      it "sets correct language_settings by default" do
+        user2 = create(:user, email: nil)
+        expect(user2.language_settings).to eq("preferred_languages" => %w[en])
+      end
+
+      it "sets correct language_settings by default after the callbacks" do
+        perform_enqueued_jobs do
+          user2 = create(:user, email: nil)
+          expect(user2.language_settings).to eq("preferred_languages" => %w[en])
+        end
+      end
+
       it "estimates default language to be nil" do
         perform_enqueued_jobs do
           user.estimate_default_language!
@@ -411,6 +423,7 @@ RSpec.describe User, type: :model do
         perform_enqueued_jobs do
           new_user = user_from_authorization_service(:twitter, nil, "navbar_basic")
           new_user.estimate_default_language!
+          expect(user.reload.estimated_default_language).to eq(nil)
         end
       end
 
@@ -419,11 +432,23 @@ RSpec.describe User, type: :model do
           user.update_column(:email, "ben@hello.jp")
           user.estimate_default_language!
         end
-        expect(user.reload.decorate.preferred_languages_array).to include("ja")
+        expect(user.reload.preferred_languages_array).to include("ja")
       end
     end
   end
 
+  describe "#preferred_languages_array" do
+    it "returns a correct array when language settings are in a new format" do
+      user.update_columns(language_settings: { estimated_default_language: "en", preferred_languages: %w[en ru it] })
+      expect(user.preferred_languages_array).to eq(%w[en ru it])
+    end
+
+    it "returns a correct array when language settings are in the old format" do
+      user.update_columns(language_settings: { estimated_default_language: "en", prefer_language_en: true, prefer_language_ja: false, prefer_language_es: true })
+      expect(user.preferred_languages_array).to eq(%w[en es])
+    end
+  end
+
   it "follows users" do
     user2 = create(:user)
     user3 = create(:user)

spec/requests/user_settings_spec.rb

@@ -112,6 +112,30 @@ RSpec.describe "UserSettings", type: :request do
     end
   end
 
+  describe "POST /users/update_language_settings" do
+    before { login_as user }
+
+    it "updates language settings" do
+      post "/users/update_language_settings", params: { user: { preferred_languages: %w[ja es] } }
+      user.reload
+      expect(user.language_settings["preferred_languages"]).to eq(%w[ja es])
+    end
+
+    it "keeps the estimated_default_language" do
+      user.update_column(:language_settings, estimated_default_language: "ru", preferred_languages: %w[en es])
+      post "/users/update_language_settings", params: { user: { preferred_languages: %w[it en] } }
+      user.reload
+      expect(user.language_settings["estimated_default_language"]).to eq("ru")
+    end
+
+    it "doesn't set non-existent languages" do
+      user.update_column(:language_settings, estimated_default_language: "ru", preferred_languages: %w[en es])
+      post "/users/update_language_settings", params: { user: { preferred_languages: %w[it en blah] } }
+      user.reload
+      expect(user.language_settings["preferred_languages"].sort).to eq(%w[en it])
+    end
+  end
+
   describe "DELETE /users/remove_association" do
     context "when user has two identities" do
       let(:user) { create(:user, :two_identities) }

spec/services/users/estimate_default_language_spec.rb

@@ -12,6 +12,13 @@ RSpec.describe Users::EstimateDefaultLanguage, type: :service do
     user = create(:user)
     described_class.call(user)
     expect(user.estimated_default_language).to eq(nil)
+    expect(user.decorate.preferred_languages_array).to eq(%w[en])
+  end
+
+  it "estimates sets preferred languages to [en] when no lang data" do
+    user = create(:user)
+    described_class.call(user)
+    expect(user.decorate.preferred_languages_array).to eq(%w[en])
   end
 
   it "estimates default language to be japan with jp email" do
@@ -34,4 +41,39 @@ RSpec.describe Users::EstimateDefaultLanguage, type: :service do
     user.reload
     expect(user.decorate.preferred_languages_array).to include("ja")
   end
+
+  it "sets correct language_settings for jp" do
+    user = create(:user, email: "annabu@example.jp")
+    described_class.call(user)
+    user.reload
+    expect(user.language_settings).to eq("preferred_languages" => %w[en ja], "estimated_default_language" => "ja")
+  end
+
+  it "sets correct language_settings for pt" do
+    user = create(:user)
+    create(:identity, provider: :twitter, user: user, auth_data_dump: { "extra" => { "raw_info" => { "lang" => "pt" } } })
+    described_class.call(user)
+    user.reload
+    expect(user.language_settings).to eq("preferred_languages" => %w[en pt], "estimated_default_language" => "pt")
+  end
+
+  it "sets correct language_settings for no lang" do
+    user = create(:user, email: nil)
+    described_class.call(user)
+    expect(user.language_settings).to eq("preferred_languages" => %w[en], "estimated_default_language" => nil)
+  end
+
+  it "doesn't set incorrect language settings" do
+    user = create(:user)
+    create(:identity, provider: :twitter, user: user, auth_data_dump: { "extra" => { "raw_info" => { "lang" => "supermario" } } })
+    described_class.call(user)
+    expect(user.language_settings).to eq("preferred_languages" => %w[en], "estimated_default_language" => nil)
+  end
+
+  it "sets language settings when language is in twitter format (en-gb)" do
+    user = create(:user)
+    create(:identity, provider: :twitter, user: user, auth_data_dump: { "extra" => { "raw_info" => { "lang" => "en-gb" } } })
+    described_class.call(user)
+    expect(user.language_settings).to eq("preferred_languages" => %w[en], "estimated_default_language" => "en")
+  end
 end