[ML] Fixing time range selection when cloning a job (#48935)

elastic · Oct 23, 2019 · 3b6c2b6 · 3b6c2b6
1 parent f7796c3
commit 3b6c2b6
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 37 deletions.
diff --git a/x-pack/legacy/plugins/ml/public/jobs/jobs_list/components/utils.js b/x-pack/legacy/plugins/ml/public/jobs/jobs_list/components/utils.js
@@ -178,7 +178,7 @@ export function cloneJob(jobId) {
           mlJobService.tempJobCloningObjects.end = end;
         }
       } else {
-        // otherwise use the currentJob
+        // otherwise use the tempJobCloningObjects
         mlJobService.tempJobCloningObjects.job = job;
       }
       window.location.href = '#/jobs/new_job';
@@ -284,13 +284,13 @@ export function filterJobs(jobs, clauses) {
   return filteredJobs;
 }
 
-// check to see if a job has been stored in mlJobService.currentJob
+// check to see if a job has been stored in mlJobService.tempJobCloningObjects
 // if it has, return an object with the minimum properties needed for the
 // start datafeed modal.
 export function checkForAutoStartDatafeed() {
-  const job = mlJobService.currentJob;
+  const job = mlJobService.tempJobCloningObjects.job;
   if (job !== undefined) {
-    mlJobService.currentJob = undefined;
+    mlJobService.tempJobCloningObjects.job = undefined;
     const hasDatafeed = (typeof job.datafeed_config === 'object' && Object.keys(job.datafeed_config).length > 0);
     const datafeedId = hasDatafeed ? job.datafeed_config.datafeed_id : '';
     return {

diff --git a/x-pack/legacy/plugins/ml/public/jobs/new_job_new/common/job_creator/util/general.ts b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/common/job_creator/util/general.ts
@@ -146,33 +146,29 @@ export function isSparseDataJob(job: Job, datafeed: Datafeed): boolean {
 function stashCombinedJob(
   jobCreator: JobCreatorType,
   skipTimeRangeStep: boolean = false,
-  advanced: boolean = false,
   includeTimeRange: boolean = false
 ) {
   const combinedJob = {
     ...jobCreator.jobConfig,
     datafeed_config: jobCreator.datafeedConfig,
   };
-  if (advanced === true) {
-    mlJobService.currentJob = combinedJob;
-  } else {
-    mlJobService.tempJobCloningObjects.job = combinedJob;
 
-    // skip over the time picker step of the wizard
-    mlJobService.tempJobCloningObjects.skipTimeRangeStep = skipTimeRangeStep;
+  mlJobService.tempJobCloningObjects.job = combinedJob;
 
-    if (includeTimeRange === true) {
-      // auto select the start and end dates of the time picker
-      mlJobService.tempJobCloningObjects.start = jobCreator.start;
-      mlJobService.tempJobCloningObjects.end = jobCreator.end;
-    }
+  // skip over the time picker step of the wizard
+  mlJobService.tempJobCloningObjects.skipTimeRangeStep = skipTimeRangeStep;
+
+  if (includeTimeRange === true) {
+    // auto select the start and end dates of the time picker
+    mlJobService.tempJobCloningObjects.start = jobCreator.start;
+    mlJobService.tempJobCloningObjects.end = jobCreator.end;
   }
 }
 
 export function convertToMultiMetricJob(jobCreator: JobCreatorType) {
   jobCreator.createdBy = CREATED_BY_LABEL.MULTI_METRIC;
   jobCreator.modelPlot = false;
-  stashCombinedJob(jobCreator, true, false, true);
+  stashCombinedJob(jobCreator, true, true);
 
   window.location.href = window.location.href.replace(
     JOB_TYPE.SINGLE_METRIC,
@@ -182,7 +178,7 @@ export function convertToMultiMetricJob(jobCreator: JobCreatorType) {
 
 export function convertToAdvancedJob(jobCreator: JobCreatorType) {
   jobCreator.createdBy = null;
-  stashCombinedJob(jobCreator, true, false, false);
+  stashCombinedJob(jobCreator, true, true);
 
   let jobType = JOB_TYPE.SINGLE_METRIC;
   if (isMultiMetricJobCreator(jobCreator)) {
@@ -196,13 +192,13 @@ export function convertToAdvancedJob(jobCreator: JobCreatorType) {
 
 export function resetJob(jobCreator: JobCreatorType) {
   jobCreator.jobId = '';
-  stashCombinedJob(jobCreator, true, false, true);
+  stashCombinedJob(jobCreator, true, true);
 
   window.location.href = '#/jobs/new_job';
 }
 
 export function advancedStartDatafeed(jobCreator: JobCreatorType) {
-  stashCombinedJob(jobCreator, false, true, false);
+  stashCombinedJob(jobCreator, false, false);
   window.location.href = '#/jobs';
 }
 

diff --git a/x-pack/legacy/plugins/ml/public/jobs/new_job_new/pages/new_job/page.tsx b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/pages/new_job/page.tsx
@@ -52,6 +52,8 @@ export const Page: FC<PageProps> = ({ existingJobsAndGroups, jobType }) => {
       ? WIZARD_STEPS.ADVANCED_CONFIGURE_DATAFEED
       : WIZARD_STEPS.TIME_RANGE;
 
+  let autoSetTimeRange = false;
+
   if (mlJobService.tempJobCloningObjects.job !== undefined) {
     // cloning a job
     const clonedJob = mlJobService.cloneJob(mlJobService.tempJobCloningObjects.job);
@@ -79,6 +81,10 @@ export const Page: FC<PageProps> = ({ existingJobsAndGroups, jobType }) => {
       );
       mlJobService.tempJobCloningObjects.start = undefined;
       mlJobService.tempJobCloningObjects.end = undefined;
+    } else {
+      // if not start and end times are set and this is an advanced job,
+      // auto set the time range based on the index
+      autoSetTimeRange = isAdvancedJobCreator(jobCreator);
     }
   } else {
     // creating a new job
@@ -99,19 +105,22 @@ export const Page: FC<PageProps> = ({ existingJobsAndGroups, jobType }) => {
       jobCreator.createdBy = null;
     }
 
-    if (isAdvancedJobCreator(jobCreator)) {
-      // for advanced jobs, load the full time range start and end times
-      // so they can be used for job validation and bucket span estimation
-      try {
-        jobCreator.autoSetTimeRange();
-      } catch (error) {
-        toastNotifications.addDanger({
-          title: i18n.translate('xpack.ml.newJob.wizard.autoSetJobCreatorTimeRange.error', {
-            defaultMessage: `Error retrieving beginning and end times of index`,
-          }),
-          text: error,
-        });
-      }
+    // auto set the time range if creating a new advanced job
+    autoSetTimeRange = isAdvancedJobCreator(jobCreator);
+  }
+
+  if (autoSetTimeRange && isAdvancedJobCreator(jobCreator)) {
+    // for advanced jobs, load the full time range start and end times
+    // so they can be used for job validation and bucket span estimation
+    try {
+      jobCreator.autoSetTimeRange();
+    } catch (error) {
+      toastNotifications.addDanger({
+        title: i18n.translate('xpack.ml.newJob.wizard.autoSetJobCreatorTimeRange.error', {
+          defaultMessage: `Error retrieving beginning and end times of index`,
+        }),
+        text: error,
+      });
     }
   }
 

diff --git a/x-pack/legacy/plugins/ml/public/services/job_service.d.ts b/x-pack/legacy/plugins/ml/public/services/job_service.d.ts
@@ -13,7 +13,6 @@ export interface ExistingJobsAndGroups {
 }
 
 declare interface JobService {
-  currentJob: any;
   createResultsUrlForJobs: (jobs: any[], target: string) => string;
   tempJobCloningObjects: {
     job: any;

diff --git a/x-pack/legacy/plugins/ml/public/services/job_service.js b/x-pack/legacy/plugins/ml/public/services/job_service.js
@@ -24,13 +24,11 @@ let datafeedIds = {};
 
 class JobService {
   constructor() {
-    // currentJob -> used to pass a job object between the job management page and
+    // tempJobCloningObjects -> used to pass a job object between the job management page and
     // and the advanced wizard.
     // if populated when loading the advanced wizard, the job is used for cloning.
     // if populated when loading the job management page, the start datafeed modal
     // is automatically opened.
-    this.currentJob = undefined;
-
     this.tempJobCloningObjects = {
       job: undefined,
       skipTimeRangeStep: false,