From 330667045934a06ae3a9bb444d9935f2062155b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A1ssio=20Souza?= Date: Fri, 13 Jan 2017 19:30:40 -0200 Subject: [PATCH] Validation (#216) * Field config options: 'required' and 'pattern' * Widget controls can implement it's own isValid * Validation errors store in redux & displayed * Support for returned Promises in isValid * Allow widget controls to return either a boolean, an error object or a promise from isValid --- src/actions/editorialWorkflow.js | 7 +- src/actions/entries.js | 17 +++- src/components/ControlPanel/ControlPane.css | 14 +++ src/components/ControlPanel/ControlPane.js | 49 +++++++--- src/components/EntryEditor/EntryEditor.js | 15 ++- src/components/Widgets/ControlHOC.js | 92 +++++++++++++++++++ src/components/Widgets/FileControl.js | 12 ++- src/components/Widgets/ImageControl.js | 12 ++- src/containers/EntryPage.js | 10 +- .../editorialWorkflow/EntryPageHOC.js | 4 +- src/reducers/__tests__/entryDraft.spec.js | 4 +- src/reducers/entryDraft.js | 13 ++- 12 files changed, 224 insertions(+), 25 deletions(-) create mode 100644 src/components/Widgets/ControlHOC.js diff --git a/src/actions/editorialWorkflow.js b/src/actions/editorialWorkflow.js index dd4b88b5..5ec74c54 100644 --- a/src/actions/editorialWorkflow.js +++ b/src/actions/editorialWorkflow.js @@ -219,9 +219,14 @@ export function loadUnpublishedEntries() { }; } -export function persistUnpublishedEntry(collection, entryDraft, existingUnpublishedEntry) { +export function persistUnpublishedEntry(collection, existingUnpublishedEntry) { return (dispatch, getState) => { const state = getState(); + const entryDraft = state.entryDraft; + + // Early return if draft contains validation errors + if (!entryDraft.get('fieldsErrors').isEmpty()) return; + const backend = currentBackend(state.config); const assetProxies = entryDraft.get('mediaFiles').map(path => getAsset(state, path)); const entry = entryDraft.get('entry'); diff --git a/src/actions/entries.js b/src/actions/entries.js index 7c530a4e..22ffd848 100644 --- a/src/actions/entries.js +++ b/src/actions/entries.js @@ -24,6 +24,7 @@ export const DRAFT_CREATE_EMPTY = 'DRAFT_CREATE_EMPTY'; export const DRAFT_DISCARD = 'DRAFT_DISCARD'; export const DRAFT_CHANGE = 'DRAFT_CHANGE'; export const DRAFT_CHANGE_FIELD = 'DRAFT_CHANGE_FIELD'; +export const DRAFT_VALIDATION_ERRORS = 'DRAFT_VALIDATION_ERRORS'; export const ENTRY_PERSIST_REQUEST = 'ENTRY_PERSIST_REQUEST'; export const ENTRY_PERSIST_SUCCESS = 'ENTRY_PERSIST_SUCCESS'; @@ -141,6 +142,7 @@ export function createDraftFromEntry(entry) { }; } + export function discardDraft() { return { type: DRAFT_DISCARD, @@ -161,6 +163,14 @@ export function changeDraftField(field, value, metadata) { }; } +export function changeDraftFieldValidation(field, errors) { + return { + type: DRAFT_VALIDATION_ERRORS, + payload: { field, errors }, + }; +} + + /* * Exported Thunk Action Creators */ @@ -213,9 +223,14 @@ export function createEmptyDraft(collection) { }; } -export function persistEntry(collection, entryDraft) { +export function persistEntry(collection) { return (dispatch, getState) => { const state = getState(); + const entryDraft = state.entryDraft; + + // Early return if draft contains validation errors + if (!entryDraft.get('fieldsErrors').isEmpty()) return; + const backend = currentBackend(state.config); const assetProxies = entryDraft.get('mediaFiles').map(path => getAsset(state, path)); const entry = entryDraft.get('entry'); diff --git a/src/components/ControlPanel/ControlPane.css b/src/components/ControlPanel/ControlPane.css index 25973c88..7915b19c 100644 --- a/src/components/ControlPanel/ControlPane.css +++ b/src/components/ControlPanel/ControlPane.css @@ -19,12 +19,26 @@ color: #7c8382; } } + .label { display: block; color: #AAB0AF; font-size: 12px; + margin-bottom: 8px; +} + +.labelWithError { + composes: label; + color: #FF706F; +} + +.errors { + list-style-type: none; + font-size: 10px; + color: #FF706F; margin-bottom: 18px; } + .widget { border-bottom: 1px solid #e8eae8; position: relative; diff --git a/src/components/ControlPanel/ControlPane.js b/src/components/ControlPanel/ControlPane.js index 400e8532..93a3ce98 100644 --- a/src/components/ControlPanel/ControlPane.js +++ b/src/components/ControlPanel/ControlPane.js @@ -1,6 +1,8 @@ import React, { Component, PropTypes } from 'react'; +import { Map, fromJS } from 'immutable'; import ImmutablePropTypes from 'react-immutable-proptypes'; import { resolveWidget } from '../Widgets'; +import ControlHOC from '../Widgets/ControlHOC'; import styles from './ControlPane.css'; function isHidden(field) { @@ -8,28 +10,47 @@ function isHidden(field) { } export default class ControlPane extends Component { + componentValidate = {}; + processControlRef(fieldName, wrappedControl) { + if (!wrappedControl) return; + this.componentValidate[fieldName] = wrappedControl.validate; + } + + validate = () => { + this.props.fields.forEach(field => this.componentValidate[field.get("name")]()); + }; controlFor(field) { - const { entry, fieldsMetaData, getAsset, onChange, onAddAsset, onRemoveAsset } = this.props; + const { entry, fieldsMetaData, fieldsErrors, getAsset, onChange, onAddAsset, onRemoveAsset } = this.props; const widget = resolveWidget(field.get('widget')); const fieldName = field.get('name'); const value = entry.getIn(['data', fieldName]); const metadata = fieldsMetaData.get(fieldName); + const errors = fieldsErrors.get(fieldName); + const labelClass = errors ? styles.labelWithError : styles.label; if (entry.size === 0 || entry.get('partial') === true) return null; return (
- - { - React.createElement(widget.control, { - field, - value, - metadata, - onChange: (newValue, newMetadata) => onChange(fieldName, newValue, newMetadata), - onAddAsset, - onRemoveAsset, - getAsset, - }) - } + + + onChange(fieldName, newValue, newMetadata)} + onValidate={this.props.onValidate.bind(this, fieldName)} + onAddAsset={onAddAsset} + onRemoveAsset={onRemoveAsset} + getAsset={getAsset} + ref={this.processControlRef.bind(this, fieldName)} + />
); } @@ -60,8 +81,10 @@ ControlPane.propTypes = { entry: ImmutablePropTypes.map.isRequired, fields: ImmutablePropTypes.list.isRequired, fieldsMetaData: ImmutablePropTypes.map.isRequired, + fieldsErrors: ImmutablePropTypes.map.isRequired, getAsset: PropTypes.func.isRequired, onAddAsset: PropTypes.func.isRequired, onChange: PropTypes.func.isRequired, + onValidate: PropTypes.func.isRequired, onRemoveAsset: PropTypes.func.isRequired, }; diff --git a/src/components/EntryEditor/EntryEditor.js b/src/components/EntryEditor/EntryEditor.js index b3e6617d..ac68bffc 100644 --- a/src/components/EntryEditor/EntryEditor.js +++ b/src/components/EntryEditor/EntryEditor.js @@ -20,17 +20,23 @@ class EntryEditor extends Component { this.setState({ showEventBlocker: false }); }; + handleOnPersist = () => { + this.controlPaneRef.validate(); + this.props.onPersist(); + }; + render() { const { collection, entry, fields, fieldsMetaData, + fieldsErrors, getAsset, onChange, + onValidate, onAddAsset, onRemoveAsset, - onPersist, onCancelEdit, } = this.props; @@ -53,10 +59,13 @@ class EntryEditor extends Component { entry={entry} fields={fields} fieldsMetaData={fieldsMetaData} + fieldsErrors={fieldsErrors} getAsset={getAsset} onChange={onChange} + onValidate={onValidate} onAddAsset={onAddAsset} onRemoveAsset={onRemoveAsset} + ref={c => this.controlPaneRef = c} // eslint-disable-line /> @@ -76,7 +85,7 @@ class EntryEditor extends Component {
@@ -91,9 +100,11 @@ EntryEditor.propTypes = { entry: ImmutablePropTypes.map.isRequired, fields: ImmutablePropTypes.list.isRequired, fieldsMetaData: ImmutablePropTypes.map.isRequired, + fieldsErrors: ImmutablePropTypes.map.isRequired, getAsset: PropTypes.func.isRequired, onAddAsset: PropTypes.func.isRequired, onChange: PropTypes.func.isRequired, + onValidate: PropTypes.func.isRequired, onPersist: PropTypes.func.isRequired, onRemoveAsset: PropTypes.func.isRequired, onCancelEdit: PropTypes.func.isRequired, diff --git a/src/components/Widgets/ControlHOC.js b/src/components/Widgets/ControlHOC.js new file mode 100644 index 00000000..602a948c --- /dev/null +++ b/src/components/Widgets/ControlHOC.js @@ -0,0 +1,92 @@ +import React, { Component, PropTypes } from 'react'; +import ImmutablePropTypes from "react-immutable-proptypes"; + +const truthy = () => ({ error: false }); + +class ControlHOC extends Component { + + static propTypes = { + controlComponent: PropTypes.func.isRequired, + field: ImmutablePropTypes.map.isRequired, + value: PropTypes.node, + metadata: ImmutablePropTypes.map, + onChange: PropTypes.func.isRequired, + onValidate: PropTypes.func.isRequired, + onAddAsset: PropTypes.func.isRequired, + onRemoveAsset: PropTypes.func.isRequired, + getAsset: PropTypes.func.isRequired, + }; + + processInnerControlRef = (wrappedControl) => { + if (!wrappedControl) return; + this.wrappedControlValid = wrappedControl.isValid || truthy; + }; + + validate = (skipWrapped = false) => { + const { field, value } = this.props; + const errors = []; + const validations = [this.validatePresence, this.validatePattern]; + validations.forEach((func) => { + const response = func(field, value); + if (response.error) errors.push(response.error); + }); + if (skipWrapped) { + if (skipWrapped.error) errors.push(skipWrapped.error); + } else { + const wrappedError = this.validateWrappedControl(field); + if (wrappedError.error) errors.push(wrappedError.error); + } + this.props.onValidate(errors); + }; + + validatePresence(field, value) { + const isRequired = field.get('required', true); + if (isRequired && (value === null || value.length === 0)) { + return { error: true }; + } + return { error: false }; + } + + validatePattern(field, value) { + const pattern = field.get('pattern', false); + if (pattern && !RegExp(pattern.first()).test(value)) { + return { error: `${ field.get('label', field.get('name')) } didn't match the pattern: ${ pattern.last() }` }; + } + return { error: false }; + } + + validateWrappedControl = (field) => { + const response = this.wrappedControlValid(); + if (typeof response === "boolean") { + const isValid = response; + return { error: (!isValid) }; + } else if (response.hasOwnProperty('error')) { + return response; + } else if (response instanceof Promise) { + response.then( + () => { this.validate({ error: false }); }, + (error) => { + this.validate({ error: `${ field.get('label', field.get('name')) } - ${ error }.` }); + } + ); + return { error: `${ field.get('label', field.get('name')) } is processing.` }; + } + return { error: false }; + }; + + render() { + const { controlComponent, field, value, metadata, onChange, onAddAsset, onRemoveAsset, getAsset } = this.props; + return React.createElement(controlComponent, { + field, + value, + metadata, + onChange, + onAddAsset, + onRemoveAsset, + getAsset, + ref: this.processInnerControlRef, + }); + } +} + +export default ControlHOC; diff --git a/src/components/Widgets/FileControl.js b/src/components/Widgets/FileControl.js index 46643e43..04ce1761 100644 --- a/src/components/Widgets/FileControl.js +++ b/src/components/Widgets/FileControl.js @@ -10,6 +10,16 @@ export default class FileControl extends React.Component { processing: false, }; + promise = null; + + isValid = () => { + if (this.promise) { + return this.promise; + } + return { error: false }; + }; + + handleFileInputRef = (el) => { this._fileInput = el; }; @@ -42,7 +52,7 @@ export default class FileControl extends React.Component { this.props.onRemoveAsset(this.props.value); if (file) { this.setState({ processing: true }); - createAssetProxy(file.name, file, false, this.props.field.get('private', false)) + this.promise = createAssetProxy(file.name, file, false, this.props.field.get('private', false)) .then((assetProxy) => { this.setState({ processing: false }); this.props.onAddAsset(assetProxy); diff --git a/src/components/Widgets/ImageControl.js b/src/components/Widgets/ImageControl.js index 0ccc46ee..5d9d7006 100644 --- a/src/components/Widgets/ImageControl.js +++ b/src/components/Widgets/ImageControl.js @@ -10,6 +10,16 @@ export default class ImageControl extends React.Component { processing: false, }; + promise = null; + + isValid = () => { + if (this.promise) { + return this.promise; + } + return { error: false }; + }; + + handleFileInputRef = (el) => { this._fileInput = el; }; @@ -46,7 +56,7 @@ export default class ImageControl extends React.Component { this.props.onRemoveAsset(this.props.value); if (file) { this.setState({ processing: true }); - createAssetProxy(file.name, file) + this.promise = createAssetProxy(file.name, file) .then((assetProxy) => { this.setState({ processing: false }); this.props.onAddAsset(assetProxy); diff --git a/src/containers/EntryPage.js b/src/containers/EntryPage.js index 67c727bd..22fe5b78 100644 --- a/src/containers/EntryPage.js +++ b/src/containers/EntryPage.js @@ -7,6 +7,7 @@ import { createEmptyDraft, discardDraft, changeDraftField, + changeDraftFieldValidation, persistEntry, } from '../actions/entries'; import { closeEntry } from '../actions/editor'; @@ -23,6 +24,7 @@ class EntryPage extends React.Component { addAsset: PropTypes.func.isRequired, boundGetAsset: PropTypes.func.isRequired, changeDraftField: PropTypes.func.isRequired, + changeDraftFieldValidation: PropTypes.func.isRequired, collection: ImmutablePropTypes.map.isRequired, createDraftFromEntry: PropTypes.func.isRequired, createEmptyDraft: PropTypes.func.isRequired, @@ -72,8 +74,8 @@ class EntryPage extends React.Component { }; handlePersistEntry = () => { - const { persistEntry, collection, entryDraft } = this.props; - persistEntry(collection, entryDraft); + const { persistEntry, collection } = this.props; + persistEntry(collection); }; render() { @@ -84,6 +86,7 @@ class EntryPage extends React.Component { boundGetAsset, collection, changeDraftField, + changeDraftFieldValidation, addAsset, removeAsset, closeEntry, @@ -104,7 +107,9 @@ class EntryPage extends React.Component { collection={collection} fields={fields} fieldsMetaData={entryDraft.get('fieldsMetaData')} + fieldsErrors={entryDraft.get('fieldsErrors')} onChange={changeDraftField} + onValidate={changeDraftFieldValidation} onAddAsset={addAsset} onRemoveAsset={removeAsset} onPersist={this.handlePersistEntry} @@ -138,6 +143,7 @@ export default connect( mapStateToProps, { changeDraftField, + changeDraftFieldValidation, addAsset, removeAsset, loadEntry, diff --git a/src/containers/editorialWorkflow/EntryPageHOC.js b/src/containers/editorialWorkflow/EntryPageHOC.js index 194a139f..c13a7b92 100644 --- a/src/containers/editorialWorkflow/EntryPageHOC.js +++ b/src/containers/editorialWorkflow/EntryPageHOC.js @@ -40,8 +40,8 @@ export default function EntryPageHOC(EntryPage) { }; // Overwrite persistEntry to persistUnpublishedEntry - returnObj.persistEntry = (collection, entryDraft) => { - dispatch(persistUnpublishedEntry(collection, entryDraft, unpublishedEntry)); + returnObj.persistEntry = (collection) => { + dispatch(persistUnpublishedEntry(collection, unpublishedEntry)); }; } diff --git a/src/reducers/__tests__/entryDraft.spec.js b/src/reducers/__tests__/entryDraft.spec.js index 75b3ad10..35fbe5fb 100644 --- a/src/reducers/__tests__/entryDraft.spec.js +++ b/src/reducers/__tests__/entryDraft.spec.js @@ -2,7 +2,7 @@ import { Map, List, fromJS } from 'immutable'; import * as actions from '../../actions/entries'; import reducer from '../entryDraft'; -let initialState = Map({ entry: Map(), mediaFiles: List(), fieldsMetaData: Map() }); +let initialState = Map({ entry: Map(), mediaFiles: List(), fieldsMetaData: Map(), fieldsErrors: Map() }); const entry = { collection: 'posts', @@ -30,6 +30,7 @@ describe('entryDraft reducer', () => { }, mediaFiles: [], fieldsMetaData: Map(), + fieldsErrors: Map(), }) ); }); @@ -50,6 +51,7 @@ describe('entryDraft reducer', () => { }, mediaFiles: [], fieldsMetaData: Map(), + fieldsErrors: Map(), }) ); }); diff --git a/src/reducers/entryDraft.js b/src/reducers/entryDraft.js index 3e9ab5b8..f19ca72e 100644 --- a/src/reducers/entryDraft.js +++ b/src/reducers/entryDraft.js @@ -4,6 +4,7 @@ import { DRAFT_CREATE_EMPTY, DRAFT_DISCARD, DRAFT_CHANGE_FIELD, + DRAFT_VALIDATION_ERRORS, ENTRY_PERSIST_REQUEST, ENTRY_PERSIST_SUCCESS, ENTRY_PERSIST_FAILURE, @@ -18,7 +19,7 @@ import { REMOVE_ASSET, } from '../actions/media'; -const initialState = Map({ entry: Map(), mediaFiles: List(), fieldsMetaData: Map() }); +const initialState = Map({ entry: Map(), mediaFiles: List(), fieldsMetaData: Map(), fieldsErrors: Map() }); const entryDraftReducer = (state = Map(), action) => { switch (action.type) { @@ -29,6 +30,7 @@ const entryDraftReducer = (state = Map(), action) => { state.setIn(['entry', 'newRecord'], false); state.set('mediaFiles', List()); state.set('fieldsMetaData', Map()); + state.set('fieldsErrors', Map()); }); case DRAFT_CREATE_EMPTY: // New Entry @@ -37,6 +39,7 @@ const entryDraftReducer = (state = Map(), action) => { state.setIn(['entry', 'newRecord'], true); state.set('mediaFiles', List()); state.set('fieldsMetaData', Map()); + state.set('fieldsErrors', Map()); }); case DRAFT_DISCARD: return initialState; @@ -45,6 +48,14 @@ const entryDraftReducer = (state = Map(), action) => { state.setIn(['entry', 'data', action.payload.field], action.payload.value); state.mergeIn(['fieldsMetaData'], fromJS(action.payload.metadata)); }); + + case DRAFT_VALIDATION_ERRORS: + if (action.payload.errors.length === 0) { + return state.deleteIn(['fieldsErrors', action.payload.field]); + } else { + return state.setIn(['fieldsErrors', action.payload.field], action.payload.errors); + } + case ENTRY_PERSIST_REQUEST: case UNPUBLISHED_ENTRY_PERSIST_REQUEST: { return state.setIn(['entry', 'isPersisting'], true);