From 87e7142f1fa325e700407933d22c36aef1b7f0cf Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Mon, 24 Jul 2017 18:26:09 -0600 Subject: [PATCH 1/3] Revert "Update parameter elements. Rework validateParam() function. Added .btn-sm class for "Add item" and "Remove item" buttons in array parameters. Reduce border-width on + ) @@ -122,7 +121,6 @@ export class JsonSchema_array extends PureComponent { render() { let { getComponent, required, schema, fn } = this.props - let errors = schema.errors || [] let itemSchema = fn.inferSchema(schema.items) const JsonSchemaForm = getComponent("JsonSchemaForm") @@ -133,17 +131,19 @@ export class JsonSchema_array extends PureComponent { if ( enumValue ) { const Select = getComponent("Select") - return () } } diff --git a/src/core/utils.js b/src/core/utils.js index 7cc5beda..f2e28948 100644 --- a/src/core/utils.js +++ b/src/core/utils.js @@ -468,18 +468,6 @@ export const validateFile = ( val ) => { } } -export const validateBoolean = ( val ) => { - if ( !(val === "true" || val === "false" || val === true || val === false) ) { - return "Value must be a boolean" - } -} - -export const validateString = ( val ) => { - if ( val && typeof val !== "string" ) { - return "Value must be a string" - } -} - // validation of parameters before execute export const validateParam = (param, isXml) => { let errors = [] @@ -487,66 +475,52 @@ export const validateParam = (param, isXml) => { let required = param.get("required") let type = param.get("type") - // If the parameter is required OR the parameter has a value (meaning optional, but filled in) - // then we should do our validation routine - if ( required || value ) { - // These checks should evaluate to true if the parameter's value is valid - let stringCheck = type === "string" && value && !validateString(value) - let arrayCheck = type === "array" && Array.isArray(value) && value.length - let listCheck = type === "array" && Im.List.isList(value) && value.count() - let fileCheck = type === "file" && value instanceof win.File - let booleanCheck = type === "boolean" && !validateBoolean(value) - let numberCheck = type === "number" && !validateNumber(value) // validateNumber returns undefined if the value is a number - let integerCheck = type === "integer" && !validateInteger(value) // validateInteger returns undefined if the value is an integer + let stringCheck = type === "string" && !value + let arrayCheck = type === "array" && Array.isArray(value) && !value.length + let listCheck = type === "array" && Im.List.isList(value) && !value.count() + let fileCheck = type === "file" && !(value instanceof win.File) - if ( required && !(stringCheck || arrayCheck || listCheck || fileCheck || booleanCheck || numberCheck || integerCheck) ) { - errors.push("Required field is not provided") - return errors - } + if ( required && (stringCheck || arrayCheck || listCheck || fileCheck) ) { + errors.push("Required field is not provided") + return errors + } - if ( type === "string" ) { - let err = validateString(value) - if (!err) return errors - errors.push(err) - } else if ( type === "boolean" ) { - let err = validateBoolean(value) - if (!err) return errors - errors.push(err) - } else if ( type === "number" ) { - let err = validateNumber(value) - if (!err) return errors - errors.push(err) - } else if ( type === "integer" ) { - let err = validateInteger(value) - if (!err) return errors - errors.push(err) - } else if ( type === "array" ) { - let itemType + if ( value === null || value === undefined ) { + return errors + } - if ( !value.count() ) { return errors } + if ( type === "number" ) { + let err = validateNumber(value) + if (!err) return errors + errors.push(err) + } else if ( type === "integer" ) { + let err = validateInteger(value) + if (!err) return errors + errors.push(err) + } else if ( type === "array" ) { + let itemType - itemType = param.getIn(["items", "type"]) + if ( !value.count() ) { return errors } - value.forEach((item, index) => { - let err + itemType = param.getIn(["items", "type"]) - if (itemType === "number") { - err = validateNumber(item) - } else if (itemType === "integer") { - err = validateInteger(item) - } else if (itemType === "string") { - err = validateString(item) - } + value.forEach((item, index) => { + let err - if ( err ) { - errors.push({ index: index, error: err}) - } - }) - } else if ( type === "file" ) { - let err = validateFile(value) - if (!err) return errors - errors.push(err) - } + if (itemType === "number") { + err = validateNumber(item) + } else if (itemType === "integer") { + err = validateInteger(item) + } + + if ( err ) { + errors.push({ index: index, error: err}) + } + }) + } else if ( type === "file" ) { + let err = validateFile(value) + if (!err) return errors + errors.push(err) } return errors diff --git a/src/style/_buttons.scss b/src/style/_buttons.scss index bfb85023..2470f8b0 100644 --- a/src/style/_buttons.scss +++ b/src/style/_buttons.scss @@ -14,11 +14,6 @@ @include text_headline(); - &.btn-sm { - font-size: 12px; - padding: 4px 23px; - } - &[disabled] { cursor: not-allowed; @@ -170,9 +165,6 @@ button { cursor: pointer; - outline: none; - &.invalid { - @include invalidFormElement(); - } + outline: none; } diff --git a/src/style/_form.scss b/src/style/_form.scss index e54d5438..185b836e 100644 --- a/src/style/_form.scss +++ b/src/style/_form.scss @@ -21,10 +21,6 @@ select background: #f7f7f7; } - - &.invalid { - @include invalidFormElement(); - } } .opblock-body select @@ -57,8 +53,12 @@ input[type=file] border-radius: 4px; background: #fff; - &.invalid { - @include invalidFormElement(); + &.invalid + { + animation: shake .4s 1; + + border-color: $_color-delete; + background: lighten($_color-delete, 35%); } } diff --git a/src/style/_mixins.scss b/src/style/_mixins.scss index 8fd01720..a2a27d48 100644 --- a/src/style/_mixins.scss +++ b/src/style/_mixins.scss @@ -166,9 +166,3 @@ $browser-context: 16; @warn 'Breakpoint mixin supports: tablet, mobile, desktop'; } } - -@mixin invalidFormElement() { - animation: shake .4s 1; - border-color: $_color-delete; - background: lighten($_color-delete, 35%); -} diff --git a/src/style/_table.scss b/src/style/_table.scss index d1481709..3d58f3d8 100644 --- a/src/style/_table.scss +++ b/src/style/_table.scss @@ -97,10 +97,6 @@ table width: 100%; max-width: 340px; } - - select { - border-width: 1px; - } } .parameter__name diff --git a/test/core/utils.js b/test/core/utils.js index acc5a14e..fbfb24ab 100644 --- a/test/core/utils.js +++ b/test/core/utils.js @@ -176,7 +176,6 @@ describe("utils", function() { let result = null it("validates required strings", function() { - // invalid string param = fromJS({ required: true, type: "string", @@ -184,39 +183,9 @@ describe("utils", function() { }) result = validateParam( param, false ) expect( result ).toEqual( ["Required field is not provided"] ) - - // valid string - param = fromJS({ - required: true, - type: "string", - value: "test string" - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - }) - - it("validates optional strings", function() { - // valid (empty) string - param = fromJS({ - required: false, - type: "string", - value: "" - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - - // valid string - param = fromJS({ - required: false, - type: "string", - value: "test" - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) }) it("validates required files", function() { - // invalid file param = fromJS({ required: true, type: "file", @@ -224,48 +193,9 @@ describe("utils", function() { }) result = validateParam( param, false ) expect( result ).toEqual( ["Required field is not provided"] ) - - // valid file - param = fromJS({ - required: true, - type: "file", - value: new win.File() - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - }) - - it("validates optional files", function() { - // invalid file - param = fromJS({ - required: false, - type: "file", - value: "not a file" - }) - result = validateParam( param, false ) - expect( result ).toEqual( ["Value must be a file"] ) - - // valid (empty) file - param = fromJS({ - required: false, - type: "file", - value: undefined - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - - // valid file - param = fromJS({ - required: false, - type: "file", - value: new win.File() - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) }) it("validates required arrays", function() { - // invalid (empty) array param = fromJS({ required: true, type: "array", @@ -274,51 +204,75 @@ describe("utils", function() { result = validateParam( param, false ) expect( result ).toEqual( ["Required field is not provided"] ) - // invalid (not an array) param = fromJS({ required: true, type: "array", - value: undefined + value: [] }) result = validateParam( param, false ) expect( result ).toEqual( ["Required field is not provided"] ) + }) - // invalid array, items do not match correct type + it("validates numbers", function() { + // string instead of a number param = fromJS({ - required: true, - type: "array", - value: [1], - items: { - type: "string" - } + required: false, + type: "number", + value: "test" }) result = validateParam( param, false ) - expect( result ).toEqual( [{index: 0, error: "Value must be a string"}] ) + expect( result ).toEqual( ["Value must be a number"] ) - // valid array, with no 'type' for items + // undefined value param = fromJS({ - required: true, - type: "array", - value: ["1"] + required: false, + type: "number", + value: undefined }) result = validateParam( param, false ) expect( result ).toEqual( [] ) - // valid array, items match type + // null value param = fromJS({ - required: true, - type: "array", - value: ["1"], - items: { - type: "string" - } + required: false, + type: "number", + value: null }) result = validateParam( param, false ) expect( result ).toEqual( [] ) }) - it("validates optional arrays", function() { - // valid, empty array + it("validates integers", function() { + // string instead of integer + param = fromJS({ + required: false, + type: "integer", + value: "test" + }) + result = validateParam( param, false ) + expect( result ).toEqual( ["Value must be an integer"] ) + + // undefined value + param = fromJS({ + required: false, + type: "integer", + value: undefined + }) + result = validateParam( param, false ) + expect( result ).toEqual( [] ) + + // null value + param = fromJS({ + required: false, + type: "integer", + value: null + }) + result = validateParam( param, false ) + expect( result ).toEqual( [] ) + }) + + it("validates arrays", function() { + // empty array param = fromJS({ required: false, type: "array", @@ -327,7 +281,7 @@ describe("utils", function() { result = validateParam( param, false ) expect( result ).toEqual( [] ) - // invalid, items do not match correct type + // numbers param = fromJS({ required: false, type: "array", @@ -339,209 +293,17 @@ describe("utils", function() { result = validateParam( param, false ) expect( result ).toEqual( [{index: 0, error: "Value must be a number"}] ) - // valid + // integers param = fromJS({ required: false, type: "array", - value: ["test"], + value: ["not", "numbers"], items: { - type: "string" + type: "integer" } }) result = validateParam( param, false ) - expect( result ).toEqual( [] ) - }) - - it("validates required booleans", function() { - // invalid boolean value - param = fromJS({ - required: true, - type: "boolean", - value: undefined - }) - result = validateParam( param, false ) - expect( result ).toEqual( ["Required field is not provided"] ) - - // invalid boolean value (not a boolean) - param = fromJS({ - required: true, - type: "boolean", - value: "test string" - }) - result = validateParam( param, false ) - expect( result ).toEqual( ["Required field is not provided"] ) - - // valid boolean value - param = fromJS({ - required: true, - type: "boolean", - value: "true" - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - - // valid boolean value - param = fromJS({ - required: true, - type: "boolean", - value: false - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - }) - - it("validates optional booleans", function() { - // valid (empty) boolean value - param = fromJS({ - required: false, - type: "boolean", - value: undefined - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - - // invalid boolean value (not a boolean) - param = fromJS({ - required: false, - type: "boolean", - value: "test string" - }) - result = validateParam( param, false ) - expect( result ).toEqual( ["Value must be a boolean"] ) - - // valid boolean value - param = fromJS({ - required: false, - type: "boolean", - value: "true" - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - - // valid boolean value - param = fromJS({ - required: false, - type: "boolean", - value: false - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - }) - - it("validates required numbers", function() { - // invalid number, string instead of a number - param = fromJS({ - required: true, - type: "number", - value: "test" - }) - result = validateParam( param, false ) - expect( result ).toEqual( ["Required field is not provided"] ) - - // invalid number, undefined value - param = fromJS({ - required: true, - type: "number", - value: undefined - }) - result = validateParam( param, false ) - expect( result ).toEqual( ["Required field is not provided"] ) - - // valid number - param = fromJS({ - required: true, - type: "number", - value: 10 - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - }) - - it("validates optional numbers", function() { - // invalid number, string instead of a number - param = fromJS({ - required: false, - type: "number", - value: "test" - }) - result = validateParam( param, false ) - expect( result ).toEqual( ["Value must be a number"] ) - - // valid (empty) number - param = fromJS({ - required: false, - type: "number", - value: undefined - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - - // valid number - param = fromJS({ - required: false, - type: "number", - value: 10 - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - }) - - it("validates required integers", function() { - // invalid integer, string instead of an integer - param = fromJS({ - required: true, - type: "integer", - value: "test" - }) - result = validateParam( param, false ) - expect( result ).toEqual( ["Required field is not provided"] ) - - // invalid integer, undefined value - param = fromJS({ - required: true, - type: "integer", - value: undefined - }) - result = validateParam( param, false ) - expect( result ).toEqual( ["Required field is not provided"] ) - - // valid integer - param = fromJS({ - required: true, - type: "integer", - value: 10 - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - }) - - it("validates optional integers", function() { - // invalid integer, string instead of an integer - param = fromJS({ - required: false, - type: "integer", - value: "test" - }) - result = validateParam( param, false ) - expect( result ).toEqual( ["Value must be an integer"] ) - - // valid (empty) integer - param = fromJS({ - required: false, - type: "integer", - value: undefined - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) - - // valid number - param = fromJS({ - required: false, - type: "integer", - value: 10 - }) - result = validateParam( param, false ) - expect( result ).toEqual( [] ) + expect( result ).toEqual( [{index: 0, error: "Value must be an integer"}, {index: 1, error: "Value must be an integer"}] ) }) }) From f570ffcd822cea9fdb4e76a70156b01854d6768b Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Mon, 24 Jul 2017 18:46:30 -0600 Subject: [PATCH 2/3] Reimplemented changes from PR #3427. Added fix for validating request bodies. --- src/core/components/layout-utils.jsx | 7 +- src/core/json-schema-components.js | 24 +- src/core/utils.js | 107 +++++--- src/style/_buttons.scss | 12 +- src/style/_form.scss | 9 +- src/style/_mixins.scss | 6 + src/style/_table.scss | 4 + test/core/utils.js | 353 +++++++++++++++++++++++---- 8 files changed, 412 insertions(+), 110 deletions(-) diff --git a/src/core/components/layout-utils.jsx b/src/core/components/layout-utils.jsx index 00a4a09d..f80e0c90 100644 --- a/src/core/components/layout-utils.jsx +++ b/src/core/components/layout-utils.jsx @@ -129,7 +129,8 @@ export class Select extends React.Component { value: PropTypes.any, onChange: PropTypes.func, multiple: PropTypes.bool, - allowEmptyValue: PropTypes.bool + allowEmptyValue: PropTypes.bool, + className: PropTypes.string } static defaultProps = { @@ -142,7 +143,7 @@ export class Select extends React.Component { let value - if (props.value !== undefined) { + if (props.value) { value = props.value } else { value = props.multiple ? [""] : "" @@ -178,7 +179,7 @@ export class Select extends React.Component { let value = this.state.value.toJS ? this.state.value.toJS() : this.state.value return ( - { allowEmptyValue ? : null } { allowedValues.map(function (item, key) { diff --git a/src/core/json-schema-components.js b/src/core/json-schema-components.js index bf4ae514..a8d92023 100644 --- a/src/core/json-schema-components.js +++ b/src/core/json-schema-components.js @@ -57,7 +57,8 @@ export class JsonSchema_string extends Component { if ( enumValue ) { const Select = getComponent("Select") - return () } - let errors = schema.errors || [] - return (
- { !value || value.count() < 1 ? - (errors.length ? { errors[0] } : null) : + { !value || value.count() < 1 ? null : value.map( (item,i) => { let schema = Object.assign({}, itemSchema) if ( errors.length ) { @@ -153,12 +153,12 @@ export class JsonSchema_array extends PureComponent { return (
this.onItemChange(val, i)} schema={schema} /> - +
) }).toArray() } - +
) } @@ -170,12 +170,14 @@ export class JsonSchema_boolean extends Component { onEnumChange = (val) => this.props.onChange(val) render() { - let { getComponent, required, value } = this.props + let { getComponent, value, schema } = this.props + let errors = schema.errors || [] const Select = getComponent("Select") - return ( +