Merge pull request #749 from sharetribe/required-validations

Fix data validation with required fields and whitespace

src/components/EditListingDescriptionPanel/EditListingDescriptionPanel.js

@@ -45,7 +45,7 @@ const EditListingDescriptionPanel = props => {
         onSubmit={values => {
           const { title, description, category } = values;
           const updateValues = {
-            title,
+            title: title.trim(),
             description,
             publicData: { category },
           };

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);
     };
 

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(
       {

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)));
       });
   };

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)]}
           />
         </AddImages>
         {uploadImageFailed}

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;

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',

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

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,
+      };
       const uploadedImage = this.props.image;
 
       // Update profileImage only if file system has been accessed

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

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,14 +55,19 @@ 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 noEmptyArray = message => value => {
+export const nonEmptyArray = message => value => {
   return value && Array.isArray(value) && value.length > 0 ? VALID : message;
 };
 

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');
+    });
+  });
+});