From 3ce1385b254c75ad026a84cb19d2ed49c10aa665 Mon Sep 17 00:00:00 2001 From: David Yip Date: Wed, 10 Jan 2018 12:08:57 -0600 Subject: [PATCH 1/4] Add examples based on errors seen in #317 --- .../settings/flavours_controller_spec.rb | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 spec/controllers/settings/flavours_controller_spec.rb diff --git a/spec/controllers/settings/flavours_controller_spec.rb b/spec/controllers/settings/flavours_controller_spec.rb new file mode 100644 index 000000000..51322c5bb --- /dev/null +++ b/spec/controllers/settings/flavours_controller_spec.rb @@ -0,0 +1,37 @@ +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 From 6fcb870d96e53a5d032c0639697b1387248339d6 Mon Sep 17 00:00:00 2001 From: David Yip Date: Tue, 9 Jan 2018 20:09:00 -0600 Subject: [PATCH 2/4] Allow for user object to be empty. Fixes #317. If a flavour has only one skin, the skin selector will be omitted. This omits the user[setting_skin] field, and because that's the only user[...] field on the page, the entire user object will not be present in the request handler's params object. This commit accounts for that scenario by avoiding params.require(:user) and instead picking out what we need from the params hash. --- app/controllers/settings/flavours_controller.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/controllers/settings/flavours_controller.rb b/app/controllers/settings/flavours_controller.rb index 865d5a479..a4bdcdf3e 100644 --- a/app/controllers/settings/flavours_controller.rb +++ b/app/controllers/settings/flavours_controller.rb @@ -16,7 +16,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(params[:flavour])) redirect_to action: 'show', flavour: params[:flavour] end @@ -27,9 +27,8 @@ class Settings::FlavoursController < Settings::BaseController end def user_settings_params(flavour) - params.require(:user).merge({ setting_flavour: flavour }).permit( - :setting_flavour, - :setting_skin - ) + { setting_flavour: params.require(:flavour), + setting_skin: params.dig(:user, :setting_skin) + }.with_indifferent_access end end From 514db316f798f3e9b65d1591ff88f7c82289ae38 Mon Sep 17 00:00:00 2001 From: David Yip Date: Tue, 9 Jan 2018 20:12:28 -0600 Subject: [PATCH 3/4] The flavour parameter is unused, so omit it (#317) --- app/controllers/settings/flavours_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/settings/flavours_controller.rb b/app/controllers/settings/flavours_controller.rb index a4bdcdf3e..a2d2c9dee 100644 --- a/app/controllers/settings/flavours_controller.rb +++ b/app/controllers/settings/flavours_controller.rb @@ -16,7 +16,7 @@ class Settings::FlavoursController < Settings::BaseController end def update - user_settings.update(user_settings_params(params[:flavour])) + user_settings.update(user_settings_params) redirect_to action: 'show', flavour: params[:flavour] end @@ -26,7 +26,7 @@ class Settings::FlavoursController < Settings::BaseController UserSettingsDecorator.new(current_user) end - def user_settings_params(flavour) + def user_settings_params { setting_flavour: params.require(:flavour), setting_skin: params.dig(:user, :setting_skin) }.with_indifferent_access From 395e64e8580e8232b817b6278ea28a7c523bd6f7 Mon Sep 17 00:00:00 2001 From: David Yip Date: Tue, 9 Jan 2018 20:16:31 -0600 Subject: [PATCH 4/4] Thank you, Officer Murphy --- app/controllers/settings/flavours_controller.rb | 6 ++---- spec/controllers/settings/flavours_controller_spec.rb | 1 + 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/controllers/settings/flavours_controller.rb b/app/controllers/settings/flavours_controller.rb index a2d2c9dee..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 @@ -28,7 +27,6 @@ class Settings::FlavoursController < Settings::BaseController def user_settings_params { setting_flavour: params.require(:flavour), - setting_skin: params.dig(:user, :setting_skin) - }.with_indifferent_access + 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 index 51322c5bb..f89bde1f9 100644 --- a/spec/controllers/settings/flavours_controller_spec.rb +++ b/spec/controllers/settings/flavours_controller_spec.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true require 'rails_helper' RSpec.describe Settings::FlavoursController, type: :controller do