From 9cebaa53e79c854ab9acc502de2363fa89073468 Mon Sep 17 00:00:00 2001 From: Vaidehi Joshi Date: Wed, 1 Apr 2020 16:56:10 -0700 Subject: [PATCH] Avoid clobbering user attributes with empty params during onboarding (#7016) [deploy] --- app/controllers/users_controller.rb | 5 +++ spec/requests/users_onboarding_spec.rb | 55 +++++++++++++++++++------- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0a0a58a7b..0934caed5 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -143,6 +143,7 @@ class UsersController < ApplicationController def onboarding_update if params[:user] + sanitize_user_params permitted_params = %i[summary location employment_title employer_name last_onboarding_page] current_user.assign_attributes(params[:user].permit(permitted_params)) current_user.profile_updated_at = Time.current @@ -251,6 +252,10 @@ class UsersController < ApplicationController private + def sanitize_user_params + params[:user].delete_if { |_k, v| v.blank? } + end + def render_update_response if current_user.save respond_to do |format| diff --git a/spec/requests/users_onboarding_spec.rb b/spec/requests/users_onboarding_spec.rb index 238f10835..fddcaf6d3 100644 --- a/spec/requests/users_onboarding_spec.rb +++ b/spec/requests/users_onboarding_spec.rb @@ -1,31 +1,56 @@ require "rails_helper" RSpec.describe "UsersOnboarding", type: :request do - let(:user) { create(:user, saw_onboarding: false) } + let(:user) { create(:user, saw_onboarding: false, location: "Llama Town") } describe "PATCH /onboarding_update" do - it "updates saw_onboarding boolean" do - sign_in user - patch "/onboarding_update.json", params: {} - expect(user.saw_onboarding).to eq(true) + context "when signed in" do + before { sign_in user } + + it "updates saw_onboarding boolean" do + sign_in user + patch "/onboarding_update.json", params: {} + expect(user.saw_onboarding).to eq(true) + end + + it "updates the attributes on the user" do + params = { user: { location: "Alpaca Town" } } + expect do + patch "/onboarding_update.json", params: params + end.to change(user, :location) + end + + it "does not update attributes if params are empty" do + params = { user: { location: "" } } + expect do + patch "/onboarding_update.json", params: params + end.not_to change(user, :location) + end end - it "returns a not found error if user is not signed in" do - patch "/onboarding_update.json", params: {} - expect(response.parsed_body["error"]).to include("Please sign in") + context "when signed out" do + it "returns a not found error if user is not signed in" do + patch "/onboarding_update.json", params: {} + expect(response.parsed_body["error"]).to include("Please sign in") + end end end describe "PATCH /onboarding_checkbox_update" do - it "updates saw_onboarding boolean" do - sign_in user - patch "/onboarding_checkbox_update.json", params: {} - expect(user.saw_onboarding).to eq(true) + context "when signed in" do + before { sign_in user } + + it "updates saw_onboarding boolean" do + patch "/onboarding_checkbox_update.json", params: {} + expect(user.saw_onboarding).to eq(true) + end end - it "returns a not found error if user is not signed in" do - patch "/onboarding_checkbox_update.json", params: {} - expect(response.parsed_body["error"]).to include("Please sign in") + context "when signed out" do + it "returns a not found error if user is not signed in" do + patch "/onboarding_checkbox_update.json", params: {} + expect(response.parsed_body["error"]).to include("Please sign in") + end end end end