From 2dda356e3fc7000ea630b802588cb91df296a89f Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 20 Apr 2017 21:26:52 -0400 Subject: [PATCH] Clean up settings/preferences controller (#2237) * Add missing fields group on preferences page * Clean up settings/preferences controller * Extract a UserSettingsDecorator --- .../settings/preferences_controller.rb | 38 ++++++------- app/lib/user_settings_decorator.rb | 56 +++++++++++++++++++ app/views/settings/preferences/show.html.haml | 7 ++- .../settings/preferences_controller_spec.rb | 39 +++++++++++-- spec/lib/user_settings_decorator_spec.rb | 45 +++++++++++++++ 5 files changed, 158 insertions(+), 27 deletions(-) create mode 100644 app/lib/user_settings_decorator.rb create mode 100644 spec/lib/user_settings_decorator_spec.rb diff --git a/app/controllers/settings/preferences_controller.rb b/app/controllers/settings/preferences_controller.rb index 31a2044209d..6762faee4cb 100644 --- a/app/controllers/settings/preferences_controller.rb +++ b/app/controllers/settings/preferences_controller.rb @@ -8,25 +8,9 @@ class Settings::PreferencesController < ApplicationController def show; end def update - current_user.settings['notification_emails'] = { - follow: user_params[:notification_emails][:follow] == '1', - follow_request: user_params[:notification_emails][:follow_request] == '1', - reblog: user_params[:notification_emails][:reblog] == '1', - favourite: user_params[:notification_emails][:favourite] == '1', - mention: user_params[:notification_emails][:mention] == '1', - digest: user_params[:notification_emails][:digest] == '1', - } + user_settings.update(user_settings_params.to_h) - current_user.settings['interactions'] = { - must_be_follower: user_params[:interactions][:must_be_follower] == '1', - must_be_following: user_params[:interactions][:must_be_following] == '1', - } - - current_user.settings['default_privacy'] = user_params[:setting_default_privacy] - current_user.settings['boost_modal'] = user_params[:setting_boost_modal] == '1' - current_user.settings['auto_play_gif'] = user_params[:setting_auto_play_gif] == '1' - - if current_user.update(user_params.except(:notification_emails, :interactions, :setting_default_privacy, :setting_boost_modal, :setting_auto_play_gif)) + if current_user.update(user_params) redirect_to settings_preferences_path, notice: I18n.t('generic.changes_saved_msg') else render :show @@ -35,7 +19,23 @@ class Settings::PreferencesController < ApplicationController private + def user_settings + UserSettingsDecorator.new(current_user) + end + def user_params - params.require(:user).permit(:locale, :setting_default_privacy, :setting_boost_modal, :setting_auto_play_gif, notification_emails: [:follow, :follow_request, :reblog, :favourite, :mention, :digest], interactions: [:must_be_follower, :must_be_following]) + params.require(:user).permit( + :locale + ) + end + + def user_settings_params + params.require(:user).permit( + :setting_default_privacy, + :setting_boost_modal, + :setting_auto_play_gif, + notification_emails: %i(follow follow_request reblog favourite mention digest), + interactions: %i(must_be_follower must_be_following) + ) end end diff --git a/app/lib/user_settings_decorator.rb b/app/lib/user_settings_decorator.rb new file mode 100644 index 00000000000..6104773addc --- /dev/null +++ b/app/lib/user_settings_decorator.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +class UserSettingsDecorator + attr_reader :user, :settings + + def initialize(user) + @user = user + end + + def update(settings) + @settings = settings + process_update + end + + private + + def process_update + user.settings['notification_emails'] = merged_notification_emails + user.settings['interactions'] = merged_interactions + user.settings['default_privacy'] = default_privacy_preference + user.settings['boost_modal'] = boost_modal_preference + user.settings['auto_play_gif'] = auto_play_gif_preference + end + + def merged_notification_emails + user.settings['notification_emails'].merge coerced_settings('notification_emails').to_h + end + + def merged_interactions + user.settings['interactions'].merge coerced_settings('interactions').to_h + end + + def default_privacy_preference + settings['setting_default_privacy'] + end + + def boost_modal_preference + boolean_cast_setting 'setting_boost_modal' + end + + def auto_play_gif_preference + boolean_cast_setting 'setting_auto_play_gif' + end + + def boolean_cast_setting(key) + settings[key] == '1' + end + + def coerced_settings(key) + coerce_values settings.fetch(key, {}) + end + + def coerce_values(params_hash) + params_hash.transform_values { |x| x == '1' } + end +end diff --git a/app/views/settings/preferences/show.html.haml b/app/views/settings/preferences/show.html.haml index ce3929629b2..d009e51ec57 100644 --- a/app/views/settings/preferences/show.html.haml +++ b/app/views/settings/preferences/show.html.haml @@ -18,9 +18,10 @@ = ff.input :mention, as: :boolean, wrapper: :with_label = ff.input :digest, as: :boolean, wrapper: :with_label - = f.simple_fields_for :interactions, hash_to_object(current_user.settings.interactions) do |ff| - = ff.input :must_be_follower, as: :boolean, wrapper: :with_label - = ff.input :must_be_following, as: :boolean, wrapper: :with_label + .fields-group + = f.simple_fields_for :interactions, hash_to_object(current_user.settings.interactions) do |ff| + = ff.input :must_be_follower, as: :boolean, wrapper: :with_label + = ff.input :must_be_following, as: :boolean, wrapper: :with_label .fields-group = f.input :setting_boost_modal, as: :boolean, wrapper: :with_label diff --git a/spec/controllers/settings/preferences_controller_spec.rb b/spec/controllers/settings/preferences_controller_spec.rb index a5d349d6df5..16cc87991c7 100644 --- a/spec/controllers/settings/preferences_controller_spec.rb +++ b/spec/controllers/settings/preferences_controller_spec.rb @@ -1,16 +1,45 @@ require 'rails_helper' -RSpec.describe Settings::PreferencesController, type: :controller do - +describe Settings::PreferencesController do + let(:user) { Fabricate(:user) } before do - sign_in Fabricate(:user), scope: :user + sign_in user, scope: :user end - describe "GET #show" do - it "returns http success" do + describe 'GET #show' do + it 'returns http success' do get :show + expect(response).to have_http_status(:success) end end + describe 'PUT #update' do + it 'udpates the user record' do + put :update, params: { user: { locale: 'en' } } + + expect(response).to redirect_to(settings_preferences_path) + expect(user.reload.locale).to eq 'en' + end + + it 'updates user settings' do + user.settings['boost_modal'] = false + user.settings['notification_emails']['follow'] = false + user.settings['interactions']['must_be_follower'] = true + + put :update, params: { + user: { + setting_boost_modal: '1', + notification_emails: { follow: '1' }, + interactions: { must_be_follower: '0' } + } + } + + expect(response).to redirect_to(settings_preferences_path) + user.reload + expect(user.settings['boost_modal']).to be true + expect(user.settings['notification_emails']['follow']).to be true + expect(user.settings['interactions']['must_be_follower']).to be false + end + end end diff --git a/spec/lib/user_settings_decorator_spec.rb b/spec/lib/user_settings_decorator_spec.rb new file mode 100644 index 00000000000..466c57fa51c --- /dev/null +++ b/spec/lib/user_settings_decorator_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe UserSettingsDecorator do + describe 'update' do + let(:user) { Fabricate(:user) } + let(:settings) { described_class.new(user) } + + it 'updates the user settings value for email notifications' do + values = { 'notification_emails' => { 'follow' => '1' } } + + settings.update(values) + expect(user.settings['notification_emails']['follow']).to eq true + end + + it 'updates the user settings value for interactions' do + values = { 'interactions' => { 'must_be_follower' => '0' } } + + settings.update(values) + expect(user.settings['interactions']['must_be_follower']).to eq false + end + + it 'updates the user settings value for privacy' do + values = { 'setting_default_privacy' => 'public' } + + settings.update(values) + expect(user.settings['default_privacy']).to eq 'public' + end + + it 'updates the user settings value for boost modal' do + values = { 'setting_boost_modal' => '1' } + + settings.update(values) + expect(user.settings['boost_modal']).to eq true + end + + it 'updates the user settings value for gif auto play' do + values = { 'setting_auto_play_gif' => '0' } + + settings.update(values) + expect(user.settings['auto_play_gif']).to eq false + end + end +end