Skip to content

Commit

Permalink
Merge pull request #6251 from open-sausages/pulls/4.0/schema-validati…
Browse files Browse the repository at this point in the history
…on-rules

Implement client-side form validation feedback in React forms
  • Loading branch information
Damian Mooyman committed Nov 2, 2016
2 parents 4ee78fc + 3449d5d commit ce10530
Show file tree
Hide file tree
Showing 26 changed files with 2,426 additions and 1,351 deletions.
330 changes: 191 additions & 139 deletions admin/client/dist/js/bundle.js

Large diffs are not rendered by default.

2,609 changes: 1,491 additions & 1,118 deletions admin/client/dist/js/vendor.js

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions admin/client/dist/styles/bundle.css
Expand Up @@ -14893,6 +14893,10 @@ div.TreeDropdownField a.jstree-loading .jstree-pageicon{
vertical-align:middle;
}

.form__field-message-item{
display:block;
}

.btn-toolbar{
margin-top:1.2308rem;
margin-bottom:1.2308rem;
Expand Down
1 change: 1 addition & 0 deletions admin/client/src/bundles/vendor.js
Expand Up @@ -26,6 +26,7 @@ require('expose?ReactAddonsCssTransitionGroup!react-addons-css-transition-group'
require('expose?ReactAddonsTestUtils!react-addons-test-utils');
require('expose?Page!page.js');
require('expose?BootstrapCollapse!bootstrap/dist/js/umd/collapse.js');
require('expose?validator!validator');

require('../../../thirdparty/jquery-ondemand/jquery.ondemand.js');
require('../../../thirdparty/jquery-ui/jquery-ui.js');
Expand Down
21 changes: 11 additions & 10 deletions admin/client/src/components/FieldHolder/FieldHolder.js
Expand Up @@ -12,7 +12,7 @@ function fieldHolder(Field) {
*
* @returns {Component}
*/
getDescription() {
renderDescription() {
if (this.props.description === null) {
return null;
}
Expand All @@ -29,10 +29,11 @@ function fieldHolder(Field) {
*
* @returns {Component}
*/
getMessage() {
const message = (this.props.meta) ? this.props.meta.error : null;
renderMessage() {
const meta = this.props.meta;
const message = (meta) ? meta.error : null;

if (!message) {
if (!message || (meta && !meta.touched)) {
return null;
}

Expand All @@ -46,7 +47,7 @@ function fieldHolder(Field) {
*
* @returns {Component}
*/
getLeftTitle() {
renderLeftTitle() {
const labelText = this.props.leftTitle !== null
? this.props.leftTitle
: this.props.title;
Expand All @@ -67,7 +68,7 @@ function fieldHolder(Field) {
*
* @returns {Component}
*/
getRightTitle() {
renderRightTitle() {
if (!this.props.rightTitle || this.props.hideLabels) {
return null;
}
Expand Down Expand Up @@ -108,13 +109,13 @@ function fieldHolder(Field) {
render() {
return (
<FormGroup {...this.getHolderProps()}>
{this.getLeftTitle()}
{this.renderLeftTitle()}
<div className="form__field-holder">
<Field {...this.props} />
{this.getMessage()}
{this.getDescription()}
{this.renderMessage()}
{this.renderDescription()}
</div>
{this.getRightTitle()}
{this.renderRightTitle()}
</FormGroup>
);
}
Expand Down
3 changes: 3 additions & 0 deletions admin/client/src/components/FieldHolder/FieldHolder.scss
@@ -0,0 +1,3 @@
.form__field-message-item {
display: block;
}
62 changes: 53 additions & 9 deletions admin/client/src/components/FormBuilder/FormBuilder.js
@@ -1,9 +1,10 @@
import React, { PropTypes } from 'react';
import merge from 'merge';
import schemaFieldValues, { findField } from 'lib/schemaFieldValues';
import SilverStripeComponent from 'lib/SilverStripeComponent';
import schemaFieldValues from 'lib/schemaFieldValues';
import Validator from 'lib/Validator';
import backend from 'lib/Backend';
import injector from 'lib/Injector';
import merge from 'merge';

class FormBuilder extends SilverStripeComponent {

Expand All @@ -21,6 +22,50 @@ class FormBuilder extends SilverStripeComponent {
this.handleSubmit = this.handleSubmit.bind(this);
this.handleAction = this.handleAction.bind(this);
this.buildComponent = this.buildComponent.bind(this);
this.validateForm = this.validateForm.bind(this);
}

/**
* Run validation for every field on the form and return an object which list issues while
* validating
*
* @param values
* @returns {*}
*/
validateForm(values) {
if (typeof this.props.validate === 'function') {
return this.props.validate(values);
}

const schema = this.props.schema && this.props.schema.schema;
if (!schema) {
return {};
}

const validator = new Validator(values);

return Object.entries(values).reduce((prev, curr) => {
const [key] = curr;
const field = findField(this.props.schema.schema.fields, key);

const { valid, errors } = validator.validateFieldSchema(field);

if (valid) {
return prev;
}

// so if there are multiple errors, it will be listed in html spans
const errorHtml = errors.map((message, index) => (
<span key={index} className="form__validation-message">{message}</span>
));

return Object.assign({}, prev, {
[key]: {
type: 'error',
value: { react: errorHtml },
},
});
}, {});
}

/**
Expand All @@ -32,14 +77,12 @@ class FormBuilder extends SilverStripeComponent {
handleAction(event) {
// Custom handlers
if (typeof this.props.handleAction === 'function') {
this.props.handleAction(event, schemaFieldValues());
this.props.handleAction(event, this.props.values);
}

const name = event.currentTarget.name;

// Allow custom handlers to cancel event
if (!event.isPropagationStopped()) {
this.setState({ submittingAction: name });
this.setState({ submittingAction: event.currentTarget.name });
}
}

Expand Down Expand Up @@ -72,6 +115,7 @@ class FormBuilder extends SilverStripeComponent {
return formSchema;
})
.catch((reason) => {
// TODO Generic CMS error reporting
this.setState({ submittingAction: null });
throw reason;
});
Expand Down Expand Up @@ -274,7 +318,6 @@ class FormBuilder extends SilverStripeComponent {
touchOnBlur,
touchOnChange,
persistentSubmitErrors,
validate,
form,
} = this.props;

Expand All @@ -297,7 +340,7 @@ class FormBuilder extends SilverStripeComponent {
touchOnBlur,
touchOnChange,
persistentSubmitErrors,
validate,
validate: this.validateForm,
};

return <BaseFormComponent {...props} />;
Expand Down Expand Up @@ -330,14 +373,15 @@ const basePropTypes = {
touchOnChange: PropTypes.bool,
persistentSubmitErrors: PropTypes.bool,
validate: PropTypes.func,
values: PropTypes.object,
submitting: PropTypes.bool,
baseFormComponent: PropTypes.func.isRequired,
baseFieldComponent: PropTypes.func.isRequired,
};

FormBuilder.propTypes = Object.assign({}, basePropTypes, {
form: PropTypes.string.isRequired,
schema: schemaPropType.isRequired,
submitting: PropTypes.bool,
});

export { basePropTypes, schemaPropType };
Expand Down
42 changes: 21 additions & 21 deletions admin/client/src/components/FormBuilder/tests/FormBuilder-test.js
Expand Up @@ -82,9 +82,9 @@ describe('FormBuilder', () => {
{ id: 'fieldTwo', name: 'fieldTwo' },
];
props.schema.state.fields = [
{ id: 'fieldOne', value: 'valOne' },
{ id: 'fieldTwo', value: null },
{ id: 'notInSchema', value: 'invalid' },
{ id: 'fieldOne', name: 'fieldOne', value: 'valOne' },
{ id: 'fieldTwo', name: 'fieldTwo', value: null },
{ id: 'notInSchema', name: 'notInSchema', value: 'invalid' },
];
fieldValues = schemaFieldValues(props.schema.schema, props.schema.state);
expect(fieldValues).toEqual({
Expand All @@ -110,9 +110,9 @@ describe('FormBuilder', () => {
{ id: 'actionTwo', name: 'actionTwo' },
];
props.schema.state.fields = [
{ id: 'fieldOne', value: 'valOne' },
{ id: 'fieldTwo', value: null },
{ id: 'notInSchema', value: 'invalid' },
{ id: 'fieldOne', name: 'fieldOne', value: 'valOne' },
{ id: 'fieldTwo', name: 'fieldTwo', value: null },
{ id: 'notInSchema', name: 'notInSchema', value: 'invalid' },
];
});

Expand Down Expand Up @@ -156,35 +156,35 @@ describe('FormBuilder', () => {

it('should retrieve the field in the shallow fields list', () => {
fields = [
{ id: 'fieldOne' },
{ id: 'fieldTwo' },
{ id: 'fieldThree' },
{ id: 'fieldFour' },
{ id: 'fieldOne', name: 'fieldOne' },
{ id: 'fieldTwo', name: 'fieldTwo' },
{ id: 'fieldThree', name: 'fieldThree' },
{ id: 'fieldFour', name: 'fieldFour' },
];
const field = findField(fields, 'fieldThree');

expect(field).toBeTruthy();
expect(field.id).toBe('fieldThree');
expect(field.name).toBe('fieldThree');
});

it('should retrieve the field that is a grandchild in the fields list', () => {
fields = [
{ id: 'fieldOne' },
{ id: 'fieldTwo', children: [
{ id: 'fieldTwoOne' },
{ id: 'fieldTwoTwo', children: [
{ id: 'fieldTwoOne' },
{ id: 'fieldTwoTwo' },
{ id: 'fieldTwoThree' },
{ id: 'fieldOne', name: 'fieldOne' },
{ id: 'fieldTwo', name: 'fieldTwo', children: [
{ id: 'fieldTwoOne', name: 'fieldTwoOne' },
{ id: 'fieldTwoTwo', name: 'fieldTwoTwo', children: [
{ id: 'fieldTwoOne', name: 'fieldTwoOne' },
{ id: 'fieldTwoTwo', name: 'fieldTwoTwo' },
{ id: 'fieldTwoThree', name: 'fieldTwoThree' },
] },
] },
{ id: 'fieldThree' },
{ id: 'fieldFour' },
{ id: 'fieldThree', name: 'fieldThree' },
{ id: 'fieldFour', name: 'fieldFour' },
];
const field = findField(fields, 'fieldTwoThree');

expect(field).toBeTruthy();
expect(field.id).toBe('fieldTwoThree');
expect(field.name).toBe('fieldTwoThree');
});
});
});
53 changes: 27 additions & 26 deletions admin/client/src/containers/FormBuilderLoader/FormBuilderLoader.js
Expand Up @@ -81,30 +81,27 @@ class FormBuilderLoader extends Component {
promise = submitFn();
}

if (promise) {
promise
.then(formSchema => {
this.props.schemaActions.setSchema(formSchema);
return formSchema;
})
// TODO Suggest storing messages in a separate redux store rather than throw an error
// ref: https://github.com/erikras/redux-form/issues/94#issuecomment-143398399
.then(formSchema => {
if (!formSchema.state) {
return formSchema;
}
const messages = this.getMessages(formSchema.state);

if (Object.keys(messages).length) {
throw new SubmissionError(messages);
}
return formSchema;
});
} else {
if (!promise) {
throw new Error('Promise was not returned for submitting');
}
return promise
.then(formSchema => {
this.props.schemaActions.setSchema(formSchema);
return formSchema;
})
// TODO Suggest storing messages in a separate redux store rather than throw an error
// ref: https://github.com/erikras/redux-form/issues/94#issuecomment-143398399
.then(formSchema => {
if (!formSchema.state) {
return formSchema;
}
const messages = this.getMessages(formSchema.state);

return promise;
if (Object.keys(messages).length) {
throw new SubmissionError(messages);
}
return formSchema;
});
}

/**
Expand Down Expand Up @@ -174,11 +171,15 @@ FormBuilderLoader.defaultProps = {
export default connect(
(state, ownProps) => {
const schema = state.schemas[ownProps.schemaUrl];
const form = schema ? schema.id : null;
const submitting = state.form
&& state.form[ownProps.schemaUrl]
&& state.form[ownProps.schemaUrl].submitting;
return { schema, form, submitting };
const form = schema && schema.id;
const reduxFormState = state.form
&& state.form[ownProps.schemaUrl];
const submitting = reduxFormState
&& reduxFormState.submitting;
const values = reduxFormState
&& reduxFormState.values;

return { schema, form, submitting, values };
},
(dispatch) => ({
schemaActions: bindActionCreators(schemaActions, dispatch),
Expand Down
9 changes: 0 additions & 9 deletions admin/client/src/legacy/AddToCampaignForm.js
Expand Up @@ -79,15 +79,6 @@ jQuery.entwine('ss', ($) => {
_handleSubmitModal(data, action, submitFn) {
event.preventDefault();

if (!data.Campaign) {
// TODO invisible submit disable, remove this when validation is implemented
// eslint-disable-next-line no-alert
alert(i18n._t(
'AddToCampaigns.ErrorCampaignNotSelected',
'There was no campaign selected to be added to'
));
return null;
}
return submitFn();
},

Expand Down

0 comments on commit ce10530

Please sign in to comment.