Merge pull request #2673 from alecmerdler/fix-manage-trigger

Fix Manage Trigger Undefined Errors

static/js/directives/ui/linear-workflow/linear-workflow-section.component.spec.ts

@@ -1,34 +1,35 @@
 import { LinearWorkflowSectionComponent } from './linear-workflow-section.component';
 import { LinearWorkflowComponent } from './linear-workflow.component';
 import { SimpleChanges } from 'ng-metadata/core';
+import { Mock } from 'ts-mocks';
 import Spy = jasmine.Spy;
 
 
 describe("LinearWorkflowSectionComponent", () => {
   var component: LinearWorkflowSectionComponent;
-  var parentMock: LinearWorkflowComponent;
+  var parentMock: Mock<LinearWorkflowComponent>;
 
   beforeEach(() => {
-    parentMock = new LinearWorkflowComponent();
+    parentMock = new Mock<LinearWorkflowComponent>();
-    component = new LinearWorkflowSectionComponent(parentMock);
+    component = new LinearWorkflowSectionComponent(parentMock.Object);
+    component.sectionId = "mysection";
   });
 
   describe("ngOnInit", () => {
 
     it("calls parent component to add itself as a section", () => {
-      var addSectionSpy: Spy = spyOn(parentMock, "addSection").and.returnValue(null);
+      parentMock.setup(mock => mock.addSection).is((section) => null);
       component.ngOnInit();
 
-      expect(addSectionSpy.calls.argsFor(0)[0]).toBe(component);
+      expect((<Spy>parentMock.Object.addSection).calls.argsFor(0)[0]).toBe(component);
     });
   });
 
   describe("ngOnChanges", () => {
-    var onSectionInvalidSpy: Spy;
     var changesObj: SimpleChanges;
 
     beforeEach(() => {
-      onSectionInvalidSpy = spyOn(parentMock, "onSectionInvalid").and.returnValue(null);
+      parentMock.setup(mock => mock.onSectionInvalid).is((section) => null);
       changesObj = {
         sectionValid: {
           currentValue: true,
@@ -46,51 +47,58 @@ describe("LinearWorkflowSectionComponent", () => {
     it("does nothing if 'sectionValid' input not changed", () => {
       component.ngOnChanges({});
 
-      expect(onSectionInvalidSpy).not.toHaveBeenCalled();
+      expect((<Spy>parentMock.Object.onSectionInvalid)).not.toHaveBeenCalled();
+    });
+
+    it("does nothing if 'sectionValid' input's previous value is falsy", () => {
+      changesObj['sectionValid'].previousValue = null;
+      component.ngOnChanges(changesObj);
+
+      expect((<Spy>parentMock.Object.onSectionInvalid)).not.toHaveBeenCalled();
     });
 
     it("does nothing if 'sectionValid' input is true", () => {
       component.ngOnChanges(changesObj);
 
-      expect(onSectionInvalidSpy).not.toHaveBeenCalled();
+      expect((<Spy>parentMock.Object.onSectionInvalid)).not.toHaveBeenCalled();
     });
 
     it("calls parent method to inform that section is invalid if 'sectionValid' input changed to false", () => {
+      changesObj['sectionValid'].previousValue = true;
       changesObj['sectionValid'].currentValue = false;
       component.ngOnChanges(changesObj);
 
-      expect(onSectionInvalidSpy.calls.argsFor(0)[0]).toEqual(component.sectionId);
+      expect((<Spy>parentMock.Object.onSectionInvalid).calls.argsFor(0)[0]).toEqual(component.sectionId);
     });
 
     it("calls parent method to go to next section if 'skipSection' input is true and is current section", () => {
       delete changesObj['sectionValid'];
-      const onNextSectionSpy: Spy = spyOn(parentMock, 'onNextSection').and.returnValue(null);
+      parentMock.setup(mock => mock.onNextSection).is(() => null);
       component.isCurrentSection = true;
       component.ngOnChanges(changesObj);
 
-      expect(onNextSectionSpy).toHaveBeenCalled();
+      expect(<Spy>parentMock.Object.onNextSection).toHaveBeenCalled();
     });
   });
 
   describe("onSubmitSection", () => {
-    var onNextSectionSpy: Spy;
 
     beforeEach(() => {
-      onNextSectionSpy = spyOn(parentMock, "onNextSection").and.returnValue(null);
+      parentMock.setup(mock => mock.onNextSection).is(() => null);
     });
 
     it("does nothing if section is invalid", () => {
       component.sectionValid = false;
       component.onSubmitSection();
 
-      expect(onNextSectionSpy).not.toHaveBeenCalled();
+      expect(<Spy>parentMock.Object.onNextSection).not.toHaveBeenCalled();
     });
 
     it("calls parent method to go to next section if section is valid", () => {
       component.sectionValid = true;
       component.onSubmitSection();
 
-      expect(onNextSectionSpy).toHaveBeenCalled();
+      expect(<Spy>parentMock.Object.onNextSection).toHaveBeenCalled();
     });
   });
 });

static/js/directives/ui/linear-workflow/linear-workflow-section.component.ts

@@ -24,7 +24,7 @@ export class LinearWorkflowSectionComponent implements OnChanges, OnInit {
   constructor(@Host() @Inject(LinearWorkflowComponent) private parent: LinearWorkflowComponent) {
 
   }
-  
+
   public ngOnInit(): void {
     if (!this.skipSection) {
       this.parent.addSection(this);
@@ -34,7 +34,7 @@ export class LinearWorkflowSectionComponent implements OnChanges, OnInit {
   public ngOnChanges(changes: SimpleChanges): void {
     switch (Object.keys(changes)[0]) {
       case 'sectionValid':
-        if (!changes['sectionValid'].currentValue && this.parent) {
+        if (changes['sectionValid'].previousValue && !changes['sectionValid'].currentValue && this.parent) {
           this.parent.onSectionInvalid(this.sectionId);
         }
         break;

static/js/directives/ui/manage-trigger/manage-trigger.component.html

@@ -6,7 +6,7 @@
                              section-id="namespace"
                              section-title="::{{ 'Select ' + $ctrl.namespaceTitle }}"
                              section-valid="$ctrl.local.selectedNamespace"
-                             skip-section="::$ctrl.githost == 'custom-git'">
+                             skip-section="$ctrl.githost == 'custom-git'">
       <div class="col-lg-7 col-md-7 col-sm-12 main-col"
            ng-if="$ctrl.local.namespaces">
         <h3>Select {{ ::$ctrl.namespaceTitle }}</h3>
@@ -72,7 +72,7 @@
                              section-id="repo"
                              section-title="Select Repository"
                              section-valid="$ctrl.local.selectedRepository.full_name"
-                             skip-section="::$ctrl.githost == 'custom-git'">
+                             skip-section="$ctrl.githost == 'custom-git'">
       <div class="col-lg-7 col-md-7 col-sm-12 main-col"
            ng-if="$ctrl.local.repositories">
         <h3>Select Repository</h3>
@@ -164,7 +164,7 @@
                              section-id="repo"
                              section-title="Git Repository"
                              section-valid="$ctrl.local.selectedRepository.full_name"
-                             skip-section="::$ctrl.githost != 'custom-git'">
+                             skip-section="$ctrl.githost != 'custom-git'">
 
       <div class="col-lg-7 col-md-7 col-sm-12 main-col">
         <h3>Enter repository</h3>

static/js/directives/ui/manage-trigger/manage-trigger.component.spec.ts

@@ -31,7 +31,7 @@ describe("ManageTriggerComponent", () => {
     component.trigger = {id: "2cac6317-754e-47d4-88d3-2a50b3f09ee3", service: "github"};
   });
 
-  describe("ngOnInit", () => {
+  describe("ngOnChanges", () => {
 
     beforeEach(() => {
       apiServiceMock.setup(mock => mock.listTriggerBuildSourceNamespaces).is(() => Promise.resolve({}));
@@ -40,7 +40,7 @@ describe("ManageTriggerComponent", () => {
     });
 
     it("sets default values for config and selected namespace", () => {
-      component.ngOnInit();
+      component.ngOnChanges({});
 
       expect(component.config).toEqual({});
       expect(component.local.selectedNamespace).toBe(null);

static/js/directives/ui/manage-trigger/manage-trigger.component.ts

@@ -1,4 +1,4 @@
-import { Input, Output, Component, Inject, EventEmitter, OnInit } from 'ng-metadata/core';
+import { Input, Output, Component, Inject, EventEmitter, OnChanges, SimpleChanges } from 'ng-metadata/core';
 import * as moment from 'moment';
 import { Local, Trigger, TriggerConfig, Repository, Namespace } from '../../../types/common.types';
 import { ContextChangeEvent } from '../context-path-select/context-path-select.component';
@@ -12,7 +12,7 @@ import { PathChangeEvent } from '../dockerfile-path-select/dockerfile-path-selec
   selector: 'manage-trigger',
   templateUrl: '/static/js/directives/ui/manage-trigger/manage-trigger.component.html'
 })
-export class ManageTriggerComponent implements OnInit {
+export class ManageTriggerComponent implements OnChanges {
 
   @Input('<') public githost: string = 'custom-git';
   @Input('<') public repository: Repository;
@@ -43,26 +43,28 @@ export class ManageTriggerComponent implements OnInit {
 
   }
 
-  public ngOnInit(): void {
+  public ngOnChanges(changes: SimpleChanges): void {
-    this.config = this.trigger.config || {};
+    if (this.githost && this.repository && this.trigger) {
-    this.namespaceTitle = 'organization';
+      this.config = this.trigger.config || {};
-    this.local.selectedNamespace = null;
+      this.namespaceTitle = 'organization';
-    if (this.githost != 'custom-git') {
+      this.local.selectedNamespace = null;
-      this.loadNamespaces();
+      if (this.githost != 'custom-git') {
-    }
+        this.loadNamespaces();
+      }
 
-    // FIXME: Need to have watchers here because cor-table doesn't have ng-change functionality yet
+      // FIXME (Alec 5/26/17): Need to have watchers here because cor-table doesn't have ng-change functionality yet
-    this.$scope.$watch(() => this.local.selectedNamespace, (namespace: Namespace) => {
+      this.$scope.$watch(() => this.local.selectedNamespace, (namespace: Namespace) => {
-      if (namespace) {
+        if (namespace) {
-        this.loadRepositories(namespace);
+          this.loadRepositories(namespace);
-      }
+        }
-    });
+      });
-    this.$scope.$watch(() => this.local.selectedRepository, (selectedRepository: Repository) => {
+      this.$scope.$watch(() => this.local.selectedRepository, (selectedRepository: Repository) => {
-      if (selectedRepository && this.githost != 'custom-git') {
+        if (selectedRepository && this.githost != 'custom-git') {
-        this.loadRepositoryRefs(selectedRepository);
+          this.loadRepositoryRefs(selectedRepository);
-        this.loadDockerfileLocations(selectedRepository);
+          this.loadDockerfileLocations(selectedRepository);
-      }
+        }
-    });
+      });
+    }
   }
 
   public getTriggerIcon(): any {
@@ -120,20 +122,21 @@ export class ManageTriggerComponent implements OnInit {
             return this.apiService.getRobots(this.repository.namespace, null, {'permissions': true});
           } else {
             this.local.triggerAnalysis = Object.assign({}, resp);
-            this.buildOrderedRobotAccounts();
           }
         })
         .catch((error) => {
           this.apiService.errorDisplay('Could not analyze trigger');
         })
         .then((resp) => {
-          this.local.triggerAnalysis = {
+          if (resp) {
-            status: 'publicbase',
+            this.local.triggerAnalysis = {
-            is_admin: true,
+              status: 'publicbase',
-            robots: resp.robots,
+              is_admin: true,
-            name: this.repository.name,
+              robots: resp.robots,
-            namespace: this.repository.namespace
+              name: this.repository.name,
-          };
+              namespace: this.repository.namespace
+            };
+          }
           this.buildOrderedRobotAccounts();
         })
         .catch((error) => {

static/partials/trigger-setup.html

@@ -38,7 +38,7 @@
       </div> <!-- /state = activated -->
 
       <!-- state = managing or activating -->
-      <div ng-if="state == 'managing' || state == 'activating'"
+      <div ng-if="(state == 'managing' || state == 'activating') && trigger"
            ng-class="{'activating': state == 'activating'}">
         <manage-trigger githost="trigger.service"
                         trigger="trigger"