COMPASS-226 hiding decimal128 option if server < 3.4 (#567)

* COMPASS-226 added string comparison to server version to determine decimal hiding

* COMPASS-226 commented out debug statement

* COMPASS-226 testing for travis issues

* COMPASS-226 put server version info in validation store

* COMPASS-226 semvar is struggling with chai tests

* COMPASS-226 slightly cleaner implementation

* COMPASS-226 removed hanging .isRequired in props that add warnings in tests

* Revert unnecessary SortableTable change

See e71d665

* Clean up comments

Author: Satya

src/internal-packages/validation/lib/components/common/bson-type-selector.jsx

@@ -1,8 +1,9 @@
 const React = require('react');
 const _ = require('lodash');
 const OptionSelector = require('./option-selector');
+const semver = require('semver');
 
-// const debug = require('debug')('mongodb-compass:validation');
+// const debug = require('debug')('mongodb-compass:bson-type-selector');
 
 const BSON_TYPES = [
   {name: 'Double', number: 1, alias: 'double'},
@@ -28,11 +29,23 @@ const BSON_TYPES = [
   {name: 'Max key', number: 127, alias: 'maxKey'}
 ];
 
-
 class BSONTypeSelector extends React.Component {
 
   constructor(props) {
     super(props);
+    // try catch block in case semver typeerrors while remove decimal128
+    // if server is < 3.4.x
+    let canRemoveDecimal = false;
+    try {
+      canRemoveDecimal = semver.gt('3.4.0-rc0', this.props.serverVersion);
+    } catch (e) {
+      canRemoveDecimal = true;
+    }
+
+    if (canRemoveDecimal) {
+      _.remove(BSON_TYPES, (type) => type.number === 19);
+    }
+
     this.state = {
       type: null
     };
@@ -81,13 +94,7 @@ class BSONTypeSelector extends React.Component {
    */
   render() {
     const selectedTypeName = _.get(this.state.type, 'alias', '');
-    // TODO: make recentServer true if server is 3.4 <
-    const recentServer = true;
-    // remove the decimal version if not recentServer
-    const typeOptions = _.zipObject(_.map(_.filter(BSON_TYPES, (t) => {
-      // filter out decimal if server < 3.4
-      return !(t.alias === 'decimal128' && !recentServer);
-    }), (type) => {
+    const typeOptions = _.zipObject(_.map(BSON_TYPES, (type) => {
       return [type.alias, type.name];
     }));
 
@@ -107,7 +114,8 @@ BSONTypeSelector.propTypes = {
   typeNumber: React.PropTypes.number,
   typeAlias: React.PropTypes.string,
   typeName: React.PropTypes.string,
-  onTypeClicked: React.PropTypes.func.isRequired
+  onTypeClicked: React.PropTypes.func,
+  serverVersion: React.PropTypes.string
 };
 
 BSONTypeSelector.displayName = 'BSONTypeSelector';

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

@@ -91,7 +91,7 @@ class RuleBuilder extends React.Component {
 
   renderRules() {
     return _.map(this.props.validationRules, (rule) => {
-      return <Rule ref={rule.id} key={rule.id} {...rule} />;
+      return <Rule ref={rule.id} key={rule.id} serverVersion={this.props.serverVersion} {...rule} />;
     });
   }
   /**
@@ -174,7 +174,8 @@ RuleBuilder.propTypes = {
   editState: React.PropTypes.oneOf(['unmodified', 'modified', 'updating', 'error', 'success']).isRequired,
   validationAction: React.PropTypes.oneOf(['warn', 'error']).isRequired,
   validationLevel: React.PropTypes.oneOf(['off', 'moderate', 'strict']).isRequired,
-  validationRules: React.PropTypes.array.isRequired
+  validationRules: React.PropTypes.array.isRequired,
+  serverVersion: React.PropTypes.string
 };
 
 RuleBuilder.displayName = 'RuleBuilder';

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

@@ -80,14 +80,15 @@ class RuleCategoryType extends React.Component {
     const typeNumber = this.props.parameters.type;
 
     return (
-      <BSONTypeSelector typeNumber={typeNumber} onTypeClicked={this.onTypeClicked.bind(this)} />
+      <BSONTypeSelector typeNumber={typeNumber} serverVersion={this.props.serverVersion} onTypeClicked={this.onTypeClicked.bind(this)} />
     );
   }
 }
 
 RuleCategoryType.propTypes = {
   id: React.PropTypes.string.isRequired,
-  parameters: React.PropTypes.object.isRequired
+  parameters: React.PropTypes.object.isRequired,
+  serverVersion: React.PropTypes.string
 };
 
 RuleCategoryType.displayName = 'RuleCategoryType';

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

@@ -59,6 +59,7 @@ class Rule extends React.Component {
       <Parameters
         ref="Parameters"
         id={this.props.id}
+        serverVersion={this.props.serverVersion}
         parameters={this.props.parameters}
       /> : null;
 
@@ -106,7 +107,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,
+  serverVersion: React.PropTypes.string
 };
 
 Rule.displayName = 'Rule';

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

@@ -46,6 +46,7 @@ class Validation extends React.Component {
             validationAction={this.props.validationAction}
             validationLevel={this.props.validationLevel}
             editState={this.props.editState}
+            serverVersion={this.props.serverVersion}
           />
         </div>
       ) : (
@@ -86,7 +87,8 @@ Validation.propTypes = {
   validationAction: React.PropTypes.oneOf(['warn', 'error']).isRequired,
   validatorDoc: React.PropTypes.object.isRequired,
   validationLevel: React.PropTypes.oneOf(['off', 'moderate', 'strict']).isRequired,
-  validationRules: React.PropTypes.array.isRequired
+  validationRules: React.PropTypes.array.isRequired,
+  serverVersion: React.PropTypes.string
 };
 
 Validation.defaultProps = {

src/internal-packages/validation/lib/stores/index.js

@@ -54,7 +54,8 @@ const ValidationStore = Reflux.createStore({
       validationAction: 'warn',    // one of `warn`, `error`
       fetchState: 'initial',       // one of `initial`, `fetching`, `success`, `error`
       editState: 'unmodified',     // one of `unmodified`, `modified`, `updating`, `success`, `error`
-      isExpressibleByRules: true   // boolean
+      isExpressibleByRules: true,   // boolean
+      serverVersion: ''
     };
   },
 
@@ -223,6 +224,11 @@ const ValidationStore = Reflux.createStore({
    */
   _fetchFromServer(callback) {
     const ns = toNS(NamespaceStore.ns);
+
+    if (this.state.serverVersion === '') {
+      const serverVersion = app.instance.build.version;
+      this.setState({serverVersion: serverVersion});
+    }
     app.dataService.listCollections(ns.database, {name: ns.collection}, function(err, res) {
       if (err) {
         return callback(err);

test/validation.store.test.js

@@ -5,7 +5,7 @@ const ValidationStore = require('../src/internal-packages/validation/lib/stores'
 const sinon = require('sinon');
 const _ = require('lodash');
 
-function mockFetchFromServer(err, res, delay) {
+function mockFetchFromServer(err, res, delay, serverVersion) {
   if (delay === undefined) {
     delay = 0;
   }
@@ -14,6 +14,10 @@ function mockFetchFromServer(err, res, delay) {
       return callback(err, res);
     }, delay);
   };
+  // the above fetchFromServer makes a call for app.instance.build.version
+  // which will be undefined at the time of this test so setting serverVersion
+  // manually here, which would otherwise be overwritten
+  ValidationStore.setState({serverVersion: serverVersion});
 }
 
 const mockValidatorDoc = {
@@ -47,6 +51,11 @@ describe('ValidationStore', function() {
     unsubscribe = function() {};
   });
 
+  it('check a pre 3.4 server version', function(done) {
+    mockFetchFromServer(null, mockValidatorDoc, 0, '3.1.9');
+    expect(ValidationStore.state.serverVersion).to.be.equal('3.1.9');
+    done();
+  });
 
   it('goes into {fetchState: "fetching"} when starting to fetch from server', function(done) {
     mockFetchFromServer(null, mockValidatorDoc);