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.

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

@@ -27,7 +27,7 @@ export class TargetedDataCache<TParams, TEntity extends Record<any>> {
      * Return the key of the cache associated to the given params
      */
     public getCacheKey(params: TParams) {
-        return this._options.key!(params);
+        return this._options.key?.call(null, params);
     }
 
     public getCache(params: TParams): DataCache<TEntity> {

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

@@ -0,0 +1,92 @@
+import { HttpClientTestingModule } from "@angular/common/http/testing";
+import { Component, DebugElement } from "@angular/core";
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { FormControl, FormsModule, ReactiveFormsModule } from "@angular/forms";
+import { By } from "@angular/platform-browser";
+import { BrowserDynamicTestingModule }
+    from "@angular/platform-browser-dynamic/testing";
+import { RouterTestingModule } from "@angular/router/testing";
+import { GlobalStorage, USER_SERVICE, UserConfigurationService }
+    from "@batch-flask/core";
+import {
+    I18nTestingModule,
+    MockGlobalStorage,
+    MockUserConfigurationService
+} from "@batch-flask/core/testing";
+import { ElectronTestingModule } from "@batch-flask/electron/testing";
+import { ButtonsModule, DialogService, FormModule, SelectModule }
+    from "@batch-flask/ui";
+import { AuthService, BatchExplorerService } from "app/services";
+import { BehaviorSubject } from "rxjs";
+import { FileGroupPickerComponent } from "./file-group-picker.component";
+import { FileGroupPickerModule } from "./file-group-picker.module";
+
+@Component({
+    template: `<bl-file-group-picker [formControl]="control"></bl-file-group-picker>`,
+})
+class TestComponent {
+    public control = new FormControl();
+}
+
+describe("FileGroupPickerComponent", () => {
+    let testComponent: TestComponent;
+    let component: FileGroupPickerComponent;
+    let fixture: ComponentFixture<TestComponent>;
+    let de: DebugElement;
+
+    const userServiceSpy = { currentUser: new BehaviorSubject(null) };
+
+    beforeEach(async () => {
+        await TestBed.configureTestingModule({
+            imports: [
+                FormModule,
+                FormsModule,
+                ReactiveFormsModule,
+                SelectModule,
+                RouterTestingModule,
+                I18nTestingModule,
+                HttpClientTestingModule,
+                ButtonsModule,
+                ElectronTestingModule,
+                BrowserDynamicTestingModule,
+                FileGroupPickerModule
+            ],
+            declarations: [TestComponent],
+            providers: [
+                { provide: BatchExplorerService, useValue: {} },
+                { provide: UserConfigurationService, useValue:
+                    new MockUserConfigurationService({}) },
+                { provide: AuthService, useValue: userServiceSpy },
+                { provide: GlobalStorage, useValue: new MockGlobalStorage() },
+                { provide: USER_SERVICE, useValue: userServiceSpy },
+                { provide: DialogService, useValue: {} },
+            ]
+        }).compileComponents();
+
+        fixture = TestBed.createComponent(TestComponent);
+        testComponent = fixture.componentInstance;
+        de = fixture.debugElement.query(By.css("bl-file-group-picker"));
+        component = fixture.debugElement.query(By.css("bl-file-group-picker"))
+            .componentInstance;
+        fixture.detectChanges();
+    });
+
+    it("should pick an existing value", () => {
+        const testValue = 'existingValue';
+        component.fileGroupPicked(testValue);
+        fixture.detectChanges();
+        expect(component.value.value).toEqual(testValue);
+    });
+
+    it("should create a new file group on null value", () => {
+        const spy = spyOn(component, "createFileGroup");
+        const testValue = null;
+        component.fileGroupPicked(testValue);
+        fixture.detectChanges();
+        expect(spy).toHaveBeenCalledOnce();
+    });
+
+    afterEach(() => {
+        fixture.destroy();
+    });
+});

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

@@ -140,18 +140,25 @@ export class FileGroupPickerComponent implements ControlValueAccessor, OnInit, O
         return null;
     }
 
-    public createFileGroup(dropdownValue: string) {
-        if (!dropdownValue) {
-            const dialog = this.dialogService.open(FileGroupCreateFormComponent);
-            dialog.afterClosed().subscribe((activity?: Activity) => {
-                const newFileGroupName = dialog.componentInstance.getCurrentValue().name;
-                this.value.setValue(this.fileGroupService.addFileGroupPrefix(newFileGroupName));
-                this.changeDetector.markForCheck();
-                this._uploadActivity.next(activity);
-            });
+    public fileGroupPicked(value: string) {
+        if (value) {
+            this.writeValue(value);
+            this.changeDetector.markForCheck();
+        } else {
+            this.createFileGroup();
         }
     }
 
+    public createFileGroup() {
+        const dialog = this.dialogService.open(FileGroupCreateFormComponent);
+        dialog.afterClosed().subscribe((activity?: Activity) => {
+            const newFileGroupName = dialog.componentInstance.getCurrentValue().name;
+            this.value.setValue(this.fileGroupService.addFileGroupPrefix(newFileGroupName));
+            this.changeDetector.markForCheck();
+            this._uploadActivity.next(activity);
+        });
+    }
+
     public trackFileGroup(_: number, fileGroup: BlobContainer) {
         return fileGroup.id;
     }

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

@@ -8,11 +8,12 @@
     <bl-hint *ngIf="hint" align="end">
         {{hint}}
     </bl-hint>
-    <bl-select placeholder="File group" [filterable]="true" (change)="createFileGroup($event)">
+    <bl-select placeholder="File group" [filterable]="true" (change)="fileGroupPicked($event)">
         <bl-option value="" label="+ {{'file-group-picker.create' | i18n}}"></bl-option>
         <bl-option
             *ngFor="let fileGroup of fileGroups;trackBy: trackFileGroup"
-            [value]="fileGroup">
+            [value]="fileGroup.name"
+            [label]="fileGroup.name">
         </bl-option>
     </bl-select>
 </bl-form-field>

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

@@ -38,9 +38,12 @@ export class NcjParameterWrapper {
     }
 
     private _computeDefaultValue() {
-        if (this.param.defaultValue) {
-            this.defaultValue = this.param.defaultValue;
+        let defaultValue = this.param.defaultValue;
+        if (typeof defaultValue === "string" &&
+            defaultValue.toLowerCase().trim() === "none") {
+            defaultValue = "";
         }
+        this.defaultValue = this.param.defaultValue = defaultValue;
     }
 
     private _computeDescription() {

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

@@ -41,11 +41,10 @@ export class ParameterInputComponent implements ControlValueAccessor, OnChanges,
         this._subs.push(this.parameterValue.valueChanges.pipe(
             distinctUntilChanged(),
         ).subscribe((query: string) => {
-                if (this._propagateChange) {
-                    this._propagateChange(query);
-                }
-            }),
-        );
+            if (this._propagateChange) {
+                this._propagateChange(query);
+            }
+        }));
     }
 
     public ngOnChanges(changes) {

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

@@ -347,7 +347,7 @@ export class SubmitNcjTemplateComponent implements OnInit, OnChanges, OnDestroy
             let validator = Validators.required;
             if (exists(template.parameters[key].defaultValue)) {
                 defaultValue = String(template.parameters[key].defaultValue);
-                if (template.parameters[key].defaultValue === "") {
+                if (defaultValue.trim() === "") {
                     validator = null;
                 }
             }

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

@@ -6,7 +6,7 @@ import { I18nTestingModule } from "@batch-flask/core/testing";
 import { SelectComponent, SelectModule } from "@batch-flask/ui";
 import { FormModule } from "@batch-flask/ui/form";
 import { ContainerConfigurationAttributes, ContainerType } from "app/models";
-import { ContainerConfigurationDto } from "app/models/dtos";
+import { ContainerConfigurationDto, ContainerRegistryDto } from "app/models/dtos";
 import { ContainerConfigurationPickerComponent } from "./container-configuration-picker.component";
 import { ContainerImagesPickerComponent } from "./images-picker/container-images-picker.component";
 import { ContainerRegistryPickerComponent } from "./registry-picker/container-registry-picker.component";
@@ -97,7 +97,7 @@ describe("ContainerConfigurationPickerComponent", () => {
             type: ContainerType.DockerCompatible,
             containerImageNames: [],
             containerRegistries: [
-                { username: "foo", password: "pass123!", registryServer: "https://bar.com" },
+                { username: "foo", password: "pass123!", registryServer: "https://bar.com" } as ContainerRegistryDto,
             ],
         }));
     });

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

