From 369d6e2206f082b4c701fa58b70571f4b42ff2c8 Mon Sep 17 00:00:00 2001 From: rhymes Date: Wed, 25 Nov 2020 16:47:38 +0100 Subject: [PATCH] Fix Settings/Customization page (#11615) --- app/controllers/users_controller.rb | 23 ++++--------- app/policies/user_policy.rb | 4 --- app/views/users/_customization.html.erb | 38 ++++++++------------- app/views/users/_language_settings.html.erb | 21 ++++++------ config/routes.rb | 1 - spec/requests/user/user_settings_spec.rb | 14 +++++--- 6 files changed, 42 insertions(+), 59 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index eab2eaa95..cb782fadd 100644 --- a/app/controllers/users_controller.rb +++ b/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") diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 0d5bc9547..8da5f823a 100644 --- a/app/policies/user_policy.rb +++ b/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 diff --git a/app/views/users/_customization.html.erb b/app/views/users/_customization.html.erb index 8150713a0..5a9df7c4a 100644 --- a/app/views/users/_customization.html.erb +++ b/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 %> +

Appearance @@ -63,7 +65,7 @@

- <%= render partial: "language_settings" %> + <%= render partial: "language_settings", locals: { f: f } %>
@@ -78,20 +80,15 @@

- <%= form_for(@user, html: { id: nil, class: "grid gap-4" }) do |f| %> -
- <%= f.check_box :display_sponsors, class: "crayons-checkbox" %> - <%= f.label :display_sponsors, "Display Sponsors (When browsing)", class: "crayons-field__label" %> -
+
+ <%= f.check_box :display_sponsors, class: "crayons-checkbox" %> + <%= f.label :display_sponsors, "Display Sponsors (When browsing)", class: "crayons-field__label" %> +
-
- <%= f.check_box :permit_adjacent_sponsors, class: "crayons-checkbox" %> - <%= f.label :permit_adjacent_sponsors, "Permit Nearby Sponsors (When publishing)", class: "crayons-field__label" %> -
- - <%= f.hidden_field :tab, value: @tab, id: nil %> -
- <% end %> +
+ <%= f.check_box :permit_adjacent_sponsors, class: "crayons-checkbox" %> + <%= f.label :permit_adjacent_sponsors, "Permit Nearby Sponsors (When publishing)", class: "crayons-field__label" %> +
@@ -105,20 +102,13 @@

- <%= form_for(@user, html: { id: nil, class: "grid gap-4" }) do |f| %>
- <%= f.check_box :display_announcements, class: "crayons-checkbox" %> - <%= f.label :display_announcements, "Display Announcements (When browsing)", class: "crayons-field__label" %> -
- - <%= f.hidden_field :tab, value: @tab, id: nil %> -
- <% end %> + <%= f.check_box :display_announcements, class: "crayons-checkbox" %> + <%= f.label :display_announcements, "Display Announcements (When browsing)", class: "crayons-field__label" %> +
- <%= f.hidden_field :tab, value: @tab, id: nil %>
- <% end %> diff --git a/app/views/users/_language_settings.html.erb b/app/views/users/_language_settings.html.erb index 688c44e63..c7f32a45c 100644 --- a/app/views/users/_language_settings.html.erb +++ b/app/views/users/_language_settings.html.erb @@ -9,17 +9,18 @@

Select which languages you'd prefer to see in your feed.

-

This setting controls which languages you are more likely to see throughout the site, but you may still see other languages, especially English.

+

+ This setting controls which languages you are more likely to see throughout the site, + but you may still see other languages, especially English. +

- <%= form_tag users_update_language_settings_path, class: "grid gap-4" do |f| %> - <% Languages::LIST.each do |code, name| %> - - <% end %> - <%= hidden_field_tag :tab, value: @tab %> -
+ <% Languages::LIST.each do |code, name| %> + <% end %>
diff --git a/config/routes.rb b/config/routes.rb index 1a1fe866a..afd320595 100644 --- a/config/routes.rb +++ b/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" diff --git a/spec/requests/user/user_settings_spec.rb b/spec/requests/user/user_settings_spec.rb index bab880f5b..6ce36cf7c 100644 --- a/spec/requests/user/user_settings_spec.rb +++ b/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