From bf6cc70d2433ae58bcf45c8d97eda2fbe2474140 Mon Sep 17 00:00:00 2001 From: James Gowdy Date: Mon, 23 Jul 2018 12:39:47 +0100 Subject: [PATCH] [ML] Adding validation to the edit job flyout (#21041) * [ML] Adding validation to the edit job flyout * removing a bit of lodash * tiny code clean up * fixing validation check --- x-pack/plugins/ml/common/util/job_utils.js | 102 ++++++++++++------ .../edit_job_flyout/edit_job_flyout.js | 29 ++++- .../components/edit_job_flyout/edit_utils.js | 5 +- .../edit_job_flyout/tabs/job_details.js | 14 +++ .../edit_job_flyout/validate_job.js | 52 +++++++++ .../multi_job_actions/actions_menu.js | 2 +- .../simple/components/utils/validate_job.js | 22 ++-- 7 files changed, 183 insertions(+), 43 deletions(-) create mode 100644 x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/validate_job.js diff --git a/x-pack/plugins/ml/common/util/job_utils.js b/x-pack/plugins/ml/common/util/job_utils.js index 2319ced16edcb..59e9273b236a8 100644 --- a/x-pack/plugins/ml/common/util/job_utils.js +++ b/x-pack/plugins/ml/common/util/job_utils.js @@ -251,20 +251,14 @@ export function basicJobValidation(job, fields, limits) { messages.push({ id: 'job_id_valid' }); } - if (job.groups !== undefined) { - let groupIdValid = true; - job.groups.forEach(group => { - if (isJobIdValid(group) === false) { - groupIdValid = false; - valid = false; - } - }); - if (job.groups.length > 0 && groupIdValid) { - messages.push({ id: 'job_group_id_valid' }); - } else if (job.groups.length > 0 && !groupIdValid) { - messages.push({ id: 'job_group_id_invalid' }); - } - } + // group names + const { + messages: groupsMessages, + valid: groupsValid, + } = validateGroupNames(job); + + messages.push(...groupsMessages); + valid = (valid && groupsValid); // Analysis Configuration if (job.analysis_config.categorization_filters) { @@ -372,22 +366,13 @@ export function basicJobValidation(job, fields, limits) { } // model memory limit - if (typeof job.analysis_limits !== 'undefined' && typeof job.analysis_limits.model_memory_limit !== 'undefined') { - if (typeof limits === 'object' && typeof limits.max_model_memory_limit !== 'undefined') { - const max = limits.max_model_memory_limit.toUpperCase(); - const mml = job.analysis_limits.model_memory_limit.toUpperCase(); - - const mmlBytes = numeral(mml).value(); - const maxBytes = numeral(max).value(); - - if(mmlBytes > maxBytes) { - messages.push({ id: 'model_memory_limit_invalid' }); - valid = false; - } else { - messages.push({ id: 'model_memory_limit_valid' }); - } - } - } + const { + messages: mmlMessages, + valid: mmlValid, + } = validateModelMemoryLimit(job, limits); + + messages.push(...mmlMessages); + valid = (valid && mmlValid); } else { valid = false; @@ -396,7 +381,60 @@ export function basicJobValidation(job, fields, limits) { return { messages, valid, - contains(id) { return _.some(messages, { id }); }, - find(id) { return _.find(messages, { id }); } + contains: id => (messages.some(m => id === m.id)), + find: id => (messages.find(m => id === m.id)), + }; +} + +export function validateModelMemoryLimit(job, limits) { + const messages = []; + let valid = true; + // model memory limit + if (typeof job.analysis_limits !== 'undefined' && typeof job.analysis_limits.model_memory_limit !== 'undefined') { + if (typeof limits === 'object' && typeof limits.max_model_memory_limit !== 'undefined') { + const max = limits.max_model_memory_limit.toUpperCase(); + const mml = job.analysis_limits.model_memory_limit.toUpperCase(); + + const mmlBytes = numeral(mml).value(); + const maxBytes = numeral(max).value(); + + if(mmlBytes > maxBytes) { + messages.push({ id: 'model_memory_limit_invalid' }); + valid = false; + } else { + messages.push({ id: 'model_memory_limit_valid' }); + } + } + } + return { + valid, + messages, + contains: id => (messages.some(m => id === m.id)), + find: id => (messages.find(m => id === m.id)), + }; +} + +export function validateGroupNames(job) { + const messages = []; + let valid = true; + if (job.groups !== undefined) { + let groupIdValid = true; + job.groups.forEach(group => { + if (isJobIdValid(group) === false) { + groupIdValid = false; + valid = false; + } + }); + if (job.groups.length > 0 && groupIdValid) { + messages.push({ id: 'job_group_id_valid' }); + } else if (job.groups.length > 0 && !groupIdValid) { + messages.push({ id: 'job_group_id_invalid' }); + } + } + return { + valid, + messages, + contains: id => (messages.some(m => id === m.id)), + find: id => (messages.find(m => id === m.id)), }; } diff --git a/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/edit_job_flyout.js b/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/edit_job_flyout.js index 67474be4f9e2f..d256c8f0868c3 100644 --- a/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/edit_job_flyout.js +++ b/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/edit_job_flyout.js @@ -26,6 +26,7 @@ import { import { JobDetails, Detectors, Datafeed, CustomUrls } from './tabs'; import { saveJob } from './edit_utils'; import { loadFullJob } from '../utils'; +import { validateModelMemoryLimit, validateGroupNames } from './validate_job'; import { toastNotifications } from 'ui/notify'; export class EditJobFlyout extends Component { @@ -46,6 +47,9 @@ export class EditJobFlyout extends Component { datafeedQueryDelay: '', datafeedFrequency: '', datafeedScrollSize: '', + jobModelMemoryLimitValidationError: '', + jobGroupsValidationError: '', + valid: true, }; this.refreshJobs = this.props.refreshJobs; @@ -111,12 +115,29 @@ export class EditJobFlyout extends Component { datafeedQueryDelay: (hasDatafeed) ? datafeedConfig.query_delay : '', datafeedFrequency: (hasDatafeed) ? frequency : '', datafeedScrollSize: (hasDatafeed) ? +datafeedConfig.scroll_size : '', + jobModelMemoryLimitValidationError: '', + jobGroupsValidationError: '', }); } setJobDetails = (jobDetails) => { + let { jobModelMemoryLimitValidationError, jobGroupsValidationError } = this.state; + + if (jobDetails.jobModelMemoryLimit !== undefined) { + jobModelMemoryLimitValidationError = validateModelMemoryLimit(jobDetails.jobModelMemoryLimit).message; + } + + if (jobDetails.jobGroups !== undefined) { + jobGroupsValidationError = validateGroupNames(jobDetails.jobGroups).message; + } + + const valid = (jobModelMemoryLimitValidationError === '' && jobGroupsValidationError === ''); + this.setState({ - ...jobDetails + ...jobDetails, + jobModelMemoryLimitValidationError, + jobGroupsValidationError, + valid, }); } @@ -180,6 +201,9 @@ export class EditJobFlyout extends Component { datafeedQueryDelay, datafeedFrequency, datafeedScrollSize, + jobGroupsValidationError, + jobModelMemoryLimitValidationError, + valid, } = this.state; const tabs = [{ @@ -190,6 +214,8 @@ export class EditJobFlyout extends Component { jobGroups={jobGroups} jobModelMemoryLimit={jobModelMemoryLimit} setJobDetails={this.setJobDetails} + jobGroupsValidationError={jobGroupsValidationError} + jobModelMemoryLimitValidationError={jobModelMemoryLimitValidationError} />, }, { id: 'detectors', @@ -258,6 +284,7 @@ export class EditJobFlyout extends Component { Save diff --git a/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/edit_utils.js b/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/edit_utils.js index 0d186afbda2bd..95bab7a8bb5d3 100644 --- a/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/edit_utils.js +++ b/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/edit_utils.js @@ -46,8 +46,11 @@ export function saveJob(job, newJobData, finish) { if (resp.success) { saveDatafeedWrapper(); } else { - reject(); + reject(resp); } + }) + .catch((error) => { + reject(error); }); } else { saveDatafeedWrapper(); diff --git a/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/tabs/job_details.js b/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/tabs/job_details.js index 876f70994a85a..95cc2c352a800 100644 --- a/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/tabs/job_details.js +++ b/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/tabs/job_details.js @@ -30,6 +30,8 @@ export class JobDetails extends Component { groups: [], selectedGroups: [], mml: '', + mmlValidationError: '', + groupsValidationError: '', }; this.setJobDetails = props.setJobDetails; @@ -56,6 +58,8 @@ export class JobDetails extends Component { description: props.jobDescription, selectedGroups, mml: props.jobModelMemoryLimit, + mmlValidationError: props.jobModelMemoryLimitValidationError, + groupsValidationError: props.jobGroupsValidationError, }; } @@ -104,6 +108,8 @@ export class JobDetails extends Component { selectedGroups, mml, groups, + mmlValidationError, + groupsValidationError, } = this.state; return ( @@ -119,6 +125,8 @@ export class JobDetails extends Component { diff --git a/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/validate_job.js b/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/validate_job.js new file mode 100644 index 0000000000000..3aec2c395cec5 --- /dev/null +++ b/x-pack/plugins/ml/public/jobs/jobs_list_new/components/edit_job_flyout/validate_job.js @@ -0,0 +1,52 @@ +/* + * 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 { newJobLimits } from 'plugins/ml/jobs/new_job/utils/new_job_defaults'; +import { populateValidationMessages } from 'plugins/ml/jobs/new_job/simple/components/utils/validate_job'; + +import { + validateModelMemoryLimit as validateModelMemoryLimitUtils, + validateGroupNames as validateGroupNamesUtils, +} from 'plugins/ml/../common/util/job_utils'; + +export function validateModelMemoryLimit(mml) { + const limits = newJobLimits(); + const tempJob = { + analysis_limits: { + model_memory_limit: mml + } + }; + const validationResults = validateModelMemoryLimitUtils(tempJob, limits); + const { valid } = validationResults; + + const modelMemoryLimit = { + valid, + message: '', + }; + + populateValidationMessages(validationResults, { modelMemoryLimit }); + + return modelMemoryLimit; +} + +export function validateGroupNames(groups) { + const tempJob = { + groups + }; + + const validationResults = validateGroupNamesUtils(tempJob); + const { valid } = validationResults; + + const groupIds = { + valid, + message: '', + }; + + populateValidationMessages(validationResults, { groupIds }); + + return groupIds; +} diff --git a/x-pack/plugins/ml/public/jobs/jobs_list_new/components/multi_job_actions/actions_menu.js b/x-pack/plugins/ml/public/jobs/jobs_list_new/components/multi_job_actions/actions_menu.js index 43581db20ca06..c4fbc39eba7c7 100644 --- a/x-pack/plugins/ml/public/jobs/jobs_list_new/components/multi_job_actions/actions_menu.js +++ b/x-pack/plugins/ml/public/jobs/jobs_list_new/components/multi_job_actions/actions_menu.js @@ -58,7 +58,7 @@ export class MultiJobActionsMenu extends Component { size="s" onClick={this.onButtonClick} iconType="gear" - aria-label="Next" + aria-label="Management actions" color="text" disabled={(this.canDeleteJob === false && this.canStartStopDatafeed === false)} /> diff --git a/x-pack/plugins/ml/public/jobs/new_job/simple/components/utils/validate_job.js b/x-pack/plugins/ml/public/jobs/new_job/simple/components/utils/validate_job.js index 752976debbc9e..01278ca06e386 100644 --- a/x-pack/plugins/ml/public/jobs/new_job/simple/components/utils/validate_job.js +++ b/x-pack/plugins/ml/public/jobs/new_job/simple/components/utils/validate_job.js @@ -20,6 +20,20 @@ export function validateJob(job, checks) { item.valid = true; }); + populateValidationMessages(validationResults, checks); + + _.each(checks, (item) => { + if (item.valid === false) { + valid = false; + } + }); + + return valid; +} + +export function populateValidationMessages(validationResults, checks) { + const limits = newJobLimits(); + if (validationResults.contains('job_id_empty')) { checks.jobId.valid = false; } else if (validationResults.contains('job_id_invalid')) { @@ -47,12 +61,4 @@ export function validateJob(job, checks) { const msg = 'Duplicate detectors were found.'; checks.duplicateDetectors.message = msg; } - - _.each(checks, (item) => { - if (item.valid === false) { - valid = false; - } - }); - - return valid; }