Clean up settings/preferences controller (#2237)

* Add missing fields group on preferences page

* Clean up settings/preferences controller

* Extract a UserSettingsDecorator

app/controllers/settings/preferences_controller.rb

@@ -8,25 +8,9 @@ class Settings::PreferencesController < ApplicationController
   def show; end
 
   def update
-    current_user.settings['notification_emails'] = {
+    user_settings.update(user_settings_params.to_h)
-      follow:         user_params[:notification_emails][:follow]         == '1',
+
-      follow_request: user_params[:notification_emails][:follow_request] == '1',
+    if current_user.update(user_params)
-      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',
-    }
-
-    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))
       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

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

app/views/settings/preferences/show.html.haml

@@ -18,6 +18,7 @@
       = ff.input :mention, as: :boolean, wrapper: :with_label
       = ff.input :digest, 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

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
+  describe 'GET #show' do
-    it "returns http success" 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

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