Skip to content

Commit 0c452bc

Browse files
gingigingi
committed
Fixes inoperable file group controls
Bug introduced in #2745: No handler for a file group that has changed. Also restores the picker option labels. Backfills container names on certain API calls because they are not returned, but are needed by various UI flows.
1 parent cf6126f commit 0c452bc

13 files changed

Lines changed: 141 additions & 31 deletions

File tree

‎desktop/src/@batch-flask/core/data/targeted-data-cache/targeted-data-cache.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class TargetedDataCache<TParams, TEntity extends Record<any>> {
2727
* Return the key of the cache associated to the given params
2828
*/
2929
public getCacheKey(params: TParams) {
30-
return this._options.key!(params);
30+
return this._options.key?.call(null, params);
3131
}
3232

3333
public getCache(params: TParams): DataCache<TEntity> {

‎desktop/src/app/components/data/shared/file-group-picker/file-group-picker.component.spec.ts‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import { HttpClientTestingModule } from "@angular/common/http/testing";
2+
import { Component, DebugElement } from "@angular/core";
3+
import { ComponentFixture, TestBed } from "@angular/core/testing";
4+
import { FormControl, FormsModule, ReactiveFormsModule } from "@angular/forms";
5+
import { By } from "@angular/platform-browser";
6+
import { BrowserDynamicTestingModule }
7+
from "@angular/platform-browser-dynamic/testing";
8+
import { RouterTestingModule } from "@angular/router/testing";
9+
import { GlobalStorage, USER_SERVICE, UserConfigurationService }
10+
from "@batch-flask/core";
11+
import {
12+
I18nTestingModule,
13+
MockGlobalStorage,
14+
MockUserConfigurationService
15+
} from "@batch-flask/core/testing";
16+
import { ElectronTestingModule } from "@batch-flask/electron/testing";
17+
import { ButtonsModule, DialogService, FormModule, SelectModule }
18+
from "@batch-flask/ui";
19+
import { AuthService, BatchExplorerService } from "app/services";
20+
import { BehaviorSubject } from "rxjs";
21+
import { FileGroupPickerComponent } from "./file-group-picker.component";
22+
import { FileGroupPickerModule } from "./file-group-picker.module";
23+
24+
@Component({
25+
template: `<bl-file-group-picker [formControl]="control"></bl-file-group-picker>`,
26+
})
27+
class TestComponent {
28+
public control = new FormControl();
29+
}
30+
31+
describe("FileGroupPickerComponent", () => {
32+
let testComponent: TestComponent;
33+
let component: FileGroupPickerComponent;
34+
let fixture: ComponentFixture<TestComponent>;
35+
let de: DebugElement;
36+
37+
const userServiceSpy = { currentUser: new BehaviorSubject(null) };
38+
39+
beforeEach(async () => {
40+
await TestBed.configureTestingModule({
41+
imports: [
42+
FormModule,
43+
FormsModule,
44+
ReactiveFormsModule,
45+
SelectModule,
46+
RouterTestingModule,
47+
I18nTestingModule,
48+
HttpClientTestingModule,
49+
ButtonsModule,
50+
ElectronTestingModule,
51+
BrowserDynamicTestingModule,
52+
FileGroupPickerModule
53+
],
54+
declarations: [TestComponent],
55+
providers: [
56+
{ provide: BatchExplorerService, useValue: {} },
57+
{ provide: UserConfigurationService, useValue:
58+
new MockUserConfigurationService({}) },
59+
{ provide: AuthService, useValue: userServiceSpy },
60+
{ provide: GlobalStorage, useValue: new MockGlobalStorage() },
61+
{ provide: USER_SERVICE, useValue: userServiceSpy },
62+
{ provide: DialogService, useValue: {} },
63+
]
64+
}).compileComponents();
65+
66+
fixture = TestBed.createComponent(TestComponent);
67+
testComponent = fixture.componentInstance;
68+
de = fixture.debugElement.query(By.css("bl-file-group-picker"));
69+
component = fixture.debugElement.query(By.css("bl-file-group-picker"))
70+
.componentInstance;
71+
fixture.detectChanges();
72+
});
73+
74+
it("should pick an existing value", () => {
75+
const testValue = 'existingValue';
76+
component.fileGroupPicked(testValue);
77+
fixture.detectChanges();
78+
expect(component.value.value).toEqual(testValue);
79+
});
80+
81+
it("should create a new file group on null value", () => {
82+
const spy = spyOn(component, "createFileGroup");
83+
const testValue = null;
84+
component.fileGroupPicked(testValue);
85+
fixture.detectChanges();
86+
expect(spy).toHaveBeenCalledOnce();
87+
});
88+
89+
afterEach(() => {
90+
fixture.destroy();
91+
});
92+
});

‎desktop/src/app/components/data/shared/file-group-picker/file-group-picker.component.ts‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -140,18 +140,25 @@ export class FileGroupPickerComponent implements ControlValueAccessor, OnInit, O
140140
return null;
141141
}
142142

143-
public createFileGroup(dropdownValue: string) {
144-
if (!dropdownValue) {
145-
const dialog = this.dialogService.open(FileGroupCreateFormComponent);
146-
dialog.afterClosed().subscribe((activity?: Activity) => {
147-
const newFileGroupName = dialog.componentInstance.getCurrentValue().name;
148-
this.value.setValue(this.fileGroupService.addFileGroupPrefix(newFileGroupName));
149-
this.changeDetector.markForCheck();
150-
this._uploadActivity.next(activity);
151-
});
143+
public fileGroupPicked(value: string) {
144+
if (value) {
145+
this.writeValue(value);
146+
this.changeDetector.markForCheck();
147+
} else {
148+
this.createFileGroup();
152149
}
153150
}
154151

152+
public createFileGroup() {
153+
const dialog = this.dialogService.open(FileGroupCreateFormComponent);
154+
dialog.afterClosed().subscribe((activity?: Activity) => {
155+
const newFileGroupName = dialog.componentInstance.getCurrentValue().name;
156+
this.value.setValue(this.fileGroupService.addFileGroupPrefix(newFileGroupName));
157+
this.changeDetector.markForCheck();
158+
this._uploadActivity.next(activity);
159+
});
160+
}
161+
155162
public trackFileGroup(_: number, fileGroup: BlobContainer) {
156163
return fileGroup.id;
157164
}

‎desktop/src/app/components/data/shared/file-group-picker/file-group-picker.html‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@
88
<bl-hint *ngIf="hint" align="end">
99
{{hint}}
1010
</bl-hint>
11-
<bl-select placeholder="File group" [filterable]="true" (change)="createFileGroup($event)">
11+
<bl-select placeholder="File group" [filterable]="true" (change)="fileGroupPicked($event)">
1212
<bl-option value="" label="+ {{'file-group-picker.create' | i18n}}"></bl-option>
1313
<bl-option
1414
*ngFor="let fileGroup of fileGroups;trackBy: trackFileGroup"
15-
[value]="fileGroup">
15+
[value]="fileGroup.name"
16+
[label]="fileGroup.name">
1617
</bl-option>
1718
</bl-select>
1819
</bl-form-field>

‎desktop/src/app/components/gallery/submit/market-application.model.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,12 @@ export class NcjParameterWrapper {
3838
}
3939

4040
private _computeDefaultValue() {
41-
if (this.param.defaultValue) {
42-
this.defaultValue = this.param.defaultValue;
41+
let defaultValue = this.param.defaultValue;
42+
if (typeof defaultValue === "string" &&
43+
defaultValue.toLowerCase().trim() === "none") {
44+
defaultValue = "";
4345
}
46+
this.defaultValue = this.param.defaultValue = defaultValue;
4447
}
4548

4649
private _computeDescription() {

‎desktop/src/app/components/gallery/submit/parameter-input/parameter-input.component.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,10 @@ export class ParameterInputComponent implements ControlValueAccessor, OnChanges,
4141
this._subs.push(this.parameterValue.valueChanges.pipe(
4242
distinctUntilChanged(),
4343
).subscribe((query: string) => {
44-
if (this._propagateChange) {
45-
this._propagateChange(query);
46-
}
47-
}),
48-
);
44+
if (this._propagateChange) {
45+
this._propagateChange(query);
46+
}
47+
}));
4948
}
5049

5150
public ngOnChanges(changes) {

‎desktop/src/app/components/gallery/submit/submit-ncj-template.component.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ export class SubmitNcjTemplateComponent implements OnInit, OnChanges, OnDestroy
347347
let validator = Validators.required;
348348
if (exists(template.parameters[key].defaultValue)) {
349349
defaultValue = String(template.parameters[key].defaultValue);
350-
if (template.parameters[key].defaultValue === "") {
350+
if (defaultValue.trim() === "") {
351351
validator = null;
352352
}
353353
}

‎desktop/src/app/components/pool/action/add/container-configuration-picker/container-configuration-picker.component.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { I18nTestingModule } from "@batch-flask/core/testing";
66
import { SelectComponent, SelectModule } from "@batch-flask/ui";
77
import { FormModule } from "@batch-flask/ui/form";
88
import { ContainerConfigurationAttributes, ContainerType } from "app/models";
9-
import { ContainerConfigurationDto } from "app/models/dtos";
9+
import { ContainerConfigurationDto, ContainerRegistryDto } from "app/models/dtos";
1010
import { ContainerConfigurationPickerComponent } from "./container-configuration-picker.component";
1111
import { ContainerImagesPickerComponent } from "./images-picker/container-images-picker.component";
1212
import { ContainerRegistryPickerComponent } from "./registry-picker/container-registry-picker.component";
@@ -97,7 +97,7 @@ describe("ContainerConfigurationPickerComponent", () => {
9797
type: ContainerType.DockerCompatible,
9898
containerImageNames: [],
9999
containerRegistries: [
100-
{ username: "foo", password: "pass123!", registryServer: "https://bar.com" },
100+
{ username: "foo", password: "pass123!", registryServer: "https://bar.com" } as ContainerRegistryDto,
101101
],
102102
}));
103103
});

‎desktop/src/app/models/azure-batch/pool/pool.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ContainerType } from "app/models";
1+
import { ContainerRegistry, ContainerType } from "app/models";
22
import { List } from "immutable";
33
import { Pool } from "./pool";
44

@@ -13,7 +13,7 @@ describe("Pool Model", () => {
1313
username: "abc",
1414
password: "foo",
1515
registryServer: "hub.docker.com",
16-
},
16+
} as ContainerRegistry,
1717
],
1818
},
1919
},

‎desktop/src/app/models/blob-container.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export interface BlobContainerAttributes {
1515
/**
1616
* Class for displaying blob container information.
1717
*/
18-
@Model()
18+
@Model("BlobContainer")
1919
export class BlobContainer extends Record<BlobContainerAttributes> implements NavigableRecord {
2020
// container name
2121
@Prop() public id: string;

0 commit comments

Comments
 (0)