[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
elastic · Jul 23, 2018 · bf6cc70 · bf6cc70
1 parent d6453fa
commit bf6cc70
Show file tree

Hide file tree

Showing 7 changed files with 183 additions and 43 deletions.
diff --git 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
@@ -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 {
                 <EuiButton
                   onClick={this.save}
                   fill
+                  isDisabled={(valid === false)}
                 >
                   Save
                 </EuiButton>

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
@@ -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
@@ -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 (
       <React.Fragment>
@@ -119,6 +125,8 @@ export class JobDetails extends Component {
           </EuiFormRow>
           <EuiFormRow
             label="Job groups"
+            isInvalid={(groupsValidationError !== '')}
+            error={groupsValidationError}
           >
             <EuiComboBox
               placeholder="Select or create groups"
@@ -127,14 +135,20 @@ export class JobDetails extends Component {
               onChange={this.onGroupsChange}
               onCreateOption={this.onCreateGroup}
               isClearable={true}
+              isInvalid={(groupsValidationError !== '')}
+              error={groupsValidationError}
             />
           </EuiFormRow>
           <EuiFormRow
             label="Model memory limit"
+            isInvalid={(mmlValidationError !== '')}
+            error={mmlValidationError}
           >
             <EuiFieldText
               value={mml}
               onChange={this.onMmlChange}
+              isInvalid={(mmlValidationError !== '')}
+              error={mmlValidationError}
             />
           </EuiFormRow>
         </EuiForm>

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
@@ -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
@@ -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
@@ -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;
 }