From 3d6c120eac3413e38a6c55bfd2d700eaa5aa3b88 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 10:41:44 +1100 Subject: [PATCH 01/43] COMPASS 235: DV - Full range component Start by turning on the range component in the GUI again. Also unskip tests that only work properly when integrated due to the nature of the test API we have built. --- .../validation/lib/components/rule-categories/index.js | 2 +- test/validation.store.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-categories/index.js b/src/internal-packages/validation/lib/components/rule-categories/index.js index 2bf2e4dcbaa..df37f88507c 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/index.js +++ b/src/internal-packages/validation/lib/components/rule-categories/index.js @@ -2,6 +2,6 @@ module.exports = { exists: require('./exists'), mustNotExist: require('./mustnotexist'), type: require('./type'), - // range: require('./range'), // work in progress + range: require('./range'), regex: require('./regex') }; diff --git a/test/validation.store.test.js b/test/validation.store.test.js index bab5dff478f..c0c2f703469 100644 --- a/test/validation.store.test.js +++ b/test/validation.store.test.js @@ -843,7 +843,7 @@ describe('ValidationStore', function() { }); describe('Range Rule when passing in values that are valid on the server', function() { - context.skip('values accepted by the rule builder UI', function() { + context('values accepted by the rule builder UI', function() { it('accepts {$gte: 21}', function() { const validatorDoc = { 'validator': { From f888ae6645b6b8277811e652df0b85d9fb7a2c7f Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Fri, 28 Oct 2016 15:45:04 +1100 Subject: [PATCH 02/43] Add JSDoc of proposed coupling between RangeInput validationStates --- .../validation/lib/components/common/range-input.jsx | 12 +++++++++++- .../lib/components/rule-categories/range.jsx | 4 ++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 0e50e806310..f14f8b2a883 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -9,6 +9,16 @@ const MenuItem = require('react-bootstrap').MenuItem; // const debug = require('debug')('mongodb-compass:validation:action-selector'); +/** + * A RangeInput represents a numeric lower or upper bound and the value of the + * lower or upper bound, if the bound exists. + * + * The `validationState` of this RangeInput can be set to 'error' either: + * - by the RangeInput validating the `value` prop is not of type `number`, or + * - by the RangeInput's parent `RuleCategoryRange` component validating the + * combined range expression, such as `5 < x < 5` is rejected as invalid + * even though the RangeInputs of `5 < x` and `5 > x` are individually valid. + */ class RangeInput extends React.Component { constructor(props) { @@ -141,7 +151,7 @@ class RangeInput extends React.Component { const placeholder = `enter ${boundString}`.toLowerCase(); return ( - +
{boundString}
diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index 9a9d0e49ae1..60cbd3d7510 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -99,12 +99,16 @@ class RuleCategoryRange extends React.Component { boundIncluded={this.props.parameters.lowerBoundType === '$gte'} disabled={this.props.parameters.lowerBoundType === null} value={this.props.parameters.lowerBoundValue} + onChange={this.onBoundOpChanged.bind(this)} + validationState={null} />
); From 1169102da195aafc070ae14393825f2e5018625b Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 14:33:28 +1100 Subject: [PATCH 03/43] Add RuleCategoryRange unit and Rule integration tests --- test/validation.range.test.js | 23 +++++++++++++++++++++++ test/validation.rule.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/test/validation.range.test.js b/test/validation.range.test.js index 084fef566aa..9d99718b5d9 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -3,12 +3,14 @@ const chai = require('chai'); const chaiEnzyme = require('chai-enzyme'); const expect = chai.expect; const React = require('react'); +const _ = require('lodash'); const shallow = require('enzyme').shallow; const bootstrap = require('react-bootstrap'); const ControlLabel = bootstrap.ControlLabel; const FormControl = bootstrap.FormControl; const RangeInput = require('../src/internal-packages/validation/lib/components/common/range-input'); +const RuleCategoryRange = require('../src/internal-packages/validation/lib/components/rule-categories/range'); chai.use(chaiEnzyme()); @@ -39,3 +41,24 @@ describe('', () => { }); }); }); + +describe('', function() { + let component; + const propsTemplate = { + id: 'my-new-rule', + field: 'created_at', + category: 'range', + parameters: { + lowerBoundValue: -5, + upperBoundValue: 5 + }, + nullable: false + }; + + it('has two child components', function() { + const props = _.assign(propsTemplate, {}); + component = shallow(); + const ranges = component.dive().find(RangeInput); + expect(ranges).to.have.length(2); + }); +}); diff --git a/test/validation.rule.test.js b/test/validation.rule.test.js index db5b57fada7..625966c4c9a 100644 --- a/test/validation.rule.test.js +++ b/test/validation.rule.test.js @@ -5,7 +5,10 @@ const expect = chai.expect; const React = require('react'); const mount = require('enzyme').mount; +const shallow = require('enzyme').shallow; const Rule = require('../src/internal-packages/validation/lib/components/rule'); +const RuleCategoryRange = require('../src/internal-packages/validation/lib/components/rule-categories/range'); +const RuleCategorySelector = require('../src/internal-packages/validation/lib/components/rule-category-selector'); const _ = require('lodash'); // const debug = require('debug')('compass:validation:test'); @@ -84,4 +87,28 @@ describe('', function() { expect(component.find('input.nullable')).to.be.checked(); }); }); + + context('when category "range" is supplied', function() { + const rangeRuleTemplate = { + id: 'my-new-rule', + field: 'created_at', + category: 'range', + parameters: {type: 9}, // type "date" + nullable: false + }; + + it('has a category of "range"', function() { + const rule = _.assign(rangeRuleTemplate, {}); + component = shallow(
); + const ruleCategory = component.find(Rule).dive().find(RuleCategorySelector); + expect(ruleCategory.props().category).to.be.equal('range'); + }); + + it('has a component with id my-new-rule', function() { + const rule = _.assign(rangeRuleTemplate, {}); + component = shallow(
); + const ruleCategory = component.find(Rule).dive().find(RuleCategoryRange); + expect(ruleCategory.props().id).to.be.equal('my-new-rule'); + }); + }); }); From e435d616fe1ca49319c740830a9f94dcb73275b1 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 15:18:03 +1100 Subject: [PATCH 04/43] Update RangeInput.value to no longer be isRequired Fixes warning with 'none' dropdown state: /Users/pzrq/Projects/compass/node_modules/fbjs/lib/warning.js:36 Warning: Failed prop type: Required prop `value` was not specified in `RangeInput`. in RangeInput (created by RuleCategoryRange) in RuleCategoryRange (created by Rule) in form (created by Form) in Form (created by Rule) in td (created by Rule) in tr (created by Rule) in Rule (created by RuleBuilder) in tbody (created by RuleBuilder) in table (created by Table) in Table (created by RuleBuilder) in div (created by Col) in Col (created by RuleBuilder) in div (created by Row) in Row (created by RuleBuilder) in div (created by Grid) in Grid (created by Editable) in div (created by Editable) in div (created by Editable) in div (created by Editable) in Editable (created by RuleBuilder) in RuleBuilder (created by Validation) in div (created by Grid) in Grid (created by Validation) in div (created by Validation) in Validation (created by ConnectedValidation) in StoreConnector (created by ConnectedValidation) in ConnectedValidationprintWarning @ /Users/pzrq/Projects/compass/node_modules/fbjs/lib/warning.js:36 --- .../validation/lib/components/common/range-input.jsx | 2 +- test/validation.range.test.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index f14f8b2a883..3eeba44e06f 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -177,7 +177,7 @@ class RangeInput extends React.Component { } RangeInput.propTypes = { - value: React.PropTypes.number.isRequired, + value: React.PropTypes.number, // Can't be required to allow "none" in GUI upperBound: React.PropTypes.bool, validationState: React.PropTypes.string, boundIncluded: React.PropTypes.bool.isRequired, diff --git a/test/validation.range.test.js b/test/validation.range.test.js index 9d99718b5d9..fcf0720fe04 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -17,12 +17,12 @@ chai.use(chaiEnzyme()); describe('', () => { context('when rendering the default control', () => { it('has label `LOWER BOUND`', () => { - const component = shallow(); + const component = shallow(); const labelText = component.find(ControlLabel).dive().text(); expect(labelText).to.be.equal('LOWER BOUND'); }); it('has placeholder text of `enter lower bound`', () => { - const component = shallow(); + const component = shallow(); const placeholderText = component.find(FormControl).props().placeholder; expect(placeholderText).to.be.equal('enter lower bound'); }); @@ -30,12 +30,12 @@ describe('', () => { context('when rendering an upperBound control', () => { it('has label `UPPER BOUND`', () => { - const component = shallow(); + const component = shallow(); const labelText = component.find(ControlLabel).dive().text(); expect(labelText).to.be.equal('UPPER BOUND'); }); it('has placeholder text of `enter upper bound`', () => { - const component = shallow(); + const component = shallow(); const placeholderText = component.find(FormControl).props().placeholder; expect(placeholderText).to.be.equal('enter upper bound'); }); From c97bfa9820928112a0c4e110e2c4b4c3f09509b1 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 15:03:46 +1100 Subject: [PATCH 05/43] Add failing test for the empty range state --- test/validation.range.test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/validation.range.test.js b/test/validation.range.test.js index fcf0720fe04..7c25d56b454 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -61,4 +61,19 @@ describe('', function() { const ranges = component.dive().find(RangeInput); expect(ranges).to.have.length(2); }); + + it('accepts empty range 5 < x < 5 but with validationState error', function() { + const props = _.assign(propsTemplate, { + lowerBoundType: '$gt', + lowerBoundValue: 5, + upperBoundType: '$lt', + upperBoundValue: 5 + }); + component = shallow(); + const ranges = component.dive().find(RangeInput); + expect(ranges).to.have.length(2); + ranges.forEach(range => { + expect(range.props().validationState).to.be.equal('error'); + }); + }); }); From 091816396caa32340c80f3fe4487fbb7e623c0f4 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 17:23:40 +1100 Subject: [PATCH 06/43] Mostly working, but for some reason renders the stale value... --- .../lib/components/common/range-input.jsx | 6 ++- .../lib/components/rule-categories/range.jsx | 45 +++++++++++++------ 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 3eeba44e06f..876c0d4bb49 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -17,7 +17,7 @@ const MenuItem = require('react-bootstrap').MenuItem; * - by the RangeInput validating the `value` prop is not of type `number`, or * - by the RangeInput's parent `RuleCategoryRange` component validating the * combined range expression, such as `5 < x < 5` is rejected as invalid - * even though the RangeInputs of `5 < x` and `5 > x` are individually valid. + * even though the RangeInputs `5 < x` and `5 > x` are individually valid. */ class RangeInput extends React.Component { @@ -57,6 +57,8 @@ class RangeInput extends React.Component { onInputBlur() { this.validate(); + // Get the parent to update both RangeInput component states + return this.props.onRangeInputBlur(); } onDropdownSelect(evtKey) { @@ -182,7 +184,7 @@ RangeInput.propTypes = { validationState: React.PropTypes.string, boundIncluded: React.PropTypes.bool.isRequired, disabled: React.PropTypes.bool.isRequired, - onChange: React.PropTypes.func, + onRangeInputBlur: React.PropTypes.func, width: React.PropTypes.number }; diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index 60cbd3d7510..b1cfa1e34e7 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -9,17 +9,32 @@ const FormGroup = bootstrap.FormGroup; class RuleCategoryRange extends React.Component { - onBoundOpChanged(eventKey) { - const params = this.props.parameters; - switch (eventKey) { - case '$lte': // fall through - case '$lt': params.upperBoundType = eventKey; break; - case '$gte': // fall through - case '$gt': params.lowerBoundType = eventKey; break; - case 'none-upper': params.upperBoundType = null; break; - case 'none-lower': params.lowerBoundType = null; break; - default: break; + onRangeInputBlur() { + const opMap = { + '>=': '$gte', + '>': '$gt', + '<=': '$lte', + '<': '$lt', + }; + let params = RuleCategoryRange.getInitialParameters(); + // Use refs to get child state, as children don't have a unique ID + // http://stackoverflow.com/a/29303324 + const lowerBoundState = this.refs.lowerBoundRangeInputChild.state; + const upperBoundState = this.refs.upperBoundRangeInputChild.state; + if (Object.keys(opMap).includes(lowerBoundState.operator)) { + params.lowerBoundType = opMap[lowerBoundState.operator]; + params.lowerBoundValue = parseFloat(lowerBoundState.value); + } else { + params.lowerBoundType = null; + } + if (Object.keys(opMap).includes(upperBoundState.operator)) { + params.upperBoundType = opMap[upperBoundState.operator]; + params.upperBoundValue = parseFloat(upperBoundState.value); + } else { + params.upperBoundType = null; } + + // Trigger an action that should update the Reflux store ValidationAction.setRuleParameters(this.props.id, params); } @@ -34,10 +49,10 @@ class RuleCategoryRange extends React.Component { static paramsToQuery(params) { const result = {}; - if (params.upperBoundType !== null) { + if (params.upperBoundType) { result[params.upperBoundType] = params.upperBoundValue; } - if (params.lowerBoundType !== null) { + if (params.lowerBoundType) { result[params.lowerBoundType] = params.lowerBoundValue; } return result; @@ -96,18 +111,20 @@ class RuleCategoryRange extends React.Component { return ( From 4a96c33a14ef2834454c44aeb5fcf4e12dbe395d Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 18:38:14 +1100 Subject: [PATCH 07/43] Refactor componentWillMount into the constructor https://facebook.github.io/react/docs/react-component.html#componentwillmount --- .../lib/components/common/range-input.jsx | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 876c0d4bb49..3cd814ea1ce 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -23,23 +23,15 @@ class RangeInput extends React.Component { constructor(props) { super(props); + const op = this._getOperatorString(props); this.state = { - disabled: false, - operator: '', - value: '', + disabled: op === 'none', + operator: op, + value: _.isNumber(this.props.value) ? String(this.props.value) : '', validationState: null }; } - componentWillMount() { - const op = this._getOperatorString(); - this.setState({ - value: _.isNumber(this.props.value) ? String(this.props.value) : '', - operator: op, - disabled: op === 'none' - }); - } - componentWillReceiveProps(nextProps) { const op = this._getOperatorString(nextProps); this.setState({ From 3e1e8dc2992c8da27af6fd0a7a1937dfe4c01cf1 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 18:38:59 +1100 Subject: [PATCH 08/43] Refactor onRangeInputBlur to the end of validate --- .../validation/lib/components/common/range-input.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 3cd814ea1ce..26d59ad4749 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -49,8 +49,6 @@ class RangeInput extends React.Component { onInputBlur() { this.validate(); - // Get the parent to update both RangeInput component states - return this.props.onRangeInputBlur(); } onDropdownSelect(evtKey) { @@ -85,6 +83,8 @@ class RangeInput extends React.Component { hasError: error }); } + // Get the parent to update both RangeInput component states + this.props.onRangeInputBlur(); } _getOperatorString(props) { From 311ccf7c05c2e235c4d2947feb81f9079b90901e Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 18:39:36 +1100 Subject: [PATCH 09/43] Remove 10 from parseFloat parseFloat does not take a second argument, unlike parseInt :) --- .../validation/lib/components/common/range-input.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 26d59ad4749..3826bb085d8 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -63,7 +63,7 @@ class RangeInput extends React.Component { } validate() { - const value = parseFloat(this.state.value, 10); + const value = parseFloat(this.state.value); let error = false; if (_.isNaN(value)) { error = true; From f14e53e516b543695a99894f766bbb252848b5a9 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 18:40:17 +1100 Subject: [PATCH 10/43] Drop unused RangeInput.onChange prop It can't possibly work with the combined range input unless you added multiple IDs somehow which would be a much bigger component refactoring. --- .../validation/lib/components/common/range-input.jsx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 3826bb085d8..2da9cfedc2c 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -75,14 +75,6 @@ class RangeInput extends React.Component { validationState: null }); } - if (this.props.onChange) { - this.props.onChange({ - disabled: this.state.disabled, - operator: this.state.operator, - value: value, - hasError: error - }); - } // Get the parent to update both RangeInput component states this.props.onRangeInputBlur(); } From d5efc0535f7316ebe2d695b9fb78c7aebae6a568 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 18:41:16 +1100 Subject: [PATCH 11/43] Be clearer which store --- .../validation/lib/components/rule-categories/range.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index b1cfa1e34e7..a1719af109d 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -34,7 +34,7 @@ class RuleCategoryRange extends React.Component { params.upperBoundType = null; } - // Trigger an action that should update the Reflux store + // Trigger an action that should update the Reflux ValidationStore ValidationAction.setRuleParameters(this.props.id, params); } From fb90307f29b332cea14db97ff90d23a0794b088a Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 18:57:44 +1100 Subject: [PATCH 12/43] Call validate after changing the dropdownOp value So it appears to start in an invalid state if switching from a 'none' state. --- .../validation/lib/components/common/range-input.jsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 2da9cfedc2c..2f046803da1 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -56,10 +56,7 @@ class RangeInput extends React.Component { disabled: evtKey === 'none', operator: evtKey }); - // need to defer validation until setState has propagated - // _.defer(() => { - // this.validate(); - // }); + this.validate(); } validate() { From 54d1a7559ff7957eb5d4853bdedeb697b972cff8 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 22:07:35 +1100 Subject: [PATCH 13/43] Remove componentWillReceiveProps It seems to confuse the component, leading to a bizarre cache issue. The best kind of bug fix - when deleting code makes things work better :) --- .../validation/lib/components/common/range-input.jsx | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 2f046803da1..aa038ca2013 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -32,15 +32,6 @@ class RangeInput extends React.Component { }; } - componentWillReceiveProps(nextProps) { - const op = this._getOperatorString(nextProps); - this.setState({ - value: _.isNumber(this.props.value) ? String(this.props.value) : '', - operator: op, - disabled: op === 'none' - }); - } - onInputChange(evt) { this.setState({ value: evt.target.value From 53d73be847bfe469ecd101110544e84c99603649 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 22:07:48 +1100 Subject: [PATCH 14/43] ESlint --- .../validation/lib/components/common/range-input.jsx | 2 -- .../validation/lib/components/rule-categories/range.jsx | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index aa038ca2013..12532c14d9c 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -52,9 +52,7 @@ class RangeInput extends React.Component { validate() { const value = parseFloat(this.state.value); - let error = false; if (_.isNaN(value)) { - error = true; this.setState({ validationState: 'error' }); diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index a1719af109d..e84aa3e065e 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -14,9 +14,9 @@ class RuleCategoryRange extends React.Component { '>=': '$gte', '>': '$gt', '<=': '$lte', - '<': '$lt', + '<': '$lt' }; - let params = RuleCategoryRange.getInitialParameters(); + const params = RuleCategoryRange.getInitialParameters(); // Use refs to get child state, as children don't have a unique ID // http://stackoverflow.com/a/29303324 const lowerBoundState = this.refs.lowerBoundRangeInputChild.state; From 3fc81156a20da337d85edb2638d8b88c21b69538 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 22:08:35 +1100 Subject: [PATCH 15/43] Fix race where validationState is not allowed to be empty string https://github.com/react-bootstrap/react-bootstrap/issues/1926 --- .../validation/lib/components/common/range-input.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 12532c14d9c..4fb8152c9dc 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -162,7 +162,7 @@ RangeInput.defaultProps = { disabled: false, boundIncluded: false, upperBound: false, - validationState: '', + validationState: null, value: null, width: 200 }; From 6a306a2f623bae32014e61532a2a521e447a4e26 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 22:19:42 +1100 Subject: [PATCH 16/43] Finally, playing with the (unvalidated) component works correctly --- .../validation/lib/components/common/range-input.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 4fb8152c9dc..dd0fcdaeb71 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -62,7 +62,7 @@ class RangeInput extends React.Component { }); } // Get the parent to update both RangeInput component states - this.props.onRangeInputBlur(); + _.defer(this.props.onRangeInputBlur); } _getOperatorString(props) { From 67b9a81f747f98ac87f40d5724b43fa2c6f9514d Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 22:23:48 +1100 Subject: [PATCH 17/43] Refactor into validateCombinedParams So I can reuse it shortly. --- .../lib/components/rule-categories/range.jsx | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index e84aa3e065e..aa8f5655a17 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -72,6 +72,22 @@ class RuleCategoryRange extends React.Component { return !isNaN(value) && Math.abs(value) !== Infinity; } + static validateCombinedParams(params) { + if (params.upperBoundType.length > 1 || params.lowerBoundType.length > 1) { + return false; + } + params.upperBoundType = params.upperBoundType[0] || null; + params.lowerBoundType = params.lowerBoundType[0] || null; + + // No documents could possibly satisfy these cases, e.g. 5 <= value < 5 + if (typeof(params.upperBoundValue) === 'number' && + typeof(params.lowerBoundValue) === 'number' && + params.upperBoundValue <= params.lowerBoundValue) { + return false; + } + return params; + } + static queryToParams(query) { // if not every key in the object is one of the comparison operators, // this rule cannot represent the query @@ -87,19 +103,7 @@ class RuleCategoryRange extends React.Component { lowerBoundValue: query.$gte || query.$gt || null, lowerBoundType: _.intersection(keys, ['$gte', '$gt']) }; - if (result.upperBoundType.length > 1 || result.lowerBoundType.length > 1) { - return false; - } - result.upperBoundType = result.upperBoundType[0] || null; - result.lowerBoundType = result.lowerBoundType[0] || null; - - // No documents could possibly satisfy these cases, e.g. 5 <= value < 5 - if (typeof(result.upperBoundValue) === 'number' && - typeof(result.lowerBoundValue) === 'number' && - result.upperBoundValue <= result.lowerBoundValue) { - return false; - } - return result; + return RuleCategoryRange.validateCombinedParams(result); } /** From e1f15b2b1e797a1e256cc045362b587240dabc94 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 23:17:48 +1100 Subject: [PATCH 18/43] Finally get comboValidationState working So we can put the common user error of mixing up the lower and upper bounds into the category of being reported by our Compass GUI, but still allow the user to proceed if they choose to. --- .../lib/components/rule-categories/range.jsx | 48 ++++++------ test/validation.range.test.js | 13 ++-- test/validation.store.test.js | 75 ++++++++++++++++--- 3 files changed, 98 insertions(+), 38 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index aa8f5655a17..d3a6832672f 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -33,6 +33,7 @@ class RuleCategoryRange extends React.Component { } else { params.upperBoundType = null; } + params.comboValidationState = RuleCategoryRange.getComboValidationState(params); // Trigger an action that should update the Reflux ValidationStore ValidationAction.setRuleParameters(this.props.id, params); @@ -47,15 +48,14 @@ class RuleCategoryRange extends React.Component { }; } - static paramsToQuery(params) { - const result = {}; - if (params.upperBoundType) { - result[params.upperBoundType] = params.upperBoundValue; - } - if (params.lowerBoundType) { - result[params.lowerBoundType] = params.lowerBoundValue; + static getComboValidationState(params) { + // No documents could possibly satisfy these cases, e.g. 5 <= value < 5 + if (typeof(params.upperBoundValue) === 'number' && + typeof(params.lowerBoundValue) === 'number' && + params.upperBoundValue <= params.lowerBoundValue) { + return 'error'; } - return result; + return null; } static validateKeyAndValue(key, value) { @@ -72,20 +72,15 @@ class RuleCategoryRange extends React.Component { return !isNaN(value) && Math.abs(value) !== Infinity; } - static validateCombinedParams(params) { - if (params.upperBoundType.length > 1 || params.lowerBoundType.length > 1) { - return false; + static paramsToQuery(params) { + const result = {}; + if (params.upperBoundType) { + result[params.upperBoundType] = params.upperBoundValue; } - params.upperBoundType = params.upperBoundType[0] || null; - params.lowerBoundType = params.lowerBoundType[0] || null; - - // No documents could possibly satisfy these cases, e.g. 5 <= value < 5 - if (typeof(params.upperBoundValue) === 'number' && - typeof(params.lowerBoundValue) === 'number' && - params.upperBoundValue <= params.lowerBoundValue) { - return false; + if (params.lowerBoundType) { + result[params.lowerBoundType] = params.lowerBoundValue; } - return params; + return result; } static queryToParams(query) { @@ -98,12 +93,19 @@ class RuleCategoryRange extends React.Component { return false; } const result = { + comboValidationState: null, upperBoundValue: query.$lte || query.$lt || null, upperBoundType: _.intersection(keys, ['$lte', '$lt']), lowerBoundValue: query.$gte || query.$gt || null, lowerBoundType: _.intersection(keys, ['$gte', '$gt']) }; - return RuleCategoryRange.validateCombinedParams(result); + if (result.upperBoundType.length > 1 || result.lowerBoundType.length > 1) { + return false; + } + result.upperBoundType = result.upperBoundType[0] || null; + result.lowerBoundType = result.lowerBoundType[0] || null; + result.comboValidationState = RuleCategoryRange.getComboValidationState(result); + return result; } /** @@ -120,7 +122,7 @@ class RuleCategoryRange extends React.Component { disabled={this.props.parameters.lowerBoundType === null} value={this.props.parameters.lowerBoundValue} onRangeInputBlur={this.onRangeInputBlur.bind(this)} - validationState={null} + validationState={this.props.parameters.comboValidationState} />
); diff --git a/test/validation.range.test.js b/test/validation.range.test.js index 7c25d56b454..526adbe908a 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -62,12 +62,15 @@ describe('', function() { expect(ranges).to.have.length(2); }); - it('accepts empty range 5 < x < 5 but with validationState error', function() { + it('accepts empty range 5 < x < 5 with getComboValidationState error', function() { const props = _.assign(propsTemplate, { - lowerBoundType: '$gt', - lowerBoundValue: 5, - upperBoundType: '$lt', - upperBoundValue: 5 + parameters: { + comboValidationState: 'error', + lowerBoundType: '$gt', + lowerBoundValue: 5, + upperBoundType: '$lt', + upperBoundValue: 5 + } }); component = shallow(); const ranges = component.dive().find(RangeInput); diff --git a/test/validation.store.test.js b/test/validation.store.test.js index c0c2f703469..b5b6d3b48b6 100644 --- a/test/validation.store.test.js +++ b/test/validation.store.test.js @@ -843,7 +843,7 @@ describe('ValidationStore', function() { }); describe('Range Rule when passing in values that are valid on the server', function() { - context('values accepted by the rule builder UI', function() { + context('values accepted with no error by the rule builder UI', function() { it('accepts {$gte: 21}', function() { const validatorDoc = { 'validator': { @@ -861,6 +861,7 @@ describe('ValidationStore', function() { field: 'age', nullable: false, parameters: { + 'comboValidationState': null, 'lowerBoundType': '$gte', 'lowerBoundValue': 21, 'upperBoundType': null, @@ -886,6 +887,7 @@ describe('ValidationStore', function() { field: 'age', nullable: false, parameters: { + 'comboValidationState': null, 'lowerBoundType': null, 'lowerBoundValue': null, 'upperBoundType': '$lt', @@ -912,6 +914,7 @@ describe('ValidationStore', function() { field: 'age', nullable: false, parameters: { + 'comboValidationState': null, 'lowerBoundType': '$gt', 'lowerBoundValue': 20, 'upperBoundType': '$lte', @@ -938,6 +941,7 @@ describe('ValidationStore', function() { field: 'age', nullable: false, parameters: { + 'comboValidationState': null, 'lowerBoundType': '$gt', 'lowerBoundValue': -20, 'upperBoundType': '$lte', @@ -948,10 +952,10 @@ describe('ValidationStore', function() { }); // Note: Server allows these cases, but we'd drop back to JSON view here - context('values rejected by the rule builder UI', function() { + context('values accepted with error by the rule builder UI', function() { // Only documents with value = 5 could be inserted, // which being a constant probably should be at the application layer - it('rejects equality constant range "5 <= value <= 5"', function() { + it('accepts equality constant range "5 <= value <= 5"', function() { const validatorDoc = { 'validator': { 'age': { @@ -963,11 +967,23 @@ describe('ValidationStore', function() { 'validationAction': 'error' }; const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - expect(result.rules).to.be.false; + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'comboValidationState': 'error', + 'lowerBoundType': '$gte', + 'lowerBoundValue': 5, + 'upperBoundType': '$lte', + 'upperBoundValue': 5 + } + }); }); // Bad as users couldn't insert any documents into the collection - it('rejects empty range "5 < value <= 5"', function() { + it('accepts empty range "5 < value <= 5"', function() { const validatorDoc = { 'validator': { 'age': { @@ -979,11 +995,23 @@ describe('ValidationStore', function() { 'validationAction': 'error' }; const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - expect(result.rules).to.be.false; + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'comboValidationState': 'error', + 'lowerBoundType': '$gt', + 'lowerBoundValue': 5, + 'upperBoundType': '$lte', + 'upperBoundValue': 5 + } + }); }); // Bad as users couldn't insert any documents into the collection - it('rejects empty range "5 <= value < 5"', function() { + it('accepts empty range "5 <= value < 5"', function() { const validatorDoc = { 'validator': { 'age': { @@ -995,11 +1023,23 @@ describe('ValidationStore', function() { 'validationAction': 'error' }; const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - expect(result.rules).to.be.false; + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'comboValidationState': 'error', + 'lowerBoundType': '$gte', + 'lowerBoundValue': 5, + 'upperBoundType': '$lt', + 'upperBoundValue': 5 + } + }); }); // Bad as users couldn't insert any documents into the collection - it('rejects empty range "6 < value < 5"', function() { + it('accepts empty range "6 < value < 5"', function() { const validatorDoc = { 'validator': { 'age': { @@ -1011,9 +1051,24 @@ describe('ValidationStore', function() { 'validationAction': 'error' }; const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - expect(result.rules).to.be.false; + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'comboValidationState': 'error', + 'lowerBoundType': '$gt', + 'lowerBoundValue': 6, + 'upperBoundType': '$lt', + 'upperBoundValue': 5 + } + }); }); + }); + // Note: Server allows these cases, but we'd drop back to JSON view here + context('values rejected by the rule builder UI', function() { // Better represented in GUI with the "None" operator drop down value it('rejects {$gte: -Infinity}', function() { const validatorDoc = { From c5d3d40bb57e7bf3a614cf3bb7602eedf2f84311 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Mon, 31 Oct 2016 23:45:30 +1100 Subject: [PATCH 19/43] Add a regex to warn the user they cannot enter expressions like 10+5 parseFloat will truncate them later, but the user doesn't see it unless they switch to JSON view or reload the page...this is at least a little better in that they are warned, not sure what the behaviour should really be though. Not sure how to unit test this one as it's deep in the validation logic so it would be somewhere deep in Enzyme if it is testable. --- .../validation/lib/components/common/range-input.jsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index dd0fcdaeb71..f59ba7befa7 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -52,7 +52,9 @@ class RangeInput extends React.Component { validate() { const value = parseFloat(this.state.value); - if (_.isNaN(value)) { + // Warn user they cannot enter NaN or expressions like `10+5`, + // as parseFloat will truncate the value `10+5` to just `10`. + if (_.isNaN(value) || !/^-?\d+\.?\d*$/.test(this.state.value)) { this.setState({ validationState: 'error' }); From b9f68ff8dae679ddef02b090abca67911bacd399 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Tue, 1 Nov 2016 00:14:35 +1100 Subject: [PATCH 20/43] Handle and test the 0 case correctly Thankfully the server already did, good thing I tried to record a demo video and check: > db.runCommand({collMod: 'validation', validator: {"age": {$gt:0, $lt:500}}}); { "ok" : 1 } > db.getCollectionInfos()[1].options.validator { "age" : { "$lt" : 500, "$gt" : 0 } } # Server also supports null which we don't because it gets mangled between my code and Thomas' code so meh until someone cares for it > db.runCommand({collMod: 'validation', validator: {"age": {$gt:null, $lt:500}}}); { "ok" : 1 } > db.getCollectionInfos()[1].options.validator { "age" : { "$gt" : null, "$lt" : 500 } } --- .../lib/components/rule-categories/range.jsx | 19 ++- test/validation.store.test.js | 108 ++++++++++++++++++ 2 files changed, 125 insertions(+), 2 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index d3a6832672f..87fd37f38f5 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -94,11 +94,26 @@ class RuleCategoryRange extends React.Component { } const result = { comboValidationState: null, - upperBoundValue: query.$lte || query.$lt || null, + upperBoundValue: null, upperBoundType: _.intersection(keys, ['$lte', '$lt']), - lowerBoundValue: query.$gte || query.$gt || null, + lowerBoundValue: null, lowerBoundType: _.intersection(keys, ['$gte', '$gt']) }; + + // Handle the 0 which is false-y case properly + if (_.isNumber(query.$lte)) { + result.upperBoundValue = query.$lte; + } + if (_.isNumber(query.$lt)) { + result.upperBoundValue = query.$lt; + } + if (_.isNumber(query.$gte)) { + result.lowerBoundValue = query.$gte; + } + if (_.isNumber(query.$gt)) { + result.lowerBoundValue = query.$gt; + } + if (result.upperBoundType.length > 1 || result.lowerBoundType.length > 1) { return false; } diff --git a/test/validation.store.test.js b/test/validation.store.test.js index b5b6d3b48b6..37e3755f031 100644 --- a/test/validation.store.test.js +++ b/test/validation.store.test.js @@ -949,6 +949,114 @@ describe('ValidationStore', function() { } }); }); + + it('accepts {$gt: 0, $lte: 5000}', function() { + const validatorDoc = { + 'validator': { + 'tree_age': { + '$gt': 0, + '$lte': 5000 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'tree_age', + nullable: false, + parameters: { + 'comboValidationState': null, + 'lowerBoundType': '$gt', + 'lowerBoundValue': 0, + 'upperBoundType': '$lte', + 'upperBoundValue': 5000 + } + }); + }); + + it('accepts {$gt: -5000000000, $lt: 0}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': -5000000000, + '$lt': 0 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'comboValidationState': null, + 'lowerBoundType': '$gt', + 'lowerBoundValue': -5000000000, + 'upperBoundType': '$lt', + 'upperBoundValue': 0 + } + }); + }); + + it('accepts {$gte: 0, $lt: 273.16}', function() { + const validatorDoc = { + 'validator': { + 'below_zero_celsius_in_kelvin': { + '$gte': 0, + '$lt': 273.16 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'below_zero_celsius_in_kelvin', + nullable: false, + parameters: { + 'comboValidationState': null, + 'lowerBoundType': '$gte', + 'lowerBoundValue': 0, + 'upperBoundType': '$lt', + 'upperBoundValue': 273.16 + } + }); + }); + + it('accepts {$gt: -273.16, $lte: 0}', function() { + const validatorDoc = { + 'validator': { + 'cold_temperature': { + '$gt': -273.16, + '$lte': 0 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'cold_temperature', + nullable: false, + parameters: { + 'comboValidationState': null, + 'lowerBoundType': '$gt', + 'lowerBoundValue': -273.16, + 'upperBoundType': '$lte', + 'upperBoundValue': 0 + } + }); + }); }); // Note: Server allows these cases, but we'd drop back to JSON view here From 770d7f79cbd895ed6a6d7b91a1584b15780b7a0a Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Tue, 1 Nov 2016 00:44:41 +1100 Subject: [PATCH 21/43] Improve comments This particular state is no longer rejected by the Rule Builder GUI, just shown highlighted in red as something we highly doubt a user would intentionally create, for the reasons outlined in `npm run ci` like the range being empty or easily hard-code-able as an application-level constant. --- .../validation/lib/components/common/range-input.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index f59ba7befa7..bc151ef2f76 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -16,7 +16,7 @@ const MenuItem = require('react-bootstrap').MenuItem; * The `validationState` of this RangeInput can be set to 'error' either: * - by the RangeInput validating the `value` prop is not of type `number`, or * - by the RangeInput's parent `RuleCategoryRange` component validating the - * combined range expression, such as `5 < x < 5` is rejected as invalid + * combined range expression, such as `5 < x < 5` is displayed as red/error * even though the RangeInputs `5 < x` and `5 > x` are individually valid. */ class RangeInput extends React.Component { From b59811d6c4f37586d9b5fa89171fef1d8e9f80ef Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Tue, 1 Nov 2016 11:11:49 +1100 Subject: [PATCH 22/43] Add better regex to handle exponential float forms --- .../validation/lib/components/common/range-input.jsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index bc151ef2f76..d6a721d75dc 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -54,7 +54,9 @@ class RangeInput extends React.Component { const value = parseFloat(this.state.value); // Warn user they cannot enter NaN or expressions like `10+5`, // as parseFloat will truncate the value `10+5` to just `10`. - if (_.isNaN(value) || !/^-?\d+\.?\d*$/.test(this.state.value)) { + // Also allow exponential forms like -1.7976931348623157e+308, but not ∞ + // http://stackoverflow.com/a/30987109/1101109 + if (_.isNaN(value) || !/^-?\d+\.?\d*([E|e][+|-]\d*)?$/.test(this.state.value)) { this.setState({ validationState: 'error' }); From 6608859903f53d209c3043192ea5cecbbe602507 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Tue, 1 Nov 2016 11:58:51 +1100 Subject: [PATCH 23/43] Fix translating regexes fail (the |, vertical bar ASCII code 0x7c should not be accepted) Though incoming hadron-type-checker should resolve this more completely (its DECIMAL_128_REGEX still accepts arbitrarily large exponents which it should not, but it's better than this one was). --- .../validation/lib/components/common/range-input.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index d6a721d75dc..2eec6b265a7 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -56,7 +56,7 @@ class RangeInput extends React.Component { // as parseFloat will truncate the value `10+5` to just `10`. // Also allow exponential forms like -1.7976931348623157e+308, but not ∞ // http://stackoverflow.com/a/30987109/1101109 - if (_.isNaN(value) || !/^-?\d+\.?\d*([E|e][+|-]\d*)?$/.test(this.state.value)) { + if (_.isNaN(value) || !/^-?\d+\.?\d*([Ee][+-]\d*)?$/.test(this.state.value)) { this.setState({ validationState: 'error' }); From ddf7a403d9902962280fd763835b54b4e1368022 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Tue, 1 Nov 2016 13:02:34 +1100 Subject: [PATCH 24/43] Add some Decimal128 tests --- test/validation.range.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/validation.range.test.js b/test/validation.range.test.js index 526adbe908a..34d430dfee8 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -26,6 +26,25 @@ describe('', () => { const placeholderText = component.find(FormControl).props().placeholder; expect(placeholderText).to.be.equal('enter lower bound'); }); + it('accepts a scientific decimal -9.001e+2', function() { + const value = '-9.0001e+2'; + const component = shallow(); + const props = component.find(FormControl).dive().props(); + expect(props.value).to.equal(value); + }); + // https://github.com/mongodb/mongo/blob/eb9810a/jstests/decimal/decimal128_test1.js + it('accepts tiniest Decimal128 9.999999999999999999999999999999999E-6143', function() { + const tiniest = '9.999999999999999999999999999999999E-6143'; + const component = shallow(); + const props = component.find(FormControl).dive().props(); + expect(props.value).to.equal(tiniest); + }); + it('accepts largest Decimal128 9.999999999999999999999999999999999E+6144', function() { + const largest = '9.999999999999999999999999999999999E+6144'; + const component = shallow(); + const props = component.find(FormControl).dive().props(); + expect(props.value).to.equal(largest); + }); }); context('when rendering an upperBound control', () => { From 8fe1b88f261575b4174220955f902b75a074f96d Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Tue, 1 Nov 2016 13:02:46 +1100 Subject: [PATCH 25/43] Handle Decimal128 and drop clunky parseFloat uses --- .../lib/components/common/range-input.jsx | 43 ++++++++++--- .../lib/components/rule-categories/range.jsx | 61 +++++++++++++++---- test/validation.range.test.js | 55 +++++++++++++++-- test/validation.store.test.js | 44 ++++++------- 4 files changed, 155 insertions(+), 48 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 2eec6b265a7..0c85903f49e 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -1,4 +1,5 @@ const React = require('react'); +const app = require('ampersand-app'); const _ = require('lodash'); const FormGroup = require('react-bootstrap').FormGroup; const InputGroup = require('react-bootstrap').InputGroup; @@ -6,9 +7,15 @@ const FormControl = require('react-bootstrap').FormControl; const DropdownButton = require('react-bootstrap').DropdownButton; const ControlLabel = require('react-bootstrap').ControlLabel; const MenuItem = require('react-bootstrap').MenuItem; +const TypeChecker = require('hadron-type-checker'); // const debug = require('debug')('mongodb-compass:validation:action-selector'); +/** + * The version at which high precision values are available. + */ +const HP_VERSION = '3.4.0'; + /** * A RangeInput represents a numeric lower or upper bound and the value of the * lower or upper bound, if the bound exists. @@ -27,9 +34,16 @@ class RangeInput extends React.Component { this.state = { disabled: op === 'none', operator: op, - value: _.isNumber(this.props.value) ? String(this.props.value) : '', + value: this.props.value, validationState: null }; + this._ENABLE_HP = app.instance && ( + app.instance.build.version >= HP_VERSION); + } + + componentWillMount() { + // Render any existing schema errors in red + this.validate(); } onInputChange(evt) { @@ -51,12 +65,18 @@ class RangeInput extends React.Component { } validate() { - const value = parseFloat(this.state.value); - // Warn user they cannot enter NaN or expressions like `10+5`, - // as parseFloat will truncate the value `10+5` to just `10`. - // Also allow exponential forms like -1.7976931348623157e+308, but not ∞ - // http://stackoverflow.com/a/30987109/1101109 - if (_.isNaN(value) || !/^-?\d+\.?\d*([Ee][+-]\d*)?$/.test(this.state.value)) { + const value = this.state.value; + const valueTypes = TypeChecker.castableTypes(value, this._ENABLE_HP); + + // Not sure if hadron-type-checker should make NUMBER_TYPES public + const NUMBER_TYPES = [ + 'Long', + 'Int32', + 'Double', + 'Decimal128' + ]; + + if (!_.intersection(valueTypes, NUMBER_TYPES).length) { this.setState({ validationState: 'error' }); @@ -66,7 +86,9 @@ class RangeInput extends React.Component { }); } // Get the parent to update both RangeInput component states - _.defer(this.props.onRangeInputBlur); + if (this.props.onRangeInputBlur) { + _.defer(this.props.onRangeInputBlur); + } } _getOperatorString(props) { @@ -153,7 +175,8 @@ class RangeInput extends React.Component { } RangeInput.propTypes = { - value: React.PropTypes.number, // Can't be required to allow "none" in GUI + value: React.PropTypes.string, // Can't be required to allow "none" in GUI, + // can't be number to work with Decimal128. upperBound: React.PropTypes.bool, validationState: React.PropTypes.string, boundIncluded: React.PropTypes.bool.isRequired, @@ -167,7 +190,7 @@ RangeInput.defaultProps = { boundIncluded: false, upperBound: false, validationState: null, - value: null, + value: '', width: 200 }; diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index 87fd37f38f5..126d6629d56 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -2,11 +2,18 @@ const React = require('react'); const _ = require('lodash'); const ValidationAction = require('../../actions'); const RangeInput = require('../common/range-input'); +const TypeChecker = require('hadron-type-checker'); +const app = require('ampersand-app'); const bootstrap = require('react-bootstrap'); const FormGroup = bootstrap.FormGroup; // const debug = require('debug')('mongodb-compass:validation'); +/** + * The version at which high precision values are available. + */ +const HP_VERSION = '3.4.0'; + class RuleCategoryRange extends React.Component { onRangeInputBlur() { @@ -23,13 +30,13 @@ class RuleCategoryRange extends React.Component { const upperBoundState = this.refs.upperBoundRangeInputChild.state; if (Object.keys(opMap).includes(lowerBoundState.operator)) { params.lowerBoundType = opMap[lowerBoundState.operator]; - params.lowerBoundValue = parseFloat(lowerBoundState.value); + params.lowerBoundValue = lowerBoundState.value; } else { params.lowerBoundType = null; } if (Object.keys(opMap).includes(upperBoundState.operator)) { params.upperBoundType = opMap[upperBoundState.operator]; - params.upperBoundValue = parseFloat(upperBoundState.value); + params.upperBoundValue = upperBoundState.value; } else { params.upperBoundType = null; } @@ -50,14 +57,32 @@ class RuleCategoryRange extends React.Component { static getComboValidationState(params) { // No documents could possibly satisfy these cases, e.g. 5 <= value < 5 - if (typeof(params.upperBoundValue) === 'number' && - typeof(params.lowerBoundValue) === 'number' && - params.upperBoundValue <= params.lowerBoundValue) { + if (params.upperBoundValue !== null && + params.lowerBoundValue !== null && + TypeChecker.cast( + params.upperBoundValue, + TypeChecker.castableTypes(params.upperBoundValue)[0] + ) + <= + TypeChecker.cast( + params.lowerBoundValue, + TypeChecker.castableTypes(params.lowerBoundValue)[0] + ) + ) { return 'error'; } return null; } + /** + * Are high precision values available? + * + * @returns {boolean} if high precision values are available. + */ + static isHighPrecision() { + return app.instance.build.version >= HP_VERSION; + } + static validateKeyAndValue(key, value) { if (!_.includes(['$gt', '$gte', '$lt', '$lte'], key)) { return false; @@ -72,13 +97,25 @@ class RuleCategoryRange extends React.Component { return !isNaN(value) && Math.abs(value) !== Infinity; } + static typeCastNumeric(value, serverVersion) { + // Override serverVersion for testing, otherwise I'd mock isHighPrecision + const highPrecision = ( + (typeof serverVersion === 'undefined' && RuleCategoryRange.isHighPrecision()) || + (serverVersion >= HP_VERSION) + ); + const castableTypes = TypeChecker.castableTypes(value, highPrecision); + // We rely on Double and Decimal128 being first in the list, + // which is fragile and hence is unit tested + return TypeChecker.cast(value, castableTypes[0]); + } + static paramsToQuery(params) { const result = {}; if (params.upperBoundType) { - result[params.upperBoundType] = params.upperBoundValue; + result[params.upperBoundType] = RuleCategoryRange.typeCastNumeric(params.upperBoundValue); } if (params.lowerBoundType) { - result[params.lowerBoundType] = params.lowerBoundValue; + result[params.lowerBoundType] = RuleCategoryRange.typeCastNumeric(params.lowerBoundValue); } return result; } @@ -100,18 +137,18 @@ class RuleCategoryRange extends React.Component { lowerBoundType: _.intersection(keys, ['$gte', '$gt']) }; - // Handle the 0 which is false-y case properly + // Handle the 0 which is false-y case properly and convert to a String type if (_.isNumber(query.$lte)) { - result.upperBoundValue = query.$lte; + result.upperBoundValue = query.$lte.toString(); } if (_.isNumber(query.$lt)) { - result.upperBoundValue = query.$lt; + result.upperBoundValue = query.$lt.toString(); } if (_.isNumber(query.$gte)) { - result.lowerBoundValue = query.$gte; + result.lowerBoundValue = query.$gte.toString(); } if (_.isNumber(query.$gt)) { - result.lowerBoundValue = query.$gt; + result.lowerBoundValue = query.$gt.toString(); } if (result.upperBoundType.length > 1 || result.lowerBoundType.length > 1) { diff --git a/test/validation.range.test.js b/test/validation.range.test.js index 34d430dfee8..d6356d932df 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -1,3 +1,4 @@ +/* eslint no-unused-expressions: 0 */ /* eslint no-unused-vars: 0 */ const chai = require('chai'); const chaiEnzyme = require('chai-enzyme'); @@ -68,8 +69,8 @@ describe('', function() { field: 'created_at', category: 'range', parameters: { - lowerBoundValue: -5, - upperBoundValue: 5 + lowerBoundValue: '-5', + upperBoundValue: '5' }, nullable: false }; @@ -86,9 +87,9 @@ describe('', function() { parameters: { comboValidationState: 'error', lowerBoundType: '$gt', - lowerBoundValue: 5, + lowerBoundValue: '5', upperBoundType: '$lt', - upperBoundValue: 5 + upperBoundValue: '5' } }); component = shallow(); @@ -98,4 +99,50 @@ describe('', function() { expect(range.props().validationState).to.be.equal('error'); }); }); + + context('for some different numeric types', function() { + const someInt32 = '32'; + const someLong = '9007199254740991'; // 2^53-1, higher nums => Decimal128 + const someDouble = '0.1'; + const someDecimal128 = '9.999999999999999999999999999999999E+6144'; + + context('when server version is 3.2.10', function() { + const serverVersion = '3.2.10'; + it('accepts Int32', function() { + const result = RuleCategoryRange.typeCastNumeric(someInt32, serverVersion); + expect(result._bsontype).to.be.equal('Int32'); + }); + it('accepts Long', function() { + const result = RuleCategoryRange.typeCastNumeric(someLong, serverVersion); + expect(result._bsontype).to.be.equal('Long'); + }); + it('accepts Double', function() { + const result = RuleCategoryRange.typeCastNumeric(someDouble, serverVersion); + expect(result._bsontype).to.be.equal('Double'); + }); + it('rejects Decimal128', function() { + const result = RuleCategoryRange.typeCastNumeric(someDecimal128, serverVersion); + expect(result._bsontype).to.be.undefined; + }); + }); + context('when server version is 3.4.0', function() { + const serverVersion = '3.4.0'; + it('accepts Int32', function() { + const result = RuleCategoryRange.typeCastNumeric(someInt32, serverVersion); + expect(result._bsontype).to.be.equal('Int32'); + }); + it('accepts Long', function() { + const result = RuleCategoryRange.typeCastNumeric(someLong, serverVersion); + expect(result._bsontype).to.be.equal('Long'); + }); + it('accepts Double', function() { + const result = RuleCategoryRange.typeCastNumeric(someDouble, serverVersion); + expect(result._bsontype).to.be.equal('Double'); + }); + it('accepts Decimal128', function() { + const result = RuleCategoryRange.typeCastNumeric(someDecimal128, serverVersion); + expect(result._bsontype).to.be.equal('Decimal128'); + }); + }); + }); }); diff --git a/test/validation.store.test.js b/test/validation.store.test.js index 37e3755f031..4d785b3c977 100644 --- a/test/validation.store.test.js +++ b/test/validation.store.test.js @@ -863,7 +863,7 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': null, 'lowerBoundType': '$gte', - 'lowerBoundValue': 21, + 'lowerBoundValue': '21', 'upperBoundType': null, 'upperBoundValue': null } @@ -891,7 +891,7 @@ describe('ValidationStore', function() { 'lowerBoundType': null, 'lowerBoundValue': null, 'upperBoundType': '$lt', - 'upperBoundValue': 21.123456789 + 'upperBoundValue': '21.123456789' } }); }); @@ -916,9 +916,9 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': null, 'lowerBoundType': '$gt', - 'lowerBoundValue': 20, + 'lowerBoundValue': '20', 'upperBoundType': '$lte', - 'upperBoundValue': 21 + 'upperBoundValue': '21' } }); }); @@ -943,9 +943,9 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': null, 'lowerBoundType': '$gt', - 'lowerBoundValue': -20, + 'lowerBoundValue': '-20', 'upperBoundType': '$lte', - 'upperBoundValue': -0.000001 + 'upperBoundValue': '-0.000001' } }); }); @@ -970,9 +970,9 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': null, 'lowerBoundType': '$gt', - 'lowerBoundValue': 0, + 'lowerBoundValue': '0', 'upperBoundType': '$lte', - 'upperBoundValue': 5000 + 'upperBoundValue': '5000' } }); }); @@ -997,9 +997,9 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': null, 'lowerBoundType': '$gt', - 'lowerBoundValue': -5000000000, + 'lowerBoundValue': '-5000000000', 'upperBoundType': '$lt', - 'upperBoundValue': 0 + 'upperBoundValue': '0' } }); }); @@ -1024,9 +1024,9 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': null, 'lowerBoundType': '$gte', - 'lowerBoundValue': 0, + 'lowerBoundValue': '0', 'upperBoundType': '$lt', - 'upperBoundValue': 273.16 + 'upperBoundValue': '273.16' } }); }); @@ -1051,9 +1051,9 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': null, 'lowerBoundType': '$gt', - 'lowerBoundValue': -273.16, + 'lowerBoundValue': '-273.16', 'upperBoundType': '$lte', - 'upperBoundValue': 0 + 'upperBoundValue': '0' } }); }); @@ -1083,9 +1083,9 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': 'error', 'lowerBoundType': '$gte', - 'lowerBoundValue': 5, + 'lowerBoundValue': '5', 'upperBoundType': '$lte', - 'upperBoundValue': 5 + 'upperBoundValue': '5' } }); }); @@ -1111,9 +1111,9 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': 'error', 'lowerBoundType': '$gt', - 'lowerBoundValue': 5, + 'lowerBoundValue': '5', 'upperBoundType': '$lte', - 'upperBoundValue': 5 + 'upperBoundValue': '5' } }); }); @@ -1139,9 +1139,9 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': 'error', 'lowerBoundType': '$gte', - 'lowerBoundValue': 5, + 'lowerBoundValue': '5', 'upperBoundType': '$lt', - 'upperBoundValue': 5 + 'upperBoundValue': '5' } }); }); @@ -1167,9 +1167,9 @@ describe('ValidationStore', function() { parameters: { 'comboValidationState': 'error', 'lowerBoundType': '$gt', - 'lowerBoundValue': 6, + 'lowerBoundValue': '6', 'upperBoundType': '$lt', - 'upperBoundValue': 5 + 'upperBoundValue': '5' } }); }); From 55978f3d2ce818b78d00817ec1979a45be102be7 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Tue, 1 Nov 2016 16:00:28 +1100 Subject: [PATCH 26/43] Be more explicit that combined ranges of Decimal128s are not validated --- .../validation/lib/components/rule-categories/range.jsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index 126d6629d56..988c66a6e85 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -56,17 +56,20 @@ class RuleCategoryRange extends React.Component { } static getComboValidationState(params) { + // Can't compare two Decimal128's for correctness easily in JS... + const highPrecision = false; + // No documents could possibly satisfy these cases, e.g. 5 <= value < 5 if (params.upperBoundValue !== null && params.lowerBoundValue !== null && TypeChecker.cast( params.upperBoundValue, - TypeChecker.castableTypes(params.upperBoundValue)[0] + TypeChecker.castableTypes(params.upperBoundValue, highPrecision)[0] ) <= TypeChecker.cast( params.lowerBoundValue, - TypeChecker.castableTypes(params.lowerBoundValue)[0] + TypeChecker.castableTypes(params.lowerBoundValue, highPrecision)[0] ) ) { return 'error'; From f19bf0558a0c7d982813497f58e176746d0c3743 Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Tue, 1 Nov 2016 22:21:51 +1100 Subject: [PATCH 27/43] shrink range inputs, remove title labels. --- .../validation/lib/components/common/range-input.jsx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 0c85903f49e..c6790287080 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -132,9 +132,6 @@ class RangeInput extends React.Component { if (this.state.disabled) { return ( -
- {boundString} -
-
- {boundString} -
Date: Tue, 1 Nov 2016 22:43:09 +1100 Subject: [PATCH 28/43] :shirt: fix linter issue. --- .../validation/lib/components/common/range-input.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index c6790287080..3a40b5c1bf7 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -5,7 +5,6 @@ const FormGroup = require('react-bootstrap').FormGroup; const InputGroup = require('react-bootstrap').InputGroup; const FormControl = require('react-bootstrap').FormControl; const DropdownButton = require('react-bootstrap').DropdownButton; -const ControlLabel = require('react-bootstrap').ControlLabel; const MenuItem = require('react-bootstrap').MenuItem; const TypeChecker = require('hadron-type-checker'); From e6453c327a9c2359ab3fa7bd7d5b46824615a5e7 Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Wed, 2 Nov 2016 11:41:49 +1100 Subject: [PATCH 29/43] =?UTF-8?q?don=E2=80=99t=20validate=20initially,=20f?= =?UTF-8?q?ix=20tests.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../lib/components/common/range-input.jsx | 5 ----- test/validation.range.test.js | 18 ++++-------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 3a40b5c1bf7..5d7b41603e1 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -40,11 +40,6 @@ class RangeInput extends React.Component { app.instance.build.version >= HP_VERSION); } - componentWillMount() { - // Render any existing schema errors in red - this.validate(); - } - onInputChange(evt) { this.setState({ value: evt.target.value diff --git a/test/validation.range.test.js b/test/validation.range.test.js index d6356d932df..89337844e37 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -17,15 +17,10 @@ chai.use(chaiEnzyme()); describe('', () => { context('when rendering the default control', () => { - it('has label `LOWER BOUND`', () => { - const component = shallow(); - const labelText = component.find(ControlLabel).dive().text(); - expect(labelText).to.be.equal('LOWER BOUND'); - }); - it('has placeholder text of `enter lower bound`', () => { + it('has placeholder text of `lower bound`', () => { const component = shallow(); const placeholderText = component.find(FormControl).props().placeholder; - expect(placeholderText).to.be.equal('enter lower bound'); + expect(placeholderText).to.be.equal('lower bound'); }); it('accepts a scientific decimal -9.001e+2', function() { const value = '-9.0001e+2'; @@ -49,15 +44,10 @@ describe('', () => { }); context('when rendering an upperBound control', () => { - it('has label `UPPER BOUND`', () => { - const component = shallow(); - const labelText = component.find(ControlLabel).dive().text(); - expect(labelText).to.be.equal('UPPER BOUND'); - }); - it('has placeholder text of `enter upper bound`', () => { + it('has placeholder text of `upper bound`', () => { const component = shallow(); const placeholderText = component.find(FormControl).props().placeholder; - expect(placeholderText).to.be.equal('enter upper bound'); + expect(placeholderText).to.be.equal('upper bound'); }); }); }); From c908d7986b6d9d98629e36b49f11c9b104305ec3 Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Wed, 2 Nov 2016 17:09:37 +1100 Subject: [PATCH 30/43] form validation. such difficult. --- .../lib/components/common/range-input.jsx | 56 +++-- .../lib/components/rule-builder.jsx | 52 ++++- .../lib/components/rule-categories/exists.jsx | 4 +- .../rule-categories/mustnotexist.jsx | 4 +- .../lib/components/rule-categories/range.jsx | 197 +++++++++++++----- .../lib/components/rule-categories/regex.jsx | 4 +- .../lib/components/rule-categories/type.jsx | 8 +- .../lib/components/rule-category-selector.jsx | 13 +- .../lib/components/rule-field-selector.jsx | 13 +- .../validation/lib/components/rule.jsx | 44 +++- 10 files changed, 307 insertions(+), 88 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 5d7b41603e1..cbd796b8f40 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -40,24 +40,54 @@ class RangeInput extends React.Component { app.instance.build.version >= HP_VERSION); } + /** + * called whenever the input changes (i.e. user is typing). We don't bubble up the + * value at this stage yet, but wait until the user blurs the input field. + * + * @param {Object} evt The onChange event + */ onInputChange(evt) { this.setState({ value: evt.target.value }); } + /** + * called whenever the field is blurred (loses focus). At this point, we want to + * validate the input and if it is valid, report the value change up to the parent. + */ onInputBlur() { - this.validate(); + if (this.validate()) { + this.props.onRangeInputBlur({ + value: this.state.value, + operator: this.state.operator + }); + } } + /** + * called when the user chooses a value from the operator dropdown. As there are + * no invalid values, we can always immediately report up the value/operator change. + * + * @param {Object} evtKey the selected value from the dropdown (e.g. "<=", "none", ...) + */ onDropdownSelect(evtKey) { this.setState({ disabled: evtKey === 'none', operator: evtKey }); - this.validate(); + this.props.onRangeInputBlur({ + value: this.state.value, + operator: evtKey + }); } + /** + * determines if the input by itself is valid (e.g. a value that can be cast to a number). + * This will also report the result up to the parent via `this.props.validate()`. + * + * @return {Boolean} whether the input is valid or not. + */ validate() { const value = this.state.value; const valueTypes = TypeChecker.castableTypes(value, this._ENABLE_HP); @@ -70,19 +100,12 @@ class RangeInput extends React.Component { 'Decimal128' ]; - if (!_.intersection(valueTypes, NUMBER_TYPES).length) { - this.setState({ - validationState: 'error' - }); - } else { - this.setState({ - validationState: null - }); - } - // Get the parent to update both RangeInput component states - if (this.props.onRangeInputBlur) { - _.defer(this.props.onRangeInputBlur); - } + const isValid = (_.intersection(valueTypes, NUMBER_TYPES).length); + this.setState({ + validationState: isValid ? null : 'error' + }); + this.props.validate(isValid); + return isValid; } _getOperatorString(props) { @@ -170,7 +193,8 @@ RangeInput.propTypes = { boundIncluded: React.PropTypes.bool.isRequired, disabled: React.PropTypes.bool.isRequired, onRangeInputBlur: React.PropTypes.func, - width: React.PropTypes.number + width: React.PropTypes.number, + validate: React.PropTypes.func }; RangeInput.defaultProps = { diff --git a/src/internal-packages/validation/lib/components/rule-builder.jsx b/src/internal-packages/validation/lib/components/rule-builder.jsx index dd3f957120f..3b887fc1c6c 100644 --- a/src/internal-packages/validation/lib/components/rule-builder.jsx +++ b/src/internal-packages/validation/lib/components/rule-builder.jsx @@ -16,6 +16,13 @@ const Table = ReactBootstrap.Table; class RuleBuilder extends React.Component { + constructor(props) { + super(props); + this.state = { + isValid: true, + childValidationStates: {} + }; + } /** * Add button clicked to create a new rule. @@ -58,23 +65,48 @@ class RuleBuilder extends React.Component { ValidationActions.saveChanges(); } + validate(key, valid) { + const childValidationStates = _.clone(this.state.childValidationStates); + childValidationStates[key] = valid; + const isValid = _.all(_.values(childValidationStates)); + this.setState({ + childValidationStates: childValidationStates, + isValid: isValid + }); + } + + renderRules() { + return _.map(this.props.validationRules, (rule) => { + return ( + + ); + }); + } /** * Render status row component. * * @returns {React.Component} The component. */ render() { - const rules = _.map(this.props.validationRules, (rule) => { - return ; - }); + const editableProps = { + editState: this.props.editState, + childName: 'Validation', + onCancel: this.onCancel.bind(this), + onUpdate: this.onUpdate.bind(this) + }; + + if (!this.state.isValid) { + editableProps.editState = 'error'; + editableProps.errorMessage = 'Input is not valid.'; + delete editableProps.childName; + } return ( - + @@ -118,7 +150,7 @@ class RuleBuilder extends React.Component { - {rules} + {this.renderRules()} diff --git a/src/internal-packages/validation/lib/components/rule-categories/exists.jsx b/src/internal-packages/validation/lib/components/rule-categories/exists.jsx index 48b7a7e45e5..a6dc44cc65b 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/exists.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/exists.jsx @@ -7,6 +7,7 @@ class RuleCategoryExists extends React.Component { constructor(props) { super(props); + props.validate(true); } /** @@ -60,7 +61,8 @@ class RuleCategoryExists extends React.Component { RuleCategoryExists.propTypes = { id: React.PropTypes.string.isRequired, - parameters: React.PropTypes.object.isRequired + parameters: React.PropTypes.object.isRequired, + validate: React.PropTypes.func.isRequired }; RuleCategoryExists.displayName = 'RuleCategoryExists'; diff --git a/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx b/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx index 31d47571ff3..404131c20af 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx @@ -8,6 +8,7 @@ class RuleCategoryMustNotExist extends React.Component { constructor(props) { super(props); + props.validate(true); } /** @@ -61,7 +62,8 @@ class RuleCategoryMustNotExist extends React.Component { RuleCategoryMustNotExist.propTypes = { id: React.PropTypes.string.isRequired, - parameters: React.PropTypes.object.isRequired + parameters: React.PropTypes.object.isRequired, + validate: React.PropTypes.func.isRequired }; RuleCategoryMustNotExist.displayName = 'RuleCategoryMustNotExist'; diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index 988c66a6e85..4d39a9c270b 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -16,34 +16,89 @@ const HP_VERSION = '3.4.0'; class RuleCategoryRange extends React.Component { - onRangeInputBlur() { + constructor(props) { + super(props); + // this tracks whether the children individually are valid or not. + // it is a bit unusually to make this an instance variable, but since + // the component does not have to re-render itself when this changes, + // it's ok. We also had issues making this part of state because the + // state does not update immediately, which was causing issues. + // + // @see http://stackoverflow.com/questions/30782948/why-calling-react-setstate-method-doesnt-mutate-the-state-immediately + this.childrenIndividuallyValid = true; + + // We are forking the `parameters` passed in as props here, and are + // updating them whenever a child component changes its values + // (onRangeInputBlur). + // Additionally, we maintain the validation states of the two + // children so that we can determine if both of them are valid. + this.state = { + parameters: _.clone(this.props.parameters), + childValidationStates: { + lower: true, + upper: true + } + }; + } + + /** + * callback called by the child components () whenever + * they change their value or operator. This method then updates the + * internal `state.parameters` accordingly and determines if the two + * combined values are still considered a valid state or not. + * + * Note: individual value validation (i.e. are both children individually + * valid) happens in `this.validate()` and is tracked by + * this.childrenIndividuallyValid. + * + * @param {String} key the key of the child component, `lower` or `upper` + * @param {Object} value the value passed up from the child of the shape + * { value: ..., operator: ... } + */ + onRangeInputBlur(key, value) { const opMap = { '>=': '$gte', '>': '$gt', '<=': '$lte', '<': '$lt' }; - const params = RuleCategoryRange.getInitialParameters(); - // Use refs to get child state, as children don't have a unique ID - // http://stackoverflow.com/a/29303324 - const lowerBoundState = this.refs.lowerBoundRangeInputChild.state; - const upperBoundState = this.refs.upperBoundRangeInputChild.state; - if (Object.keys(opMap).includes(lowerBoundState.operator)) { - params.lowerBoundType = opMap[lowerBoundState.operator]; - params.lowerBoundValue = lowerBoundState.value; - } else { - params.lowerBoundType = null; - } - if (Object.keys(opMap).includes(upperBoundState.operator)) { - params.upperBoundType = opMap[upperBoundState.operator]; - params.upperBoundValue = upperBoundState.value; - } else { - params.upperBoundType = null; - } - params.comboValidationState = RuleCategoryRange.getComboValidationState(params); - - // Trigger an action that should update the Reflux ValidationStore - ValidationAction.setRuleParameters(this.props.id, params); + + const params = _.clone(this.state.parameters); + + if (key === 'lower') { + if (value.operator === 'none') { + params.lowerBoundType = null; + } else { + params.lowerBoundType = opMap[value.operator]; + params.lowerBoundValue = value.value; + } + } + + if (key === 'upper') { + if (value.operator === 'none') { + params.upperBoundType = null; + } else { + params.upperBoundType = opMap[value.operator]; + params.upperBoundValue = value.value; + } + } + + this.setState({ + parameters: params + }); + + // this component is valid if the children are individually valid and + // their combined values are also valid. + const overallValid = this.validateCombinedValues( + params.lowerBoundType && params.lowerBoundValue, + params.upperBoundType && params.upperBoundValue + ) && this.childrenIndividuallyValid; + + // report up the chain to the parent our overall valid state + this.props.validate(overallValid); + if (overallValid) { + ValidationAction.setRuleParameters(this.props.id, params); + } } static getInitialParameters() { @@ -55,28 +110,6 @@ class RuleCategoryRange extends React.Component { }; } - static getComboValidationState(params) { - // Can't compare two Decimal128's for correctness easily in JS... - const highPrecision = false; - - // No documents could possibly satisfy these cases, e.g. 5 <= value < 5 - if (params.upperBoundValue !== null && - params.lowerBoundValue !== null && - TypeChecker.cast( - params.upperBoundValue, - TypeChecker.castableTypes(params.upperBoundValue, highPrecision)[0] - ) - <= - TypeChecker.cast( - params.lowerBoundValue, - TypeChecker.castableTypes(params.lowerBoundValue, highPrecision)[0] - ) - ) { - return 'error'; - } - return null; - } - /** * Are high precision values available? * @@ -133,7 +166,6 @@ class RuleCategoryRange extends React.Component { return false; } const result = { - comboValidationState: null, upperBoundValue: null, upperBoundType: _.intersection(keys, ['$lte', '$lt']), lowerBoundValue: null, @@ -159,10 +191,70 @@ class RuleCategoryRange extends React.Component { } result.upperBoundType = result.upperBoundType[0] || null; result.lowerBoundType = result.lowerBoundType[0] || null; - result.comboValidationState = RuleCategoryRange.getComboValidationState(result); return result; } + /** + * checks if the children values (just the numeric values, not operators) + * are valid together. We consider them invalid, if both bounds are "none", + * or if the lower bound is not smaller than the upper bound. + * + * We consider them always valid if only one bound is set. + * + * @param {Number} lower the lower bound value + * @param {Number} upper the upper bound value + * + * @return {Boolean} whether the combined values are valid or not + */ + validateCombinedValues(lower, upper) { + // TODO Can't compare two Decimal128's for correctness easily in JS... + const highPrecision = false; + + // first check that not both values are "none" + if (!upper && !lower) { + return false; + } + // if only one value is "none", it's automatically valid + if (!upper || !lower) { + return true; + } + + const castedUpper = TypeChecker.cast( + upper, + TypeChecker.castableTypes(upper, highPrecision)[0] + ); + + const castedLower = TypeChecker.cast( + lower, + TypeChecker.castableTypes(lower, highPrecision)[0] + ); + + // only return true if the lower value strictly less than the upper value + return castedLower < castedUpper; + } + + /** + * callback for child components () to be called when they + * validate their own state. In this method, we only compute if all children + * are individually valid. If they are not, we can immediately report up the + * chain that this component is currently not valid. Otherwise, we have to + * check the combined validity, which is done in `this.validateCombinedValues`. + * + * @param {String} key The key of the child component: `lower` or `upper` + * @param {Boolean} valid The valid state of the child component + */ + validate(key, valid) { + const childValidationStates = _.clone(this.state.childValidationStates); + childValidationStates[key] = valid; + this.childrenIndividuallyValid = _.all(_.values(childValidationStates)); + this.setState({ + childValidationStates: childValidationStates + }); + if (!this.childrenIndividuallyValid) { + this.props.validate(false); + } + } + /** * Render ValidationHeader. * @@ -176,8 +268,9 @@ class RuleCategoryRange extends React.Component { boundIncluded={this.props.parameters.lowerBoundType === '$gte'} disabled={this.props.parameters.lowerBoundType === null} value={this.props.parameters.lowerBoundValue} - onRangeInputBlur={this.onRangeInputBlur.bind(this)} - validationState={this.props.parameters.comboValidationState} + onRangeInputBlur={this.onRangeInputBlur.bind(this, 'lower')} + // validationState={this.props.parameters.comboValidationState} + validate={this.validate.bind(this, 'lower')} />
); @@ -195,7 +289,8 @@ class RuleCategoryRange extends React.Component { RuleCategoryRange.propTypes = { id: React.PropTypes.string.isRequired, - parameters: React.PropTypes.object.isRequired + parameters: React.PropTypes.object.isRequired, + validate: React.PropTypes.func.isRequired }; RuleCategoryRange.displayName = 'RuleCategoryRange'; diff --git a/src/internal-packages/validation/lib/components/rule-categories/regex.jsx b/src/internal-packages/validation/lib/components/rule-categories/regex.jsx index c8a957abcca..decd07e0753 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/regex.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/regex.jsx @@ -22,6 +22,7 @@ class RuleCategoryRegex extends React.Component { value: _.get(this.props.parameters, 'regex', ''), options: _.get(this.props.parameters, 'options', '') }; + props.validate(true); } onChange(evt) { @@ -185,7 +186,8 @@ class RuleCategoryRegex extends React.Component { RuleCategoryRegex.propTypes = { id: React.PropTypes.string.isRequired, - parameters: React.PropTypes.object.isRequired + parameters: React.PropTypes.object.isRequired, + validate: React.PropTypes.func.isRequired }; RuleCategoryRegex.displayName = 'RuleCategoryRegex'; diff --git a/src/internal-packages/validation/lib/components/rule-categories/type.jsx b/src/internal-packages/validation/lib/components/rule-categories/type.jsx index 7c1ddbf255e..a65a68528b4 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/type.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/type.jsx @@ -7,6 +7,11 @@ const _ = require('lodash'); class RuleCategoryType extends React.Component { + constructor(props) { + super(props); + props.validate(true); + } + onTypeClicked(type, evt) { evt.preventDefault(); ValidationAction.setRuleParameters(this.props.id, { @@ -87,7 +92,8 @@ class RuleCategoryType extends React.Component { RuleCategoryType.propTypes = { id: React.PropTypes.string.isRequired, - parameters: React.PropTypes.object.isRequired + parameters: React.PropTypes.object.isRequired, + validate: React.PropTypes.func.isRequired }; RuleCategoryType.displayName = 'RuleCategoryType'; diff --git a/src/internal-packages/validation/lib/components/rule-category-selector.jsx b/src/internal-packages/validation/lib/components/rule-category-selector.jsx index 388e7540744..4055cbccbec 100644 --- a/src/internal-packages/validation/lib/components/rule-category-selector.jsx +++ b/src/internal-packages/validation/lib/components/rule-category-selector.jsx @@ -13,7 +13,15 @@ const _ = require('lodash'); class RuleCategorySelector extends React.Component { onSelect(category) { - ValidationActions.setRuleCategory(this.props.id, category); + if (this.validate(category)) { + ValidationActions.setRuleCategory(this.props.id, category); + } + } + + validate(category) { + const isValid = category !== ''; + this.props.validate(isValid); + return isValid; } /** @@ -41,7 +49,8 @@ class RuleCategorySelector extends React.Component { RuleCategorySelector.propTypes = { id: React.PropTypes.string.isRequired, - category: React.PropTypes.string.isRequired + category: React.PropTypes.string.isRequired, + validate: React.PropTypes.func }; RuleCategorySelector.displayName = 'RuleCategorySelector'; diff --git a/src/internal-packages/validation/lib/components/rule-field-selector.jsx b/src/internal-packages/validation/lib/components/rule-field-selector.jsx index c712fcbdcff..c5bffe321a7 100644 --- a/src/internal-packages/validation/lib/components/rule-field-selector.jsx +++ b/src/internal-packages/validation/lib/components/rule-field-selector.jsx @@ -46,12 +46,20 @@ class RuleFieldSelector extends React.Component { }); } + validate() { + const isValid = this.state.value !== ''; + this.props.validate(isValid); + return isValid; + } + /** * The input field has lost focus (onBlur). Trigger an action to inform * the store of the change. */ submit() { - ValidationActions.setRuleField(this.props.id, this.state.value); + if (this.validate()) { + ValidationActions.setRuleField(this.props.id, this.state.value); + } } /** @@ -77,7 +85,8 @@ class RuleFieldSelector extends React.Component { RuleFieldSelector.propTypes = { id: React.PropTypes.string.isRequired, - field: React.PropTypes.string.isRequired + field: React.PropTypes.string.isRequired, + validate: React.PropTypes.func }; RuleFieldSelector.displayName = 'RuleFieldSelector'; diff --git a/src/internal-packages/validation/lib/components/rule.jsx b/src/internal-packages/validation/lib/components/rule.jsx index 647b44cd834..04764e05183 100644 --- a/src/internal-packages/validation/lib/components/rule.jsx +++ b/src/internal-packages/validation/lib/components/rule.jsx @@ -17,15 +17,50 @@ const debug = require('debug')('compass:validation:rule'); */ class Rule extends React.Component { + constructor(props) { + super(props); + + this.state = { + isValid: true, + childValidationStates: { + RuleFieldSelector: true, + RuleCategorySelector: true + } + }; + } + checkBoxClicked() { ValidationActions.setRuleNullable(this.props.id, !this.props.nullable); } + /** + * called by child components when they evaluate if they are valid. + * This method saves the valid state for each child, and then determine + * if itself is valid. This is only the case when all children are valid. + * It then reports its own state up the chain by calling this.props.validate(). + * + * @param {String} key an identifier string for the child component + * @param {Boolean} valid whether the child was valid or not + */ + validate(key, valid) { + const childValidationStates = _.clone(this.state.childValidationStates); + childValidationStates[key] = valid; + const isValid = _.all(_.values(childValidationStates)); + this.setState({ + childValidationStates: childValidationStates, + isValid: isValid + }); + this.props.validate(isValid); + } + render() { - debug('props', this.props); const Category = _.get(ruleCategories, this.props.category, null); const ruleParameters = Category ? - : + : null; const nullableDisabled = _.includes(['exists', 'mustNotExist'], @@ -37,6 +72,7 @@ class Rule extends React.Component { @@ -44,6 +80,7 @@ class Rule extends React.Component { {ruleParameters} @@ -68,7 +105,8 @@ Rule.propTypes = { field: React.PropTypes.string.isRequired, category: React.PropTypes.string.isRequired, parameters: React.PropTypes.object.isRequired, - nullable: React.PropTypes.bool.isRequired + nullable: React.PropTypes.bool.isRequired, + validate: React.PropTypes.func }; Rule.displayName = 'Rule'; From a301d458f135f682f195c486b2369b63f3a2b1de Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Wed, 2 Nov 2016 23:15:39 +1100 Subject: [PATCH 31/43] [wip] up and downwards validation almost working --- .../lib/components/rule-builder.jsx | 32 ++++++----- .../lib/components/rule-categories/exists.jsx | 5 +- .../rule-categories/mustnotexist.jsx | 5 +- .../lib/components/rule-categories/range.jsx | 38 +++++++------ .../lib/components/rule-categories/regex.jsx | 5 +- .../lib/components/rule-categories/type.jsx | 5 +- .../lib/components/rule-category-selector.jsx | 45 +++++++++++----- .../lib/components/rule-delete-button.jsx | 10 ++-- .../lib/components/rule-field-selector.jsx | 16 +++--- .../validation/lib/components/rule.jsx | 54 ++++++++++--------- .../validation/styles/option-selector.less | 7 +++ 11 files changed, 140 insertions(+), 82 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-builder.jsx b/src/internal-packages/validation/lib/components/rule-builder.jsx index 3b887fc1c6c..108ac8258b6 100644 --- a/src/internal-packages/validation/lib/components/rule-builder.jsx +++ b/src/internal-packages/validation/lib/components/rule-builder.jsx @@ -18,10 +18,8 @@ class RuleBuilder extends React.Component { constructor(props) { super(props); - this.state = { - isValid: true, - childValidationStates: {} - }; + this.isValid = true; + this.childValidationStates = {}; } /** @@ -54,6 +52,7 @@ class RuleBuilder extends React.Component { * Revert all changes to the server state. */ onCancel() { + this.isValid = true; ValidationActions.cancelChanges(); } @@ -62,23 +61,30 @@ class RuleBuilder extends React.Component { * Send the new validator doc to the server. */ onUpdate() { - ValidationActions.saveChanges(); + this.validate(); + if (this.isValid) { + ValidationActions.saveChanges(); + } } validate(key, valid) { - const childValidationStates = _.clone(this.state.childValidationStates); - childValidationStates[key] = valid; - const isValid = _.all(_.values(childValidationStates)); - this.setState({ - childValidationStates: childValidationStates, - isValid: isValid - }); + if (key === undefined) { + // downwards validation, call children's validate() method. + _.each(this.props.validationRules, (rule) => { + this.refs[rule.id].validate(); + }); + return; + } + this.childValidationStates[key] = valid; + this.isValid = _.all(_.values(this.childValidationStates)); + this.forceUpdate(); } renderRules() { return _.map(this.props.validationRules, (rule) => { return ( ) whenever * they change their value or operator. This method then updates the @@ -89,14 +92,14 @@ class RuleCategoryRange extends React.Component { // this component is valid if the children are individually valid and // their combined values are also valid. - const overallValid = this.validateCombinedValues( + this.isValid = this.validateCombinedValues( params.lowerBoundType && params.lowerBoundValue, params.upperBoundType && params.upperBoundValue ) && this.childrenIndividuallyValid; // report up the chain to the parent our overall valid state - this.props.validate(overallValid); - if (overallValid) { + this.props.validate(this.isValid); + if (this.isValid) { ValidationAction.setRuleParameters(this.props.id, params); } } @@ -244,12 +247,15 @@ class RuleCategoryRange extends React.Component { * @param {Boolean} valid The valid state of the child component */ validate(key, valid) { - const childValidationStates = _.clone(this.state.childValidationStates); - childValidationStates[key] = valid; - this.childrenIndividuallyValid = _.all(_.values(childValidationStates)); - this.setState({ - childValidationStates: childValidationStates - }); + if (key === undefined) { + // downwards validation, call children's validate() method. + this.refs.lowerBoundRangeInputChild.validate(); + this.refs.upperBoundRangeInputChild.validate(); + return; + } + this.childValidationStates[key] = valid; + this.childrenIndividuallyValid = _.all(_.values(this.childValidationStates)); + if (!this.childrenIndividuallyValid) { this.props.validate(false); } diff --git a/src/internal-packages/validation/lib/components/rule-categories/regex.jsx b/src/internal-packages/validation/lib/components/rule-categories/regex.jsx index decd07e0753..eaa777989e8 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/regex.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/regex.jsx @@ -22,7 +22,10 @@ class RuleCategoryRegex extends React.Component { value: _.get(this.props.parameters, 'regex', ''), options: _.get(this.props.parameters, 'options', '') }; - props.validate(true); + } + + componentWillMount() { + this.props.validate(true); } onChange(evt) { diff --git a/src/internal-packages/validation/lib/components/rule-categories/type.jsx b/src/internal-packages/validation/lib/components/rule-categories/type.jsx index a65a68528b4..1742dfdd411 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/type.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/type.jsx @@ -9,7 +9,10 @@ class RuleCategoryType extends React.Component { constructor(props) { super(props); - props.validate(true); + } + + componentWillMount() { + this.props.validate(true); } onTypeClicked(type, evt) { diff --git a/src/internal-packages/validation/lib/components/rule-category-selector.jsx b/src/internal-packages/validation/lib/components/rule-category-selector.jsx index 4055cbccbec..0112cee3773 100644 --- a/src/internal-packages/validation/lib/components/rule-category-selector.jsx +++ b/src/internal-packages/validation/lib/components/rule-category-selector.jsx @@ -2,6 +2,7 @@ const React = require('react'); const OptionSelector = require('./common/option-selector'); const ValidationActions = require('../actions'); const ruleCategories = require('./rule-categories'); +const { FormGroup } = require('react-bootstrap'); const _ = require('lodash'); // const debug = require('debug')('mongodb-compass:validation:rule-category'); @@ -12,15 +13,31 @@ const _ = require('lodash'); */ class RuleCategorySelector extends React.Component { + /** + * constructor sets the initial state. + * + * @param {Object} props initial props, passed to super class. + */ + constructor(props) { + super(props); + this.state = { + isValid: true + }; + } + onSelect(category) { - if (this.validate(category)) { - ValidationActions.setRuleCategory(this.props.id, category); - } + this.setState({ + isValid: true + }); + ValidationActions.setRuleCategory(this.props.id, category); } - validate(category) { - const isValid = category !== ''; + validate() { + const isValid = this.props.category !== ''; this.props.validate(isValid); + this.setState({ + isValid: isValid + }); return isValid; } @@ -35,14 +52,18 @@ class RuleCategorySelector extends React.Component { _.map(_.keys(ruleCategories), _.startCase) ); + const validationState = this.state.isValid ? null : 'error'; + return ( - + + + ); } } diff --git a/src/internal-packages/validation/lib/components/rule-delete-button.jsx b/src/internal-packages/validation/lib/components/rule-delete-button.jsx index a3e540220f7..2189e408a4f 100644 --- a/src/internal-packages/validation/lib/components/rule-delete-button.jsx +++ b/src/internal-packages/validation/lib/components/rule-delete-button.jsx @@ -1,5 +1,4 @@ const React = require('react'); -const ValidationActions = require('../actions'); const Button = require('react-bootstrap').Button; const FontAwesome = require('react-fontawesome'); @@ -8,10 +7,6 @@ const FontAwesome = require('react-fontawesome'); */ class RuleDeleteButton extends React.Component { - onDeleteClicked() { - ValidationActions.deleteValidationRule(this.props.id); - } - /** * Render RuleDeleteButton * @@ -19,7 +14,7 @@ class RuleDeleteButton extends React.Component { */ render() { return ( - ); @@ -27,7 +22,8 @@ class RuleDeleteButton extends React.Component { } RuleDeleteButton.propTypes = { - id: React.PropTypes.string.isRequired + id: React.PropTypes.string.isRequired, + onClick: React.PropTypes.func.isRequired }; RuleDeleteButton.displayName = 'RuleDeleteButton'; diff --git a/src/internal-packages/validation/lib/components/rule-field-selector.jsx b/src/internal-packages/validation/lib/components/rule-field-selector.jsx index c5bffe321a7..c06e1e70c68 100644 --- a/src/internal-packages/validation/lib/components/rule-field-selector.jsx +++ b/src/internal-packages/validation/lib/components/rule-field-selector.jsx @@ -1,6 +1,6 @@ const React = require('react'); const ValidationActions = require('../actions'); - +const { FormGroup, FormControl } = require('react-bootstrap'); /** * Component to select a field for which the rule applies. * @@ -19,7 +19,8 @@ class RuleFieldSelector extends React.Component { constructor(props) { super(props); this.state = { - value: this.props.field + value: this.props.field, + isValid: true }; } @@ -49,6 +50,9 @@ class RuleFieldSelector extends React.Component { validate() { const isValid = this.state.value !== ''; this.props.validate(isValid); + this.setState({ + isValid: isValid + }); return isValid; } @@ -68,17 +72,17 @@ class RuleFieldSelector extends React.Component { * @returns {React.Component} The view component. */ render() { + const validationState = this.state.isValid ? null : 'error'; return ( -
- + -
+ ); } } diff --git a/src/internal-packages/validation/lib/components/rule.jsx b/src/internal-packages/validation/lib/components/rule.jsx index 04764e05183..43b52f7f8da 100644 --- a/src/internal-packages/validation/lib/components/rule.jsx +++ b/src/internal-packages/validation/lib/components/rule.jsx @@ -9,7 +9,7 @@ const Form = require('react-bootstrap').Form; const ruleCategories = require('./rule-categories'); const _ = require('lodash'); -const debug = require('debug')('compass:validation:rule'); +// const debug = require('debug')('compass:validation:rule'); /** * Implements a single rule with RuleFieldSelector, RuleCategorySelector and @@ -19,20 +19,20 @@ class Rule extends React.Component { constructor(props) { super(props); + this.isValid = true; + this.childValidationStates = {}; + } - this.state = { - isValid: true, - childValidationStates: { - RuleFieldSelector: true, - RuleCategorySelector: true - } - }; + onDeleteClicked() { + this.props.validate(true); + ValidationActions.deleteValidationRule(this.props.id); } checkBoxClicked() { ValidationActions.setRuleNullable(this.props.id, !this.props.nullable); } + /** * called by child components when they evaluate if they are valid. * This method saves the valid state for each child, and then determine @@ -43,25 +43,27 @@ class Rule extends React.Component { * @param {Boolean} valid whether the child was valid or not */ validate(key, valid) { - const childValidationStates = _.clone(this.state.childValidationStates); - childValidationStates[key] = valid; - const isValid = _.all(_.values(childValidationStates)); - this.setState({ - childValidationStates: childValidationStates, - isValid: isValid - }); - this.props.validate(isValid); + if (key === undefined) { + // downwards validation, call children's validate() method. + _.result(this.refs.Parameters, 'validate'); + this.refs.RuleFieldSelector.validate(); + this.refs.RuleCategorySelector.validate(); + return; + } + this.childValidationStates[key] = valid; + this.isValid = _.all(_.values(this.childValidationStates)); + this.props.validate(this.isValid); } render() { - const Category = _.get(ruleCategories, this.props.category, null); - const ruleParameters = Category ? - : - null; + validate={this.validate.bind(this, 'Parameters')} + /> : null; const nullableDisabled = _.includes(['exists', 'mustNotExist'], this.props.category); @@ -70,6 +72,7 @@ class Rule extends React.Component {
- + ); @@ -106,7 +112,7 @@ Rule.propTypes = { category: React.PropTypes.string.isRequired, parameters: React.PropTypes.object.isRequired, nullable: React.PropTypes.bool.isRequired, - validate: React.PropTypes.func + validate: React.PropTypes.func.isRequired }; Rule.displayName = 'Rule'; diff --git a/src/internal-packages/validation/styles/option-selector.less b/src/internal-packages/validation/styles/option-selector.less index bc442e900ca..3db049903d6 100644 --- a/src/internal-packages/validation/styles/option-selector.less +++ b/src/internal-packages/validation/styles/option-selector.less @@ -2,3 +2,10 @@ display: inline-block; padding: 0 5px; } + + +.has-error .option-selector { + button.dropdown-toggle { + border-color: #843534; + } +} From a50f8f9f58444caa50c2c9be63e08d893cc43a77 Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Thu, 3 Nov 2016 00:26:21 +1100 Subject: [PATCH 32/43] fixing some edge cases, updating tests. --- .../lib/components/rule-builder.jsx | 2 +- .../lib/components/rule-categories/range.jsx | 10 +- .../lib/components/rule-category-selector.jsx | 8 +- .../lib/components/rule-field-selector.jsx | 24 +- test/validation.range.test.js | 39 +- test/validation.rule.test.js | 3 +- test/validation.store.test.js | 332 ------------------ 7 files changed, 53 insertions(+), 365 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-builder.jsx b/src/internal-packages/validation/lib/components/rule-builder.jsx index 108ac8258b6..1b36ca35b96 100644 --- a/src/internal-packages/validation/lib/components/rule-builder.jsx +++ b/src/internal-packages/validation/lib/components/rule-builder.jsx @@ -53,7 +53,7 @@ class RuleBuilder extends React.Component { */ onCancel() { this.isValid = true; - ValidationActions.cancelChanges(); + ValidationActions.cancelChanges(true); } /** diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index 8592059e0d6..d9eba250b1f 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -267,15 +267,17 @@ class RuleCategoryRange extends React.Component { * @returns {React.Component} The view component. */ render() { + const validationState = (!this.isValid && this.childrenIndividuallyValid) ? + 'error' : null; return ( diff --git a/src/internal-packages/validation/lib/components/rule-category-selector.jsx b/src/internal-packages/validation/lib/components/rule-category-selector.jsx index 0112cee3773..18b9bf9790c 100644 --- a/src/internal-packages/validation/lib/components/rule-category-selector.jsx +++ b/src/internal-packages/validation/lib/components/rule-category-selector.jsx @@ -26,14 +26,12 @@ class RuleCategorySelector extends React.Component { } onSelect(category) { - this.setState({ - isValid: true - }); + this.validate(category); ValidationActions.setRuleCategory(this.props.id, category); } - validate() { - const isValid = this.props.category !== ''; + validate(category) { + const isValid = (category || this.props.category) !== ''; this.props.validate(isValid); this.setState({ isValid: isValid diff --git a/src/internal-packages/validation/lib/components/rule-field-selector.jsx b/src/internal-packages/validation/lib/components/rule-field-selector.jsx index c06e1e70c68..abab836230e 100644 --- a/src/internal-packages/validation/lib/components/rule-field-selector.jsx +++ b/src/internal-packages/validation/lib/components/rule-field-selector.jsx @@ -37,14 +37,12 @@ class RuleFieldSelector extends React.Component { } /** - * set internal state when receiving new props - * - * @param {Object} nextProps props the component will receive. + * The input field has lost focus (onBlur). Trigger an action to inform + * the store of the change. */ - willReceiveProps(nextProps) { - this.setState({ - value: nextProps.field - }); + onBlur() { + this.validate(); + ValidationActions.setRuleField(this.props.id, this.state.value); } validate() { @@ -56,16 +54,6 @@ class RuleFieldSelector extends React.Component { return isValid; } - /** - * The input field has lost focus (onBlur). Trigger an action to inform - * the store of the change. - */ - submit() { - if (this.validate()) { - ValidationActions.setRuleField(this.props.id, this.state.value); - } - } - /** * Render RuleFieldSelector * @@ -80,7 +68,7 @@ class RuleFieldSelector extends React.Component { id={this.props.id} value={this.state.value} onChange={this.onFieldChanged.bind(this)} - onBlur={this.submit.bind(this)} + onBlur={this.onBlur.bind(this)} /> ); diff --git a/test/validation.range.test.js b/test/validation.range.test.js index 89337844e37..f6c6561b0fc 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -7,12 +7,15 @@ const React = require('react'); const _ = require('lodash'); const shallow = require('enzyme').shallow; +const mount = require('enzyme').mount; const bootstrap = require('react-bootstrap'); const ControlLabel = bootstrap.ControlLabel; const FormControl = bootstrap.FormControl; const RangeInput = require('../src/internal-packages/validation/lib/components/common/range-input'); const RuleCategoryRange = require('../src/internal-packages/validation/lib/components/rule-categories/range'); +const debug = require('debug')('mongodb-compass:test:validation'); + chai.use(chaiEnzyme()); describe('', () => { @@ -62,7 +65,8 @@ describe('', function() { lowerBoundValue: '-5', upperBoundValue: '5' }, - nullable: false + nullable: false, + validate: function() {} }; it('has two child components', function() { @@ -72,10 +76,9 @@ describe('', function() { expect(ranges).to.have.length(2); }); - it('accepts empty range 5 < x < 5 with getComboValidationState error', function() { + it('accepts empty range 5 < x < 5 initially', function() { const props = _.assign(propsTemplate, { parameters: { - comboValidationState: 'error', lowerBoundType: '$gt', lowerBoundValue: '5', upperBoundType: '$lt', @@ -86,8 +89,36 @@ describe('', function() { const ranges = component.dive().find(RangeInput); expect(ranges).to.have.length(2); ranges.forEach(range => { - expect(range.props().validationState).to.be.equal('error'); + expect(range.props().validationState).to.be.null; + }); + }); + + it('rejects empty range 5 < x < 3 after calling validate()', function() { + const props = _.assign(propsTemplate, { + parameters: { + lowerBoundType: '$gt', + lowerBoundValue: '5', + upperBoundType: '$lt', + upperBoundValue: '3' + } + }); + component = mount(); + component.instance().onRangeInputBlur('fake-key'); + expect(component.instance().isValid).to.be.false; + }); + + it('rejects empty both range values being "none" after calling validate()', function() { + const props = _.assign(propsTemplate, { + parameters: { + lowerBoundType: null, + lowerBoundValue: '5', + upperBoundType: null, + upperBoundValue: '3' + } }); + component = mount(); + component.instance().onRangeInputBlur('fake-key'); + expect(component.instance().isValid).to.be.false; }); context('for some different numeric types', function() { diff --git a/test/validation.rule.test.js b/test/validation.rule.test.js index 625966c4c9a..bd1b1f38880 100644 --- a/test/validation.rule.test.js +++ b/test/validation.rule.test.js @@ -23,7 +23,8 @@ describe('', function() { field: 'created_at', category: 'type', parameters: {type: 9}, // type "date" - nullable: false + nullable: false, + validate: function() {} }; it('has an input field with value "created_at"', function() { diff --git a/test/validation.store.test.js b/test/validation.store.test.js index 4d785b3c977..1a66b80d94a 100644 --- a/test/validation.store.test.js +++ b/test/validation.store.test.js @@ -843,338 +843,6 @@ describe('ValidationStore', function() { }); describe('Range Rule when passing in values that are valid on the server', function() { - context('values accepted with no error by the rule builder UI', function() { - it('accepts {$gte: 21}', function() { - const validatorDoc = { - 'validator': { - 'age': { - '$gte': 21 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = result.rules[0]; - expect(_.omit(rule, 'id')).to.be.deep.equal({ - category: 'range', - field: 'age', - nullable: false, - parameters: { - 'comboValidationState': null, - 'lowerBoundType': '$gte', - 'lowerBoundValue': '21', - 'upperBoundType': null, - 'upperBoundValue': null - } - }); - }); - - it('accepts {$lt: 21.1234567890}', function() { - const validatorDoc = { - 'validator': { - 'age': { - '$lt': 21.1234567890 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'age', - nullable: false, - parameters: { - 'comboValidationState': null, - 'lowerBoundType': null, - 'lowerBoundValue': null, - 'upperBoundType': '$lt', - 'upperBoundValue': '21.123456789' - } - }); - }); - - it('accepts {$gt: 20, $lte: 21}', function() { - const validatorDoc = { - 'validator': { - 'age': { - '$gt': 20, - '$lte': 21 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'age', - nullable: false, - parameters: { - 'comboValidationState': null, - 'lowerBoundType': '$gt', - 'lowerBoundValue': '20', - 'upperBoundType': '$lte', - 'upperBoundValue': '21' - } - }); - }); - - it('accepts {$gt: -20, $lte: -0.000001}', function() { - const validatorDoc = { - 'validator': { - 'age': { - '$gt': -20, - '$lte': -0.000001 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'age', - nullable: false, - parameters: { - 'comboValidationState': null, - 'lowerBoundType': '$gt', - 'lowerBoundValue': '-20', - 'upperBoundType': '$lte', - 'upperBoundValue': '-0.000001' - } - }); - }); - - it('accepts {$gt: 0, $lte: 5000}', function() { - const validatorDoc = { - 'validator': { - 'tree_age': { - '$gt': 0, - '$lte': 5000 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'tree_age', - nullable: false, - parameters: { - 'comboValidationState': null, - 'lowerBoundType': '$gt', - 'lowerBoundValue': '0', - 'upperBoundType': '$lte', - 'upperBoundValue': '5000' - } - }); - }); - - it('accepts {$gt: -5000000000, $lt: 0}', function() { - const validatorDoc = { - 'validator': { - 'age': { - '$gt': -5000000000, - '$lt': 0 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'age', - nullable: false, - parameters: { - 'comboValidationState': null, - 'lowerBoundType': '$gt', - 'lowerBoundValue': '-5000000000', - 'upperBoundType': '$lt', - 'upperBoundValue': '0' - } - }); - }); - - it('accepts {$gte: 0, $lt: 273.16}', function() { - const validatorDoc = { - 'validator': { - 'below_zero_celsius_in_kelvin': { - '$gte': 0, - '$lt': 273.16 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'below_zero_celsius_in_kelvin', - nullable: false, - parameters: { - 'comboValidationState': null, - 'lowerBoundType': '$gte', - 'lowerBoundValue': '0', - 'upperBoundType': '$lt', - 'upperBoundValue': '273.16' - } - }); - }); - - it('accepts {$gt: -273.16, $lte: 0}', function() { - const validatorDoc = { - 'validator': { - 'cold_temperature': { - '$gt': -273.16, - '$lte': 0 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'cold_temperature', - nullable: false, - parameters: { - 'comboValidationState': null, - 'lowerBoundType': '$gt', - 'lowerBoundValue': '-273.16', - 'upperBoundType': '$lte', - 'upperBoundValue': '0' - } - }); - }); - }); - - // Note: Server allows these cases, but we'd drop back to JSON view here - context('values accepted with error by the rule builder UI', function() { - // Only documents with value = 5 could be inserted, - // which being a constant probably should be at the application layer - it('accepts equality constant range "5 <= value <= 5"', function() { - const validatorDoc = { - 'validator': { - 'age': { - '$gte': 5, - '$lte': 5 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'age', - nullable: false, - parameters: { - 'comboValidationState': 'error', - 'lowerBoundType': '$gte', - 'lowerBoundValue': '5', - 'upperBoundType': '$lte', - 'upperBoundValue': '5' - } - }); - }); - - // Bad as users couldn't insert any documents into the collection - it('accepts empty range "5 < value <= 5"', function() { - const validatorDoc = { - 'validator': { - 'age': { - '$gt': 5, - '$lte': 5 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'age', - nullable: false, - parameters: { - 'comboValidationState': 'error', - 'lowerBoundType': '$gt', - 'lowerBoundValue': '5', - 'upperBoundType': '$lte', - 'upperBoundValue': '5' - } - }); - }); - - // Bad as users couldn't insert any documents into the collection - it('accepts empty range "5 <= value < 5"', function() { - const validatorDoc = { - 'validator': { - 'age': { - '$gte': 5, - '$lt': 5 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'age', - nullable: false, - parameters: { - 'comboValidationState': 'error', - 'lowerBoundType': '$gte', - 'lowerBoundValue': '5', - 'upperBoundType': '$lt', - 'upperBoundValue': '5' - } - }); - }); - - // Bad as users couldn't insert any documents into the collection - it('accepts empty range "6 < value < 5"', function() { - const validatorDoc = { - 'validator': { - 'age': { - '$gt': 6, - '$lt': 5 - } - }, - 'validationLevel': 'strict', - 'validationAction': 'error' - }; - const result = ValidationStore._deconstructValidatorDoc(validatorDoc); - const rule = _.omit(result.rules[0], 'id'); - expect(rule).to.be.deep.equal({ - category: 'range', - field: 'age', - nullable: false, - parameters: { - 'comboValidationState': 'error', - 'lowerBoundType': '$gt', - 'lowerBoundValue': '6', - 'upperBoundType': '$lt', - 'upperBoundValue': '5' - } - }); - }); - }); - // Note: Server allows these cases, but we'd drop back to JSON view here context('values rejected by the rule builder UI', function() { // Better represented in GUI with the "None" operator drop down value From 400d382266882a0c8a4440109e0a950368978f01 Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Thu, 3 Nov 2016 11:53:34 +1100 Subject: [PATCH 33/43] Restore tests so accepts/rejects is clearer Just needed to drop the comboValidationState. --- test/validation.store.test.js | 319 ++++++++++++++++++++++++++++++++++ 1 file changed, 319 insertions(+) diff --git a/test/validation.store.test.js b/test/validation.store.test.js index 1a66b80d94a..0f1334bcf3e 100644 --- a/test/validation.store.test.js +++ b/test/validation.store.test.js @@ -843,6 +843,325 @@ describe('ValidationStore', function() { }); describe('Range Rule when passing in values that are valid on the server', function() { + context('values accepted with no error by the rule builder UI', function() { + it('accepts {$gte: 21}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gte': 21 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = result.rules[0]; + expect(_.omit(rule, 'id')).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gte', + 'lowerBoundValue': '21', + 'upperBoundType': null, + 'upperBoundValue': null + } + }); + }); + + it('accepts {$lt: 21.1234567890}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$lt': 21.1234567890 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': null, + 'lowerBoundValue': null, + 'upperBoundType': '$lt', + 'upperBoundValue': '21.123456789' + } + }); + }); + + it('accepts {$gt: 20, $lte: 21}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': 20, + '$lte': 21 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gt', + 'lowerBoundValue': '20', + 'upperBoundType': '$lte', + 'upperBoundValue': '21' + } + }); + }); + + it('accepts {$gt: -20, $lte: -0.000001}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': -20, + '$lte': -0.000001 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gt', + 'lowerBoundValue': '-20', + 'upperBoundType': '$lte', + 'upperBoundValue': '-0.000001' + } + }); + }); + + it('accepts {$gt: 0, $lte: 5000}', function() { + const validatorDoc = { + 'validator': { + 'tree_age': { + '$gt': 0, + '$lte': 5000 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'tree_age', + nullable: false, + parameters: { + 'lowerBoundType': '$gt', + 'lowerBoundValue': '0', + 'upperBoundType': '$lte', + 'upperBoundValue': '5000' + } + }); + }); + + it('accepts {$gt: -5000000000, $lt: 0}', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': -5000000000, + '$lt': 0 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gt', + 'lowerBoundValue': '-5000000000', + 'upperBoundType': '$lt', + 'upperBoundValue': '0' + } + }); + }); + + it('accepts {$gte: 0, $lt: 273.16}', function() { + const validatorDoc = { + 'validator': { + 'below_zero_celsius_in_kelvin': { + '$gte': 0, + '$lt': 273.16 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'below_zero_celsius_in_kelvin', + nullable: false, + parameters: { + 'lowerBoundType': '$gte', + 'lowerBoundValue': '0', + 'upperBoundType': '$lt', + 'upperBoundValue': '273.16' + } + }); + }); + + it('accepts {$gt: -273.16, $lte: 0}', function() { + const validatorDoc = { + 'validator': { + 'cold_temperature': { + '$gt': -273.16, + '$lte': 0 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'cold_temperature', + nullable: false, + parameters: { + 'lowerBoundType': '$gt', + 'lowerBoundValue': '-273.16', + 'upperBoundType': '$lte', + 'upperBoundValue': '0' + } + }); + }); + }); + + context('values accepted but will error later in the rule builder UI', function() { + // Only documents with value = 5 could be inserted, + // which being a constant probably should be at the application layer + it('accepts equality constant range "5 <= value <= 5"', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gte': 5, + '$lte': 5 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gte', + 'lowerBoundValue': '5', + 'upperBoundType': '$lte', + 'upperBoundValue': '5' + } + }); + }); + + // Bad as users couldn't insert any documents into the collection + it('accepts empty range "5 < value <= 5"', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': 5, + '$lte': 5 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gt', + 'lowerBoundValue': '5', + 'upperBoundType': '$lte', + 'upperBoundValue': '5' + } + }); + }); + + // Bad as users couldn't insert any documents into the collection + it('accepts empty range "5 <= value < 5"', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gte': 5, + '$lt': 5 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gte', + 'lowerBoundValue': '5', + 'upperBoundType': '$lt', + 'upperBoundValue': '5' + } + }); + }); + + // Bad as users couldn't insert any documents into the collection + it('accepts empty range "6 < value < 5"', function() { + const validatorDoc = { + 'validator': { + 'age': { + '$gt': 6, + '$lt': 5 + } + }, + 'validationLevel': 'strict', + 'validationAction': 'error' + }; + const result = ValidationStore._deconstructValidatorDoc(validatorDoc); + const rule = _.omit(result.rules[0], 'id'); + expect(rule).to.be.deep.equal({ + category: 'range', + field: 'age', + nullable: false, + parameters: { + 'lowerBoundType': '$gt', + 'lowerBoundValue': '6', + 'upperBoundType': '$lt', + 'upperBoundValue': '5' + } + }); + }); + }); + // Note: Server allows these cases, but we'd drop back to JSON view here context('values rejected by the rule builder UI', function() { // Better represented in GUI with the "None" operator drop down value From 0d1054ec9e509fa6de6302ab37ca36c8be1a200e Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Thu, 3 Nov 2016 14:30:16 +1100 Subject: [PATCH 34/43] make rule id consistent (based on index of rule). --- src/internal-packages/validation/lib/stores/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/internal-packages/validation/lib/stores/index.js b/src/internal-packages/validation/lib/stores/index.js index 07913332d0f..b6f8ea2b784 100644 --- a/src/internal-packages/validation/lib/stores/index.js +++ b/src/internal-packages/validation/lib/stores/index.js @@ -82,7 +82,7 @@ const ValidationStore = Reflux.createStore({ const validator = helper.filterAndFromValidator(validatorDoc.validator); - const rules = _.map(validator, (field) => { + const rules = _.map(validator, (field, idx) => { const fieldName = field[0]; const rule = field[1]; @@ -98,7 +98,7 @@ const ValidationStore = Reflux.createStore({ return false; } return { - id: uuid.v4(), + id: `rule-${idx}`, field: result.field, category: category, parameters: parameters, @@ -297,7 +297,7 @@ const ValidationStore = Reflux.createStore({ addValidationRule() { const rules = _.clone(this.state.validationRules); - const id = uuid.v4(); + const id = `rule-${rules.length}`; rules.push({ id: id, field: '', From 978538f3e3ee1fbe78131101ae8d3f0ee60bed26 Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Thu, 3 Nov 2016 22:41:48 +1100 Subject: [PATCH 35/43] store cancel always needs to call _updateState() --- .../validation/lib/stores/index.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/internal-packages/validation/lib/stores/index.js b/src/internal-packages/validation/lib/stores/index.js index b6f8ea2b784..cea65ed2996 100644 --- a/src/internal-packages/validation/lib/stores/index.js +++ b/src/internal-packages/validation/lib/stores/index.js @@ -419,16 +419,10 @@ const ValidationStore = Reflux.createStore({ } }, - cancelChanges(setByRuleBuilder) { - if (setByRuleBuilder) { - const params = this._deconstructValidatorDoc(this.lastFetchedValidatorDoc); - this._updateState(params); - return; - } - this.setState({ - editState: 'unmodified', - validatorDoc: _.clone(this.lastFetchedValidatorDoc) - }); + cancelChanges() { + const params = this._deconstructValidatorDoc(this.lastFetchedValidatorDoc); + this._updateState(params); + return; }, saveChanges() { From 2a911754adbcdac14cfd29c44efeac209b882ce1 Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Thu, 3 Nov 2016 22:42:38 +1100 Subject: [PATCH 36/43] range: disable bson type casting --- .../lib/components/rule-categories/range.jsx | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index d9eba250b1f..9cbf879da60 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -136,25 +136,25 @@ class RuleCategoryRange extends React.Component { return !isNaN(value) && Math.abs(value) !== Infinity; } - static typeCastNumeric(value, serverVersion) { - // Override serverVersion for testing, otherwise I'd mock isHighPrecision - const highPrecision = ( - (typeof serverVersion === 'undefined' && RuleCategoryRange.isHighPrecision()) || - (serverVersion >= HP_VERSION) - ); - const castableTypes = TypeChecker.castableTypes(value, highPrecision); - // We rely on Double and Decimal128 being first in the list, - // which is fragile and hence is unit tested - return TypeChecker.cast(value, castableTypes[0]); - } + // static typeCastNumeric(value, serverVersion) { + // // Override serverVersion for testing, otherwise I'd mock isHighPrecision + // const highPrecision = ( + // (typeof serverVersion === 'undefined' && RuleCategoryRange.isHighPrecision()) || + // (serverVersion >= HP_VERSION) + // ); + // const castableTypes = TypeChecker.castableTypes(value, highPrecision); + // // We rely on Double and Decimal128 being first in the list, + // // which is fragile and hence is unit tested + // return TypeChecker.cast(value, castableTypes[0]); + // } static paramsToQuery(params) { const result = {}; if (params.upperBoundType) { - result[params.upperBoundType] = RuleCategoryRange.typeCastNumeric(params.upperBoundValue); + result[params.upperBoundType] = parseFloat(params.upperBoundValue, 10); } if (params.lowerBoundType) { - result[params.lowerBoundType] = RuleCategoryRange.typeCastNumeric(params.lowerBoundValue); + result[params.lowerBoundType] = parseFloat(params.lowerBoundValue, 10); } return result; } @@ -211,7 +211,7 @@ class RuleCategoryRange extends React.Component { */ validateCombinedValues(lower, upper) { // TODO Can't compare two Decimal128's for correctness easily in JS... - const highPrecision = false; + // const highPrecision = false; // first check that not both values are "none" if (!upper && !lower) { @@ -222,18 +222,18 @@ class RuleCategoryRange extends React.Component { return true; } - const castedUpper = TypeChecker.cast( - upper, - TypeChecker.castableTypes(upper, highPrecision)[0] - ); - - const castedLower = TypeChecker.cast( - lower, - TypeChecker.castableTypes(lower, highPrecision)[0] - ); + // const castedUpper = TypeChecker.cast( + // upper, + // TypeChecker.castableTypes(upper, highPrecision)[0] + // ); + // + // const castedLower = TypeChecker.cast( + // lower, + // TypeChecker.castableTypes(lower, highPrecision)[0] + // ); // only return true if the lower value strictly less than the upper value - return castedLower < castedUpper; + return parseFloat(lower, 10) < parseFloat(upper, 10); } /** From 8214d3312544f0ef33cc6d8fb34d258cfeec0185 Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Thu, 3 Nov 2016 22:43:21 +1100 Subject: [PATCH 37/43] cancelChanges no longer takes a boolean. --- .../validation/lib/components/rule-builder.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/internal-packages/validation/lib/components/rule-builder.jsx b/src/internal-packages/validation/lib/components/rule-builder.jsx index 1b36ca35b96..a701703e2ae 100644 --- a/src/internal-packages/validation/lib/components/rule-builder.jsx +++ b/src/internal-packages/validation/lib/components/rule-builder.jsx @@ -53,7 +53,8 @@ class RuleBuilder extends React.Component { */ onCancel() { this.isValid = true; - ValidationActions.cancelChanges(true); + this.childValidationStates = {}; + ValidationActions.cancelChanges(); } /** From 6f5e7d6eda6cfa9a867840ab50dfdc52d03440fe Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Sun, 6 Nov 2016 22:25:00 +1100 Subject: [PATCH 38/43] validation rewritten. only use single validate() method --- .../lib/components/common/range-input.jsx | 39 ++--- .../lib/components/rule-builder.jsx | 47 +++--- .../lib/components/rule-categories/exists.jsx | 7 +- .../rule-categories/mustnotexist.jsx | 7 +- .../lib/components/rule-categories/range.jsx | 137 ++++++------------ .../lib/components/rule-categories/regex.jsx | 23 ++- .../lib/components/rule-categories/type.jsx | 7 +- .../lib/components/rule-category-selector.jsx | 30 +++- .../lib/components/rule-field-selector.jsx | 17 ++- .../validation/lib/components/rule.jsx | 48 +++--- .../validation/lib/stores/index.js | 4 +- 11 files changed, 157 insertions(+), 209 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index cbd796b8f40..0b749331a75 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -34,7 +34,8 @@ class RangeInput extends React.Component { disabled: op === 'none', operator: op, value: this.props.value, - validationState: null + isValid: true, + hasStartedValidating: false }; this._ENABLE_HP = app.instance && ( app.instance.build.version >= HP_VERSION); @@ -57,12 +58,11 @@ class RangeInput extends React.Component { * validate the input and if it is valid, report the value change up to the parent. */ onInputBlur() { - if (this.validate()) { - this.props.onRangeInputBlur({ - value: this.state.value, - operator: this.state.operator - }); - } + this.validate(true); + this.props.onRangeInputBlur({ + value: this.state.value, + operator: this.state.operator + }); } /** @@ -83,12 +83,18 @@ class RangeInput extends React.Component { } /** - * determines if the input by itself is valid (e.g. a value that can be cast to a number). - * This will also report the result up to the parent via `this.props.validate()`. + * determines if the input by itself is valid (e.g. a value that can be + * cast to a number). * * @return {Boolean} whether the input is valid or not. */ - validate() { + validate(force) { + if (!force && !this.state.hasStartedValidating) { + return true; + } + if (this.state.disabled) { + return true; + } const value = this.state.value; const valueTypes = TypeChecker.castableTypes(value, this._ENABLE_HP); @@ -100,11 +106,11 @@ class RangeInput extends React.Component { 'Decimal128' ]; - const isValid = (_.intersection(valueTypes, NUMBER_TYPES).length); + const isValid = (_.intersection(valueTypes, NUMBER_TYPES).length > 0); this.setState({ - validationState: isValid ? null : 'error' + isValid: isValid, + hasStartedValidating: true }); - this.props.validate(isValid); return isValid; } @@ -161,9 +167,9 @@ class RangeInput extends React.Component { } // not disabled, render input group with value input and operator dropdown const placeholder = `${boundString}`.toLowerCase(); - + const validationState = this.state.isValid ? null : 'error'; return ( - + { - this.refs[rule.id].validate(); - }); - return; - } - this.childValidationStates[key] = valid; - this.isValid = _.all(_.values(this.childValidationStates)); - this.forceUpdate(); + validate(force) { + const isValid = _.all(this.props.validationRules, (rule) => { + return this.refs[rule.id].validate(force); + }); + this.setState({ + isValid: isValid + }); + return isValid; } renderRules() { return _.map(this.props.validationRules, (rule) => { - return ( - - ); + return ; }); } /** @@ -106,7 +99,7 @@ class RuleBuilder extends React.Component { onUpdate: this.onUpdate.bind(this) }; - if (!this.isValid) { + if (!this.state.isValid) { editableProps.editState = 'error'; editableProps.errorMessage = 'Input is not valid.'; delete editableProps.childName; diff --git a/src/internal-packages/validation/lib/components/rule-categories/exists.jsx b/src/internal-packages/validation/lib/components/rule-categories/exists.jsx index 3f36b3cce60..48b7a7e45e5 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/exists.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/exists.jsx @@ -9,10 +9,6 @@ class RuleCategoryExists extends React.Component { super(props); } - componentWillMount() { - this.props.validate(true); - } - /** * get the initial parameters for this rule category. * @@ -64,8 +60,7 @@ class RuleCategoryExists extends React.Component { RuleCategoryExists.propTypes = { id: React.PropTypes.string.isRequired, - parameters: React.PropTypes.object.isRequired, - validate: React.PropTypes.func.isRequired + parameters: React.PropTypes.object.isRequired }; RuleCategoryExists.displayName = 'RuleCategoryExists'; diff --git a/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx b/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx index beccebd83c5..31d47571ff3 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx @@ -10,10 +10,6 @@ class RuleCategoryMustNotExist extends React.Component { super(props); } - componentWillMount() { - this.props.validate(true); - } - /** * get the initial parameters for this rule category. * @@ -65,8 +61,7 @@ class RuleCategoryMustNotExist extends React.Component { RuleCategoryMustNotExist.propTypes = { id: React.PropTypes.string.isRequired, - parameters: React.PropTypes.object.isRequired, - validate: React.PropTypes.func.isRequired + parameters: React.PropTypes.object.isRequired }; RuleCategoryMustNotExist.displayName = 'RuleCategoryMustNotExist'; diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index 9cbf879da60..f8b898f9ca2 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -2,12 +2,11 @@ const React = require('react'); const _ = require('lodash'); const ValidationAction = require('../../actions'); const RangeInput = require('../common/range-input'); -const TypeChecker = require('hadron-type-checker'); const app = require('ampersand-app'); const bootstrap = require('react-bootstrap'); const FormGroup = bootstrap.FormGroup; -// const debug = require('debug')('mongodb-compass:validation'); +// const debug = require('debug')('mongodb-compass:validation:range'); /** * The version at which high precision values are available. @@ -18,41 +17,17 @@ class RuleCategoryRange extends React.Component { constructor(props) { super(props); - // this tracks whether the children individually are valid or not. - // it is a bit unusually to make this an instance variable, but since - // the component does not have to re-render itself when this changes, - // it's ok. We also had issues making this part of state because the - // state does not update immediately, which was causing issues. - // - // @see http://stackoverflow.com/questions/30782948/why-calling-react-setstate-method-doesnt-mutate-the-state-immediately - this.childrenIndividuallyValid = true; - - // Additionally, we maintain the validation states of the two - // children so that we can determine if both of them are valid. - this.isValid = true; - this.childValidationStates = {}; - - // We are forking the `parameters` passed in as props here, and are - // updating them whenever a child component changes its values - // (onRangeInputBlur). this.state = { + isValid: true, + combinedValid: true, parameters: _.clone(this.props.parameters) }; } - componentWillMount() { - this.props.validate(true); - } - /** * callback called by the child components () whenever * they change their value or operator. This method then updates the - * internal `state.parameters` accordingly and determines if the two - * combined values are still considered a valid state or not. - * - * Note: individual value validation (i.e. are both children individually - * valid) happens in `this.validate()` and is tracked by - * this.childrenIndividuallyValid. + * internal `state.parameters` accordingly. * * @param {String} key the key of the child component, `lower` or `upper` * @param {Object} value the value passed up from the child of the shape @@ -89,26 +64,14 @@ class RuleCategoryRange extends React.Component { this.setState({ parameters: params }); - - // this component is valid if the children are individually valid and - // their combined values are also valid. - this.isValid = this.validateCombinedValues( - params.lowerBoundType && params.lowerBoundValue, - params.upperBoundType && params.upperBoundValue - ) && this.childrenIndividuallyValid; - - // report up the chain to the parent our overall valid state - this.props.validate(this.isValid); - if (this.isValid) { - ValidationAction.setRuleParameters(this.props.id, params); - } + ValidationAction.setRuleParameters(this.props.id, params); } static getInitialParameters() { return { - upperBoundValue: null, + upperBoundValue: '', upperBoundType: '$lte', - lowerBoundValue: null, + lowerBoundValue: '', lowerBoundType: '$gte' }; } @@ -209,56 +172,54 @@ class RuleCategoryRange extends React.Component { * * @return {Boolean} whether the combined values are valid or not */ - validateCombinedValues(lower, upper) { - // TODO Can't compare two Decimal128's for correctness easily in JS... - // const highPrecision = false; + validateCombinedValues() { + const { + upperBoundType, + upperBoundValue, + lowerBoundType, + lowerBoundValue + } = this.state.parameters; // first check that not both values are "none" - if (!upper && !lower) { + if (!upperBoundType && !lowerBoundType) { return false; } // if only one value is "none", it's automatically valid - if (!upper || !lower) { + if (!upperBoundType || !lowerBoundType) { return true; } - // const castedUpper = TypeChecker.cast( - // upper, - // TypeChecker.castableTypes(upper, highPrecision)[0] - // ); - // - // const castedLower = TypeChecker.cast( - // lower, - // TypeChecker.castableTypes(lower, highPrecision)[0] - // ); + // if one of the fields is still empty, don't invalidate here. The + // RangeInput component will take care of this case. + if (upperBoundValue === '' || lowerBoundValue === '') { + return true; + } + + // if either of the children don't validate, return true here as that + // case is handled in the children directly, and we're not invalidating + // both children if just one is invalid. + if (!_.result(this.refs.lower, 'validate', false) || + !_.result(this.refs.upper, 'validate', false)) { + return true; + } // only return true if the lower value strictly less than the upper value - return parseFloat(lower, 10) < parseFloat(upper, 10); + return parseFloat(lowerBoundValue, 10) < parseFloat(upperBoundValue, 10); } - /** - * callback for child components () to be called when they - * validate their own state. In this method, we only compute if all children - * are individually valid. If they are not, we can immediately report up the - * chain that this component is currently not valid. Otherwise, we have to - * check the combined validity, which is done in `this.validateCombinedValues`. - * - * @param {String} key The key of the child component: `lower` or `upper` - * @param {Boolean} valid The valid state of the child component - */ - validate(key, valid) { - if (key === undefined) { - // downwards validation, call children's validate() method. - this.refs.lowerBoundRangeInputChild.validate(); - this.refs.upperBoundRangeInputChild.validate(); - return; - } - this.childValidationStates[key] = valid; - this.childrenIndividuallyValid = _.all(_.values(this.childValidationStates)); - - if (!this.childrenIndividuallyValid) { - this.props.validate(false); - } + validate(force) { + // if (!force && !this.state.hasStartedValidating) { + // return true; + // } + const lowerValid = this.refs.lower.validate(force); + const upperValid = this.refs.upper.validate(force); + const combinedValid = this.validateCombinedValues(); + const isValid = lowerValid && upperValid && combinedValid; + this.setState({ + isValid: isValid, + combinedValid: combinedValid + }); + return isValid; } /** @@ -267,28 +228,25 @@ class RuleCategoryRange extends React.Component { * @returns {React.Component} The view component. */ render() { - const validationState = (!this.isValid && this.childrenIndividuallyValid) ? - 'error' : null; + const validationState = this.state.combinedValid ? null : 'error'; return ( ); @@ -297,8 +255,7 @@ class RuleCategoryRange extends React.Component { RuleCategoryRange.propTypes = { id: React.PropTypes.string.isRequired, - parameters: React.PropTypes.object.isRequired, - validate: React.PropTypes.func.isRequired + parameters: React.PropTypes.object.isRequired }; RuleCategoryRange.displayName = 'RuleCategoryRange'; diff --git a/src/internal-packages/validation/lib/components/rule-categories/regex.jsx b/src/internal-packages/validation/lib/components/rule-categories/regex.jsx index eaa777989e8..28ada2762b5 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/regex.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/regex.jsx @@ -24,10 +24,6 @@ class RuleCategoryRegex extends React.Component { }; } - componentWillMount() { - this.props.validate(true); - } - onChange(evt) { this.setState({ value: evt.target.value @@ -48,6 +44,13 @@ class RuleCategoryRegex extends React.Component { this.submit(); } + onBlur() { + ValidationAction.setRuleParameters(this.props.id, { + regex: this.state.value, + options: this.state.options + }); + } + /** * get the initial parameters for this rule category. * @@ -114,13 +117,6 @@ class RuleCategoryRegex extends React.Component { return false; } - submit() { - ValidationAction.setRuleParameters(this.props.id, { - regex: this.state.value, - options: this.state.options - }); - } - isOptionSelected(option) { return this.state.options.indexOf(option) > -1; } @@ -139,7 +135,7 @@ class RuleCategoryRegex extends React.Component { value={this.state.value} placeholder="Enter regular expression" onChange={this.onChange.bind(this)} - onBlur={this.submit.bind(this)} + onBlur={this.onBlur.bind(this)} /> @@ -68,8 +83,7 @@ class RuleCategorySelector extends React.Component { RuleCategorySelector.propTypes = { id: React.PropTypes.string.isRequired, - category: React.PropTypes.string.isRequired, - validate: React.PropTypes.func + category: React.PropTypes.string.isRequired }; RuleCategorySelector.displayName = 'RuleCategorySelector'; diff --git a/src/internal-packages/validation/lib/components/rule-field-selector.jsx b/src/internal-packages/validation/lib/components/rule-field-selector.jsx index abab836230e..e7618d4ed15 100644 --- a/src/internal-packages/validation/lib/components/rule-field-selector.jsx +++ b/src/internal-packages/validation/lib/components/rule-field-selector.jsx @@ -20,7 +20,8 @@ class RuleFieldSelector extends React.Component { super(props); this.state = { value: this.props.field, - isValid: true + isValid: true, + hasStartedValidating: false }; } @@ -41,15 +42,18 @@ class RuleFieldSelector extends React.Component { * the store of the change. */ onBlur() { - this.validate(); + this.validate(true); ValidationActions.setRuleField(this.props.id, this.state.value); } - validate() { + validate(force) { + if (!force && !this.state.hasStartedValidating) { + return true; + } const isValid = this.state.value !== ''; - this.props.validate(isValid); this.setState({ - isValid: isValid + isValid: isValid, + hasStartedValidating: true }); return isValid; } @@ -77,8 +81,7 @@ class RuleFieldSelector extends React.Component { RuleFieldSelector.propTypes = { id: React.PropTypes.string.isRequired, - field: React.PropTypes.string.isRequired, - validate: React.PropTypes.func + field: React.PropTypes.string.isRequired }; RuleFieldSelector.displayName = 'RuleFieldSelector'; diff --git a/src/internal-packages/validation/lib/components/rule.jsx b/src/internal-packages/validation/lib/components/rule.jsx index 43b52f7f8da..4ad1289e6d1 100644 --- a/src/internal-packages/validation/lib/components/rule.jsx +++ b/src/internal-packages/validation/lib/components/rule.jsx @@ -19,12 +19,12 @@ class Rule extends React.Component { constructor(props) { super(props); - this.isValid = true; - this.childValidationStates = {}; + this.state = { + isValid: true + }; } onDeleteClicked() { - this.props.validate(true); ValidationActions.deleteValidationRule(this.props.id); } @@ -32,27 +32,25 @@ class Rule extends React.Component { ValidationActions.setRuleNullable(this.props.id, !this.props.nullable); } - /** - * called by child components when they evaluate if they are valid. - * This method saves the valid state for each child, and then determine - * if itself is valid. This is only the case when all children are valid. - * It then reports its own state up the chain by calling this.props.validate(). - * - * @param {String} key an identifier string for the child component - * @param {Boolean} valid whether the child was valid or not + * validates all children components and combines the result for its own + * isValid state. + * @param {Boolean} force force validation (before submit) + * @return {Boolean} whether or not it valid inputs. */ - validate(key, valid) { - if (key === undefined) { - // downwards validation, call children's validate() method. - _.result(this.refs.Parameters, 'validate'); - this.refs.RuleFieldSelector.validate(); - this.refs.RuleCategorySelector.validate(); - return; + validate(force) { + const fieldValid = this.refs.RuleFieldSelector.validate(force); + const categoryValid = this.refs.RuleCategorySelector.validate(force); + + let isValid = fieldValid && categoryValid; + if (this.refs.Parameters && this.refs.Parameters.validate) { + isValid = isValid && this.refs.Parameters.validate(force); } - this.childValidationStates[key] = valid; - this.isValid = _.all(_.values(this.childValidationStates)); - this.props.validate(this.isValid); + + this.setState({ + isValid: isValid + }); + return isValid; } render() { @@ -62,10 +60,9 @@ class Rule extends React.Component { ref="Parameters" id={this.props.id} parameters={this.props.parameters} - validate={this.validate.bind(this, 'Parameters')} /> : null; - const nullableDisabled = _.includes(['exists', 'mustNotExist'], + const nullableDisabled = _.includes(['exists', 'mustNotExist', ''], this.props.category); return ( @@ -75,7 +72,6 @@ class Rule extends React.Component { ref="RuleFieldSelector" id={this.props.id} field={this.props.field} - validate={this.validate.bind(this, 'RuleFieldSelector')} /> @@ -84,7 +80,6 @@ class Rule extends React.Component { ref="RuleCategorySelector" id={this.props.id} category={this.props.category} - validate={this.validate.bind(this, 'RuleCategorySelector')} /> {ruleParameters} @@ -111,8 +106,7 @@ Rule.propTypes = { field: React.PropTypes.string.isRequired, category: React.PropTypes.string.isRequired, parameters: React.PropTypes.object.isRequired, - nullable: React.PropTypes.bool.isRequired, - validate: React.PropTypes.func.isRequired + nullable: React.PropTypes.bool.isRequired }; Rule.displayName = 'Rule'; diff --git a/src/internal-packages/validation/lib/stores/index.js b/src/internal-packages/validation/lib/stores/index.js index cea65ed2996..77dd8bca6d6 100644 --- a/src/internal-packages/validation/lib/stores/index.js +++ b/src/internal-packages/validation/lib/stores/index.js @@ -332,7 +332,9 @@ const ValidationStore = Reflux.createStore({ _.includes(_.keys(ruleCategories), category)) { rules[ruleIndex].category = category; rules[ruleIndex].parameters = ruleCategories[category].getInitialParameters(); - + if (_.contains(['exists', 'mustNotExist'], category)) { + rules[ruleIndex].nullable = false; + } this._updateState({rules: rules}); } }, From 7d9cf64e1e9ba8181036ff7ba54dbdc1344962c5 Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Mon, 7 Nov 2016 00:37:14 +1100 Subject: [PATCH 39/43] use componentWillReceiveProps where props are forked. --- .../lib/components/rule-categories/exists.jsx | 4 ---- .../components/rule-categories/mustnotexist.jsx | 4 ---- .../lib/components/rule-categories/range.jsx | 6 ++++++ .../lib/components/rule-categories/regex.jsx | 14 +++++++------- .../lib/components/rule-categories/type.jsx | 4 ---- .../lib/components/rule-field-selector.jsx | 7 +++++++ .../validation/lib/stores/index.js | 1 - 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-categories/exists.jsx b/src/internal-packages/validation/lib/components/rule-categories/exists.jsx index 48b7a7e45e5..92dbaef323a 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/exists.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/exists.jsx @@ -5,10 +5,6 @@ const _ = require('lodash'); class RuleCategoryExists extends React.Component { - constructor(props) { - super(props); - } - /** * get the initial parameters for this rule category. * diff --git a/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx b/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx index 31d47571ff3..1ba74105645 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx @@ -6,10 +6,6 @@ const _ = require('lodash'); class RuleCategoryMustNotExist extends React.Component { - constructor(props) { - super(props); - } - /** * get the initial parameters for this rule category. * diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index f8b898f9ca2..c1ff44c2674 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -24,6 +24,12 @@ class RuleCategoryRange extends React.Component { }; } + componentWillReceiveProps(props) { + this.setState({ + parameters: _.clone(props.parameters) + }); + } + /** * callback called by the child components () whenever * they change their value or operator. This method then updates the diff --git a/src/internal-packages/validation/lib/components/rule-categories/regex.jsx b/src/internal-packages/validation/lib/components/rule-categories/regex.jsx index 28ada2762b5..570f4795c33 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/regex.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/regex.jsx @@ -24,6 +24,13 @@ class RuleCategoryRegex extends React.Component { }; } + componentWillReceiveProps(nextProps) { + this.setState({ + value: _.get(nextProps.parameters, 'regex', ''), + options: _.get(nextProps.parameters, 'options', '') + }); + } + onChange(evt) { this.setState({ value: evt.target.value @@ -63,13 +70,6 @@ class RuleCategoryRegex extends React.Component { }; } - willReceiveProps(nextProps) { - this.setState({ - value: _.get(nextProps.parameters, 'regex', ''), - options: _.get(nextProps.parameters, 'options', '') - }); - } - /** * Convert the parameters describing the state of this rule to a query * value that MongoDB understands. diff --git a/src/internal-packages/validation/lib/components/rule-categories/type.jsx b/src/internal-packages/validation/lib/components/rule-categories/type.jsx index 035a9598b77..7c1ddbf255e 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/type.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/type.jsx @@ -7,10 +7,6 @@ const _ = require('lodash'); class RuleCategoryType extends React.Component { - constructor(props) { - super(props); - } - onTypeClicked(type, evt) { evt.preventDefault(); ValidationAction.setRuleParameters(this.props.id, { diff --git a/src/internal-packages/validation/lib/components/rule-field-selector.jsx b/src/internal-packages/validation/lib/components/rule-field-selector.jsx index e7618d4ed15..986366238be 100644 --- a/src/internal-packages/validation/lib/components/rule-field-selector.jsx +++ b/src/internal-packages/validation/lib/components/rule-field-selector.jsx @@ -1,6 +1,7 @@ const React = require('react'); const ValidationActions = require('../actions'); const { FormGroup, FormControl } = require('react-bootstrap'); + /** * Component to select a field for which the rule applies. * @@ -25,6 +26,12 @@ class RuleFieldSelector extends React.Component { }; } + componentWillReceiveProps(props) { + this.setState({ + value: props.field + }); + } + /** * The content of the input field has changed (onChange). Update internal * state but do not trigger an action to change the store yet. diff --git a/src/internal-packages/validation/lib/stores/index.js b/src/internal-packages/validation/lib/stores/index.js index 77dd8bca6d6..463cbffadb0 100644 --- a/src/internal-packages/validation/lib/stores/index.js +++ b/src/internal-packages/validation/lib/stores/index.js @@ -424,7 +424,6 @@ const ValidationStore = Reflux.createStore({ cancelChanges() { const params = this._deconstructValidatorDoc(this.lastFetchedValidatorDoc); this._updateState(params); - return; }, saveChanges() { From 478057a71840d001b7835d0de198be14e7dd9a9f Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Mon, 7 Nov 2016 00:55:54 +1100 Subject: [PATCH 40/43] force a redraw when cancel was pressed via key change --- .../validation/lib/components/rule-builder.jsx | 15 ++++++++++++--- .../lib/components/rule-categories/regex.jsx | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-builder.jsx b/src/internal-packages/validation/lib/components/rule-builder.jsx index 17e0fe2b1e0..7a398beac10 100644 --- a/src/internal-packages/validation/lib/components/rule-builder.jsx +++ b/src/internal-packages/validation/lib/components/rule-builder.jsx @@ -19,12 +19,20 @@ class RuleBuilder extends React.Component { constructor(props) { super(props); this.state = { - isValid: true + isValid: true, + forceRenderKey: 0 }; } - componentWillReceiveProps() { + componentWillReceiveProps(props) { this.validate(false); + if (props.editState === 'unmodified' && this.props.editState !== 'unmodified') { + // force a complete redraw of the component by increasing the key + this.setState({ + forceRenderKey: this.state.forceRenderKey + 1, + isValid: true + }); + } } /** @@ -96,7 +104,8 @@ class RuleBuilder extends React.Component { editState: this.props.editState, childName: 'Validation', onCancel: this.onCancel.bind(this), - onUpdate: this.onUpdate.bind(this) + onUpdate: this.onUpdate.bind(this), + key: this.state.forceRenderKey }; if (!this.state.isValid) { diff --git a/src/internal-packages/validation/lib/components/rule-categories/regex.jsx b/src/internal-packages/validation/lib/components/rule-categories/regex.jsx index 570f4795c33..a8f3f84e95f 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/regex.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/regex.jsx @@ -48,7 +48,7 @@ class RuleCategoryRegex extends React.Component { } onDropdownClosed() { - this.submit(); + this.onBlur(); } onBlur() { From 20f734c428468f81a1a5cef63feae8c1e5b3aa1f Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Mon, 7 Nov 2016 01:00:04 +1100 Subject: [PATCH 41/43] :shirt: fixing eslint issues. --- .../validation/lib/components/common/range-input.jsx | 3 ++- src/internal-packages/validation/lib/stores/index.js | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal-packages/validation/lib/components/common/range-input.jsx b/src/internal-packages/validation/lib/components/common/range-input.jsx index 0b749331a75..04524c28794 100644 --- a/src/internal-packages/validation/lib/components/common/range-input.jsx +++ b/src/internal-packages/validation/lib/components/common/range-input.jsx @@ -86,7 +86,8 @@ class RangeInput extends React.Component { * determines if the input by itself is valid (e.g. a value that can be * cast to a number). * - * @return {Boolean} whether the input is valid or not. + * @param {Boolean} force forces validation from now on. + * @return {Boolean} whether the input is valid or not. */ validate(force) { if (!force && !this.state.hasStartedValidating) { diff --git a/src/internal-packages/validation/lib/stores/index.js b/src/internal-packages/validation/lib/stores/index.js index 463cbffadb0..98a4df45dcf 100644 --- a/src/internal-packages/validation/lib/stores/index.js +++ b/src/internal-packages/validation/lib/stores/index.js @@ -2,7 +2,6 @@ const Reflux = require('reflux'); const ValidationActions = require('../actions'); const StateMixin = require('reflux-state-mixin'); const _ = require('lodash'); -const uuid = require('uuid'); const ruleCategories = require('../components/rule-categories'); const helper = require('./helpers'); const toNS = require('mongodb-ns'); From 106d49643b8bb75c5d90a327ed6eb402e90c619a Mon Sep 17 00:00:00 2001 From: Peter Schmidt Date: Tue, 8 Nov 2016 16:02:25 +1100 Subject: [PATCH 42/43] Drop HP_VERSION until COMPASS-294 --- .../lib/components/rule-categories/range.jsx | 27 ----------- test/validation.range.test.js | 46 ------------------- 2 files changed, 73 deletions(-) diff --git a/src/internal-packages/validation/lib/components/rule-categories/range.jsx b/src/internal-packages/validation/lib/components/rule-categories/range.jsx index c1ff44c2674..0788fddf29d 100644 --- a/src/internal-packages/validation/lib/components/rule-categories/range.jsx +++ b/src/internal-packages/validation/lib/components/rule-categories/range.jsx @@ -2,17 +2,11 @@ const React = require('react'); const _ = require('lodash'); const ValidationAction = require('../../actions'); const RangeInput = require('../common/range-input'); -const app = require('ampersand-app'); const bootstrap = require('react-bootstrap'); const FormGroup = bootstrap.FormGroup; // const debug = require('debug')('mongodb-compass:validation:range'); -/** - * The version at which high precision values are available. - */ -const HP_VERSION = '3.4.0'; - class RuleCategoryRange extends React.Component { constructor(props) { @@ -82,15 +76,6 @@ class RuleCategoryRange extends React.Component { }; } - /** - * Are high precision values available? - * - * @returns {boolean} if high precision values are available. - */ - static isHighPrecision() { - return app.instance.build.version >= HP_VERSION; - } - static validateKeyAndValue(key, value) { if (!_.includes(['$gt', '$gte', '$lt', '$lte'], key)) { return false; @@ -105,18 +90,6 @@ class RuleCategoryRange extends React.Component { return !isNaN(value) && Math.abs(value) !== Infinity; } - // static typeCastNumeric(value, serverVersion) { - // // Override serverVersion for testing, otherwise I'd mock isHighPrecision - // const highPrecision = ( - // (typeof serverVersion === 'undefined' && RuleCategoryRange.isHighPrecision()) || - // (serverVersion >= HP_VERSION) - // ); - // const castableTypes = TypeChecker.castableTypes(value, highPrecision); - // // We rely on Double and Decimal128 being first in the list, - // // which is fragile and hence is unit tested - // return TypeChecker.cast(value, castableTypes[0]); - // } - static paramsToQuery(params) { const result = {}; if (params.upperBoundType) { diff --git a/test/validation.range.test.js b/test/validation.range.test.js index f6c6561b0fc..9263ada9c7b 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -120,50 +120,4 @@ describe('', function() { component.instance().onRangeInputBlur('fake-key'); expect(component.instance().isValid).to.be.false; }); - - context('for some different numeric types', function() { - const someInt32 = '32'; - const someLong = '9007199254740991'; // 2^53-1, higher nums => Decimal128 - const someDouble = '0.1'; - const someDecimal128 = '9.999999999999999999999999999999999E+6144'; - - context('when server version is 3.2.10', function() { - const serverVersion = '3.2.10'; - it('accepts Int32', function() { - const result = RuleCategoryRange.typeCastNumeric(someInt32, serverVersion); - expect(result._bsontype).to.be.equal('Int32'); - }); - it('accepts Long', function() { - const result = RuleCategoryRange.typeCastNumeric(someLong, serverVersion); - expect(result._bsontype).to.be.equal('Long'); - }); - it('accepts Double', function() { - const result = RuleCategoryRange.typeCastNumeric(someDouble, serverVersion); - expect(result._bsontype).to.be.equal('Double'); - }); - it('rejects Decimal128', function() { - const result = RuleCategoryRange.typeCastNumeric(someDecimal128, serverVersion); - expect(result._bsontype).to.be.undefined; - }); - }); - context('when server version is 3.4.0', function() { - const serverVersion = '3.4.0'; - it('accepts Int32', function() { - const result = RuleCategoryRange.typeCastNumeric(someInt32, serverVersion); - expect(result._bsontype).to.be.equal('Int32'); - }); - it('accepts Long', function() { - const result = RuleCategoryRange.typeCastNumeric(someLong, serverVersion); - expect(result._bsontype).to.be.equal('Long'); - }); - it('accepts Double', function() { - const result = RuleCategoryRange.typeCastNumeric(someDouble, serverVersion); - expect(result._bsontype).to.be.equal('Double'); - }); - it('accepts Decimal128', function() { - const result = RuleCategoryRange.typeCastNumeric(someDecimal128, serverVersion); - expect(result._bsontype).to.be.equal('Decimal128'); - }); - }); - }); }); From f9eb65b29076346dd0c578cc3a70791a838eeaf7 Mon Sep 17 00:00:00 2001 From: Thomas Rueckstiess Date: Tue, 8 Nov 2016 16:09:42 +1100 Subject: [PATCH 43/43] :green_heart: fix tests. --- test/validation.range.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/validation.range.test.js b/test/validation.range.test.js index 9263ada9c7b..f3439d922cf 100644 --- a/test/validation.range.test.js +++ b/test/validation.range.test.js @@ -103,8 +103,8 @@ describe('', function() { } }); component = mount(); - component.instance().onRangeInputBlur('fake-key'); - expect(component.instance().isValid).to.be.false; + const result = component.instance().validate(); + expect(result).to.be.false; }); it('rejects empty both range values being "none" after calling validate()', function() { @@ -117,7 +117,7 @@ describe('', function() { } }); component = mount(); - component.instance().onRangeInputBlur('fake-key'); - expect(component.instance().isValid).to.be.false; + const result = component.instance().validate(); + expect(result).to.be.false; }); });