Skip to content

Commit

Permalink
[ML] Fixing wizard validation delay (#45265) (#45362)
Browse files Browse the repository at this point in the history
* [ML] Fixing wizard validation delay

* changes based on review
  • Loading branch information
jgowdyelastic authored Sep 11, 2019
1 parent cb07ae9 commit f651e3f
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class JobValidator {
private _jobCreator: JobCreatorType;
private _validationSummary: ValidationSummary;
private _lastJobConfig: string;
private _validateTimeout: NodeJS.Timeout;
private _validateTimeout: ReturnType<typeof setTimeout> | null = null;
private _existingJobsAndGroups: ExistingJobsAndGroups;
private _basicValidations: BasicValidations = {
jobId: { valid: true },
Expand All @@ -46,6 +46,7 @@ export class JobValidator {
bucketSpan: { valid: true },
duplicateDetectors: { valid: true },
};
private _validating: boolean = false;

constructor(jobCreator: JobCreatorType, existingJobsAndGroups: ExistingJobsAndGroups) {
this._jobCreator = jobCreator;
Expand All @@ -54,25 +55,30 @@ export class JobValidator {
basic: false,
advanced: false,
};
this._validateTimeout = setTimeout(() => {}, 0);
this._existingJobsAndGroups = existingJobsAndGroups;
}

public validate() {
public validate(callback: () => void) {
this._validating = true;
const formattedJobConfig = this._jobCreator.formattedJobJson;
return new Promise((resolve: () => void) => {
// only validate if the config has changed
if (formattedJobConfig !== this._lastJobConfig) {
// only validate if the config has changed
if (formattedJobConfig !== this._lastJobConfig) {
if (this._validateTimeout !== null) {
// clear any previous on going validation timeouts
clearTimeout(this._validateTimeout);
this._lastJobConfig = formattedJobConfig;
this._validateTimeout = setTimeout(() => {
this._runBasicValidation();
resolve();
}, VALIDATION_DELAY_MS);
} else {
resolve();
}
});
this._lastJobConfig = formattedJobConfig;
this._validateTimeout = setTimeout(() => {
this._runBasicValidation();
this._validating = false;
this._validateTimeout = null;
callback();
}, VALIDATION_DELAY_MS);
} else {
// _validating is still true if there is a previous validation timeout on going.
this._validating = this._validateTimeout !== null;
}
callback();
}

private _resetBasicValidations() {
Expand Down Expand Up @@ -135,4 +141,8 @@ export class JobValidator {
public set advancedValid(valid: boolean) {
this._validationSummary.advanced = valid;
}

public get validating(): boolean {
return this._validating;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ export const JobDetailsStep: FC<Props> = ({
const active =
jobValidator.jobId.valid &&
jobValidator.modelMemoryLimit.valid &&
jobValidator.groupIds.valid;
jobValidator.groupIds.valid &&
jobValidator.validating === false;
setNextActive(active);
}, [jobValidatorUpdated]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export const PickFieldsStep: FC<StepProps> = ({ setCurrentStep, isCurrentStep })
const active =
jobCreator.detectors.length > 0 &&
jobValidator.bucketSpan.valid &&
jobValidator.duplicateDetectors.valid;
jobValidator.duplicateDetectors.valid &&
jobValidator.validating === false;
setNextActive(active);
}, [jobValidatorUpdated]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,9 @@ export const Wizard: FC<Props> = ({
);

useEffect(() => {
// IIFE to run the validation. the useEffect callback can't be async
(async () => {
await jobValidator.validate();
jobValidator.validate(() => {
setJobValidatorUpdate(jobValidatorUpdated);
})();
});

// if the job config has changed, reset the highestStep
// compare a stringified config to ensure the configs have actually changed
Expand Down

0 comments on commit f651e3f

Please sign in to comment.