Skip to content

Commit d0a1c11

Browse files
pzrqThomas Rueckstiess
authored andcommitted
COMPASS 235 DV: Full range component (#551)
* Add JSDoc of proposed coupling between RangeInput validationStates * 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. * Add RuleCategoryRange unit and Rule integration tests * Add failing test for the empty range state * 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 * Mostly working, but for some reason renders the stale value... * Refactor componentWillMount into the constructor https://facebook.github.io/react/docs/react-component.html#componentwillmount * Refactor onRangeInputBlur to the end of validate * Remove 10 from parseFloat parseFloat does not take a second argument, unlike parseInt :) * 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. * Be clearer which store * Call validate after changing the dropdownOp value So it appears to start in an invalid state if switching from a 'none' state. * 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 :) * ESlint * Fix race where validationState is not allowed to be empty string react-bootstrap/react-bootstrap#1926 * Finally, playing with the (unvalidated) component works correctly * Refactor into validateCombinedParams So I can reuse it shortly. * 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. * 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. * 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 } } * 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. * Add better regex to handle exponential float forms * 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). * Add some Decimal128 tests * Handle Decimal128 and drop clunky parseFloat uses * Be more explicit that combined ranges of Decimal128s are not validated * shrink range inputs, remove title labels. * 👕 fix linter issue. * don’t validate initially, fix tests. * form validation. such difficult. * [wip] up and downwards validation almost working * fixing some edge cases, updating tests. * Restore tests so accepts/rejects is clearer Just needed to drop the comboValidationState. * make rule id consistent (based on index of rule). * store cancel always needs to call _updateState() * range: disable bson type casting * cancelChanges no longer takes a boolean. * validation rewritten. only use single validate() method * use componentWillReceiveProps where props are forked. * force a redraw when cancel was pressed via key change * 👕 fixing eslint issues. * Drop HP_VERSION until COMPASS-294 * 💚 fix tests.
1 parent cfe43f7 commit d0a1c11

File tree

16 files changed

+749
-220
lines changed

16 files changed

+749
-220
lines changed

src/internal-packages/validation/lib/components/common/range-input.jsx

Lines changed: 88 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,86 +1,118 @@
11
const React = require('react');
2+
const app = require('ampersand-app');
23
const _ = require('lodash');
34
const FormGroup = require('react-bootstrap').FormGroup;
45
const InputGroup = require('react-bootstrap').InputGroup;
56
const FormControl = require('react-bootstrap').FormControl;
67
const DropdownButton = require('react-bootstrap').DropdownButton;
7-
const ControlLabel = require('react-bootstrap').ControlLabel;
88
const MenuItem = require('react-bootstrap').MenuItem;
9+
const TypeChecker = require('hadron-type-checker');
910

1011
// const debug = require('debug')('mongodb-compass:validation:action-selector');
1112

13+
/**
14+
* The version at which high precision values are available.
15+
*/
16+
const HP_VERSION = '3.4.0';
17+
18+
/**
19+
* A RangeInput represents a numeric lower or upper bound and the value of the
20+
* lower or upper bound, if the bound exists.
21+
*
22+
* The `validationState` of this RangeInput can be set to 'error' either:
23+
* - by the RangeInput validating the `value` prop is not of type `number`, or
24+
* - by the RangeInput's parent `RuleCategoryRange` component validating the
25+
* combined range expression, such as `5 < x < 5` is displayed as red/error
26+
* even though the RangeInputs `5 < x` and `5 > x` are individually valid.
27+
*/
1228
class RangeInput extends React.Component {
1329

1430
constructor(props) {
1531
super(props);
32+
const op = this._getOperatorString(props);
1633
this.state = {
17-
disabled: false,
18-
operator: '',
19-
value: '',
20-
validationState: null
21-
};
22-
}
23-
24-
componentWillMount() {
25-
const op = this._getOperatorString();
26-
this.setState({
27-
value: _.isNumber(this.props.value) ? String(this.props.value) : '',
28-
operator: op,
29-
disabled: op === 'none'
30-
});
31-
}
32-
33-
componentWillReceiveProps(nextProps) {
34-
const op = this._getOperatorString(nextProps);
35-
this.setState({
36-
value: _.isNumber(this.props.value) ? String(this.props.value) : '',
34+
disabled: op === 'none',
3735
operator: op,
38-
disabled: op === 'none'
39-
});
36+
value: this.props.value,
37+
isValid: true,
38+
hasStartedValidating: false
39+
};
40+
this._ENABLE_HP = app.instance && (
41+
app.instance.build.version >= HP_VERSION);
4042
}
4143

44+
/**
45+
* called whenever the input changes (i.e. user is typing). We don't bubble up the
46+
* value at this stage yet, but wait until the user blurs the input field.
47+
*
48+
* @param {Object} evt The onChange event
49+
*/
4250
onInputChange(evt) {
4351
this.setState({
4452
value: evt.target.value
4553
});
4654
}
4755

56+
/**
57+
* called whenever the field is blurred (loses focus). At this point, we want to
58+
* validate the input and if it is valid, report the value change up to the parent.
59+
*/
4860
onInputBlur() {
49-
this.validate();
61+
this.validate(true);
62+
this.props.onRangeInputBlur({
63+
value: this.state.value,
64+
operator: this.state.operator
65+
});
5066
}
5167

68+
/**
69+
* called when the user chooses a value from the operator dropdown. As there are
70+
* no invalid values, we can always immediately report up the value/operator change.
71+
*
72+
* @param {Object} evtKey the selected value from the dropdown (e.g. "<=", "none", ...)
73+
*/
5274
onDropdownSelect(evtKey) {
5375
this.setState({
5476
disabled: evtKey === 'none',
5577
operator: evtKey
5678
});
57-
// need to defer validation until setState has propagated
58-
// _.defer(() => {
59-
// this.validate();
60-
// });
79+
this.props.onRangeInputBlur({
80+
value: this.state.value,
81+
operator: evtKey
82+
});
6183
}
6284

63-
validate() {
64-
const value = parseFloat(this.state.value, 10);
65-
let error = false;
66-
if (_.isNaN(value)) {
67-
error = true;
68-
this.setState({
69-
validationState: 'error'
70-
});
71-
} else {
72-
this.setState({
73-
validationState: null
74-
});
85+
/**
86+
* determines if the input by itself is valid (e.g. a value that can be
87+
* cast to a number).
88+
*
89+
* @param {Boolean} force forces validation from now on.
90+
* @return {Boolean} whether the input is valid or not.
91+
*/
92+
validate(force) {
93+
if (!force && !this.state.hasStartedValidating) {
94+
return true;
7595
}
76-
if (this.props.onChange) {
77-
this.props.onChange({
78-
disabled: this.state.disabled,
79-
operator: this.state.operator,
80-
value: value,
81-
hasError: error
82-
});
96+
if (this.state.disabled) {
97+
return true;
8398
}
99+
const value = this.state.value;
100+
const valueTypes = TypeChecker.castableTypes(value, this._ENABLE_HP);
101+
102+
// Not sure if hadron-type-checker should make NUMBER_TYPES public
103+
const NUMBER_TYPES = [
104+
'Long',
105+
'Int32',
106+
'Double',
107+
'Decimal128'
108+
];
109+
110+
const isValid = (_.intersection(valueTypes, NUMBER_TYPES).length > 0);
111+
this.setState({
112+
isValid: isValid,
113+
hasStartedValidating: true
114+
});
115+
return isValid;
84116
}
85117

86118
_getOperatorString(props) {
@@ -124,9 +156,6 @@ class RangeInput extends React.Component {
124156
if (this.state.disabled) {
125157
return (
126158
<FormGroup>
127-
<div>
128-
<ControlLabel>{boundString}</ControlLabel>
129-
</div>
130159
<DropdownButton
131160
id={`range-input-${this.props.upperBound ? 'upper' : 'lower'}`}
132161
style={{width: this.props.width}}
@@ -138,13 +167,10 @@ class RangeInput extends React.Component {
138167
);
139168
}
140169
// not disabled, render input group with value input and operator dropdown
141-
const placeholder = `enter ${boundString}`.toLowerCase();
142-
170+
const placeholder = `${boundString}`.toLowerCase();
171+
const validationState = this.state.isValid ? null : 'error';
143172
return (
144-
<FormGroup validationState={this.state.validationState}>
145-
<div>
146-
<ControlLabel>{boundString}</ControlLabel>
147-
</div>
173+
<FormGroup validationState={this.props.validationState || validationState}>
148174
<InputGroup style={{width: this.props.width}}>
149175
<DropdownButton
150176
id={`range-input-${this.props.upperBound ? 'upper' : 'lower'}`}
@@ -167,22 +193,23 @@ class RangeInput extends React.Component {
167193
}
168194

169195
RangeInput.propTypes = {
170-
value: React.PropTypes.number.isRequired,
196+
value: React.PropTypes.string, // Can't be required to allow "none" in GUI,
197+
// can't be number to work with Decimal128.
171198
upperBound: React.PropTypes.bool,
172199
validationState: React.PropTypes.string,
173200
boundIncluded: React.PropTypes.bool.isRequired,
174201
disabled: React.PropTypes.bool.isRequired,
175-
onChange: React.PropTypes.func,
202+
onRangeInputBlur: React.PropTypes.func,
176203
width: React.PropTypes.number
177204
};
178205

179206
RangeInput.defaultProps = {
180207
disabled: false,
181208
boundIncluded: false,
182209
upperBound: false,
183-
validationState: '',
184-
value: null,
185-
width: 200
210+
validationState: null,
211+
value: '',
212+
width: 160
186213
};
187214

188215
RangeInput.displayName = 'RangeInput';

src/internal-packages/validation/lib/components/rule-builder.jsx

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,24 @@ const Table = ReactBootstrap.Table;
1616

1717
class RuleBuilder extends React.Component {
1818

19+
constructor(props) {
20+
super(props);
21+
this.state = {
22+
isValid: true,
23+
forceRenderKey: 0
24+
};
25+
}
26+
27+
componentWillReceiveProps(props) {
28+
this.validate(false);
29+
if (props.editState === 'unmodified' && this.props.editState !== 'unmodified') {
30+
// force a complete redraw of the component by increasing the key
31+
this.setState({
32+
forceRenderKey: this.state.forceRenderKey + 1,
33+
isValid: true
34+
});
35+
}
36+
}
1937

2038
/**
2139
* Add button clicked to create a new rule.
@@ -52,29 +70,52 @@ class RuleBuilder extends React.Component {
5270

5371
/**
5472
* The "Update" button from the `Editable component has been clicked.
55-
* Send the new validator doc to the server.
73+
* Check that all rules are valid, then send the new validator doc to
74+
* the server.
5675
*/
5776
onUpdate() {
58-
ValidationActions.saveChanges();
77+
if (this.validate(true)) {
78+
ValidationActions.saveChanges();
79+
}
5980
}
6081

82+
validate(force) {
83+
const isValid = _.all(this.props.validationRules, (rule) => {
84+
return this.refs[rule.id].validate(force);
85+
});
86+
this.setState({
87+
isValid: isValid
88+
});
89+
return isValid;
90+
}
91+
92+
renderRules() {
93+
return _.map(this.props.validationRules, (rule) => {
94+
return <Rule ref={rule.id} key={rule.id} {...rule} />;
95+
});
96+
}
6197
/**
6298
* Render status row component.
6399
*
64100
* @returns {React.Component} The component.
65101
*/
66102
render() {
67-
const rules = _.map(this.props.validationRules, (rule) => {
68-
return <Rule key={rule.id} {...rule} />;
69-
});
103+
const editableProps = {
104+
editState: this.props.editState,
105+
childName: 'Validation',
106+
onCancel: this.onCancel.bind(this),
107+
onUpdate: this.onUpdate.bind(this),
108+
key: this.state.forceRenderKey
109+
};
110+
111+
if (!this.state.isValid) {
112+
editableProps.editState = 'error';
113+
editableProps.errorMessage = 'Input is not valid.';
114+
delete editableProps.childName;
115+
}
70116

71117
return (
72-
<Editable
73-
editState={this.props.editState}
74-
childName="Validation"
75-
onCancel={this.onCancel.bind(this)}
76-
onUpdate={this.onUpdate.bind(this)}
77-
>
118+
<Editable {...editableProps} >
78119
<Grid fluid className="rule-builder">
79120
<Row className="header">
80121
<Col lg={6} md={6} sm={6} xs={6}>
@@ -118,7 +159,7 @@ class RuleBuilder extends React.Component {
118159
</tr>
119160
</thead>
120161
<tbody>
121-
{rules}
162+
{this.renderRules()}
122163
</tbody>
123164
</Table>
124165
</Col>

src/internal-packages/validation/lib/components/rule-categories/exists.jsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,6 @@ const _ = require('lodash');
55

66
class RuleCategoryExists extends React.Component {
77

8-
constructor(props) {
9-
super(props);
10-
}
11-
128
/**
139
* get the initial parameters for this rule category.
1410
*

src/internal-packages/validation/lib/components/rule-categories/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@ module.exports = {
22
exists: require('./exists'),
33
mustNotExist: require('./mustnotexist'),
44
type: require('./type'),
5-
// range: require('./range'), // work in progress
5+
range: require('./range'),
66
regex: require('./regex')
77
};

src/internal-packages/validation/lib/components/rule-categories/mustnotexist.jsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,6 @@ const _ = require('lodash');
66

77
class RuleCategoryMustNotExist extends React.Component {
88

9-
constructor(props) {
10-
super(props);
11-
}
12-
139
/**
1410
* get the initial parameters for this rule category.
1511
*

0 commit comments

Comments
 (0)