From d615857af66672080d55efa1e727011c77b1ab3a Mon Sep 17 00:00:00 2001 From: Andrew Scobie Date: Tue, 29 May 2018 16:18:26 +1200 Subject: [PATCH 1/7] add job id adv type. clean up css for hint, add margin to bottom of ncj form --- .../cloud-file-picker/cloud-file-picker.scss | 5 -- .../data/shared/data.shared.module.ts | 10 ++- .../file-group-picker/file-group-picker.scss | 8 --- .../shared/file-group-sas/file-group-sas.html | 2 +- app/components/data/shared/index.ts | 1 + app/components/data/shared/job-id/index.ts | 1 + .../data/shared/job-id/job-id.component.ts | 67 +++++++++++++++++++ app/components/data/shared/job-id/job-id.html | 10 +++ app/components/data/shared/job-id/job-id.scss | 7 ++ .../market/submit/market-application.model.ts | 1 + .../parameter-input/parameter-input.html | 4 ++ .../parameter-input/parameter-input.scss | 15 +++++ .../market/submit/submit-ncj-template.scss | 3 +- 13 files changed, 112 insertions(+), 22 deletions(-) create mode 100644 app/components/data/shared/job-id/index.ts create mode 100644 app/components/data/shared/job-id/job-id.component.ts create mode 100644 app/components/data/shared/job-id/job-id.html create mode 100644 app/components/data/shared/job-id/job-id.scss diff --git a/app/components/data/shared/cloud-file-picker/cloud-file-picker.scss b/app/components/data/shared/cloud-file-picker/cloud-file-picker.scss index fccaef3e8d..1438e2d6eb 100644 --- a/app/components/data/shared/cloud-file-picker/cloud-file-picker.scss +++ b/app/components/data/shared/cloud-file-picker/cloud-file-picker.scss @@ -5,9 +5,6 @@ bl-cloud-file-picker { bl-form-field { flex: 1; - .warning { - color: map-get($warn, 500); - } .input-container > .input-suffix { border: none; padding: 0; @@ -29,6 +26,4 @@ bl-cloud-file-picker { } } } - - } diff --git a/app/components/data/shared/data.shared.module.ts b/app/components/data/shared/data.shared.module.ts index 63d25d1ccc..edf9fd47eb 100644 --- a/app/components/data/shared/data.shared.module.ts +++ b/app/components/data/shared/data.shared.module.ts @@ -9,15 +9,13 @@ import { FileGroupPickerComponent } from "./file-group-picker"; import { FileGroupSasComponent } from "./file-group-sas"; import { FileGroupsPickerComponent } from "./file-groups-picker"; import { FileOrDirectoryPickerComponent } from "./file-or-directory-picker"; +import { JobIdComponent } from "./job-id/job-id.component"; import { StorageAccountPickerComponent } from "./storage-account-picker"; const components = [ - FileGroupPickerComponent, FileGroupSasComponent, FileGroupsPickerComponent, - CloudFilePickerComponent, CloudFilePickerDialogComponent, - StorageErrorDisplayComponent, - BlobContainerPickerComponent, - StorageAccountPickerComponent, - FileOrDirectoryPickerComponent, + BlobContainerPickerComponent, CloudFilePickerComponent, CloudFilePickerDialogComponent, + FileGroupPickerComponent, FileGroupSasComponent, FileGroupsPickerComponent, FileOrDirectoryPickerComponent, + JobIdComponent, StorageAccountPickerComponent, StorageErrorDisplayComponent, ]; @NgModule({ diff --git a/app/components/data/shared/file-group-picker/file-group-picker.scss b/app/components/data/shared/file-group-picker/file-group-picker.scss index bff79e6b72..390d1454ef 100644 --- a/app/components/data/shared/file-group-picker/file-group-picker.scss +++ b/app/components/data/shared/file-group-picker/file-group-picker.scss @@ -4,14 +4,6 @@ bl-file-group-picker { bl-form-field { width: 100%; flex: 1; - - .warning { - color: map-get($warn, 500); - } - } - - bl-button { - margin-left: 5px; } } .mat-autocomplete-panel { diff --git a/app/components/data/shared/file-group-sas/file-group-sas.html b/app/components/data/shared/file-group-sas/file-group-sas.html index d4ae4d963f..5136879122 100644 --- a/app/components/data/shared/file-group-sas/file-group-sas.html +++ b/app/components/data/shared/file-group-sas/file-group-sas.html @@ -1,7 +1,7 @@ + {{hint}} - diff --git a/app/components/data/shared/index.ts b/app/components/data/shared/index.ts index d187d95754..d7e0912cf6 100644 --- a/app/components/data/shared/index.ts +++ b/app/components/data/shared/index.ts @@ -2,3 +2,4 @@ export * from "./data.shared.module"; export * from "./file-group-picker"; export * from "./file-groups-picker"; export * from "./errors"; +export * from "./job-id"; diff --git a/app/components/data/shared/job-id/index.ts b/app/components/data/shared/job-id/index.ts new file mode 100644 index 0000000000..a19cd7faba --- /dev/null +++ b/app/components/data/shared/job-id/index.ts @@ -0,0 +1 @@ +export * from "./job-id.component"; diff --git a/app/components/data/shared/job-id/job-id.component.ts b/app/components/data/shared/job-id/job-id.component.ts new file mode 100644 index 0000000000..ee62ae86f2 --- /dev/null +++ b/app/components/data/shared/job-id/job-id.component.ts @@ -0,0 +1,67 @@ +import { Component, Input, OnDestroy, forwardRef } from "@angular/core"; +import { ControlValueAccessor, FormControl, NG_VALIDATORS, NG_VALUE_ACCESSOR } from "@angular/forms"; +import { Subscription } from "rxjs"; + +import { JobService } from "app/services"; + +import "./job-id.scss"; + +// tslint:disable:no-forward-ref +@Component({ + selector: "bl-job-id", + templateUrl: "job-id.html", + providers: [ + { provide: NG_VALUE_ACCESSOR, useExisting: forwardRef(() => JobIdComponent), multi: true }, + { provide: NG_VALIDATORS, useExisting: forwardRef(() => JobIdComponent), multi: true }, + ], +}) +export class JobIdComponent implements ControlValueAccessor, OnDestroy { + @Input() public label: string; + @Input() public hint: string; + + public value = new FormControl(); + public warning = false; + + private _propagateChange: (value: any) => void = null; + private _subscriptions: Subscription[] = []; + + constructor(private jobService: JobService) { + this._subscriptions.push(this.value.valueChanges.debounceTime(400).distinctUntilChanged().subscribe((value) => { + this._checkValid(value); + if (this._propagateChange) { + this._propagateChange(value); + } + })); + } + + public ngOnDestroy() { + this._subscriptions.forEach(x => x.unsubscribe()); + } + + public writeValue(value: string) { + this.value.setValue(value); + } + + public registerOnChange(fn) { + this._propagateChange = fn; + } + + public registerOnTouched() { + // Do nothing + } + + public validate(c: FormControl) { + return null; + } + + private _checkValid(value: string) { + this.jobService.get(value).subscribe({ + next: (job: any) => { + this.warning = true; + }, + error: () => { + this.warning = false; + }, + }); + } +} diff --git a/app/components/data/shared/job-id/job-id.html b/app/components/data/shared/job-id/job-id.html new file mode 100644 index 0000000000..5d168d80de --- /dev/null +++ b/app/components/data/shared/job-id/job-id.html @@ -0,0 +1,10 @@ + + + + The job ID has already been used, please choose a unique job ID + + + {{hint}} + + + diff --git a/app/components/data/shared/job-id/job-id.scss b/app/components/data/shared/job-id/job-id.scss new file mode 100644 index 0000000000..8271894ebf --- /dev/null +++ b/app/components/data/shared/job-id/job-id.scss @@ -0,0 +1,7 @@ +@import "app/styles/variables"; + +bl-job-id { + bl-form-field { + flex: 1; + } +} diff --git a/app/components/market/submit/market-application.model.ts b/app/components/market/submit/market-application.model.ts index 42ad3b7bbe..0caad9c93d 100644 --- a/app/components/market/submit/market-application.model.ts +++ b/app/components/market/submit/market-application.model.ts @@ -10,6 +10,7 @@ export enum NcjParameterExtendedType { fileGroupSas = "file-group-sas", fileGroupWriteSas = "file-group-write-sas", dropDown = "drop-down", + jobId = "job-id", } export class NcjParameterWrapper { diff --git a/app/components/market/submit/parameter-input/parameter-input.html b/app/components/market/submit/parameter-input/parameter-input.html index 2b91ef9aa3..b22f2877a2 100644 --- a/app/components/market/submit/parameter-input/parameter-input.html +++ b/app/components/market/submit/parameter-input/parameter-input.html @@ -24,6 +24,10 @@ [label]="parameter.name" class="form-element" [hint]="parameter.description" [wildcards]="parameter.wildcards"> +
+ + +
diff --git a/app/components/market/submit/parameter-input/parameter-input.scss b/app/components/market/submit/parameter-input/parameter-input.scss index 49e8c20b43..7002f3b02c 100644 --- a/app/components/market/submit/parameter-input/parameter-input.scss +++ b/app/components/market/submit/parameter-input/parameter-input.scss @@ -2,4 +2,19 @@ bl-parameter-input { width: 100%; + + bl-form-field { + bl-hint { + display: block; + width:100%; + + &.warning { + color: map-get($warn, 500); + } + + &.bl-align-right { + text-align: right; + } + } + } } diff --git a/app/components/market/submit/submit-ncj-template.scss b/app/components/market/submit/submit-ncj-template.scss index ff657643ea..1878061a5f 100644 --- a/app/components/market/submit/submit-ncj-template.scss +++ b/app/components/market/submit/submit-ncj-template.scss @@ -4,11 +4,10 @@ bl-submit-ncj-template { display: block; width: 100%; - // height: $contentview-height; - // background: $whitesmoke-darker; form { margin-left: 10px; + margin-bottom: 50px; } .appselect { From 3797ff83785e4266c731408fe8018f8a9f60e0b2 Mon Sep 17 00:00:00 2001 From: Andrew Scobie Date: Wed, 30 May 2018 09:56:55 +1200 Subject: [PATCH 2/7] as per review --- app/components/data/shared/data.shared.module.ts | 13 ++++++++++--- .../data/shared/job-id/job-id.component.ts | 8 ++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/app/components/data/shared/data.shared.module.ts b/app/components/data/shared/data.shared.module.ts index edf9fd47eb..fb0764f2da 100644 --- a/app/components/data/shared/data.shared.module.ts +++ b/app/components/data/shared/data.shared.module.ts @@ -13,9 +13,16 @@ import { JobIdComponent } from "./job-id/job-id.component"; import { StorageAccountPickerComponent } from "./storage-account-picker"; const components = [ - BlobContainerPickerComponent, CloudFilePickerComponent, CloudFilePickerDialogComponent, - FileGroupPickerComponent, FileGroupSasComponent, FileGroupsPickerComponent, FileOrDirectoryPickerComponent, - JobIdComponent, StorageAccountPickerComponent, StorageErrorDisplayComponent, + BlobContainerPickerComponent, + CloudFilePickerComponent, + CloudFilePickerDialogComponent, + FileGroupPickerComponent, + FileGroupSasComponent, + FileGroupsPickerComponent, + FileOrDirectoryPickerComponent, + JobIdComponent, + StorageAccountPickerComponent, + StorageErrorDisplayComponent, ]; @NgModule({ diff --git a/app/components/data/shared/job-id/job-id.component.ts b/app/components/data/shared/job-id/job-id.component.ts index ee62ae86f2..92e9b9db24 100644 --- a/app/components/data/shared/job-id/job-id.component.ts +++ b/app/components/data/shared/job-id/job-id.component.ts @@ -1,4 +1,4 @@ -import { Component, Input, OnDestroy, forwardRef } from "@angular/core"; +import { ChangeDetectionStrategy, Component, Input, OnDestroy, forwardRef } from "@angular/core"; import { ControlValueAccessor, FormControl, NG_VALIDATORS, NG_VALUE_ACCESSOR } from "@angular/forms"; import { Subscription } from "rxjs"; @@ -6,10 +6,10 @@ import { JobService } from "app/services"; import "./job-id.scss"; -// tslint:disable:no-forward-ref @Component({ selector: "bl-job-id", templateUrl: "job-id.html", + changeDetection: ChangeDetectionStrategy.OnPush, providers: [ { provide: NG_VALUE_ACCESSOR, useExisting: forwardRef(() => JobIdComponent), multi: true }, { provide: NG_VALIDATORS, useExisting: forwardRef(() => JobIdComponent), multi: true }, @@ -58,9 +58,13 @@ export class JobIdComponent implements ControlValueAccessor, OnDestroy { this.jobService.get(value).subscribe({ next: (job: any) => { this.warning = true; + this.value.setErrors({ + notUnique: true, + }); }, error: () => { this.warning = false; + this.value.setErrors({}); }, }); } From a2c3bf4fc366b4206ddcf79aa2ac57b3f7d404e2 Mon Sep 17 00:00:00 2001 From: Andrew Scobie Date: Wed, 30 May 2018 17:45:09 +1200 Subject: [PATCH 3/7] commit so can work from home --- .../market/submit/parameter-input/parameter-input.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/app/components/market/submit/parameter-input/parameter-input.component.ts b/app/components/market/submit/parameter-input/parameter-input.component.ts index f44477f4df..550383741f 100644 --- a/app/components/market/submit/parameter-input/parameter-input.component.ts +++ b/app/components/market/submit/parameter-input/parameter-input.component.ts @@ -63,6 +63,7 @@ export class ParameterInputComponent implements ControlValueAccessor, OnChanges, public validate() { const valid = this.parameterValue.valid; + console.log(`validate: ${this.parameterValue.name} - ${valid}`); if (valid) { return null; } else { From 83e395c7aead069c9170c1679be7f7b416e2c2af Mon Sep 17 00:00:00 2001 From: Andrew Scobie Date: Thu, 31 May 2018 12:21:12 +1200 Subject: [PATCH 4/7] validation give up --- .../data/shared/job-id/job-id.component.ts | 67 ++++++++++++------- .../parameter-input.component.ts | 6 +- .../submit/submit-ncj-template.component.ts | 11 ++- 3 files changed, 54 insertions(+), 30 deletions(-) diff --git a/app/components/data/shared/job-id/job-id.component.ts b/app/components/data/shared/job-id/job-id.component.ts index 92e9b9db24..07436e5e9d 100644 --- a/app/components/data/shared/job-id/job-id.component.ts +++ b/app/components/data/shared/job-id/job-id.component.ts @@ -1,7 +1,15 @@ -import { ChangeDetectionStrategy, Component, Input, OnDestroy, forwardRef } from "@angular/core"; -import { ControlValueAccessor, FormControl, NG_VALIDATORS, NG_VALUE_ACCESSOR } from "@angular/forms"; -import { Subscription } from "rxjs"; +import { ChangeDetectionStrategy, ChangeDetectorRef, Component, Input, OnDestroy, forwardRef } from "@angular/core"; +import { + AsyncValidator, + ControlValueAccessor, + FormBuilder, + FormControl, + NG_ASYNC_VALIDATORS, + NG_VALUE_ACCESSOR, +} from "@angular/forms"; +import { Observable, Subscription } from "rxjs"; +import { Job } from "app/models"; import { JobService } from "app/services"; import "./job-id.scss"; @@ -12,30 +20,34 @@ import "./job-id.scss"; changeDetection: ChangeDetectionStrategy.OnPush, providers: [ { provide: NG_VALUE_ACCESSOR, useExisting: forwardRef(() => JobIdComponent), multi: true }, - { provide: NG_VALIDATORS, useExisting: forwardRef(() => JobIdComponent), multi: true }, + { provide: NG_ASYNC_VALIDATORS, useExisting: forwardRef(() => JobIdComponent), multi: true }, ], }) -export class JobIdComponent implements ControlValueAccessor, OnDestroy { +export class JobIdComponent implements AsyncValidator, ControlValueAccessor, OnDestroy { @Input() public label: string; @Input() public hint: string; - public value = new FormControl(); + public value: FormControl; public warning = false; private _propagateChange: (value: any) => void = null; - private _subscriptions: Subscription[] = []; + private _subscription: Subscription; - constructor(private jobService: JobService) { - this._subscriptions.push(this.value.valueChanges.debounceTime(400).distinctUntilChanged().subscribe((value) => { - this._checkValid(value); + constructor( + private changeDetector: ChangeDetectorRef, + private formBuilder: FormBuilder, + private jobService: JobService) { + + this.value = this.formBuilder.control([], null, this._validateJobUnique()); + this._subscription = this.value.valueChanges.debounceTime(400).distinctUntilChanged().subscribe((value) => { if (this._propagateChange) { this._propagateChange(value); } - })); + }); } public ngOnDestroy() { - this._subscriptions.forEach(x => x.unsubscribe()); + this._subscription.unsubscribe(); } public writeValue(value: string) { @@ -51,21 +63,26 @@ export class JobIdComponent implements ControlValueAccessor, OnDestroy { } public validate(c: FormControl) { - return null; + return Observable.of(null); } - private _checkValid(value: string) { - this.jobService.get(value).subscribe({ - next: (job: any) => { - this.warning = true; - this.value.setErrors({ - notUnique: true, + private _validateJobUnique() { + return (control: FormControl) => { + return Observable.of(null).debounceTime(250) + .flatMap(() => this.jobService.get(control.value)) + .map((job: Job) => { + this.warning = true; + this.changeDetector.markForCheck(); + return Observable.of({ + duplicateJob: { + valid: false, + }, + }); + }).catch(() => { + this.warning = false; + this.changeDetector.markForCheck(); + return Observable.of(null); }); - }, - error: () => { - this.warning = false; - this.value.setErrors({}); - }, - }); + }; } } diff --git a/app/components/market/submit/parameter-input/parameter-input.component.ts b/app/components/market/submit/parameter-input/parameter-input.component.ts index 550383741f..e71010eb56 100644 --- a/app/components/market/submit/parameter-input/parameter-input.component.ts +++ b/app/components/market/submit/parameter-input/parameter-input.component.ts @@ -5,9 +5,9 @@ import { Subscription } from "rxjs"; import { NcjParameterRawType } from "app/models"; import { NcjFileGroupService } from "app/services"; import { NcjParameterExtendedType, NcjParameterWrapper } from "../market-application.model"; + import "./parameter-input.scss"; -// tslint:disable:no-forward-ref @Component({ selector: "bl-parameter-input", templateUrl: "parameter-input.html", @@ -62,9 +62,7 @@ export class ParameterInputComponent implements ControlValueAccessor, OnChanges, } public validate() { - const valid = this.parameterValue.valid; - console.log(`validate: ${this.parameterValue.name} - ${valid}`); - if (valid) { + if (this.parameterValue.valid) { return null; } else { let messageText = "unknown error"; diff --git a/app/components/market/submit/submit-ncj-template.component.ts b/app/components/market/submit/submit-ncj-template.component.ts index 7c7aa83b30..cc49a952e3 100644 --- a/app/components/market/submit/submit-ncj-template.component.ts +++ b/app/components/market/submit/submit-ncj-template.component.ts @@ -1,4 +1,11 @@ -import { Component, Input, OnChanges, OnDestroy, OnInit } from "@angular/core"; +import { + ChangeDetectionStrategy, + Component, + Input, + OnChanges, + OnDestroy, + OnInit, +} from "@angular/core"; import { FormBuilder, FormControl, FormGroup, Validators } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; import { Observable, Subscription } from "rxjs"; @@ -21,6 +28,7 @@ import "./submit-ncj-template.scss"; @Component({ selector: "bl-submit-ncj-template", templateUrl: "submit-ncj-template.html", + changeDetection: ChangeDetectionStrategy.OnPush, }) export class SubmitNcjTemplateComponent implements OnInit, OnChanges, OnDestroy { @Input() public jobTemplate: NcjJobTemplate; @@ -349,6 +357,7 @@ export class SubmitNcjTemplateComponent implements OnInit, OnChanges, OnDestroy pool: pool, }, }; + return this.ncjSubmitService.submitJob(jobTemplate, this.jobParams.value) .cascade((data) => this._redirectToJob(data.properties.id)); } From 6d74e2ac7affff6a95e8cbd15b52aa385c56530c Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Thu, 31 May 2018 10:44:36 -0700 Subject: [PATCH 5/7] More validation --- .../data/shared/job-id/job-id.component.ts | 39 +++++++------- .../parameter-input.component.ts | 54 +++++++++++-------- .../utils/form-utils/form-utils.ts | 16 +++++- 3 files changed, 68 insertions(+), 41 deletions(-) diff --git a/app/components/data/shared/job-id/job-id.component.ts b/app/components/data/shared/job-id/job-id.component.ts index 07436e5e9d..2bfe5d197d 100644 --- a/app/components/data/shared/job-id/job-id.component.ts +++ b/app/components/data/shared/job-id/job-id.component.ts @@ -12,6 +12,8 @@ import { Observable, Subscription } from "rxjs"; import { Job } from "app/models"; import { JobService } from "app/services"; +import { autobind } from "@batch-flask/core"; +import { FormUtils } from "@batch-flask/utils"; import "./job-id.scss"; @Component({ @@ -38,7 +40,7 @@ export class JobIdComponent implements AsyncValidator, ControlValueAccessor, OnD private formBuilder: FormBuilder, private jobService: JobService) { - this.value = this.formBuilder.control([], null, this._validateJobUnique()); + this.value = this.formBuilder.control([], null, this._validateJobUnique); this._subscription = this.value.valueChanges.debounceTime(400).distinctUntilChanged().subscribe((value) => { if (this._propagateChange) { this._propagateChange(value); @@ -63,26 +65,25 @@ export class JobIdComponent implements AsyncValidator, ControlValueAccessor, OnD } public validate(c: FormControl) { - return Observable.of(null); + return FormUtils.passValidation(this.value); } - private _validateJobUnique() { - return (control: FormControl) => { - return Observable.of(null).debounceTime(250) - .flatMap(() => this.jobService.get(control.value)) - .map((job: Job) => { - this.warning = true; - this.changeDetector.markForCheck(); - return Observable.of({ - duplicateJob: { - valid: false, - }, - }); - }).catch(() => { - this.warning = false; - this.changeDetector.markForCheck(); - return Observable.of(null); + @autobind() + private _validateJobUnique(control: FormControl) { + return Observable.of(null).debounceTime(250) + .flatMap(() => this.jobService.get(control.value)) + .map((job: Job) => { + this.warning = true; + this.changeDetector.markForCheck(); + return Observable.of({ + duplicateJob: { + valid: false, + }, }); - }; + }).catch(() => { + this.warning = false; + this.changeDetector.markForCheck(); + return Observable.of(null); + }); } } diff --git a/app/components/market/submit/parameter-input/parameter-input.component.ts b/app/components/market/submit/parameter-input/parameter-input.component.ts index e71010eb56..a041855397 100644 --- a/app/components/market/submit/parameter-input/parameter-input.component.ts +++ b/app/components/market/submit/parameter-input/parameter-input.component.ts @@ -1,11 +1,19 @@ import { Component, Input, OnChanges, OnDestroy, forwardRef } from "@angular/core"; -import { ControlValueAccessor, FormControl, NG_VALIDATORS, NG_VALUE_ACCESSOR, Validators } from "@angular/forms"; -import { Subscription } from "rxjs"; +import { + ControlValueAccessor, + FormControl, + NG_ASYNC_VALIDATORS, + NG_VALIDATORS, + NG_VALUE_ACCESSOR, + Validators, +} from "@angular/forms"; +import { Observable, Subscription } from "rxjs"; import { NcjParameterRawType } from "app/models"; import { NcjFileGroupService } from "app/services"; import { NcjParameterExtendedType, NcjParameterWrapper } from "../market-application.model"; +import { FormUtils } from "@batch-flask/utils"; import "./parameter-input.scss"; @Component({ @@ -13,7 +21,7 @@ import "./parameter-input.scss"; templateUrl: "parameter-input.html", providers: [ { provide: NG_VALUE_ACCESSOR, useExisting: forwardRef(() => ParameterInputComponent), multi: true }, - { provide: NG_VALIDATORS, useExisting: forwardRef(() => ParameterInputComponent), multi: true }, + { provide: NG_ASYNC_VALIDATORS, useExisting: forwardRef(() => ParameterInputComponent), multi: true }, ], }) export class ParameterInputComponent implements ControlValueAccessor, OnChanges, OnDestroy { @@ -62,32 +70,36 @@ export class ParameterInputComponent implements ControlValueAccessor, OnChanges, } public validate() { + return FormUtils.passValidation(this.parameterValue, (e) => this._computeError(e)); + } + + private _computeError(errors) { if (this.parameterValue.valid) { return null; - } else { - let messageText = "unknown error"; - const error = this.parameterValue.errors; - if (error.minlength) { - const minLength = String(error.minlength.requiredLength); + } + let messageText = "unknown error"; + if (errors) { + if (errors.minlength) { + const minLength = String(errors.minlength.requiredLength); messageText = `Should be at least ${minLength} characters`; - } else if (error.maxlength) { - const maxLength = String(error.maxlength.requiredLength); + } else if (errors.maxlength) { + const maxLength = String(errors.maxlength.requiredLength); messageText = `Should be at most ${maxLength} characters`; - } else if (error.min) { - const minValue = String(error.min.min); + } else if (errors.min) { + const minValue = String(errors.min.min); messageText = `Should be greater than or equal to ${minValue}`; - } else if (error.max) { - const maxValue = String(error.max.max); + } else if (errors.max) { + const maxValue = String(errors.max.max); messageText = `Should be less than or equal to ${maxValue}`; } - - return { - validFormInput: { - valid: false, - message: messageText, - }, - }; } + + return { + validFormInput: { + valid: false, + message: messageText, + }, + }; } public registerOnChange(fn: any): void { diff --git a/src/@batch-flask/utils/form-utils/form-utils.ts b/src/@batch-flask/utils/form-utils/form-utils.ts index 93853a43a7..5e1fb70158 100644 --- a/src/@batch-flask/utils/form-utils/form-utils.ts +++ b/src/@batch-flask/utils/form-utils/form-utils.ts @@ -1,4 +1,6 @@ -import { AbstractControl, FormGroup } from "@angular/forms"; +import { AbstractControl, FormControl, FormGroup } from "@angular/forms"; +import { Observable } from "rxjs"; + export class FormUtils { public static getControl(formGroup: FormGroup, path: string | string[]): AbstractControl { const actualPath = Array.isArray(path) ? path : [path]; @@ -11,4 +13,16 @@ export class FormUtils { } return current; } + + public static passValidation(control: FormControl, processErrors?: (errors: any) => any): Observable { + processErrors = processErrors || ((errors) => errors); + + if (control.status === "PENDING") { + return control.statusChanges.filter(x => x !== "PENDING").take(1).map(() => { + return processErrors(control.errors); + }); + } else { + return Observable.of(processErrors(control.errors)); + } + } } From 78172d7c4696ae37e4d07c8220fbdc6e5aa10722 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Thu, 31 May 2018 10:45:21 -0700 Subject: [PATCH 6/7] Parameter input --- .../parameter-input.component.ts | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/app/components/market/submit/parameter-input/parameter-input.component.ts b/app/components/market/submit/parameter-input/parameter-input.component.ts index a041855397..06cf000504 100644 --- a/app/components/market/submit/parameter-input/parameter-input.component.ts +++ b/app/components/market/submit/parameter-input/parameter-input.component.ts @@ -3,11 +3,10 @@ import { ControlValueAccessor, FormControl, NG_ASYNC_VALIDATORS, - NG_VALIDATORS, NG_VALUE_ACCESSOR, Validators, } from "@angular/forms"; -import { Observable, Subscription } from "rxjs"; +import { Subscription } from "rxjs"; import { NcjParameterRawType } from "app/models"; import { NcjFileGroupService } from "app/services"; @@ -73,35 +72,6 @@ export class ParameterInputComponent implements ControlValueAccessor, OnChanges, return FormUtils.passValidation(this.parameterValue, (e) => this._computeError(e)); } - private _computeError(errors) { - if (this.parameterValue.valid) { - return null; - } - let messageText = "unknown error"; - if (errors) { - if (errors.minlength) { - const minLength = String(errors.minlength.requiredLength); - messageText = `Should be at least ${minLength} characters`; - } else if (errors.maxlength) { - const maxLength = String(errors.maxlength.requiredLength); - messageText = `Should be at most ${maxLength} characters`; - } else if (errors.min) { - const minValue = String(errors.min.min); - messageText = `Should be greater than or equal to ${minValue}`; - } else if (errors.max) { - const maxValue = String(errors.max.max); - messageText = `Should be less than or equal to ${maxValue}`; - } - } - - return { - validFormInput: { - valid: false, - message: messageText, - }, - }; - } - public registerOnChange(fn: any): void { this._propagateChange = fn; } @@ -137,4 +107,34 @@ export class ParameterInputComponent implements ControlValueAccessor, OnChanges, this.parameterValue.setValidators(Validators.compose(validatorGroup)); } + + private _computeError(errors) { + if (this.parameterValue.valid) { + return null; + } + let messageText = "unknown error"; + if (errors) { + if (errors.minlength) { + const minLength = String(errors.minlength.requiredLength); + messageText = `Should be at least ${minLength} characters`; + } else if (errors.maxlength) { + const maxLength = String(errors.maxlength.requiredLength); + messageText = `Should be at most ${maxLength} characters`; + } else if (errors.min) { + const minValue = String(errors.min.min); + messageText = `Should be greater than or equal to ${minValue}`; + } else if (errors.max) { + const maxValue = String(errors.max.max); + messageText = `Should be less than or equal to ${maxValue}`; + } + } + + return { + validFormInput: { + valid: false, + message: messageText, + }, + }; + } + } From 2ca5bef5da4a3d292fe695674b222e165070a5d0 Mon Sep 17 00:00:00 2001 From: Andrew Scobie Date: Mon, 11 Jun 2018 15:20:48 +1200 Subject: [PATCH 7/7] tests --- .../shared/job-id/job-id.component.spec.ts | 81 ++++++++++++++ .../submit/market-application.model.spec.ts | 23 ++++ .../parameter-input.component.spec.ts | 100 +++++++++++++++--- test/fixture.ts | 7 +- 4 files changed, 197 insertions(+), 14 deletions(-) create mode 100644 app/components/data/shared/job-id/job-id.component.spec.ts diff --git a/app/components/data/shared/job-id/job-id.component.spec.ts b/app/components/data/shared/job-id/job-id.component.spec.ts new file mode 100644 index 0000000000..361fe68543 --- /dev/null +++ b/app/components/data/shared/job-id/job-id.component.spec.ts @@ -0,0 +1,81 @@ +import { Component, DebugElement, NO_ERRORS_SCHEMA } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormControl, ReactiveFormsModule } from "@angular/forms"; +import { BrowserModule, By } from "@angular/platform-browser"; +import { ServerError } from "@batch-flask/core"; +import { Observable } from "rxjs/Observable"; + +import { JobService } from "app/services"; +import { JobIdComponent } from "./job-id.component"; + +import * as Fixtures from "test/fixture"; + +@Component({ + template: ``, +}) +class TestComponent { + public jobId = new FormControl(""); +} + +describe("JobIdComponent", () => { + let fixture: ComponentFixture; + let testComponent: TestComponent; + let de: DebugElement; + let jobServiceSpy; + + const error = "The job ID has already been used, please choose a unique job ID"; + + beforeEach(() => { + jobServiceSpy = { + get: jasmine.createSpy("get").and.callFake((jobId, ...args) => { + if (jobId === "not-found") { + return Observable.throw(ServerError.fromBatch({ + statusCode: 404, + code: "RandomTestErrorCode", + message: { value: "Try again ..." }, + })); + } + + return Observable.of(Fixtures.job.create({ id: jobId })); + }), + }; + + TestBed.configureTestingModule({ + imports: [ + BrowserModule, + ReactiveFormsModule, + ], + declarations: [ + JobIdComponent, + TestComponent, + ], + providers: [ + { provide: JobService, useValue: jobServiceSpy }, + ], + schemas: [NO_ERRORS_SCHEMA], + }); + + fixture = TestBed.createComponent(TestComponent); + testComponent = fixture.componentInstance; + de = fixture.debugElement.query(By.css("bl-job-id")); + fixture.detectChanges(); + }); + + it("Validate success when job not found", () => { + testComponent.jobId.setValue("not-found"); + fixture.detectChanges(); + + expect(testComponent.jobId.valid).toBe(true); + expect(testComponent.jobId.status).toBe("VALID"); + expect(de.nativeElement.textContent).not.toContain(error); + }); + + it("Validate fail when existing job found", () => { + testComponent.jobId.setValue("found"); + fixture.detectChanges(); + + expect(testComponent.jobId.valid).toBe(false); + expect(testComponent.jobId.status).toBe("INVALID"); + expect(de.nativeElement.textContent).toContain(error); + }); +}); diff --git a/app/components/market/submit/market-application.model.spec.ts b/app/components/market/submit/market-application.model.spec.ts index 3c0bfb15b7..6c5d289e56 100644 --- a/app/components/market/submit/market-application.model.spec.ts +++ b/app/components/market/submit/market-application.model.spec.ts @@ -204,4 +204,27 @@ describe("marketApplicationModel", () => { }); expect(parameter.type).toBe(NcjParameterExtendedType.fileGroupSas); }); + + it("should detect filegroup writable sas type", () => { + parameter = new NcjParameterWrapper("fileGroupWritableSas", { + type: NcjParameterRawType.string, + metadata : { + description: "description", + advancedType: NcjParameterExtendedType.fileGroupWriteSas, + }, + }); + expect(parameter.type).toBe(NcjParameterExtendedType.fileGroupWriteSas); + }); + + it("should detect job-ids type", () => { + parameter = new NcjParameterWrapper("jobName", { + type: NcjParameterRawType.string, + metadata : { + description: "description", + advancedType: NcjParameterExtendedType.jobId, + }, + }); + + expect(parameter.type).toBe(NcjParameterExtendedType.jobId); + }); }); diff --git a/app/components/market/submit/parameter-input/parameter-input.component.spec.ts b/app/components/market/submit/parameter-input/parameter-input.component.spec.ts index 110a9601ad..f11a51a115 100644 --- a/app/components/market/submit/parameter-input/parameter-input.component.spec.ts +++ b/app/components/market/submit/parameter-input/parameter-input.component.spec.ts @@ -4,19 +4,22 @@ import { FormControl, FormsModule, ReactiveFormsModule } from "@angular/forms"; import { By } from "@angular/platform-browser"; import { NoopAnimationsModule } from "@angular/platform-browser/animations"; import { RouterTestingModule } from "@angular/router/testing"; +import { MaterialModule, ServerError } from "@batch-flask/core"; import { PermissionService, SelectComponent, SelectModule } from "@batch-flask/ui"; +import { ButtonsModule } from "@batch-flask/ui/buttons"; +import { DialogService } from "@batch-flask/ui/dialogs"; +import { FormModule } from "@batch-flask/ui/form"; +import { SidebarManager } from "@batch-flask/ui/sidebar"; import { Subject } from "rxjs"; import { Observable } from "rxjs/Observable"; -import { MaterialModule } from "@batch-flask/core"; -import { DialogService } from "@batch-flask/ui/dialogs"; -import { SidebarManager } from "@batch-flask/ui/sidebar"; import { FileGroupPickerComponent } from "app/components/data/shared"; import { CloudFilePickerComponent } from "app/components/data/shared/cloud-file-picker"; import { FileGroupSasComponent } from "app/components/data/shared/file-group-sas"; +import { JobIdComponent } from "app/components/data/shared/job-id"; import { NcjParameterExtendedType, NcjParameterWrapper, ParameterInputComponent } from "app/components/market/submit"; -import { BatchApplication, NcjParameterRawType } from "app/models"; -import { NcjFileGroupService } from "app/services"; +import { BlobContainer, NcjParameterRawType } from "app/models"; +import { JobService, NcjFileGroupService } from "app/services"; import { AutoStorageService, StorageBlobService, StorageContainerService } from "app/services/storage"; import { Constants } from "app/utils"; @@ -64,13 +67,14 @@ describe("ParameterInputComponent", () => { let dialogServiceSpy: any; let storageBlobServiceSpy: any; let sidebarSpy: any; - let listProxy: MockListView; + let jobServiceSpy: any; + let listProxy: MockListView; beforeEach(() => { - listProxy = new MockListView(BatchApplication, { + listProxy = new MockListView(BlobContainer, { cacheKey: "id", items: [ - Fixtures.application.create({ id: "app-1" }), + Fixtures.container.create(), ], }); @@ -86,9 +90,23 @@ describe("ParameterInputComponent", () => { }, }; - storageBlobServiceSpy = { + jobServiceSpy = { + get: jasmine.createSpy("get").and.callFake((jobId, ...args) => { + if (jobId === "good") { + return Observable.throw(ServerError.fromBatch({ + statusCode: 404, + code: "RandomTestErrorCode", + message: { value: "Try again ..." }, + })); + } + + return Observable.of(Fixtures.job.create({ id: jobId })); + }), + }; + storageBlobServiceSpy = { }; + fileGroupServiceSpy = { addFileGroupPrefix: jasmine.createSpy("addFileGroupPrefix").and.callFake((fgName) => { return `${Constants.ncjFileGroupPrefix}${fgName}`; @@ -105,11 +123,24 @@ describe("ParameterInputComponent", () => { TestBed.configureTestingModule({ imports: [ - RouterTestingModule, ReactiveFormsModule, FormsModule, - MaterialModule, SelectModule, NoopAnimationsModule, + ButtonsModule, + FormModule, + FormsModule, + MaterialModule, + NoopAnimationsModule, + RouterTestingModule, + ReactiveFormsModule, + SelectModule, + ], + declarations: [ + CloudFilePickerComponent, + FileGroupPickerComponent, + FileGroupSasComponent, + JobIdComponent, + NoItemMockComponent, + ParameterInputComponent, + TestComponent, ], - declarations: [NoItemMockComponent, ParameterInputComponent, FileGroupSasComponent, - TestComponent, FileGroupPickerComponent, CloudFilePickerComponent], providers: [ { provide: NcjFileGroupService, useValue: fileGroupServiceSpy }, { provide: StorageContainerService, useValue: storageContainerSpy }, @@ -118,6 +149,7 @@ describe("ParameterInputComponent", () => { { provide: DialogService, useValue: dialogServiceSpy }, { provide: SidebarManager, useValue: sidebarSpy }, { provide: PermissionService, useValue: {} }, + { provide: JobService, useValue: jobServiceSpy }, ], schemas: [NO_ERRORS_SCHEMA], }); @@ -517,4 +549,46 @@ describe("ParameterInputComponent", () => { expect(fileInputComponent.value.value).toBe(`https://storage-acc-1.com?sastoken`); }); }); + + describe("job-id parameter type", () => { + const initialInput = ""; + const goodJobId = "good"; + const badJobId = "bad"; + let jobIdComponent: JobIdComponent; + let jobIdInputEl: DebugElement; + + beforeEach(() => { + testComponent.param = new NcjParameterWrapper("jobName", { + type: NcjParameterRawType.string, + metadata: { + description: "description", + advancedType: NcjParameterExtendedType.jobId, + }, + }); + + testComponent.paramControl.setValue(initialInput); + fixture.detectChanges(); + jobIdInputEl = de.query(By.css("bl-job-id")); + expect(jobIdInputEl).not.toBeFalsy(); + jobIdComponent = jobIdInputEl.componentInstance; + }); + + it("should show initial input", () => { + expect(jobIdComponent.value.value).toBe(initialInput); + }); + + it("should show updated input and validate", () => { + testComponent.paramControl.setValue(goodJobId); + fixture.detectChanges(); + expect(jobIdComponent.value.value).toBe(goodJobId); + expect(component.parameterValue.valid).toBe(true); + }); + + it("should update but fail validation with non existant job-id", () => { + testComponent.paramControl.setValue(badJobId); + fixture.detectChanges(); + expect(jobIdComponent.value.value).toBe(badJobId); + expect(component.parameterValue.valid).toBe(false); + }); + }); }); diff --git a/test/fixture.ts b/test/fixture.ts index f77f37721f..488acb6127 100644 --- a/test/fixture.ts +++ b/test/fixture.ts @@ -2,7 +2,7 @@ import { Type } from "@angular/core"; import * as moment from "moment"; import { PinnedEntityType } from "@batch-flask/core"; -import { AccountResource, ApplicationPackage, BatchApplication, File, Job, Node, PackageState, +import { AccountResource, ApplicationPackage, BatchApplication, BlobContainer, File, Job, Node, PackageState, Pool, Subscription, SubtaskInformation, Task, } from "app/models"; @@ -258,3 +258,8 @@ export const pinnable = new FixtureFactory(Object, { pinnableType: PinnedEntityType.Application, url: "", }); + +export const container = new FixtureFactory(BlobContainer, { + id: "fgrp-container", + name: "container", +});