diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 7b86b939c..02d2490b0 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -10,7 +10,7 @@ class UsersController < ApplicationController
before_action :set_suggested_users, only: %i[index]
before_action :initialize_stripe, only: %i[edit]
- ALLOWED_USER_PARAMS = %i[last_onboarding_page].freeze
+ ALLOWED_USER_PARAMS = %i[last_onboarding_page username].freeze
INDEX_ATTRIBUTES_FOR_SERIALIZATION = %i[id name username summary profile_image].freeze
private_constant :INDEX_ATTRIBUTES_FOR_SERIALIZATION
@@ -165,19 +165,20 @@ class UsersController < ApplicationController
end
def onboarding_update
- if params[:user]
- sanitize_user_params
- current_user.assign_attributes(params[:user].permit(ALLOWED_USER_PARAMS))
- current_user.profile_updated_at = Time.current
- end
-
- if current_user.save && params[:profile]
- update_result = Profiles::Update.call(current_user, { profile: profile_params })
- end
-
- current_user.saw_onboarding = true
authorize User
- render_update_response(update_result&.success?)
+ user_params = { saw_onboarding: true }
+
+ if params[:user]
+ if params.dig(: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))
+ end
+
+ update_result = Profiles::Update.call(current_user, { user: user_params, profile: profile_params })
+ render_update_response(update_result.success?, update_result.errors_as_sentence)
end
def onboarding_checkbox_update
@@ -313,11 +314,11 @@ class UsersController < ApplicationController
recent_suggestions.presence || default_suggested_users
end
- def render_update_response(success)
- outcome = success ? "updated successfully" : "update failed"
+ def render_update_response(success, errors = nil)
+ status = success ? 200 : 422
respond_to do |format|
- format.json { render json: { outcome: outcome } }
+ format.json { render json: { errors: errors }, status: status }
end
end
@@ -376,7 +377,7 @@ class UsersController < ApplicationController
end
def profile_params
- params[:profile].permit(Profile.attributes)
+ params[:profile] ? params[:profile].permit(Profile.attributes) : nil
end
def password_params
diff --git a/app/javascript/onboarding/__tests__/Onboarding.test.jsx b/app/javascript/onboarding/__tests__/Onboarding.test.jsx
index c18fecfab..b739ae809 100644
--- a/app/javascript/onboarding/__tests__/Onboarding.test.jsx
+++ b/app/javascript/onboarding/__tests__/Onboarding.test.jsx
@@ -133,7 +133,7 @@ describe('', () => {
termsCheckbox.click();
// click to next step
- const nextButton = await findByText(/continue/i);
+ let nextButton = await findByText(/continue/i);
fetch.mockResponse(fakeEmptyResponse);
nextButton.click();
@@ -148,10 +148,10 @@ describe('', () => {
// we should be on the Profile Form step
await findByTestId('onboarding-profile-form');
- // click on skip for now
- skipButton = getByText(/Skip for now/i);
+ // click on continue without adjusting form fields
+ nextButton = getByText(/Continue/i);
fetch.mockResponse(fakeEmptyResponse);
- skipButton.click();
+ nextButton.click();
// we should be on the Follow Users step
await findByTestId('onboarding-follow-users');
diff --git a/app/javascript/onboarding/components/ProfileForm.jsx b/app/javascript/onboarding/components/ProfileForm.jsx
index eb4e0981b..c70cdb149 100644
--- a/app/javascript/onboarding/components/ProfileForm.jsx
+++ b/app/javascript/onboarding/components/ProfileForm.jsx
@@ -1,7 +1,7 @@
import { h, Component } from 'preact';
import PropTypes from 'prop-types';
-import { userData, getContentOfToken, updateOnboarding } from '../utilities';
+import { userData, updateOnboarding } from '../utilities';
import { Navigation } from './Navigation';
import { ColorPicker } from './ProfileForm/ColorPicker';
@@ -22,8 +22,8 @@ export class ProfileForm extends Component {
this.user = userData();
this.state = {
groups: [],
- formValues: {},
- canSkip: true,
+ formValues: { username: this.user.username },
+ canSkip: false,
last_onboarding_page: 'v2: personal info form',
};
}
@@ -47,26 +47,34 @@ export class ProfileForm extends Component {
}
}
- onSubmit() {
- const csrfToken = getContentOfToken('csrf-token');
+ async onSubmit() {
const { formValues, last_onboarding_page } = this.state;
- fetch('/onboarding_update', {
- method: 'PATCH',
- headers: {
- 'X-CSRF-Token': csrfToken,
- 'Content-Type': 'application/json',
- },
- body: JSON.stringify({
- user: { last_onboarding_page },
- profile: { ...formValues },
- }),
- credentials: 'same-origin',
- }).then((response) => {
- if (response.ok) {
- const { next } = this.props;
- next();
+ const { username, ...newFormValues } = formValues;
+ try {
+ const response = await request('/onboarding_update', {
+ method: 'PATCH',
+ body: {
+ user: { last_onboarding_page, username },
+ profile: { ...newFormValues },
+ },
+ });
+ if (!response.ok) {
+ throw response;
}
- });
+ const { next } = this.props;
+ next();
+ } catch (error) {
+ Honeybadger.notify(error.statusText);
+ let errorMessage = 'Unable to continue, please try again.';
+ if (error.status === 422) {
+ // parse validation error messages from UsersController#onboarding_update
+ const errorData = await error.json();
+ errorMessage = errorData.errors;
+ this.setState({ error: true, errorMessage });
+ } else {
+ this.setState({ error: true, errorMessage });
+ }
+ }
}
handleFieldChange(e) {
@@ -153,14 +161,6 @@ export class ProfileForm extends Component {
const { profile_image_90, username, name } = this.user;
const { canSkip, groups = [], error, errorMessage } = this.state;
- if (error) {
- return (
-
- An error occurred: {errorMessage}
-
- );
- }
-
const sections = groups.map((group) => {
return (
@@ -195,6 +195,11 @@ export class ProfileForm extends Component {
slidesCount={slidesCount}
currentSlideIndex={currentSlideIndex}
/>
+ {error && (
+
+ An error occurred: {errorMessage}
+
+ )}
@@ -219,9 +224,19 @@ export class ProfileForm extends Component {
/>
{name}
-
{username}
-
{sections}
+
+
+
+ {sections}
diff --git a/app/javascript/onboarding/components/ProfileForm/TextInput.jsx b/app/javascript/onboarding/components/ProfileForm/TextInput.jsx
index 7cbfce59f..c0fd7d363 100644
--- a/app/javascript/onboarding/components/ProfileForm/TextInput.jsx
+++ b/app/javascript/onboarding/components/ProfileForm/TextInput.jsx
@@ -25,8 +25,15 @@ import PropTypes from 'prop-types';
import { FormField } from '@crayons';
export function TextInput(props) {
- const { onFieldChange } = props;
- const { attribute_name, placeholder_text, description, label } = props.field;
+ const { onFieldChange, field } = props;
+ const {
+ attribute_name,
+ placeholder_text,
+ default_value,
+ description,
+ label,
+ required,
+ } = field;
return (
@@ -36,9 +43,11 @@ export function TextInput(props) {
{description && {description}
}
diff --git a/app/javascript/onboarding/components/__tests__/ProfileForm.test.jsx b/app/javascript/onboarding/components/__tests__/ProfileForm.test.jsx
index a6feec190..9b334e820 100644
--- a/app/javascript/onboarding/components/__tests__/ProfileForm.test.jsx
+++ b/app/javascript/onboarding/components/__tests__/ProfileForm.test.jsx
@@ -1,5 +1,5 @@
import { h } from 'preact';
-import { render, fireEvent } from '@testing-library/preact';
+import { render } from '@testing-library/preact';
import fetch from 'jest-fetch-mock';
import '@testing-library/jest-dom';
import { axe } from 'jest-axe';
@@ -130,14 +130,17 @@ describe('ProfileForm', () => {
const { findByLabelText } = renderProfileForm();
const field1 = await findByLabelText(/Education/i);
- const field2 = await findByLabelText(/Name/i);
+ const field2 = await findByLabelText(/^Name/i);
const field3 = await findByLabelText(/Website URL/i);
+ const field4 = await findByLabelText(/Username/i);
expect(field1).toBeInTheDocument();
expect(field2).toBeInTheDocument();
expect(field2.getAttribute('placeholder')).toEqual('John Doe');
expect(field3).toBeInTheDocument();
expect(field3.getAttribute('placeholder')).toEqual('https://yoursite.com');
+ expect(field4).toBeInTheDocument();
+ expect(field4.getAttribute('value')).toEqual('username');
});
it('should render a stepper', () => {
@@ -152,44 +155,9 @@ describe('ProfileForm', () => {
expect(queryByTestId('back-button')).toBeDefined();
});
- it('should update the text on the forward button', async () => {
- const {
- getByLabelText,
- getByText,
- queryByText,
- findByLabelText,
- findByText,
- } = renderProfileForm();
+ it('should not be skippable', async () => {
+ const { getByText } = renderProfileForm();
- // input the bio
- const field2 = await findByLabelText(/Name/i);
- expect(field2.value).toEqual('');
- getByText(/skip for now/i);
- expect(queryByText(/continue/i)).toBeNull();
-
- fireEvent.keyDown(field2, {
- key: 'Enter',
- keyCode: 13,
- which: 13,
- target: { value: 'Hong Kong Fuey' },
- });
- expect(field2.value).toEqual('Hong Kong Fuey');
-
- // input the location too (since we're using firevent and it doesn't call the focus events
- // that will trigger the continue )
- let field3 = getByLabelText(/Website URL/i);
- expect(field3.value).toEqual('');
- fireEvent.keyDown(field3, {
- key: 'Enter',
- keyCode: 13,
- which: 13,
- target: { value: 'www.website.com' },
- });
-
- field3 = await findByLabelText(/Website URL/i);
-
- expect(field3.value).toEqual('www.website.com');
-
- findByText(/continue/i);
+ expect(getByText(/continue/i)).toBeInTheDocument();
});
});
diff --git a/spec/requests/users_onboarding_spec.rb b/spec/requests/users_onboarding_spec.rb
index 73d1a2e77..41998a183 100644
--- a/spec/requests/users_onboarding_spec.rb
+++ b/spec/requests/users_onboarding_spec.rb
@@ -13,12 +13,32 @@ RSpec.describe "UsersOnboarding", type: :request do
end
it "updates the user's last_onboarding_page attribute" do
- params = { user: { last_onboarding_page: "v2: personal info form" } }
+ params = { user: { last_onboarding_page: "v2: personal info form", username: "test" } }
expect do
patch "/onboarding_update.json", params: params
end.to change(user, :last_onboarding_page)
end
+ it "updates the user's username attribute" do
+ params = { user: { username: "WilhuffTarkin" } }
+ expect do
+ patch "/onboarding_update.json", params: params
+ end.to change(user, :username).to("wilhufftarkin")
+ end
+
+ it "returns a 422 error if the username is blank" do
+ params = { user: { username: "" } }
+ patch "/onboarding_update.json", params: params
+ expect(response).to have_http_status(:unprocessable_entity)
+ end
+
+ it "updates the user's profile" do
+ params = { profile: { employer_name: "Galatic Empire" } }
+ expect do
+ patch "/onboarding_update.json", params: params
+ end.to change(user.profile, :employer_name).to("Galatic Empire")
+ end
+
it "does not update the user's last_onboarding_page if it is empty" do
params = { user: { last_onboarding_page: "" } }
expect do