From 659e4712eb4d78956dbeba4e1788ab93b318dbf0 Mon Sep 17 00:00:00 2001 From: Pete Harverson Date: Mon, 8 Apr 2019 11:54:42 +0100 Subject: [PATCH] [ML] Limits maximum annotation text length to 1000 characters (#34540) * [ML] Limits maximum annotation text length to 1000 characters * [ML] Fix initialization of annotation errors array * [ML] Fix typo in annotation flyout comment --- .../ml/common/constants/annotations.ts | 3 + .../annotation_flyout/index.test.tsx | 42 ++++++++++- .../annotations/annotation_flyout/index.tsx | 69 ++++++++++++++++++- .../__mocks__/mock_annotations.json | 25 +++++++ .../annotations_table.test.js | 2 +- 5 files changed, 135 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/ml/common/constants/annotations.ts b/x-pack/plugins/ml/common/constants/annotations.ts index c388df553df7..936ff610361a 100644 --- a/x-pack/plugins/ml/common/constants/annotations.ts +++ b/x-pack/plugins/ml/common/constants/annotations.ts @@ -10,3 +10,6 @@ export enum ANNOTATION_TYPE { } export const ANNOTATION_USER_UNKNOWN = ''; + +// UI enforced limit to the maximum number of characters that can be entered for an annotation. +export const ANNOTATION_MAX_LENGTH_CHARS = 1000; diff --git a/x-pack/plugins/ml/public/components/annotations/annotation_flyout/index.test.tsx b/x-pack/plugins/ml/public/components/annotations/annotation_flyout/index.test.tsx index 96b1d3e9bdb1..871dcd74d090 100644 --- a/x-pack/plugins/ml/public/components/annotations/annotation_flyout/index.test.tsx +++ b/x-pack/plugins/ml/public/components/annotations/annotation_flyout/index.test.tsx @@ -4,8 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import React from 'react'; -import { shallowWithIntl } from 'test_utils/enzyme_helpers'; +import { injectObservablesAsProps } from '../../../util/observable_utils'; +import mockAnnotations from '../annotations_table/__mocks__/mock_annotations.json'; + +import React, { ComponentType } from 'react'; +import { mountWithIntl, shallowWithIntl } from 'test_utils/enzyme_helpers'; + +import { Annotation } from '../../../../common/types/annotations'; +import { annotation$ } from '../../../services/annotations_service'; import { AnnotationFlyout } from './index'; @@ -14,4 +20,36 @@ describe('AnnotationFlyout', () => { const wrapper = shallowWithIntl(); expect(wrapper).toMatchSnapshot(); }); + + test('Update button is disabled with empty annotation', () => { + const annotation = mockAnnotations[1] as Annotation; + annotation$.next(annotation); + + // injectObservablesAsProps wraps the observable in a new component + const ObservableComponent = injectObservablesAsProps( + { annotation: annotation$ }, + (AnnotationFlyout as any) as ComponentType + ); + + const wrapper = mountWithIntl(); + const updateBtn = wrapper.find('EuiButton').first(); + expect(updateBtn.prop('isDisabled')).toEqual(true); + }); + + test('Error displayed and update button displayed if annotation text is longer than max chars', () => { + const annotation = mockAnnotations[2] as Annotation; + annotation$.next(annotation); + + // injectObservablesAsProps wraps the observable in a new component + const ObservableComponent = injectObservablesAsProps( + { annotation: annotation$ }, + (AnnotationFlyout as any) as ComponentType + ); + + const wrapper = mountWithIntl(); + const updateBtn = wrapper.find('EuiButton').first(); + expect(updateBtn.prop('isDisabled')).toEqual(true); + + expect(wrapper.find('EuiFormErrorText')).toHaveLength(1); + }); }); diff --git a/x-pack/plugins/ml/public/components/annotations/annotation_flyout/index.tsx b/x-pack/plugins/ml/public/components/annotations/annotation_flyout/index.tsx index 9b53adb72332..5086b29a8572 100644 --- a/x-pack/plugins/ml/public/components/annotations/annotation_flyout/index.tsx +++ b/x-pack/plugins/ml/public/components/annotations/annotation_flyout/index.tsx @@ -26,6 +26,7 @@ import { CommonProps } from '@elastic/eui'; import { FormattedMessage, injectI18n } from '@kbn/i18n/react'; import { InjectedIntlProps } from 'react-intl'; import { toastNotifications } from 'ui/notify'; +import { ANNOTATION_MAX_LENGTH_CHARS } from '../../../../common/constants/annotations'; import { annotation$, annotationsRefresh$, @@ -112,6 +113,45 @@ class AnnotationFlyoutIntl extends Component { + // Validates the entered text, returning an array of error messages + // for display in the form. An empty array is returned if the text is valid. + const { annotation, intl } = this.props; + const errors: string[] = []; + if (annotation === null) { + return errors; + } + + if (annotation.annotation.trim().length === 0) { + errors.push( + intl.formatMessage({ + id: 'xpack.ml.timeSeriesExplorer.annotationFlyout.noAnnotationTextError', + defaultMessage: 'Enter annotation text', + }) + ); + } + + const textLength = annotation.annotation.length; + if (textLength > ANNOTATION_MAX_LENGTH_CHARS) { + const charsOver = textLength - ANNOTATION_MAX_LENGTH_CHARS; + errors.push( + intl.formatMessage( + { + id: 'xpack.ml.timeSeriesExplorer.annotationFlyout.maxLengthError', + defaultMessage: + '{charsOver, number} {charsOver, plural, one {character} other {characters}} above maximum length of {maxChars}', + }, + { + maxChars: ANNOTATION_MAX_LENGTH_CHARS, + charsOver, + } + ) + ); + } + + return errors; + }; + public saveOrUpdateAnnotation = () => { const { annotation, intl } = this.props; @@ -179,7 +219,7 @@ class AnnotationFlyoutIntl extends Component 0; + const lengthRatioToShowWarning = 0.95; + let helpText = null; + if ( + isInvalid === false && + annotation.annotation.length > ANNOTATION_MAX_LENGTH_CHARS * lengthRatioToShowWarning + ) { + helpText = intl.formatMessage( + { + id: 'xpack.ml.timeSeriesExplorer.annotationFlyout.approachingMaxLengthWarning', + defaultMessage: + '{charsRemaining, number} {charsRemaining, plural, one {character} other {characters}} remaining', + }, + { charsRemaining: ANNOTATION_MAX_LENGTH_CHARS - annotation.annotation.length } + ); + } + return ( @@ -219,10 +279,13 @@ class AnnotationFlyoutIntl extends Component } fullWidth + helpText={helpText} + isInvalid={isInvalid} + error={validationErrors} > {isExistingAnnotation ? ( diff --git a/x-pack/plugins/ml/public/components/annotations/annotations_table/__mocks__/mock_annotations.json b/x-pack/plugins/ml/public/components/annotations/annotations_table/__mocks__/mock_annotations.json index 2964989316da..06ed1937626c 100644 --- a/x-pack/plugins/ml/public/components/annotations/annotations_table/__mocks__/mock_annotations.json +++ b/x-pack/plugins/ml/public/components/annotations/annotations_table/__mocks__/mock_annotations.json @@ -10,5 +10,30 @@ "modified_time": 1546417097181, "modified_username": "", "_id": "KCCkDWgB_ZdQ1MFDSYPi" + }, + { + "timestamp": 1455026177994, + "end_timestamp": 1455041968976, + "annotation": "", + "job_id": "farequote", + "type": "annotation", + "create_time": 1554377048000, + "create_username": "sysadmin", + "modified_time": 1554377048000, + "modified_username": "sysadmin", + "_id": "KCCkDWgB_ZdQ1MFDSYPj" + }, + { + "timestamp": 1455026177994, + "end_timestamp": 1455041968976, + "annotation": + "A very long annotation with more than the maximum allowed characters. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.", + "job_id": "farequote", + "type": "annotation", + "create_time": 1554377253000, + "create_username": "sysadmin", + "modified_time": 1554377253000, + "modified_username": "sysadmin", + "_id": "KCCkDWgB_ZdQ1MFDSYPk" } ] diff --git a/x-pack/plugins/ml/public/components/annotations/annotations_table/annotations_table.test.js b/x-pack/plugins/ml/public/components/annotations/annotations_table/annotations_table.test.js index f2633e4fbb7a..9ec775db4e2b 100644 --- a/x-pack/plugins/ml/public/components/annotations/annotations_table/annotations_table.test.js +++ b/x-pack/plugins/ml/public/components/annotations/annotations_table/annotations_table.test.js @@ -43,7 +43,7 @@ describe('AnnotationsTable', () => { }); test('Initialization with annotations prop.', () => { - const wrapper = shallowWithIntl(); + const wrapper = shallowWithIntl(); expect(wrapper).toMatchSnapshot(); });