From 9d319a6275f41f6d3e11c2798eefd4dbb695c9c9 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Mon, 12 Mar 2018 16:16:07 +0200 Subject: [PATCH 1/6] Rename validator: noEmptyArray -> nonEmptyArray --- src/containers/EditListingPhotosForm/EditListingPhotosForm.js | 4 ++-- src/util/validators.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/containers/EditListingPhotosForm/EditListingPhotosForm.js b/src/containers/EditListingPhotosForm/EditListingPhotosForm.js index 70efed72..bd6009dc 100644 --- a/src/containers/EditListingPhotosForm/EditListingPhotosForm.js +++ b/src/containers/EditListingPhotosForm/EditListingPhotosForm.js @@ -7,7 +7,7 @@ import { isEqual } from 'lodash'; import { arrayMove } from 'react-sortable-hoc'; import classNames from 'classnames'; import { propTypes } from '../../util/types'; -import { noEmptyArray } from '../../util/validators'; +import { nonEmptyArray } from '../../util/validators'; import { isUploadListingImageOverLimitError } from '../../util/errors'; import { Form, AddImages, Button, ValidationError } from '../../components'; @@ -192,7 +192,7 @@ export class EditListingPhotosFormComponent extends Component { }} name="images" type="hidden" - validate={[noEmptyArray(imageRequiredMessage)]} + validate={[nonEmptyArray(imageRequiredMessage)]} /> {uploadImageFailed} diff --git a/src/util/validators.js b/src/util/validators.js index 5f015b75..db2e00e9 100644 --- a/src/util/validators.js +++ b/src/util/validators.js @@ -44,7 +44,7 @@ export const maxLength = (message, maximumLength) => value => { return !value || value.length <= maximumLength ? VALID : message; }; -export const noEmptyArray = message => value => { +export const nonEmptyArray = message => value => { return value && Array.isArray(value) && value.length > 0 ? VALID : message; }; From 8dd6e329d5bda734b03defd4ce2a2b099174b8d7 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Mon, 12 Mar 2018 16:20:22 +0200 Subject: [PATCH 2/6] Trim listing title and description before saving --- .../EditListingDescriptionPanel.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/EditListingDescriptionPanel/EditListingDescriptionPanel.js b/src/components/EditListingDescriptionPanel/EditListingDescriptionPanel.js index ce061a4d..42479c7d 100644 --- a/src/components/EditListingDescriptionPanel/EditListingDescriptionPanel.js +++ b/src/components/EditListingDescriptionPanel/EditListingDescriptionPanel.js @@ -45,8 +45,8 @@ const EditListingDescriptionPanel = props => { onSubmit={values => { const { title, description, category } = values; const updateValues = { - title, - description, + title: title.trim(), + description: description.trim(), publicData: { category }, }; From f792d580624143b08180bb1c522bd1c897100d24 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Mon, 12 Mar 2018 16:51:25 +0200 Subject: [PATCH 3/6] Improve validators --- src/util/validators.js | 31 +++++++-- src/util/validators.test.js | 123 ++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 src/util/validators.test.js diff --git a/src/util/validators.js b/src/util/validators.js index db2e00e9..dc25fe7d 100644 --- a/src/util/validators.js +++ b/src/util/validators.js @@ -7,6 +7,10 @@ const { LatLng } = types; export const PASSWORD_MIN_LENGTH = 8; export const PASSWORD_MAX_LENGTH = 256; +const isNonEmptyString = val => { + return typeof val === 'string' && val.trim().length > 0; +}; + /** * Validator functions and helpers for Redux Forms */ @@ -15,15 +19,29 @@ export const PASSWORD_MAX_LENGTH = 256; const VALID = undefined; export const required = message => value => { - return value ? VALID : message; + if (typeof value === 'undefined' || value === null) { + // undefined or null values are invalid + return message; + } + if (typeof value === 'string') { + // string must be nonempty when trimmed + return isNonEmptyString(value) ? VALID : message; + } + return VALID; }; +export const requiredStringNoTrim = message => value => { + return typeof value === 'string' && value.length > 0 ? VALID : message; +}; + +// DEPRECATED in favor of required export const requiredBoolean = message => value => { return typeof value === 'boolean' ? VALID : message; }; +// DEPRECATED in favor of required export const requiredAndNonEmptyString = message => value => { - return value && typeof value === 'string' && value.trim() ? VALID : message; + return isNonEmptyString(value) ? VALID : message; }; export const requiredFieldArrayCheckbox = message => value => { @@ -37,11 +55,16 @@ export const requiredFieldArrayCheckbox = message => value => { }; export const minLength = (message, minimumLength) => value => { - return value && value.length >= minimumLength ? VALID : message; + const hasLength = value && typeof value.length === 'number'; + return hasLength && value.length >= minimumLength ? VALID : message; }; export const maxLength = (message, maximumLength) => value => { - return !value || value.length <= maximumLength ? VALID : message; + if (!value) { + return VALID; + } + const hasLength = value && typeof value.length === 'number'; + return hasLength && value.length <= maximumLength ? VALID : message; }; export const nonEmptyArray = message => value => { diff --git a/src/util/validators.test.js b/src/util/validators.test.js new file mode 100644 index 00000000..5c6e3489 --- /dev/null +++ b/src/util/validators.test.js @@ -0,0 +1,123 @@ +import { required, requiredStringNoTrim, minLength, maxLength } from './validators'; + +describe('validators', () => { + describe('required()', () => { + it('should not allow no value', () => { + expect(required('fail')()).toEqual('fail'); + }); + it('should not allow undefined', () => { + expect(required('fail')(undefined)).toEqual('fail'); + }); + it('should not allow null', () => { + expect(required('fail')(null)).toEqual('fail'); + }); + it('should not allow empty string', () => { + expect(required('fail')('')).toEqual('fail'); + }); + it('should not allow string with only whitespace', () => { + expect(required('fail')(' ')).toEqual('fail'); + }); + it('should allow boolean false', () => { + expect(required('fail')(false)).toBeUndefined(); + }); + it('should allow boolean true', () => { + expect(required('fail')(true)).toBeUndefined(); + }); + it('should allow numeric 0', () => { + expect(required('fail')(0)).toBeUndefined(); + }); + it('should allow nonempty string', () => { + expect(required('fail')('abc')).toBeUndefined(); + }); + it('should allow empty array', () => { + expect(required('fail')([])).toBeUndefined(); + }); + it('should allow empty object', () => { + expect(required('fail')({})).toBeUndefined(); + }); + }); + describe('requiredStringNoTrim()', () => { + it('should not allow no value', () => { + expect(requiredStringNoTrim('fail')()).toEqual('fail'); + }); + it('should not allow undefined', () => { + expect(requiredStringNoTrim('fail')(undefined)).toEqual('fail'); + }); + it('should not allow null', () => { + expect(requiredStringNoTrim('fail')(null)).toEqual('fail'); + }); + it('should not allow empty string', () => { + expect(requiredStringNoTrim('fail')('')).toEqual('fail'); + }); + it('should allow string with only whitespace', () => { + expect(requiredStringNoTrim('fail')(' ')).toBeUndefined(); + }); + it('should allow string with chars', () => { + expect(requiredStringNoTrim('fail')('abc')).toBeUndefined(); + }); + }); + describe('minLength()', () => { + it('should not allow no value', () => { + expect(minLength('fail', 3)()).toEqual('fail'); + }); + it('should not allow undefined', () => { + expect(minLength('fail', 3)(undefined)).toEqual('fail'); + }); + it('should not allow null', () => { + expect(minLength('fail', 3)(null)).toEqual('fail'); + }); + it('should not allow empty string', () => { + expect(minLength('fail', 3)('')).toEqual('fail'); + }); + it('should allow string with only whitespace', () => { + expect(minLength('fail', 3)(' ')).toBeUndefined(); + }); + it('should allow string with enough length', () => { + expect(minLength('fail', 3)('abc')).toBeUndefined(); + }); + it('should allow string with enough length after trim', () => { + expect(minLength('fail', 3)(' abc ')).toBeUndefined(); + }); + it('should not allow empty arrays', () => { + expect(minLength('fail', 3)([])).toEqual('fail'); + }); + it('should not allow too short arrays', () => { + expect(minLength('fail', 3)([1, 2])).toEqual('fail'); + }); + it('should allow arrays with enough length', () => { + expect(minLength('fail', 3)([1, 2, 3])).toBeUndefined(); + }); + }); + describe('maxLength()', () => { + it('should allow no value', () => { + expect(maxLength('fail', 3)()).toBeUndefined(); + }); + it('should allow undefined', () => { + expect(maxLength('fail', 3)(undefined)).toBeUndefined(); + }); + it('should allow null', () => { + expect(maxLength('fail', 3)(null)).toBeUndefined(); + }); + it('should allow empty string', () => { + expect(maxLength('fail', 3)('')).toBeUndefined(); + }); + it('should allow string that short enough', () => { + expect(maxLength('fail', 3)('abc')).toBeUndefined(); + }); + it('should allow string that short enough', () => { + expect(maxLength('fail', 3)('abc')).toBeUndefined(); + }); + it('should nost allow string with extra whitespace', () => { + expect(maxLength('fail', 3)('abc ')).toEqual('fail'); + }); + it('should allow empty array', () => { + expect(maxLength('fail', 3)([])).toBeUndefined(); + }); + it('should allow array with max length', () => { + expect(maxLength('fail', 3)([1, 2, 3])).toBeUndefined(); + }); + it('should not allow too long array', () => { + expect(maxLength('fail', 3)([1, 2, 3, 4])).toEqual('fail'); + }); + }); +}); From 6e707c623dafc9398d2c5a80fae0cbfe43a617b1 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Mon, 12 Mar 2018 16:52:01 +0200 Subject: [PATCH 4/6] Use the non-trimming validator for passwords --- src/containers/ContactDetailsForm/ContactDetailsForm.js | 2 +- src/containers/LoginForm/LoginForm.js | 2 +- src/containers/PasswordChangeForm/PasswordChangeForm.js | 4 ++-- src/containers/PasswordResetForm/PasswordResetForm.js | 2 +- src/containers/SignupForm/SignupForm.js | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/containers/ContactDetailsForm/ContactDetailsForm.js b/src/containers/ContactDetailsForm/ContactDetailsForm.js index 440c9e09..cf19eea3 100644 --- a/src/containers/ContactDetailsForm/ContactDetailsForm.js +++ b/src/containers/ContactDetailsForm/ContactDetailsForm.js @@ -168,7 +168,7 @@ class ContactDetailsFormComponent extends Component { id: 'ContactDetailsForm.passwordRequired', }); - const passwordRequired = validators.required(passwordRequiredMessage); + const passwordRequired = validators.requiredStringNoTrim(passwordRequiredMessage); const passwordMinLengthMessage = intl.formatMessage( { diff --git a/src/containers/LoginForm/LoginForm.js b/src/containers/LoginForm/LoginForm.js index f5f9c6ad..1354c88c 100644 --- a/src/containers/LoginForm/LoginForm.js +++ b/src/containers/LoginForm/LoginForm.js @@ -47,7 +47,7 @@ const LoginFormComponent = props => { const passwordRequiredMessage = intl.formatMessage({ id: 'LoginForm.passwordRequired', }); - const passwordRequired = validators.required(passwordRequiredMessage); + const passwordRequired = validators.requiredStringNoTrim(passwordRequiredMessage); const classes = classNames(rootClassName || css.root, className); const submitInProgress = submitting || inProgress; diff --git a/src/containers/PasswordChangeForm/PasswordChangeForm.js b/src/containers/PasswordChangeForm/PasswordChangeForm.js index 4a041f9d..83e76089 100644 --- a/src/containers/PasswordChangeForm/PasswordChangeForm.js +++ b/src/containers/PasswordChangeForm/PasswordChangeForm.js @@ -55,7 +55,7 @@ class PasswordChangeFormComponent extends Component { const newPasswordRequiredMessage = intl.formatMessage({ id: 'PasswordChangeForm.newPasswordRequired', }); - const newPasswordRequired = validators.required(newPasswordRequiredMessage); + const newPasswordRequired = validators.requiredStringNoTrim(newPasswordRequiredMessage); const passwordMinLengthMessage = intl.formatMessage( { @@ -94,7 +94,7 @@ class PasswordChangeFormComponent extends Component { id: 'PasswordChangeForm.passwordRequired', }); - const passwordRequired = validators.required(passwordRequiredMessage); + const passwordRequired = validators.requiredStringNoTrim(passwordRequiredMessage); const passwordFailedMessage = intl.formatMessage({ id: 'PasswordChangeForm.passwordFailed', diff --git a/src/containers/PasswordResetForm/PasswordResetForm.js b/src/containers/PasswordResetForm/PasswordResetForm.js index e57e9d4b..e89d4f42 100644 --- a/src/containers/PasswordResetForm/PasswordResetForm.js +++ b/src/containers/PasswordResetForm/PasswordResetForm.js @@ -47,7 +47,7 @@ const PasswordResetFormComponent = props => { maxLength: validators.PASSWORD_MAX_LENGTH, } ); - const passwordRequired = validators.required(passwordRequiredMessage); + const passwordRequired = validators.requiredStringNoTrim(passwordRequiredMessage); const passwordMinLength = validators.minLength( passwordMinLengthMessage, validators.PASSWORD_MIN_LENGTH diff --git a/src/containers/SignupForm/SignupForm.js b/src/containers/SignupForm/SignupForm.js index 00ac103c..00c78d0f 100644 --- a/src/containers/SignupForm/SignupForm.js +++ b/src/containers/SignupForm/SignupForm.js @@ -74,7 +74,7 @@ const SignupFormComponent = props => { passwordMaxLengthMessage, validators.PASSWORD_MAX_LENGTH ); - const passwordRequired = validators.required(passwordRequiredMessage); + const passwordRequired = validators.requiredStringNoTrim(passwordRequiredMessage); const passwordValidators = [passwordRequired, passwordMinLength, passwordMaxLength]; // firstName From fc730244ba4f49cb1b3db8ebef140cc6a7ea8175 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Mon, 12 Mar 2018 16:52:34 +0200 Subject: [PATCH 5/6] Trim data before saving --- src/containers/AuthenticationPage/AuthenticationPage.js | 2 +- src/containers/ProfileSettingsPage/ProfileSettingsPage.js | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/containers/AuthenticationPage/AuthenticationPage.js b/src/containers/AuthenticationPage/AuthenticationPage.js index 909eeefe..1435e488 100644 --- a/src/containers/AuthenticationPage/AuthenticationPage.js +++ b/src/containers/AuthenticationPage/AuthenticationPage.js @@ -131,7 +131,7 @@ export class AuthenticationPageComponent extends Component { const handleSubmitSignup = values => { const { fname, lname, ...rest } = values; - const params = { firstName: fname, lastName: lname, ...rest }; + const params = { firstName: fname.trim(), lastName: lname.trim(), ...rest }; submitSignup(params); }; diff --git a/src/containers/ProfileSettingsPage/ProfileSettingsPage.js b/src/containers/ProfileSettingsPage/ProfileSettingsPage.js index 74e6a823..c55fb8fd 100644 --- a/src/containers/ProfileSettingsPage/ProfileSettingsPage.js +++ b/src/containers/ProfileSettingsPage/ProfileSettingsPage.js @@ -54,7 +54,11 @@ export class ProfileSettingsPageComponent extends Component { // Ensure that the optional bio is a string const bio = rawBio || ''; - const profile = { firstName, lastName, bio }; + const profile = { + firstName: firstName.trim(), + lastName: lastName.trim(), + bio: bio.trim(), + }; const uploadedImage = this.props.image; // Update profileImage only if file system has been accessed From 85e29f31264085dd3ccdbfcb30648193a5e02551 Mon Sep 17 00:00:00 2001 From: Kimmo Puputti Date: Tue, 13 Mar 2018 09:53:37 +0200 Subject: [PATCH 6/6] Remove trimming whitespace from listing description and user bio --- .../EditListingDescriptionPanel.js | 2 +- .../EditListingPage/EditListingPage.duck.js | 24 ++++++------------- .../ProfileSettingsPage.js | 2 +- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/components/EditListingDescriptionPanel/EditListingDescriptionPanel.js b/src/components/EditListingDescriptionPanel/EditListingDescriptionPanel.js index 42479c7d..48f6aac4 100644 --- a/src/components/EditListingDescriptionPanel/EditListingDescriptionPanel.js +++ b/src/components/EditListingDescriptionPanel/EditListingDescriptionPanel.js @@ -46,7 +46,7 @@ const EditListingDescriptionPanel = props => { const { title, description, category } = values; const updateValues = { title: title.trim(), - description: description.trim(), + description, publicData: { category }, }; diff --git a/src/containers/EditListingPage/EditListingPage.duck.js b/src/containers/EditListingPage/EditListingPage.duck.js index c5e18d7a..3036f86c 100644 --- a/src/containers/EditListingPage/EditListingPage.duck.js +++ b/src/containers/EditListingPage/EditListingPage.duck.js @@ -264,21 +264,12 @@ export function requestShowListing(actionPayload) { }; } -const cleanUpListingData = data => { - // Since we display the line breaks in the listing description, we - // should trim the extra whitespace from the start and the end of - // the description. - return data.description ? { ...data, description: data.description.trim() } : data; -}; - export function requestCreateListing(data) { return (dispatch, getState, sdk) => { - const cleanedData = cleanUpListingData(data); - - dispatch(createListing(cleanedData)); + dispatch(createListing(data)); return sdk.ownListings - .create(cleanedData) + .create(data) .then(response => { const id = response.data.data.id.uuid; // Modify store to understand that we have created listing and can redirect away @@ -295,7 +286,7 @@ export function requestCreateListing(data) { return response; }) .catch(e => { - log.error(e, 'create-listing-failed', { listingData: cleanedData }); + log.error(e, 'create-listing-failed', { listingData: data }); return dispatch(createListingError(storableError(e))); }); }; @@ -318,12 +309,11 @@ export function requestImageUpload(actionPayload) { // display the state. export function requestUpdateListing(tab, data) { return (dispatch, getState, sdk) => { - const cleanedData = cleanUpListingData(data); - dispatch(updateListing(cleanedData)); - const { id } = cleanedData; + dispatch(updateListing(data)); + const { id } = data; let updateResponse; return sdk.ownListings - .update(cleanedData) + .update(data) .then(response => { updateResponse = response; const payload = { id, include: ['author', 'images'] }; @@ -334,7 +324,7 @@ export function requestUpdateListing(tab, data) { dispatch(updateListingSuccess(updateResponse)); }) .catch(e => { - log.error(e, 'update-listing-failed', { listingData: cleanedData }); + log.error(e, 'update-listing-failed', { listingData: data }); return dispatch(updateListingError(storableError(e))); }); }; diff --git a/src/containers/ProfileSettingsPage/ProfileSettingsPage.js b/src/containers/ProfileSettingsPage/ProfileSettingsPage.js index c55fb8fd..eb234a68 100644 --- a/src/containers/ProfileSettingsPage/ProfileSettingsPage.js +++ b/src/containers/ProfileSettingsPage/ProfileSettingsPage.js @@ -57,7 +57,7 @@ export class ProfileSettingsPageComponent extends Component { const profile = { firstName: firstName.trim(), lastName: lastName.trim(), - bio: bio.trim(), + bio, }; const uploadedImage = this.props.image;