Merge pull request #319 from glitch-soc/317-attempt-to-switch-flavors-raises-actioncontroller-parametermissing

Fix #317: Attempting to switch flavors raises ActionController::ParameterMissing

2 files changed, 43 insertions, 8 deletions

diff --git a/app/controllers/settings/flavours_controller.rb b/app/controllers/settings/flavours_controller.rb
index 865d5a479..634387715 100644
--- a/app/controllers/settings/flavours_controller.rb
+++ b/app/controllers/settings/flavours_controller.rb
@@ -1,13 +1,12 @@
 # frozen_string_literal: true
 
 class Settings::FlavoursController < Settings::BaseController
-
   def index
     redirect_to action: 'show', flavour: current_flavour
   end
 
   def show
-    unless Themes.instance.flavours.include?(params[:flavour]) or params[:flavour] == current_flavour
+    unless Themes.instance.flavours.include?(params[:flavour]) || (params[:flavour] == current_flavour)
       redirect_to action: 'show', flavour: current_flavour
     end
 
@@ -16,7 +15,7 @@ class Settings::FlavoursController < Settings::BaseController
   end
 
   def update
-    user_settings.update(user_settings_params(params[:flavour]).to_h)
+    user_settings.update(user_settings_params)
     redirect_to action: 'show', flavour: params[:flavour]
   end
 
@@ -26,10 +25,8 @@ class Settings::FlavoursController < Settings::BaseController
     UserSettingsDecorator.new(current_user)
   end
 
-  def user_settings_params(flavour)
-    params.require(:user).merge({ setting_flavour: flavour }).permit(
-      :setting_flavour,
-      :setting_skin
-    )
+  def user_settings_params
+    { setting_flavour: params.require(:flavour),
+      setting_skin: params.dig(:user, :setting_skin) }.with_indifferent_access
   end
 end
diff --git a/spec/controllers/settings/flavours_controller_spec.rb b/spec/controllers/settings/flavours_controller_spec.rb
new file mode 100644
index 000000000..f89bde1f9
--- /dev/null
+++ b/spec/controllers/settings/flavours_controller_spec.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+require 'rails_helper'
+
+RSpec.describe Settings::FlavoursController, type: :controller do
+  let(:user) { Fabricate(:user) }
+
+  before do
+    sign_in user, scope: :user
+  end
+
+  describe 'PUT #update' do
+    describe 'without a user[setting_skin] parameter' do
+      it 'sets the selected flavour' do
+        put :update, params: { flavour: 'schnozzberry' }
+
+        user.reload
+
+        expect(user.setting_flavour).to eq 'schnozzberry'
+      end
+    end
+
+    describe 'with a user[setting_skin] parameter' do
+      before do
+        put :update, params: { flavour: 'schnozzberry', user: { setting_skin: 'wallpaper' } }
+
+        user.reload
+      end
+
+      it 'sets the selected flavour' do
+        expect(user.setting_flavour).to eq 'schnozzberry'
+      end
+
+      it 'sets the selected skin' do
+        expect(user.setting_skin).to eq 'wallpaper'
+      end
+    end
+  end
+end