Fix: Custom image show the correct os when using windows images (#1481)

app/components/account/details/account-details.html

@@ -80,7 +80,7 @@ <h2>Pool status</h2>
                                 </bl-column>
                                 <bl-column name="state" [defaultWidth]="30">
                                     <div *blHeadCellDef>State</div>
-                                    <div *blCellDef="let pool"><i aria-hidden="true" class="fa fa-{{pool.osIconName()}}"></i></div>
+                                    <div *blCellDef="let pool"><bl-pool-os-icon [pool]="pool"></bl-pool-os-icon></div>
                                 </bl-column>
                                 <bl-column name="nodes" [defaultWidth]="60">
                                     <div *blCellDef="let pool"><bl-pool-nodes-preview [pool]="pool"></bl-pool-nodes-preview></div>

app/components/pool/base/pool-base.module.ts

@@ -3,16 +3,20 @@ import { FormsModule, ReactiveFormsModule } from "@angular/forms";
 import { BrowserModule } from "@angular/platform-browser";
 import { RouterModule } from "@angular/router";
 import { MaterialModule } from "@batch-flask/core";
-
 import { BaseModule } from "@batch-flask/ui";
 import { AppPackagePickerComponent } from "./app-packages/app-package-picker.component";
 import { PoolNodesPreviewComponent } from "./pool-nodes-preview.component";
+import { PoolOsIconComponent } from "./pool-os-icon";
 
-const components = [AppPackagePickerComponent, PoolNodesPreviewComponent];
+const publicComponents = [
+    AppPackagePickerComponent,
+    PoolNodesPreviewComponent,
+    PoolOsIconComponent,
+];
 
 @NgModule({
-    declarations: components,
-    exports: components,
+    declarations: publicComponents,
+    exports: publicComponents,
     imports: [
         BrowserModule, MaterialModule, RouterModule, BaseModule, FormsModule,
         ReactiveFormsModule,

app/components/pool/base/pool-os-icon/index.ts

@@ -0,0 +1 @@
+export * from "./pool-os-icon.component";

app/components/pool/base/pool-os-icon/pool-os-icon.component.spec.ts

@@ -0,0 +1,138 @@
+import { Component, DebugElement } from "@angular/core";
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { MatTooltip, MatTooltipModule } from "@angular/material";
+import { By } from "@angular/platform-browser";
+import { Pool } from "app/models";
+import { PoolOsIconComponent } from "./pool-os-icon.component";
+
+const ubuntuPool = new Pool({
+    id: "ubuntu-pool",
+    virtualMachineConfiguration: {
+        imageReference: {
+            offer: "UbuntuServer",
+            publisher: "Canonical",
+            sku: "16.04-LTS",
+            version: "latest",
+        },
+        nodeAgentSKUId: "batch.node.ubuntu 16.04",
+    },
+});
+
+const windowsServerPool = new Pool({
+    id: "windows-server-pool",
+    virtualMachineConfiguration: {
+        imageReference: {
+            offer: "WindowsServer",
+            publisher: "MicrosoftWindowsServer",
+            sku: "2016-Datacenter",
+            version: "latest",
+        },
+        nodeAgentSKUId: "batch.node.windows amd64",
+    },
+});
+
+const windowsCustomImage = new Pool({
+    id: "windows-server-pool",
+    virtualMachineConfiguration: {
+        imageReference: {
+            virtualMachineImageId: "some-windows-image-id",
+        },
+        nodeAgentSKUId: "batch.node.windows amd64",
+    },
+});
+
+const linuxCustomImage = new Pool({
+    id: "windows-server-pool",
+    virtualMachineConfiguration: {
+        imageReference: {
+            virtualMachineImageId: "some-windows-image-id",
+        },
+        nodeAgentSKUId: "batch.node.centos 7",
+    },
+});
+
+@Component({
+    template: `<bl-pool-os-icon [pool]="pool"></bl-pool-os-icon>`,
+})
+class TestComponent {
+    public pool: Pool = ubuntuPool;
+}
+
+describe("PoolOsIconComponent", () => {
+    let fixture: ComponentFixture<TestComponent>;
+    let testComponent: TestComponent;
+    let de: DebugElement;
+    let iconEl: DebugElement;
+
+    beforeEach(() => {
+        TestBed.configureTestingModule({
+            imports: [MatTooltipModule],
+            declarations: [PoolOsIconComponent, TestComponent],
+        });
+        fixture = TestBed.createComponent(TestComponent);
+        testComponent = fixture.componentInstance;
+        de = fixture.debugElement.query(By.css("bl-pool-os-icon"));
+        fixture.detectChanges();
+        iconEl = de.query(By.css(".fa"));
+    });
+
+    describe("when pool is ubuntu", () => {
+        beforeEach(() => {
+            testComponent.pool = ubuntuPool;
+            fixture.detectChanges();
+        });
+
+        it("shows linux icon", () => {
+            expect(iconEl.nativeElement.classList).toContain("fa-linux");
+        });
+
+        it("shows os in tooltip", () => {
+            expect(iconEl.injector.get(MatTooltip).message).toEqual("UbuntuServer 16.04-LTS");
+        });
+    });
+
+    describe("when pool is windows server", () => {
+        beforeEach(() => {
+            testComponent.pool = windowsServerPool;
+            fixture.detectChanges();
+        });
+
+        it("shows windows icon", () => {
+            expect(iconEl.nativeElement.classList).toContain("fa-windows");
+        });
+
+        it("shows os in tooltip", () => {
+            expect(iconEl.injector.get(MatTooltip).message).toEqual("Windows Server 2016-Datacenter");
+        });
+    });
+
+    describe("when pool is custom image(Linux)", () => {
+        beforeEach(() => {
+            testComponent.pool = linuxCustomImage;
+            fixture.detectChanges();
+        });
+
+        it("shows linux icon", () => {
+            expect(iconEl.nativeElement.classList).toContain("fa-linux");
+        });
+
+        it("shows os in tooltip", () => {
+            expect(iconEl.injector.get(MatTooltip).message).toEqual("Custom image (linux)");
+        });
+    });
+
+    describe("when pool is custom image(Windows)", () => {
+        beforeEach(() => {
+            testComponent.pool = windowsCustomImage;
+            fixture.detectChanges();
+        });
+
+        it("shows windows icon", () => {
+            expect(iconEl.nativeElement.classList).toContain("fa-windows");
+        });
+
+        it("shows os in tooltip", () => {
+            expect(iconEl.injector.get(MatTooltip).message).toEqual("Custom image (windows)");
+        });
+    });
+});

app/components/pool/base/pool-os-icon/pool-os-icon.component.ts

@@ -0,0 +1,32 @@
+import { ChangeDetectionStrategy, Component, Input, OnChanges } from "@angular/core";
+import { OSType, Pool } from "app/models";
+
+import "./pool-os-icon.scss";
+
+@Component({
+    selector: "bl-pool-os-icon",
+    templateUrl: "pool-os-icon.html",
+    changeDetection: ChangeDetectionStrategy.OnPush,
+})
+export class PoolOsIconComponent implements OnChanges {
+    @Input() public pool: Pool;
+    @Input() public tooltipPosition: string = "right";
+
+    public icon: string;
+
+    public ngOnChanges(inputs) {
+        if (inputs.pool) {
+            this.icon = this._computeIcon();
+        }
+    }
+
+    private _computeIcon() {
+        if (this.pool.osType === OSType.Windows) {
+            return "fa-windows";
+        } else if (this.pool.osType === OSType.Linux)  {
+            return "fa-linux";
+        } else {
+            return "fa-cloud";
+        }
+    }
+}

app/components/pool/base/pool-os-icon/pool-os-icon.html

@@ -0,0 +1 @@
+<i class="fa {{icon}}" [matTooltip]="pool.osName" [matTooltipPosition]="tooltipPosition"></i>

app/components/pool/browse/pool-list.component.ts

@@ -17,6 +17,8 @@ import { ListView } from "app/services/core";
 import { ComponentUtils } from "app/utils";
 import { DeletePoolTask, PoolCommands } from "../action";
 
+import "./pool-list.scss";
+
 @Component({
     selector: "bl-pool-list",
     templateUrl: "pool-list.html",

app/components/pool/browse/pool-list.html

@@ -11,6 +11,7 @@
             </bl-quick-list-item-status>
             <div bl-quick-list-item-title>{{pool.id}}</div>
             <div bl-quick-list-item-field>
+                <bl-pool-os-icon [pool]="pool"></bl-pool-os-icon>
                 {{pool.state}}
                 <bl-tags [tags]="pool?.tags" [maxTags]="3"></bl-tags>
             </div>
@@ -32,7 +33,7 @@
 
         <bl-column name="id">
             <div *blHeadCellDef>ID</div>
-            <div *blCellDef="let pool">{{pool.id}}</div>
+            <div *blCellDef="let pool"><bl-pool-os-icon [pool]="pool"></bl-pool-os-icon>{{pool.id}}</div>
         </bl-column>
 
         <bl-column name="state">

app/components/pool/browse/pool-list.scss

@@ -0,0 +1,5 @@
+bl-pool-list {
+    bl-pool-os-icon {
+        margin-right: 4px;
+    }
+}

app/components/pool/details/pool-details.component.ts

@@ -1,4 +1,4 @@
-import { Component, OnDestroy, OnInit } from "@angular/core";
+import { ChangeDetectionStrategy, ChangeDetectorRef, Component, OnDestroy, OnInit } from "@angular/core";
 import { ActivatedRoute, Router } from "@angular/router";
 import { autobind } from "@batch-flask/core";
 import { List } from "immutable";
@@ -16,6 +16,7 @@ import "./pool-details.scss";
 @Component({
     selector: "bl-pool-details",
     templateUrl: "pool-details.html",
+    changeDetection: ChangeDetectionStrategy.OnPush,
     providers: [PoolCommands],
 })
 export class PoolDetailsComponent implements OnInit, OnDestroy {
@@ -43,6 +44,7 @@ export class PoolDetailsComponent implements OnInit, OnDestroy {
 
     constructor(
         public commands: PoolCommands,
+        private changeDetector: ChangeDetectorRef,
         private router: Router,
         private activatedRoute: ActivatedRoute,
         private batchLabs: BatchLabsService,
@@ -52,6 +54,7 @@ export class PoolDetailsComponent implements OnInit, OnDestroy {
         this.data = this.poolService.view();
         this.data.item.subscribe((pool) => {
             this.pool = pool;
+            this.changeDetector.markForCheck();
             this._updatePrice();
         });
 
@@ -102,6 +105,7 @@ export class PoolDetailsComponent implements OnInit, OnDestroy {
     private _updatePrice() {
         if (!this.pool) {
             this.estimatedCost = "-";
+            this.changeDetector.markForCheck();
             return;
         }
         this.pricingService.computePoolPrice(this.pool).subscribe((cost) => {
@@ -110,6 +114,7 @@ export class PoolDetailsComponent implements OnInit, OnDestroy {
             } else {
                 this.estimatedCost = `${cost.unit} ${NumberUtils.pretty(cost.total)}`;
             }
+            this.changeDetector.markForCheck();
         });
     }
 }

app/components/pool/details/pool-details.html

@@ -6,7 +6,7 @@
             <div summaryDetails>
                 <div>Last resized {{poolDecorator.lastResized}}</div>
                 <div>
-                    <i class="fa fa-{{poolDecorator.poolOsIcon}}" aria-hidden="true"></i> {{poolDecorator.poolOs}}
+                    <bl-pool-os-icon [pool]="pool"></bl-pool-os-icon> {{poolDecorator.poolOs}}
                 </div>
             </div>
             <bl-tags summaryTags [tags]="pool.tags" [editable]="true" [save]="updateTags" noTagsMessage="No tags."></bl-tags>

app/models/decorators/pool-decorator.ts

@@ -39,7 +39,6 @@ export class PoolDecorator extends DecoratorBase<Pool> {
     public vmSize: string;
     public poolEndpointConfiguration: PoolEndpointConfigurationDecorator;
     public poolOs: string;
-    public poolOsIcon: string;
     public lastResized: string;
     public userAccounts: string;
     public dedicatedNodes: string;
@@ -82,7 +81,6 @@ export class PoolDecorator extends DecoratorBase<Pool> {
             pool.currentLowPriorityNodes, pool.targetLowPriorityNodes);
 
         this.poolOs = this._computePoolOs();
-        this.poolOsIcon = this._computePoolOsIcon(this.poolOs);
 
         this.cloudServiceConfiguration =
             new CloudServiceConfigurationDecorator(pool.cloudServiceConfiguration || {} as any, this.poolOs);
@@ -102,11 +100,7 @@ export class PoolDecorator extends DecoratorBase<Pool> {
     }
 
     private _computePoolOs(): string {
-        return this.pool.osName();
-    }
-
-    private _computePoolOsIcon(os): string {
-        return this.pool.osIconName();
+        return this.pool.osName;
     }
 
     private _decorateUserAccount(user: UserAccount) {

app/models/pool.ts

@@ -12,6 +12,11 @@ import { TaskSchedulingPolicy } from "./task-scheduling-policy";
 import { UserAccount, UserAccountAttributes } from "./user-account";
 import { VirtualMachineConfiguration, VirtualMachineConfigurationAttributes } from "./virtual-machine-configuration";
 
+export enum OSType {
+    Windows = "windows",
+    Linux = "linux",
+}
+
 export interface PoolAttributes {
     allocationState: string;
     allocationStateTransitionTime: Date;
@@ -127,26 +132,18 @@ export class Pool extends Record<PoolAttributes> implements NavigableRecord  {
      */
     public currentNodes: number;
 
-    private _osName: string;
-    private _osIcon: string;
+    public readonly osName: string;
+    public readonly osType: OSType;
 
     constructor(data: Partial<PoolAttributes> = {}) {
         super(data);
         this.tags = ModelUtils.tagsFromMetadata(this.metadata);
-        this._osName = PoolUtils.getOsName(this);
-        this._osIcon = PoolUtils.getComputePoolOsIcon(this._osName);
+        this.osName = PoolUtils.getOsName(this);
+        this.osType = PoolUtils.getOsType(this);
         this.targetNodes = this.targetDedicatedNodes + this.targetLowPriorityNodes;
         this.currentNodes = this.currentDedicatedNodes + this.currentLowPriorityNodes;
     }
 
-    public osIconName(): string {
-        return this._osIcon;
-    }
-
-    public osName(): string {
-        return this._osName;
-    }
-
     public get routerLink(): string[] {
         return ["/pools", this.id];
     }

app/utils/pool-utils.spec.ts

@@ -144,14 +144,14 @@ describe("PoolUtils", () => {
                 },
             });
 
-            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsName(cs2Pool))).toBe("windows");
-            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsName(cs3Pool))).toBe("windows");
-            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsName(cs4Pool))).toBe("windows");
-            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsName(cs5Pool))).toBe("windows");
-            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsName(vm1Pool))).toBe("linux");
-            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsName(vm2Pool))).toBe("windows");
-            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsName(vm3Pool))).toBe("windows");
-            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsName(vm4Pool))).toBe("linux");
+            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsType(cs2Pool))).toBe("windows");
+            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsType(cs3Pool))).toBe("windows");
+            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsType(cs4Pool))).toBe("windows");
+            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsType(cs5Pool))).toBe("windows");
+            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsType(vm1Pool))).toBe("linux");
+            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsType(vm2Pool))).toBe("windows");
+            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsType(vm3Pool))).toBe("windows");
+            expect(PoolUtils.getComputePoolOsIcon(PoolUtils.getOsType(vm4Pool))).toBe("linux");
         });
     });