From c885f3c9ff83de3fa39cd65a4003a88be1f7251f Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Thu, 31 Oct 2019 08:54:20 +0100 Subject: [PATCH 1/3] [ML] Improve processing of groups in data recognizer wizard (#49310) * [ML] add support for job overrides * [ML] disable Save button for invalid form, add translation * [ML] add a tooltip for edit button * [ML] fix IE flex layout issue * [ML] conditionally set jobOverrides to the request payload * [ML] remove hideCloseButton * [ML] fix applyJobConfigOverrides * [ML] processArrayValues * [ML] check deeper arrays * [ML] PR remarks * [ML] check for an empty value --- .../legacy/plugins/ml/common/types/modules.ts | 2 + .../recognize/components/edit_job.tsx | 136 ++++++++++++++ .../recognize/components/job_item.tsx | 172 ++++++++++++++++++ .../components/job_settings_form.tsx | 43 +---- .../recognize/components/module_jobs.tsx | 160 ++++++---------- .../jobs/new_job_new/recognize/page.tsx | 41 ++++- .../public/services/ml_api_service/index.js | 3 +- .../__tests__/data_recognizer.js | 62 ++++++- .../models/data_recognizer/data_recognizer.js | 93 +++++++--- 9 files changed, 525 insertions(+), 187 deletions(-) create mode 100644 x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/edit_job.tsx create mode 100644 x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/job_item.tsx diff --git a/x-pack/legacy/plugins/ml/common/types/modules.ts b/x-pack/legacy/plugins/ml/common/types/modules.ts index 18879304d7c7f2..9eb77d2140323a 100644 --- a/x-pack/legacy/plugins/ml/common/types/modules.ts +++ b/x-pack/legacy/plugins/ml/common/types/modules.ts @@ -80,3 +80,5 @@ export interface DataRecognizerConfigResponse { dashboard: KibanaObjectResponse; }; } + +export type JobOverride = Partial; diff --git a/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/edit_job.tsx b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/edit_job.tsx new file mode 100644 index 00000000000000..7ec8cddfe3ed5d --- /dev/null +++ b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/edit_job.tsx @@ -0,0 +1,136 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { FC, useEffect, useState } from 'react'; +import { + EuiButton, + EuiButtonEmpty, + EuiFlexGroup, + EuiFlexItem, + EuiFlyout, + EuiFlyoutBody, + EuiFlyoutFooter, + EuiFlyoutHeader, + EuiForm, + EuiFormRow, + EuiSpacer, + EuiTitle, +} from '@elastic/eui'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { ModuleJobUI } from '../page'; +import { usePartialState } from '../../../../components/custom_hooks'; +import { composeValidators, maxLengthValidator } from '../../../../../common/util/validators'; +import { isJobIdValid } from '../../../../../common/util/job_utils'; +import { JOB_ID_MAX_LENGTH } from '../../../../../common/constants/validation'; +import { JobGroupsInput } from '../../common/components'; +import { JobOverride } from '../../../../../common/types/modules'; + +interface EditJobProps { + job: ModuleJobUI; + jobOverride: JobOverride | undefined; + existingGroupIds: string[]; + onClose: (job: JobOverride | null) => void; +} + +/** + * Edit job flyout for overriding job configuration. + */ +export const EditJob: FC = ({ job, jobOverride, existingGroupIds, onClose }) => { + const [formState, setFormState] = usePartialState({ + jobGroups: (jobOverride && jobOverride.groups) || job.config.groups, + }); + const [validationResult, setValidationResult] = useState>({}); + + const groupValidator = composeValidators( + (value: string) => (isJobIdValid(value) ? null : { pattern: true }), + maxLengthValidator(JOB_ID_MAX_LENGTH) + ); + + const handleValidation = () => { + const jobGroupsValidationResult = formState.jobGroups + .map(group => groupValidator(group)) + .filter(result => result !== null); + + setValidationResult({ + jobGroups: jobGroupsValidationResult, + formValid: jobGroupsValidationResult.length === 0, + }); + }; + + useEffect(() => { + handleValidation(); + }, [formState.jobGroups]); + + const onSave = () => { + const result: JobOverride = { + job_id: job.id, + groups: formState.jobGroups, + }; + onClose(result); + }; + + return ( + onClose(null)}> + + +

+ +

+ + + + + + + { + setFormState({ + jobGroups: value, + }); + }} + validation={{ + valid: !validationResult.jobGroups || validationResult.jobGroups.length === 0, + message: ( + + ), + }} + /> + + + + + + + + onClose(null)} flush="left"> + + + + + onSave()} fill disabled={!validationResult.formValid}> + + + + + + + ); +}; diff --git a/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/job_item.tsx b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/job_item.tsx new file mode 100644 index 00000000000000..ace8409734b742 --- /dev/null +++ b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/job_item.tsx @@ -0,0 +1,172 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { FC, memo } from 'react'; +import { + EuiBadge, + EuiButtonIcon, + EuiFlexGroup, + EuiFlexItem, + EuiIcon, + EuiLoadingSpinner, + EuiText, + EuiToolTip, +} from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { ModuleJobUI } from '../page'; +import { SETUP_RESULTS_WIDTH } from './module_jobs'; +import { tabColor } from '../../../../../common/util/group_color_utils'; +import { JobOverride } from '../../../../../common/types/modules'; + +interface JobItemProps { + job: ModuleJobUI; + jobPrefix: string; + jobOverride: JobOverride | undefined; + isSaving: boolean; + onEditRequest: (job: ModuleJobUI) => void; +} + +export const JobItem: FC = memo( + ({ job, jobOverride, isSaving, jobPrefix, onEditRequest }) => { + const { + id, + config: { description, groups }, + datafeedResult, + setupResult, + } = job; + + const jobGroups = (jobOverride && jobOverride.groups) || groups; + + return ( + + + + + + {jobPrefix} + {id} + + + + + } + > + onEditRequest(job)} + /> + + + + + + {description} + + + + {jobGroups.map(group => ( + + {group} + + ))} + + + {setupResult && setupResult.error && ( + + {setupResult.error.msg} + + )} + + {datafeedResult && datafeedResult.error && ( + + {datafeedResult.error.msg} + + )} + + + {isSaving && } + {setupResult && datafeedResult && ( + + + + + + + + + + + + + + )} + + + ); + } +); diff --git a/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/job_settings_form.tsx b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/job_settings_form.tsx index 617f6b31e7e537..bab45a7d77a3de 100644 --- a/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/job_settings_form.tsx +++ b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/components/job_settings_form.tsx @@ -27,9 +27,8 @@ import { patternValidator, } from '../../../../../common/util/validators'; import { JOB_ID_MAX_LENGTH } from '../../../../../common/constants/validation'; -import { isJobIdValid } from '../../../../../common/util/job_utils'; import { usePartialState } from '../../../../components/custom_hooks'; -import { JobGroupsInput, TimeRangePicker, TimeRange } from '../../common/components'; +import { TimeRange, TimeRangePicker } from '../../common/components'; export interface JobSettingsFormValues { jobPrefix: string; @@ -37,15 +36,12 @@ export interface JobSettingsFormValues { useFullIndexData: boolean; timeRange: TimeRange; useDedicatedIndex: boolean; - jobGroups: string[]; } interface JobSettingsFormProps { saveState: SAVE_STATE; onSubmit: (values: JobSettingsFormValues) => any; onChange: (values: JobSettingsFormValues) => any; - jobGroups: string[]; - existingGroupIds: string[]; jobs: ModuleJobUI[]; } @@ -53,9 +49,7 @@ export const JobSettingsForm: FC = ({ onSubmit, onChange, saveState, - existingGroupIds, jobs, - jobGroups, }) => { const { from, to } = getTimeFilterRange(); const { currentIndexPattern: indexPattern } = useKibanaContext(); @@ -64,10 +58,6 @@ export const JobSettingsForm: FC = ({ patternValidator(/^([a-z0-9]+[a-z0-9\-_]*)?$/), maxLengthValidator(JOB_ID_MAX_LENGTH - Math.max(...jobs.map(({ id }) => id.length))) ); - const groupValidator = composeValidators( - (value: string) => (isJobIdValid(value) ? null : { pattern: true }), - maxLengthValidator(JOB_ID_MAX_LENGTH) - ); const [formState, setFormState] = usePartialState({ jobPrefix: '', @@ -78,7 +68,6 @@ export const JobSettingsForm: FC = ({ end: to, }, useDedicatedIndex: false, - jobGroups: [] as string[], }); const [validationResult, setValidationResult] = useState>({}); @@ -90,29 +79,21 @@ export const JobSettingsForm: FC = ({ const handleValidation = () => { const jobPrefixValidationResult = jobPrefixValidator(formState.jobPrefix); - const jobGroupsValidationResult = formState.jobGroups - .map(group => groupValidator(group)) - .filter(result => result !== null); setValidationResult({ jobPrefix: jobPrefixValidationResult, - jobGroups: jobGroupsValidationResult, - formValid: !jobPrefixValidationResult && jobGroupsValidationResult.length === 0, + formValid: !jobPrefixValidationResult, }); }; useEffect(() => { handleValidation(); - }, [formState.jobPrefix, formState.jobGroups]); + }, [formState.jobPrefix]); useEffect(() => { onChange(formState); }, [formState]); - useEffect(() => { - setFormState({ jobGroups }); - }, [jobGroups]); - return ( <> @@ -174,24 +155,6 @@ export const JobSettingsForm: FC = ({ /> - { - setFormState({ - jobGroups: value, - }); - }} - validation={{ - valid: !validationResult.jobGroups || validationResult.jobGroups.length === 0, - message: ( - - ), - }} - /> void; } -const SETUP_RESULTS_WIDTH = '200px'; +export const SETUP_RESULTS_WIDTH = '200px'; -export const ModuleJobs: FC = ({ jobs, jobPrefix, saveState }) => { +export const ModuleJobs: FC = ({ + jobs, + jobPrefix, + jobOverrides, + saveState, + existingGroupIds, + onJobOverridesChange, +}) => { const isSaving = saveState === SAVE_STATE.SAVING; + + const [jobToEdit, setJobToEdit] = useState(null); + + const onFlyoutClose = (result: JobOverride | null) => { + setJobToEdit(null); + + if (result === null) { + return; + } + + onJobOverridesChange(result); + }; + + const getJobOverride = (job: ModuleJobUI): JobOverride | undefined => { + return jobOverrides[job.id]; + }; + + const editJobFlyout = + jobToEdit !== null ? ( + + ) : null; + return ( <> @@ -70,109 +107,26 @@ export const ModuleJobs: FC = ({ jobs, jobPrefix, saveState }) )}
    - {jobs.map(({ id, config: { description }, setupResult, datafeedResult }, i) => ( -
  • - + {jobs.map((job, i) => ( +
  • + - - {jobPrefix} - {id} - - - - {description} - - - {setupResult && setupResult.error && ( - - {setupResult.error.msg} - - )} - - {datafeedResult && datafeedResult.error && ( - - {datafeedResult.error.msg} - - )} - - - {isSaving && } - {setupResult && datafeedResult && ( - - - - - - - - - - - - - - )} + setJobToEdit(job)} + /> + {i < jobs.length - 1 && }
    • ))}
+ + {editJobFlyout} ); }; diff --git a/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/page.tsx b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/page.tsx index 2c7600dcb99b20..f9a5230ef17d9f 100644 --- a/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/page.tsx +++ b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/recognize/page.tsx @@ -21,12 +21,13 @@ import { EuiPanel, } from '@elastic/eui'; import { toastNotifications } from 'ui/notify'; -import { merge, flatten } from 'lodash'; +import { merge } from 'lodash'; import { ml } from '../../../services/ml_api_service'; import { useKibanaContext } from '../../../contexts/kibana'; import { DatafeedResponse, DataRecognizerConfigResponse, + JobOverride, JobResponse, KibanaObject, KibanaObjectResponse, @@ -40,6 +41,7 @@ import { ModuleJobs } from './components/module_jobs'; import { checkForSavedObjects } from './resolvers'; import { JobSettingsForm, JobSettingsFormValues } from './components/job_settings_form'; import { TimeRange } from '../common/components'; +import { JobId } from '../common/job_creator/configs'; export interface ModuleJobUI extends ModuleJob { datafeedResult?: DatafeedResponse; @@ -57,6 +59,8 @@ interface PageProps { existingGroupIds: string[]; } +export type JobOverrides = Record; + export enum SAVE_STATE { NOT_SAVED, SAVING, @@ -69,7 +73,7 @@ export const Page: FC = ({ moduleId, existingGroupIds }) => { // #region State const [jobPrefix, setJobPrefix] = useState(''); const [jobs, setJobs] = useState([]); - const [jobGroups, setJobGroups] = useState([]); + const [jobOverrides, setJobOverrides] = useState({}); const [kibanaObjects, setKibanaObjects] = useState({}); const [saveState, setSaveState] = useState(SAVE_STATE.NOT_SAVED); const [resultsUrl, setResultsUrl] = useState(''); @@ -93,6 +97,9 @@ export const Page: FC = ({ moduleId, existingGroupIds }) => { const displayQueryWarning = savedSearch.id !== undefined; const tempQuery = savedSearch.id === undefined ? undefined : combinedQuery; + /** + * Loads recognizer module configuration. + */ const loadModule = async () => { try { const response: Module = await ml.getDataRecognizerModule({ moduleId }); @@ -101,9 +108,6 @@ export const Page: FC = ({ moduleId, existingGroupIds }) => { const kibanaObjectsResult = await checkForSavedObjects(response.kibana as KibanaObjects); setKibanaObjects(kibanaObjectsResult); - setJobGroups([ - ...new Set(flatten(response.jobs.map(({ config: { groups = [] } }) => groups))), - ]); setSaveState(SAVE_STATE.NOT_SAVED); } catch (e) { // eslint-disable-next-line no-console @@ -134,11 +138,13 @@ export const Page: FC = ({ moduleId, existingGroupIds }) => { loadModule(); }, []); + /** + * Sets up recognizer module configuration. + */ const save = async (formValues: JobSettingsFormValues) => { setSaveState(SAVE_STATE.SAVING); const { jobPrefix: resultJobPrefix, - jobGroups: resultJobGroups, startDatafeedAfterSave, useDedicatedIndex, useFullIndexData, @@ -148,14 +154,17 @@ export const Page: FC = ({ moduleId, existingGroupIds }) => { const resultTimeRange = await getTimeRange(useFullIndexData, timeRange); try { + let jobOverridesPayload: JobOverride[] | null = Object.values(jobOverrides); + jobOverridesPayload = jobOverridesPayload.length > 0 ? jobOverridesPayload : null; + const response: DataRecognizerConfigResponse = await ml.setupDataRecognizerConfig({ moduleId, prefix: resultJobPrefix, - groups: resultJobGroups, query: tempQuery, indexPatternName: indexPattern.title, useDedicatedIndex, startDatafeed: startDatafeedAfterSave, + ...(jobOverridesPayload !== null ? { jobOverrides: jobOverridesPayload } : {}), ...resultTimeRange, }); const { datafeeds: datafeedsResponse, jobs: jobsResponse, kibana: kibanaResponse } = response; @@ -208,6 +217,13 @@ export const Page: FC = ({ moduleId, existingGroupIds }) => { } }; + const onJobOverridesChange = (job: JobOverride) => { + setJobOverrides({ + ...jobOverrides, + [job.job_id as string]: job, + }); + }; + const isFormVisible = [SAVE_STATE.NOT_SAVED, SAVE_STATE.SAVING].includes(saveState); return ( @@ -271,10 +287,8 @@ export const Page: FC = ({ moduleId, existingGroupIds }) => { onChange={formValues => { setJobPrefix(formValues.jobPrefix); }} - existingGroupIds={existingGroupIds} saveState={saveState} jobs={jobs} - jobGroups={jobGroups} /> )} = ({ moduleId, existingGroupIds }) => { - + {Object.keys(kibanaObjects).length > 0 && ( <> diff --git a/x-pack/legacy/plugins/ml/public/services/ml_api_service/index.js b/x-pack/legacy/plugins/ml/public/services/ml_api_service/index.js index c6a60a9eff7daa..94c79fe470236d 100644 --- a/x-pack/legacy/plugins/ml/public/services/ml_api_service/index.js +++ b/x-pack/legacy/plugins/ml/public/services/ml_api_service/index.js @@ -292,7 +292,8 @@ export const ml = { 'useDedicatedIndex', 'startDatafeed', 'start', - 'end' + 'end', + 'jobOverrides', ]); return http({ diff --git a/x-pack/legacy/plugins/ml/server/models/data_recognizer/__tests__/data_recognizer.js b/x-pack/legacy/plugins/ml/server/models/data_recognizer/__tests__/data_recognizer.js index 2ed0f9b2db734e..af5039fafdc604 100644 --- a/x-pack/legacy/plugins/ml/server/models/data_recognizer/__tests__/data_recognizer.js +++ b/x-pack/legacy/plugins/ml/server/models/data_recognizer/__tests__/data_recognizer.js @@ -4,8 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ - - import expect from '@kbn/expect'; import { DataRecognizer } from '../data_recognizer'; @@ -36,10 +34,68 @@ describe('ML - data recognizer', () => { expect(ids.join()).to.equal(moduleIds.join()); }); - it('getModule - load a single module', async () => { const module = await dr.getModule(moduleIds[0]); expect(module.id).to.equal(moduleIds[0]); }); + describe('jobOverrides', () => { + it('should apply job overrides correctly', () => { + // arrange + const prefix = 'pre-'; + const testJobId = 'test-job'; + const moduleConfig = { + jobs: [ + { + id: `${prefix}${testJobId}`, + config: { + groups: ['nginx'], + analysis_config: { + bucket_span: '1h' + }, + analysis_limits: { + model_memory_limit: '256mb', + influencers: [ + 'region' + ] + }, + calendars: ['calendar-1'], + } + }, + ], + }; + const jobOverrides = [ + { + analysis_limits: { + model_memory_limit: '512mb', + influencers: [], + } + }, + { + job_id: testJobId, + groups: [], + }, + ]; + // act + dr.applyJobConfigOverrides(moduleConfig, jobOverrides, prefix); + // assert + expect(moduleConfig.jobs).to.eql([ + { + config: { + analysis_config: { + bucket_span: '1h' + }, + analysis_limits: { + model_memory_limit: '512mb', + influencers: [], + }, + groups: [], + calendars: ['calendar-1'], + }, + id: 'pre-test-job' + } + ]); + }); + }); }); + diff --git a/x-pack/legacy/plugins/ml/server/models/data_recognizer/data_recognizer.js b/x-pack/legacy/plugins/ml/server/models/data_recognizer/data_recognizer.js index 0b0d2c5adfea09..b5c897a5a3cc98 100644 --- a/x-pack/legacy/plugins/ml/server/models/data_recognizer/data_recognizer.js +++ b/x-pack/legacy/plugins/ml/server/models/data_recognizer/data_recognizer.js @@ -811,45 +811,78 @@ export class DataRecognizer { } applyJobConfigOverrides(moduleConfig, jobOverrides, jobPrefix = '') { - if(jobOverrides !== undefined && jobOverrides !== null) { - if (typeof jobOverrides !== 'object') { - throw Boom.badRequest( - `Incompatible jobOverrides type (${typeof jobOverrides}). It needs to be an object or array of objects.` - ); + if (jobOverrides === undefined || jobOverrides === null) { + return; + } + + if (typeof jobOverrides !== 'object') { + throw Boom.badRequest( + `Incompatible jobOverrides type (${typeof jobOverrides}). It needs to be an object or array of objects.` + ); + } + + // jobOverrides could be a single object or an array of objects. + // if single, convert to an array + const overrides = Array.isArray(jobOverrides) ? jobOverrides : [jobOverrides]; + const { jobs } = moduleConfig; + + // separate all the overrides. + // the overrides which don't contain a job id will be applied to all jobs in the module + const generalOverrides = []; + const jobSpecificOverrides = []; + + overrides.forEach(override => { + if (override.job_id === undefined) { + generalOverrides.push(override); + } else { + jobSpecificOverrides.push(override); } + }); - // jobOverrides could be a single object or an array of objects. - // if single, convert to an array - const overrides = Array.isArray(jobOverrides) ? jobOverrides : [jobOverrides]; - const { jobs } = moduleConfig; + function processArrayValues(source, update) { + if (typeof source !== 'object' || typeof update !== 'object') { + return; + } - // separate all the overrides. - // the overrides which don't contain a job id will be applied to all jobs in the module - const generalOverrides = []; - const jobSpecificOverrides = []; - overrides.forEach(o => { - if (o.job_id === undefined) { - generalOverrides.push(o); + Object.keys(source).forEach(key => { + const sourceValue = source[key]; + const updateValue = update[key]; + + if ( + typeof sourceValue !== 'object' || + sourceValue === null || + typeof updateValue !== 'object' || + updateValue === null + ) { + return; + } + + if (Array.isArray(sourceValue) && Array.isArray(updateValue)) { + source[key] = updateValue; } else { - jobSpecificOverrides.push(o); + processArrayValues(sourceValue, updateValue); } }); + } - generalOverrides.forEach(o => { - jobs.forEach(({ config }) => merge(config, o)); + generalOverrides.forEach(generalOverride => { + jobs.forEach(job => { + merge(job.config, generalOverride); + processArrayValues(job.config, generalOverride); }); + }); - jobSpecificOverrides.forEach(o => { - // for each override, find the relevant job. - // note, the job id already has the prefix prepended to it - const job = jobs.find(j => j.id === `${jobPrefix}${o.job_id}`); - if (job !== undefined) { - // delete the job_id in the override as this shouldn't be overridden - delete o.job_id; - merge(job.config, o); - } - }); - } + jobSpecificOverrides.forEach(jobSpecificOverride => { + // for each override, find the relevant job. + // note, the job id already has the prefix prepended to it + const job = jobs.find(j => j.id === `${jobPrefix}${jobSpecificOverride.job_id}`); + if (job !== undefined) { + // delete the job_id in the override as this shouldn't be overridden + delete jobSpecificOverride.job_id; + merge(job.config, jobSpecificOverride); + processArrayValues(job.config, jobSpecificOverride); + } + }); } applyDatafeedConfigOverrides(moduleConfig, datafeedOverrides, jobPrefix = '') { From 9e00c78f4c568af1a1585ba747ed0af398e53cef Mon Sep 17 00:00:00 2001 From: Daniil Suleiman <31325372+sulemanof@users.noreply.github.com> Date: Thu, 31 Oct 2019 11:27:57 +0300 Subject: [PATCH 2/3] [Vis: Default editor] Create number_input with numeric value required (#48117) * Created number_input with numeric value required * Remove extra license * Make value be possibly null --- .../public/components/common/number_input.tsx | 6 ++ .../common/required_number_input.tsx | 87 +++++++++++++++++++ .../public/components/common/utils.ts | 34 ++++++++ .../options/point_series/point_series.tsx | 5 +- .../options/point_series/threshold_panel.tsx | 19 +++- .../kbn_vislib_vis_types/public/types.ts | 4 +- .../public/utils/common_config.tsx | 4 +- 7 files changed, 150 insertions(+), 9 deletions(-) create mode 100644 src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/required_number_input.tsx create mode 100644 src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/utils.ts diff --git a/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/number_input.tsx b/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/number_input.tsx index 282c2e6a356bff..b614e00ba8cd08 100644 --- a/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/number_input.tsx +++ b/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/number_input.tsx @@ -34,6 +34,12 @@ interface NumberInputOptionProps { setValue: (paramName: ParamName, value: number | '') => void; } +/** + * Do not use this component anymore. + * Please, use NumberInputOption in 'required_number_input.tsx'. + * It is required for compatibility with TS 3.7.0 + * This should be removed in the future + */ function NumberInputOption({ disabled, error, diff --git a/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/required_number_input.tsx b/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/required_number_input.tsx new file mode 100644 index 00000000000000..7b62016c4e502c --- /dev/null +++ b/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/required_number_input.tsx @@ -0,0 +1,87 @@ +/* + * 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 React, { ReactNode, useCallback, ChangeEvent } from 'react'; +import { EuiFormRow, EuiFieldNumber } from '@elastic/eui'; +import { useValidation } from './utils'; + +interface NumberInputOptionProps { + disabled?: boolean; + error?: ReactNode; + isInvalid?: boolean; + label?: React.ReactNode; + max?: number; + min?: number; + paramName: ParamName; + step?: number; + value: number | null; + 'data-test-subj'?: string; + setValue(paramName: ParamName, value: number | null): void; + setValidity(paramName: ParamName, isValid: boolean): void; +} + +/** + * Use only this component instead of NumberInputOption in 'number_input.tsx'. + * It is required for compatibility with TS 3.7.0 + * + * @param {number} props.value Should be numeric only + */ +function NumberInputOption({ + disabled, + error, + isInvalid, + label, + max, + min, + paramName, + step, + value, + setValue, + setValidity, + 'data-test-subj': dataTestSubj, +}: NumberInputOptionProps) { + const isValid = value !== null; + useValidation(setValidity, paramName, isValid); + + const onChange = useCallback( + (ev: ChangeEvent) => + setValue(paramName, isNaN(ev.target.valueAsNumber) ? null : ev.target.valueAsNumber), + [setValue, paramName] + ); + + return ( + + + + ); +} + +export { NumberInputOption }; diff --git a/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/utils.ts b/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/utils.ts new file mode 100644 index 00000000000000..d51631106dda76 --- /dev/null +++ b/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/utils.ts @@ -0,0 +1,34 @@ +/* + * 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 { useEffect } from 'react'; + +function useValidation( + setValidity: (paramName: ParamName, isValid: boolean) => void, + paramName: ParamName, + isValid: boolean +) { + useEffect(() => { + setValidity(paramName, isValid); + + return () => setValidity(paramName, true); + }, [isValid, paramName, setValidity]); +} + +export { useValidation }; diff --git a/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/point_series/point_series.tsx b/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/point_series/point_series.tsx index 8e3f66d12b9bdf..ed3b52b83c2345 100644 --- a/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/point_series/point_series.tsx +++ b/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/point_series/point_series.tsx @@ -21,13 +21,12 @@ import { EuiPanel, EuiTitle, EuiSpacer } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; -import { VisOptionsProps } from 'ui/vis/editors/default'; -import { BasicOptions, SwitchOption } from '../../common'; +import { BasicOptions, SwitchOption, ValidationVisOptionsProps } from '../../common'; import { GridPanel } from './grid_panel'; import { ThresholdPanel } from './threshold_panel'; import { BasicVislibParams } from '../../../types'; -function PointSeriesOptions(props: VisOptionsProps) { +function PointSeriesOptions(props: ValidationVisOptionsProps) { const { stateParams, setValue, vis } = props; return ( diff --git a/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/point_series/threshold_panel.tsx b/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/point_series/threshold_panel.tsx index 49e56e377a8d56..591ad2eb3a001a 100644 --- a/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/point_series/threshold_panel.tsx +++ b/src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/point_series/threshold_panel.tsx @@ -21,11 +21,16 @@ import { EuiPanel, EuiTitle, EuiColorPicker, EuiFormRow, EuiSpacer } from '@elas import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; -import { VisOptionsProps } from 'ui/vis/editors/default'; -import { NumberInputOption, SelectOption, SwitchOption } from '../../common'; +import { SelectOption, SwitchOption, ValidationVisOptionsProps } from '../../common'; +import { NumberInputOption } from '../../common/required_number_input'; import { BasicVislibParams } from '../../../types'; -function ThresholdPanel({ stateParams, setValue, vis }: VisOptionsProps) { +function ThresholdPanel({ + stateParams, + setValue, + setMultipleValidity, + vis, +}: ValidationVisOptionsProps) { const setThresholdLine = useCallback( ( paramName: T, @@ -39,6 +44,12 @@ function ThresholdPanel({ stateParams, setValue, vis }: VisOptionsProps + setMultipleValidity(`thresholdLine__${paramName}`, isValid), + [setMultipleValidity] + ); + return ( @@ -72,6 +83,7 @@ function ThresholdPanel({ stateParams, setValue, vis }: VisOptionsProps ) => ( + + ), }, ]; } From faa48d591d30c19fd1c057129769ac5498f18709 Mon Sep 17 00:00:00 2001 From: sainthkh Date: Thu, 31 Oct 2019 19:40:53 +0900 Subject: [PATCH 3/3] eslint no-restricted-path false positive: bug caused by dir names that start with "index". (#46544) * index* to index.{ts,tsx} * Added test. * Added invalid test. * Added js. --- .eslintrc.js | 4 +- .../server/index_patterns/index.js | 1 + .../rules/__tests__/no_restricted_paths.js | 50 +++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 packages/kbn-eslint-plugin-eslint/rules/__tests__/files/no_restricted_paths/server/index_patterns/index.js diff --git a/.eslintrc.js b/.eslintrc.js index 8ed89d47d43ebb..310ee47819e30a 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -362,10 +362,10 @@ module.exports = { '!src/core/server/*.test.mocks.ts', 'src/plugins/**/public/**/*', - '!src/plugins/**/public/index*', + '!src/plugins/**/public/index.{js,ts,tsx}', 'src/plugins/**/server/**/*', - '!src/plugins/**/server/index*', + '!src/plugins/**/server/index.{js,ts,tsx}', ], allowSameFolder: true, }, diff --git a/packages/kbn-eslint-plugin-eslint/rules/__tests__/files/no_restricted_paths/server/index_patterns/index.js b/packages/kbn-eslint-plugin-eslint/rules/__tests__/files/no_restricted_paths/server/index_patterns/index.js new file mode 100644 index 00000000000000..d15de7d98a9e0c --- /dev/null +++ b/packages/kbn-eslint-plugin-eslint/rules/__tests__/files/no_restricted_paths/server/index_patterns/index.js @@ -0,0 +1 @@ +/* eslint-disable */ diff --git a/packages/kbn-eslint-plugin-eslint/rules/__tests__/no_restricted_paths.js b/packages/kbn-eslint-plugin-eslint/rules/__tests__/no_restricted_paths.js index f393a867d95e0d..577be820ccc7bc 100644 --- a/packages/kbn-eslint-plugin-eslint/rules/__tests__/no_restricted_paths.js +++ b/packages/kbn-eslint-plugin-eslint/rules/__tests__/no_restricted_paths.js @@ -172,6 +172,27 @@ ruleTester.run('@kbn/eslint/no-restricted-paths', rule, { }, ], }, + + { + // Check if dirs that start with 'index' work correctly. + code: 'import { X } from "./index_patterns"', + filename: path.join(__dirname, './files/no_restricted_paths/server/b.js'), + options: [ + { + basePath: __dirname, + zones: [ + { + target: ['files/no_restricted_paths/(public|server)/**/*'], + from: [ + 'files/no_restricted_paths/server/**/*', + '!files/no_restricted_paths/server/index.{ts,tsx}', + ], + allowSameFolder: true, + }, + ], + }, + ], + }, ], invalid: [ @@ -369,5 +390,34 @@ ruleTester.run('@kbn/eslint/no-restricted-paths', rule, { }, ], }, + + { + // Don't use index*. + // It won't work with dirs that start with 'index'. + code: 'import { X } from "./index_patterns"', + filename: path.join(__dirname, './files/no_restricted_paths/server/b.js'), + options: [ + { + basePath: __dirname, + zones: [ + { + target: ['files/no_restricted_paths/(public|server)/**/*'], + from: [ + 'files/no_restricted_paths/server/**/*', + '!files/no_restricted_paths/server/index*', + ], + allowSameFolder: true, + }, + ], + }, + ], + errors: [ + { + message: 'Unexpected path "./index_patterns" imported in restricted zone.', + line: 1, + column: 19, + }, + ], + }, ], });