@@ -1,4 +1,4 @@
-import { ContainerType } from "app/models";
+import { ContainerRegistry, ContainerType } from "app/models";
 import { List } from "immutable";
 import { Pool } from "./pool";
 
@@ -13,7 +13,7 @@ describe("Pool Model", () => {
                             username: "abc",
                             password: "foo",
                             registryServer: "hub.docker.com",
-                        },
+                        } as ContainerRegistry,
                     ],
                 },
             },

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

@@ -15,7 +15,7 @@ export interface BlobContainerAttributes {
 /**
  * Class for displaying blob container information.
  */
-@Model()
+@Model("BlobContainer")
 export class BlobContainer extends Record<BlobContainerAttributes> implements NavigableRecord {
     // container name
     @Prop() public id: string;

desktop/src/app/services/storage/blob-storage-client-proxy.ts

@@ -173,8 +173,13 @@ export class BlobStorageClientProxy implements StorageClient {
         containerName: string,
         options?: blob.RequestOptions
     ): Promise<blob.GetContainerPropertiesResult> {
-        return this.remote.send(storageIpc.getContainerProperties,
+        const result = await this.remote.send(storageIpc.getContainerProperties,
             { ...this.storageInfo, containerName, options });
+
+        // The container name is not returned by the API, but we add it here,
+        // since it's used by the UI model
+        result.data.name =  containerName;
+        return result;
     }
 
     /**

desktop/src/app/services/storage/models/storage-blob.ts

@@ -1,7 +1,7 @@
-import { BlobUploadCommonResponse, ContainerGetPropertiesResponse, CommonOptions, ContainerItem } from "@azure/storage-blob";
+import { BlobUploadCommonResponse, CommonOptions, ContainerGetPropertiesResponse } from "@azure/storage-blob";
 import { Model, Prop, Record } from "@batch-flask/core";
-import { SharedAccessPolicy } from "./shared-access-policy";
 import { BlobContainer } from "app/models";
+import { SharedAccessPolicy } from "./shared-access-policy";
 
 // Placeholder; we don't use any options to storage-blob API requests
 export type RequestOptions = Partial<CommonOptions>;
@@ -111,7 +111,7 @@ export interface StorageBlobResult<T> {
     continuationToken?: string;
 }
 
-export type ListContainersResult = StorageBlobResult<ContainerItem[]>;
+export type ListContainersResult = StorageBlobResult<BlobContainer[]>;
 export type GetBlobPropertiesResult = StorageBlobResult<BlobProperties>;
 export type ListBlobsResult = StorageBlobResult<BlobItem[]>;
 export type GetContainerPropertiesResult =

desktop/src/app/services/storage/storage-container.service.ts

@@ -72,8 +72,11 @@ export class StorageContainerService {
                     continuationToken,
                     { maxResults: options && options.maxResults });
 
-                response.data.map(x => x.storageAccountId = params.storageAccountId);
-                return response;
+                const containers = response.data.map((container: BlobContainer) => {
+                    container.name = container.id;
+                    return container;
+                });
+                return { data: containers };
             },
             logIgnoreError: storageIgnoredErrors,
         });