Fix Settings/Customization page (#11615)

app/controllers/users_controller.rb

@@ -1,9 +1,7 @@
 class UsersController < ApplicationController
   before_action :set_no_cache_header
   before_action :raise_suspended, only: %i[update]
-  before_action :set_user, only: %i[
-    update update_language_settings confirm_destroy request_destroy full_delete remove_identity
-  ]
+  before_action :set_user, only: %i[update confirm_destroy request_destroy full_delete remove_identity]
   after_action :verify_authorized, except: %i[index signout_confirm add_org_admin remove_org_admin remove_from_org]
   before_action :authenticate_user!, only: %i[onboarding_update onboarding_checkbox_update]
   before_action :set_suggested_users, only: %i[index]
@@ -39,7 +37,12 @@ class UsersController < ApplicationController
   def update
     set_current_tab(params["user"]["tab"])
 
-    if @user.update(permitted_attributes(@user))
+    # preferred_languages is handled manually
+    @user.language_settings["preferred_languages"] = Languages::LIST.keys & params[:user][:preferred_languages].to_a
+
+    @user.attributes = permitted_attributes(@user)
+
+    if @user.save
       # NOTE: [@rhymes] this queues a job to fetch the feed each time the profile is updated, regardless if the user
       # explicitly requested "Feed fetch now" or simply updated any other field
       import_articles_from_feed(@user)
@@ -69,18 +72,6 @@ class UsersController < ApplicationController
     end
   end
 
-  def update_language_settings
-    set_current_tab("misc")
-    @user.language_settings["preferred_languages"] = Languages::LIST.keys & params[:user][:preferred_languages].to_a
-    if @user.save
-      flash[:settings_notice] = "Your language settings were successfully updated."
-      @user.touch(:profile_updated_at)
-      redirect_to "/settings/#{@tab}"
-    else
-      render :edit
-    end
-  end
-
   def request_destroy
     set_current_tab("account")
 

app/policies/user_policy.rb

@@ -79,10 +79,6 @@ class UserPolicy < ApplicationPolicy
     current_user?
   end
 
-  def update_language_settings?
-    current_user?
-  end
-
   def destroy?
     current_user?
   end

app/views/users/_customization.html.erb

@@ -1,6 +1,8 @@
 <%= javascript_packs_with_chunks_tag "stickySaveFooter", defer: true %>
 
 <%= form_for @user, html: { id: "ux-customization-form", class: "sticky-footer-form" } do |f| %>
+  <%= f.hidden_field :tab, value: @tab, id: nil %>
+
   <div class="crayons-card crayons-card--content-rows">
     <h2 class="crayons-subtitle-1">
       Appearance
@@ -63,7 +65,7 @@
     </div>
   </div>
 
-  <%= render partial: "language_settings" %>
+  <%= render partial: "language_settings", locals: { f: f } %>
 
   <div class="crayons-card crayons-card--content-rows">
     <header>
@@ -78,20 +80,15 @@
       </p>
     </header>
 
-    <%= form_for(@user, html: { id: nil, class: "grid gap-4" }) do |f| %>
-      <div class="crayons-field crayons-field--checkbox">
-        <%= f.check_box :display_sponsors, class: "crayons-checkbox" %>
-        <%= f.label :display_sponsors, "Display Sponsors (When browsing)", class: "crayons-field__label" %>
-      </div>
+    <div class="crayons-field crayons-field--checkbox">
+      <%= f.check_box :display_sponsors, class: "crayons-checkbox" %>
+      <%= f.label :display_sponsors, "Display Sponsors (When browsing)", class: "crayons-field__label" %>
+    </div>
 
-      <div class="crayons-field crayons-field--checkbox">
-        <%= f.check_box :permit_adjacent_sponsors, class: "crayons-checkbox" %>
-        <%= f.label :permit_adjacent_sponsors, "Permit Nearby Sponsors (When publishing)", class: "crayons-field__label" %>
-      </div>
-
-      <%= f.hidden_field :tab, value: @tab, id: nil %>
-      <div><button type="submit" class="crayons-btn">Save Sponsors Settings</button></div>
-    <% end %>
+    <div class="crayons-field crayons-field--checkbox">
+      <%= f.check_box :permit_adjacent_sponsors, class: "crayons-checkbox" %>
+      <%= f.label :permit_adjacent_sponsors, "Permit Nearby Sponsors (When publishing)", class: "crayons-field__label" %>
+    </div>
   </div>
 
   <div class="crayons-card crayons-card--content-rows">
@@ -105,20 +102,13 @@
       </p>
     </header>
 
-    <%= form_for(@user, html: { id: nil, class: "grid gap-4" }) do |f| %>
     <div class="crayons-field crayons-field--checkbox">
-        <%= f.check_box :display_announcements, class: "crayons-checkbox" %>
-        <%= f.label :display_announcements, "Display Announcements (When browsing)", class: "crayons-field__label" %>
-      </div>
-
-      <%= f.hidden_field :tab, value: @tab, id: nil %>
-      <div><button type="submit" class="crayons-btn">Save Announcements Settings</button></div>
-    <% end %>
+      <%= f.check_box :display_announcements, class: "crayons-checkbox" %>
+      <%= f.label :display_announcements, "Display Announcements (When browsing)", class: "crayons-field__label" %>
+    </div>
   </div>
 
   <div class="save-footer crayons-card crayons-card--content-rows">
-    <%= f.hidden_field :tab, value: @tab, id: nil %>
     <button class="crayons-btn" type="submit">Save</button>
   </div>
-
 <% end %>

app/views/users/_language_settings.html.erb

@@ -9,17 +9,18 @@
       <div>
     </h2>
     <p>Select which languages you'd prefer to see in your feed.</p>
-    <p>This setting controls which languages you are more likely to see throughout the site, but you may still see other languages, especially English.</p>
+    <p>
+      This setting controls which languages you are more likely to see throughout the site,
+      but you may still see other languages, especially English.
+    </p>
   </header>
 
-  <%= form_tag users_update_language_settings_path, class: "grid gap-4"  do |f| %>
-    <% Languages::LIST.each do |code, name| %>
-      <label class="crayons-field crayons-field--checkbox">
-        <%= check_box_tag "user[preferred_languages][]", code, @user.preferred_languages_array&.include?(code), class: "crayons-checkbox" %>
-        <div class="crayons-field__label"><%= name %></div>
-      </label>
-    <% end %>
-    <%= hidden_field_tag :tab, value: @tab %>
-      <div class="mt-6"><button type="submit" class="crayons-btn">Save Language Settings</button></div>
+  <% Languages::LIST.each do |code, name| %>
+    <label class="crayons-field crayons-field--checkbox">
+      <%= check_box_tag "user[preferred_languages][]",
+                        code, @user.preferred_languages_array&.include?(code),
+                        class: "crayons-checkbox" %>
+      <div class="crayons-field__label"><%= name %></div>
+    </label>
   <% end %>
 </div>

config/routes.rb

@@ -367,7 +367,6 @@ Rails.application.routes.draw do
     get "/async_info/shell_version", controller: "async_info#shell_version", defaults: { format: :json }
 
     # Settings
-    post "users/update_language_settings" => "users#update_language_settings"
     post "users/join_org" => "users#join_org"
     post "users/leave_org/:organization_id" => "users#leave_org", :as => :users_leave_org
     post "users/add_org_admin" => "users#add_org_admin"

spec/requests/user/user_settings_spec.rb

@@ -331,25 +331,31 @@ RSpec.describe "UserSettings", type: :request do
     end
   end
 
-  describe "POST /users/update_language_settings" do
+  describe "update language settings" do
     before { sign_in user }
 
     it "updates language settings" do
-      post "/users/update_language_settings", params: { user: { preferred_languages: %w[ja es] } }
+      put user_path(user), 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] } }
+
+      put user_path(user), 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] } }
+
+      put user_path(user), params: { user: { preferred_languages: %w[it en blah] } }
+
       user.reload
       expect(user.language_settings["preferred_languages"].sort).to eq(%w[en it])
     end