Commit 357cece1 authored by Florent Chehab's avatar Florent Chehab

feat(form errors):

* Form level errors now go down to the fields in an awesome manner !
* Improved type hints in Field and Form

Fixes #70
parent 6724a80c
Pipeline #37967 passed with stages
in 4 minutes and 58 seconds
...@@ -8,7 +8,6 @@ import IconButton from "@material-ui/core/IconButton"; ...@@ -8,7 +8,6 @@ import IconButton from "@material-ui/core/IconButton";
import Typography from "@material-ui/core/Typography"; import Typography from "@material-ui/core/Typography";
import CloseIcon from "@material-ui/icons/Close"; import CloseIcon from "@material-ui/icons/Close";
import Alert from "../common/Alert"; import Alert from "../common/Alert";
// Form is imported only for type hints // Form is imported only for type hints
// eslint-disable-next-line no-unused-vars // eslint-disable-next-line no-unused-vars
import Form from "../form/Form"; import Form from "../form/Form";
...@@ -50,7 +49,7 @@ class Editor extends Component { ...@@ -50,7 +49,7 @@ class Editor extends Component {
* Extra attributes that should be retrieved in rawModelData when parsing * Extra attributes that should be retrieved in rawModelData when parsing
* I.e. the fields that are not present in the form but still need to be there * I.e. the fields that are not present in the form but still need to be there
* *
* @memberof Editor * @type {Array.<string>}
*/ */
extraFieldMappings = []; extraFieldMappings = [];
...@@ -58,7 +57,6 @@ class Editor extends Component { ...@@ -58,7 +57,6 @@ class Editor extends Component {
* Creates an instance of Editor. and subscribes to the module wrapper so that it can access its functions. * Creates an instance of Editor. and subscribes to the module wrapper so that it can access its functions.
* *
* @param {object} props * @param {object} props
* @memberof Editor
*/ */
constructor(props) { constructor(props) {
super(props); super(props);
...@@ -75,7 +73,6 @@ class Editor extends Component { ...@@ -75,7 +73,6 @@ class Editor extends Component {
* Plut the id. * Plut the id.
* *
* @returns * @returns
* @memberof Editor
*/ */
parseRawModelData() { parseRawModelData() {
let out = {}; let out = {};
...@@ -97,7 +94,6 @@ class Editor extends Component { ...@@ -97,7 +94,6 @@ class Editor extends Component {
* Returns the form instance associated with the editor * Returns the form instance associated with the editor
* *
* @returns {Form} * @returns {Form}
* @memberof Editor
*/ */
getForm() { getForm() {
return this.formRef.current; return this.formRef.current;
...@@ -125,7 +121,6 @@ class Editor extends Component { ...@@ -125,7 +121,6 @@ class Editor extends Component {
* Function to handle save editor events, eg when clicking on the save button * Function to handle save editor events, eg when clicking on the save button
* This function is not trivial and checks are performed. * This function is not trivial and checks are performed.
* *
* @memberof Editor
*/ */
handleSaveEditorRequest() { handleSaveEditorRequest() {
const getFormError = this.getFormError(); const getFormError = this.getFormError();
...@@ -143,7 +138,7 @@ class Editor extends Component { ...@@ -143,7 +138,7 @@ class Editor extends Component {
} }
} else { } else {
this.notifyFormHasErrors(getFormError.messages); this.notifyFormHasErrors();
} }
} }
...@@ -152,7 +147,6 @@ class Editor extends Component { ...@@ -152,7 +147,6 @@ class Editor extends Component {
* Function to save the `data` to the server. * Function to save the `data` to the server.
* *
* @param {Object} data * @param {Object} data
* @memberof Editor
*/ */
performSave(data) { performSave(data) {
this.notifyIsSaving(); this.notifyIsSaving();
...@@ -168,7 +162,6 @@ class Editor extends Component { ...@@ -168,7 +162,6 @@ class Editor extends Component {
* This enables not to use weird save detection method. * This enables not to use weird save detection method.
* *
* @param {object} newData object returned by the server * @param {object} newData object returned by the server
* @memberof Editor
*/ */
handleSaveRequestWasSuccessful(newData) { handleSaveRequestWasSuccessful(newData) {
// We check if data was moderated // We check if data was moderated
...@@ -194,7 +187,6 @@ class Editor extends Component { ...@@ -194,7 +187,6 @@ class Editor extends Component {
* Function to handle close editor request from the user. * Function to handle close editor request from the user.
* It checks if there is data to save or not. * It checks if there is data to save or not.
* *
* @memberof Editor
*/ */
handleCloseEditorRequest() { handleCloseEditorRequest() {
if (this.formHasChanges()) { if (this.formHasChanges()) {
...@@ -208,7 +200,6 @@ class Editor extends Component { ...@@ -208,7 +200,6 @@ class Editor extends Component {
* Effectively close the editor window and notify if there was something new that was saved * Effectively close the editor window and notify if there was something new that was saved
* *
* @param {Boolean} somethingWasSaved * @param {Boolean} somethingWasSaved
* @memberof Editor
*/ */
closeEditor(somethingWasSaved) { closeEditor(somethingWasSaved) {
this.props.closeFullScreenDialog(); this.props.closeFullScreenDialog();
...@@ -222,7 +213,6 @@ class Editor extends Component { ...@@ -222,7 +213,6 @@ class Editor extends Component {
* - Detecting when there was a successful save * - Detecting when there was a successful save
* - etc. * - etc.
* *
* @memberof Editor
*/ */
componentDidUpdate() { componentDidUpdate() {
...@@ -246,7 +236,6 @@ class Editor extends Component { ...@@ -246,7 +236,6 @@ class Editor extends Component {
* Basically something that extends the `Form` component * Basically something that extends the `Form` component
* *
* @returns * @returns
* @memberof Editor
*/ */
renderForm() { renderForm() {
throw new Error("The renderForm function must be extended in sub classes"); throw new Error("The renderForm function must be extended in sub classes");
...@@ -258,7 +247,6 @@ class Editor extends Component { ...@@ -258,7 +247,6 @@ class Editor extends Component {
* fullscreen app dialog through redux. * fullscreen app dialog through redux.
* *
* @returns * @returns
* @memberof Editor
*/ */
renderEditor() { renderEditor() {
const {classes} = this.props; const {classes} = this.props;
...@@ -347,8 +335,8 @@ class Editor extends Component { ...@@ -347,8 +335,8 @@ class Editor extends Component {
this.props.enqueueSnackbar("Aucun changement n'a été repéré.", {variant: "info"}); this.props.enqueueSnackbar("Aucun changement n'a été repéré.", {variant: "info"});
} }
notifyFormHasErrors(errors) { notifyFormHasErrors() {
this.props.enqueueSnackbar(`Le formulaire semble incohérent, merci de vérifier son contenu. ${errors}`, {variant: "error"}); this.props.enqueueSnackbar("Le formulaire semble incohérent, merci de vérifier son contenu.", {variant: "error"});
} }
notifyIsSaving() { notifyIsSaving() {
......
import { Component } from "react"; import {Component} from "react";
import PropTypes from "prop-types"; import PropTypes from "prop-types";
import areSameObjects from "../../utils/areSameObjects"; import areSameObjects from "../../utils/areSameObjects";
import renderFieldsMixIn from "./renderFieldsMixIn"; import renderFieldsMixIn from "./renderFieldsMixIn";
import CustomError from "../common/CustomError"; import CustomError from "../common/CustomError";
/**
* Function to build a chached version of the fields mapping to cascade update.
*
* @param {Array.<FormLevelError>} formLevelErrors
* @returns {Map<string, Set>}
*/
function buildFieldUpdateCache(formLevelErrors) {
let out = new Map();
formLevelErrors.map(e => e.getFields())
.forEach(fieldList =>
fieldList.forEach(field => {
if (!out.has(field)) {
out.set(field, new Set());
}
fieldList
.filter(linkedField => linkedField !== field)
.forEach(linkedField => {
out.set(field, out.get(field).add(linkedField));
});
}));
return out;
}
/** /**
* React component that should contain `Field` instances. * React component that should contain `Field` instances.
* Custom function have been implemented to ease form handling. *
* It has a ton of custom behavior implemented in it. `Fields` and `Form`
* are depending on each other with some custom subscribing behavior.
* *
* @class Form * @class Form
* @extends {React.Component} * @extends {React.Component}
*/ */
class Form extends Component { class Form extends Component {
// Store it as an object to make sure we don't have issues
// with resetting it for some reason /**
*
*
* handling of the interconnection between the fields and the form
*
*
*/
/**
* YOU SHOULDN'T OVERRIDE THIS
*
* We store it as an object to make sure we don't have issues
* with resetting it for some reason
* @type {object.<string, Field>}
*/
fields = Object(); fields = Object();
/**
* Array containing the possible form level errors
* @abstract
* @type {Array.<FormLevelError>}
*/
formLevelErrors = [];
/** /**
* YOU SHOULDN'T OVERRIDE THIS
*
* This method MUST be used on all field inside a `Form` instance. * This method MUST be used on all field inside a `Form` instance.
* *
* Function that returns the value corresponding to `fieldMapping` and * Function that returns the value corresponding to `fieldMapping` and
...@@ -25,123 +76,196 @@ class Form extends Component { ...@@ -25,123 +76,196 @@ class Form extends Component {
* *
* @param {string} fieldMapping * @param {string} fieldMapping
* @param {function} [convertValue=v => v] Method applied to the value from the modelData to get "the value" * @param {function} [convertValue=v => v] Method applied to the value from the modelData to get "the value"
* @memberof Form
*/ */
getReferenceAndValue(fieldMapping, convertValue = v => v) { getReferenceAndValue(fieldMapping, convertValue = v => v) {
// using react ref was to complicated with ref forwarding not working with withStyles. // using react ref was to complicated with ref forwarding not working with withStyles.
return { value: convertValue(this.props.modelData[fieldMapping]), form: this, fieldMapping }; return {value: convertValue(this.props.modelData[fieldMapping]), form: this, fieldMapping};
} }
/**
*
*
*
* General Utils
*
*
*
*/
/** /**
* YOU SHOULDN'T OVERRIDE THIS
*
* Function to be used in fields so that they subscribe to a form * Function to be used in fields so that they subscribe to a form
* *
* Using ref to field is not working with withStyles ref forwarding issues. * Using ref to field is not working with withStyles ref forwarding issues.
* *
* @param {string} fieldMapping * @param {string} fieldMapping
* @param {Field} field * @param {Field} field
* @memberof Form
*/ */
fieldSubscribe(fieldMapping, field) { fieldSubscribe(fieldMapping, field) {
this.fields[fieldMapping] = field; this.fields[fieldMapping] = field;
} }
/** /**
* YOU SHOULDN'T OVERRIDE THIS
*
* Function that returns the fields contained in the form * Function that returns the fields contained in the form
* as an array of {fieldMapping: string, field: Field} * as an array of {fieldMapping: string, field: Field}
* *
* Works only if the `getReferenceAndValue` was used on the Field props and the field subscribed. * Works only if the `getReferenceAndValue` was used on the Field props and the field subscribed.
* *
* @returns {Array} * @returns {{fieldMapping: string, field: Field}[]}
* @memberof Form
*/ */
getFields() { getFields() {
return Object.keys(this.fields) return Object.keys(this.fields)
.map(fieldMapping => .map(fieldMapping =>
({ fieldMapping, field: this.fields[fieldMapping] }) ({fieldMapping, field: this.fields[fieldMapping]})
); );
} }
/** /**
* YOU SHOULDN'T OVERRIDE THIS
*
* Function to look if there has been changes compared to the data
* this is already saved.
*
* @returns {Boolean}
*/
hasChanges() {
const formData = this.getDataFromFields(),
modelData = this.props.modelData;
return Object.keys(formData).some(fieldMapping => {
const cmp1 = formData[fieldMapping],
cmp2 = modelData[fieldMapping];
// we need to compare objects (ie JSON objects) differently
if (typeof cmp1 === "object") {
return !areSameObjects(cmp1, cmp2);
} else {
return cmp1 !== cmp2;
}
});
}
/**
*
*
*
*
* Error handling
*
*
*
*/
/**
* YOU SHOULDN'T OVERRIDE THIS
*
* Returns an object containing with {fieldMapping: valueInField} * Returns an object containing with {fieldMapping: valueInField}
* *
* @returns {Object} * @returns {object.<string, any>}
* @memberof Form
*/ */
getDataFromFields() { getDataFromFields() {
return this.getFields() return this.getFields()
.reduce( .reduce(
(acc, { field, fieldMapping }) => { (acc, {field, fieldMapping}) => {
acc[fieldMapping] = field.getValue(); acc[fieldMapping] = field.getValue();
return acc; return acc;
}, },
Object()); Object());
} }
/**
* YOU SHOULDN'T OVERRIDE THIS
*
* Returns the error object corresponding to the errors **from the form** for a field.
*
* @param {string} fieldMapping
* @returns {CustomError}
*/
getErrorForField(fieldMapping) {
const formData = this.getDataFromFields();
const errorMessages = this.formLevelErrors
.filter(formLevelError => formLevelError.getFields().includes(fieldMapping))
.filter(formLevelError => formLevelError.hasError(formData))
.map(formLevelError => formLevelError.getMessage());
return new CustomError(errorMessages);
}
/**
* YOU SHOULDN'T OVERRIDE THIS
*
* Function to call once a field has been updated, to trigger a rerendering of all the fields
* linked by a form level error handling.
* @param {string} fieldMapping
*/
fieldUpdated(fieldMapping) {
const fieldsToUpdate = this.getFieldsToUpdate();
if (fieldsToUpdate.has(fieldMapping)) {
fieldsToUpdate
.get(fieldMapping)
.forEach(field => this.fields[field].forceUpdate());
}
}
/**
* YOU SHOULDN'T OVERRIDE THIS
*
* Helper function to cache the map containin the mapping between a field(name) and the set of fields
* that should be updated when the previous one is updated.
* @returns {Map<string, Set>}
*/
getFieldsToUpdate() {
if (!this.fieldsToUpdate) {
this.fieldsToUpdate = buildFieldUpdateCache(this.formLevelErrors);
}
return this.fieldsToUpdate;
}
/** /**
* YOU SHOULDN'T OVERRIDE THIS
* Function to build all the errors from the fields of the form. * Function to build all the errors from the fields of the form.
* *
* @returns * @returns {CustomError}
* @memberof Form
*/ */
fieldsHaveError() { getFieldsErrors() {
return CustomError.superCombine( return CustomError.superCombine(
this.getFields() this.getFields()
.map(({ field }) => field.getError()) .map(({field}) => field.getError())
); );
} }
/** /**
* YOU SHOULDN'T OVERRIDE THIS
* Function to know if the form has errors either at the field level * Function to know if the form has errors either at the field level
* Or when running checks that combine fields * Or when running checks that combine fields
* *
* YOU SHOULDN'T OVERRIDE THIS * @returns {CustomError}
*
* @returns
* @memberof Form
*/ */
getError() { getError() {
return this.fieldsHaveError() return this.getFieldsErrors()
.combine(this.hasFormLevelErrors()); .combine(this.hasFormLevelErrors());
} }
/** /**
* YOU SHOULDN'T OVERRIDE THIS
* Function to check for errors in the form * Function to check for errors in the form
* *
* Can be override in sub classes
*
* @virtual
* @returns {CustomError} * @returns {CustomError}
* @memberof Form
*/ */
hasFormLevelErrors() { hasFormLevelErrors() {
return new CustomError([]); // default: no errors const formData = this.getDataFromFields();
} const errors = this.formLevelErrors
.filter(formLevelError => formLevelError.hasError(formData))
.map(formLevelError => formLevelError.getMessage());
/** return new CustomError(errors);
* Function to look if there has been changes compared to the data
* this is already saved.
*
* @returns {Boolean}
* @memberof Form
*/
hasChanges() {
const formData = this.getDataFromFields(),
modelData = this.props.modelData;
return Object.keys(formData).some(fieldMapping => {
const cmp1 = formData[fieldMapping],
cmp2 = modelData[fieldMapping];
// we need to compare objects (ie JSON objects) differently
if (typeof cmp1 === "object") {
return !areSameObjects(cmp1, cmp2);
} else {
return cmp1 !== cmp2;
}
});
} }
} }
...@@ -150,7 +274,6 @@ class Form extends Component { ...@@ -150,7 +274,6 @@ class Form extends Component {
Object.assign(Form.prototype, renderFieldsMixIn); Object.assign(Form.prototype, renderFieldsMixIn);
Form.propTypes = { Form.propTypes = {
modelData: PropTypes.object.isRequired, modelData: PropTypes.object.isRequired,
outsideData: PropTypes.object, outsideData: PropTypes.object,
......
/**
* Class to handle easily form level errors
*/
export default class FormLevelError {
/**
* @param {Array.<string>} fields Array of the field concerned by the error.
* @param {function(**): boolean} check Function that performs the check of the fields. The values of the `fields` are passed as parameters and the function is expected to return a boolean.
* @param {string} message Message for the error in case the check doesn't pass.
*/
constructor(fields, check, message) {
this.fields = fields;
this.check = check;
this.message = message;
}
/**
* Do we currently have an error ?
* @param {object} formData data from the form
* @returns {boolean}
*/
hasError(formData) {
return this.check(...this.fields.map(fieldMapping => formData[fieldMapping]));
}
/**
* Get the message associated with the error
* @returns {string}
*/
getMessage() {
return this.message;
}
/**
* Return an array of string corresponding to the field name.
* @returns {Array<string>}
*/
getFields() {
return this.fields;
}
}
import React, { PureComponent } from "react"; import React, {PureComponent} from "react";
import PropTypes from "prop-types"; import PropTypes from "prop-types";
import Form from "../Form"; import Form from "../Form";
import FieldWrapper from "./FieldWrapper"; import FieldWrapper from "./FieldWrapper";
...@@ -18,18 +18,17 @@ class Field extends PureComponent { ...@@ -18,18 +18,17 @@ class Field extends PureComponent {
defaultNullValue = undefined; defaultNullValue = undefined;
/** /**
*Creates an instance of Field. * Creates an instance of Field.
* @param {object} props * @param {object} props
* @param {object} [customStateAttrs={}] add custom state attributes on creation * @param {object} [customStateAttrs={}] add custom state attributes on creation
* @memberof Field
*/ */
constructor(props, customStateAttrs = {}) { constructor(props, customStateAttrs = {}) {
super(props); super(props);
// make sure to subscribe ! IMPORTANT // make sure to subscribe to the form ! IMPORTANT
props.form.fieldSubscribe(props.fieldMapping, this); props.form.fieldSubscribe(props.fieldMapping, this);
let { value } = props; let {value} = props;
this.state = { this.state = {
value: value, value: value,
...customStateAttrs, ...customStateAttrs,
...@@ -41,14 +40,17 @@ class Field extends PureComponent { ...@@ -41,14 +40,17 @@ class Field extends PureComponent {
* *
* @override * @override
* @param {object} newState * @param {object} newState
* @memberof Field
*/ */
setState(newState) { setState(newState) {
let state = Object.assign({}, newState); let state = Object.assign({}, newState);
if (typeof this.defaultNullValue !== "undefined") { if (typeof this.defaultNullValue !== "undefined") {
Object.assign(state, { value: newState.value === this.defaultNullValue ? null : newState.value }); Object.assign(state, {value: newState.value === this.defaultNullValue ? null : newState.value});
} }
super.setState(newState); const {form, fieldMapping} = this.props;
super.setState(newState, () => {
form.fieldUpdated(fieldMapping);
});
} }
/** /**
...@@ -56,19 +58,28 @@ class Field extends PureComponent { ...@@ -56,19 +58,28 @@ class Field extends PureComponent {
* *
* @virtual * @virtual
* @returns {CustomError} * @returns {CustomError}
* @memberof Field
*/ */
getError() { getError() {
throw Error("This methods has to be override in sub classes"); throw Error("This methods has to be override in sub classes");
} }
/**
* YOU SHOULDN'T OVERRIDE THIS
*
* Return the errors from the field and the errors from the form corresponding to the field.
* @returns {CustomError}
*/
getAllErrors() {
const {form, fieldMapping} = this.props;
return this.getError().combine(form.getErrorForField(fieldMapping));
}
/** /**