Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: job id advanced parameter type #1383

Merged
merged 14 commits into from
Jun 11, 2018
Merged

Feature: job id advanced parameter type #1383

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d615857
add job id adv type. clean up css for hint, add margin to bottom of n…
ascobie May 29, 2018
3797ff8
as per review
ascobie May 29, 2018
ddbee42
Merge branch 'master' into feature/ncj-job-id
ascobie May 29, 2018
a2c3bf4
commit so can work from home
ascobie May 30, 2018
9a489f0
Merge branch 'master' into feature/ncj-job-id
ascobie May 30, 2018
83e395c
validation give up
ascobie May 31, 2018
b426109
Merge branch 'master' into feature/ncj-job-id
timotheeguerin May 31, 2018
6d74e2a
More validation
timotheeguerin May 31, 2018
78172d7
Parameter input
timotheeguerin May 31, 2018
2620511
Merge branch 'master' into feature/ncj-job-id
timotheeguerin May 31, 2018
677d3e7
Merge branch 'master' into feature/ncj-job-id
ascobie May 31, 2018
3c662da
Merge branch 'feature/ncj-job-id' of https://github.com/Azure/BatchLa…
ascobie May 31, 2018
2ca5bef
tests
ascobie Jun 11, 2018
2cb5a62
Merge branch 'master' into feature/ncj-job-id
ascobie Jun 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions app/components/data/shared/cloud-file-picker/cloud-file-picker.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ bl-cloud-file-picker {
bl-form-field {
flex: 1;

.warning {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you remove this one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm saw at the bottom

color: map-get($warn, 500);
}
.input-container > .input-suffix {
border: none;
padding: 0;
Expand All @@ -29,6 +26,4 @@ bl-cloud-file-picker {
}
}
}


}
13 changes: 9 additions & 4 deletions app/components/data/shared/data.shared.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,20 @@ 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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make those 1 per line instead

CloudFilePickerComponent, CloudFilePickerDialogComponent,
StorageErrorDisplayComponent,
BlobContainerPickerComponent,
StorageAccountPickerComponent,
CloudFilePickerComponent,
CloudFilePickerDialogComponent,
FileGroupPickerComponent,
FileGroupSasComponent,
FileGroupsPickerComponent,
FileOrDirectoryPickerComponent,
JobIdComponent,
StorageAccountPickerComponent,
StorageErrorDisplayComponent,
];

@NgModule({
Expand Down
8 changes: 0 additions & 8 deletions app/components/data/shared/file-group-picker/file-group-picker.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion app/components/data/shared/file-group-sas/file-group-sas.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<bl-form-field>
<input type="text" blInput [formControl]="value" [placeholder]="label">
<bl-button type="round" [action]="generateSasToken" [disabled]="!containerId" title="Generate SAS"><i class="fa fa-refresh"></i></bl-button>
<bl-hint *ngIf="hint" align="end">
{{hint}}
</bl-hint>
</bl-form-field>
<bl-button type="round" [action]="generateSasToken" [disabled]="!containerId" title="Generate SAS"><i class="fa fa-refresh"></i></bl-button>
1 change: 1 addition & 0 deletions app/components/data/shared/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
1 change: 1 addition & 0 deletions app/components/data/shared/job-id/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./job-id.component";
71 changes: 71 additions & 0 deletions app/components/data/shared/job-id/job-id.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
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 { JobService } from "app/services";

import "./job-id.scss";

@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 },
],
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add changeDetection OnPush, we should be doing this for all the new components we write(And when we edit one try to update it to have onPush)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, will make sure to add this whenever i am pottering about in the code.

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that should probably be a validator so it prevent the form from being submitted

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah i noticed this just before i left last night. Seems that none of the advanced components use validators, so file-in-file-group also passes validation if you add a rubbish filename.

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you might want to add changeDetector.markForCheck() here now that it is OnPush or it might not get detected

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, cheers. This will probably change somewhat when i look to use the validator.

this.value.setErrors({
notUnique: true,
});
},
error: () => {
this.warning = false;
this.value.setErrors({});
},
});
}
}
10 changes: 10 additions & 0 deletions app/components/data/shared/job-id/job-id.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<bl-form-field>
<input type="text" blInput [formControl]="value" [placeholder]="label">
<bl-hint class="warning" *ngIf="warning">
The job ID has already been used, please choose a unique job ID
</bl-hint>
<bl-hint *ngIf="hint" align="end">
{{hint}}
</bl-hint>
</bl-form-field>

7 changes: 7 additions & 0 deletions app/components/data/shared/job-id/job-id.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@import "app/styles/variables";

bl-job-id {
bl-form-field {
flex: 1;
}
}
1 change: 1 addition & 0 deletions app/components/market/submit/market-application.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions app/components/market/submit/parameter-input/parameter-input.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
[label]="parameter.name" class="form-element" [hint]="parameter.description" [wildcards]="parameter.wildcards">
</bl-cloud-file-picker>
</div>
<div *ngSwitchCase="NcjParameterExtendedType.jobId">
<bl-job-id [formControl]="parameterValue" [label]="parameter.name" class="form-element" [hint]="parameter.description">
</bl-job-id>
</div>
<div *ngSwitchCase="NcjParameterExtendedType.dropDown">
<bl-form-field>
<bl-select [formControl]="parameterValue" [placeholder]="parameter.name">
Expand Down
15 changes: 15 additions & 0 deletions app/components/market/submit/parameter-input/parameter-input.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
}
3 changes: 1 addition & 2 deletions app/components/market/submit/submit-ncj-template.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down