Provide InputRange component valid value when Range slider is not set. (#20002) (#20735)

* move unsetLogic into react component

* remove extra comment

* add comment about why empty state is needed for InputRange component

* fix broken jest test

* move empty state logic from react component to ListControl class, add comments about reset and clear functions

* add comments about clear and reset to control class

* calculate hasChanged to avoid bug where clear form still shows 'cancel changes' as active for an empty form

* use hasValue in range control to check if value exists

* add unit test for list_control_factory and cleanup range_control from review comments

src/core_plugins/input_control_vis/public/components/vis/list_control.js

@@ -132,5 +132,6 @@ ListControl.defaultProps = {
 };
 
 ListControl.defaultProps = {
+  selectedOptions: [],
   options: [],
 };

src/core_plugins/input_control_vis/public/components/vis/range_control.js

@@ -29,18 +29,22 @@ import {
   EuiFlexItem,
 } from '@elastic/eui';
 
-const toState = (props) => {
+const toState = ({ control }) => {
+  const sliderValue = control.hasValue() ?
+    control.value :
+    // InputRange component does not have an "empty state"
+    // Faking an empty state by setting the slider value range to length of zero anchored at the range minimum
+    {
+      min: control.min,
+      max: control.min
+    };
   const state = {
-    sliderValue: props.control.value,
-    minValue: '',
-    maxValue: '',
+    sliderValue,
+    minValue: control.hasValue() ? control.value.min : '',
+    maxValue: control.hasValue() ? control.value.max : '',
     isRangeValid: true,
     errorMessage: '',
   };
-  if (props.control.hasValue()) {
-    state.minValue = props.control.value.min;
-    state.maxValue = props.control.value.max;
-  }
   return state;
 };
 

src/core_plugins/input_control_vis/public/control/control.js

@@ -100,7 +100,6 @@ export class Control {
 
   set(newValue) {
     this.value = newValue;
-    this._hasChanged = true;
     if (this.hasValue()) {
       this._kbnFilter = this.filterManager.createFilter(this.value);
     } else {
@@ -108,18 +107,23 @@ export class Control {
     }
   }
 
+  /*
+   * Remove any user changes to value by resetting value to that as provided by Kibana filter pills
+   */
   reset() {
-    this._hasChanged = false;
     this._kbnFilter = null;
     this.value = this.filterManager.getValueFromFilterBar();
   }
 
+  /*
+   * Clear any filter on the field by setting the control value to undefined.
+   */
   clear() {
-    this.set(this.filterManager.getUnsetValue());
+    this.value = undefined;
   }
 
   hasChanged() {
-    return this._hasChanged;
+    return !_.isEqual(this.value, this.filterManager.getValueFromFilterBar());
   }
 
   hasKbnFilter() {
@@ -134,6 +138,6 @@ export class Control {
   }
 
   hasValue() {
-    return !_.isEqual(this.value, this.filterManager.getUnsetValue());
+    return this.value !== undefined;
   }
 }

src/core_plugins/input_control_vis/public/control/control.test.js

@@ -28,19 +28,46 @@ function createControlParams(id, label) {
   };
 }
 
-const UNSET_VALUE = '';
+let valueFromFilterBar;
 const mockFilterManager = {
   getValueFromFilterBar: () => {
-    return '';
+    return valueFromFilterBar;
   },
   createFilter: (value) => {
     return `mockKbnFilter:${value}`;
   },
-  getUnsetValue: () => { return UNSET_VALUE; },
   getIndexPattern: () => { return 'mockIndexPattern'; }
 };
 const mockKbnApi = {};
 
+describe('hasChanged', () => {
+  let control;
+
+  beforeEach(() => {
+    control = new Control(createControlParams(3, 'control'), mockFilterManager, mockKbnApi);
+  });
+
+  afterEach(() => {
+    valueFromFilterBar = undefined;
+  });
+
+  test('should be false if value has not changed', () => {
+    expect(control.hasChanged()).to.be(false);
+  });
+
+  test('should be true if value has been set', () => {
+    control.set('new value');
+    expect(control.hasChanged()).to.be(true);
+  });
+
+  test('should be false if value has been set and control is cleared', () => {
+    control.set('new value');
+    control.clear();
+    expect(control.hasChanged()).to.be(false);
+  });
+
+});
+
 describe('ancestors', () => {
 
   let grandParentControl;
@@ -57,6 +84,8 @@ describe('ancestors', () => {
       grandParentControl.set('myGrandParentValue');
 
       childControl.setAncestors([parentControl, grandParentControl]);
+      expect(grandParentControl.hasValue()).to.be(true);
+      expect(parentControl.hasValue()).to.be(false);
       expect(childControl.hasUnsetAncestor()).to.be(true);
     });
 
@@ -64,6 +93,8 @@ describe('ancestors', () => {
       parentControl.set('myParentValue');
 
       childControl.setAncestors([parentControl, grandParentControl]);
+      expect(grandParentControl.hasValue()).to.be(false);
+      expect(parentControl.hasValue()).to.be(true);
       expect(childControl.hasUnsetAncestor()).to.be(true);
     });
 
@@ -72,6 +103,8 @@ describe('ancestors', () => {
       parentControl.set('myParentValue');
 
       childControl.setAncestors([parentControl, grandParentControl]);
+      expect(grandParentControl.hasValue()).to.be(true);
+      expect(parentControl.hasValue()).to.be(true);
       expect(childControl.hasUnsetAncestor()).to.be(false);
     });
   });

src/core_plugins/input_control_vis/public/control/filter_manager/filter_manager.js

@@ -21,12 +21,11 @@ import _ from 'lodash';
 
 export class FilterManager {
 
-  constructor(controlId, fieldName, indexPattern, queryFilter, unsetValue) {
+  constructor(controlId, fieldName, indexPattern, queryFilter) {
     this.controlId = controlId;
     this.fieldName = fieldName;
     this.indexPattern = indexPattern;
     this.queryFilter = queryFilter;
-    this.unsetValue = unsetValue;
   }
 
   getIndexPattern() {
@@ -51,8 +50,4 @@ export class FilterManager {
   getValueFromFilterBar() {
     throw new Error('Must implement getValueFromFilterBar.');
   }
-
-  getUnsetValue() {
-    return this.unsetValue;
-  }
 }

src/core_plugins/input_control_vis/public/control/filter_manager/filter_manager.test.js

@@ -34,7 +34,7 @@ describe('FilterManager', function () {
     let filterManager;
     beforeEach(() => {
       kbnFilters = [];
-      filterManager = new FilterManager(controlId, 'field1', indexPatternMock, queryFilterMock, '');
+      filterManager = new FilterManager(controlId, 'field1', indexPatternMock, queryFilterMock);
     });
 
     test('should not find filters that are not controlled by any visualization', function () {

src/core_plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.js

@@ -24,7 +24,7 @@ import { buildPhrasesFilter } from 'ui/filter_manager/lib/phrases';
 
 export class PhraseFilterManager extends FilterManager {
   constructor(controlId, fieldName, indexPattern, queryFilter) {
-    super(controlId, fieldName, indexPattern, queryFilter, []);
+    super(controlId, fieldName, indexPattern, queryFilter);
   }
 
   /**
@@ -58,19 +58,19 @@ export class PhraseFilterManager extends FilterManager {
   getValueFromFilterBar() {
     const kbnFilters = this.findFilters();
     if (kbnFilters.length === 0) {
-      return this.getUnsetValue();
-    } else {
-      return kbnFilters
-        .map((kbnFilter) => {
-          return this._getValueFromFilter(kbnFilter);
-        })
-        .reduce((accumulator, currentValue) => {
-          return accumulator.concat(currentValue);
-        }, [])
-        .map(value => {
-          return { value, label: value };
-        });
+      return;
     }
+
+    return kbnFilters
+      .map((kbnFilter) => {
+        return this._getValueFromFilter(kbnFilter);
+      })
+      .reduce((accumulator, currentValue) => {
+        return accumulator.concat(currentValue);
+      }, [])
+      .map(value => {
+        return { value, label: value };
+      });
   }
 
   /**
@@ -96,19 +96,17 @@ export class PhraseFilterManager extends FilterManager {
 
     // scripted field filter
     if (_.has(kbnFilter, 'script')) {
-      return _.get(kbnFilter, 'script.script.params.value', this.getUnsetValue());
+      return _.get(kbnFilter, 'script.script.params.value');
     }
 
     // single phrase filter
     if (_.has(kbnFilter, ['query', 'match', this.fieldName])) {
-      return _.get(kbnFilter, ['query', 'match', this.fieldName, 'query'], this.getUnsetValue());
+      return _.get(kbnFilter, ['query', 'match', this.fieldName, 'query']);
     }
 
     // single phrase filter from bool filter
     if (_.has(kbnFilter, ['match_phrase', this.fieldName])) {
-      return _.get(kbnFilter, ['match_phrase', this.fieldName], this.getUnsetValue());
+      return _.get(kbnFilter, ['match_phrase', this.fieldName]);
     }
-
-    return this.getUnsetValue();
   }
 }

src/core_plugins/input_control_vis/public/control/filter_manager/range_filter_manager.js

@@ -66,16 +66,17 @@ export class RangeFilterManager extends FilterManager {
   getValueFromFilterBar() {
     const kbnFilters = this.findFilters();
     if (kbnFilters.length === 0) {
-      return this.getUnsetValue();
-    } else {
-      let range = null;
-      if (_.has(kbnFilters[0], 'script')) {
-        range = _.get(kbnFilters[0], 'script.script.params');
-      } else {
-        range = _.get(kbnFilters[0], ['range', this.fieldName]);
-      }
-
-      return fromRange(range);
+      return;
     }
+
+    let range = null;
+    if (_.has(kbnFilters[0], 'script')) {
+      range = _.get(kbnFilters[0], 'script.script.params');
+    } else {
+      range = _.get(kbnFilters[0], ['range', this.fieldName]);
+    }
+
+    return fromRange(range);
+
   }
 }

src/core_plugins/input_control_vis/public/control/list_control_factory.js

@@ -137,6 +137,10 @@ class ListControl extends Control {
     this.enable = true;
     this.disabledReason = '';
   }
+
+  hasValue() {
+    return typeof this.value !== 'undefined' && this.value.length > 0;
+  }
 }
 
 export async function listControlFactory(controlParams, kbnApi, useTimeFilter) {

src/core_plugins/input_control_vis/public/control/list_control_factory.test.js

@@ -0,0 +1,79 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { listControlFactory } from './list_control_factory';
+
+const mockField = {
+  name: 'myField',
+  format: {
+    convert: (val) => { return val; }
+  }
+};
+
+const mockIndexPattern = {
+  fields: {
+    byName: {
+      myField: mockField,
+    }
+  }
+};
+
+const mockKbnApi = {
+  indexPatterns: {
+    get: async () => {
+      return mockIndexPattern;
+    }
+  },
+  queryFilter: {
+    getAppFilters: () => {
+      return [];
+    },
+    getGlobalFilters: () => {
+      return [];
+    }
+  }
+};
+
+describe('hasValue', () => {
+  const controlParams = {
+    id: '1',
+    fieldName: 'myField',
+    options: {}
+  };
+  const useTimeFilter = false;
+
+  let listControl;
+  beforeEach(async () => {
+    listControl = await listControlFactory(controlParams, mockKbnApi, useTimeFilter);
+  });
+
+  test('should be false when control has no value', () => {
+    expect(listControl.hasValue()).toBe(false);
+  });
+
+  test('should be true when control has value', () => {
+    listControl.set([{ value: 'selected option', label: 'selection option' }]);
+    expect(listControl.hasValue()).toBe(true);
+  });
+
+  test('should be true when control has value that is the string "false"', () => {
+    listControl.set([{ value: 'false', label: 'selection option' }]);
+    expect(listControl.hasValue()).toBe(true);
+  });
+});

src/core_plugins/input_control_vis/public/control/range_control_factory.js

@@ -80,7 +80,6 @@ class RangeControl extends Control {
     if (!minMaxReturnedFromAggregation) {
       this.disable(noValuesDisableMsg(fieldName, indexPattern.title));
     } else {
-      this.unsetValue = { min: min, max: min };
       this.min = min;
       this.max = max;
       this.enable = true;
@@ -97,10 +96,9 @@ export async function rangeControlFactory(controlParams, kbnApi, useTimeFilter)
   } catch (err) {
     // ignore not found error and return control so it can be displayed in disabled state.
   }
-  const unsetValue = { min: 0, max: 1 };
   return new RangeControl(
     controlParams,
-    new RangeFilterManager(controlParams.id, controlParams.fieldName, indexPattern, kbnApi.queryFilter, unsetValue),
+    new RangeFilterManager(controlParams.id, controlParams.fieldName, indexPattern, kbnApi.queryFilter),
     kbnApi,
     useTimeFilter
   );