Disallow duplicate percentiles (#57444)

src/legacy/core_plugins/vis_default_editor/public/components/controls/components/number_list/number_list.test.tsx

@@ -63,12 +63,31 @@ describe('NumberList', () => {
 
   test('should show an order error', () => {
     defaultProps.numberArray = [3, 1];
+    defaultProps.validateAscendingOrder = true;
     defaultProps.showValidation = true;
     const comp = mountWithIntl(<NumberList {...defaultProps} />);
 
     expect(comp.find('EuiFormErrorText').length).toBe(1);
   });
 
+  test('should show a duplicate error', () => {
+    defaultProps.numberArray = [3, 1, 3];
+    defaultProps.disallowDuplicates = true;
+    defaultProps.showValidation = true;
+    const comp = mountWithIntl(<NumberList {...defaultProps} />);
+
+    expect(comp.find('EuiFormErrorText').length).toBeGreaterThan(0);
+  });
+
+  test('should show many duplicate errors', () => {
+    defaultProps.numberArray = [3, 1, 3, 1, 3, 1, 3, 1];
+    defaultProps.disallowDuplicates = true;
+    defaultProps.showValidation = true;
+    const comp = mountWithIntl(<NumberList {...defaultProps} />);
+
+    expect(comp.find('EuiFormErrorText').length).toBe(6);
+  });
+
   test('should set validity as true', () => {
     mountWithIntl(<NumberList {...defaultProps} />);
 
@@ -77,6 +96,7 @@ describe('NumberList', () => {
 
   test('should set validity as false when the order is invalid', () => {
     defaultProps.numberArray = [3, 2];
+    defaultProps.validateAscendingOrder = true;
     const comp = mountWithIntl(<NumberList {...defaultProps} />);
 
     expect(defaultProps.setValidity).lastCalledWith(false);

src/legacy/core_plugins/vis_default_editor/public/components/controls/components/number_list/number_list.tsx

@@ -21,17 +21,14 @@ import React, { Fragment, useState, useEffect, useMemo, useCallback } from 'reac
 
 import { EuiSpacer, EuiButtonEmpty, EuiFlexItem, EuiFormErrorText } from '@elastic/eui';
 import { FormattedMessage } from '@kbn/i18n/react';
-import { i18n } from '@kbn/i18n';
 import { NumberRow, NumberRowModel } from './number_row';
 import {
   parse,
   EMPTY_STRING,
   getRange,
-  validateOrder,
-  validateValue,
   getNextModel,
   getInitModelList,
-  getUpdatedModels,
+  getValidatedModels,
   hasInvalidValues,
 } from './utils';
 import { useValidation } from '../../utils';
@@ -41,6 +38,7 @@ export interface NumberListProps {
   numberArray: Array<number | undefined>;
   range?: string;
   showValidation: boolean;
+  disallowDuplicates?: boolean;
   unitName: string;
   validateAscendingOrder?: boolean;
   onChange(list: Array<number | undefined>): void;
@@ -54,31 +52,27 @@ function NumberList({
   range,
   showValidation,
   unitName,
-  validateAscendingOrder = true,
+  validateAscendingOrder = false,
+  disallowDuplicates = false,
   onChange,
   setTouched,
   setValidity,
 }: NumberListProps) {
   const numberRange = useMemo(() => getRange(range), [range]);
   const [models, setModels] = useState(getInitModelList(numberArray));
-  const [ascendingError, setAscendingError] = useState(EMPTY_STRING);
 
   // set up validity for each model
   useEffect(() => {
-    let id: number | undefined;
-    if (validateAscendingOrder) {
-      const { isValidOrder, modelIndex } = validateOrder(numberArray);
-      id = isValidOrder ? undefined : modelIndex;
-      setAscendingError(
-        isValidOrder
-          ? EMPTY_STRING
-          : i18n.translate('visDefaultEditor.controls.numberList.invalidAscOrderErrorMessage', {
-              defaultMessage: 'The values should be in ascending order.',
-            })
-      );
-    }
-    setModels(state => getUpdatedModels(numberArray, state, numberRange, id));
-  }, [numberArray, numberRange, validateAscendingOrder]);
+    setModels(state =>
+      getValidatedModels(
+        numberArray,
+        state,
+        numberRange,
+        validateAscendingOrder,
+        disallowDuplicates
+      )
+    );
+  }, [numberArray, numberRange, validateAscendingOrder, disallowDuplicates]);
 
   // responsible for setting up an initial value ([0]) when there is no default value
   useEffect(() => {
@@ -105,12 +99,10 @@ function NumberList({
       onUpdate(
         models.map(model => {
           if (model.id === id) {
-            const { isInvalid, error } = validateValue(parsedValue, numberRange);
             return {
               id,
               value: parsedValue,
-              isInvalid,
-              error,
+              isInvalid: false,
             };
           }
           return model;
@@ -155,7 +147,6 @@ function NumberList({
           {models.length - 1 !== arrayIndex && <EuiSpacer size="s" />}
         </Fragment>
       ))}
-      {showValidation && ascendingError && <EuiFormErrorText>{ascendingError}</EuiFormErrorText>}
       <EuiSpacer size="s" />
       <EuiFlexItem>
         <EuiButtonEmpty iconType="plusInCircleFilled" onClick={onAdd} size="xs">

src/legacy/core_plugins/vis_default_editor/public/components/controls/components/number_list/utils.test.ts

@@ -19,20 +19,20 @@
 
 import {
   getInitModelList,
-  getUpdatedModels,
-  validateOrder,
   hasInvalidValues,
   parse,
   validateValue,
   getNextModel,
   getRange,
+  getValidatedModels,
 } from './utils';
 import { NumberListRange } from './range';
 import { NumberRowModel } from './number_row';
 
 describe('NumberList utils', () => {
   let modelList: NumberRowModel[];
   let range: NumberListRange;
+  let invalidEntry: NumberRowModel;
 
   beforeEach(() => {
     modelList = [
@@ -46,6 +46,12 @@ describe('NumberList utils', () => {
       maxInclusive: true,
       within: jest.fn(() => true),
     };
+    invalidEntry = {
+      value: expect.any(Number),
+      isInvalid: true,
+      error: expect.any(String),
+      id: expect.any(String),
+    };
   });
 
   describe('getInitModelList', () => {
@@ -65,73 +71,74 @@ describe('NumberList utils', () => {
     });
   });
 
-  describe('getUpdatedModels', () => {
+  describe('getValidatedModels', () => {
     test('should return model list when number list is empty', () => {
-      const updatedModelList = getUpdatedModels([], modelList, range);
+      const updatedModelList = getValidatedModels([], modelList, range);
 
       expect(updatedModelList).toEqual([{ value: 0, id: expect.any(String), isInvalid: false }]);
     });
 
     test('should not update model list when number list is the same', () => {
-      const updatedModelList = getUpdatedModels([1, 2], modelList, range);
+      const updatedModelList = getValidatedModels([1, 2], modelList, range);
 
       expect(updatedModelList).toEqual(modelList);
     });
 
     test('should update model list when number list was changed', () => {
-      const updatedModelList = getUpdatedModels([1, 3], modelList, range);
+      const updatedModelList = getValidatedModels([1, 3], modelList, range);
       modelList[1].value = 3;
       expect(updatedModelList).toEqual(modelList);
     });
 
     test('should update model list when number list increased', () => {
-      const updatedModelList = getUpdatedModels([1, 2, 3], modelList, range);
+      const updatedModelList = getValidatedModels([1, 2, 3], modelList, range);
       expect(updatedModelList).toEqual([
         ...modelList,
         { value: 3, id: expect.any(String), isInvalid: false },
       ]);
     });
 
     test('should update model list when number list decreased', () => {
-      const updatedModelList = getUpdatedModels([2], modelList, range);
+      const updatedModelList = getValidatedModels([2], modelList, range);
       expect(updatedModelList).toEqual([{ value: 2, id: '1', isInvalid: false }]);
     });
 
     test('should update model list when number list has undefined value', () => {
-      const updatedModelList = getUpdatedModels([1, undefined], modelList, range);
+      const updatedModelList = getValidatedModels([1, undefined], modelList, range);
       modelList[1].value = '';
       modelList[1].isInvalid = true;
       expect(updatedModelList).toEqual(modelList);
     });
 
-    test('should update model list when number order is invalid', () => {
-      const updatedModelList = getUpdatedModels([1, 3, 2], modelList, range, 2);
-      expect(updatedModelList).toEqual([
-        modelList[0],
-        { ...modelList[1], value: 3 },
-        { value: 2, id: expect.any(String), isInvalid: true },
-      ]);
+    test('should identify when a number is out of order', () => {
+      const updatedModelList = getValidatedModels([1, 3, 2], modelList, range, true);
+      expect(updatedModelList[2]).toEqual(invalidEntry);
     });
-  });
 
-  describe('validateOrder', () => {
-    test('should return true when order is valid', () => {
-      expect(validateOrder([1, 2])).toEqual({
-        isValidOrder: true,
-      });
+    test('should identify when many numbers are out of order', () => {
+      const updatedModelList = getValidatedModels([1, 3, 2, 3, 4, 2], modelList, range, true);
+      expect(updatedModelList[2]).toEqual(invalidEntry);
+      expect(updatedModelList[5]).toEqual(invalidEntry);
     });
 
-    test('should return true when a number is undefined', () => {
-      expect(validateOrder([1, undefined])).toEqual({
-        isValidOrder: true,
-      });
+    test('should identify a duplicate', () => {
+      const updatedModelList = getValidatedModels([1, 2, 3, 6, 2], modelList, range, false, true);
+      expect(updatedModelList[4]).toEqual(invalidEntry);
     });
 
-    test('should return false when order is invalid', () => {
-      expect(validateOrder([2, 1])).toEqual({
-        isValidOrder: false,
-        modelIndex: 1,
-      });
+    test('should identify many duplicates', () => {
+      const updatedModelList = getValidatedModels(
+        [2, 2, 2, 3, 4, 5, 2, 2, 3],
+        modelList,
+        range,
+        false,
+        true
+      );
+      expect(updatedModelList[1]).toEqual(invalidEntry);
+      expect(updatedModelList[2]).toEqual(invalidEntry);
+      expect(updatedModelList[6]).toEqual(invalidEntry);
+      expect(updatedModelList[7]).toEqual(invalidEntry);
+      expect(updatedModelList[8]).toEqual(invalidEntry);
     });
   });
 

src/legacy/core_plugins/vis_default_editor/public/components/controls/components/number_list/utils.ts

@@ -49,6 +49,7 @@ function validateValue(value: number | '', numberRange: NumberListRange) {
 
   if (value === EMPTY_STRING) {
     result.isInvalid = true;
+    result.error = EMPTY_STRING;
   } else if (!numberRange.within(value)) {
     result.isInvalid = true;
     result.error = i18n.translate('visDefaultEditor.controls.numberList.invalidRangeErrorMessage', {
@@ -60,19 +61,46 @@ function validateValue(value: number | '', numberRange: NumberListRange) {
   return result;
 }
 
-function validateOrder(list: Array<number | undefined>) {
-  const result: { isValidOrder: boolean; modelIndex?: number } = {
-    isValidOrder: true,
+function validateValueAscending(
+  inputValue: number | '',
+  index: number,
+  list: Array<number | undefined>
+) {
+  const result: { isInvalidOrder: boolean; error?: string } = {
+    isInvalidOrder: false,
   };
 
-  list.forEach((inputValue, index, array) => {
-    const previousModel = array[index - 1];
-    if (previousModel !== undefined && inputValue !== undefined && inputValue <= previousModel) {
-      result.isValidOrder = false;
-      result.modelIndex = index;
-    }
-  });
+  const previousModel = list[index - 1];
+  if (previousModel !== undefined && inputValue !== undefined && inputValue <= previousModel) {
+    result.isInvalidOrder = true;
+    result.error = i18n.translate(
+      'visDefaultEditor.controls.numberList.invalidAscOrderErrorMessage',
+      {
+        defaultMessage: 'Value is not in ascending order.',
+      }
+    );
+  }
+  return result;
+}
+
+function validateValueUnique(
+  inputValue: number | '',
+  index: number,
+  list: Array<number | undefined>
+) {
+  const result: { isDuplicate: boolean; error?: string } = {
+    isDuplicate: false,
+  };
 
+  if (inputValue && list.indexOf(inputValue) !== index) {
+    result.isDuplicate = true;
+    result.error = i18n.translate(
+      'visDefaultEditor.controls.numberList.duplicateValueErrorMessage',
+      {
+        defaultMessage: 'Duplicate value.',
+      }
+    );
+  }
   return result;
 }
 
@@ -101,24 +129,40 @@ function getInitModelList(list: Array<number | undefined>): NumberRowModel[] {
     : [defaultModel];
 }
 
-function getUpdatedModels(
+function getValidatedModels(
   numberList: Array<number | undefined>,
   modelList: NumberRowModel[],
   numberRange: NumberListRange,
-  invalidOrderModelIndex?: number
+  validateAscendingOrder: boolean = false,
+  disallowDuplicates: boolean = false
 ): NumberRowModel[] {
   if (!numberList.length) {
     return [defaultModel];
   }
   return numberList.map((number, index) => {
     const model = modelList[index] || { id: generateId() };
     const newValue: NumberRowModel['value'] = number === undefined ? EMPTY_STRING : number;
-    const { isInvalid, error } = validateValue(newValue, numberRange);
+
+    const valueResult = numberRange ? validateValue(newValue, numberRange) : { isInvalid: false };
+
+    const ascendingResult = validateAscendingOrder
+      ? validateValueAscending(newValue, index, numberList)
+      : { isInvalidOrder: false };
+
+    const duplicationResult = disallowDuplicates
+      ? validateValueUnique(newValue, index, numberList)
+      : { isDuplicate: false };
+
+    const allErrors = [valueResult.error, ascendingResult.error, duplicationResult.error]
+      .filter(Boolean)
+      .join(' ');
+
     return {
       ...model,
       value: newValue,
-      isInvalid: invalidOrderModelIndex === index ? true : isInvalid,
-      error,
+      isInvalid:
+        valueResult.isInvalid || ascendingResult.isInvalidOrder || duplicationResult.isDuplicate,
+      error: allErrors === EMPTY_STRING ? undefined : allErrors,
     };
   });
 }
@@ -132,9 +176,8 @@ export {
   parse,
   getRange,
   validateValue,
-  validateOrder,
   getNextModel,
   getInitModelList,
-  getUpdatedModels,
+  getValidatedModels,
   hasInvalidValues,
 };