From 085c566a7bf053effbfcad04be16cfad248bef26 Mon Sep 17 00:00:00 2001 From: Daniel Uber Date: Fri, 21 Jan 2022 10:58:05 -0600 Subject: [PATCH] Onboarding "improvements" (#16210) * Don't set saw_onboarding automatically This should be set by the onboarding checkbox update (user accepted terms of service and code of conduct). The issue is a PATCH request to track the last seen page is sent on the first page, _before_ the user consents to the terms via the checkboxes. This issue is masked by a 422 unprocessable entity response when we don't get a username in the patch request (next commit). * Only validate username on the username page Every other "last page seen" dialog was returning a 422 because the username is only submitted in the 3rd dialog "build your profile". Only validate the username field when it's submitted, process other onboarding updates normally. * Remove failing test Since the onboarding_update action no longer sets saw onboarding (since we send the patch before they've accepted the code of conduct), don't test that we do. There's a parallel test for the onboarding checkbox update later in this file that covers the checkboxes have been accepted. * overwrite instead of merging user_params We now initialize it empty, no need to merge params here. --- app/controllers/users_controller.rb | 6 +++--- spec/requests/users_onboarding_spec.rb | 5 ----- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 30c85413b..b416a1d02 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -169,15 +169,15 @@ class UsersController < ApplicationController def onboarding_update authorize User - user_params = { saw_onboarding: true } + user_params = {} if params[:user] - if params.dig(:user, :username).blank? + if params[:user].key?(:username) && params[:user][:username].blank? return render_update_response(false, "Username cannot be blank") end sanitize_user_params - user_params.merge!(params[:user].permit(ALLOWED_USER_PARAMS)) + user_params = params[:user].permit(ALLOWED_USER_PARAMS) end update_result = Users::Update.call(current_user, user: user_params, profile: profile_params) diff --git a/spec/requests/users_onboarding_spec.rb b/spec/requests/users_onboarding_spec.rb index 6a59eeb22..94e867d5d 100644 --- a/spec/requests/users_onboarding_spec.rb +++ b/spec/requests/users_onboarding_spec.rb @@ -12,11 +12,6 @@ RSpec.describe "UsersOnboarding", type: :request do context "when signed in" do before { sign_in user } - it "updates saw_onboarding boolean" do - patch "/onboarding_update.json", params: {} - expect(user.saw_onboarding).to eq(true) - end - it "updates the user's last_onboarding_page attribute" do params = { user: { last_onboarding_page: "v2: personal info form", username: "test" } } expect do